[alsa-devel] [PATCH v5 8/8] ASoC: Intel: Skylake: add option to control HDAudio + DSP usage

[Pierre-Louis Bossart]

On 7/31/18 9:20 AM, Mark Brown wrote:
> On Tue, Jul 31, 2018 at 09:06:54AM -0700, Pierre-Louis Bossart wrote:
>> On 7/31/18 3:48 AM, Takashi Iwai wrote:
>>> Pierre-Louis Bossart wrote:
> 
>>>> +config SND_SOC_INTEL_SKYLAKE_FORCE_HDA_DSP
>>>> +	bool "Force HDAudio Codec + DSP support"
>>>> +	depends on SND_SOC_INTEL_SKYLAKE_HDA_DSP
>>>> +	help
>>>> +	  If you have a Intel Skylake+ platform with the DSP enabled in
>>>> +	  the BIOS and an HDAudio codec, then enable this option by saying Y
>>>> +	  This option will ignore information from the BIOS and force the use
>>>> +	  of the HDaudio codec, if present.
>>>> +	  This is a debug option not recommended for distros.
> 
>>> ... and this one is better to be a module option.
>>> Distros tend to enable all possible options, and this may be set
>>> unnecessarily.  If any, this kconfig should be only toggling the
>>> default option value.
> 
>> Sorry, I don't get this one. this wouldn't change anything between built-in
>> or module, it's just a yes/no answer to ignore ACPI stuff. the default is
>> also no and there is a clear statement not to include it except for debug.
>> We have a similar option for SOF to bypass all ACPI information and go
>> straight to 'nocodec' mode.
> 
> Debug options like these are really a lot more useful as something
> runtime selectable with something like a module parameter as Takashi
> suggests - an innatentive distro maintainer could easily enable it by
> mistake when going through a bunch of new options in a new kernel
> release and if someone needs it for debugging purposes in a distro
> context it's a real pain to have to rebuild the kernel.

Ah yes, I get it now and yes it's probably a better idea to have a 
kernel parameter. I saw multiple cases of distros such as Arch enabling 
the baytrail nocodec mode and it's not straightforward to reach all 
maintainers of all distros.
Will fix, thanks for the feedback.

> 
> It's also a bit confusing as the config text says:
> 
>>>> +	  If you have a Intel Skylake+ platform with the DSP enabled in
>>>> +	  the BIOS and an HDAudio codec, then enable this option by saying Y
> 
> which sounds like people should turn it on, the bit about it being a
> debug option is quite deep in the text.
>