diff mbox series

[RFC,v2,2/6] ASoC: Intel: skl-hda-dsp-generic: use snd-hda-codec-hdmi

Message ID 20190906142909.770-3-kai.vehmanen@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series adapt SOF to use snd-hda-codec-hdmi | expand

Commit Message

Kai Vehmanen Sept. 6, 2019, 2:29 p.m. UTC 
Add support for using snd-hda-codec-hdmi driver for HDMI/DP
instead of ASoC hdac-hdmi. This is aligned with how other
HDA codecs are already handled.

When snd-hda-codec-hdmi is used, the PCM device numbers are
parsed from card topology and passed to the codec driver.
This needs to be done at runtime as topology changes may
affect PCM device allocation.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 sound/soc/intel/boards/skl_hda_dsp_common.c  | 26 +++++++-
 sound/soc/intel/boards/skl_hda_dsp_common.h  | 69 ++++++++++++++++++++
 sound/soc/intel/boards/skl_hda_dsp_generic.c |  7 --
 3 files changed, 94 insertions(+), 8 deletions(-)

Comments

Pierre-Louis Bossart Sept. 6, 2019, 3:10 p.m. UTC | #1 
On 9/6/19 9:29 AM, Kai Vehmanen wrote:
> Add support for using snd-hda-codec-hdmi driver for HDMI/DP
> instead of ASoC hdac-hdmi. This is aligned with how other
> HDA codecs are already handled.
> 
> When snd-hda-codec-hdmi is used, the PCM device numbers are
> parsed from card topology and passed to the codec driver.
> This needs to be done at runtime as topology changes may
> affect PCM device allocation.
> 
> Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> ---
>   sound/soc/intel/boards/skl_hda_dsp_common.c  | 26 +++++++-
>   sound/soc/intel/boards/skl_hda_dsp_common.h  | 69 ++++++++++++++++++++
>   sound/soc/intel/boards/skl_hda_dsp_generic.c |  7 --
>   3 files changed, 94 insertions(+), 8 deletions(-)
> 
> diff --git a/sound/soc/intel/boards/skl_hda_dsp_common.c b/sound/soc/intel/boards/skl_hda_dsp_common.c
> index 55fd82e05e2c..23784178015b 100644
> --- a/sound/soc/intel/boards/skl_hda_dsp_common.c
> +++ b/sound/soc/intel/boards/skl_hda_dsp_common.c
> @@ -14,6 +14,9 @@
>   #include "../../codecs/hdac_hdmi.h"
>   #include "skl_hda_dsp_common.h"
>   
> +#include <sound/hda_codec.h>
> +#include "../../codecs/hdac_hda.h"
> +
>   #define NAME_SIZE	32
>   
>   int skl_hda_hdmi_add_pcm(struct snd_soc_card *card, int device)
> @@ -133,17 +136,38 @@ int skl_hda_hdmi_jack_init(struct snd_soc_card *card)
>   	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(card);
>   	struct snd_soc_component *component = NULL;
>   	struct skl_hda_hdmi_pcm *pcm;
> +	const char *modname;
> +	int use_common_codec = 0;
>   	char jack_name[NAME_SIZE];
>   	int err;
>   
> +	/*
> +	 * Detect whether this machine driver is used with SOF and
> +	 * alter behaviour based on which HDMI codec driver has
> +	 * been selected.
> +	 */
> +	if (!strncmp(card->name, "sof-skl_hda_card", 16)) {
> +		for_each_card_components(card, component) {
> +			modname = module_name(component->dev->driver->owner);
> +			if (!strncmp(component->name, "ehdaudio0D2", 11) &&
> +			    !strncmp(modname, "snd_hda_codec_hdmi", 18))
> +				use_common_codec = 1;
> +		}
> +	}

yuk. I am not a big fan of this...

It seems that we could pass information from the SOF side to the machine 
driver using the mach_params argument. we already pass the codec_mask 
and other fields, it wouldn't be too hard to reclaim a field or extend 
the structure to pass the information.

