[RFC PATCH 0/7] adapt SOF to use snd-hda-codec-hdmi

[RFC PATCH 0/7] adapt SOF to use snd-hda-codec-hdmi

[Kai Vehmanen]

Hi all,
here's a RFC patch series that adapts SOF (and one example machine
driver) to use snd-hda-codec-hdmi (patch_hdmi.c) codec driver
instead of hdac_hdmi (soc/codecs/hdac_hdmi.c). The primary goal
is unify the HDMI codec implementation between DSP and non-DSP HDA
configurations, offer same interface to user-space and reduce
maintenance load for all.

Main points I'd like your input on:

1) Is the high-level approach ok?
 - SOF already uses pci/hda/ for all others codecs, so HDMI
   has been the sole exception where code has been duplicated.
 - I've tried to keep changes to hda/hdmi minimal.
 - This series implements logic to parse the PCM topology
   in a dynamic fashion, so we do not have to change all
   existing (and future) DSP topologis to use fixed PCM device
   numbers for HDMI, and we avoid need to hardcode PCM
   device numbers in machine driver code.

2) Can we drop hdac_hdmi and its support from machine drivers, or
   do we need to make it optional and keep it around?

 - Current series does not add any Kconfig options, but
   simply switches SOF to use HDA codecs for all, including
   HDMI/DP. This means hdac_hdmi is never used with SOF
   and could be dropped (if SST is ok as well).
 - This may break some usage with SST (input is welcome!)
 - The change is visible to applications. The ALSA mixer
   interface is different (OTOH with the new driver, playback
   works out-of-the-box while with hdac_hdmi you needed to
   set the multiple controls first, to have any audio out).
 - Alternatively I could add a KConfig option and we could
   have a deprecation period for hdac_hdmi, allowing people
   to compile with the old driver during transition time.
   This will require #ifdef'ing in all the machine drivers.

.. once these are addressed, I can proceed to extend the patchset
with all affected machine drivers.

Feature and testing info:

 - Tested on multiple Intel platforms supported by SOF.
 - Tested with ALSA console tools as well as with Pulseaudio.
      - requires Pulseaudio 12.x or newer, see
        https://lists.freedesktop.org/archives/pulseaudio-discuss/2019-August/031358.html
 - HDMI, DP, DP-MST with multi-monitor use-scenarios work ok.
 - New feature for SOF: ELD /proc fs works just like in
   DSP-less mode.
 - New feature for SOF: jack detection works out-of-the-box
   with Pulseaudio (no need for card specific UCM for HDMI)
 - Pre-reviewed at:
   https://github.com/thesofproject/linux/pull/1155

Kai Vehmanen (7):
  ALSA: hda - add mst_no_extra_pcms flag to hda_codec
  ASoC: Intel: skl-hda-dsp-generic: use snd-hda-codec-hdmi
  ASoC: hdac_hda: add support for HDMI/DP as a HDA codec
  ALSA: hda/hdmi - allow control creation without a linked pcm
  ALSA: hda/hdmi - implement mst_no_extra_pcms flag
  ALSA: hda/hdmi - complete pcm_setup_pin without snd_pcm link
  ASoC: SOF: Intel: load hda codec module also for HDMI/DP

 include/sound/hda_codec.h                    |  1 +
 sound/pci/hda/patch_hdmi.c                   | 31 ++++---
 sound/soc/codecs/hdac_hda.c                  | 95 +++++++++++++++++---
 sound/soc/codecs/hdac_hda.h                  | 10 ++-
 sound/soc/intel/boards/skl_hda_dsp_common.c  | 27 ++----
 sound/soc/intel/boards/skl_hda_dsp_common.h  | 61 +++++++++++++
 sound/soc/intel/boards/skl_hda_dsp_generic.c |  7 --
 sound/soc/sof/intel/hda-codec.c              | 11 +--
 sound/soc/sof/intel/hda.h                    |  5 +-
 9 files changed, 189 insertions(+), 59 deletions(-)

-- 
2.17.1

