[PATCH 0/7] ASoC: Intel: Soundwire related board and match updates

[PATCH 0/7] ASoC: Intel: Soundwire related board and match updates

[Peter Ujfalusi]

Hi,

A small update for SDW machine support:
Small fixes for sof_sdw machine driver
Support for rt722
New TGL/MTL and LNL match for new configurations

Regards,
Peter
---
Bard Liao (1):
  ASoC: Intel: soc-acpi-intel-tgl-match: add cs42l43 and cs56l56 support

Chao Song (4):
  ASoC: Intel: sof_sdw: remove unused function declaration
  ASoC: Intel: sof_sdw: Add rt722 support
  ASoC: Intel: soc-acpi: add Gen4.1 SDCA board support for LNL RVP
  ASoC: Intel: soc-acpi-intel-mtl-match: Add rt722 support

Mac Chiang (1):
  ASoC: Intel: soc-acpi: rt713+rt1316, no sdw-dmic config

Peter Ujfalusi (1):
  ASoC: Intel: sof_sdw: Make use of dev_err_probe()

 sound/soc/intel/boards/Kconfig                |  1 +
 sound/soc/intel/boards/Makefile               |  3 +-
 sound/soc/intel/boards/sof_sdw.c              | 32 +++++-
 sound/soc/intel/boards/sof_sdw_common.h       | 18 ++--
 sound/soc/intel/boards/sof_sdw_rt722_sdca.c   | 97 +++++++++++++++++++
 .../boards/sof_sdw_rt_sdca_jack_common.c      |  8 ++
 .../intel/common/soc-acpi-intel-lnl-match.c   | 71 ++++++++++++++
 .../intel/common/soc-acpi-intel-mtl-match.c   | 74 ++++++++++++++
 .../intel/common/soc-acpi-intel-tgl-match.c   | 78 +++++++++++++++
 9 files changed, 374 insertions(+), 8 deletions(-)
 create mode 100644 sound/soc/intel/boards/sof_sdw_rt722_sdca.c

-- 
2.43.0

[PATCH 1/7] ASoC: Intel: sof_sdw: Make use of dev_err_probe()

[Peter Ujfalusi]

The devm_snd_soc_register_card() can return with
-EPROBE_DEFER and in that case the driver should not print
an error message.

Closes: https://github.com/thesofproject/linux/issues/4668
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Chao Song <chao.song@linux.intel.com>
---
 sound/soc/intel/boards/sof_sdw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c
index 1e788859c863..13089182dc17 100644
--- a/sound/soc/intel/boards/sof_sdw.c
+++ b/sound/soc/intel/boards/sof_sdw.c
@@ -1947,7 +1947,7 @@ static int mc_probe(struct platform_device *pdev)
 	/* Register the card */
 	ret = devm_snd_soc_register_card(card->dev, card);
 	if (ret) {
-		dev_err(card->dev, "snd_soc_register_card failed %d\n", ret);
+		dev_err_probe(card->dev, ret, "snd_soc_register_card failed %d\n", ret);
 		mc_dailink_exit_loop(card);
 		return ret;
 	}
-- 
2.43.0

[PATCH 2/7] ASoC: Intel: sof_sdw: remove unused function declaration

[Peter Ujfalusi]

From: Chao Song <chao.song@linux.intel.com>

The functions sof_sdw_rt712_sdca_init() and sof_sdw_rt712_sdca_exit()
declared in header file are never implemented and used, remove them.

