Message ID | 20190910182916.29693-5-kai.vehmanen@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | adapt SOF to use snd-hda-codec-hdmi | expand |
On 9/10/19 1:29 PM, Kai Vehmanen wrote: > Add support to implement HDMI/DP audio by using the common > snd-hda-codec-hdmi driver. > > Change of codec driver affects user-space as the the two > drivers expose different mixer controls. A new kernel > module option "use_common_hdmi" is added to user-space > to indicate which interface should be used. The default > driver can be selected via a Kconfig option. > > Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> > --- > sound/soc/sof/intel/Kconfig | 10 ++++++++++ > sound/soc/sof/intel/hda-codec.c | 19 +++++++++++++++---- > sound/soc/sof/intel/hda.c | 6 ++++++ > sound/soc/sof/intel/hda.h | 6 ++++-- > 4 files changed, 35 insertions(+), 6 deletions(-) > > diff --git a/sound/soc/sof/intel/Kconfig b/sound/soc/sof/intel/Kconfig > index 479ba249e219..a8f4e69b044d 100644 > --- a/sound/soc/sof/intel/Kconfig > +++ b/sound/soc/sof/intel/Kconfig > @@ -273,6 +273,16 @@ config SND_SOC_SOF_HDA_AUDIO_CODEC > Say Y if you want to enable HDAudio codecs with SOF. > If unsure select "N". > > +config SND_SOC_SOF_HDA_COMMON_HDMI_CODEC > + bool "SOF common HDA HDMI codec driver" > + depends on SND_SOC_SOF_HDA_LINK > + depends on SND_HDA_CODEC_HDMI > + help > + This adds support for HDMI audio by using the common HDA > + HDMI/DisplayPort codec driver. > + Say Y if you want to use the common codec driver with SOF. > + If unsure select "Y". > + > endif ## SND_SOC_SOF_HDA_COMMON > > config SND_SOC_SOF_HDA_LINK_BASELINE > diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c > index 3ca6795a89ba..817ebba00b47 100644 > --- a/sound/soc/sof/intel/hda-codec.c > +++ b/sound/soc/sof/intel/hda-codec.c > @@ -84,6 +84,8 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int address) > { > #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC) > struct hdac_hda_priv *hda_priv; > + struct snd_soc_acpi_mach_params *mach_params = 0; > + struct snd_sof_pdata *pdata = sdev->pdata; > #endif > struct hda_bus *hbus = sof_to_hbus(sdev); > struct hdac_device *hdev; > @@ -113,8 +115,16 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int address) > if (ret < 0) > return ret; > > - /* use legacy bus only for HDA codecs, idisp uses ext bus */ > - if ((resp & 0xFFFF0000) != IDISP_VID_INTEL) { > + if (pdata->machine) > + mach_params = (struct snd_soc_acpi_mach_params *) > + &pdata->machine->mach_params; > + > + /* > + * if common HDMI codec driver is not used, codec load > + * is skipped here and hdac_hdmi is used instead > + */ > + if ((mach_params && mach_params->common_hdmi_codec_drv) || > + (resp & 0xFFFF0000) != IDISP_VID_INTEL) { > hdev->type = HDA_DEV_LEGACY; > hda_codec_load_module(&hda_priv->codec); This part is might be problematic. For SoundWire stuff, I had to move all the machine detection part out of hda_init_caps() and at the end of hda_dsp_probe. It's not final since I am still trying to figure out what the earliest time I can power-up the SoundWire IP is, but it would help if you don't make strong assumptions on when mach_params is set. The fact that all this code is currently in hda_init_caps() is not really by design, more because of incremental code changes. > } > @@ -155,7 +165,8 @@ int hda_codec_probe_bus(struct snd_sof_dev *sdev) > } > EXPORT_SYMBOL(hda_codec_probe_bus); > > -#if IS_ENABLED(CONFIG_SND_SOC_HDAC_HDMI) > +#if IS_ENABLED(CONFIG_SND_HDA_CODEC_HDMI) || \ > + IS_ENABLED(CONFIG_SND_SOC_HDAC_HDMI) > > void hda_codec_i915_get(struct snd_sof_dev *sdev) > { > @@ -204,6 +215,6 @@ int hda_codec_i915_exit(struct snd_sof_dev *sdev) > } > EXPORT_SYMBOL(hda_codec_i915_exit); > > -#endif /* CONFIG_SND_SOC_HDAC_HDMI */ > +#endif > > MODULE_LICENSE("Dual BSD/GPL"); > diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c > index c72e9a09eee1..ee742157516e 100644 > --- a/sound/soc/sof/intel/hda.c > +++ b/sound/soc/sof/intel/hda.c > @@ -54,6 +54,11 @@ MODULE_PARM_DESC(use_msi, "SOF HDA use PCI MSI mode"); > static int hda_dmic_num = -1; > module_param_named(dmic_num, hda_dmic_num, int, 0444); > MODULE_PARM_DESC(dmic_num, "SOF HDA DMIC number"); > + > +static bool hda_codec_use_common_hdmi = > + IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_COMMON_HDMI_CODEC); > +module_param_named(use_common_hdmi, hda_codec_use_common_hdmi, bool, 0444); > +MODULE_PARM_DESC(use_common_hdmi, "SOF HDA use common HDMI codec driver"); > #endif > > static const struct hda_dsp_msg_code hda_dsp_rom_msg[] = { > @@ -458,6 +463,7 @@ static int hda_init_caps(struct snd_sof_dev *sdev) > &pdata->machine->mach_params; > mach_params->codec_mask = bus->codec_mask; > mach_params->platform = dev_name(sdev->dev); > + mach_params->common_hdmi_codec_drv = hda_codec_use_common_hdmi; > } > > /* create codec instances */ > diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h > index 5591841a1b6f..28640a29e1b6 100644 > --- a/sound/soc/sof/intel/hda.h > +++ b/sound/soc/sof/intel/hda.h > @@ -562,7 +562,9 @@ void hda_codec_jack_check(struct snd_sof_dev *sdev); > > #endif /* CONFIG_SND_SOC_SOF_HDA */ > > -#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) && IS_ENABLED(CONFIG_SND_SOC_HDAC_HDMI) > +#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) && \ > + (IS_ENABLED(CONFIG_SND_HDA_CODEC_HDMI) || \ > + IS_ENABLED(CONFIG_SND_SOC_HDAC_HDMI)) > > void hda_codec_i915_get(struct snd_sof_dev *sdev); > void hda_codec_i915_put(struct snd_sof_dev *sdev); > @@ -576,7 +578,7 @@ static inline void hda_codec_i915_put(struct snd_sof_dev *sdev) { } > static inline int hda_codec_i915_init(struct snd_sof_dev *sdev) { return 0; } > static inline int hda_codec_i915_exit(struct snd_sof_dev *sdev) { return 0; } > > -#endif /* CONFIG_SND_SOC_SOF_HDA && CONFIG_SND_SOC_HDAC_HDMI */ > +#endif > > /* > * Trace Control. >
Hey, On Tue, 10 Sep 2019, Pierre-Louis Bossart wrote: > > --- a/sound/soc/sof/intel/hda-codec.c > > +++ b/sound/soc/sof/intel/hda-codec.c > > @@ -84,6 +84,8 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int [...] > > + /* > > + * if common HDMI codec driver is not used, codec load > > + * is skipped here and hdac_hdmi is used instead > > + */ > > + if ((mach_params && mach_params->common_hdmi_codec_drv) || > > + (resp & 0xFFFF0000) != IDISP_VID_INTEL) { > > hdev->type = HDA_DEV_LEGACY; > > hda_codec_load_module(&hda_priv->codec); > > This part is might be problematic. For SoundWire stuff, I had to move all the > machine detection part out of hda_init_caps() and at the end of hda_dsp_probe. > It's not final since I am still trying to figure out what the earliest time I > can power-up the SoundWire IP is, but it would help if you don't make strong > assumptions on when mach_params is set. The fact that all this code is > currently in hda_init_caps() is not really by design, more because of > incremental code changes. hmm. Currently the settings part of 'common_hdmi_codec_drv' is in in init_caps, just before call to hda_codec_probe_bus() (which uses the flag). There are also other fields set there (like mach_params->codec_mask), so issues should apply to codec_mask passing as well. I.e. if functionality is moved out from init_caps, the call to bus probe should be moved as well. Other options to pass the flag do not seem good. We don't want to add hw specific stuff like this to snd_sof_dev or hdac_bus (latter used to pass "codec_mask"). If this becomes an issue, maybe we just need an explicit parameter to hda_codec_probe_bus(), and/or move the module parameter directly to intel/hda-codec.c. Both are a bit ugly, but at least contained in the "sof/intel/" subfolder. Br, Kai
On 9/12/19 7:06 AM, Kai Vehmanen wrote: > Hey, > > On Tue, 10 Sep 2019, Pierre-Louis Bossart wrote: >>> --- a/sound/soc/sof/intel/hda-codec.c >>> +++ b/sound/soc/sof/intel/hda-codec.c >>> @@ -84,6 +84,8 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int > [...] >>> + /* >>> + * if common HDMI codec driver is not used, codec load >>> + * is skipped here and hdac_hdmi is used instead >>> + */ >>> + if ((mach_params && mach_params->common_hdmi_codec_drv) || >>> + (resp & 0xFFFF0000) != IDISP_VID_INTEL) { >>> hdev->type = HDA_DEV_LEGACY; >>> hda_codec_load_module(&hda_priv->codec); >> >> This part is might be problematic. For SoundWire stuff, I had to move all the >> machine detection part out of hda_init_caps() and at the end of hda_dsp_probe. >> It's not final since I am still trying to figure out what the earliest time I >> can power-up the SoundWire IP is, but it would help if you don't make strong >> assumptions on when mach_params is set. The fact that all this code is >> currently in hda_init_caps() is not really by design, more because of >> incremental code changes. > > hmm. Currently the settings part of 'common_hdmi_codec_drv' is in in > init_caps, just before call to hda_codec_probe_bus() (which uses the > flag). There are also other fields set there (like > mach_params->codec_mask), so issues should apply to codec_mask passing as > well. I.e. if functionality is moved out from init_caps, the call to bus > probe should be moved as well. > > Other options to pass the flag do not seem good. We don't want to add hw > specific stuff like this to snd_sof_dev or hdac_bus (latter used to pass > "codec_mask"). If this becomes an issue, maybe we just need an explicit > parameter to hda_codec_probe_bus(), and/or move the module parameter > directly to intel/hda-codec.c. Both are a bit ugly, but at least contained > in the "sof/intel/" subfolder. never mind, with the SoundWire plumbing rework we can do all the machine driver checks in hda_init_caps. I split the ACPI scan, driver probe and startup in 3 steps so we have more freedom. Before this rework I was constrained to detect the ACPI stuff only when the hardware was powered, which was after init_caps. In short don't worry about me/SoundWire.
diff --git a/sound/soc/sof/intel/Kconfig b/sound/soc/sof/intel/Kconfig index 479ba249e219..a8f4e69b044d 100644 --- a/sound/soc/sof/intel/Kconfig +++ b/sound/soc/sof/intel/Kconfig @@ -273,6 +273,16 @@ config SND_SOC_SOF_HDA_AUDIO_CODEC Say Y if you want to enable HDAudio codecs with SOF. If unsure select "N". +config SND_SOC_SOF_HDA_COMMON_HDMI_CODEC + bool "SOF common HDA HDMI codec driver" + depends on SND_SOC_SOF_HDA_LINK + depends on SND_HDA_CODEC_HDMI + help + This adds support for HDMI audio by using the common HDA + HDMI/DisplayPort codec driver. + Say Y if you want to use the common codec driver with SOF. + If unsure select "Y". + endif ## SND_SOC_SOF_HDA_COMMON config SND_SOC_SOF_HDA_LINK_BASELINE diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c index 3ca6795a89ba..817ebba00b47 100644 --- a/sound/soc/sof/intel/hda-codec.c +++ b/sound/soc/sof/intel/hda-codec.c @@ -84,6 +84,8 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int address) { #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC) struct hdac_hda_priv *hda_priv; + struct snd_soc_acpi_mach_params *mach_params = 0; + struct snd_sof_pdata *pdata = sdev->pdata; #endif struct hda_bus *hbus = sof_to_hbus(sdev); struct hdac_device *hdev; @@ -113,8 +115,16 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int address) if (ret < 0) return ret; - /* use legacy bus only for HDA codecs, idisp uses ext bus */ - if ((resp & 0xFFFF0000) != IDISP_VID_INTEL) { + if (pdata->machine) + mach_params = (struct snd_soc_acpi_mach_params *) + &pdata->machine->mach_params; + + /* + * if common HDMI codec driver is not used, codec load + * is skipped here and hdac_hdmi is used instead + */ + if ((mach_params && mach_params->common_hdmi_codec_drv) || + (resp & 0xFFFF0000) != IDISP_VID_INTEL) { hdev->type = HDA_DEV_LEGACY; hda_codec_load_module(&hda_priv->codec); } @@ -155,7 +165,8 @@ int hda_codec_probe_bus(struct snd_sof_dev *sdev) } EXPORT_SYMBOL(hda_codec_probe_bus); -#if IS_ENABLED(CONFIG_SND_SOC_HDAC_HDMI) +#if IS_ENABLED(CONFIG_SND_HDA_CODEC_HDMI) || \ + IS_ENABLED(CONFIG_SND_SOC_HDAC_HDMI) void hda_codec_i915_get(struct snd_sof_dev *sdev) { @@ -204,6 +215,6 @@ int hda_codec_i915_exit(struct snd_sof_dev *sdev) } EXPORT_SYMBOL(hda_codec_i915_exit); -#endif /* CONFIG_SND_SOC_HDAC_HDMI */ +#endif MODULE_LICENSE("Dual BSD/GPL"); diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index c72e9a09eee1..ee742157516e 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -54,6 +54,11 @@ MODULE_PARM_DESC(use_msi, "SOF HDA use PCI MSI mode"); static int hda_dmic_num = -1; module_param_named(dmic_num, hda_dmic_num, int, 0444); MODULE_PARM_DESC(dmic_num, "SOF HDA DMIC number"); + +static bool hda_codec_use_common_hdmi = + IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_COMMON_HDMI_CODEC); +module_param_named(use_common_hdmi, hda_codec_use_common_hdmi, bool, 0444); +MODULE_PARM_DESC(use_common_hdmi, "SOF HDA use common HDMI codec driver"); #endif static const struct hda_dsp_msg_code hda_dsp_rom_msg[] = { @@ -458,6 +463,7 @@ static int hda_init_caps(struct snd_sof_dev *sdev) &pdata->machine->mach_params; mach_params->codec_mask = bus->codec_mask; mach_params->platform = dev_name(sdev->dev); + mach_params->common_hdmi_codec_drv = hda_codec_use_common_hdmi; } /* create codec instances */ diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 5591841a1b6f..28640a29e1b6 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -562,7 +562,9 @@ void hda_codec_jack_check(struct snd_sof_dev *sdev); #endif /* CONFIG_SND_SOC_SOF_HDA */ -#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) && IS_ENABLED(CONFIG_SND_SOC_HDAC_HDMI) +#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) && \ + (IS_ENABLED(CONFIG_SND_HDA_CODEC_HDMI) || \ + IS_ENABLED(CONFIG_SND_SOC_HDAC_HDMI)) void hda_codec_i915_get(struct snd_sof_dev *sdev); void hda_codec_i915_put(struct snd_sof_dev *sdev); @@ -576,7 +578,7 @@ static inline void hda_codec_i915_put(struct snd_sof_dev *sdev) { } static inline int hda_codec_i915_init(struct snd_sof_dev *sdev) { return 0; } static inline int hda_codec_i915_exit(struct snd_sof_dev *sdev) { return 0; } -#endif /* CONFIG_SND_SOC_SOF_HDA && CONFIG_SND_SOC_HDAC_HDMI */ +#endif /* * Trace Control.
Add support to implement HDMI/DP audio by using the common snd-hda-codec-hdmi driver. Change of codec driver affects user-space as the the two drivers expose different mixer controls. A new kernel module option "use_common_hdmi" is added to user-space to indicate which interface should be used. The default driver can be selected via a Kconfig option. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> --- sound/soc/sof/intel/Kconfig | 10 ++++++++++ sound/soc/sof/intel/hda-codec.c | 19 +++++++++++++++---- sound/soc/sof/intel/hda.c | 6 ++++++ sound/soc/sof/intel/hda.h | 6 ++++-- 4 files changed, 35 insertions(+), 6 deletions(-)