Skip to content

ASoC: soc-acpi-intel-ptl-match: add ptl_cs42l43_agg_l3_cs35l56_l12_ghost_rt722 #5754

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bardliao wants to merge 1 commit into
base: topic/sof-dev
Choose a base branch
from
Open

ASoC: soc-acpi-intel-ptl-match: add ptl_cs42l43_agg_l3_cs35l56_l12_ghost_rt722 #5754

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions sound/soc/intel/common/soc-acpi-intel-ptl-match.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,25 @@ static const struct snd_soc_acpi_adr_device cs42l43_2_adr[] = {
}
};

static const struct snd_soc_acpi_adr_device cs42l43_3_agg_rt722_ghost_adr[] = {
Copy link
Copy Markdown

@charleskeepax charleskeepax May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agg should go here.

{
.adr = 0x00033001FA424301ull,
.num_endpoints = ARRAY_SIZE(cs42l43_amp_spkagg_endpoints),
.endpoints = cs42l43_amp_spkagg_endpoints,
Copy link
Copy Markdown
Collaborator Author

@bardliao bardliao May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will wait for the test result since I am not quite sure if cs42l43 amp is aggregated with cs35l56s. @charleskeepax FYI

Copy link
Copy Markdown
Collaborator Author

@bardliao bardliao May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@charleskeepax Is there a way that we can support the same codec/amp combination but with different endpoints? To be more specific, cs42l43 with and without amp aggregated. The endpoint will be skipped by the machine driver, but the topology was selected before we check is the endpoint present.

Copy link
Copy Markdown

@charleskeepax charleskeepax May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah its not aggregated here, I think the host thinks it is because of the smart amps bits on the rt722. If we use match tables you can't tell the difference between two systems with identical parts one of which aggregates.

I kinda wonder if this match table approach is the best way to go, it makes me nervous because I don't like mixing match tables and function topologies. Feels like it introduces chances of things matching when they should actually have used function topologies. Two other approaches that I can think of:

  1. We could disable the realtek codec using a SSDT something like:
DefinitionBlock ("", "SSDT", 2, "Test", "rt722", 0x00001000)
{
    External (_SB.PC00.HDAS.IDA.SNDW.SWD0, DeviceObj)
    Scope (\_SB.PC00.HDAS.IDA.SNDW.SWD0)
    {
        Method (_STA, 0, NotSerialized)
        {
            Return (Zero)
        }
    }
}

That should make the kernel ignore the device. Leaves the kernel code the cleanest and since this is an ACPI problem perhaps its the right place to fix it? Does mean people have to install and SSDT to make the laptop work though.... so dunno.

  1. Could we do some sort of quirk thing, like the stuff in sdca_device.c? Get the core to ignore the rt722 just for this system.

Copy link
Copy Markdown
Collaborator Author

@bardliao bardliao May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@charleskeepax If the first approach is to make the change in the BIOS, I am afraid the ASIS's answer is NO. We reported this issue to ASUS and asked them to fix it in the BIOS, but there is no issue on Windows and they don't want to take any risk to change the BIOS. But, yes, I agree fixing it in the BIOS is the right and the best way to fix the issue. Or are you talking about adding a new SSDT to overwrite the existing SSDT?
Regarding the second approach , there is a similar quirk in drivers/soundwire/dmi-quirks.c. However, I guess this project is not the only product that has the issue. I think it is common is the ODM use a new audio solution but didn't remove the previous codec information from the BIOS.
Back to the PR, on second thought, we can actually use function topology with acpi match table as long as we set .sof_tplg_filename = "sof-ptl-dummy.tplg". Let's give it a try.

Copy link
Copy Markdown

@charleskeepax charleskeepax May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should be loadable as a separate SSDT it adds a disable to the rt722 ACPI, that could be loaded through either initfs or efivars. It has the advantage of fixing broken ACPI by fixing it, but end users would need to make sure they loaded that SSDT which isn't ideal.

To some extent adding matches has the same problem if this is going to be a common problem then we are likely going to keep having to add new matches. Although I guess maybe one match hits more SKUs than a quirk which would hit a single SKU. I guess maybe if the "dummy" topology causes us to still load function topologies maybe its ok, but still nervous about mixing matches and function topologies.

Copy link
Copy Markdown
Collaborator Author

@bardliao bardliao May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about adding a module parameter to allow user ignoring a specific codec?

Copy link
Copy Markdown

@charleskeepax charleskeepax May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah that's probably worse. With the kernel fix (as in this patch) the user doesn't need to do anything, and the ACPI fix is probably the "correct" fix. Using a module parameter, we are both not doing the "correct" thing and the user needs to do something, so worst of both.
I think I am happy with the kernel fix as long as we are relatively sure we are not going to hit weird interactions with function topologies. If this "dummy" thing triggers the function stuff we should be good. Although I am not too familiar with those mechanisms so to some extent trusting you guys on that.

Copy link
Copy Markdown

@charleskeepax charleskeepax May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should make a new array without the aggregated speaker this system doesn't have an aggregated speaker.

Copy link
Copy Markdown

@Pegorim Pegorim May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let me know if you need another test. @bardliao

Copy link
Copy Markdown
Collaborator Author

@bardliao bardliao May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should make a new array without the aggregated speaker this system doesn't have an aggregated speaker.

For this device, yes, but the smart amp function will not be in the SDCA function and the speaker endpoint will be skipped, right? INHO, we could keep the speaker endpoint here and let the DisCo table decide whether the endpoint should present or not. That's what we did for other configurations, right?

.name_prefix = "cs42l43"
},
/*
* To handle cases where the rt722 codec is listed in the BIOS while the
* hardware is not physically present.
*/
{
.adr = 0x000330025d072201ull, /* Ghost rt722 */
Comment thread
bardliao marked this conversation as resolved.
Comment on lines +221 to +226
.num_endpoints = 0,
.endpoints = NULL,
.name_prefix = "ghost"
Copy link
Copy Markdown
Collaborator

@ujfalusi ujfalusi May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gah :(

}
};

static const struct snd_soc_acpi_adr_device cs42l43_3_agg_adr[] = {
{
.adr = 0x00033001FA424301ull,
Expand All @@ -235,6 +254,36 @@ static const struct snd_soc_acpi_adr_device cs35l56_2_lr_adr[] = {
}
};

static const struct snd_soc_acpi_adr_device cs35l56_2_2amp_adr[] = {
{
.adr = 0x00023001fa355601ull,
.num_endpoints = 1,
.endpoints = &spk_1_endpoint,
.name_prefix = "AMP1"
},
{
.adr = 0x00023101fa355601ull,
.num_endpoints = 1,
.endpoints = &spk_2_endpoint,
.name_prefix = "AMP2"
}
};

static const struct snd_soc_acpi_adr_device cs35l56_1_2amp_adr[] = {
{
.adr = 0x00013201fa355601ull,
.num_endpoints = 1,
.endpoints = &spk_3_endpoint,
.name_prefix = "AMP3"
},
{
.adr = 0x00013301fa355601ull,
.num_endpoints = 1,
.endpoints = &spk_4_endpoint,
.name_prefix = "AMP4"
}
};

static const struct snd_soc_acpi_adr_device cs35l56_1_3amp_adr[] = {
{
.adr = 0x00013001fa355601ull,
Expand Down Expand Up @@ -394,6 +443,25 @@ static const struct snd_soc_acpi_adr_device rt1320_3_group2_adr[] = {
}
};

static const struct snd_soc_acpi_link_adr ptl_cs42l43_l3_cs35l56_l12_ghost_rt722[] = {
{
.mask = BIT(3),
.num_adr = ARRAY_SIZE(cs42l43_3_agg_rt722_ghost_adr),
.adr_d = cs42l43_3_agg_rt722_ghost_adr,
},
{
.mask = BIT(2),
.num_adr = ARRAY_SIZE(cs35l56_2_2amp_adr),
.adr_d = cs35l56_2_2amp_adr,
},
{
.mask = BIT(1),
.num_adr = ARRAY_SIZE(cs35l56_1_2amp_adr),
.adr_d = cs35l56_1_2amp_adr,
},
{}
};

static const struct snd_soc_acpi_link_adr ptl_cs42l43_agg_l3_cs35l56_l2[] = {
{
.mask = BIT(3),
Expand Down Expand Up @@ -584,6 +652,13 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_ptl_sdw_machines[] = {
.drv_name = "sof_sdw",
.sof_tplg_filename = "sof-ptl-cs42l43-l2-cs35l56x6-l13.tplg",
},
{
.link_mask = BIT(1) | BIT(2) | BIT(3),
.links = ptl_cs42l43_l3_cs35l56_l12_ghost_rt722,
.drv_name = "sof_sdw",
.sof_tplg_filename = "sof-ptl-dummy.tplg",
Comment thread
bardliao marked this conversation as resolved.
.get_function_tplg_files = sof_sdw_get_tplg_files,
},
{
.link_mask = BIT(0) | BIT(2) | BIT(3),
.links = ptl_rt722_l0_rt1320_l23,
Expand Down
Loading