[RFC PATCH 1/7] ALSA: hda - add mst_no_extra_pcms flag to hda_codec

[Kai Vehmanen]

Add a flag to hda_codec struct to control whether legacy PCM
numbering should be followed in DP-MST mode. When set, no backup
PCMs will be created and PCM count is limited to number of
converters.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 include/sound/hda_codec.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h
index 9a0393cf024c..ac18f428eda6 100644
--- a/include/sound/hda_codec.h
+++ b/include/sound/hda_codec.h
@@ -254,6 +254,7 @@ struct hda_codec {
 	unsigned int force_pin_prefix:1; /* Add location prefix */
 	unsigned int link_down_at_suspend:1; /* link down at runtime suspend */
 	unsigned int relaxed_resume:1;	/* don't resume forcibly for jack */
+	unsigned int mst_no_extra_pcms:1; /* no backup PCMs for DP-MST */
 
 #ifdef CONFIG_PM
 	unsigned long power_on_acct;
-- 
2.17.1

[RFC PATCH 2/7] ASoC: Intel: skl-hda-dsp-generic: use snd-hda-codec-hdmi

[Kai Vehmanen]

Switch to use the snd-hda-codec-hdmi driver for HDMI/DP instead
of ASoC hdac-hdmi. This is aligned with how other HDA codecs
are handled.

PCM device numbers are parsed from card topology and passed
to snd-hda-codec-hdmi. 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  | 27 +++------
 sound/soc/intel/boards/skl_hda_dsp_common.h  | 61 ++++++++++++++++++++
 sound/soc/intel/boards/skl_hda_dsp_generic.c |  7 ---
 3 files changed, 68 insertions(+), 27 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..3db9aee8aa83 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,28 +136,12 @@ 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;
-	char jack_name[NAME_SIZE];
-	int err;
-
-	list_for_each_entry(pcm, &ctx->hdmi_pcm_list, head) {
-		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;
-
-		err = hdac_hdmi_jack_init(pcm->codec_dai, pcm->device,
-					  &pcm->hdmi_jack);
-		if (err < 0)
-			return err;
-	}
 
+	pcm = list_first_entry(&ctx->hdmi_pcm_list, struct skl_hda_hdmi_pcm,
+			       head);
+	component = pcm->codec_dai->component;
 	if (!component)
 		return -EINVAL;
 
-	return hdac_hdmi_jack_port_init(component, &card->dapm);
+	return skl_hda_hdmi_build_controls(card, component);
 }
diff --git a/sound/soc/intel/boards/skl_hda_dsp_common.h b/sound/soc/intel/boards/skl_hda_dsp_common.h
index daa582e513b2..6154688b09e8 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,63 @@ 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 int skl_hda_hdmi_build_controls(struct snd_soc_card *card,
+				       struct snd_soc_component *component)
+{
+	struct hdac_hda_priv *hda_pvt;
+	struct hda_codec *hcodec;
+	struct snd_pcm *spcm;
+	struct hda_pcm *hpcm;
+	int err = 0, i = 0;
+
+	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" },
-- 
2.17.1

[RFC PATCH 3/7] ASoC: hdac_hda: add support for HDMI/DP as a HDA codec

[Kai Vehmanen]

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 | 95 ++++++++++++++++++++++++++++++++-----
 sound/soc/codecs/hdac_hda.h | 10 +++-
 2 files changed, 92 insertions(+), 13 deletions(-)

diff --git a/sound/soc/codecs/hdac_hda.c b/sound/soc/codecs/hdac_hda.c
index 7d4940256914..1c950e089e39 100644
--- a/sound/soc/codecs/hdac_hda.c
+++ b/sound/soc/codecs/hdac_hda.c
@@ -16,11 +16,8 @@
 #include <sound/hdaudio_ext.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 | \
@@ -121,7 +118,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    = "HDMI 1 Playback",
+		.channels_min   = 1,
+		.channels_max   = 16,
+		.rates          = SNDRV_PCM_RATE_8000_192000,
+		.formats        = STUB_FORMATS,
+		.sig_bits = 24,
+	},
+},
+{
+	.id = HDAC_HDMI_1_DAI_ID,
+	.name = "intel-hdmi-hifi2",
+	.ops = &hdac_hda_dai_ops,
+	.playback = {
+		.stream_name    = "HDMI 2 Playback",
+		.channels_min   = 1,
+		.channels_max   = 16,
+		.rates          = SNDRV_PCM_RATE_8000_192000,
+		.formats        = STUB_FORMATS,
+		.sig_bits = 24,
+	},
+},
+{
+	.id = HDAC_HDMI_2_DAI_ID,
+	.name = "intel-hdmi-hifi3",
+	.ops = &hdac_hda_dai_ops,
+	.playback = {
+		.stream_name    = "HDMI 3 Playback",
+		.channels_min   = 1,
+		.channels_max   = 16,
+		.rates          = SNDRV_PCM_RATE_8000_192000,
+		.formats        = STUB_FORMATS,
+		.sig_bits = 24,
+	},
+},
 
 };
 