Signed-off-by: Chao Song <chao.song@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 sound/soc/intel/boards/sof_sdw_common.h | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/sound/soc/intel/boards/sof_sdw_common.h b/sound/soc/intel/boards/sof_sdw_common.h
index e6b98523b4e7..9528f147b719 100644
--- a/sound/soc/intel/boards/sof_sdw_common.h
+++ b/sound/soc/intel/boards/sof_sdw_common.h
@@ -138,12 +138,6 @@ int sof_sdw_rt_sdca_jack_init(struct snd_soc_card *card,
 int sof_sdw_rt_sdca_jack_exit(struct snd_soc_card *card, struct snd_soc_dai_link *dai_link);
 
 /* RT712-SDCA support */
-int sof_sdw_rt712_sdca_init(struct snd_soc_card *card,
-			    const struct snd_soc_acpi_link_adr *link,
-			    struct snd_soc_dai_link *dai_links,
-			    struct sof_sdw_codec_info *info,
-			    bool playback);
-int sof_sdw_rt712_sdca_exit(struct snd_soc_card *card, struct snd_soc_dai_link *dai_link);
 int sof_sdw_rt712_spk_init(struct snd_soc_card *card,
 			   const struct snd_soc_acpi_link_adr *link,
 			   struct snd_soc_dai_link *dai_links,
-- 
2.43.0

[PATCH 3/7] ASoC: Intel: sof_sdw: Add rt722 support

[Peter Ujfalusi]

From: Chao Song <chao.song@linux.intel.com>

RT722 is a multi-function codec which supports headset,
amp, and dmic functions. Each function provides a DAI
which can be used in different dailinks.

The RT711 supports up to 3 SoundWire lanes, but that
should not have any impact in the machine driver:
the lanes are allocated and controlled by the manager
and bandwidth allocation algorithm.

Signed-off-by: Chao Song <chao.song@linux.intel.com>
Reviewed-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>
---
 sound/soc/intel/boards/Kconfig                |  1 +
 sound/soc/intel/boards/Makefile               |  3 +-
 sound/soc/intel/boards/sof_sdw.c              | 30 ++++++
 sound/soc/intel/boards/sof_sdw_common.h       | 12 +++
 sound/soc/intel/boards/sof_sdw_rt722_sdca.c   | 97 +++++++++++++++++++
 .../boards/sof_sdw_rt_sdca_jack_common.c      |  8 ++
 6 files changed, 150 insertions(+), 1 deletion(-)
 create mode 100644 sound/soc/intel/boards/sof_sdw_rt722_sdca.c

diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig
index 9e427f00deac..99ebe48216ea 100644
--- a/sound/soc/intel/boards/Kconfig
+++ b/sound/soc/intel/boards/Kconfig
@@ -686,6 +686,7 @@ config SND_SOC_INTEL_SOUNDWIRE_SOF_MACH
 	select SND_SOC_RT712_SDCA_DMIC_SDW
 	select SND_SOC_RT715_SDW
 	select SND_SOC_RT715_SDCA_SDW
+	select SND_SOC_RT722_SDCA_SDW
 	select SND_SOC_RT1308_SDW
 	select SND_SOC_RT1308
 	select SND_SOC_RT1316_SDW
diff --git a/sound/soc/intel/boards/Makefile b/sound/soc/intel/boards/Makefile
index 943bf8b80e01..bbf796a5f7ba 100644
--- a/sound/soc/intel/boards/Makefile
+++ b/sound/soc/intel/boards/Makefile
@@ -41,9 +41,10 @@ snd-soc-sof-sdw-objs += sof_sdw.o				\
 			sof_sdw_rt5682.o sof_sdw_rt700.o	\
 			sof_sdw_rt711.o sof_sdw_rt_sdca_jack_common.o	\
 			sof_sdw_rt712_sdca.o sof_sdw_rt715.o	\
-			sof_sdw_rt715_sdca.o sof_sdw_dmic.o	\
+			sof_sdw_rt715_sdca.o sof_sdw_rt722_sdca.o	\
 			sof_sdw_cs42l42.o sof_sdw_cs42l43.o	\
 			sof_sdw_cs_amp.o			\
+			sof_sdw_dmic.o				\
 			sof_sdw_hdmi.o
 obj-$(CONFIG_SND_SOC_INTEL_SOF_RT5682_MACH) += snd-soc-sof_rt5682.o
 obj-$(CONFIG_SND_SOC_INTEL_SOF_CS42L42_MACH) += snd-soc-sof_cs42l42.o
diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c
index 13089182dc17..a7cb9ec1f151 100644
--- a/sound/soc/intel/boards/sof_sdw.c
+++ b/sound/soc/intel/boards/sof_sdw.c
@@ -860,6 +860,36 @@ static struct sof_sdw_codec_info codec_info_list[] = {
 		},
 		.dai_num = 1,
 	},
+	{
+		.part_id = 0x722,
+		.version_id = 3,
+		.dais = {
+			{
+				.direction = {true, true},
+				.dai_name = "rt722-sdca-aif1",
+				.dai_type = SOF_SDW_DAI_TYPE_JACK,
+				.dailink = {SDW_JACK_OUT_DAI_ID, SDW_JACK_IN_DAI_ID},
+				.init = sof_sdw_rt_sdca_jack_init,
+				.exit = sof_sdw_rt_sdca_jack_exit,
+			},
+			{
+				.direction = {true, false},
+				.dai_name = "rt722-sdca-aif2",
+				.dai_type = SOF_SDW_DAI_TYPE_AMP,
+				/* No feedback capability is provided by rt722-sdca codec driver*/
+				.dailink = {SDW_AMP_OUT_DAI_ID, SDW_UNUSED_DAI_ID},
+				.init = sof_sdw_rt722_spk_init,
+			},
+			{
+				.direction = {false, true},
+				.dai_name = "rt722-sdca-aif3",
+				.dai_type = SOF_SDW_DAI_TYPE_MIC,
+				.dailink = {SDW_UNUSED_DAI_ID, SDW_DMIC_DAI_ID},
+				.init = sof_sdw_rt722_sdca_dmic_init,
+			},
+		},
+		.dai_num = 3,
+	},
 	{
 		.part_id = 0x8373,
 		.dais = {
diff --git a/sound/soc/intel/boards/sof_sdw_common.h b/sound/soc/intel/boards/sof_sdw_common.h
index 9528f147b719..f16456945edb 100644
--- a/sound/soc/intel/boards/sof_sdw_common.h
+++ b/sound/soc/intel/boards/sof_sdw_common.h
@@ -183,6 +183,18 @@ int sof_sdw_rt715_sdca_init(struct snd_soc_card *card,
 			    struct sof_sdw_codec_info *info,
 			    bool playback);
 
+/* RT722-SDCA support */
+int sof_sdw_rt722_spk_init(struct snd_soc_card *card,
+			   const struct snd_soc_acpi_link_adr *link,
+			   struct snd_soc_dai_link *dai_links,
+			   struct sof_sdw_codec_info *info,
+			   bool playback);
+int sof_sdw_rt722_sdca_dmic_init(struct snd_soc_card *card,
+				 const struct snd_soc_acpi_link_adr *link,
+				 struct snd_soc_dai_link *dai_links,
+				 struct sof_sdw_codec_info *info,
+				 bool playback);
+
 /* MAXIM codec support */
 int sof_sdw_maxim_init(struct snd_soc_card *card,
 		       const struct snd_soc_acpi_link_adr *link,
diff --git a/sound/soc/intel/boards/sof_sdw_rt722_sdca.c b/sound/soc/intel/boards/sof_sdw_rt722_sdca.c
new file mode 100644
index 000000000000..fe3a2bff95bc
--- /dev/null
+++ b/sound/soc/intel/boards/sof_sdw_rt722_sdca.c
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Copyright (c) 2023 Intel Corporation
+
+/*
+ *  sof_sdw_rt722_sdca - Helpers to handle RT722-SDCA from generic machine driver
+ */
+
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/soundwire/sdw.h>
+#include <linux/soundwire/sdw_type.h>
+#include <sound/control.h>
+#include <sound/soc.h>
+#include <sound/soc-acpi.h>
+#include <sound/soc-dapm.h>
+#include "sof_sdw_common.h"
+
+static const struct snd_soc_dapm_widget rt722_spk_widgets[] = {
+	SND_SOC_DAPM_SPK("Speaker", NULL),
+};
+
+static const struct snd_soc_dapm_route rt722_spk_map[] = {
+	{ "Speaker", NULL, "rt722 SPK" },
+};
+
+static const struct snd_kcontrol_new rt722_spk_controls[] = {
+	SOC_DAPM_PIN_SWITCH("Speaker"),
+};
+
+static int rt722_spk_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_soc_card *card = rtd->card;
+	int ret;
+
+	card->components = devm_kasprintf(card->dev, GFP_KERNEL,
+					  "%s spk:rt722",
+					  card->components);
+	if (!card->components)
+		return -ENOMEM;
+
+	ret = snd_soc_add_card_controls(card, rt722_spk_controls,
+					ARRAY_SIZE(rt722_spk_controls));
+	if (ret) {
+		dev_err(card->dev, "failed to add rt722 spk controls: %d\n", ret);
+		return ret;
+	}
+
+	ret = snd_soc_dapm_new_controls(&card->dapm, rt722_spk_widgets,
+					ARRAY_SIZE(rt722_spk_widgets));
+	if (ret) {
+		dev_err(card->dev, "failed to add rt722 spk widgets: %d\n", ret);
+		return ret;
+	}
+
+	ret = snd_soc_dapm_add_routes(&card->dapm, rt722_spk_map, ARRAY_SIZE(rt722_spk_map));
+	if (ret)
+		dev_err(rtd->dev, "failed to add rt722 spk map: %d\n", ret);
+
+	return ret;
+}
+
+int sof_sdw_rt722_spk_init(struct snd_soc_card *card,
+			   const struct snd_soc_acpi_link_adr *link,
+			   struct snd_soc_dai_link *dai_links,
+			   struct sof_sdw_codec_info *info,
+			   bool playback)
+{
+	dai_links->init = rt722_spk_init;
+
+	return 0;
+}
+
+static int rt722_sdca_dmic_rtd_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_soc_card *card = rtd->card;
+	struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(rtd, 0);
+	struct snd_soc_component *component = codec_dai->component;
+
+	card->components = devm_kasprintf(card->dev, GFP_KERNEL,
+					  "%s mic:%s",
+					  card->components, component->name_prefix);
+	if (!card->components)
+		return -ENOMEM;
+
+	return 0;
+}
+
+int sof_sdw_rt722_sdca_dmic_init(struct snd_soc_card *card,
+				 const struct snd_soc_acpi_link_adr *link,
+				 struct snd_soc_dai_link *dai_links,
+				 struct sof_sdw_codec_info *info,
+				 bool playback)
+{
+	dai_links->init = rt722_sdca_dmic_rtd_init;
+
+	return 0;
+}
diff --git a/sound/soc/intel/boards/sof_sdw_rt_sdca_jack_common.c b/sound/soc/intel/boards/sof_sdw_rt_sdca_jack_common.c
index 65bbcee88d6d..e430be7681d2 100644
--- a/sound/soc/intel/boards/sof_sdw_rt_sdca_jack_common.c
+++ b/sound/soc/intel/boards/sof_sdw_rt_sdca_jack_common.c
@@ -63,6 +63,11 @@ static const struct snd_soc_dapm_route rt713_sdca_map[] = {
 	{ "rt713 MIC2", NULL, "Headset Mic" },
 };
 
+static const struct snd_soc_dapm_route rt722_sdca_map[] = {
+	{ "Headphone", NULL, "rt722 HP" },
+	{ "rt722 MIC2", NULL, "Headset Mic" },
+};
+
 static const struct snd_kcontrol_new rt_sdca_jack_controls[] = {
 	SOC_DAPM_PIN_SWITCH("Headphone"),
 	SOC_DAPM_PIN_SWITCH("Headset Mic"),
@@ -117,6 +122,9 @@ static int rt_sdca_jack_rtd_init(struct snd_soc_pcm_runtime *rtd)
 	} else if (strstr(component->name_prefix, "rt713")) {
 		ret = snd_soc_dapm_add_routes(&card->dapm, rt713_sdca_map,
 					      ARRAY_SIZE(rt713_sdca_map));
+	} else if (strstr(component->name_prefix, "rt722")) {
+		ret = snd_soc_dapm_add_routes(&card->dapm, rt722_sdca_map,
+					      ARRAY_SIZE(rt722_sdca_map));
 	} else {
 		dev_err(card->dev, "%s is not supported\n", component->name_prefix);
 		return -EINVAL;
-- 
2.43.0

[PATCH 4/7] ASoC: Intel: soc-acpi: rt713+rt1316, no sdw-dmic config

[Peter Ujfalusi]

From: Mac Chiang <mac.chiang@intel.com>

This is additional HW board: rt713+rt1316 without rt713-dmic configuration:

SDW0: rt713 audio jack
SDW1: rt1316 spk_amp_l
SDW2: rt1316 spk_amp_r

Signed-off-by: Mac Chiang <mac.chiang@intel.com>
Reviewed-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-mtl-match.c   | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/sound/soc/intel/common/soc-acpi-intel-mtl-match.c b/sound/soc/intel/common/soc-acpi-intel-mtl-match.c
index af4224bff718..2035f561ca50 100644
--- a/sound/soc/intel/common/soc-acpi-intel-mtl-match.c
+++ b/sound/soc/intel/common/soc-acpi-intel-mtl-match.c
@@ -434,6 +434,25 @@ static const struct snd_soc_acpi_link_adr mtl_rt713_l0_rt1316_l12_rt1713_l3[] =
 	{}
 };
 
+static const struct snd_soc_acpi_link_adr mtl_rt713_l0_rt1316_l12[] = {
+	{
+		.mask = BIT(0),
+		.num_adr = ARRAY_SIZE(rt713_0_single_adr),
+		.adr_d = rt713_0_single_adr,
+	},
+	{
+		.mask = BIT(1),
+		.num_adr = ARRAY_SIZE(rt1316_1_group2_adr),
+		.adr_d = rt1316_1_group2_adr,
+	},
+	{
+		.mask = BIT(2),
+		.num_adr = ARRAY_SIZE(rt1316_2_group2_adr),
+		.adr_d = rt1316_2_group2_adr,
+	},
+	{}
+};
+
 static const struct snd_soc_acpi_adr_device mx8363_2_adr[] = {
 	{
 		.adr = 0x000230019F836300ull,
@@ -519,6 +538,12 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_mtl_sdw_machines[] = {
 		.drv_name = "sof_sdw",
 		.sof_tplg_filename = "sof-mtl-rt713-l0-rt1316-l12-rt1713-l3.tplg",
 	},
+	{
+		.link_mask = GENMASK(2, 0),
+		.links = mtl_rt713_l0_rt1316_l12,
+		.drv_name = "sof_sdw",
+		.sof_tplg_filename = "sof-mtl-rt713-l0-rt1316-l12.tplg",
+	},
 	{
 		.link_mask = BIT(3) | BIT(0),
 		.links = mtl_712_only,
-- 
2.43.0

[PATCH 5/7] ASoC: Intel: soc-acpi: add Gen4.1 SDCA board support for LNL RVP

[Peter Ujfalusi]

From: Chao Song <chao.song@linux.intel.com>

This patch adds support for LNL RVP with Realtek
Gen4.1 SDCA codec board, the codec layout is:
  SDW0: RT711 Headphone
  SDW1: RT714 DMIC
  SDW2: RT1316 Speaker
  SDW3: RT1316 Speaker

Signed-off-by: Chao Song <chao.song@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 .../intel/common/soc-acpi-intel-lnl-match.c   | 71 +++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/sound/soc/intel/common/soc-acpi-intel-lnl-match.c b/sound/soc/intel/common/soc-acpi-intel-lnl-match.c
index 9f35b77deb11..5897bb6b28b8 100644
--- a/sound/soc/intel/common/soc-acpi-intel-lnl-match.c
+++ b/sound/soc/intel/common/soc-acpi-intel-lnl-match.c
@@ -22,6 +22,20 @@ static const struct snd_soc_acpi_endpoint single_endpoint = {
 	.group_id = 0,
 };
 
+static const struct snd_soc_acpi_endpoint spk_l_endpoint = {
+	.num = 0,
+	.aggregated = 1,
+	.group_position = 0,
+	.group_id = 1,
+};
+
+static const struct snd_soc_acpi_endpoint spk_r_endpoint = {
+	.num = 0,
+	.aggregated = 1,
+	.group_position = 1,
+	.group_id = 1,
+};
+
 static const struct snd_soc_acpi_adr_device rt711_sdca_0_adr[] = {
 	{
 		.adr = 0x000030025D071101ull,
@@ -31,6 +45,33 @@ static const struct snd_soc_acpi_adr_device rt711_sdca_0_adr[] = {
 	}
 };
 
+static const struct snd_soc_acpi_adr_device rt1316_2_group1_adr[] = {
+	{
+		.adr = 0x000230025D131601ull,
+		.num_endpoints = 1,
+		.endpoints = &spk_l_endpoint,
+		.name_prefix = "rt1316-1"
+	}
+};
+
+static const struct snd_soc_acpi_adr_device rt1316_3_group1_adr[] = {
+	{
+		.adr = 0x000331025D131601ull,
+		.num_endpoints = 1,
+		.endpoints = &spk_r_endpoint,
+		.name_prefix = "rt1316-2"
+	}
+};
+
+static const struct snd_soc_acpi_adr_device rt714_1_adr[] = {
+	{
+		.adr = 0x000130025D071401ull,
+		.num_endpoints = 1,
+		.endpoints = &single_endpoint,
+		.name_prefix = "rt714"
+	}
+};
+
 static const struct snd_soc_acpi_link_adr lnl_rvp[] = {
 	{
 		.mask = BIT(0),
@@ -40,6 +81,30 @@ static const struct snd_soc_acpi_link_adr lnl_rvp[] = {
 	{}
 };
 
+static const struct snd_soc_acpi_link_adr lnl_3_in_1_sdca[] = {
+	{
+		.mask = BIT(0),
+		.num_adr = ARRAY_SIZE(rt711_sdca_0_adr),
+		.adr_d = rt711_sdca_0_adr,
+	},
+	{
+		.mask = BIT(2),
+		.num_adr = ARRAY_SIZE(rt1316_2_group1_adr),
+		.adr_d = rt1316_2_group1_adr,
+	},
+	{
+		.mask = BIT(3),
+		.num_adr = ARRAY_SIZE(rt1316_3_group1_adr),
+		.adr_d = rt1316_3_group1_adr,
+	},
+	{
+		.mask = BIT(1),
+		.num_adr = ARRAY_SIZE(rt714_1_adr),
+		.adr_d = rt714_1_adr,
+	},
+	{}
+};
+
 /* this table is used when there is no I2S codec present */
 struct snd_soc_acpi_mach snd_soc_acpi_intel_lnl_sdw_machines[] = {
 	/* mockup tests need to be first */
@@ -61,6 +126,12 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_lnl_sdw_machines[] = {
 		.drv_name = "sof_sdw",
 		.sof_tplg_filename = "sof-lnl-rt715-rt711-rt1308-mono.tplg",
 	},
+	{
+		.link_mask = GENMASK(3, 0),
+		.links = lnl_3_in_1_sdca,
+		.drv_name = "sof_sdw",
+		.sof_tplg_filename = "sof-lnl-rt711-l0-rt1316-l23-rt714-l1.tplg",
+	},
 	{
 		.link_mask = BIT(0),
 		.links = lnl_rvp,
-- 
2.43.0

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

[Peter Ujfalusi]

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,
+		.name_prefix = "cs35l56-8"
+	},
+	{
+		.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,
+	},
+	{}
+};
+
 static const struct snd_soc_acpi_codecs tgl_max98373_amp = {
 	.num_codecs = 1,
 	.codecs = {"MX98373"}
@@ -494,6 +566,12 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_tgl_sdw_machines[] = {
 		.drv_name = "sof_sdw",
 		.sof_tplg_filename = "sof-tgl-rt715-rt711-rt1308-mono.tplg",
 	},
+	{
+		.link_mask = 0xB,
+		.links = tgl_cs42l43_cs35l56,
+		.drv_name = "sof_sdw",
+		.sof_tplg_filename = "sof-tgl-cs42l43-l3-cs35l56-l01.tplg",
+	},
 	{
 		.link_mask = 0xF, /* 4 active links required */
 		.links = tgl_3_in_1_default,
-- 
2.43.0

[PATCH 7/7] ASoC: Intel: soc-acpi-intel-mtl-match: Add rt722 support

[Peter Ujfalusi]

From: Chao Song <chao.song@linux.intel.com>

This patch adds match table for rt722 codec on link 0.

RT722 is a multi-function codec, three endpoints are
created for its headset, amp and dmic functions.

Signed-off-by: Chao Song <chao.song@linux.intel.com>
Reviewed-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-mtl-match.c   | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/sound/soc/intel/common/soc-acpi-intel-mtl-match.c b/sound/soc/intel/common/soc-acpi-intel-mtl-match.c
index 2035f561ca50..f2c17cee1a5d 100644
--- a/sound/soc/intel/common/soc-acpi-intel-mtl-match.c
+++ b/sound/soc/intel/common/soc-acpi-intel-mtl-match.c
@@ -135,6 +135,31 @@ static const struct snd_soc_acpi_endpoint rt712_endpoints[] = {
 	},
 };
 
+/*
+ * RT722 is a multi-function codec, three endpoints are created for
+ * its headset, amp and dmic functions.
+ */
+static const struct snd_soc_acpi_endpoint rt722_endpoints[] = {
+	{
+		.num = 0,
+		.aggregated = 0,
+		.group_position = 0,
+		.group_id = 0,
+	},
+	{
+		.num = 1,
+		.aggregated = 0,
+		.group_position = 0,
+		.group_id = 0,
+	},
+	{
+		.num = 2,
+		.aggregated = 0,
+		.group_position = 0,
+		.group_id = 0,
+	},
+};
+
 static const struct snd_soc_acpi_endpoint spk_2_endpoint = {
 	.num = 0,
 	.aggregated = 1,
@@ -176,6 +201,15 @@ static const struct snd_soc_acpi_adr_device rt1712_3_single_adr[] = {
 	}
 };
 
+static const struct snd_soc_acpi_adr_device rt722_0_single_adr[] = {
+	{
+		.adr = 0x000030025d072201ull,
+		.num_endpoints = ARRAY_SIZE(rt722_endpoints),
+		.endpoints = rt722_endpoints,
+		.name_prefix = "rt722"
+	}
+};
+
 static const struct snd_soc_acpi_adr_device rt713_0_single_adr[] = {
 	{
 		.adr = 0x000031025D071301ull,
@@ -367,6 +401,15 @@ static const struct snd_soc_acpi_link_adr mtl_rvp[] = {
 	{}
 };
 
+static const struct snd_soc_acpi_link_adr mtl_rt722_only[] = {
+	{
+		.mask = BIT(0),
+		.num_adr = ARRAY_SIZE(rt722_0_single_adr),
+		.adr_d = rt722_0_single_adr,
+	},
+	{}
+};
+
 static const struct snd_soc_acpi_link_adr mtl_3_in_1_sdca[] = {
 	{
 		.mask = BIT(0),
@@ -568,6 +611,12 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_mtl_sdw_machines[] = {
 		.drv_name = "sof_sdw",
 		.sof_tplg_filename = "sof-mtl-rt711-l0-rt1316-l23-rt714-l1.tplg",
 	},
+	{
+		.link_mask = BIT(0),
+		.links = mtl_rt722_only,
+		.drv_name = "sof_sdw",
+		.sof_tplg_filename = "sof-mtl-rt722-l0.tplg",
+	},
 	{
 		.link_mask = BIT(0),
 		.links = mtl_rvp,
-- 
2.43.0

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

[Richard Fitzgerald]

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,
> +	},
> +	{}
> +};
> +

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

[Pierre-Louis Bossart]

>> +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.

That's a problem if the kernel does not know which amplifier is on which
side, no? How would one change the balance if this information is known
only within a binary/opaque firmware?

> 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.

The notion of endpoint is completely half-baked today and the settings
used have no bearing on the behavior and user-experience. I am inches
away from sending a patch that removes all of the endpoint definitions,
we can re-add them if/when we can get the information from platform
firmware.

>> +        .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" ?

IIRC this name_prefix is just used for the codec_conf and hence for
control names/UCM. At some point userspace/driver need to know if amp5
is left or right.

We can certainly align on conventions but the values set in this ACPI
match table will not be used for firmware download - different scope.

>> +    },
>> +    {
>> +        .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,
>> +    },
>> +    {}
>> +};
>> +

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

[Richard Fitzgerald]

On 27/11/2023 17:36, Pierre-Louis Bossart wrote:
> 
>>> +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.
> 
> That's a problem if the kernel does not know which amplifier is on which
> side, no? How would one change the balance if this information is known
> only within a binary/opaque firmware?
> 

SDCA allows the posture (orientation) of amplifiers to be changed at
runtime. CS35L56 is designed as a SDCA device so it doesn't have any
hardwired position. SDCA doesn't define what the posture numbers mean,
they are an integer that is system-specific.

Because SDCA doesn't define the meaning of postures, and an SDCA device
should work with a generic SDCA driver (which obviously wouldn't have
hardcoded knowledge of the system) the speaker positions and postures
are coded into the firmware

It's difficult to say what is "default". For example, if you say that
the default for a tablet is left/right/top/bottom, assuming it is
used in portrait orientation, that would be wrong if the user always
uses it in landscape.

Matching by what amp is on what bus doesn't work well here because two
systems could have the same arrangement of CS35L56 on each bus but use
them for different purposes. So they could both match the "2 on bus 0, 2
on bus 1" table entry, but could be left/right/top/bottom on one device
and left woofer/right woofer/left tweeter/right tweeter on another
device.

I assume that if the system supports rotation there should be something
in the UCM or other userland that manages this. At least, it seems like
it's a UCM problem to decide which speakers are doing what audio.
If Linux-based distros don't have something like that - well, that just
means Linux is behind Windows.

>> 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.
> 
> The notion of endpoint is completely half-baked today and the settings
> used have no bearing on the behavior and user-experience. I am inches
> away from sending a patch that removes all of the endpoint definitions,
> we can re-add them if/when we can get the information from platform
> firmware.
> 
>>> +        .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" ?
> 
> IIRC this name_prefix is just used for the codec_conf and hence for
> control names/UCM. At some point userspace/driver need to know if amp5
> is left or right.
> 
> We can certainly align on conventions but the values set in this ACPI
> match table will not be used for firmware download - different scope.
> 

They are used for our firmware download. Each amp can have its own
unique firmware file. The ALSA prefix is used to identify which firmware
file to load to which amp.

>>> +    },
>>> +    {
>>> +        .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,
>>> +    },
>>> +    {}
>>> +};
>>> +

Re: [PATCH 0/7] ASoC: Intel: Soundwire related board and match updates

[Mark Brown]

On Mon, 27 Nov 2023 15:34:41 +0200, Peter Ujfalusi wrote:
> A small update for SDW machine support:
> Small fixes for sof_sdw machine driver
> Support for rt722
> New TGL/MTL and LNL match for new configurations
> 
> Regards,
> Peter
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/7] ASoC: Intel: sof_sdw: Make use of dev_err_probe()
      commit: 5c0e047ab629bcb5efa94de63fcdc75c9fe69516
[2/7] ASoC: Intel: sof_sdw: remove unused function declaration
      commit: fd8ff49d35f917c65383642c8f8f20656734f3ad
[3/7] ASoC: Intel: sof_sdw: Add rt722 support
      commit: def127feaa8a9d8684ac41139638b079e6e637e2
[4/7] ASoC: Intel: soc-acpi: rt713+rt1316, no sdw-dmic config
      commit: 817178e7674bd8ca35344b2212a3105ed75559e5
[5/7] ASoC: Intel: soc-acpi: add Gen4.1 SDCA board support for LNL RVP
      commit: faca26b6ca90b220cba787ff7c6a05e99528731c
[6/7] ASoC: Intel: soc-acpi-intel-tgl-match: add cs42l43 and cs56l56 support
      (no commit info)
[7/7] ASoC: Intel: soc-acpi-intel-mtl-match: Add rt722 support
      commit: ed99878462ccc143395987faebda33c50529b116

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

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

[Pierre-Louis Bossart]

On 11/28/23 04:31, Richard Fitzgerald wrote:
> On 27/11/2023 17:36, Pierre-Louis Bossart wrote:
>>
>>>> +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.
>>
>> That's a problem if the kernel does not know which amplifier is on which
>> side, no? How would one change the balance if this information is known
>> only within a binary/opaque firmware?
>>
> 
> SDCA allows the posture (orientation) of amplifiers to be changed at
> runtime. CS35L56 is designed as a SDCA device so it doesn't have any
> hardwired position. SDCA doesn't define what the posture numbers mean,
> they are an integer that is system-specific.
> 
> Because SDCA doesn't define the meaning of postures, and an SDCA device
> should work with a generic SDCA driver (which obviously wouldn't have
> hardcoded knowledge of the system) the speaker positions and postures
> are coded into the firmware
> 
> It's difficult to say what is "default". For example, if you say that
> the default for a tablet is left/right/top/bottom, assuming it is
> used in portrait orientation, that would be wrong if the user always
> uses it in landscape.
> 
> Matching by what amp is on what bus doesn't work well here because two
> systems could have the same arrangement of CS35L56 on each bus but use
> them for different purposes. So they could both match the "2 on bus 0, 2
> on bus 1" table entry, but could be left/right/top/bottom on one device
> and left woofer/right woofer/left tweeter/right tweeter on another
> device.

In the absence of any platform firmware information, I am not sure how
we can deal with such systems. The match tables are already hard to
support given that a number of OEMs get the _ADR wrong, the speaker
position is the next-level...

Or did you just volunteer to maintain a DMI quirk table for Cirrus-based
systems :-)

I also bet that at some point the wrong firmware will be loaded on the
wrong amplifiers, that could be fun as well.

> I assume that if the system supports rotation there should be something
> in the UCM or other userland that manages this. At least, it seems like
> it's a UCM problem to decide which speakers are doing what audio.
> If Linux-based distros don't have something like that - well, that just
> means Linux is behind Windows.

SDCA has lots of fancy concepts, posture is one. Last time I checked we
don't have any reports of the hinge angle in Linux so the best we can do
is landscape/portrait, and even that is questionable given that tablets
or detachables have not reached any developers so far. CI automation is
another fun issue, we'll need robotic arms to move the device around and
intelligent alsa-bat-v2 to verify sound levels...

The notion of which speakers do what is something that will clearly take
years to figure out. For now the main issue is to get all parts
connected and basic "loud enough" sound working.

>>> 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.
>>
>> The notion of endpoint is completely half-baked today and the settings
>> used have no bearing on the behavior and user-experience. I am inches
>> away from sending a patch that removes all of the endpoint definitions,
>> we can re-add them if/when we can get the information from platform
>> firmware.
>>
>>>> +        .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" ?
>>
>> IIRC this name_prefix is just used for the codec_conf and hence for
>> control names/UCM. At some point userspace/driver need to know if amp5
>> is left or right.
>>
>> We can certainly align on conventions but the values set in this ACPI
>> match table will not be used for firmware download - different scope.
>>
> 
> They are used for our firmware download. Each amp can have its own
> unique firmware file. The ALSA prefix is used to identify which firmware
> file to load to which amp.

The prefix will only be used when the card is created, specifically for
control names.
The firmware should be selected and downloaded when the device shows up
on the bus.
Card creation and device enumeration/initialization happen on different
timelines, if the machine driver is "blacklisted" or unbound I am not
sure what happens.

There is a dependency between machine driver probe and codec firmware
download that I am not able to follow, can you please elaborate?

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

[Richard Fitzgerald]

On 28/11/2023 15:23, Pierre-Louis Bossart wrote:
> 
> 
> On 11/28/23 04:31, Richard Fitzgerald wrote:
>> On 27/11/2023 17:36, Pierre-Louis Bossart wrote:
>>>
>>>>> +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.
>>>
>>> That's a problem if the kernel does not know which amplifier is on which
>>> side, no? How would one change the balance if this information is known
>>> only within a binary/opaque firmware?
>>>
>>
>> SDCA allows the posture (orientation) of amplifiers to be changed at
>> runtime. CS35L56 is designed as a SDCA device so it doesn't have any
>> hardwired position. SDCA doesn't define what the posture numbers mean,
>> they are an integer that is system-specific.
>>
>> Because SDCA doesn't define the meaning of postures, and an SDCA device
>> should work with a generic SDCA driver (which obviously wouldn't have
>> hardcoded knowledge of the system) the speaker positions and postures
>> are coded into the firmware
>>
>> It's difficult to say what is "default". For example, if you say that
>> the default for a tablet is left/right/top/bottom, assuming it is
>> used in portrait orientation, that would be wrong if the user always
>> uses it in landscape.
>>
>> Matching by what amp is on what bus doesn't work well here because two
>> systems could have the same arrangement of CS35L56 on each bus but use
>> them for different purposes. So they could both match the "2 on bus 0, 2
>> on bus 1" table entry, but could be left/right/top/bottom on one device
>> and left woofer/right woofer/left tweeter/right tweeter on another
>> device.
> 
> In the absence of any platform firmware information, I am not sure how
> we can deal with such systems. The match tables are already hard to
> support given that a number of OEMs get the _ADR wrong, the speaker
> position is the next-level...
> 
> Or did you just volunteer to maintain a DMI quirk table for Cirrus-based
> systems :-)
>

Short answer: "That's SDCA."

I don't think a quirk table is needed. It's just that we can't hardcode
"this speaker is left, that speaker is right". SDCA defers orientation
changes to the amp through the posture control.

If you have a daemon to handle rotation, everything will be fine and
left audio is on your left. Let's say you have a tablet and you hold it
in portrait with left and right correct. You then rotate it 180 degrees,
if the daemon updates the posture control, the amps will swap channels
so left audio is still on your left, and right is still on your right.

If Linux distros don't have any daemon that can handle rotation, then
rotating the tablet 180 degrees is going to give you left and right
audio on the wrong sides. But that's what you'd expect if nothing is
handling rotation, and you'd get the same problem if it was all done
by changing the routing in ALSA controls but there was nothing to
change that routing.

Getting back to my original comment about endpoints. It really doesn't
matter what the endpoint structs are called because all they do is
declare the aggregation. I was only suggesting to maybe avoid names
that imply a specific function. When I said "Confusing" that was
overstating things. A bit misleading perhaps.

> I also bet that at some point the wrong firmware will be loaded on the
> wrong amplifiers, that could be fun as well.
>

Hence using the SSDI + ALSA prefix to qualify the firmware files. We aim
to push out all the firmware to linux-firmware for products we know
about. So far it's worked ok for CS35L41 and CS35L51 - problems with
those have been with incorrect ACPI.

>> I assume that if the system supports rotation there should be something
>> in the UCM or other userland that manages this. At least, it seems like
>> it's a UCM problem to decide which speakers are doing what audio.
>> If Linux-based distros don't have something like that - well, that just
>> means Linux is behind Windows.
> 
> SDCA has lots of fancy concepts, posture is one. Last time I checked we
> don't have any reports of the hinge angle in Linux so the best we can do
> is landscape/portrait, and even that is questionable given that tablets
> or detachables have not reached any developers so far. CI automation is
> another fun issue, we'll need robotic arms to move the device around and
> intelligent alsa-bat-v2 to verify sound levels...
> 
> The notion of which speakers do what is something that will clearly take
> years to figure out. For now the main issue is to get all parts
> connected and basic "loud enough" sound working.
> 

It's still the case that shiny new things on x86 platforms will be
designed around Windows and made to work there. Then Linux has to catch
up with that.

>>>> 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.
>>>
>>> The notion of endpoint is completely half-baked today and the settings
>>> used have no bearing on the behavior and user-experience. I am inches
>>> away from sending a patch that removes all of the endpoint definitions,
>>> we can re-add them if/when we can get the information from platform
>>> firmware.
>>>
>>>>> +        .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" ?
>>>
>>> IIRC this name_prefix is just used for the codec_conf and hence for
>>> control names/UCM. At some point userspace/driver need to know if amp5
>>> is left or right.
>>>
>>> We can certainly align on conventions but the values set in this ACPI
>>> match table will not be used for firmware download - different scope.
>>>
>>
>> They are used for our firmware download. Each amp can have its own
>> unique firmware file. The ALSA prefix is used to identify which firmware
>> file to load to which amp.
> 
> The prefix will only be used when the card is created, specifically for
> control names.
> The firmware should be selected and downloaded when the device shows up
> on the bus.
> Card creation and device enumeration/initialization happen on different
> timelines, if the machine driver is "blacklisted" or unbound I am not
> sure what happens.
> 
> There is a dependency between machine driver probe and codec firmware
> download that I am not able to follow, can you please elaborate?
>

The codec driver has to choose which firmware to load from under
/lib/firmware. It does this using a combination of SSID (to identify the
target product), the ALSA prefix string (to identify which amp) and
in some systems a GPIO on the motherboard to select between different
models of speaker when they have multiple suppliers. This results in a
firmware name like:

cs35l56-<silicon rev>-dsp1-misc-<SSID>[-<SPEAKER MODEL>]-<ALSA PREFIX>

You can see this if you look in the linux-firmware repo under cirrus/
for cs35l41 firmware files (though the ALSA PREFIX section in those
cases is not "AMPn" because they are not SDCA parts with rotation,
they have a fixed left/right assignment.)

We have to be careful of the length of the prefix. The 44 characters of
an ALSA control name get eaten up very quickly when we start creating
fully-qualified names for controls published by the firmware. So "AMPn"
was nice because it was descriptive enough but only uses 5 characters
of the 44.

Having said that, I've calculated that we have enough characters (just)
to use a prefix of "cs35l56-n". If there's a reason why that is
necessary/desirable for SOF or SoundWire then we could do that. But we'd
intended to use "AMPn" prefixes.

We just need to decide whether to go with "AMPn". Or switch to using
"cs35l56-n" for the ALSA prefix (the therefore the qualifier at the end
of the firmware filename).

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

[Pierre-Louis Bossart]

>>>>>> +        .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" ?
>>>>
>>>> IIRC this name_prefix is just used for the codec_conf and hence for
>>>> control names/UCM. At some point userspace/driver need to know if amp5
>>>> is left or right.
>>>>
>>>> We can certainly align on conventions but the values set in this ACPI
>>>> match table will not be used for firmware download - different scope.
>>>>
>>>
>>> They are used for our firmware download. Each amp can have its own
>>> unique firmware file. The ALSA prefix is used to identify which firmware
>>> file to load to which amp.
>>
>> The prefix will only be used when the card is created, specifically for
>> control names.
>> The firmware should be selected and downloaded when the device shows up
>> on the bus.
>> Card creation and device enumeration/initialization happen on different
>> timelines, if the machine driver is "blacklisted" or unbound I am not
>> sure what happens.
>>
>> There is a dependency between machine driver probe and codec firmware
>> download that I am not able to follow, can you please elaborate?
>>
> 
> The codec driver has to choose which firmware to load from under
> /lib/firmware. It does this using a combination of SSID (to identify the
> target product), the ALSA prefix string (to identify which amp) and
> in some systems a GPIO on the motherboard to select between different
> models of speaker when they have multiple suppliers. This results in a
> firmware name like:
> 
> cs35l56-<silicon rev>-dsp1-misc-<SSID>[-<SPEAKER MODEL>]-<ALSA PREFIX>
> 
> You can see this if you look in the linux-firmware repo under cirrus/
> for cs35l41 firmware files (though the ALSA PREFIX section in those
> cases is not "AMPn" because they are not SDCA parts with rotation,
> they have a fixed left/right assignment.)
> 
> We have to be careful of the length of the prefix. The 44 characters of
> an ALSA control name get eaten up very quickly when we start creating
> fully-qualified names for controls published by the firmware. So "AMPn"
> was nice because it was descriptive enough but only uses 5 characters
> of the 44.
> 
> Having said that, I've calculated that we have enough characters (just)
> to use a prefix of "cs35l56-n". If there's a reason why that is
> necessary/desirable for SOF or SoundWire then we could do that. But we'd
> intended to use "AMPn" prefixes.
> 
> We just need to decide whether to go with "AMPn". Or switch to using
> "cs35l56-n" for the ALSA prefix (the therefore the qualifier at the end
> of the firmware filename).

Yes we have similar issues with control names in topology, the limit is
hit very quickly.

I think you missed my point though that the ALSA prefix is only set when
the card is created, which can be sometime after the firmware needs to
be downloaded. I guess you could pick the firmware in the component
probe, which happens during the card creation, but that could be
sub-optimal. Given the download times you want the download to proceed
as early as possible.

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

[Richard Fitzgerald]

On 29/11/23 16:39, Pierre-Louis Bossart wrote:
> 
>>>>>>> +        .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" ?
>>>>>
>>>>> IIRC this name_prefix is just used for the codec_conf and hence for
>>>>> control names/UCM. At some point userspace/driver need to know if amp5
>>>>> is left or right.
>>>>>
>>>>> We can certainly align on conventions but the values set in this ACPI
>>>>> match table will not be used for firmware download - different scope.
>>>>>
>>>>
>>>> They are used for our firmware download. Each amp can have its own
>>>> unique firmware file. The ALSA prefix is used to identify which firmware
>>>> file to load to which amp.
>>>
>>> The prefix will only be used when the card is created, specifically for
>>> control names.
>>> The firmware should be selected and downloaded when the device shows up
>>> on the bus.
>>> Card creation and device enumeration/initialization happen on different
>>> timelines, if the machine driver is "blacklisted" or unbound I am not
>>> sure what happens.
>>>
>>> There is a dependency between machine driver probe and codec firmware
>>> download that I am not able to follow, can you please elaborate?
>>>
>>
>> The codec driver has to choose which firmware to load from under
>> /lib/firmware. It does this using a combination of SSID (to identify the
>> target product), the ALSA prefix string (to identify which amp) and
>> in some systems a GPIO on the motherboard to select between different
>> models of speaker when they have multiple suppliers. This results in a
>> firmware name like:
>>
>> cs35l56-<silicon rev>-dsp1-misc-<SSID>[-<SPEAKER MODEL>]-<ALSA PREFIX>
>>
>> You can see this if you look in the linux-firmware repo under cirrus/
>> for cs35l41 firmware files (though the ALSA PREFIX section in those
>> cases is not "AMPn" because they are not SDCA parts with rotation,
>> they have a fixed left/right assignment.)
>>
>> We have to be careful of the length of the prefix. The 44 characters of
>> an ALSA control name get eaten up very quickly when we start creating
>> fully-qualified names for controls published by the firmware. So "AMPn"
>> was nice because it was descriptive enough but only uses 5 characters
>> of the 44.
>>
>> Having said that, I've calculated that we have enough characters (just)
>> to use a prefix of "cs35l56-n". If there's a reason why that is
>> necessary/desirable for SOF or SoundWire then we could do that. But we'd
>> intended to use "AMPn" prefixes.
>>
>> We just need to decide whether to go with "AMPn". Or switch to using
>> "cs35l56-n" for the ALSA prefix (the therefore the qualifier at the end
>> of the firmware filename).
> 
> Yes we have similar issues with control names in topology, the limit is
> hit very quickly.
> 
> I think you missed my point though that the ALSA prefix is only set when
> the card is created, which can be sometime after the firmware needs to
> be downloaded. I guess you could pick the firmware in the component
> probe, which happens during the card creation, but that could be
> sub-optimal. Given the download times you want the download to proceed
> as early as possible.

We kick off a background task from our component_probe() to do the
firmware download. We need the ALSA prefix, and also the wm_adsp library
that actually handles the DSP is ASoC code so it needs a probed
component. Doing it in a background work means it doesn't block probe().
And the download to multiple amps can proceed in parallel - obviously
that's constrained by bus bandwidth but we are seeing that they
interleave.

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

[Pierre-Louis Bossart]

On 11/30/23 04:15, Richard Fitzgerald wrote:
> On 29/11/23 16:39, Pierre-Louis Bossart wrote:
>>
>>>>>>>> +        .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" ?
>>>>>>
>>>>>> IIRC this name_prefix is just used for the codec_conf and hence for
>>>>>> control names/UCM. At some point userspace/driver need to know if
>>>>>> amp5
>>>>>> is left or right.
>>>>>>
>>>>>> We can certainly align on conventions but the values set in this ACPI
>>>>>> match table will not be used for firmware download - different scope.
>>>>>>
>>>>>
>>>>> They are used for our firmware download. Each amp can have its own
>>>>> unique firmware file. The ALSA prefix is used to identify which
>>>>> firmware
>>>>> file to load to which amp.
>>>>
>>>> The prefix will only be used when the card is created, specifically for
>>>> control names.
>>>> The firmware should be selected and downloaded when the device shows up
>>>> on the bus.
>>>> Card creation and device enumeration/initialization happen on different
>>>> timelines, if the machine driver is "blacklisted" or unbound I am not
>>>> sure what happens.
>>>>
>>>> There is a dependency between machine driver probe and codec firmware
>>>> download that I am not able to follow, can you please elaborate?
>>>>
>>>
>>> The codec driver has to choose which firmware to load from under
>>> /lib/firmware. It does this using a combination of SSID (to identify the
>>> target product), the ALSA prefix string (to identify which amp) and
>>> in some systems a GPIO on the motherboard to select between different
>>> models of speaker when they have multiple suppliers. This results in a
>>> firmware name like:
>>>
>>> cs35l56-<silicon rev>-dsp1-misc-<SSID>[-<SPEAKER MODEL>]-<ALSA PREFIX>
>>>
>>> You can see this if you look in the linux-firmware repo under cirrus/
>>> for cs35l41 firmware files (though the ALSA PREFIX section in those
>>> cases is not "AMPn" because they are not SDCA parts with rotation,
>>> they have a fixed left/right assignment.)
>>>
>>> We have to be careful of the length of the prefix. The 44 characters of
>>> an ALSA control name get eaten up very quickly when we start creating
>>> fully-qualified names for controls published by the firmware. So "AMPn"
>>> was nice because it was descriptive enough but only uses 5 characters
>>> of the 44.
>>>
>>> Having said that, I've calculated that we have enough characters (just)
>>> to use a prefix of "cs35l56-n". If there's a reason why that is
>>> necessary/desirable for SOF or SoundWire then we could do that. But we'd
>>> intended to use "AMPn" prefixes.
>>>
>>> We just need to decide whether to go with "AMPn". Or switch to using
>>> "cs35l56-n" for the ALSA prefix (the therefore the qualifier at the end
>>> of the firmware filename).
>>
>> Yes we have similar issues with control names in topology, the limit is
>> hit very quickly.
>>
>> I think you missed my point though that the ALSA prefix is only set when
>> the card is created, which can be sometime after the firmware needs to
>> be downloaded. I guess you could pick the firmware in the component
>> probe, which happens during the card creation, but that could be
>> sub-optimal. Given the download times you want the download to proceed
>> as early as possible.
> 
> We kick off a background task from our component_probe() to do the
> firmware download. We need the ALSA prefix, and also the wm_adsp library
> that actually handles the DSP is ASoC code so it needs a probed
> component. Doing it in a background work means it doesn't block probe().
> And the download to multiple amps can proceed in parallel - obviously
> that's constrained by bus bandwidth but we are seeing that they
> interleave.

ah ok, that makes sense. In practice the delta between codec enumeration
and component probe is probably negligible, and in a multi-amplifier
setup the download times are much larger.

So to circle back to the initial feedback, do you mind submitting a
patch with the exact naming you'd want for the prefix?

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

[Richard Fitzgerald]

On 27/11/23 14:40, Richard Fitzgerald wrote:
> 
> 
>> +        .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" ?
> 

We're thinking about whether to change to using "cs35l56-n" prefix.
Hold on this while we make a decision...