On 7/31/18 3:48 AM, Takashi Iwai wrote:
On Sat, 28 Jul 2018 01:05:54 +0200, Pierre-Louis Bossart wrote:
Add two options to explicitly enable HDAudio + DSP usage and force its use
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
sound/soc/intel/Kconfig | 19 +++++++++++++++++++ sound/soc/intel/skylake/skl.c | 7 ++++++- 2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig index 0caa1f4eb94d..93c3693cdf13 100644 --- a/sound/soc/intel/Kconfig +++ b/sound/soc/intel/Kconfig @@ -117,6 +117,25 @@ config SND_SOC_INTEL_SKYLAKE GeminiLake or CannonLake platform with the DSP enabled in the BIOS then enable this option by saying Y or m.
+config SND_SOC_INTEL_SKYLAKE_HDA_DSP
- bool "Enable HDAudio Codec + DSP support"
- depends on SND_SOC_INTEL_SKYLAKE
- help
If you have a Intel Skylake+ platform with the DSP enabled inthe BIOS and an HDAudio codec, then enable this option by saying YThis option will only have an effect if there are no ACPI-enumeratedaudio devices (I2C, SoundWire).IMO, the change needed for this config should be folded into the patch 4 "ASoC: Intel: Skylake: use HDAudio if ACPI enumeration fails").
Yes, agreed.
+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 inthe BIOS and an HDAudio codec, then enable this option by saying YThis option will ignore information from the BIOS and force the useof 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.
Just another nitpicking:
@@ -474,6 +474,7 @@ static struct skl_ssp_clk skl_ssp_clks[] = { {.name = "ssp5_sclkfs"}, };
+#if IS_ENABLED(CONFIG_SND_SOC_INTEL_SKYLAKE_HDA_DSP) static struct snd_soc_acpi_mach *skl_find_hda_machine(struct skl *skl, struct snd_soc_acpi_mach *machines) { @@ -492,6 +493,7 @@ static struct snd_soc_acpi_mach *skl_find_hda_machine(struct skl *skl,
return mach; } +#endif
Defining a dumb skl_find_hda_machine() returning NULL as #else would reduce one more ifdef. Also, CONFIG_SND_SOC_INTEL_SKYLAKE_HDA_DSP is a bool, so it can be a simple ifdef without IS_ENABLED().
#ifdef CONFIG_SND_SOC_INTEL_SKYLAKE_HDA_DSP static struct snd_soc_acpi_mach *skl_find_hda_machine(struct skl *skl, struct snd_soc_acpi_mach *machines) { .... } #else static struct snd_soc_acpi_mach *skl_find_hda_machine(struct skl *skl, struct snd_soc_acpi_mach *machines) { return NULL; } #endif
yes, thanks for the suggestion.
thanks,
Takashi