ASoC: Intel: sof_sdw: subtract dmic_num when dmic link is not created #5126
Conversation
bardliao
requested review from
lgirdwood,
ranj063,
kv2019i,
dbaluta and
ujfalusi
as code owners
sound/soc/intel/boards/sof_sdw.c
Outdated
| @@ -1258,6 +1258,7 @@ static int sof_card_dai_links_create(struct snd_soc_card *card) | |||
| if (dmic_num > 0) { | |||
| if (ctx->ignore_internal_dmic) { | |||
| dev_warn(dev, "Ignoring PCH DMIC\n"); | |||
| card->num_links -= dmic_num; | |||
There was a problem hiding this comment.
I feel like it might make more sense to move the whole check for ignore_internal_dmic up to its initialisation, something like:
if (sof_sdw_quirk & SOC_SDW_PCH_DMIC || mach_params->dmic_num) {
if (ctx->ignore_internal_dmic)
dev_warn(dev, "Ignoring PCH DMIC\n");
else
dmic_num = 2;
}
That way the dmic_num will be accurrate and all the code handling the number of dmics is together.
dmic links will not be created if ctx->ignore_internal_dmic is set, and dmic_num should be 0 in this case. Move ignore_internal_dmic check earlier where dmic_num is set to get an accurate dmic_num. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
mach_params->dmic_num will be used to set the cfg-mics value of card->components string. Overwrite it to the actual number of PCH DMICs used in the device. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
|
@plbossart @charleskeepax I added a commit to make card->components use the actual dmic_num. Could you review again? |
| * mach_params->dmic_num will be used to set the cfg-mics value of card->components | ||
| * string. Overwrite it to the actual number of PCH DMICs used in the device. | ||
| */ | ||
| mach_params->dmic_num = dmic_num; |
There was a problem hiding this comment.
that seems ok, but now that we are looking at the code why do we need this SOC_SDW_PCH_DMIC quirk? if all the information is provided by the dmic_num, the first part of the test line 1212 seems useless.
There was a problem hiding this comment.
It is weird but I am not sure it is pointless. The driver will add 2 dmics if either the quirk is set or mach_params->dmic_num is non-zero. So presumably there are systems where mach_params->dmic_num is zero but one still wants the dmics added.
There was a problem hiding this comment.
if the mach_params->dmic_num is zero, then the SOF driver did not select a topology with the mics enabled, so adding a dailink is useless IMHO. I think this is an old quirk from way back, I don't think it serves any purpose now.
There was a problem hiding this comment.
Well in that case, I certainly have no objection to removing it.
There was a problem hiding this comment.
if the mach_params->dmic_num is zero, then the SOF driver did not select a topology with the mics enabled, so adding a dailink is useless IMHO. I think this is an old quirk from way back, I don't think it serves any purpose now.
With SOC_SDW_PCH_DMIC quick enabled, the PCH DMIC link will be created. The topology name may not have -4ch or -2ch suffix even if the PCH DMIC is including in the topology. For example, cavs-sdw;sof-mtl-sdw-cs42l42-l0-max98363-l2;PLATFORM=mtl,NUM_DMICS=4
There was a problem hiding this comment.
is this correct to say that this quirk is only useful for Chromebooks then, where there is no NHLT ?
There was a problem hiding this comment.
is this correct to say that this quirk is only useful for Chromebooks then, where there is no NHLT ?
As far as I know, yes.
There was a problem hiding this comment.
ok then, let's keep it. thanks for educating me on this...
WARN_ON(dai_links != card->dai_link + card->num_links); is checked at the end of sof_card_dai_links_create(). We need to subtract dmic_num from card->num_links when dmic link is not created.