On Tue, Apr 5, 2022 at 4:14 AM Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > > From: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> > > [ Upstream commit 651c304df7f6e3fbb4779527efa3eb128ef91329 ] > > Since we see a proliferation of devices with various configurations, > we want to automatically set the DMIC and SSP information. This patch > relies on the information extracted from NHLT and partially reverts > existing DMI quirks added by commit a164137ce91a ("ASoC: Intel: add > machine driver for SOF+ES8336") > > Note that NHLT can report multiple SSPs, choosing from the > ssp_link_mask in an MSB-first manner was found experimentally to work > fine. > > The only thing that cannot be detected is the GPIO type, and users may > want to use the quirk override parameter if the 'wrong' solution is > provided. > > Tested-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx> > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> > Reviewed-by: Bard Liao <yung-chuan.liao@xxxxxxxxxxxxxxx> > Reviewed-by: Péter Ujfalusi <peter.ujfalusi@xxxxxxxxxxxxxxx> > Link: https://lore.kernel.org/r/20220308192610.392950-15-pierre-louis.bossart@xxxxxxxxxxxxxxx > Signed-off-by: Mark Brown <broonie@xxxxxxxxxx> > Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx> It seems this patch is missing a dependent patch in the backport, specifically commit 679aa83a0fb70dcbf9e97cbdfd573e6fc8bf9b1a ASoC: soc-acpi: add information on I2S/TDM link mask sound/soc/intel/boards/sof_es8336.c: In function 'sof_es8336_probe': sound/soc/intel/boards/sof_es8336.c:482:32: error: 'struct snd_soc_acpi_mach_params' has no member named 'i2s_link_mask'; did you mean 'link_mask'? 482 | if (!mach->mach_params.i2s_link_mask) { | ^~~~~~~~~~~~~ | link_mask sound/soc/intel/boards/sof_es8336.c:494:39: error: 'struct snd_soc_acpi_mach_params' has no member named 'i2s_link_mask'; did you mean 'link_mask'? 494 | if (mach->mach_params.i2s_link_mask & BIT(2)) | ^~~~~~~~~~~~~ | link_mask sound/soc/intel/boards/sof_es8336.c:496:44: error: 'struct snd_soc_acpi_mach_params' has no member named 'i2s_link_mask'; did you mean 'link_mask'? 496 | else if (mach->mach_params.i2s_link_mask & BIT(1)) | ^~~~~~~~~~~~~ | link_mask sound/soc/intel/boards/sof_es8336.c:498:45: error: 'struct snd_soc_acpi_mach_params' has no member named 'i2s_link_mask'; did you mean 'link_mask'? 498 | else if (mach->mach_params.i2s_link_mask & BIT(0)) | ^~~~~~~~~~~~~ | link_mask make[4]: *** [scripts/Makefile.build:288: sound/soc/intel/boards/sof_es8336.o] Error 1 Justin > sound/soc/intel/boards/sof_es8336.c | 56 +++++++++++++++++++++-------- > 1 file changed, 41 insertions(+), 15 deletions(-) > > diff --git a/sound/soc/intel/boards/sof_es8336.c b/sound/soc/intel/boards/sof_es8336.c > index 20d577eaab6d..46e453915f82 100644 > --- a/sound/soc/intel/boards/sof_es8336.c > +++ b/sound/soc/intel/boards/sof_es8336.c > @@ -228,24 +228,25 @@ static int sof_es8336_quirk_cb(const struct dmi_system_id *id) > return 1; > } > > +/* > + * this table should only be used to add GPIO or jack-detection quirks > + * that cannot be detected from ACPI tables. The SSP and DMIC > + * information are providing by the platform driver and are aligned > + * with the topology used. > + * > + * If the GPIO support is missing, the quirk parameter can be used to > + * enable speakers. In that case it's recommended to keep the SSP and DMIC > + * information consistent, overriding the SSP and DMIC can only be done > + * if the topology file is modified as well. > + */ > static const struct dmi_system_id sof_es8336_quirk_table[] = { > - { > - .callback = sof_es8336_quirk_cb, > - .matches = { > - DMI_MATCH(DMI_SYS_VENDOR, "CHUWI Innovation And Technology"), > - DMI_MATCH(DMI_BOARD_NAME, "Hi10 X"), > - }, > - .driver_data = (void *)SOF_ES8336_SSP_CODEC(2) > - }, > { > .callback = sof_es8336_quirk_cb, > .matches = { > DMI_MATCH(DMI_SYS_VENDOR, "IP3 tech"), > DMI_MATCH(DMI_BOARD_NAME, "WN1"), > }, > - .driver_data = (void *)(SOF_ES8336_SSP_CODEC(0) | > - SOF_ES8336_TGL_GPIO_QUIRK | > - SOF_ES8336_ENABLE_DMIC) > + .driver_data = (void *)(SOF_ES8336_TGL_GPIO_QUIRK) > }, > {} > }; > @@ -470,11 +471,33 @@ static int sof_es8336_probe(struct platform_device *pdev) > card = &sof_es8336_card; > card->dev = dev; > > - if (!dmi_check_system(sof_es8336_quirk_table)) > - quirk = SOF_ES8336_SSP_CODEC(2); > + /* check GPIO DMI quirks */ > + dmi_check_system(sof_es8336_quirk_table); > > - if (quirk & SOF_ES8336_ENABLE_DMIC) > - dmic_be_num = 2; > + if (!mach->mach_params.i2s_link_mask) { > + dev_warn(dev, "No I2S link information provided, using SSP0. This may need to be modified with the quirk module parameter\n"); > + } else { > + /* > + * Set configuration based on platform NHLT. > + * In this machine driver, we can only support one SSP for the > + * ES8336 link, the else-if below are intentional. > + * In some cases multiple SSPs can be reported by NHLT, starting MSB-first > + * seems to pick the right connection. > + */ > + unsigned long ssp = 0; > + > + if (mach->mach_params.i2s_link_mask & BIT(2)) > + ssp = SOF_ES8336_SSP_CODEC(2); > + else if (mach->mach_params.i2s_link_mask & BIT(1)) > + ssp = SOF_ES8336_SSP_CODEC(1); > + else if (mach->mach_params.i2s_link_mask & BIT(0)) > + ssp = SOF_ES8336_SSP_CODEC(0); > + > + quirk |= ssp; > + } > + > + if (mach->mach_params.dmic_num) > + quirk |= SOF_ES8336_ENABLE_DMIC; > > if (quirk_override != -1) { > dev_info(dev, "Overriding quirk 0x%lx => 0x%x\n", > @@ -483,6 +506,9 @@ static int sof_es8336_probe(struct platform_device *pdev) > } > log_quirks(dev); > > + if (quirk & SOF_ES8336_ENABLE_DMIC) > + dmic_be_num = 2; > + > sof_es8336_card.num_links += dmic_be_num + hdmi_num; > dai_links = sof_card_dai_links_create(dev, > SOF_ES8336_SSP_CODEC(quirk), > -- > 2.34.1 > > >