@@ -135,10 +171,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 +315,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 +331,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 +354,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 =
@@ -366,16 +430,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..1a4af0b1f38f 100644
--- a/sound/soc/codecs/hdac_hda.h
+++ b/sound/soc/codecs/hdac_hda.h
@@ -6,6 +6,14 @@
 #ifndef __HDAC_HDA_H__
 #define __HDAC_HDA_H__
 
+#define HDAC_ANALOG_DAI_ID		0
+#define HDAC_DIGITAL_DAI_ID		1
+#define HDAC_ALT_ANALOG_DAI_ID		2
+#define HDAC_HDMI_0_DAI_ID		3
+#define HDAC_HDMI_1_DAI_ID		4
+#define HDAC_HDMI_2_DAI_ID		5
+#define HDAC_LAST_DAI_ID		6
+
 struct hdac_hda_pcm {
 	int stream_tag[2];
 	unsigned int format_val[2];
@@ -13,7 +21,7 @@ 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];
 };
 
 #define hdac_to_hda_priv(_hdac) \
-- 
2.17.1

[RFC PATCH 4/7] ALSA: hda/hdmi - allow control creation without a linked pcm

[Kai Vehmanen]

Fix the logic in generic_hdmi_build_controls() to identify
unused hda_pcm entries by searching for SNDRV_PCM_INVALID_DEVICE.
This matches with logic in snd_hda_codec_build_pcms().

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 sound/pci/hda/patch_hdmi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 83b8b9d27711..f2022f75afb6 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -2174,11 +2174,13 @@ static int generic_hdmi_build_jack(struct hda_codec *codec, int pcm_idx)
 static int generic_hdmi_build_controls(struct hda_codec *codec)
 {
 	struct hdmi_spec *spec = codec->spec;
+	struct hda_pcm *hda_pcm;
 	int dev, err;
 	int pin_idx, pcm_idx;
 
 	for (pcm_idx = 0; pcm_idx < spec->pcm_used; pcm_idx++) {
-		if (!get_pcm_rec(spec, pcm_idx)->pcm) {
+		hda_pcm = get_pcm_rec(spec, pcm_idx);
+		if (hda_pcm->device == SNDRV_PCM_INVALID_DEVICE) {
 			/* no PCM: mark this for skipping permanently */
 			set_bit(pcm_idx, &spec->pcm_bitmap);
 			continue;
-- 
2.17.1

[RFC PATCH 5/7] ALSA: hda/hdmi - implement mst_no_extra_pcms flag

[Kai Vehmanen]

When mst_no_exxtra_pcms flag is set, the codec should not
use backup PCMs to handle DP-MST scenarios. Instead a simple
1:1 mapping is assumed between PCMs and converters.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 sound/pci/hda/patch_hdmi.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index f2022f75afb6..4372c87c48f0 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -2072,15 +2072,24 @@ static bool is_hdmi_pcm_attached(struct hdac_device *hdac, int pcm_idx)
 static int generic_hdmi_build_pcms(struct hda_codec *codec)
 {
 	struct hdmi_spec *spec = codec->spec;
-	int idx;
+	int idx, pcm_num;
 
 	/*
 	 * for non-mst mode, pcm number is the same as before
-	 * for DP MST mode, pcm number is (nid number + dev_num - 1)
-	 *  dev_num is the device entry number in a pin
-	 *
+	 * for DP MST mode without extra PCM, pcm number is same
+	 * for DP MST mode with extra PCMs, pcm number is
+	 *  (nid number + dev_num - 1)
+	 * dev_num is the device entry number in a pin
 	 */
-	for (idx = 0; idx < spec->num_nids + spec->dev_num - 1; idx++) {
+
+	if (codec->mst_no_extra_pcms)
+		pcm_num = spec->num_nids;
+	else
+		pcm_num = spec->num_nids + spec->dev_num - 1;
+
+	codec_dbg(codec, "hdmi: pcm_num set to %d\n", pcm_num);
+
+	for (idx = 0; idx < pcm_num; idx++) {
 		struct hda_pcm *info;
 		struct hda_pcm_stream *pstr;
 
-- 
2.17.1

[RFC PATCH 6/7] ALSA: hda/hdmi - complete pcm_setup_pin without snd_pcm link

[Kai Vehmanen]

When used with e.g. SOF, the PCM entity is handled by ASoC and
there is no direct link from the HDA codec to the snd_pcm_t
object. Remaining configuration steps of hdmi_pcm_setup_pin()
should still be done in this case.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 sound/pci/hda/patch_hdmi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 4372c87c48f0..65afb833c125 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1386,8 +1386,6 @@ static void hdmi_pcm_setup_pin(struct hdmi_spec *spec,
 		pcm = get_pcm_rec(spec, per_pin->pcm_idx);
 	else
 		return;
-	if (!pcm->pcm)
-		return;
 	if (!test_bit(per_pin->pcm_idx, &spec->pcm_in_use))
 		return;
 
@@ -1408,8 +1406,10 @@ static void hdmi_pcm_setup_pin(struct hdmi_spec *spec,
 	snd_hda_spdif_ctls_assign(codec, per_pin->pcm_idx, hinfo->nid);
 
 	non_pcm = check_non_pcm_per_cvt(codec, hinfo->nid);
-	if (substream->runtime)
-		per_pin->channels = substream->runtime->channels;
+	if (pcm->pcm) {
+		if (substream->runtime)
+			per_pin->channels = substream->runtime->channels;
+	}
 	per_pin->setup = true;
 	per_pin->mux_idx = mux_idx;
 
-- 
2.17.1

[RFC PATCH 7/7] ASoC: SOF: Intel: load hda codec module also for HDMI/DP

[Kai Vehmanen]

Handle all HDA codecs in the same way and remove exceptions
for HDMI/DP.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 sound/soc/sof/intel/hda-codec.c | 11 ++++-------
 sound/soc/sof/intel/hda.h       |  5 +++--
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c
index b8b37f082309..3eb3e1505481 100644
--- a/sound/soc/sof/intel/hda-codec.c
+++ b/sound/soc/sof/intel/hda-codec.c
@@ -74,11 +74,8 @@ 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) {
-		hdev->type = HDA_DEV_LEGACY;
-		hda_codec_load_module(&hda_priv->codec);
-	}
+	hdev->type = HDA_DEV_LEGACY;
+	hda_codec_load_module(&hda_priv->codec);
 
 	return 0;
 #else
@@ -117,7 +114,7 @@ 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)
 
 void hda_codec_i915_get(struct snd_sof_dev *sdev)
 {
@@ -166,6 +163,6 @@ int hda_codec_i915_exit(struct snd_sof_dev *sdev)
 }
 EXPORT_SYMBOL(hda_codec_i915_exit);
 
-#endif /* CONFIG_SND_SOC_HDAC_HDMI */
+#endif /* CONFIG_SND_HDA_CODEC_HDMI */
 
 MODULE_LICENSE("Dual BSD/GPL");
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 75b096050fa2..1cf8b039a21a 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -559,7 +559,8 @@ int hda_codec_probe_bus(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)
 
 void hda_codec_i915_get(struct snd_sof_dev *sdev);
 void hda_codec_i915_put(struct snd_sof_dev *sdev);
@@ -573,7 +574,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 /* CONFIG_SND_SOC_SOF_HDA && CONFIG_SND_HDA_CODEC_HDMI */
 
 /*
  * Trace Control.
-- 
2.17.1

Re: [RFC PATCH 5/7] ALSA: hda/hdmi - implement mst_no_extra_pcms flag

[Takashi Iwai]

On Thu, 29 Aug 2019 15:53:46 +0200,
Kai Vehmanen wrote:
> 
> When mst_no_exxtra_pcms flag is set, the codec should not
> use backup PCMs to handle DP-MST scenarios. Instead a simple
> 1:1 mapping is assumed between PCMs and converters.
> 
> Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> ---
>  sound/pci/hda/patch_hdmi.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index f2022f75afb6..4372c87c48f0 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -2072,15 +2072,24 @@ static bool is_hdmi_pcm_attached(struct hdac_device *hdac, int pcm_idx)
>  static int generic_hdmi_build_pcms(struct hda_codec *codec)
>  {
>  	struct hdmi_spec *spec = codec->spec;
> -	int idx;
> +	int idx, pcm_num;
>  
>  	/*
>  	 * for non-mst mode, pcm number is the same as before
> -	 * for DP MST mode, pcm number is (nid number + dev_num - 1)
> -	 *  dev_num is the device entry number in a pin
> -	 *
> +	 * for DP MST mode without extra PCM, pcm number is same
> +	 * for DP MST mode with extra PCMs, pcm number is
> +	 *  (nid number + dev_num - 1)
> +	 * dev_num is the device entry number in a pin
>  	 */
> -	for (idx = 0; idx < spec->num_nids + spec->dev_num - 1; idx++) {
> +
> +	if (codec->mst_no_extra_pcms)
> +		pcm_num = spec->num_nids;
> +	else
> +		pcm_num = spec->num_nids + spec->dev_num - 1;
> +
> +	codec_dbg(codec, "hdmi: pcm_num set to %d\n", pcm_num);
> +
> +	for (idx = 0; idx < pcm_num; idx++) {
>  		struct hda_pcm *info;
>  		struct hda_pcm_stream *pstr;

Instead of changing this, we can simply take dev_num=1 like below.


thanks,

Takashi

--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1706,7 +1706,11 @@ static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
 	 * To simplify the implementation, malloc all
 	 * the virtual pins in the initialization statically
 	 */
-	if (is_haswell_plus(codec)) {
+	if (codec->mst_no_extra_pcms) {
+		/* for SOF/SST, no backup PCM streams can be assigned */
+		dev_num = 1; 
+		spec->dev_num = 1;
+	} else if (is_haswell_plus(codec)) {
 		/*
 		 * On Intel platforms, device entries number is
 		 * changed dynamically. If there is a DP MST

Re: [RFC PATCH 0/7] adapt SOF to use snd-hda-codec-hdmi

[Takashi Iwai]

On Thu, 29 Aug 2019 15:53:41 +0200,
Kai Vehmanen wrote:
> 
> Hi all,
> here's a RFC patch series that adapts SOF (and one example machine
> driver) to use snd-hda-codec-hdmi (patch_hdmi.c) codec driver
> instead of hdac_hdmi (soc/codecs/hdac_hdmi.c). The primary goal
> is unify the HDMI codec implementation between DSP and non-DSP HDA
> configurations, offer same interface to user-space and reduce
> maintenance load for all.
> 
> Main points I'd like your input on:
> 
> 1) Is the high-level approach ok?
>  - SOF already uses pci/hda/ for all others codecs, so HDMI
>    has been the sole exception where code has been duplicated.
>  - I've tried to keep changes to hda/hdmi minimal.
>  - This series implements logic to parse the PCM topology
>    in a dynamic fashion, so we do not have to change all
>    existing (and future) DSP topologis to use fixed PCM device
>    numbers for HDMI, and we avoid need to hardcode PCM
>    device numbers in machine driver code.
> 
> 2) Can we drop hdac_hdmi and its support from machine drivers, or
>    do we need to make it optional and keep it around?
> 
>  - Current series does not add any Kconfig options, but
>    simply switches SOF to use HDA codecs for all, including
>    HDMI/DP. This means hdac_hdmi is never used with SOF
>    and could be dropped (if SST is ok as well).
>  - This may break some usage with SST (input is welcome!)
>  - The change is visible to applications. The ALSA mixer
>    interface is different (OTOH with the new driver, playback
>    works out-of-the-box while with hdac_hdmi you needed to
>    set the multiple controls first, to have any audio out).
>  - Alternatively I could add a KConfig option and we could
>    have a deprecation period for hdac_hdmi, allowing people
>    to compile with the old driver during transition time.
>    This will require #ifdef'ing in all the machine drivers.
> 
> .. once these are addressed, I can proceed to extend the patchset
> with all affected machine drivers.
> 
> Feature and testing info:
> 
>  - Tested on multiple Intel platforms supported by SOF.
>  - Tested with ALSA console tools as well as with Pulseaudio.
>       - requires Pulseaudio 12.x or newer, see
>         https://lists.freedesktop.org/archives/pulseaudio-discuss/2019-August/031358.html
>  - HDMI, DP, DP-MST with multi-monitor use-scenarios work ok.
>  - New feature for SOF: ELD /proc fs works just like in
>    DSP-less mode.
>  - New feature for SOF: jack detection works out-of-the-box
>    with Pulseaudio (no need for card specific UCM for HDMI)
>  - Pre-reviewed at:
>    https://github.com/thesofproject/linux/pull/1155

IMO, the only and the most important point is whether it works as-is
without changing the existing user-space, or exactly what scenario
would be broken.  If the breakage is significant, we may introduce a
Kconfig, as you suggested.

I don't think the mixer contents change are problematic.  In the case
of HDMI/DP, it's mostly read-only for fetching ELD or jack state.

Other than that, I like the idea, the code change looks simple
enough, and it'd make maintenance easier.


thanks,

Takashi

> 
> Kai Vehmanen (7):
>   ALSA: hda - add mst_no_extra_pcms flag to hda_codec
>   ASoC: Intel: skl-hda-dsp-generic: use snd-hda-codec-hdmi
>   ASoC: hdac_hda: add support for HDMI/DP as a HDA codec
>   ALSA: hda/hdmi - allow control creation without a linked pcm
>   ALSA: hda/hdmi - implement mst_no_extra_pcms flag
>   ALSA: hda/hdmi - complete pcm_setup_pin without snd_pcm link
>   ASoC: SOF: Intel: load hda codec module also for HDMI/DP
> 
>  include/sound/hda_codec.h                    |  1 +
>  sound/pci/hda/patch_hdmi.c                   | 31 ++++---
>  sound/soc/codecs/hdac_hda.c                  | 95 +++++++++++++++++---
>  sound/soc/codecs/hdac_hda.h                  | 10 ++-
>  sound/soc/intel/boards/skl_hda_dsp_common.c  | 27 ++----
>  sound/soc/intel/boards/skl_hda_dsp_common.h  | 61 +++++++++++++
>  sound/soc/intel/boards/skl_hda_dsp_generic.c |  7 --
>  sound/soc/sof/intel/hda-codec.c              | 11 +--
>  sound/soc/sof/intel/hda.h                    |  5 +-
>  9 files changed, 189 insertions(+), 59 deletions(-)
> 
> -- 
> 2.17.1
> 

Re: [alsa-devel] [RFC PATCH 5/7] ALSA: hda/hdmi - implement mst_no_extra_pcms flag

[Kai Vehmanen]

Hi Takashi,

On Thu, 29 Aug 2019, Takashi Iwai wrote:

> On Thu, 29 Aug 2019 15:53:46 +0200,
> Kai Vehmanen wrote:
> >  static int generic_hdmi_build_pcms(struct hda_codec *codec)
> >  {
[...]
> > +	if (codec->mst_no_extra_pcms)
> > +		pcm_num = spec->num_nids;
> > +	else
> > +		pcm_num = spec->num_nids + spec->dev_num - 1;
>
> Instead of changing this, we can simply take dev_num=1 like below.
[...]
> -	if (is_haswell_plus(codec)) {
> +	if (codec->mst_no_extra_pcms) {
> +		/* for SOF/SST, no backup PCM streams can be assigned */
> +		dev_num = 1; 
> +		spec->dev_num = 1;
> +	} else if (is_haswell_plus(codec)) {

hmm, I think we need to keep dev_num as 3, to create the MST per_pin 
structs for each port. I.e. we still have the virtual pins, although we 
limit PCM count to 3. Unless we do this, at least jack detection is broken 
in DP-MST mode.

One option would be to set
   dev_num = 3;
   spec->dev_num = 1;

... spec->dev_num is not really used elsewhere than 
generic_hdmi_build_pcms(), so this works.

But, but, this seems quite confusing. So ok if I keep in original form and 
have separate logic in generic_hdmi_build_pcms()?

Br, Kai
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [RFC PATCH 5/7] ALSA: hda/hdmi - implement mst_no_extra_pcms flag

[Takashi Iwai]

On Mon, 02 Sep 2019 14:52:44 +0200,
Kai Vehmanen wrote:
> 
> Hi Takashi,
> 
> On Thu, 29 Aug 2019, Takashi Iwai wrote:
> 
> > On Thu, 29 Aug 2019 15:53:46 +0200,
> > Kai Vehmanen wrote:
> > >  static int generic_hdmi_build_pcms(struct hda_codec *codec)
> > >  {
> [...]
> > > +	if (codec->mst_no_extra_pcms)
> > > +		pcm_num = spec->num_nids;
> > > +	else
> > > +		pcm_num = spec->num_nids + spec->dev_num - 1;
> >
> > Instead of changing this, we can simply take dev_num=1 like below.
> [...]
> > -	if (is_haswell_plus(codec)) {
> > +	if (codec->mst_no_extra_pcms) {
> > +		/* for SOF/SST, no backup PCM streams can be assigned */
> > +		dev_num = 1; 
> > +		spec->dev_num = 1;
> > +	} else if (is_haswell_plus(codec)) {
> 
> hmm, I think we need to keep dev_num as 3, to create the MST per_pin 
> structs for each port. I.e. we still have the virtual pins, although we 
> limit PCM count to 3. Unless we do this, at least jack detection is broken 
> in DP-MST mode.
> 
> One option would be to set
>    dev_num = 3;
>    spec->dev_num = 1;
> 
> ... spec->dev_num is not really used elsewhere than 
> generic_hdmi_build_pcms(), so this works.
> 
> But, but, this seems quite confusing. So ok if I keep in original form and 
> have separate logic in generic_hdmi_build_pcms()?

Fair enough, let's keep your version as is.


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [RFC PATCH 0/7] adapt SOF to use snd-hda-codec-hdmi

[Kai Vehmanen]

Hi,

On Thu, 29 Aug 2019, Takashi Iwai wrote:

>> here's a RFC patch series that adapts SOF (and one example machine
>> driver) to use snd-hda-codec-hdmi (patch_hdmi.c) codec driver
>> instead of hdac_hdmi (soc/codecs/hdac_hdmi.c). The primary goal
[...]
>> 2) Can we drop hdac_hdmi and its support from machine drivers, or
>>    do we need to make it optional and keep it around?
> 
> IMO, the only and the most important point is whether it works as-is
> without changing the existing user-space, or exactly what scenario
> would be broken.  If the breakage is significant, we may introduce a
> Kconfig, as you suggested.
> 
> I don't think the mixer contents change are problematic.  In the case
> of HDMI/DP, it's mostly read-only for fetching ELD or jack state.

I've been now continuing testing with different combinations of 
kernel/user-space and the two main problematic areas are:

1) systems with UCM defined with hdac-hdmi style controls

-> as the card name will not change, the UCM usage will fail 
when kernel is updated to use different HDMI codec driver

On some systems this is manageable as e.g. pulseaudio will fallback to 
legacy non-UCM path and e.g. HDMI/DP audio keeps working. But, but, this 
may be problematic if UCM is needed for other functionality on these 
systems.

2) machine drivers shared with SST/SOF

Doing the HDMI codec change for old platforms handled with SST driver 
looks difficult to do, so we probably need to keep hdac-hdmi around for 
SST usage. That does mean machine drivers that are shared, need to support 
both options.

Combining 1+2, it would seem safer to have a opt-in/opt-out possibility 
via Kconfig. I'm preparing a patchset for this -- let's see how it will 
look.

Br, Kai

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [RFC PATCH 0/7] adapt SOF to use snd-hda-codec-hdmi

[Takashi Iwai]

On Tue, 03 Sep 2019 16:18:11 +0200,
Kai Vehmanen wrote:
> 
> Hi,
> 
> On Thu, 29 Aug 2019, Takashi Iwai wrote:
> 
> >> here's a RFC patch series that adapts SOF (and one example machine
> >> driver) to use snd-hda-codec-hdmi (patch_hdmi.c) codec driver
> >> instead of hdac_hdmi (soc/codecs/hdac_hdmi.c). The primary goal
> [...]
> >> 2) Can we drop hdac_hdmi and its support from machine drivers, or
> >>    do we need to make it optional and keep it around?
> > 
> > IMO, the only and the most important point is whether it works as-is
> > without changing the existing user-space, or exactly what scenario
> > would be broken.  If the breakage is significant, we may introduce a
> > Kconfig, as you suggested.
> > 
> > I don't think the mixer contents change are problematic.  In the case
> > of HDMI/DP, it's mostly read-only for fetching ELD or jack state.
> 
> I've been now continuing testing with different combinations of 
> kernel/user-space and the two main problematic areas are:
> 
> 1) systems with UCM defined with hdac-hdmi style controls
> 
> -> as the card name will not change, the UCM usage will fail 
> when kernel is updated to use different HDMI codec driver
> 
> On some systems this is manageable as e.g. pulseaudio will fallback to 
> legacy non-UCM path and e.g. HDMI/DP audio keeps working. But, but, this 
> may be problematic if UCM is needed for other functionality on these 
> systems.

Just out of curiosity: which systems are with such UCM profiles?
Chromebooks?

> 2) machine drivers shared with SST/SOF
> 
> Doing the HDMI codec change for old platforms handled with SST driver 
> looks difficult to do, so we probably need to keep hdac-hdmi around for 
> SST usage. That does mean machine drivers that are shared, need to support 
> both options.
> 
> Combining 1+2, it would seem safer to have a opt-in/opt-out possibility 
> via Kconfig. I'm preparing a patchset for this -- let's see how it will 
> look.

Agreed, some Kconfig is a safer option for now.


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [RFC PATCH 0/7] adapt SOF to use snd-hda-codec-hdmi

[Kai Vehmanen]

Hi,

On Tue, 3 Sep 2019, Takashi Iwai wrote:

> On Tue, 03 Sep 2019 16:18:11 +0200, Kai Vehmanen wrote:
> > On some systems this is manageable as e.g. pulseaudio will fallback to 
> > legacy non-UCM path and e.g. HDMI/DP audio keeps working. But, but, this 
> > may be problematic if UCM is needed for other functionality on these 
> > systems.
> 
> Just out of curiosity: which systems are with such UCM profiles?
> Chromebooks?

I believe this year's XPS 13 Developer edition is one such device. I'll 
try to confirm if there are more cases.

Br, Kai
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel