[PATCH] ASoC: snd-sof-intel-hda-common - add hda_model parameter and pass it to HDA codec driver

[PATCH] ASoC: snd-sof-intel-hda-common - add hda_model parameter and pass it to HDA codec driver

[Jaroslav Kysela]

It may be useful to pass the specific model to the generic HDA codec
routines like the legacy HDA driver (snd-hda-intel) allows.
The model name "sofbus" is tricky anyway.

Original proposal: https://lore.kernel.org/alsa-devel/20191203161908.7496-1-perex@perex.cz/

Signed-off-by: Jaroslav Kysela <perex@perex.cz>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: Mark Brown <broonie@kernel.org>
---
 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 211e91e79eae..ea0189ee8939 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -282,6 +282,10 @@ module_param_named(use_msi, hda_use_msi, bool, 0444);
 MODULE_PARM_DESC(use_msi, "SOF HDA use PCI MSI mode");
 #endif
 
+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)
 static int hda_dmic_num = -1;
 module_param_named(dmic_num, hda_dmic_num, int, 0444);
@@ -503,7 +507,7 @@ static int hda_init(struct snd_sof_dev *sdev)
 	mutex_init(&hbus->prepare_mutex);
 	hbus->pci = pci;
 	hbus->mixer_assigned = -1;
-	hbus->modelname = "sofbus";
+	hbus->modelname = hda_model;
 
 	/* initialise hdac bus */
 	bus->addr = pci_resource_start(pci, 0);
-- 
2.25.3

Re: [PATCH] ASoC: snd-sof-intel-hda-common - add hda_model parameter and pass it to HDA codec driver

[Kai Vehmanen]

Hey,

On Fri, 24 Apr 2020, Jaroslav Kysela wrote:

> It may be useful to pass the specific model to the generic HDA codec
> routines like the legacy HDA driver (snd-hda-intel) allows.
[...]
> Original proposal: https://lore.kernel.org/alsa-devel/20191203161908.7496-1-perex@perex.cz/

not sure why this got stuck last year, but seems like a welcome
addition:

Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>

> The model name "sofbus" is tricky anyway.

Hmm, I wonder is this now doing more harm than good. Based on browsing 
through the related code in hda-codec.c and friends, it would seem 
"sofbus" as the default is mostly harmless, but I could have missed
something.

Br, Kai

Re: [PATCH] ASoC: snd-sof-intel-hda-common - add hda_model parameter and pass it to HDA codec driver

[Takashi Iwai]

On Fri, 24 Apr 2020 14:24:30 +0200,
Kai Vehmanen wrote:
> 
> Hey,
> 
> On Fri, 24 Apr 2020, Jaroslav Kysela wrote:
> 
> > It may be useful to pass the specific model to the generic HDA codec
> > routines like the legacy HDA driver (snd-hda-intel) allows.
> [...]
> > Original proposal: https://lore.kernel.org/alsa-devel/20191203161908.7496-1-perex@perex.cz/
> 
> not sure why this got stuck last year, but seems like a welcome
> addition:
> 
> Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> 
> > The model name "sofbus" is tricky anyway.
> 
> Hmm, I wonder is this now doing more harm than good. Based on browsing 
> through the related code in hda-codec.c and friends, it would seem 
> "sofbus" as the default is mostly harmless, but I could have missed
> something.

That's currently harmless since no codec driver defines "sofbus"
model, hence the HDA parser continues to match with the default
quirks. OTOH, the fixed "sofbus" model is fairly useless.  So, feel
free to take my ack, too:

Reviewed-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi

Re: [PATCH] ASoC: snd-sof-intel-hda-common - add hda_model parameter and pass it to HDA codec driver

[Pierre-Louis Bossart]

>> Hmm, I wonder is this now doing more harm than good. Based on browsing
>> through the related code in hda-codec.c and friends, it would seem
>> "sofbus" as the default is mostly harmless, but I could have missed
>> something.
> 
> That's currently harmless since no codec driver defines "sofbus"
> model, hence the HDA parser continues to match with the default
> quirks. OTOH, the fixed "sofbus" model is fairly useless.  So, feel
> free to take my ack, too:

For my education, are you saying that the default should be that the 
modelname is NULL, and the hda auto parser will use known quirks based 
on PCI/SSID information, and when the user sets the model name to a 
non-NULL string it will force a specific quirk to be used?
Thanks!

Re: [PATCH] ASoC: snd-sof-intel-hda-common - add hda_model parameter and pass it to HDA codec driver

[Takashi Iwai]

On Fri, 24 Apr 2020 17:44:27 +0200,
Pierre-Louis Bossart wrote:
> 
> 
> 
> >> Hmm, I wonder is this now doing more harm than good. Based on browsing
> >> through the related code in hda-codec.c and friends, it would seem
> >> "sofbus" as the default is mostly harmless, but I could have missed
> >> something.
> >
> > That's currently harmless since no codec driver defines "sofbus"
> > model, hence the HDA parser continues to match with the default
> > quirks. OTOH, the fixed "sofbus" model is fairly useless.  So, feel
> > free to take my ack, too:
> 
> For my education, are you saying that the default should be that the
> modelname is NULL, and the hda auto parser will use known quirks based
> on PCI/SSID information, and when the user sets the model name to a
> non-NULL string it will force a specific quirk to be used?

Yes.  If the given string matches with the pre-defined table, the
quirk entry is used and applied.  If no string is given or it doesn't
match, it continues to the fallback quirk, that is, matching with PCI
SSID, then codec SSID.


Takashi

Re: [PATCH] ASoC: snd-sof-intel-hda-common - add hda_model parameter and pass it to HDA codec driver

[Mark Brown]

On Fri, 24 Apr 2020 11:25:20 +0200, Jaroslav Kysela wrote:
> It may be useful to pass the specific model to the generic HDA codec
> routines like the legacy HDA driver (snd-hda-intel) allows.
> The model name "sofbus" is tricky anyway.
> 
> Original proposal: https://lore.kernel.org/alsa-devel/20191203161908.7496-1-perex@perex.cz/
> 
> Signed-off-by: Jaroslav Kysela <perex@perex.cz>
> Reviewed-by: Takashi Iwai <tiwai@suse.de>
> Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Cc: Mark Brown <broonie@kernel.org>
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.8

Thanks!

[1/1] ASoC: snd-sof-intel-hda-common - add hda_model parameter and pass it to HDA codec driver
      commit: b8d3ad51dfec3631763cfef3d30c16f40140058b

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

Re: [PATCH] ASoC: snd-sof-intel-hda-common - add hda_model parameter and pass it to HDA codec driver

[Pierre-Louis Bossart]

>>>> Hmm, I wonder is this now doing more harm than good. Based on browsing
>>>> through the related code in hda-codec.c and friends, it would seem
>>>> "sofbus" as the default is mostly harmless, but I could have missed
>>>> something.
>>>
>>> That's currently harmless since no codec driver defines "sofbus"
>>> model, hence the HDA parser continues to match with the default
>>> quirks. OTOH, the fixed "sofbus" model is fairly useless.  So, feel
>>> free to take my ack, too:
>>
>> For my education, are you saying that the default should be that the
>> modelname is NULL, and the hda auto parser will use known quirks based
>> on PCI/SSID information, and when the user sets the model name to a
>> non-NULL string it will force a specific quirk to be used?
> 
> Yes.  If the given string matches with the pre-defined table, the
> quirk entry is used and applied.  If no string is given or it doesn't
> match, it continues to the fallback quirk, that is, matching with PCI
> SSID, then codec SSID.

Sounds good, thanks for the precisions Takashi