diff mbox series

[v5,2/9] ASoC: hdac_hda: add support for HDMI/DP as a HDA codec

Message ID 20190925112409.1762-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. 25, 2019, 11:24 a.m. UTC 
Handle all HDA codecs using same logic, including HDMI/DP.

Call to snd_hda_codec_build_controls() is delayed for HDMI/DP HDA
devices. This is needed to discover the PCM device numbers as
defined in topology.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 sound/soc/codecs/hdac_hda.c | 110 ++++++++++++++++++++++++++++++++----
 sound/soc/codecs/hdac_hda.h |  13 ++++-
 2 files changed, 110 insertions(+), 13 deletions(-)

Comments

Kai Vehmanen Sept. 25, 2019, 11:38 a.m. UTC | #1 
Hello,

Takashi, please check whether this is ok. I'll highlight the changed 
section below:

On Wed, 25 Sep 2019, Kai Vehmanen wrote:

>  static int hdac_hda_codec_probe(struct snd_soc_component *component)
>  {
>  	struct hdac_hda_priv *hda_pvt =
> @@ -322,6 +392,15 @@ static int hdac_hda_codec_probe(struct snd_soc_component *component)
>  
>  	snd_hdac_ext_bus_link_get(hdev->bus, hlink);
>  
> +	/*
> +	 * Ensure any HDA display is powered at codec probe.
> +	 * After snd_hda_codec_device_new(), display power is
> +	 * managed by runtime PM.
> +	 */
> +	if (hda_pvt->need_display_power)
> +		snd_hdac_display_power(hdev->bus, HDA_CODEC_IDX_CONTROLLER,
> +				       true);
> +

This is the new bit (and matching logic in patch 5).The bug required a 
very specific timing sequence to trigger, but a clear bug nevertheless. I 
tried to fix it in spirit of your refactoring patch of to this area 
029d92c289bd, "ALSA: hda: Refactor display power management". I.e. just 
like snd_hda_intel's controller code, display power is enabled before 
probe and later managed by codec using common code.

Additional SOF specific twist is that I need to pass the 
"need_display_power" info from SOF code (where the initial codec probe is 
done and we detect a HDMI HDA codec)), to soc/codecs/hdac_hda.c where the 
actual driver probe is run for the codec. In snd_hda_intel this is all in 
one place, so somewhat more straighforward, but logic is the same.

Br, Kai
Takashi Iwai Oct. 17, 2019, 7:59 a.m. UTC | #2 
On Wed, 25 Sep 2019 13:38:50 +0200,
Kai Vehmanen wrote:
> 
> Hello,
> 
> Takashi, please check whether this is ok. I'll highlight the changed 
> section below:
> 
> On Wed, 25 Sep 2019, Kai Vehmanen wrote:
> 
> >  static int hdac_hda_codec_probe(struct snd_soc_component *component)
> >  {
> >  	struct hdac_hda_priv *hda_pvt =
> > @@ -322,6 +392,15 @@ static int hdac_hda_codec_probe(struct snd_soc_component *component)
> >  
> >  	snd_hdac_ext_bus_link_get(hdev->bus, hlink);
> >  
> > +	/*
> > +	 * Ensure any HDA display is powered at codec probe.
> > +	 * After snd_hda_codec_device_new(), display power is
> > +	 * managed by runtime PM.
> > +	 */
> > +	if (hda_pvt->need_display_power)
> > +		snd_hdac_display_power(hdev->bus, HDA_CODEC_IDX_CONTROLLER,
> > +				       true);
> > +
> 
> This is the new bit (and matching logic in patch 5).The bug required a 
> very specific timing sequence to trigger, but a clear bug nevertheless. I 
> tried to fix it in spirit of your refactoring patch of to this area 
> 029d92c289bd, "ALSA: hda: Refactor display power management". I.e. just 
> like snd_hda_intel's controller code, display power is enabled before 
> probe and later managed by codec using common code.
> 
> Additional SOF specific twist is that I need to pass the 
> "need_display_power" info from SOF code (where the initial codec probe is 
> done and we detect a HDMI HDA codec)), to soc/codecs/hdac_hda.c where the 
> actual driver probe is run for the codec. In snd_hda_intel this is all in 
> one place, so somewhat more straighforward, but logic is the same.

This change looks OK to me.  The recent display power management is no
longer refcounted, so it's fine to toggle power on at the early stage
like this patch does.


thanks,

Takashi
diff mbox series

Patch

