| Message ID | 20200424092520.23989-1-perex@perex.cz (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b8d3ad51dfec3631763cfef3d30c16f40140058b |
| Headers | show |
| Series | ASoC: snd-sof-intel-hda-common - add hda_model parameter and pass it to HDA codec driver | expand |
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
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
>> 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!
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
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
>>>> 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
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);
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(-)