[PATCH 0/7] ASoC: Intel: Soundwire related board and match updates
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
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; }
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,
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;
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,
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,
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,
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,- },
- {}
+};
+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, + }, + {} +};
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, + }, + {} +};
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?
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).
+ .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.
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.
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?
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...
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,
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
participants (4)
-
Mark Brown -
Peter Ujfalusi -
Pierre-Louis Bossart -
Richard Fitzgerald