Re: [PATCH 6/7] ASoC: Intel: soc-acpi-intel-tgl-match: add cs42l43 and cs56l56 support

Richard Fitzgerald
2023-11-27T14:40:50Z

On 27/11/2023 13:34, Peter Ujfalusi wrote:
> From: Bard Liao <yung-chuan.liao@linux.intel.com>
>
> This is a test configuration for UpExtreme.
>
> The codec layout is configured as:
> - Link3: CS42L43 Jack
> - Link0: 2x CS35L56 Speaker
> - Link1: 2x CS35L56 Speaker
>
> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
> ---
> .../intel/common/soc-acpi-intel-tgl-match.c | 78 +++++++++++++++++++
> 1 file changed, 78 insertions(+)
>
> diff --git a/sound/soc/intel/common/soc-acpi-intel-tgl-match.c b/sound/soc/intel/common/soc-acpi-intel-tgl-match.c
> index 5804926c8b56..49834bffa50c 100644
> --- a/sound/soc/intel/common/soc-acpi-intel-tgl-match.c
> +++ b/sound/soc/intel/common/soc-acpi-intel-tgl-match.c
> @@ -41,6 +41,20 @@ static const struct snd_soc_acpi_endpoint spk_r_endpoint = {
> .group_id = 1,
> };
>
> +static const struct snd_soc_acpi_endpoint spk_2_endpoint = {
> + .num = 0,
> + .aggregated = 1,
> + .group_position = 2,
> + .group_id = 1,
> +};
> +
> +static const struct snd_soc_acpi_endpoint spk_3_endpoint = {
> + .num = 0,
> + .aggregated = 1,
> + .group_position = 3,
> + .group_id = 1,
> +};
> +
> static const struct snd_soc_acpi_endpoint rt712_endpoints[] = {
> {
> .num = 0,
> @@ -400,6 +414,64 @@ static const struct snd_soc_acpi_link_adr tgl_712_only[] = {
> {}
> };
>
> +static const struct snd_soc_acpi_adr_device cs42l43_3_adr[] = {
> + {
> + .adr = 0x00033001FA424301ull,
> + .num_endpoints = 1,
> + .endpoints = &single_endpoint,
> + .name_prefix = "cs42l43"
> + }
> +};
> +
> +static const struct snd_soc_acpi_adr_device cs35l56_0_adr[] = {
> + {
> + .adr = 0x00003301FA355601ull,
> + .num_endpoints = 1,
> + .endpoints = &spk_r_endpoint,

Assigning CS35L56 to "left" or "right" endpoints might be confusing.
All CS35L56 in a system receive both left and right channels and by
default they output a mono-mix of left+right.

The left/right of an amp is determined by the firmware file (.bin) that
is loaded and the current settings of the "Posture" ALSA control. So
this amp might be the left channel after a .bin is loaded.

It would be better to have generic names for the endpoint that don't
imply position, for example:

group1_spk1_endpoint
group1_spk2_endpoint
group1_spk3_endpoint
group1_spk4_endpoint.

> + .name_prefix = "cs35l56-8"

Can these prefixes be "AMPn" to match the CS35L41, CS35L51 and
CS35L56-hda driver? This prefix is used to find the matching firmware
files and our naming convention for these has been cs35lxx-xxxx-ampn

Is there anything that depends on the prefixes being "cs35l56-n" ?

> + },
> + {
> + .adr = 0x00003201FA355601ull,
> + .num_endpoints = 1,
> + .endpoints = &spk_3_endpoint,
> + .name_prefix = "cs35l56-7"
> + }
> +};
> +
> +static const struct snd_soc_acpi_adr_device cs35l56_1_adr[] = {
> + {
> + .adr = 0x00013701FA355601ull,
> + .num_endpoints = 1,
> + .endpoints = &spk_l_endpoint,
> + .name_prefix = "cs35l56-1"
> + },
> + {
> + .adr = 0x00013601FA355601ull,
> + .num_endpoints = 1,
> + .endpoints = &spk_2_endpoint,
> + .name_prefix = "cs35l56-2"
> + }
> +};
> +
> +static const struct snd_soc_acpi_link_adr tgl_cs42l43_cs35l56[] = {
> + {
> + .mask = BIT(3),
> + .num_adr = ARRAY_SIZE(cs42l43_3_adr),
> + .adr_d = cs42l43_3_adr,
> + },
> + {
> + .mask = BIT(0),
> + .num_adr = ARRAY_SIZE(cs35l56_0_adr),
> + .adr_d = cs35l56_0_adr,
> + },
> + {
> + .mask = BIT(1),
> + .num_adr = ARRAY_SIZE(cs35l56_1_adr),
> + .adr_d = cs35l56_1_adr,
> + },
> + {}
> +};
> +