diff --git a/sound/soc/codecs/hdac_hda.c b/sound/soc/codecs/hdac_hda.c
index 91242b6f8ea7..4b79a02d31f0 100644
--- a/sound/soc/codecs/hdac_hda.c
+++ b/sound/soc/codecs/hdac_hda.c
@@ -14,13 +14,11 @@ 
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
 #include <sound/hdaudio_ext.h>
+#include <sound/hda_i915.h>
 #include <sound/hda_codec.h>
 #include <sound/hda_register.h>
-#include "hdac_hda.h"
 
-#define HDAC_ANALOG_DAI_ID		0
-#define HDAC_DIGITAL_DAI_ID		1
-#define HDAC_ALT_ANALOG_DAI_ID		2
+#include "hdac_hda.h"
 
 #define STUB_FORMATS	(SNDRV_PCM_FMTBIT_S8 | \
 			SNDRV_PCM_FMTBIT_U8 | \
@@ -32,6 +30,11 @@ 
 			SNDRV_PCM_FMTBIT_U32_LE | \
 			SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE)
 
+#define STUB_HDMI_RATES	(SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 |\
+				 SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_88200 |\
+				 SNDRV_PCM_RATE_96000 | SNDRV_PCM_RATE_176400 |\
+				 SNDRV_PCM_RATE_192000)
+
 static int hdac_hda_dai_open(struct snd_pcm_substream *substream,
 			     struct snd_soc_dai *dai);
 static void hdac_hda_dai_close(struct snd_pcm_substream *substream,
@@ -121,7 +124,46 @@  static struct snd_soc_dai_driver hdac_hda_dais[] = {
 		.formats = STUB_FORMATS,
 		.sig_bits = 24,
 	},
-}
+},
+{
+	.id = HDAC_HDMI_0_DAI_ID,
+	.name = "intel-hdmi-hifi1",
+	.ops = &hdac_hda_dai_ops,
+	.playback = {
+		.stream_name    = "hifi1",
+		.channels_min   = 1,
+		.channels_max   = 32,
+		.rates          = STUB_HDMI_RATES,
+		.formats        = STUB_FORMATS,
+		.sig_bits = 24,
+	},
+},
+{
+	.id = HDAC_HDMI_1_DAI_ID,
+	.name = "intel-hdmi-hifi2",
+	.ops = &hdac_hda_dai_ops,
+	.playback = {
+		.stream_name    = "hifi2",
+		.channels_min   = 1,
+		.channels_max   = 32,
+		.rates          = STUB_HDMI_RATES,
+		.formats        = STUB_FORMATS,
+		.sig_bits = 24,
+	},
+},
+{
+	.id = HDAC_HDMI_2_DAI_ID,
+	.name = "intel-hdmi-hifi3",
+	.ops = &hdac_hda_dai_ops,
+	.playback = {
+		.stream_name    = "hifi3",
+		.channels_min   = 1,
+		.channels_max   = 32,
+		.rates          = STUB_HDMI_RATES,
+		.formats        = STUB_FORMATS,
+		.sig_bits = 24,
+	},
+},
 
 };
 
@@ -135,10 +177,11 @@  static int hdac_hda_dai_set_tdm_slot(struct snd_soc_dai *dai,
 
 	hda_pvt = snd_soc_component_get_drvdata(component);
 	pcm = &hda_pvt->pcm[dai->id];
+
 	if (tx_mask)
-		pcm[dai->id].stream_tag[SNDRV_PCM_STREAM_PLAYBACK] = tx_mask;
+		pcm->stream_tag[SNDRV_PCM_STREAM_PLAYBACK] = tx_mask;
 	else
-		pcm[dai->id].stream_tag[SNDRV_PCM_STREAM_CAPTURE] = rx_mask;
+		pcm->stream_tag[SNDRV_PCM_STREAM_CAPTURE] = rx_mask;
 
 	return 0;
 }
