[PATCH 00/14] ASoC: SOF/Intel/SoundWire: add missing quirks and DMIC support
HP Spectre x360 convertible devices rely on a mixed SoundWire+DMIC configuration which wasn't well supported. This lead to the discovery that we missed the TGL_HDMI quirk on a number of Dell devices, the addition of DMIC autodetection (based on NHLT tables), the addition of new component strings needed by UCM, and work-arounds due to problematic DSDT tables.
Bard Liao (1): ASoC: Intel: sof_sdw: add trace for dai links
Guennadi Liakhovetski (1): ASoC: SOF: Intel: HDA: don't keep a temporary variable
Pierre-Louis Bossart (11): ASoC: Intel: sof_sdw: add missing TGL_HDMI quirk for Dell SKU 0A3E ASoC: Intel: sof_sdw: add missing TGL_HDMI quirk for Dell SKU 0A5E ASoC: Intel: sof_sdw: add missing TGL_HDMI quirk for Dell SKU 0A32 ASoC: Intel: sof_sdw: reorganize quirks by generation ASoC: Intel: sof-sdw: indent and add quirks consistently ASoC: Intel: sof_sdw: add quirk for HP Spectre x360 convertible ASoC: Intel: sof_sdw: add mic:dmic and cfg-mics component strings ASoC: Intel: soc-acpi: add ACPI matching table for HP Spectre x360 ASoC: SOF: Intel: SoundWire: refine ACPI match ASoC: SOF: Intel: detect DMIC number in SoundWire mixed config ASoC: SOF: Intel: hda: add dev_dbg() when DMIC number is overridden
Rander Wang (1): ASoC: Intel: sof_sdw: detect DMIC number based on mach params
sound/soc/intel/boards/sof_sdw.c | 131 ++++++----- .../intel/common/soc-acpi-intel-tgl-match.c | 20 ++ sound/soc/sof/intel/hda.c | 208 ++++++++++++------ 3 files changed, 241 insertions(+), 118 deletions(-)
We missed adding the TGL_HDMI quirk which is very much needed to expose the 4 display pipelines and will be required on TGL topologies.
Fixes: e787f5b5b1406('ASoC: Intel: add support for new SoundWire hardware layout on TGL') Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Guennadi Liakhovetski guennadi.liakhovetski@intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- sound/soc/intel/boards/sof_sdw.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c index 6d0d6ef711e0..aece6ba9a846 100644 --- a/sound/soc/intel/boards/sof_sdw.c +++ b/sound/soc/intel/boards/sof_sdw.c @@ -64,7 +64,8 @@ static const struct dmi_system_id sof_sdw_quirk_table[] = { DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc"), DMI_EXACT_MATCH(DMI_PRODUCT_SKU, "0A3E") }, - .driver_data = (void *)(SOF_RT711_JD_SRC_JD2 | + .driver_data = (void *)(SOF_SDW_TGL_HDMI | + SOF_RT711_JD_SRC_JD2 | SOF_RT715_DAI_ID_FIX), }, {
We missed adding the TGL_HDMI quirk which is very much needed to expose the 4 display pipelines and will be required on TGL topologies.
Fixes: 9ad9bc59dde10 ('ASoC: Intel: sof_sdw: set proper flags for Dell TGL-H SKU 0A5E') Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Guennadi Liakhovetski guennadi.liakhovetski@intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- sound/soc/intel/boards/sof_sdw.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c index aece6ba9a846..e79b0bad44f3 100644 --- a/sound/soc/intel/boards/sof_sdw.c +++ b/sound/soc/intel/boards/sof_sdw.c @@ -74,7 +74,8 @@ static const struct dmi_system_id sof_sdw_quirk_table[] = { DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc"), DMI_EXACT_MATCH(DMI_PRODUCT_SKU, "0A5E") }, - .driver_data = (void *)(SOF_RT711_JD_SRC_JD2 | + .driver_data = (void *)(SOF_SDW_TGL_HDMI | + SOF_RT711_JD_SRC_JD2 | SOF_RT715_DAI_ID_FIX | SOF_SDW_FOUR_SPK), },
We missed adding the TGL_HDMI quirk which is very much needed to expose the 4 display pipelines and will be required on TGL topologies.
Fixes: 488cdbd8931fe ('ASoC: Intel: sof_sdw: add quirk for new TigerLake-SDCA device') Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Guennadi Liakhovetski guennadi.liakhovetski@intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- sound/soc/intel/boards/sof_sdw.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c index e79b0bad44f3..152ea166eeae 100644 --- a/sound/soc/intel/boards/sof_sdw.c +++ b/sound/soc/intel/boards/sof_sdw.c @@ -54,7 +54,8 @@ static const struct dmi_system_id sof_sdw_quirk_table[] = { DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc"), DMI_EXACT_MATCH(DMI_PRODUCT_SKU, "0A32") }, - .driver_data = (void *)(SOF_RT711_JD_SRC_JD2 | + .driver_data = (void *)(SOF_SDW_TGL_HDMI | + SOF_RT711_JD_SRC_JD2 | SOF_RT715_DAI_ID_FIX | SOF_SDW_FOUR_SPK), },
The quirk table is a mess, let's reorganize it by generation before making sure that the quirks are consistent for each generation.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Guennadi Liakhovetski guennadi.liakhovetski@intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- sound/soc/intel/boards/sof_sdw.c | 73 +++++++++++++++++--------------- 1 file changed, 38 insertions(+), 35 deletions(-)
diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c index 152ea166eeae..8bd1d6c84f19 100644 --- a/sound/soc/intel/boards/sof_sdw.c +++ b/sound/soc/intel/boards/sof_sdw.c @@ -48,37 +48,14 @@ static int sof_sdw_quirk_cb(const struct dmi_system_id *id) }
static const struct dmi_system_id sof_sdw_quirk_table[] = { + /* CometLake devices */ { .callback = sof_sdw_quirk_cb, .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc"), - DMI_EXACT_MATCH(DMI_PRODUCT_SKU, "0A32") - }, - .driver_data = (void *)(SOF_SDW_TGL_HDMI | - SOF_RT711_JD_SRC_JD2 | - SOF_RT715_DAI_ID_FIX | - SOF_SDW_FOUR_SPK), - }, - { - .callback = sof_sdw_quirk_cb, - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc"), - DMI_EXACT_MATCH(DMI_PRODUCT_SKU, "0A3E") - }, - .driver_data = (void *)(SOF_SDW_TGL_HDMI | - SOF_RT711_JD_SRC_JD2 | - SOF_RT715_DAI_ID_FIX), - }, - { - .callback = sof_sdw_quirk_cb, - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc"), - DMI_EXACT_MATCH(DMI_PRODUCT_SKU, "0A5E") + DMI_MATCH(DMI_SYS_VENDOR, "Intel Corporation"), + DMI_MATCH(DMI_PRODUCT_NAME, "CometLake Client"), }, - .driver_data = (void *)(SOF_SDW_TGL_HDMI | - SOF_RT711_JD_SRC_JD2 | - SOF_RT715_DAI_ID_FIX | - SOF_SDW_FOUR_SPK), + .driver_data = (void *)SOF_SDW_PCH_DMIC, }, { .callback = sof_sdw_quirk_cb, @@ -109,7 +86,7 @@ static const struct dmi_system_id sof_sdw_quirk_table[] = { SOF_RT715_DAI_ID_FIX | SOF_SDW_FOUR_SPK), }, - { + { .callback = sof_sdw_quirk_cb, .matches = { DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc"), @@ -119,6 +96,16 @@ static const struct dmi_system_id sof_sdw_quirk_table[] = { SOF_RT715_DAI_ID_FIX | SOF_SDW_FOUR_SPK), }, + /* IceLake devices */ + { + .callback = sof_sdw_quirk_cb, + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Intel Corporation"), + DMI_MATCH(DMI_PRODUCT_NAME, "Ice Lake Client"), + }, + .driver_data = (void *)SOF_SDW_PCH_DMIC, + }, + /* TigerLake devices */ { .callback = sof_sdw_quirk_cb, .matches = { @@ -133,18 +120,23 @@ static const struct dmi_system_id sof_sdw_quirk_table[] = { { .callback = sof_sdw_quirk_cb, .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "Intel Corporation"), - DMI_MATCH(DMI_PRODUCT_NAME, "Ice Lake Client"), + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc"), + DMI_EXACT_MATCH(DMI_PRODUCT_SKU, "0A3E") }, - .driver_data = (void *)SOF_SDW_PCH_DMIC, + .driver_data = (void *)(SOF_SDW_TGL_HDMI | + SOF_RT711_JD_SRC_JD2 | + SOF_RT715_DAI_ID_FIX), }, { .callback = sof_sdw_quirk_cb, .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "Intel Corporation"), - DMI_MATCH(DMI_PRODUCT_NAME, "CometLake Client"), + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc"), + DMI_EXACT_MATCH(DMI_PRODUCT_SKU, "0A5E") }, - .driver_data = (void *)SOF_SDW_PCH_DMIC, + .driver_data = (void *)(SOF_SDW_TGL_HDMI | + SOF_RT711_JD_SRC_JD2 | + SOF_RT715_DAI_ID_FIX | + SOF_SDW_FOUR_SPK), }, { .callback = sof_sdw_quirk_cb, @@ -164,7 +156,18 @@ static const struct dmi_system_id sof_sdw_quirk_table[] = { .driver_data = (void *)(SOF_SDW_TGL_HDMI | SOF_SDW_PCH_DMIC | SOF_SDW_FOUR_SPK), }, - + /* TigerLake-SDCA devices */ + { + .callback = sof_sdw_quirk_cb, + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc"), + DMI_EXACT_MATCH(DMI_PRODUCT_SKU, "0A32") + }, + .driver_data = (void *)(SOF_SDW_TGL_HDMI | + SOF_RT711_JD_SRC_JD2 | + SOF_RT715_DAI_ID_FIX | + SOF_SDW_FOUR_SPK), + }, {} };
Use the same style for all quirks to avoid misses and errors
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Guennadi Liakhovetski guennadi.liakhovetski@intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- sound/soc/intel/boards/sof_sdw.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c index 8bd1d6c84f19..99c0e4b12d1c 100644 --- a/sound/soc/intel/boards/sof_sdw.c +++ b/sound/soc/intel/boards/sof_sdw.c @@ -113,9 +113,10 @@ static const struct dmi_system_id sof_sdw_quirk_table[] = { DMI_MATCH(DMI_PRODUCT_NAME, "Tiger Lake Client Platform"), }, - .driver_data = (void *)(SOF_RT711_JD_SRC_JD1 | - SOF_SDW_TGL_HDMI | SOF_SDW_PCH_DMIC | - SOF_SSP_PORT(SOF_I2S_SSP2)), + .driver_data = (void *)(SOF_SDW_TGL_HDMI | + SOF_RT711_JD_SRC_JD1 | + SOF_SDW_PCH_DMIC | + SOF_SSP_PORT(SOF_I2S_SSP2)), }, { .callback = sof_sdw_quirk_cb, @@ -144,7 +145,8 @@ static const struct dmi_system_id sof_sdw_quirk_table[] = { DMI_MATCH(DMI_SYS_VENDOR, "Google"), DMI_MATCH(DMI_PRODUCT_NAME, "Volteer"), }, - .driver_data = (void *)(SOF_SDW_TGL_HDMI | SOF_SDW_PCH_DMIC | + .driver_data = (void *)(SOF_SDW_TGL_HDMI | + SOF_SDW_PCH_DMIC | SOF_SDW_FOUR_SPK), }, { @@ -153,7 +155,8 @@ static const struct dmi_system_id sof_sdw_quirk_table[] = { DMI_MATCH(DMI_SYS_VENDOR, "Google"), DMI_MATCH(DMI_PRODUCT_NAME, "Ripto"), }, - .driver_data = (void *)(SOF_SDW_TGL_HDMI | SOF_SDW_PCH_DMIC | + .driver_data = (void *)(SOF_SDW_TGL_HDMI | + SOF_SDW_PCH_DMIC | SOF_SDW_FOUR_SPK), }, /* TigerLake-SDCA devices */
This set of devices has SoundWire support along with DMICs. The DMI information was provided by users for 3 separate skus.
BugLink: https://github.com/thesofproject/linux/issues/2700 Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Guennadi Liakhovetski guennadi.liakhovetski@intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- sound/soc/intel/boards/sof_sdw.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c index 99c0e4b12d1c..0e83db947a57 100644 --- a/sound/soc/intel/boards/sof_sdw.c +++ b/sound/soc/intel/boards/sof_sdw.c @@ -159,6 +159,22 @@ static const struct dmi_system_id sof_sdw_quirk_table[] = { SOF_SDW_PCH_DMIC | SOF_SDW_FOUR_SPK), }, + { + /* + * this entry covers multiple HP SKUs. The family name + * does not seem robust enough, so we use a partial + * match that ignores the product name suffix + * (e.g. 15-eb1xxx, 14t-ea000 or 13-aw2xxx) + */ + .callback = sof_sdw_quirk_cb, + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "HP"), + DMI_MATCH(DMI_PRODUCT_NAME, "HP Spectre x360 Convertible"), + }, + .driver_data = (void *)(SOF_SDW_TGL_HDMI | + SOF_SDW_PCH_DMIC | + SOF_RT711_JD_SRC_JD2), + }, /* TigerLake-SDCA devices */ { .callback = sof_sdw_quirk_cb,
UCM needs to know which microphone is used (dmic or RT715-based), let's add the information in the component string.
Note the slight change from HDAudio platforms where 'cfg-dmics' was used. 'cfg-mics' is used here with the intent that this component string describes either the number of PCH-attached microphones or the number of RT715-attached ones (the assumption is that the two configurations are mutually exclusive).
Suggested-by: Jaroslav Kysela perex@perex.cz Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Guennadi Liakhovetski guennadi.liakhovetski@intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- sound/soc/intel/boards/sof_sdw.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c index 0e83db947a57..d1251a6ac3af 100644 --- a/sound/soc/intel/boards/sof_sdw.c +++ b/sound/soc/intel/boards/sof_sdw.c @@ -1222,6 +1222,15 @@ static int mc_probe(struct platform_device *pdev) if (!card->components) return -ENOMEM;
+ if (mach->mach_params.dmic_num) { + card->components = devm_kasprintf(card->dev, GFP_KERNEL, + "%s mic:dmic cfg-mics:%d", + card->components, + mach->mach_params.dmic_num); + if (!card->components) + return -ENOMEM; + } + card->long_name = sdw_card_long_name;
/* Register the card */
From: Rander Wang rander.wang@intel.com
Current driver create DMIC dai based on quirk for each platforms, so we need to add quirk for new platforms. Now driver reports DMIC number to machine driver and machine driver can create DMIC dai based on this information. The old check is reserved for some platforms may be failed to set the DMIC number in BIOS.
Reviewed-by: Bard Liao bard.liao@intel.com Signed-off-by: Rander Wang rander.wang@intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@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 d1251a6ac3af..06f07dd45fb4 100644 --- a/sound/soc/intel/boards/sof_sdw.c +++ b/sound/soc/intel/boards/sof_sdw.c @@ -955,7 +955,7 @@ static int sof_card_dai_links_create(struct device *dev, ctx->idisp_codec = true;
/* enable dmic01 & dmic16k */ - dmic_num = (sof_sdw_quirk & SOF_SDW_PCH_DMIC) ? 2 : 0; + dmic_num = (sof_sdw_quirk & SOF_SDW_PCH_DMIC || mach_params->dmic_num) ? 2 : 0; comp_num += dmic_num;
dev_dbg(dev, "sdw %d, ssp %d, dmic %d, hdmi %d", sdw_be_num, ssp_num,
From: Bard Liao yung-chuan.liao@linux.intel.com
We create dai links dynamically, so it is not easy to know what dai links are created. So adding trace for dai link name and id.
Reviewed-by: Rander Wang rander.wang@intel.com Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/intel/boards/sof_sdw.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c index 06f07dd45fb4..8adce6417b02 100644 --- a/sound/soc/intel/boards/sof_sdw.c +++ b/sound/soc/intel/boards/sof_sdw.c @@ -476,15 +476,14 @@ static int get_sdw_dailink_info(const struct snd_soc_acpi_link_adr *links, return 0; }
-static void init_dai_link(struct snd_soc_dai_link *dai_links, int be_id, - char *name, int playback, int capture, - struct snd_soc_dai_link_component *cpus, - int cpus_num, - struct snd_soc_dai_link_component *codecs, - int codecs_num, +static void init_dai_link(struct device *dev, struct snd_soc_dai_link *dai_links, + int be_id, char *name, int playback, int capture, + struct snd_soc_dai_link_component *cpus, int cpus_num, + struct snd_soc_dai_link_component *codecs, int codecs_num, int (*init)(struct snd_soc_pcm_runtime *rtd), const struct snd_soc_ops *ops) { + dev_dbg(dev, "create dai link %s, id %d\n", name, be_id); dai_links->id = be_id; dai_links->name = name; dai_links->platforms = platform_component; @@ -822,7 +821,7 @@ static int create_sdw_dailink(struct device *dev, int *be_index,
playback = (stream == SNDRV_PCM_STREAM_PLAYBACK); capture = (stream == SNDRV_PCM_STREAM_CAPTURE); - init_dai_link(dai_links + *be_index, *be_index, name, + init_dai_link(dev, dai_links + *be_index, *be_index, name, playback, capture, cpus + *cpu_id, cpu_dai_num, codecs, codec_num, @@ -1061,7 +1060,7 @@ static int sof_card_dai_links_create(struct device *dev,
playback = info->direction[SNDRV_PCM_STREAM_PLAYBACK]; capture = info->direction[SNDRV_PCM_STREAM_CAPTURE]; - init_dai_link(links + link_id, be_id, name, + init_dai_link(dev, links + link_id, be_id, name, playback, capture, cpus + cpu_id, 1, ssp_components, 1, @@ -1078,7 +1077,7 @@ static int sof_card_dai_links_create(struct device *dev, /* dmic */ if (dmic_num > 0) { cpus[cpu_id].dai_name = "DMIC01 Pin"; - init_dai_link(links + link_id, be_id, "dmic01", + init_dai_link(dev, links + link_id, be_id, "dmic01", 0, 1, // DMIC only supports capture cpus + cpu_id, 1, dmic_component, 1, @@ -1086,7 +1085,7 @@ static int sof_card_dai_links_create(struct device *dev, INC_ID(be_id, cpu_id, link_id);
cpus[cpu_id].dai_name = "DMIC16k Pin"; - init_dai_link(links + link_id, be_id, "dmic16k", + init_dai_link(dev, links + link_id, be_id, "dmic16k", 0, 1, // DMIC only supports capture cpus + cpu_id, 1, dmic_component, 1, @@ -1129,7 +1128,7 @@ static int sof_card_dai_links_create(struct device *dev, return -ENOMEM;
cpus[cpu_id].dai_name = cpu_name; - init_dai_link(links + link_id, be_id, name, + init_dai_link(dev, links + link_id, be_id, name, 1, 0, // HDMI only supports playback cpus + cpu_id, 1, idisp_components + i, 1,
This device only has a single amplifier on link1, so we need a dedicated entry to find a match.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Bard Liao bard.liao@intel.com Reviewed-by: Guennadi Liakhovetski guennadi.liakhovetski@intel.com --- .../intel/common/soc-acpi-intel-tgl-match.c | 20 +++++++++++++++++++ 1 file changed, 20 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 e3ddbf05bf72..40f31c8a3aba 100644 --- a/sound/soc/intel/common/soc-acpi-intel-tgl-match.c +++ b/sound/soc/intel/common/soc-acpi-intel-tgl-match.c @@ -205,6 +205,20 @@ static const struct snd_soc_acpi_link_adr tgl_rvp[] = { {} };
+static const struct snd_soc_acpi_link_adr tgl_hp[] = { + { + .mask = BIT(0), + .num_adr = ARRAY_SIZE(rt711_0_adr), + .adr_d = rt711_0_adr, + }, + { + .mask = BIT(1), + .num_adr = ARRAY_SIZE(rt1308_1_single_adr), + .adr_d = rt1308_1_single_adr, + }, + {} +}; + static const struct snd_soc_acpi_link_adr tgl_chromebook_base[] = { { .mask = BIT(0), @@ -383,6 +397,12 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_tgl_sdw_machines[] = { .drv_name = "sof_sdw", .sof_tplg_filename = "sof-tgl-rt711-rt1316-rt714.tplg", }, + { + .link_mask = 0x3, /* rt711 on link 0 and 1 rt1308 on link 1 */ + .links = tgl_hp, + .drv_name = "sof_sdw", + .sof_tplg_filename = "sof-tgl-rt711-rt1308.tplg", + }, { .link_mask = 0x3, /* rt711 on link 0 and 2 rt1308s on link 1 */ .links = tgl_rvp,
We have existing platforms where the wrong machine is selected. We need to make sure the number of devices reported on a link matches what we expect for a link descriptor. This helps avoid using the TGL-RVP configuration for an HP platform or vice-versa, depending on the order in which they are inserted in the table.
Co-developed-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Bard Liao bard.liao@intel.com Reviewed-by: Guennadi Liakhovetski guennadi.liakhovetski@intel.com --- sound/soc/sof/intel/hda.c | 59 +++++++++++++++++++++++++++++---------- 1 file changed, 45 insertions(+), 14 deletions(-)
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index 447163494b05..db868376039a 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -1071,32 +1071,63 @@ static bool link_slaves_found(struct snd_sof_dev *sdev, struct sdw_intel_slave_id *ids = sdw->ids; int num_slaves = sdw->num_slaves; unsigned int part_id, link_id, unique_id, mfg_id; - int i, j; + int i, j, k;
for (i = 0; i < link->num_adr; i++) { u64 adr = link->adr_d[i].adr; + int reported_part_count = 0;
mfg_id = SDW_MFG_ID(adr); part_id = SDW_PART_ID(adr); link_id = SDW_DISCO_LINK_ID(adr); + + for (j = 0; j < num_slaves; j++) { + /* find out how many identical parts were reported on that link */ + if (ids[j].link_id == link_id && + ids[j].id.part_id == part_id && + ids[j].id.mfg_id == mfg_id) + reported_part_count++; + } + for (j = 0; j < num_slaves; j++) { + int expected_part_count = 0; + if (ids[j].link_id != link_id || ids[j].id.part_id != part_id || ids[j].id.mfg_id != mfg_id) continue; - /* - * we have to check unique id - * if there is more than one - * Slave on the link - */ - unique_id = SDW_UNIQUE_ID(adr); - if (link->num_adr == 1 || - ids[j].id.unique_id == SDW_IGNORED_UNIQUE_ID || - ids[j].id.unique_id == unique_id) { - dev_dbg(bus->dev, - "found %x at link %d\n", - part_id, link_id); - break; + + /* find out how many identical parts are expected */ + for (k = 0; k < link->num_adr; k++) { + u64 adr2 = link->adr_d[i].adr; + unsigned int part_id2, link_id2, mfg_id2; + + mfg_id2 = SDW_MFG_ID(adr2); + part_id2 = SDW_PART_ID(adr2); + link_id2 = SDW_DISCO_LINK_ID(adr2); + + if (link_id2 == link_id && + part_id2 == part_id && + mfg_id2 == mfg_id) + expected_part_count++; + } + + if (reported_part_count == expected_part_count) { + /* + * we have to check unique id + * if there is more than one + * Slave on the link + */ + unique_id = SDW_UNIQUE_ID(adr); + if (reported_part_count == 1 || + ids[j].id.unique_id == unique_id) { + dev_dbg(bus->dev, "found %x at link %d\n", + part_id, link_id); + break; + } + } else { + dev_dbg(bus->dev, "part %x reported %d expected %d on link %d, skipping\n", + part_id, reported_part_count, expected_part_count, link_id); } } if (j == num_slaves) {
The pinmux allows for 2 SoundWire links to be enabled along with DMICs. This was the default configuration on the TGL-RVP.
One issue with this configuration is that we don't have a means to automatically detect how many DMICs are used, which in turn requires the user to manually rename the topology file required on a platform.
This was borderline acceptable for Intel RVPs, but now that this configuration is present in HP devices we need to automate the process.
This patch makes use of the NHLT information and will pass the DMIC number to the machine driver as a parameter. A follow-up patch will expose the DMIC number to userspace/UCM with the configuration strings.
The Google devices do make use of DMICs instead of SoundWire link 2 and 3, but their topology is unique enough that they do not need any NHTL support or topology renaming.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Guennadi Liakhovetski guennadi.liakhovetski@intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- sound/soc/sof/intel/hda.c | 129 ++++++++++++++++++++++++-------------- 1 file changed, 82 insertions(+), 47 deletions(-)
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index db868376039a..7dfa30c8a168 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -555,7 +555,7 @@ static int hda_init(struct snd_sof_dev *sdev) return ret; }
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) +#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) || IS_ENABLED(CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE)
static int check_nhlt_dmic(struct snd_sof_dev *sdev) { @@ -598,6 +598,53 @@ static const char *fixup_tplg_name(struct snd_sof_dev *sdev, return tplg_filename; }
+static int dmic_topology_fixup(struct snd_sof_dev *sdev, + const char **tplg_filename, + const char *idisp_str, + int *dmic_found) +{ + const char *default_tplg_filename = *tplg_filename; + const char *fixed_tplg_filename; + const char *dmic_str; + int dmic_num; + + /* first check NHLT for DMICs */ + dmic_num = check_nhlt_dmic(sdev); + + /* allow for module parameter override */ + if (hda_dmic_num != -1) + dmic_num = hda_dmic_num; + + switch (dmic_num) { + case 1: + dmic_str = "-1ch"; + break; + case 2: + dmic_str = "-2ch"; + break; + case 3: + dmic_str = "-3ch"; + break; + case 4: + dmic_str = "-4ch"; + break; + default: + dmic_num = 0; + dmic_str = ""; + break; + } + + fixed_tplg_filename = fixup_tplg_name(sdev, default_tplg_filename, + idisp_str, dmic_str); + if (!fixed_tplg_filename) + return -ENOMEM; + + dev_info(sdev->dev, "DMICs detected in NHLT tables: %d\n", dmic_num); + *dmic_found = dmic_num; + *tplg_filename = fixed_tplg_filename; + + return 0; +} #endif
static int hda_init_caps(struct snd_sof_dev *sdev) @@ -963,9 +1010,9 @@ static int hda_generic_machine_select(struct snd_sof_dev *sdev) struct snd_sof_pdata *pdata = sdev->pdata; const char *tplg_filename; const char *idisp_str; - const char *dmic_str; int dmic_num = 0; int codec_num = 0; + int ret; int i;
/* codec detection */ @@ -990,10 +1037,6 @@ static int hda_generic_machine_select(struct snd_sof_dev *sdev) if (!pdata->machine && codec_num <= 2) { hda_mach = snd_soc_acpi_intel_hda_machines;
- /* topology: use the info from hda_machines */ - pdata->tplg_filename = - hda_mach->sof_tplg_filename; - dev_info(bus->dev, "using HDA machine driver %s now\n", hda_mach->drv_name);
@@ -1002,42 +1045,13 @@ static int hda_generic_machine_select(struct snd_sof_dev *sdev) else idisp_str = "";
- /* first check NHLT for DMICs */ - dmic_num = check_nhlt_dmic(sdev); - - /* allow for module parameter override */ - if (hda_dmic_num != -1) - dmic_num = hda_dmic_num; - - switch (dmic_num) { - case 1: - dmic_str = "-1ch"; - break; - case 2: - dmic_str = "-2ch"; - break; - case 3: - dmic_str = "-3ch"; - break; - case 4: - dmic_str = "-4ch"; - break; - default: - dmic_num = 0; - dmic_str = ""; - break; - } - - tplg_filename = pdata->tplg_filename; - tplg_filename = fixup_tplg_name(sdev, tplg_filename, - idisp_str, dmic_str); - if (!tplg_filename) - return -EINVAL; - - dev_info(bus->dev, - "DMICs detected in NHLT tables: %d\n", - dmic_num); + /* topology: use the info from hda_machines */ + tplg_filename = hda_mach->sof_tplg_filename; + ret = dmic_topology_fixup(sdev, &tplg_filename, idisp_str, &dmic_num); + if (ret < 0) + return ret;
+ hda_mach->mach_params.dmic_num = dmic_num; pdata->machine = hda_mach; pdata->tplg_filename = tplg_filename; } @@ -1049,7 +1063,6 @@ static int hda_generic_machine_select(struct snd_sof_dev *sdev) &pdata->machine->mach_params; mach_params->codec_mask = bus->codec_mask; mach_params->common_hdmi_codec_drv = hda_codec_use_common_hdmi; - mach_params->dmic_num = dmic_num; }
return 0; @@ -1144,7 +1157,6 @@ static int hda_sdw_machine_select(struct snd_sof_dev *sdev) { struct snd_sof_pdata *pdata = sdev->pdata; const struct snd_soc_acpi_link_adr *link; - struct hdac_bus *bus = sof_to_bus(sdev); struct snd_soc_acpi_mach *mach; struct sof_intel_hda_dev *hdev; u32 link_mask; @@ -1192,10 +1204,8 @@ static int hda_sdw_machine_select(struct snd_sof_dev *sdev) break; } if (mach && mach->link_mask) { - dev_dbg(bus->dev, - "SoundWire machine driver %s topology %s\n", - mach->drv_name, - mach->sof_tplg_filename); + int dmic_num = 0; + pdata->machine = mach; mach->mach_params.links = mach->links; mach->mach_params.link_mask = mach->link_mask; @@ -1205,6 +1215,31 @@ static int hda_sdw_machine_select(struct snd_sof_dev *sdev) else pdata->fw_filename = pdata->desc->default_fw_filename; pdata->tplg_filename = mach->sof_tplg_filename; + + /* + * DMICs use up to 4 pins and are typically pin-muxed with SoundWire + * link 2 and 3, thus we only try to enable dmics if all conditions + * are true: + * a) link 2 and 3 are not used by SoundWire + * b) the NHLT table reports the presence of microphones + */ + if (!(mach->link_mask & GENMASK(3, 2))) { + const char *tplg_filename = mach->sof_tplg_filename; + int ret; + + ret = dmic_topology_fixup(sdev, &tplg_filename, "", &dmic_num); + + if (ret < 0) + return ret; + + pdata->tplg_filename = tplg_filename; + } + mach->mach_params.dmic_num = dmic_num; + + dev_dbg(sdev->dev, + "SoundWire machine driver %s topology %s\n", + mach->drv_name, + pdata->tplg_filename); } else { dev_info(sdev->dev, "No SoundWire machine driver found\n");
From: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com
fixup_tplg_name() doesn't need to keep the string, allocated for filename - it's temporary.
Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Signed-off-by: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index 7dfa30c8a168..a61eb7afd364 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -579,22 +579,22 @@ static const char *fixup_tplg_name(struct snd_sof_dev *sdev, const char *dmic_str) { const char *tplg_filename = NULL; - char *filename; - char *split_ext; + char *filename, *tmp; + const char *split_ext;
- filename = devm_kstrdup(sdev->dev, sof_tplg_filename, GFP_KERNEL); + filename = kstrdup(sof_tplg_filename, GFP_KERNEL); if (!filename) return NULL;
/* this assumes a .tplg extension */ - split_ext = strsep(&filename, "."); - if (split_ext) { + tmp = filename; + split_ext = strsep(&tmp, "."); + if (split_ext) tplg_filename = devm_kasprintf(sdev->dev, GFP_KERNEL, "%s%s%s.tplg", split_ext, idisp_str, dmic_str); - if (!tplg_filename) - return NULL; - } + kfree(filename); + return tplg_filename; }
It's useful for debug and system integration to show cases where we ignore the number of microphones reported by NHLT.
Suggested-by: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Guennadi Liakhovetski guennadi.liakhovetski@intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- sound/soc/sof/intel/hda.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index a61eb7afd364..5926d23da9ae 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -612,8 +612,12 @@ static int dmic_topology_fixup(struct snd_sof_dev *sdev, dmic_num = check_nhlt_dmic(sdev);
/* allow for module parameter override */ - if (hda_dmic_num != -1) + if (hda_dmic_num != -1) { + dev_dbg(sdev->dev, + "overriding DMICs detected in NHLT tables %d by kernel param %d\n", + dmic_num, hda_dmic_num); dmic_num = hda_dmic_num; + }
switch (dmic_num) { case 1:
On Thu, Feb 04, 2021 at 02:33:12PM -0600, Pierre-Louis Bossart wrote:
/* allow for module parameter override */
- if (hda_dmic_num != -1)
- if (hda_dmic_num != -1) {
This is breaking in an x86 allmodconfig build due to:
/mnt/kernel/sound/soc/sof/intel/hda.c: In function 'dmic_topology_fixup': /mnt/kernel/sound/soc/sof/intel/hda.c:615:6: error: 'hda_dmic_num' undeclared (first use in this function); did you mean 'dmic_num'? if (hda_dmic_num != -1) { ^~~~~~~~~~~~ dmic_num /mnt/kernel/sound/soc/sof/intel/hda.c:615:6: note: each undeclared identifier is reported only once for each function it appears in
which will actually be triggered by one of the earlier patches in the series (my script is going through things in reverse order), that variable is only defined for CONFIG_SOC_SOF_HDA.
On 2/5/21 9:33 AM, Mark Brown wrote:
On Thu, Feb 04, 2021 at 02:33:12PM -0600, Pierre-Louis Bossart wrote:
/* allow for module parameter override */
- if (hda_dmic_num != -1)
- if (hda_dmic_num != -1) {
This is breaking in an x86 allmodconfig build due to:
/mnt/kernel/sound/soc/sof/intel/hda.c: In function 'dmic_topology_fixup': /mnt/kernel/sound/soc/sof/intel/hda.c:615:6: error: 'hda_dmic_num' undeclared (first use in this function); did you mean 'dmic_num'? if (hda_dmic_num != -1) { ^~~~~~~~~~~~ dmic_num /mnt/kernel/sound/soc/sof/intel/hda.c:615:6: note: each undeclared identifier is reported only once for each function it appears in
which will actually be triggered by one of the earlier patches in the series (my script is going through things in reverse order), that variable is only defined for CONFIG_SOC_SOF_HDA.
That's not good. Please drop this patch for now, it's nice-to-have and not critical.
I'll have to figure out why this wasn't reported earlier.
On Fri, Feb 05, 2021 at 09:47:39AM -0600, Pierre-Louis Bossart wrote:
On 2/5/21 9:33 AM, Mark Brown wrote:
On Thu, Feb 04, 2021 at 02:33:12PM -0600, Pierre-Louis Bossart wrote:
This is breaking in an x86 allmodconfig build due to:
which will actually be triggered by one of the earlier patches in the series (my script is going through things in reverse order), that variable is only defined for CONFIG_SOC_SOF_HDA.
That's not good. Please drop this patch for now, it's nice-to-have and not critical.
I'll have to figure out why this wasn't reported earlier.
Like I say it's not that patch that's causing the break, I didn't walk through and work out which one was - the patch isn't actually adding the usage or anything.
Like I say it's not that patch that's causing the break, I didn't walk through and work out which one was - the patch isn't actually adding the usage or anything.
Ah yes, you're correct. Mea culpa. We're using the parameter for HDA and SoundWire now, but HDA is disabled with allmodconfig due to a mutual exclusion with NOCODEC. I've had this on my todo list for a while.
We need something like this, will test and resend a v2. Sorry about the noise.
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index 2014058bddf2..0dc3a8c0f5e3 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -285,11 +285,13 @@ static char *hda_model; module_param(hda_model, charp, 0444); MODULE_PARM_DESC(hda_model, "Use the given HDA board model.");
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) +#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) || IS_ENABLED(CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE) static int hda_dmic_num = -1; module_param_named(dmic_num, hda_dmic_num, int, 0444); MODULE_PARM_DESC(dmic_num, "SOF HDA DMIC number"); +#endif
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) static bool hda_codec_use_common_hdmi = IS_ENABLED(CONFIG_SND_HDA_CODEC_HDMI); module_param_named(use_common_hdmi, hda_codec_use_common_hdmi, bool, 0444); MODULE_PARM_DESC(use_common_hdmi, "SOF HDA use common HDMI codec driver");
On Fri, Feb 05, 2021 at 10:38:54AM -0600, Pierre-Louis Bossart wrote:
Like I say it's not that patch that's causing the break, I didn't walk through and work out which one was - the patch isn't actually adding the usage or anything.
Ah yes, you're correct. Mea culpa. We're using the parameter for HDA and SoundWire now, but HDA is disabled with allmodconfig due to a mutual exclusion with NOCODEC. I've had this on my todo list for a while.
We need something like this, will test and resend a v2. Sorry about the noise.
Yeah, that looks like it should do the job.
On Thu, 4 Feb 2021 14:32:58 -0600, Pierre-Louis Bossart wrote:
HP Spectre x360 convertible devices rely on a mixed SoundWire+DMIC configuration which wasn't well supported. This lead to the discovery that we missed the TGL_HDMI quirk on a number of Dell devices, the addition of DMIC autodetection (based on NHLT tables), the addition of new component strings needed by UCM, and work-arounds due to problematic DSDT tables.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[01/14] ASoC: Intel: sof_sdw: add missing TGL_HDMI quirk for Dell SKU 0A3E commit: 5ab3ff4d66960be766a544886667e7c002f17fd6 [02/14] ASoC: Intel: sof_sdw: add missing TGL_HDMI quirk for Dell SKU 0A5E commit: f12bbc50f3b14c9b8ed902c6d1da980dd5addcce [03/14] ASoC: Intel: sof_sdw: add missing TGL_HDMI quirk for Dell SKU 0A32 commit: 45c92ec32b43c6cb42341ebf07577eefed9d87ec
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
On Thu, 4 Feb 2021 14:32:58 -0600, Pierre-Louis Bossart wrote:
HP Spectre x360 convertible devices rely on a mixed SoundWire+DMIC configuration which wasn't well supported. This lead to the discovery that we missed the TGL_HDMI quirk on a number of Dell devices, the addition of DMIC autodetection (based on NHLT tables), the addition of new component strings needed by UCM, and work-arounds due to problematic DSDT tables.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[04/14] ASoC: Intel: sof_sdw: reorganize quirks by generation commit: 3d09cf8d0d791a41a75123e135f604d59f4aa870 [05/14] ASoC: Intel: sof-sdw: indent and add quirks consistently commit: 8caf37e2be761688c396c609880936a807af490f [06/14] ASoC: Intel: sof_sdw: add quirk for HP Spectre x360 convertible commit: d92e279dee56b4b65c1af21f972413f172a9734a [07/14] ASoC: Intel: sof_sdw: add mic:dmic and cfg-mics component strings commit: 209b0b0d8d5a469a2892ad51cb448811d00b4ff4 [08/14] ASoC: Intel: sof_sdw: detect DMIC number based on mach params commit: f88dcb9b98d3f86ead04d2453475267910448bb8 [09/14] ASoC: Intel: sof_sdw: add trace for dai links commit: 3827b7ca399245e609b3ca717550b0638d1f69cd [10/14] ASoC: Intel: soc-acpi: add ACPI matching table for HP Spectre x360 commit: 717a8fdd150c495cc202880cf6955294c7acae4f [11/14] ASoC: SOF: Intel: SoundWire: refine ACPI match commit: 6f5d506d7ff1d9b1ffac0130f2958b9da41175f4 [12/14] ASoC: SOF: Intel: detect DMIC number in SoundWire mixed config commit: 7aecf59770920cce5ff6e94b3809574364178126 [13/14] ASoC: SOF: Intel: HDA: don't keep a temporary variable commit: b9088535e1021f11500f8417598b6af1f381f7dc [14/14] ASoC: SOF: Intel: hda: add dev_dbg() when DMIC number is overridden commit: 026370cb5bd7ef7999bc4379ab89ffd7a73874f2
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 (2)
-
Mark Brown -
Pierre-Louis Bossart