ASoC: SOF: topology: Use acpi mach from the machine driver #5665
Conversation
| @@ -2530,8 +2531,12 @@ int snd_sof_load_topology(struct snd_soc_component *scomp, const char *file) | |||
| */ | |||
| bool no_fallback = strstr(file, "dummy"); | |||
|
|
|||
There was a problem hiding this comment.
We use the SOC_SDW_PCH_DMIC quirk to indicate that the PCH DMIC is used. We will set mach_params->dmic_num = DMIC_DEFAULT_CHANNELS; to the mach_params from dev_get_platdata(card->dev) when the quirk is set. But the mach_params of the sof_sdw_get_tplg_files() function is from sdev->pdata. In other words, the mach_params->dmic_num of sof_pdata->machine remains 0 and it will cause the unsupported number of dmics: 0 issue.
There was a problem hiding this comment.
Will the dmic_num override still going to be working?
But in other words we have two places whit dmic_num and they are used in a 'random' way around the code?
ujfalusi
left a comment
ujfalusi
left a comment
There was a problem hiding this comment.
do we want it to be backported to stable kernels? Fixes tag?
sound/soc/sof/topology.c
Outdated
| /* Try to use function topologies if possible */ | ||
| if (!sof_pdata->disable_function_topology && !disable_function_topology && | ||
| sof_pdata->machine && sof_pdata->machine->get_function_tplg_files) { | ||
| struct snd_soc_acpi_mach *card_mach = dev_get_platdata(scomp->card->dev); |
There was a problem hiding this comment.
This never going to be NULL, right?
There was a problem hiding this comment.
Good question @ujfalusi I don't think it will be NULL, but it is not guarantee that it is with the snd_soc_acpi_mach struct. I will move the change to sof_sdw_get_tplg_files() which is specific to Intel SDW machines.
| @@ -2530,8 +2531,12 @@ int snd_sof_load_topology(struct snd_soc_component *scomp, const char *file) | |||
| */ | |||
| bool no_fallback = strstr(file, "dummy"); | |||
|
|
|||
There was a problem hiding this comment.
Will the dmic_num override still going to be working?
But in other words we have two places whit dmic_num and they are used in a 'random' way around the code?
Yes, I will add it. |
bardliao
force-pushed
the
fix-dmic-quirk
branch
from
2f23594 to
3589a5e
Compare
bardliao
requested review from
dbaluta,
lgirdwood and
plbossart
as code owners
The parameters may be changed by the of_sdw achine driver based on the machine driver quirk. We need to use the acpi mach from the machine driver. Fixes: 2fbeff3 ("ASoC: Intel: add sof_sdw_get_tplg_files ops") Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
bardliao
force-pushed
the
fix-dmic-quirk
branch
from
3589a5e to
64f86c1
Compare
| struct snd_soc_acpi_mach *card_mach = dev_get_platdata(card->dev); | ||
| /* | ||
| * Use the acpi mach from the machine driver because the machine driver | ||
| * may change the dmic_num based on the machine driver quirk. |
There was a problem hiding this comment.
but the mach passed by parameter contains the num_dmic or other overrides coming from module parameters of SOF modules?
In code we use one of the match here and there, would be nice to clarify the rules?
There was a problem hiding this comment.
anything provided by the user should override data found in ACPI, NHLT, DISCO or quirks. i.e. user is overriding for a reason
There was a problem hiding this comment.
but the mach passed by parameter contains the num_dmic or other overrides coming from module parameters of SOF modules?
The mach passed by parameter contains the num_dmic detected by the NHLT blob or the module parameters of the snd_sof_intel_hda_generic module. The dmic_num value will be set by the check_dmic_num() function.
And the sof_sdw machine driver may overwrite the num_dmic of its own mach by the quirk.
In code we use one of the match here and there, would be nice to clarify the rules?
Currently, I only see the sof_sdw machine driver will change the dmic_num.
anything provided by the user should override data found in ACPI, NHLT, DISCO or quirks. i.e. user is overriding for a reason
Agree. In general, module parameters have higher priority than the NHLT, quirks. However, the machine driver changes the dmic_num with below rules.
if (ctx->ignore_internal_dmic) {
dev_dbg(dev, "SoundWire DMIC is used, ignoring internal DMIC\n");
mach_params->dmic_num = 0;
} else if (mach_params->dmic_num) {
dmic_num = 2;
} else if (sof_sdw_quirk & SOC_SDW_PCH_DMIC) {
dmic_num = 2;
/*
* mach_params->dmic_num will be used to set the cfg-mics value of
* card->components string. Set it to the default value.
*/
mach_params->dmic_num = DMIC_DEFAULT_CHANNELS;
}ctx->ignore_internal_dmic will be true if a standalone SoundWire DMIC is used. We assume that PCH DMIC will not be used in such case. And we need the SOC_SDW_PCH_DMIC quirk for Chrome because there is no NHLT blob in the coreboot.
4 participants
The parameters may be changed by the machine driver based on the actual hardware configuration. We need to use the acpi mach from the machine driver.