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@xxxxxxxxxxxxxxx>
---
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 in
+ the BIOS and an HDAudio codec, then enable this option by saying Y
+ This option will only have an effect if there are no ACPI-enumerated
+ audio 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 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.
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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