> +
> +	if (use_common_codec)
> +		return skl_hda_hdmi_build_controls(card);
> +
>   	list_for_each_entry(pcm, &ctx->hdmi_pcm_list, head) {
> +		if (!pcm)
> +			continue;
> +
>   		component = pcm->codec_dai->component;
>   		snprintf(jack_name, sizeof(jack_name),
>   			 "HDMI/DP, pcm=%d Jack", pcm->device);
>   		err = snd_soc_card_jack_new(card, jack_name,
>   					    SND_JACK_AVOUT, &pcm->hdmi_jack,
>   					    NULL, 0);
> -
>   		if (err)
>   			return err;
>   
> diff --git a/sound/soc/intel/boards/skl_hda_dsp_common.h b/sound/soc/intel/boards/skl_hda_dsp_common.h
> index daa582e513b2..5438631be565 100644
> --- a/sound/soc/intel/boards/skl_hda_dsp_common.h
> +++ b/sound/soc/intel/boards/skl_hda_dsp_common.h
> @@ -14,6 +14,8 @@
>   #include <linux/platform_device.h>
>   #include <sound/core.h>
>   #include <sound/jack.h>
> +#include <sound/hda_codec.h>
> +#include "../../codecs/hdac_hda.h"
>   
>   #define HDA_DSP_MAX_BE_DAI_LINKS 7
>   
> @@ -35,4 +37,71 @@ extern struct snd_soc_dai_link skl_hda_be_dai_links[HDA_DSP_MAX_BE_DAI_LINKS];
>   int skl_hda_hdmi_jack_init(struct snd_soc_card *card);
>   int skl_hda_hdmi_add_pcm(struct snd_soc_card *card, int device);
>   
> +/*
> + * Search card topology and return PCM device number
> + * matching Nth HDMI device (zero-based index).
> + */
> +static inline struct snd_pcm *skl_hda_hdmi_pcm_handle(struct snd_soc_card *card,
> +						      int hdmi_idx)
> +{
> +	struct snd_soc_pcm_runtime *rtd;
> +	int i = 0;
> +	struct snd_pcm *spcm;
> +
> +	for_each_card_rtds(card, rtd) {
> +		spcm = rtd->pcm ?
> +			rtd->pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].pcm : 0;
> +		if (spcm && strstr(spcm->id, "HDMI")) {
> +			if (i == hdmi_idx)
> +				return rtd->pcm;
> +			++i;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * Search card topology and register HDMI PCM related controls
> + * to codec driver.
> + */
> +static inline int skl_hda_hdmi_build_controls(struct snd_soc_card *card)
> +{
> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(card);
> +	struct snd_soc_component *component;
> +	struct hdac_hda_priv *hda_pvt;
> +	struct skl_hda_hdmi_pcm *pcm;
> +	struct hda_codec *hcodec;
> +	struct snd_pcm *spcm;
> +	struct hda_pcm *hpcm;
> +	int err = 0, i = 0;
> +
> +	pcm = list_first_entry(&ctx->hdmi_pcm_list, struct skl_hda_hdmi_pcm,
> +			       head);
> +	component = pcm->codec_dai->component;
> +	if (!component)
> +		return -EINVAL;
> +
> +	hda_pvt = snd_soc_component_get_drvdata(component);
> +	hcodec = &hda_pvt->codec;
> +
> +	list_for_each_entry(hpcm, &hcodec->pcm_list_head, list) {
> +		spcm = skl_hda_hdmi_pcm_handle(card, i);
> +		if (spcm) {
> +			hpcm->pcm = spcm;
> +			hpcm->device = spcm->device;
> +			dev_dbg(card->dev,
> +				"%s: mapping HDMI converter %d to PCM %d (%p)\n",
> +				__func__, i, hpcm->device, spcm);
> +		}
> +		i++;
> +	}
> +
> +	err = snd_hda_codec_build_controls(hcodec);
> +	if (err < 0)
> +		dev_err(card->dev, "unable to create controls %d\n", err);
> +
> +	return err;
> +}
> +
>   #endif /* __SOUND_SOC_HDA_DSP_COMMON_H */
> diff --git a/sound/soc/intel/boards/skl_hda_dsp_generic.c b/sound/soc/intel/boards/skl_hda_dsp_generic.c
> index 9ed68eb4f058..9b6f8cc87f99 100644
> --- a/sound/soc/intel/boards/skl_hda_dsp_generic.c
> +++ b/sound/soc/intel/boards/skl_hda_dsp_generic.c
> @@ -26,13 +26,6 @@ static const struct snd_soc_dapm_widget skl_hda_widgets[] = {
>   };
>   
>   static const struct snd_soc_dapm_route skl_hda_map[] = {
> -	{ "hifi3", NULL, "iDisp3 Tx"},
> -	{ "iDisp3 Tx", NULL, "iDisp3_out"},
> -	{ "hifi2", NULL, "iDisp2 Tx"},
> -	{ "iDisp2 Tx", NULL, "iDisp2_out"},
> -	{ "hifi1", NULL, "iDisp1 Tx"},
> -	{ "iDisp1 Tx", NULL, "iDisp1_out"},
> -
>   	{ "Analog Out", NULL, "Codec Output Pin1" },
>   	{ "Digital Out", NULL, "Codec Output Pin2" },
>   	{ "Alt Analog Out", NULL, "Codec Output Pin3" },
>
Kai Vehmanen Sept. 6, 2019, 3:33 p.m. UTC | #2 
Hey,

On Fri, 6 Sep 2019, Pierre-Louis Bossart wrote:
> > +	if (!strncmp(card->name, "sof-skl_hda_card", 16)) {
> > +		for_each_card_components(card, component) {
> > +			modname = module_name(component->dev->driver->owner);
> > +			if (!strncmp(component->name, "ehdaudio0D2", 11) &&
> > +			    !strncmp(modname, "snd_hda_codec_hdmi", 18))
> > +				use_common_codec = 1;
> > +		}
> > +	}
> 
> yuk. I am not a big fan of this...
> 
> It seems that we could pass information from the SOF side to the machine
> driver using the mach_params argument. we already pass the codec_mask and
> other fields, it wouldn't be too hard to reclaim a field or extend the
> structure to pass the information.

it is ugly, no question about it. :) My reasoning for this was to contain 
the ugliness within the machine drivers (especially these that are used 
with both SOF and SST). New machine drivers with no legacy to maintain 
could just skip it.

If the general concept of compiling both codec drivers in, and selecting 
at runtime, is acceptable, I'll check how this would look via mach_params.

Br, Kai
diff mbox series

Patch

diff --git a/sound/soc/intel/boards/skl_hda_dsp_common.c b/sound/soc/intel/boards/skl_hda_dsp_common.c
index 55fd82e05e2c..23784178015b 100644
--- a/sound/soc/intel/boards/skl_hda_dsp_common.c
+++ b/sound/soc/intel/boards/skl_hda_dsp_common.c
@@ -14,6 +14,9 @@ 
 #include "../../codecs/hdac_hdmi.h"
 #include "skl_hda_dsp_common.h"
 
+#include <sound/hda_codec.h>
+#include "../../codecs/hdac_hda.h"
+
 #define NAME_SIZE	32
 
 int skl_hda_hdmi_add_pcm(struct snd_soc_card *card, int device)
@@ -133,17 +136,38 @@  int skl_hda_hdmi_jack_init(struct snd_soc_card *card)
 	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(card);
 	struct snd_soc_component *component = NULL;
 	struct skl_hda_hdmi_pcm *pcm;
+	const char *modname;
+	int use_common_codec = 0;
 	char jack_name[NAME_SIZE];
 	int err;
 
+	/*
+	 * Detect whether this machine driver is used with SOF and
+	 * alter behaviour based on which HDMI codec driver has
+	 * been selected.
+	 */
+	if (!strncmp(card->name, "sof-skl_hda_card", 16)) {
+		for_each_card_components(card, component) {
+			modname = module_name(component->dev->driver->owner);
+			if (!strncmp(component->name, "ehdaudio0D2", 11) &&
+			    !strncmp(modname, "snd_hda_codec_hdmi", 18))
+				use_common_codec = 1;
+		}
+	}
+
+	if (use_common_codec)
+		return skl_hda_hdmi_build_controls(card);
+
 	list_for_each_entry(pcm, &ctx->hdmi_pcm_list, head) {
+		if (!pcm)
+			continue;
+
 		component = pcm->codec_dai->component;
 		snprintf(jack_name, sizeof(jack_name),
 			 "HDMI/DP, pcm=%d Jack", pcm->device);
 		err = snd_soc_card_jack_new(card, jack_name,
 					    SND_JACK_AVOUT, &pcm->hdmi_jack,
 					    NULL, 0);
-
 		if (err)
 			return err;
 
diff --git a/sound/soc/intel/boards/skl_hda_dsp_common.h b/sound/soc/intel/boards/skl_hda_dsp_common.h
index daa582e513b2..5438631be565 100644
--- a/sound/soc/intel/boards/skl_hda_dsp_common.h
+++ b/sound/soc/intel/boards/skl_hda_dsp_common.h
@@ -14,6 +14,8 @@ 
 #include <linux/platform_device.h>
 #include <sound/core.h>
 #include <sound/jack.h>
+#include <sound/hda_codec.h>
+#include "../../codecs/hdac_hda.h"
 
 #define HDA_DSP_MAX_BE_DAI_LINKS 7
 
@@ -35,4 +37,71 @@  extern struct snd_soc_dai_link skl_hda_be_dai_links[HDA_DSP_MAX_BE_DAI_LINKS];
 int skl_hda_hdmi_jack_init(struct snd_soc_card *card);
 int skl_hda_hdmi_add_pcm(struct snd_soc_card *card, int device);
 
+/*
+ * Search card topology and return PCM device number
+ * matching Nth HDMI device (zero-based index).
+ */
+static inline struct snd_pcm *skl_hda_hdmi_pcm_handle(struct snd_soc_card *card,
+						      int hdmi_idx)
+{
+	struct snd_soc_pcm_runtime *rtd;
+	int i = 0;
+	struct snd_pcm *spcm;
+
+	for_each_card_rtds(card, rtd) {
+		spcm = rtd->pcm ?
+			rtd->pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].pcm : 0;
+		if (spcm && strstr(spcm->id, "HDMI")) {
+			if (i == hdmi_idx)
+				return rtd->pcm;
+			++i;
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * Search card topology and register HDMI PCM related controls
+ * to codec driver.
+ */
+static inline int skl_hda_hdmi_build_controls(struct snd_soc_card *card)
+{
+	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(card);
+	struct snd_soc_component *component;
+	struct hdac_hda_priv *hda_pvt;
+	struct skl_hda_hdmi_pcm *pcm;
+	struct hda_codec *hcodec;
+	struct snd_pcm *spcm;
+	struct hda_pcm *hpcm;
+	int err = 0, i = 0;
+
+	pcm = list_first_entry(&ctx->hdmi_pcm_list, struct skl_hda_hdmi_pcm,
+			       head);
+	component = pcm->codec_dai->component;
+	if (!component)
+		return -EINVAL;
+
+	hda_pvt = snd_soc_component_get_drvdata(component);
+	hcodec = &hda_pvt->codec;
+
+	list_for_each_entry(hpcm, &hcodec->pcm_list_head, list) {
+		spcm = skl_hda_hdmi_pcm_handle(card, i);
+		if (spcm) {
+			hpcm->pcm = spcm;
+			hpcm->device = spcm->device;
+			dev_dbg(card->dev,
+				"%s: mapping HDMI converter %d to PCM %d (%p)\n",
+				__func__, i, hpcm->device, spcm);
+		}
+		i++;
+	}
+
+	err = snd_hda_codec_build_controls(hcodec);
+	if (err < 0)
+		dev_err(card->dev, "unable to create controls %d\n", err);
+
+	return err;
+}
+
 #endif /* __SOUND_SOC_HDA_DSP_COMMON_H */
diff --git a/sound/soc/intel/boards/skl_hda_dsp_generic.c b/sound/soc/intel/boards/skl_hda_dsp_generic.c
index 9ed68eb4f058..9b6f8cc87f99 100644
--- a/sound/soc/intel/boards/skl_hda_dsp_generic.c
+++ b/sound/soc/intel/boards/skl_hda_dsp_generic.c
@@ -26,13 +26,6 @@  static const struct snd_soc_dapm_widget skl_hda_widgets[] = {
 };
 
 static const struct snd_soc_dapm_route skl_hda_map[] = {
-	{ "hifi3", NULL, "iDisp3 Tx"},
-	{ "iDisp3 Tx", NULL, "iDisp3_out"},
-	{ "hifi2", NULL, "iDisp2 Tx"},
-	{ "iDisp2 Tx", NULL, "iDisp2_out"},
-	{ "hifi1", NULL, "iDisp1 Tx"},
-	{ "iDisp1 Tx", NULL, "iDisp1_out"},
-
 	{ "Analog Out", NULL, "Codec Output Pin1" },
 	{ "Digital Out", NULL, "Codec Output Pin2" },
 	{ "Alt Analog Out", NULL, "Codec Output Pin3" },