@@ -278,6 +321,12 @@  static struct hda_pcm *snd_soc_find_pcm_from_dai(struct hdac_hda_priv *hda_pvt,
 	struct hda_pcm *cpcm;
 	const char *pcm_name;
 
+	/*
+	 * map DAI ID to the closest matching PCM name, using the naming
+	 * scheme used by hda-codec snd_hda_gen_build_pcms() and for
+	 * HDMI in hda_codec patch_hdmi.c)
+	 */
+
 	switch (dai->id) {
 	case HDAC_ANALOG_DAI_ID:
 		pcm_name = "Analog";
@@ -288,13 +337,22 @@  static struct hda_pcm *snd_soc_find_pcm_from_dai(struct hdac_hda_priv *hda_pvt,
 	case HDAC_ALT_ANALOG_DAI_ID:
 		pcm_name = "Alt Analog";
 		break;
+	case HDAC_HDMI_0_DAI_ID:
+		pcm_name = "HDMI 0";
+		break;
+	case HDAC_HDMI_1_DAI_ID:
+		pcm_name = "HDMI 1";
+		break;
+	case HDAC_HDMI_2_DAI_ID:
+		pcm_name = "HDMI 2";
+		break;
 	default:
 		dev_err(&hcodec->core.dev, "invalid dai id %d\n", dai->id);
 		return NULL;
 	}
 
 	list_for_each_entry(cpcm, &hcodec->pcm_list_head, list) {
-		if (strpbrk(cpcm->name, pcm_name))
+		if (strstr(cpcm->name, pcm_name))
 			return cpcm;
 	}
 
@@ -302,6 +360,18 @@  static struct hda_pcm *snd_soc_find_pcm_from_dai(struct hdac_hda_priv *hda_pvt,
 	return NULL;
 }
 
+static bool is_hdmi_codec(struct hda_codec *hcodec)
+{
+	struct hda_pcm *cpcm;
+
+	list_for_each_entry(cpcm, &hcodec->pcm_list_head, list) {
+		if (cpcm->pcm_type == HDA_PCM_TYPE_HDMI)
+			return true;
+	}
+
+	return false;
+}
+
 static int hdac_hda_codec_probe(struct snd_soc_component *component)
 {
 	struct hdac_hda_priv *hda_pvt =
@@ -322,6 +392,15 @@  static int hdac_hda_codec_probe(struct snd_soc_component *component)
 
 	snd_hdac_ext_bus_link_get(hdev->bus, hlink);
 
+	/*
+	 * Ensure any HDA display is powered at codec probe.
+	 * After snd_hda_codec_device_new(), display power is
+	 * managed by runtime PM.
+	 */
+	if (hda_pvt->need_display_power)
+		snd_hdac_display_power(hdev->bus, HDA_CODEC_IDX_CONTROLLER,
+				       true);
+
 	ret = snd_hda_codec_device_new(hcodec->bus, component->card->snd_card,
 				       hdev->addr, hcodec);
 	if (ret < 0) {
@@ -366,16 +445,23 @@  static int hdac_hda_codec_probe(struct snd_soc_component *component)
 		dev_dbg(&hdev->dev, "no patch file found\n");
 	}
 
+	/* configure codec for 1:1 PCM:DAI mapping */
+	hcodec->mst_no_extra_pcms = 1;
+
 	ret = snd_hda_codec_parse_pcms(hcodec);
 	if (ret < 0) {
 		dev_err(&hdev->dev, "unable to map pcms to dai %d\n", ret);
 		goto error;
 	}
 
-	ret = snd_hda_codec_build_controls(hcodec);
-	if (ret < 0) {
-		dev_err(&hdev->dev, "unable to create controls %d\n", ret);
-		goto error;
+	/* HDMI controls need to be created in machine drivers */
+	if (!is_hdmi_codec(hcodec)) {
+		ret = snd_hda_codec_build_controls(hcodec);
+		if (ret < 0) {
+			dev_err(&hdev->dev, "unable to create controls %d\n",
+				ret);
+			goto error;
+		}
 	}
 
 	hcodec->core.lazy_cache = true;
diff --git a/sound/soc/codecs/hdac_hda.h b/sound/soc/codecs/hdac_hda.h
index 6b1bd4f428e7..e145cec085b8 100644
--- a/sound/soc/codecs/hdac_hda.h
+++ b/sound/soc/codecs/hdac_hda.h
@@ -6,6 +6,16 @@ 
 #ifndef __HDAC_HDA_H__
 #define __HDAC_HDA_H__
 
+enum {
+	HDAC_ANALOG_DAI_ID = 0,
+	HDAC_DIGITAL_DAI_ID,
+	HDAC_ALT_ANALOG_DAI_ID,
+	HDAC_HDMI_0_DAI_ID,
+	HDAC_HDMI_1_DAI_ID,
+	HDAC_HDMI_2_DAI_ID,
+	HDAC_LAST_DAI_ID = HDAC_HDMI_2_DAI_ID,
+};
+
 struct hdac_hda_pcm {
 	int stream_tag[2];
 	unsigned int format_val[2];
@@ -13,7 +23,8 @@  struct hdac_hda_pcm {
 
 struct hdac_hda_priv {
 	struct hda_codec codec;
-	struct hdac_hda_pcm pcm[2];
+	struct hdac_hda_pcm pcm[HDAC_LAST_DAI_ID];
+	bool need_display_power;
 };
 
 #define hdac_to_hda_priv(_hdac) \