haswell displayport MST audio support

From: Dave Airlie <[email protected]>

This just adds enables for the codecs and debugfs support for mst connectors to print the audio info.

This relies on patches to the audio code to do anything more useful.

Signed-off-by Dave Airlie <[email protected]>

---

Quoted 8 lines · Dave Airlie

From: Dave Airlie <[email protected]>

This just adds enables for the codecs and debugfs support for mst connectors to print the audio info.

This relies on patches to the audio code to do anything more useful.

drivers/gpu/drm/i915/intel_audio.c static void hsw_audio_codec_enable(struct drm_connector *connector, · line 235

Quoted 8 lines

Signed-off-by Dave Airlie <[email protected]>

---

drivers/gpu/drm/i915/intel_audio.c +1 -0

@@ -231,6 +231,7 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
 	/* Reset ELD write address */
 	tmp = I915_READ(HSW_AUD_DIP_ELD_CTRL(pipe));
 	tmp &= ~IBX_ELD_ADDRESS_MASK;
+	tmp |= ((pipe + 1) << 29);
 	I915_WRITE(HSW_AUD_DIP_ELD_CTRL(pipe), tmp);

This is not MST specific. Is this a bug fix we want ASAP?

-Chris

Signature · 5 lines

Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Quoted 9 lines · Chris Wilson

Quoted 8 lines · Dave Airlie

From: Dave Airlie <[email protected]>

This just adds enables for the codecs and debugfs support for mst connectors to print the audio info.

This relies on patches to the audio code to do anything more useful.

Quoted 10 lines

drivers/gpu/drm/i915/intel_audio.c static void hsw_audio_codec_enable(struct drm_connector *connector, · line 235

Quoted 8 lines

Signed-off-by Dave Airlie <[email protected]>

---

drivers/gpu/drm/i915/intel_audio.c +1 -0

@@ -231,6 +231,7 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
 	/* Reset ELD write address */
 	tmp = I915_READ(HSW_AUD_DIP_ELD_CTRL(pipe));
 	tmp &= ~IBX_ELD_ADDRESS_MASK;
+	tmp |= ((pipe + 1) << 29);
 	I915_WRITE(HSW_AUD_DIP_ELD_CTRL(pipe), tmp);

This is not MST specific. Is this a bug fix we want ASAP?

"""
This read-only bit reflects which port is used to transmit the DIP data. This can only change when DIP is disabled. If one or more audio-related DIP packets is enabled and audio is enabled on a digital port, these bits will reflect the digital port to which audio is directed.

For DP MST, this is the device select/pipe select. """

We shouldn't mess with the field if it's regular DP. Also "pipe + 1" is too magic; unfortunately I don't know what it should be. :(

BR,
Jani.

Quoted 8 lines

-Chris

Signature · 5 lines

Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 1 line

Jani Nikula, Intel Open Source Technology Center

-----Original Message-----
From: Intel-gfx [mailto:[email protected]] On Behalf Of Jani Nikula

Quoted 8 lines · Chris Wilson

Quoted 7 lines · Dave Airlie

From: Dave Airlie <[email protected]>

This just adds enables for the codecs and debugfs support for mst connectors to print the audio info.

This relies on patches to the audio code to do anything more useful.

Quoted 3 lines

Review static void hsw_audio_codec_enable(struct

Quoted 3 lines

Signed-off-by Dave Airlie <[email protected]>

---

(quoted diff) +0 -0

@@ -231,6 +231,7 @@ static void hsw_audio_codec_enable(struct

drm_connector *connector,

Quoted 4 lines

drivers/gpu/drm/i915/intel_audio.c static void hsw_audio_codec_enable(struct drm_connector *connector, · line 234

Quoted 4 lines

drivers/gpu/drm/i915/intel_audio.c +1 -0

 	/* Reset ELD write address */
 	tmp = I915_READ(HSW_AUD_DIP_ELD_CTRL(pipe));
 	tmp &= ~IBX_ELD_ADDRESS_MASK;
+	tmp |= ((pipe + 1) << 29);

Quoted 3 lines

drivers/gpu/drm/i915/intel_audio.c static void hsw_audio_codec_enable(struct drm_connector *connector, · line 235

Quoted 1 line

drivers/gpu/drm/i915/intel_audio.c +0 -0

 	I915_WRITE(HSW_AUD_DIP_ELD_CTRL(pipe), tmp);

This is not MST specific. Is this a bug fix we want ASAP?

"""
This read-only bit reflects which port is used to transmit the DIP data. This can only change when DIP is disabled. If one or more audio-related DIP packets is enabled and audio is enabled on a digital port, these bits will reflect the digital port to which audio is directed.

For DP MST, this is the device select/pipe select. """

We shouldn't mess with the field if it's regular DP. Also "pipe + 1" is too magic; unfortunately I don't know what it should be. :(

BR,
Jani.

Quoted 8 lines

-Chris

--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

From: Dave Airlie <[email protected]>

This just adds enables for the codecs and debugfs support for mst connectors to print the audio info.

This relies on patches to the audio code to do anything more useful.

Signed-off-by Dave Airlie <[email protected]>

---
 drivers/gpu/drm/i915/i915_debugfs.c | 13 +++++++++++++
 drivers/gpu/drm/i915/intel_audio.c  |  7 ++++---
 drivers/gpu/drm/i915/intel_dp_mst.c | 25 +++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 3 deletions(-)

drivers/gpu/drm/i915/i915_debugfs.c +13 -0

index dc55c51..a109a21 100644
@@ -2577,6 +2577,17 @@ static void intel_dp_info(struct seq_file *m,
 		intel_panel_info(m, &intel_connector->panel);
 }
 
+static void intel_dp_mst_info(struct seq_file *m,
+			  struct intel_connector *intel_connector)
+{
+	struct intel_encoder *intel_encoder = intel_connector->encoder;
+	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&intel_encoder->base);
+	struct intel_digital_port *intel_dig_port = intel_mst->primary;
+	struct intel_dp *intel_dp = &intel_dig_port->dp;
+
+	seq_printf(m, "\taudio support: %s\n", drm_dp_mst_port_has_audio(&intel_dp->mst_mgr, intel_connector->port) ? "yes" : "no");
+}
+
 static void intel_hdmi_info(struct seq_file *m,
 			    struct intel_connector *intel_connector)
 {

@@ -2621,6 +2632,8 @@ static void intel_connector_info(struct seq_file *m,
 			intel_hdmi_info(m, intel_connector);
 		else if (intel_encoder->type == INTEL_OUTPUT_LVDS)
 			intel_lvds_info(m, intel_connector);
+		else if (intel_encoder->type == INTEL_OUTPUT_DP_MST)
+			intel_dp_mst_info(m, intel_connector);
 	}
 
 	seq_printf(m, "\tmodes:\n");

drivers/gpu/drm/i915/intel_audio.c +4 -3

index 2396cc7..77d2a01 100644
@@ -190,7 +190,7 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
 	tmp |= AUD_CONFIG_N_PROG_ENABLE;
 	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
 	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
-	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
+	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT) || intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DP_MST))
 		tmp |= AUD_CONFIG_N_VALUE_INDEX;
 	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
 

@@ -231,6 +231,7 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
 	/* Reset ELD write address */
 	tmp = I915_READ(HSW_AUD_DIP_ELD_CTRL(pipe));
 	tmp &= ~IBX_ELD_ADDRESS_MASK;
+	tmp |= ((pipe + 1) << 29);
 	I915_WRITE(HSW_AUD_DIP_ELD_CTRL(pipe), tmp);
 
 	/* Up to 84 bytes of hw ELD buffer */

@@ -248,7 +249,7 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
 	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
 	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
 	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
-	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
+	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT) || intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DP_MST))
 		tmp |= AUD_CONFIG_N_VALUE_INDEX;
 	else
 		tmp |= audio_config_hdmi_pixel_clock(mode);

@@ -417,7 +418,7 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
 
 	/* ELD Conn_Type */
 	connector->eld[5] &= ~(3 << 2);
-	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT))
+	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT) || intel_pipe_has_type(crtc, INTEL_OUTPUT_DP_MST))
 		connector->eld[5] |= (1 << 2);
 
 	connector->eld[6] = drm_av_sync_delay(connector, mode) / 2;

drivers/gpu/drm/i915/intel_dp_mst.c +25 -0

index 5cb4748..8a39e99 100644
@@ -85,6 +85,8 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
 		return false;
 	}
 
+	if (drm_dp_mst_port_has_audio(&intel_dp->mst_mgr, found->port))
+		pipe_config->has_audio = true;
 	mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->clock, bpp);
 
 	pipe_config->pbn = mst_pbn;

@@ -105,6 +107,11 @@ static void intel_mst_disable_dp(struct intel_encoder *encoder)
 	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
 	struct intel_digital_port *intel_dig_port = intel_mst->primary;
 	struct intel_dp *intel_dp = &intel_dig_port->dp;
+	struct drm_device *dev = encoder->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc = encoder->base.crtc;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
 	int ret;
 
 	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);

@@ -115,6 +122,10 @@ static void intel_mst_disable_dp(struct intel_encoder *encoder)
 	if (ret) {
 		DRM_ERROR("failed to update payload %d\n", ret);
 	}
+	if (intel_crtc->config->has_audio) {
+		intel_audio_codec_disable(encoder);
+		intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
+	}
 }
 
 static void intel_mst_post_disable_dp(struct intel_encoder *encoder)

@@ -209,6 +220,7 @@ static void intel_mst_enable_dp(struct intel_encoder *encoder)
 	struct intel_dp *intel_dp = &intel_dig_port->dp;
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
 	enum port port = intel_dig_port->port;
 	int ret;
 

@@ -221,6 +233,13 @@ static void intel_mst_enable_dp(struct intel_encoder *encoder)
 	ret = drm_dp_check_act_status(&intel_dp->mst_mgr);
 
 	ret = drm_dp_update_payload_part2(&intel_dp->mst_mgr);
+
+	if (crtc->config->has_audio) {
+		DRM_DEBUG_DRIVER("Enabling DP audio on pipe %c\n",
+				 pipe_name(crtc->pipe));
+		intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
+		intel_audio_codec_enable(encoder);
+	}
 }
 
 static bool intel_dp_mst_enc_get_hw_state(struct intel_encoder *encoder,

@@ -246,6 +265,12 @@ static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
 
 	pipe_config->has_dp_encoder = true;
 
+       if (intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_AUDIO)) {
+                temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
+                if (temp & AUDIO_OUTPUT_ENABLE(crtc->pipe))
+                        pipe_config->has_audio = true;
+        }
+
 	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
 	if (temp & TRANS_DDI_PHSYNC)
 		flags |= DRM_MODE_FLAG_PHSYNC;

Signature · 6 lines

2.4.1

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-----Original Message-----
From: Intel-gfx [mailto:[email protected]] On Behalf Of Dave Airlie
Sent: Wednesday, June 17, 2015 12:02 PM
To: [email protected]
Subject: [Intel-gfx] [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.

From: Dave Airlie <[email protected]>

Add new verbs GET_DP_STREAM_ID and SET_DP_STREAM_ID from Intel docs. get stream id and print in proc
split ELD to be per device not per pin
handle pd/eldv per device not per pin
setup codec->dp_mst earlier.

Signed-off-by Dave Airlie <[email protected]>

---
 include/sound/hda_verbs.h  |   2 +
 sound/pci/hda/hda_codec.c  |   1 +
 sound/pci/hda/hda_proc.c   |   5 +-
 sound/pci/hda/patch_hdmi.c | 181
+++++++++++++++++++++++++++++++--------------
 4 files changed, 131 insertions(+), 58 deletions(-)

include/sound/hda_verbs.h index renamed from include/sound/hda_verbs.h +0 -0

d0509db..3b62ac5 100644

include/sound/hda_verbs.h +2 -0

@@ -75,6 +75,7 @@ enum {
 #define AC_VERB_GET_HDMI_CHAN_SLOT		0x0f34
 #define AC_VERB_GET_DEVICE_SEL			0xf35
 #define AC_VERB_GET_DEVICE_LIST			0xf36
+#define AC_VERB_GET_DP_STREAM_ID		0xf3c
 
 /*
  * SET verbs

@@ -115,6 +116,7 @@ enum {
 #define AC_VERB_SET_HDMI_CP_CTRL		0x733
 #define AC_VERB_SET_HDMI_CHAN_SLOT		0x734
 #define AC_VERB_SET_DEVICE_SEL			0x735
+#define AC_VERB_SET_DP_STREAM_ID		0x73C
 
 /*
  * Parameter IDs

sound/pci/hda/hda_codec.c index renamed from sound/pci/hda/hda_codec.c +0 -0

5645481..3981c63 100644

sound/pci/hda/hda_codec.c +1 -0

@@ -482,6 +482,7 @@ int snd_hda_get_devices(struct hda_codec *codec,
 hda_nid_t nid,
 	}
 	return devices;
 }
+EXPORT_SYMBOL_GPL(snd_hda_get_devices);
 
 /*

* destructor

sound/pci/hda/hda_proc.c index renamed from sound/pci/hda/hda_proc.c +0 -0

baaf7ed0..39fac53 100644

sound/pci/hda/hda_proc.c +4 -1

@@ -644,10 +644,13 @@ static void print_device_list(struct snd_info_buffer
 *buffer,
 	int i, curr = -1;
 	u8 dev_list[AC_MAX_DEV_LIST_LEN];
 	int devlist_len;
+	int dp_s_id;
 
+	dp_s_id = snd_hda_codec_read(codec, nid, 0,
+				AC_VERB_GET_DP_STREAM_ID, 0);
 	devlist_len = snd_hda_get_devices(codec, nid, dev_list,
 					AC_MAX_DEV_LIST_LEN);
-	snd_iprintf(buffer, "  Devices: %d\n", devlist_len);
+	snd_iprintf(buffer, "  Devices: %d: 0x%x\n", devlist_len, dp_s_id);
 	if (devlist_len <= 0)
 		return;

sound/pci/hda/patch_hdmi.c index renamed from sound/pci/hda/patch_hdmi.c +0 -0

5f44f60..8272656 100644

sound/pci/hda/patch_hdmi.c +16 -1

@@ -68,6 +68,17 @@ struct hdmi_spec_per_cvt {
 /* max. connections to a widget */
 #define HDA_MAX_CONNECTIONS	32
 
+struct hdmi_spec_per_pin;
+#define HDA_MAX_DEVICES 3
+struct hdmi_spec_per_device {
+	struct hdmi_spec_per_pin *pin;
+	int device_idx;
+	struct hdmi_eld sink_eld;
+#ifdef CONFIG_PROC_FS
+	struct snd_info_entry *proc_entry;
+#endif
+};
+
 struct hdmi_spec_per_pin {
 	hda_nid_t pin_nid;
 	int num_mux_nids;

@@ -76,7 +87,11 @@ struct hdmi_spec_per_pin {
 	hda_nid_t cvt_nid;
 
 	struct hda_codec *codec;
-	struct hdmi_eld sink_eld;
+
+	int num_devices;
+	u8 dev_list[AC_MAX_DEV_LIST_LEN];
+	struct hdmi_spec_per_device devices[HDA_MAX_DEVICES];
+
 	struct mutex lock;
 	struct delayed_work work;
 	struct snd_kcontrol *eld_ctl;

@@ -86,9 +101,6 @@ struct hdmi_spec_per_pin {
 	bool non_pcm;
 	bool chmap_set;		/* channel-map override by ALSA API? */
 	unsigned char chmap[8]; /* ALSA API channel-map */ -#ifdef

CONFIG_PROC_FS

sound/pci/hda/patch_hdmi.c +1 -3

-	struct snd_info_entry *proc_entry;
-#endif
 };
 
 struct cea_channel_speaker_allocation;

@@ -409,7 +421,7 @@ static int hdmi_eld_ctl_info(struct snd_kcontrol
 *kcontrol,
 
 	pin_idx = kcontrol->private_value;
 	per_pin = get_pin(spec, pin_idx);
-	eld = &per_pin->sink_eld;
+	eld = &per_pin->devices[0].sink_eld;
 
 	mutex_lock(&per_pin->lock);

uinfo->count = eld->eld_valid ? eld->eld_size : 0; @@ -429,7 +441,7 @@
static int hdmi_eld_ctl_get(struct snd_kcontrol *kcontrol,

sound/pci/hda/patch_hdmi.c +21 -16

 	pin_idx = kcontrol->private_value;
 	per_pin = get_pin(spec, pin_idx);
-	eld = &per_pin->sink_eld;
+	eld = &per_pin->devices[0].sink_eld;
 
 	mutex_lock(&per_pin->lock);
 	if (eld->eld_size > ARRAY_SIZE(ucontrol->value.bytes.data)) { @@ -549,60
+561,63 @@ static void hdmi_set_channel_count(struct hda_codec *codec,
  */
 
 #ifdef CONFIG_PROC_FS
-static void print_eld_info(struct snd_info_entry *entry,
-			   struct snd_info_buffer *buffer)
+static void print_eld_info_device(struct snd_info_entry *entry,
+				  struct snd_info_buffer *buffer)
 {
-	struct hdmi_spec_per_pin *per_pin = entry->private_data;
+	struct hdmi_spec_per_device *per_device = entry->private_data;
 
-	mutex_lock(&per_pin->lock);
-	snd_hdmi_print_eld_info(&per_pin->sink_eld, buffer);
-	mutex_unlock(&per_pin->lock);
+	mutex_lock(&per_device->pin->lock);
+	snd_hdmi_print_eld_info(&per_device->sink_eld, buffer);
+	mutex_unlock(&per_device->pin->lock);
 }
 
-static void write_eld_info(struct snd_info_entry *entry,
-			   struct snd_info_buffer *buffer)
+static void write_eld_info_device(struct snd_info_entry *entry,
+				  struct snd_info_buffer *buffer)
 {
-	struct hdmi_spec_per_pin *per_pin = entry->private_data;
+	struct hdmi_spec_per_device *per_device = entry->private_data;
 
-	mutex_lock(&per_pin->lock);
-	snd_hdmi_write_eld_info(&per_pin->sink_eld, buffer);
-	mutex_unlock(&per_pin->lock);
+	mutex_lock(&per_device->pin->lock);
+	snd_hdmi_write_eld_info(&per_device->sink_eld, buffer);
+	mutex_unlock(&per_device->pin->lock);
 }
 
-static int eld_proc_new(struct hdmi_spec_per_pin *per_pin, int index)
+static int eld_device_proc_new(struct hdmi_spec_per_device *per_device,
+int pin_idx, int dev_idx)
 {
 	char name[32];
-	struct hda_codec *codec = per_pin->codec;
+	struct hda_codec *codec = per_device->pin->codec;
 	struct snd_info_entry *entry;
 	int err;
 
-	snprintf(name, sizeof(name), "eld#%d.%d", codec->addr, index);
+	if (dev_idx == -1)
+		snprintf(name, sizeof(name), "eld#%d.%d", codec->addr, pin_idx);
+	else
+		snprintf(name, sizeof(name), "eld#%d.%d.%d", codec->addr,

pin_idx,

sound/pci/hda/patch_hdmi.c +7 -7

+dev_idx);
 	err = snd_card_proc_new(codec->card, name, &entry);
 	if (err < 0)
 		return err;
 
-	snd_info_set_text_ops(entry, per_pin, print_eld_info);
-	entry->c.text.write = write_eld_info;
+	snd_info_set_text_ops(entry, per_device, print_eld_info_device);
+	entry->c.text.write = write_eld_info_device;
 	entry->mode |= S_IWUSR;
-	per_pin->proc_entry = entry;
+	per_device->proc_entry = entry;
 
 	return 0;
 }
 
-static void eld_proc_free(struct hdmi_spec_per_pin *per_pin)
+static void eld_device_proc_free(struct hdmi_spec_per_device
+*per_device)
 {
-	if (!per_pin->codec->bus->shutdown && per_pin->proc_entry) {
-		snd_device_free(per_pin->codec->card, per_pin->proc_entry);
-		per_pin->proc_entry = NULL;
+	if (!per_device->pin->codec->bus->shutdown && per_device->proc_entry)

{

sound/pci/hda/patch_hdmi.c +1 -0

+		snd_device_free(per_device->pin->codec->card,

per_device->proc_entry);

sound/pci/hda/patch_hdmi.c +2 -2

+		per_device->proc_entry = NULL;
 	}
 }
 #else
-static inline int eld_proc_new(struct hdmi_spec_per_pin *per_pin,
-			       int index)
+static inline int eld_device_proc_new(struct hdmi_spec_per_device

*per_device,

sound/pci/hda/patch_hdmi.c +4 -2

+				      int pin_idx, int dev_idx)
 {
 	return 0;
 }
-static inline void eld_proc_free(struct hdmi_spec_per_pin *per_pin)
+static inline void eld_device_proc_free(struct hdmi_spec_per_device
+*per_device)
 {
 }
 #endif

@@ -1112,13 +1127,13 @@ static void hdmi_pin_setup_infoframe(struct
 hda_codec *codec,
 
 static void hdmi_setup_audio_infoframe(struct hda_codec *codec,
 				       struct hdmi_spec_per_pin *per_pin,
+				       struct hdmi_eld *eld,
 				       bool non_pcm)
 {
 	struct hdmi_spec *spec = codec->spec;
 	hda_nid_t pin_nid = per_pin->pin_nid;
 	int channels = per_pin->channels;
 	int active_channels;
-	struct hdmi_eld *eld;
 	int ca, ordered_ca;
 

sound/pci/hda/patch_hdmi.c +2 -1

 	if (!channels)

@@ -1129,7 +1144,8 @@ static void hdmi_setup_audio_infoframe(struct
 hda_codec *codec,
 					    AC_VERB_SET_AMP_GAIN_MUTE,
 					    AMP_OUT_UNMUTE);
 
-	eld = &per_pin->sink_eld;
+	if (!eld)
+		eld = &per_pin->devices[0].sink_eld;
 
 	if (!non_pcm && per_pin->chmap_set)

sound/pci/hda/patch_hdmi.c +1 -1

 		ca = hdmi_manual_channel_allocation(channels, per_pin->chmap);

@@ -1191,7 +1207,7 @@ static void hdmi_intrinsic_event(struct hda_codec
 *codec, unsigned int res)
 		return;
 	jack->jack_dirty = 1;
 
-	codec_dbg(codec,
+	codec_info(codec,
 		"HDMI hot plug event: Codec=%d Pin=%d Device=%d Inactive=%d
 Presence_Detect=%d ELD_Valid=%d\n",

sound/pci/hda/patch_hdmi.c +0 -1

 		codec->addr, jack->nid, dev_entry, !!(res & AC_UNSOL_RES_IA),
 		!!(res & AC_UNSOL_RES_PD), !!(res & AC_UNSOL_RES_ELDV)); @@
-1449,7 +1465,7 @@ static int hdmi_pcm_open(struct hda_pcm_stream

*hinfo,

sound/pci/hda/patch_hdmi.c +2 -2

 	if (snd_BUG_ON(pin_idx < 0))
 		return -EINVAL;
 	per_pin = get_pin(spec, pin_idx);
-	eld = &per_pin->sink_eld;
+	eld = &per_pin->devices[0].sink_eld;
 
 	err = hdmi_choose_cvt(codec, pin_idx, &cvt_idx, &mux_idx);
 	if (err < 0)

@@ -1530,7 +1546,7 @@ static bool hdmi_present_sense(struct
 hdmi_spec_per_pin *per_pin, int repoll)
 	struct hda_codec *codec = per_pin->codec;
 	struct hdmi_spec *spec = codec->spec;
 	struct hdmi_eld *eld = &spec->temp_eld;
-	struct hdmi_eld *pin_eld = &per_pin->sink_eld;
+	struct hdmi_eld *pin_eld;
 	hda_nid_t pin_nid = per_pin->pin_nid;
 	/*

* Always execute a GetPinSense verb here, even when called from @@
-1544,20 +1560,41 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)

sound/pci/hda/patch_hdmi.c +3 -7

 	bool update_eld = false;
 	bool eld_changed = false;
 	bool ret;
+	int device_num = 0;
+	bool need_repoll = false;
+	bool any_eld_valid = false;
 
 	snd_hda_power_up_pm(codec);
 	present = snd_hda_pin_sense(codec, pin_nid);
 
 	mutex_lock(&per_pin->lock);
-	pin_eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE);
-	if (pin_eld->monitor_present)
-		eld->eld_valid  = !!(present & AC_PINSENSE_ELDV);
-	else
-		eld->eld_valid = false;
 
-	codec_dbg(codec,
-		"HDMI status: Codec=%d Pin=%d Presence_Detect=%d

ELD_Valid=%d\n",

sound/pci/hda/patch_hdmi.c +2 -1

-		codec->addr, pin_nid, pin_eld->monitor_present, eld->eld_valid);
+	if (codec->dp_mst)
+		per_pin->num_devices = snd_hda_get_devices(codec, pin_nid,

per_pin->dev_list,

sound/pci/hda/patch_hdmi.c +7 -0

+							   AC_MAX_DEV_LIST_LEN);
+next_device:
+	pin_eld = &per_pin->devices[device_num].sink_eld;
+	eld_changed = false;
+	update_eld = false;
+	if (per_pin->num_devices) {
+		pin_eld->monitor_present = !!(per_pin->dev_list[device_num] &

AC_DE_PD);

sound/pci/hda/patch_hdmi.c +2 -0

+		if (pin_eld->monitor_present)
+			eld->eld_valid = !!(per_pin->dev_list[device_num] &

AC_DE_ELDV);

sound/pci/hda/patch_hdmi.c +4 -0

+		else
+			eld->eld_valid = false;
+		if (eld->eld_valid)
+			snd_hda_codec_write(codec, pin_nid, 0,

AC_VERB_SET_DEVICE_SEL, device_num);

sound/pci/hda/patch_hdmi.c +10 -0

+	} else {
+		pin_eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE);
+		if (pin_eld->monitor_present)
+			eld->eld_valid  = !!(present & AC_PINSENSE_ELDV);
+		else
+			eld->eld_valid = false;
+	}
+
+	codec_info(codec,
+		"HDMI status: Codec=%d Pin=%d Device=%d Presence_Detect=%d

ELD_Valid=%d\n",

sound/pci/hda/patch_hdmi.c +2 -1

+		   codec->addr, pin_nid, device_num, pin_eld->monitor_present,
+eld->eld_valid);
 
 	if (eld->eld_valid) {
 		if (spec->ops.pin_get_eld(codec, pin_nid, eld->eld_buffer, @@
-1573,11 +1610,11 @@ static bool hdmi_present_sense(struct

hdmi_spec_per_pin *per_pin, int repoll)

sound/pci/hda/patch_hdmi.c +4 -4

 		if (eld->eld_valid) {
 			snd_hdmi_show_eld(codec, &eld->info);
 			update_eld = true;
+			any_eld_valid = true;
 		}
 		else if (repoll) {
-			schedule_delayed_work(&per_pin->work,
-					      msecs_to_jiffies(300));
-			goto unlock;
+			need_repoll = true;
+			goto skip_to_next_device;
 		}
 	}

@@ -1614,7 +1651,7 @@ static bool hdmi_present_sense(struct
 hdmi_spec_per_pin *per_pin, int repoll)
 							per_pin->mux_idx);
 			}
 
-			hdmi_setup_audio_infoframe(codec, per_pin,
+			hdmi_setup_audio_infoframe(codec, per_pin, eld,
 						   per_pin->non_pcm);
 		}

sound/pci/hda/patch_hdmi.c +13 -2

 	}

@@ -1623,8 +1660,19 @@ static bool hdmi_present_sense(struct
 hdmi_spec_per_pin *per_pin, int repoll)
 		snd_ctl_notify(codec->card,
 			       SNDRV_CTL_EVENT_MASK_VALUE |
 SNDRV_CTL_EVENT_MASK_INFO,
 			       &per_pin->eld_ctl->id);
- unlock:
-	ret = !repoll || !pin_eld->monitor_present || pin_eld->eld_valid;
+skip_to_next_device:
+	if (codec->dp_mst) {
+		device_num++;
+		if (device_num < per_pin->num_devices)
+			goto next_device;
+	}
+
+	if (need_repoll) {
+		schedule_delayed_work(&per_pin->work,
+				      msecs_to_jiffies(300));
+		repoll = true;
+	}
+	ret = !repoll || any_eld_valid;
 

sound/pci/hda/patch_hdmi.c +1 -1

 	jack = snd_hda_jack_tbl_get(codec, pin_nid);
 	if (jack)

@@ -1807,7 +1855,7 @@ static int
 generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
 	per_pin->channels = substream->runtime->channels;
 	per_pin->setup = true;
 
-	hdmi_setup_audio_infoframe(codec, per_pin, non_pcm);
+	hdmi_setup_audio_infoframe(codec, per_pin, NULL, non_pcm);
 	mutex_unlock(&per_pin->lock);
 

sound/pci/hda/patch_hdmi.c +1 -1

 	if (spec->dyn_pin_out) {

@@ -2035,7 +2083,7 @@ static int hdmi_chmap_ctl_put(struct snd_kcontrol
 *kcontrol,
 	per_pin->chmap_set = true;
 	memcpy(per_pin->chmap, chmap, sizeof(chmap));
 	if (prepared)
-		hdmi_setup_audio_infoframe(codec, per_pin, per_pin->non_pcm);
+		hdmi_setup_audio_infoframe(codec, per_pin, NULL,
 per_pin->non_pcm);
 	mutex_unlock(&per_pin->lock);

sound/pci/hda/patch_hdmi.c +0 -0

 	return 0;

@@ -2147,7 +2195,7 @@ static int generic_hdmi_build_controls(struct

hda_codec *codec) static int generic_hdmi_init_per_pins(struct hda_codec *codec) {

sound/pci/hda/patch_hdmi.c +1 -2

 	struct hdmi_spec *spec = codec->spec;
-	int pin_idx;
+	int pin_idx, dev_idx;
 
 	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
 		struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx); @@
-2155,7 +2203,20 @@ static int generic_hdmi_init_per_pins(struct hda_codec

*codec)

sound/pci/hda/patch_hdmi.c +7 -1

 		per_pin->codec = codec;
 		mutex_init(&per_pin->lock);
 		INIT_DELAYED_WORK(&per_pin->work, hdmi_repoll_eld);
-		eld_proc_new(per_pin, pin_idx);
+
+		if (codec->dp_mst) {
+			for (dev_idx = 0; dev_idx < HDA_MAX_DEVICES; dev_idx++) {
+				per_pin->devices[dev_idx].device_idx = dev_idx;
+				per_pin->devices[dev_idx].pin = per_pin;
+
+				eld_device_proc_new(&per_pin->devices[dev_idx], pin_idx,

dev_idx);

sound/pci/hda/patch_hdmi.c +16 -2

+			}
+		} else {
+			per_pin->num_devices = 0;
+			per_pin->devices[0].device_idx = 0;
+			per_pin->devices[0].pin = per_pin;
+			eld_device_proc_new(&per_pin->devices[0], pin_idx, -1);
+		}
 	}
 	return 0;
 }

@@ -2191,13 +2252,19 @@ static void hdmi_array_free(struct hdmi_spec
 *spec)  static void generic_hdmi_free(struct hda_codec *codec)  {
 	struct hdmi_spec *spec = codec->spec;
-	int pin_idx;
+	int pin_idx, dev_idx;
 
 	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
 		struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
 
 		cancel_delayed_work_sync(&per_pin->work);
-		eld_proc_free(per_pin);
+		if (per_pin->num_devices) {
+			for (dev_idx = 0; dev_idx < per_pin->num_devices; dev_idx++) {
+				struct hdmi_spec_per_device *per_device =
 &per_pin->devices[dev_idx];
+				eld_device_proc_free(per_device);
+			}
+		} else
+			  eld_device_proc_free(&per_pin->devices[0]);
 	}
 
 	hdmi_array_free(spec);

@@ -2333,6 +2400,7 @@ static int patch_generic_hdmi(struct hda_codec
 *codec)
 	if (is_haswell_plus(codec)) {
 		intel_haswell_enable_all_pins(codec, true);
 		intel_haswell_fixup_enable_dp12(codec);
+		codec->dp_mst = true;
 	}
 

sound/pci/hda/patch_hdmi.c +1 -2

 	if (is_haswell_plus(codec) || is_valleyview_plus(codec)) @@ -2346,7
+2414,6 @@ static int patch_generic_hdmi(struct hda_codec *codec)
 	codec->patch_ops = generic_hdmi_patch_ops;
 	if (is_haswell_plus(codec)) {
 		codec->patch_ops.set_power_state = haswell_set_power_state;
-		codec->dp_mst = true;
 	}
 
 	generic_hdmi_init_per_pins(codec);
--

2.4.1

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Hi Takashi/Dave,

Shall we move or cc this discussion on audio driver side to ALSA ML?

I think we also need to decide how to manage PCM devices for DP MST. Now the HD-A driver create a PCM device for each pin, and the substream number is 1 for each PCM. Now with DP MST enabled, each pin can support multiple streams (e.g. 3 on Intel HSW/BDW/SKL).

There may be 2 options:
-#1: Let an HDMI codec specify number of substreams, same as the number of device entries on a pin. We can specify 3 for HSW/BDW/SKL. Other vendors can also specify a value according to actual HW capabilities.

So for HSW, we have 3x3 subtreams totally. But we only have 3 convertors (for 3 display pipelines), so we can open up to 3 substreams at the same time. When the audio driver finds all 3 convertors are used when opening a new substream, it will fail.

- #2: Create PCM device dynamically. Only create a PCM devices for a device entry which connects to monitor with audio support. When the monitor is removed, the PCM device will be disconnected, closed and removed, similar to the USB case.

This will change ALSA core. But there will be less PCM devices and substreams, since the number of connected monitors Is decided by the actual GPU display pipeline.

Quoted 481 lines

Signed-off-by Dave Airlie <[email protected]>

---
 include/sound/hda_verbs.h  |   2 +
 sound/pci/hda/hda_codec.c  |   1 +
 sound/pci/hda/hda_proc.c   |   5 +-
 sound/pci/hda/patch_hdmi.c | 181
+++++++++++++++++++++++++++++++--------------
 4 files changed, 131 insertions(+), 58 deletions(-)

include/sound/hda_verbs.h index renamed from include/sound/hda_verbs.h +0 -0

d0509db..3b62ac5 100644

include/sound/hda_verbs.h +2 -0

@@ -75,6 +75,7 @@ enum {
 #define AC_VERB_GET_HDMI_CHAN_SLOT           0x0f34
 #define AC_VERB_GET_DEVICE_SEL                       0xf35
 #define AC_VERB_GET_DEVICE_LIST                      0xf36
+#define AC_VERB_GET_DP_STREAM_ID             0xf3c
 
 /*
  * SET verbs

@@ -115,6 +116,7 @@ enum {
 #define AC_VERB_SET_HDMI_CP_CTRL             0x733
 #define AC_VERB_SET_HDMI_CHAN_SLOT           0x734
 #define AC_VERB_SET_DEVICE_SEL                       0x735
+#define AC_VERB_SET_DP_STREAM_ID             0x73C
 
 /*
  * Parameter IDs

sound/pci/hda/hda_codec.c index renamed from sound/pci/hda/hda_codec.c +0 -0

5645481..3981c63 100644

sound/pci/hda/hda_codec.c +1 -0

@@ -482,6 +482,7 @@ int snd_hda_get_devices(struct hda_codec *codec,
 hda_nid_t nid,
      }
      return devices;
 }
+EXPORT_SYMBOL_GPL(snd_hda_get_devices);
 
 /*

* destructor

sound/pci/hda/hda_proc.c index renamed from sound/pci/hda/hda_proc.c +0 -0

baaf7ed0..39fac53 100644

sound/pci/hda/hda_proc.c +4 -1

@@ -644,10 +644,13 @@ static void print_device_list(struct snd_info_buffer
 *buffer,
      int i, curr = -1;
      u8 dev_list[AC_MAX_DEV_LIST_LEN];
      int devlist_len;
+     int dp_s_id;
 
+     dp_s_id = snd_hda_codec_read(codec, nid, 0,
+                             AC_VERB_GET_DP_STREAM_ID, 0);
      devlist_len = snd_hda_get_devices(codec, nid, dev_list,
                                      AC_MAX_DEV_LIST_LEN);
-     snd_iprintf(buffer, "  Devices: %d\n", devlist_len);
+     snd_iprintf(buffer, "  Devices: %d: 0x%x\n", devlist_len, dp_s_id);
      if (devlist_len <= 0)
              return;

sound/pci/hda/patch_hdmi.c index renamed from sound/pci/hda/patch_hdmi.c +0 -0

5f44f60..8272656 100644

sound/pci/hda/patch_hdmi.c +16 -1

@@ -68,6 +68,17 @@ struct hdmi_spec_per_cvt {
 /* max. connections to a widget */
 #define HDA_MAX_CONNECTIONS  32
 
+struct hdmi_spec_per_pin;
+#define HDA_MAX_DEVICES 3
+struct hdmi_spec_per_device {
+     struct hdmi_spec_per_pin *pin;
+     int device_idx;
+     struct hdmi_eld sink_eld;
+#ifdef CONFIG_PROC_FS
+     struct snd_info_entry *proc_entry;
+#endif
+};
+
 struct hdmi_spec_per_pin {
      hda_nid_t pin_nid;
      int num_mux_nids;

@@ -76,7 +87,11 @@ struct hdmi_spec_per_pin {
      hda_nid_t cvt_nid;
 
      struct hda_codec *codec;
-     struct hdmi_eld sink_eld;
+
+     int num_devices;
+     u8 dev_list[AC_MAX_DEV_LIST_LEN];
+     struct hdmi_spec_per_device devices[HDA_MAX_DEVICES];
+
      struct mutex lock;
      struct delayed_work work;
      struct snd_kcontrol *eld_ctl;

@@ -86,9 +101,6 @@ struct hdmi_spec_per_pin {
      bool non_pcm;
      bool chmap_set;         /* channel-map override by ALSA API? */
      unsigned char chmap[8]; /* ALSA API channel-map */ -#ifdef

CONFIG_PROC_FS

sound/pci/hda/patch_hdmi.c +1 -3

-     struct snd_info_entry *proc_entry;
-#endif
 };
 
 struct cea_channel_speaker_allocation;

@@ -409,7 +421,7 @@ static int hdmi_eld_ctl_info(struct snd_kcontrol
 *kcontrol,
 
      pin_idx = kcontrol->private_value;
      per_pin = get_pin(spec, pin_idx);
-     eld = &per_pin->sink_eld;
+     eld = &per_pin->devices[0].sink_eld;
 
      mutex_lock(&per_pin->lock);

      uinfo->count = eld->eld_valid ? eld->eld_size : 0; @@ -429,7 +441,7 @@

static int hdmi_eld_ctl_get(struct snd_kcontrol *kcontrol,

      pin_idx = kcontrol->private_value;
      per_pin = get_pin(spec, pin_idx);

sound/pci/hda/patch_hdmi.c +21 -16

-     eld = &per_pin->sink_eld;
+     eld = &per_pin->devices[0].sink_eld;
 
      mutex_lock(&per_pin->lock);
      if (eld->eld_size > ARRAY_SIZE(ucontrol->value.bytes.data)) { @@ -549,60
+561,63 @@ static void hdmi_set_channel_count(struct hda_codec *codec,
  */
 
 #ifdef CONFIG_PROC_FS
-static void print_eld_info(struct snd_info_entry *entry,
-                        struct snd_info_buffer *buffer)
+static void print_eld_info_device(struct snd_info_entry *entry,
+                               struct snd_info_buffer *buffer)
 {
-     struct hdmi_spec_per_pin *per_pin = entry->private_data;
+     struct hdmi_spec_per_device *per_device = entry->private_data;
 
-     mutex_lock(&per_pin->lock);
-     snd_hdmi_print_eld_info(&per_pin->sink_eld, buffer);
-     mutex_unlock(&per_pin->lock);
+     mutex_lock(&per_device->pin->lock);
+     snd_hdmi_print_eld_info(&per_device->sink_eld, buffer);
+     mutex_unlock(&per_device->pin->lock);
 }
 
-static void write_eld_info(struct snd_info_entry *entry,
-                        struct snd_info_buffer *buffer)
+static void write_eld_info_device(struct snd_info_entry *entry,
+                               struct snd_info_buffer *buffer)
 {
-     struct hdmi_spec_per_pin *per_pin = entry->private_data;
+     struct hdmi_spec_per_device *per_device = entry->private_data;
 
-     mutex_lock(&per_pin->lock);
-     snd_hdmi_write_eld_info(&per_pin->sink_eld, buffer);
-     mutex_unlock(&per_pin->lock);
+     mutex_lock(&per_device->pin->lock);
+     snd_hdmi_write_eld_info(&per_device->sink_eld, buffer);
+     mutex_unlock(&per_device->pin->lock);
 }
 
-static int eld_proc_new(struct hdmi_spec_per_pin *per_pin, int index)
+static int eld_device_proc_new(struct hdmi_spec_per_device *per_device,
+int pin_idx, int dev_idx)
 {
      char name[32];
-     struct hda_codec *codec = per_pin->codec;
+     struct hda_codec *codec = per_device->pin->codec;
      struct snd_info_entry *entry;
      int err;
 
-     snprintf(name, sizeof(name), "eld#%d.%d", codec->addr, index);
+     if (dev_idx == -1)
+             snprintf(name, sizeof(name), "eld#%d.%d", codec->addr, pin_idx);
+     else
+             snprintf(name, sizeof(name), "eld#%d.%d.%d", codec->addr,

pin_idx,

sound/pci/hda/patch_hdmi.c +7 -7

+dev_idx);
      err = snd_card_proc_new(codec->card, name, &entry);
      if (err < 0)
              return err;
 
-     snd_info_set_text_ops(entry, per_pin, print_eld_info);
-     entry->c.text.write = write_eld_info;
+     snd_info_set_text_ops(entry, per_device, print_eld_info_device);
+     entry->c.text.write = write_eld_info_device;
      entry->mode |= S_IWUSR;
-     per_pin->proc_entry = entry;
+     per_device->proc_entry = entry;
 
      return 0;
 }
 
-static void eld_proc_free(struct hdmi_spec_per_pin *per_pin)
+static void eld_device_proc_free(struct hdmi_spec_per_device
+*per_device)
 {
-     if (!per_pin->codec->bus->shutdown && per_pin->proc_entry) {
-             snd_device_free(per_pin->codec->card, per_pin->proc_entry);
-             per_pin->proc_entry = NULL;
+     if (!per_device->pin->codec->bus->shutdown && per_device->proc_entry)

{
+ snd_device_free(per_device->pin->codec->card,
per_device->proc_entry);

sound/pci/hda/patch_hdmi.c +2 -2

+             per_device->proc_entry = NULL;
      }
 }
 #else
-static inline int eld_proc_new(struct hdmi_spec_per_pin *per_pin,
-                            int index)
+static inline int eld_device_proc_new(struct hdmi_spec_per_device

*per_device,

sound/pci/hda/patch_hdmi.c +4 -2

+                                   int pin_idx, int dev_idx)
 {
      return 0;
 }
-static inline void eld_proc_free(struct hdmi_spec_per_pin *per_pin)
+static inline void eld_device_proc_free(struct hdmi_spec_per_device
+*per_device)
 {
 }
 #endif

@@ -1112,13 +1127,13 @@ static void hdmi_pin_setup_infoframe(struct
 hda_codec *codec,
 
 static void hdmi_setup_audio_infoframe(struct hda_codec *codec,
                                     struct hdmi_spec_per_pin *per_pin,
+                                    struct hdmi_eld *eld,
                                     bool non_pcm)
 {
      struct hdmi_spec *spec = codec->spec;
      hda_nid_t pin_nid = per_pin->pin_nid;
      int channels = per_pin->channels;
      int active_channels;
-     struct hdmi_eld *eld;
      int ca, ordered_ca;
 

if (!channels)

sound/pci/hda/patch_hdmi.c +2 -1

@@ -1129,7 +1144,8 @@ static void hdmi_setup_audio_infoframe(struct
 hda_codec *codec,
                                          AC_VERB_SET_AMP_GAIN_MUTE,
                                          AMP_OUT_UNMUTE);
 
-     eld = &per_pin->sink_eld;
+     if (!eld)
+             eld = &per_pin->devices[0].sink_eld;
 
      if (!non_pcm && per_pin->chmap_set)

              ca = hdmi_manual_channel_allocation(channels, per_pin->chmap);

sound/pci/hda/patch_hdmi.c +1 -1

@@ -1191,7 +1207,7 @@ static void hdmi_intrinsic_event(struct hda_codec
 *codec, unsigned int res)
              return;
      jack->jack_dirty = 1;
 
-     codec_dbg(codec,
+     codec_info(codec,
              "HDMI hot plug event: Codec=%d Pin=%d Device=%d Inactive=%d
 Presence_Detect=%d ELD_Valid=%d\n",

              codec->addr, jack->nid, dev_entry, !!(res & AC_UNSOL_RES_IA),
              !!(res & AC_UNSOL_RES_PD), !!(res & AC_UNSOL_RES_ELDV)); @@
-1449,7 +1465,7 @@ static int hdmi_pcm_open(struct hda_pcm_stream
*hinfo,
      if (snd_BUG_ON(pin_idx < 0))
              return -EINVAL;
      per_pin = get_pin(spec, pin_idx);

sound/pci/hda/patch_hdmi.c +2 -2

-     eld = &per_pin->sink_eld;
+     eld = &per_pin->devices[0].sink_eld;
 
      err = hdmi_choose_cvt(codec, pin_idx, &cvt_idx, &mux_idx);
      if (err < 0)

@@ -1530,7 +1546,7 @@ static bool hdmi_present_sense(struct
 hdmi_spec_per_pin *per_pin, int repoll)
      struct hda_codec *codec = per_pin->codec;
      struct hdmi_spec *spec = codec->spec;
      struct hdmi_eld *eld = &spec->temp_eld;
-     struct hdmi_eld *pin_eld = &per_pin->sink_eld;
+     struct hdmi_eld *pin_eld;
      hda_nid_t pin_nid = per_pin->pin_nid;
      /*

* Always execute a GetPinSense verb here, even when called from @@
-1544,20 +1560,41 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)

      bool update_eld = false;
      bool eld_changed = false;
      bool ret;

sound/pci/hda/patch_hdmi.c +3 -7

+     int device_num = 0;
+     bool need_repoll = false;
+     bool any_eld_valid = false;
 
      snd_hda_power_up_pm(codec);
      present = snd_hda_pin_sense(codec, pin_nid);
 
      mutex_lock(&per_pin->lock);
-     pin_eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE);
-     if (pin_eld->monitor_present)
-             eld->eld_valid  = !!(present & AC_PINSENSE_ELDV);
-     else
-             eld->eld_valid = false;
 
-     codec_dbg(codec,
-             "HDMI status: Codec=%d Pin=%d Presence_Detect=%d

ELD_Valid=%d\n",

sound/pci/hda/patch_hdmi.c +2 -1

-             codec->addr, pin_nid, pin_eld->monitor_present, eld->eld_valid);
+     if (codec->dp_mst)
+             per_pin->num_devices = snd_hda_get_devices(codec, pin_nid,

per_pin->dev_list,

sound/pci/hda/patch_hdmi.c +7 -0

+                                                        AC_MAX_DEV_LIST_LEN);
+next_device:
+     pin_eld = &per_pin->devices[device_num].sink_eld;
+     eld_changed = false;
+     update_eld = false;
+     if (per_pin->num_devices) {
+             pin_eld->monitor_present = !!(per_pin->dev_list[device_num] &

AC_DE_PD);

sound/pci/hda/patch_hdmi.c +2 -0

+             if (pin_eld->monitor_present)
+                     eld->eld_valid = !!(per_pin->dev_list[device_num] &

AC_DE_ELDV);

sound/pci/hda/patch_hdmi.c +4 -0

+             else
+                     eld->eld_valid = false;
+             if (eld->eld_valid)
+                     snd_hda_codec_write(codec, pin_nid, 0,

AC_VERB_SET_DEVICE_SEL, device_num);

sound/pci/hda/patch_hdmi.c +10 -0

+     } else {
+             pin_eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE);
+             if (pin_eld->monitor_present)
+                     eld->eld_valid  = !!(present & AC_PINSENSE_ELDV);
+             else
+                     eld->eld_valid = false;
+     }
+
+     codec_info(codec,
+             "HDMI status: Codec=%d Pin=%d Device=%d Presence_Detect=%d

ELD_Valid=%d\n",

sound/pci/hda/patch_hdmi.c +2 -1

+                codec->addr, pin_nid, device_num, pin_eld->monitor_present,
+eld->eld_valid);
 
      if (eld->eld_valid) {
              if (spec->ops.pin_get_eld(codec, pin_nid, eld->eld_buffer, @@
-1573,11 +1610,11 @@ static bool hdmi_present_sense(struct

hdmi_spec_per_pin *per_pin, int repoll)

              if (eld->eld_valid) {
                      snd_hdmi_show_eld(codec, &eld->info);
                      update_eld = true;

sound/pci/hda/patch_hdmi.c +4 -4

+                     any_eld_valid = true;
              }
              else if (repoll) {
-                     schedule_delayed_work(&per_pin->work,
-                                           msecs_to_jiffies(300));
-                     goto unlock;
+                     need_repoll = true;
+                     goto skip_to_next_device;
              }
      }

@@ -1614,7 +1651,7 @@ static bool hdmi_present_sense(struct
 hdmi_spec_per_pin *per_pin, int repoll)
                                                      per_pin->mux_idx);
                      }
 
-                     hdmi_setup_audio_infoframe(codec, per_pin,
+                     hdmi_setup_audio_infoframe(codec, per_pin, eld,
                                                 per_pin->non_pcm);
              }

      }

sound/pci/hda/patch_hdmi.c +13 -2

@@ -1623,8 +1660,19 @@ static bool hdmi_present_sense(struct
 hdmi_spec_per_pin *per_pin, int repoll)
              snd_ctl_notify(codec->card,
                             SNDRV_CTL_EVENT_MASK_VALUE |
 SNDRV_CTL_EVENT_MASK_INFO,
                             &per_pin->eld_ctl->id);
- unlock:
-     ret = !repoll || !pin_eld->monitor_present || pin_eld->eld_valid;
+skip_to_next_device:
+     if (codec->dp_mst) {
+             device_num++;
+             if (device_num < per_pin->num_devices)
+                     goto next_device;
+     }
+
+     if (need_repoll) {
+             schedule_delayed_work(&per_pin->work,
+                                   msecs_to_jiffies(300));
+             repoll = true;
+     }
+     ret = !repoll || any_eld_valid;
 

      jack = snd_hda_jack_tbl_get(codec, pin_nid);
      if (jack)

sound/pci/hda/patch_hdmi.c +1 -1

@@ -1807,7 +1855,7 @@ static int
 generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
      per_pin->channels = substream->runtime->channels;
      per_pin->setup = true;
 
-     hdmi_setup_audio_infoframe(codec, per_pin, non_pcm);
+     hdmi_setup_audio_infoframe(codec, per_pin, NULL, non_pcm);
      mutex_unlock(&per_pin->lock);
 

      if (spec->dyn_pin_out) {

sound/pci/hda/patch_hdmi.c +1 -1

@@ -2035,7 +2083,7 @@ static int hdmi_chmap_ctl_put(struct snd_kcontrol
 *kcontrol,
      per_pin->chmap_set = true;
      memcpy(per_pin->chmap, chmap, sizeof(chmap));
      if (prepared)
-             hdmi_setup_audio_infoframe(codec, per_pin, per_pin->non_pcm);
+             hdmi_setup_audio_infoframe(codec, per_pin, NULL,
 per_pin->non_pcm);
      mutex_unlock(&per_pin->lock);

      return 0;

sound/pci/hda/patch_hdmi.c +0 -0

@@ -2147,7 +2195,7 @@ static int generic_hdmi_build_controls(struct

hda_codec *codec) static int generic_hdmi_init_per_pins(struct hda_codec

*codec)  {
      struct hdmi_spec *spec = codec->spec;

sound/pci/hda/patch_hdmi.c +1 -2

-     int pin_idx;
+     int pin_idx, dev_idx;
 
      for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
              struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx); @@
-2155,7 +2203,20 @@ static int generic_hdmi_init_per_pins(struct hda_codec

*codec)

              per_pin->codec = codec;
              mutex_init(&per_pin->lock);
              INIT_DELAYED_WORK(&per_pin->work, hdmi_repoll_eld);

sound/pci/hda/patch_hdmi.c +7 -1

-             eld_proc_new(per_pin, pin_idx);
+
+             if (codec->dp_mst) {
+                     for (dev_idx = 0; dev_idx < HDA_MAX_DEVICES; dev_idx++) {
+                             per_pin->devices[dev_idx].device_idx = dev_idx;
+                             per_pin->devices[dev_idx].pin = per_pin;
+
+                             eld_device_proc_new(&per_pin->devices[dev_idx], pin_idx,

dev_idx);

sound/pci/hda/patch_hdmi.c +16 -2

+                     }
+             } else {
+                     per_pin->num_devices = 0;
+                     per_pin->devices[0].device_idx = 0;
+                     per_pin->devices[0].pin = per_pin;
+                     eld_device_proc_new(&per_pin->devices[0], pin_idx, -1);
+             }
      }
      return 0;
 }

@@ -2191,13 +2252,19 @@ static void hdmi_array_free(struct hdmi_spec
 *spec)  static void generic_hdmi_free(struct hda_codec *codec)  {
      struct hdmi_spec *spec = codec->spec;
-     int pin_idx;
+     int pin_idx, dev_idx;
 
      for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
              struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
 
              cancel_delayed_work_sync(&per_pin->work);
-             eld_proc_free(per_pin);
+             if (per_pin->num_devices) {
+                     for (dev_idx = 0; dev_idx < per_pin->num_devices; dev_idx++) {
+                             struct hdmi_spec_per_device *per_device =
 &per_pin->devices[dev_idx];
+                             eld_device_proc_free(per_device);
+                     }
+             } else
+                       eld_device_proc_free(&per_pin->devices[0]);
      }
 
      hdmi_array_free(spec);

@@ -2333,6 +2400,7 @@ static int patch_generic_hdmi(struct hda_codec
 *codec)
      if (is_haswell_plus(codec)) {
              intel_haswell_enable_all_pins(codec, true);
              intel_haswell_fixup_enable_dp12(codec);
+             codec->dp_mst = true;
      }
 

      if (is_haswell_plus(codec) || is_valleyview_plus(codec)) @@ -2346,7
+2414,6 @@ static int patch_generic_hdmi(struct hda_codec *codec)
      codec->patch_ops = generic_hdmi_patch_ops;
      if (is_haswell_plus(codec)) {
              codec->patch_ops.set_power_state = haswell_set_power_state;
-             codec->dp_mst = true;
      }

Quoted 8 lines

      generic_hdmi_init_per_pins(codec);

--
2.4.1

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Quoted 3 lines · Mengdong

Hi Takashi/Dave,

Shall we move or cc this discussion on audio driver side to ALSA ML?

Oops I thought I had cc'ed these patches to alsa-devel as well when I sent them.

Quoted 10 lines

I think we also need to decide how to manage PCM devices for DP MST. Now the HD-A driver create a PCM device for each pin, and the substream number is 1 for each PCM. Now with DP MST enabled, each pin can support multiple streams (e.g. 3 on Intel HSW/BDW/SKL).

There may be 2 options:
-#1: Let an HDMI codec specify number of substreams, same as the number of device entries on a pin. We can specify 3 for HSW/BDW/SKL. Other vendors can also specify a value according to actual HW capabilities.

Quoted 4 lines

So for HSW, we have 3x3 subtreams totally. But we only have 3 convertors (for 3 display pipelines), so we can open up to 3 substreams at the same time. When the audio driver finds all 3 convertors are used when opening a new substream, it will fail.

One thing I noticed is the number of devices on a PIN is only updated when the MST device is plugged in so normally pins 5,6,7 have 0 devices, and when I plug in MST device, I get the 3 devices on port 6. So it seems dynamic enough at this point, though I guess it'll always be 0 or 3.

Quoted 6 lines

- #2: Create PCM device dynamically. Only create a PCM devices for a device entry which connects to monitor with audio support. When the monitor is removed, the PCM device will be disconnected, closed and removed, similar to the USB case.

Quoted 3 lines

This will change ALSA core. But there will be less PCM devices and substreams, since the number of connected monitors Is decided by the actual GPU display pipeline.

I like this option more, since I think it should be more like USB, but I've no idea how much work it would be from the alsa side, this patch was probably as deep into alsa as I've gone.

Dave.

Quoted 481 lines

Quoted 481 lines

Signed-off-by Dave Airlie <[email protected]>

---
 include/sound/hda_verbs.h  |   2 +
 sound/pci/hda/hda_codec.c  |   1 +
 sound/pci/hda/hda_proc.c   |   5 +-
 sound/pci/hda/patch_hdmi.c | 181
+++++++++++++++++++++++++++++++--------------
 4 files changed, 131 insertions(+), 58 deletions(-)

include/sound/hda_verbs.h index renamed from include/sound/hda_verbs.h +0 -0

d0509db..3b62ac5 100644

include/sound/hda_verbs.h +2 -0

@@ -75,6 +75,7 @@ enum {
 #define AC_VERB_GET_HDMI_CHAN_SLOT           0x0f34
 #define AC_VERB_GET_DEVICE_SEL                       0xf35
 #define AC_VERB_GET_DEVICE_LIST                      0xf36
+#define AC_VERB_GET_DP_STREAM_ID             0xf3c
 
 /*
  * SET verbs

@@ -115,6 +116,7 @@ enum {
 #define AC_VERB_SET_HDMI_CP_CTRL             0x733
 #define AC_VERB_SET_HDMI_CHAN_SLOT           0x734
 #define AC_VERB_SET_DEVICE_SEL                       0x735
+#define AC_VERB_SET_DP_STREAM_ID             0x73C
 
 /*
  * Parameter IDs

sound/pci/hda/hda_codec.c index renamed from sound/pci/hda/hda_codec.c +0 -0

5645481..3981c63 100644

sound/pci/hda/hda_codec.c +1 -0

@@ -482,6 +482,7 @@ int snd_hda_get_devices(struct hda_codec *codec,
 hda_nid_t nid,
      }
      return devices;
 }
+EXPORT_SYMBOL_GPL(snd_hda_get_devices);
 
 /*

* destructor

sound/pci/hda/hda_proc.c index renamed from sound/pci/hda/hda_proc.c +0 -0

baaf7ed0..39fac53 100644

sound/pci/hda/hda_proc.c +4 -1

@@ -644,10 +644,13 @@ static void print_device_list(struct snd_info_buffer
 *buffer,
      int i, curr = -1;
      u8 dev_list[AC_MAX_DEV_LIST_LEN];
      int devlist_len;
+     int dp_s_id;
 
+     dp_s_id = snd_hda_codec_read(codec, nid, 0,
+                             AC_VERB_GET_DP_STREAM_ID, 0);
      devlist_len = snd_hda_get_devices(codec, nid, dev_list,
                                      AC_MAX_DEV_LIST_LEN);
-     snd_iprintf(buffer, "  Devices: %d\n", devlist_len);
+     snd_iprintf(buffer, "  Devices: %d: 0x%x\n", devlist_len, dp_s_id);
      if (devlist_len <= 0)
              return;

sound/pci/hda/patch_hdmi.c index renamed from sound/pci/hda/patch_hdmi.c +0 -0

5f44f60..8272656 100644

sound/pci/hda/patch_hdmi.c +16 -1

@@ -68,6 +68,17 @@ struct hdmi_spec_per_cvt {
 /* max. connections to a widget */
 #define HDA_MAX_CONNECTIONS  32
 
+struct hdmi_spec_per_pin;
+#define HDA_MAX_DEVICES 3
+struct hdmi_spec_per_device {
+     struct hdmi_spec_per_pin *pin;
+     int device_idx;
+     struct hdmi_eld sink_eld;
+#ifdef CONFIG_PROC_FS
+     struct snd_info_entry *proc_entry;
+#endif
+};
+
 struct hdmi_spec_per_pin {
      hda_nid_t pin_nid;
      int num_mux_nids;

@@ -76,7 +87,11 @@ struct hdmi_spec_per_pin {
      hda_nid_t cvt_nid;
 
      struct hda_codec *codec;
-     struct hdmi_eld sink_eld;
+
+     int num_devices;
+     u8 dev_list[AC_MAX_DEV_LIST_LEN];
+     struct hdmi_spec_per_device devices[HDA_MAX_DEVICES];
+
      struct mutex lock;
      struct delayed_work work;
      struct snd_kcontrol *eld_ctl;

@@ -86,9 +101,6 @@ struct hdmi_spec_per_pin {
      bool non_pcm;
      bool chmap_set;         /* channel-map override by ALSA API? */
      unsigned char chmap[8]; /* ALSA API channel-map */ -#ifdef

CONFIG_PROC_FS

sound/pci/hda/patch_hdmi.c +1 -3

-     struct snd_info_entry *proc_entry;
-#endif
 };
 
 struct cea_channel_speaker_allocation;

@@ -409,7 +421,7 @@ static int hdmi_eld_ctl_info(struct snd_kcontrol
 *kcontrol,
 
      pin_idx = kcontrol->private_value;
      per_pin = get_pin(spec, pin_idx);
-     eld = &per_pin->sink_eld;
+     eld = &per_pin->devices[0].sink_eld;
 
      mutex_lock(&per_pin->lock);

      uinfo->count = eld->eld_valid ? eld->eld_size : 0; @@ -429,7 +441,7 @@

static int hdmi_eld_ctl_get(struct snd_kcontrol *kcontrol,

      pin_idx = kcontrol->private_value;
      per_pin = get_pin(spec, pin_idx);

sound/pci/hda/patch_hdmi.c +21 -16

-     eld = &per_pin->sink_eld;
+     eld = &per_pin->devices[0].sink_eld;
 
      mutex_lock(&per_pin->lock);
      if (eld->eld_size > ARRAY_SIZE(ucontrol->value.bytes.data)) { @@ -549,60
+561,63 @@ static void hdmi_set_channel_count(struct hda_codec *codec,
  */
 
 #ifdef CONFIG_PROC_FS
-static void print_eld_info(struct snd_info_entry *entry,
-                        struct snd_info_buffer *buffer)
+static void print_eld_info_device(struct snd_info_entry *entry,
+                               struct snd_info_buffer *buffer)
 {
-     struct hdmi_spec_per_pin *per_pin = entry->private_data;
+     struct hdmi_spec_per_device *per_device = entry->private_data;
 
-     mutex_lock(&per_pin->lock);
-     snd_hdmi_print_eld_info(&per_pin->sink_eld, buffer);
-     mutex_unlock(&per_pin->lock);
+     mutex_lock(&per_device->pin->lock);
+     snd_hdmi_print_eld_info(&per_device->sink_eld, buffer);
+     mutex_unlock(&per_device->pin->lock);
 }
 
-static void write_eld_info(struct snd_info_entry *entry,
-                        struct snd_info_buffer *buffer)
+static void write_eld_info_device(struct snd_info_entry *entry,
+                               struct snd_info_buffer *buffer)
 {
-     struct hdmi_spec_per_pin *per_pin = entry->private_data;
+     struct hdmi_spec_per_device *per_device = entry->private_data;
 
-     mutex_lock(&per_pin->lock);
-     snd_hdmi_write_eld_info(&per_pin->sink_eld, buffer);
-     mutex_unlock(&per_pin->lock);
+     mutex_lock(&per_device->pin->lock);
+     snd_hdmi_write_eld_info(&per_device->sink_eld, buffer);
+     mutex_unlock(&per_device->pin->lock);
 }
 
-static int eld_proc_new(struct hdmi_spec_per_pin *per_pin, int index)
+static int eld_device_proc_new(struct hdmi_spec_per_device *per_device,
+int pin_idx, int dev_idx)
 {
      char name[32];
-     struct hda_codec *codec = per_pin->codec;
+     struct hda_codec *codec = per_device->pin->codec;
      struct snd_info_entry *entry;
      int err;
 
-     snprintf(name, sizeof(name), "eld#%d.%d", codec->addr, index);
+     if (dev_idx == -1)
+             snprintf(name, sizeof(name), "eld#%d.%d", codec->addr, pin_idx);
+     else
+             snprintf(name, sizeof(name), "eld#%d.%d.%d", codec->addr,

pin_idx,

sound/pci/hda/patch_hdmi.c +7 -7

+dev_idx);
      err = snd_card_proc_new(codec->card, name, &entry);
      if (err < 0)
              return err;
 
-     snd_info_set_text_ops(entry, per_pin, print_eld_info);
-     entry->c.text.write = write_eld_info;
+     snd_info_set_text_ops(entry, per_device, print_eld_info_device);
+     entry->c.text.write = write_eld_info_device;
      entry->mode |= S_IWUSR;
-     per_pin->proc_entry = entry;
+     per_device->proc_entry = entry;
 
      return 0;
 }
 
-static void eld_proc_free(struct hdmi_spec_per_pin *per_pin)
+static void eld_device_proc_free(struct hdmi_spec_per_device
+*per_device)
 {
-     if (!per_pin->codec->bus->shutdown && per_pin->proc_entry) {
-             snd_device_free(per_pin->codec->card, per_pin->proc_entry);
-             per_pin->proc_entry = NULL;
+     if (!per_device->pin->codec->bus->shutdown && per_device->proc_entry)

{
+ snd_device_free(per_device->pin->codec->card,
per_device->proc_entry);

sound/pci/hda/patch_hdmi.c +2 -2

+             per_device->proc_entry = NULL;
      }
 }
 #else
-static inline int eld_proc_new(struct hdmi_spec_per_pin *per_pin,
-                            int index)
+static inline int eld_device_proc_new(struct hdmi_spec_per_device

*per_device,

sound/pci/hda/patch_hdmi.c +4 -2

+                                   int pin_idx, int dev_idx)
 {
      return 0;
 }
-static inline void eld_proc_free(struct hdmi_spec_per_pin *per_pin)
+static inline void eld_device_proc_free(struct hdmi_spec_per_device
+*per_device)
 {
 }
 #endif

@@ -1112,13 +1127,13 @@ static void hdmi_pin_setup_infoframe(struct
 hda_codec *codec,
 
 static void hdmi_setup_audio_infoframe(struct hda_codec *codec,
                                     struct hdmi_spec_per_pin *per_pin,
+                                    struct hdmi_eld *eld,
                                     bool non_pcm)
 {
      struct hdmi_spec *spec = codec->spec;
      hda_nid_t pin_nid = per_pin->pin_nid;
      int channels = per_pin->channels;
      int active_channels;
-     struct hdmi_eld *eld;
      int ca, ordered_ca;
 

if (!channels)

sound/pci/hda/patch_hdmi.c +2 -1

@@ -1129,7 +1144,8 @@ static void hdmi_setup_audio_infoframe(struct
 hda_codec *codec,
                                          AC_VERB_SET_AMP_GAIN_MUTE,
                                          AMP_OUT_UNMUTE);
 
-     eld = &per_pin->sink_eld;
+     if (!eld)
+             eld = &per_pin->devices[0].sink_eld;
 
      if (!non_pcm && per_pin->chmap_set)

              ca = hdmi_manual_channel_allocation(channels, per_pin->chmap);

sound/pci/hda/patch_hdmi.c +1 -1

@@ -1191,7 +1207,7 @@ static void hdmi_intrinsic_event(struct hda_codec
 *codec, unsigned int res)
              return;
      jack->jack_dirty = 1;
 
-     codec_dbg(codec,
+     codec_info(codec,
              "HDMI hot plug event: Codec=%d Pin=%d Device=%d Inactive=%d
 Presence_Detect=%d ELD_Valid=%d\n",

              codec->addr, jack->nid, dev_entry, !!(res & AC_UNSOL_RES_IA),
              !!(res & AC_UNSOL_RES_PD), !!(res & AC_UNSOL_RES_ELDV)); @@
-1449,7 +1465,7 @@ static int hdmi_pcm_open(struct hda_pcm_stream
*hinfo,
      if (snd_BUG_ON(pin_idx < 0))
              return -EINVAL;
      per_pin = get_pin(spec, pin_idx);

sound/pci/hda/patch_hdmi.c +2 -2

-     eld = &per_pin->sink_eld;
+     eld = &per_pin->devices[0].sink_eld;
 
      err = hdmi_choose_cvt(codec, pin_idx, &cvt_idx, &mux_idx);
      if (err < 0)

@@ -1530,7 +1546,7 @@ static bool hdmi_present_sense(struct
 hdmi_spec_per_pin *per_pin, int repoll)
      struct hda_codec *codec = per_pin->codec;
      struct hdmi_spec *spec = codec->spec;
      struct hdmi_eld *eld = &spec->temp_eld;
-     struct hdmi_eld *pin_eld = &per_pin->sink_eld;
+     struct hdmi_eld *pin_eld;
      hda_nid_t pin_nid = per_pin->pin_nid;
      /*

* Always execute a GetPinSense verb here, even when called from @@
-1544,20 +1560,41 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)

      bool update_eld = false;
      bool eld_changed = false;
      bool ret;

sound/pci/hda/patch_hdmi.c +3 -7

+     int device_num = 0;
+     bool need_repoll = false;
+     bool any_eld_valid = false;
 
      snd_hda_power_up_pm(codec);
      present = snd_hda_pin_sense(codec, pin_nid);
 
      mutex_lock(&per_pin->lock);
-     pin_eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE);
-     if (pin_eld->monitor_present)
-             eld->eld_valid  = !!(present & AC_PINSENSE_ELDV);
-     else
-             eld->eld_valid = false;
 
-     codec_dbg(codec,
-             "HDMI status: Codec=%d Pin=%d Presence_Detect=%d

ELD_Valid=%d\n",

sound/pci/hda/patch_hdmi.c +2 -1

-             codec->addr, pin_nid, pin_eld->monitor_present, eld->eld_valid);
+     if (codec->dp_mst)
+             per_pin->num_devices = snd_hda_get_devices(codec, pin_nid,

per_pin->dev_list,

sound/pci/hda/patch_hdmi.c +7 -0

+                                                        AC_MAX_DEV_LIST_LEN);
+next_device:
+     pin_eld = &per_pin->devices[device_num].sink_eld;
+     eld_changed = false;
+     update_eld = false;
+     if (per_pin->num_devices) {
+             pin_eld->monitor_present = !!(per_pin->dev_list[device_num] &

AC_DE_PD);

sound/pci/hda/patch_hdmi.c +2 -0

+             if (pin_eld->monitor_present)
+                     eld->eld_valid = !!(per_pin->dev_list[device_num] &

AC_DE_ELDV);

sound/pci/hda/patch_hdmi.c +4 -0

+             else
+                     eld->eld_valid = false;
+             if (eld->eld_valid)
+                     snd_hda_codec_write(codec, pin_nid, 0,

AC_VERB_SET_DEVICE_SEL, device_num);

sound/pci/hda/patch_hdmi.c +10 -0

+     } else {
+             pin_eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE);
+             if (pin_eld->monitor_present)
+                     eld->eld_valid  = !!(present & AC_PINSENSE_ELDV);
+             else
+                     eld->eld_valid = false;
+     }
+
+     codec_info(codec,
+             "HDMI status: Codec=%d Pin=%d Device=%d Presence_Detect=%d

ELD_Valid=%d\n",

sound/pci/hda/patch_hdmi.c +2 -1

+                codec->addr, pin_nid, device_num, pin_eld->monitor_present,
+eld->eld_valid);
 
      if (eld->eld_valid) {
              if (spec->ops.pin_get_eld(codec, pin_nid, eld->eld_buffer, @@
-1573,11 +1610,11 @@ static bool hdmi_present_sense(struct

hdmi_spec_per_pin *per_pin, int repoll)

              if (eld->eld_valid) {
                      snd_hdmi_show_eld(codec, &eld->info);
                      update_eld = true;

sound/pci/hda/patch_hdmi.c +4 -4

+                     any_eld_valid = true;
              }
              else if (repoll) {
-                     schedule_delayed_work(&per_pin->work,
-                                           msecs_to_jiffies(300));
-                     goto unlock;
+                     need_repoll = true;
+                     goto skip_to_next_device;
              }
      }

@@ -1614,7 +1651,7 @@ static bool hdmi_present_sense(struct
 hdmi_spec_per_pin *per_pin, int repoll)
                                                      per_pin->mux_idx);
                      }
 
-                     hdmi_setup_audio_infoframe(codec, per_pin,
+                     hdmi_setup_audio_infoframe(codec, per_pin, eld,
                                                 per_pin->non_pcm);
              }

      }

sound/pci/hda/patch_hdmi.c +13 -2

@@ -1623,8 +1660,19 @@ static bool hdmi_present_sense(struct
 hdmi_spec_per_pin *per_pin, int repoll)
              snd_ctl_notify(codec->card,
                             SNDRV_CTL_EVENT_MASK_VALUE |
 SNDRV_CTL_EVENT_MASK_INFO,
                             &per_pin->eld_ctl->id);
- unlock:
-     ret = !repoll || !pin_eld->monitor_present || pin_eld->eld_valid;
+skip_to_next_device:
+     if (codec->dp_mst) {
+             device_num++;
+             if (device_num < per_pin->num_devices)
+                     goto next_device;
+     }
+
+     if (need_repoll) {
+             schedule_delayed_work(&per_pin->work,
+                                   msecs_to_jiffies(300));
+             repoll = true;
+     }
+     ret = !repoll || any_eld_valid;
 

      jack = snd_hda_jack_tbl_get(codec, pin_nid);
      if (jack)

sound/pci/hda/patch_hdmi.c +1 -1

@@ -1807,7 +1855,7 @@ static int
 generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
      per_pin->channels = substream->runtime->channels;
      per_pin->setup = true;
 
-     hdmi_setup_audio_infoframe(codec, per_pin, non_pcm);
+     hdmi_setup_audio_infoframe(codec, per_pin, NULL, non_pcm);
      mutex_unlock(&per_pin->lock);
 

      if (spec->dyn_pin_out) {

sound/pci/hda/patch_hdmi.c +1 -1

@@ -2035,7 +2083,7 @@ static int hdmi_chmap_ctl_put(struct snd_kcontrol
 *kcontrol,
      per_pin->chmap_set = true;
      memcpy(per_pin->chmap, chmap, sizeof(chmap));
      if (prepared)
-             hdmi_setup_audio_infoframe(codec, per_pin, per_pin->non_pcm);
+             hdmi_setup_audio_infoframe(codec, per_pin, NULL,
 per_pin->non_pcm);
      mutex_unlock(&per_pin->lock);

      return 0;

sound/pci/hda/patch_hdmi.c +0 -0

@@ -2147,7 +2195,7 @@ static int generic_hdmi_build_controls(struct

hda_codec *codec) static int generic_hdmi_init_per_pins(struct hda_codec

*codec)  {
      struct hdmi_spec *spec = codec->spec;

sound/pci/hda/patch_hdmi.c +1 -2

-     int pin_idx;
+     int pin_idx, dev_idx;
 
      for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
              struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx); @@
-2155,7 +2203,20 @@ static int generic_hdmi_init_per_pins(struct hda_codec

*codec)

              per_pin->codec = codec;
              mutex_init(&per_pin->lock);
              INIT_DELAYED_WORK(&per_pin->work, hdmi_repoll_eld);

sound/pci/hda/patch_hdmi.c +7 -1

-             eld_proc_new(per_pin, pin_idx);
+
+             if (codec->dp_mst) {
+                     for (dev_idx = 0; dev_idx < HDA_MAX_DEVICES; dev_idx++) {
+                             per_pin->devices[dev_idx].device_idx = dev_idx;
+                             per_pin->devices[dev_idx].pin = per_pin;
+
+                             eld_device_proc_new(&per_pin->devices[dev_idx], pin_idx,

dev_idx);

sound/pci/hda/patch_hdmi.c +16 -2

+                     }
+             } else {
+                     per_pin->num_devices = 0;
+                     per_pin->devices[0].device_idx = 0;
+                     per_pin->devices[0].pin = per_pin;
+                     eld_device_proc_new(&per_pin->devices[0], pin_idx, -1);
+             }
      }
      return 0;
 }

@@ -2191,13 +2252,19 @@ static void hdmi_array_free(struct hdmi_spec
 *spec)  static void generic_hdmi_free(struct hda_codec *codec)  {
      struct hdmi_spec *spec = codec->spec;
-     int pin_idx;
+     int pin_idx, dev_idx;
 
      for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
              struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
 
              cancel_delayed_work_sync(&per_pin->work);
-             eld_proc_free(per_pin);
+             if (per_pin->num_devices) {
+                     for (dev_idx = 0; dev_idx < per_pin->num_devices; dev_idx++) {
+                             struct hdmi_spec_per_device *per_device =
 &per_pin->devices[dev_idx];
+                             eld_device_proc_free(per_device);
+                     }
+             } else
+                       eld_device_proc_free(&per_pin->devices[0]);
      }
 
      hdmi_array_free(spec);

@@ -2333,6 +2400,7 @@ static int patch_generic_hdmi(struct hda_codec
 *codec)
      if (is_haswell_plus(codec)) {
              intel_haswell_enable_all_pins(codec, true);
              intel_haswell_fixup_enable_dp12(codec);
+             codec->dp_mst = true;
      }
 

      if (is_haswell_plus(codec) || is_valleyview_plus(codec)) @@ -2346,7
+2414,6 @@ static int patch_generic_hdmi(struct hda_codec *codec)
      codec->patch_ops = generic_hdmi_patch_ops;
      if (is_haswell_plus(codec)) {
              codec->patch_ops.set_power_state = haswell_set_power_state;
-             codec->dp_mst = true;
      }

Quoted 8 lines

Quoted 8 lines

      generic_hdmi_init_per_pins(codec);

--
2.4.1

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

At Fri, 19 Jun 2015 20:33:39 +1000,

Quoted 33 lines · Dave Airlie

Quoted 3 lines · Mengdong

Hi Takashi/Dave,

Shall we move or cc this discussion on audio driver side to ALSA ML?

Oops I thought I had cc'ed these patches to alsa-devel as well when I sent them.

Quoted 10 lines

I think we also need to decide how to manage PCM devices for DP MST. Now the HD-A driver create a PCM device for each pin, and the substream number is 1 for each PCM. Now with DP MST enabled, each pin can support multiple streams (e.g. 3 on Intel HSW/BDW/SKL).

There may be 2 options:
-#1: Let an HDMI codec specify number of substreams, same as the number of device entries on a pin. We can specify 3 for HSW/BDW/SKL. Other vendors can also specify a value according to actual HW capabilities.

Quoted 4 lines

So for HSW, we have 3x3 subtreams totally. But we only have 3 convertors (for 3 display pipelines), so we can open up to 3 substreams at the same time. When the audio driver finds all 3 convertors are used when opening a new substream, it will fail.

One thing I noticed is the number of devices on a PIN is only updated when the MST device is plugged in so normally pins 5,6,7 have 0 devices, and when I plug in MST device, I get the 3 devices on port 6. So it seems dynamic enough at this point, though I guess it'll always be 0 or 3.

Quoted 6 lines

- #2: Create PCM device dynamically. Only create a PCM devices for a device entry which connects to monitor with audio support. When the monitor is removed, the PCM device will be disconnected, closed and removed, similar to the USB case.

Quoted 7 lines

Quoted 3 lines

This will change ALSA core. But there will be less PCM devices and substreams, since the number of connected monitors Is decided by the actual GPU display pipeline.

I like this option more, since I think it should be more like USB, but I've no idea how much work it would be from the alsa side, this patch was probably as deep into alsa as I've gone.

Two things have to be considered for compatibility:
- ELD, channel map and jack detection: these are created per PCM device, and extending to substream would confuse user space a lot. In theory, it can be extended using subdevice number, but in anyway this won't work with PulseAudio as is.

- The per-pin assignment provides a more or less persistent route to a certain device. Changing the assignment method may break the previous setup.

Also, the dynamic PCM creation / removal is an issue that has been discussed many times. Unfortunately it won't work as is, at lest for PA. Currently PA does probing of devices only at the card probe time. The hotplug of USB-audio works because it's always tied with the card. But in this case, the card remains while only the PCM devices will be created / removed, thus the probe in PA won't be triggered.

Quoted 53 lines · Takashi Iwai

At Fri, 19 Jun 2015 20:33:39 +1000,

Quoted 33 lines · Dave Airlie

Quoted 3 lines · Mengdong

Hi Takashi/Dave,

Shall we move or cc this discussion on audio driver side to ALSA ML?

Oops I thought I had cc'ed these patches to alsa-devel as well when I sent them.

Quoted 10 lines

I think we also need to decide how to manage PCM devices for DP MST. Now the HD-A driver create a PCM device for each pin, and the substream number is 1 for each PCM. Now with DP MST enabled, each pin can support multiple streams (e.g. 3 on Intel HSW/BDW/SKL).

There may be 2 options:
-#1: Let an HDMI codec specify number of substreams, same as the number of device entries on a pin. We can specify 3 for HSW/BDW/SKL. Other vendors can also specify a value according to actual HW capabilities.

Quoted 4 lines

So for HSW, we have 3x3 subtreams totally. But we only have 3 convertors (for 3 display pipelines), so we can open up to 3 substreams at the same time. When the audio driver finds all 3 convertors are used when opening a new substream, it will fail.

One thing I noticed is the number of devices on a PIN is only updated when the MST device is plugged in so normally pins 5,6,7 have 0 devices, and when I plug in MST device, I get the 3 devices on port 6. So it seems dynamic enough at this point, though I guess it'll always be 0 or 3.

Quoted 6 lines

- #2: Create PCM device dynamically. Only create a PCM devices for a device entry which connects to monitor with audio support. When the monitor is removed, the PCM device will be disconnected, closed and removed, similar to the USB case.

Quoted 7 lines

Quoted 3 lines

This will change ALSA core. But there will be less PCM devices and substreams, since the number of connected monitors Is decided by the actual GPU display pipeline.

I like this option more, since I think it should be more like USB, but I've no idea how much work it would be from the alsa side, this patch was probably as deep into alsa as I've gone.

Two things have to be considered for compatibility:
- ELD, channel map and jack detection: these are created per PCM device, and extending to substream would confuse user space a lot. In theory, it can be extended using subdevice number, but in anyway this won't work with PulseAudio as is.

- The per-pin assignment provides a more or less persistent route to a certain device. Changing the assignment method may break the previous setup.

Quoted 6 lines

Also, the dynamic PCM creation / removal is an issue that has been discussed many times. Unfortunately it won't work as is, at lest for PA. Currently PA does probing of devices only at the card probe time. The hotplug of USB-audio works because it's always tied with the card. But in this case, the card remains while only the PCM devices will be created / removed, thus the probe in PA won't be triggered.

I guess that means we either have to hotplug entire (fake) cards or fix up userspace to support mst audio properly?

We had to do some minimal changes
to the ddx drivers too to make sure they're rescanning the connector list properly. Imo since this is all new I think we could require PA to rescan the PCM dev list on hotplug events too to support DP MST. And we kinda do need hotplug at that level since if we'd hotplug the entire card we'd kill a stream that's running on some other display.

And always registering all of them feels like a very bad hack too.

At Mon, 22 Jun 2015 14:54:29 +0200,

Quoted 62 lines · Daniel Vetter

Quoted 53 lines · Takashi Iwai

At Fri, 19 Jun 2015 20:33:39 +1000,

Quoted 33 lines · Dave Airlie

Quoted 3 lines · Mengdong

Hi Takashi/Dave,

Shall we move or cc this discussion on audio driver side to ALSA ML?

Oops I thought I had cc'ed these patches to alsa-devel as well when I sent them.

Quoted 10 lines

I think we also need to decide how to manage PCM devices for DP MST. Now the HD-A driver create a PCM device for each pin, and the substream number is 1 for each PCM. Now with DP MST enabled, each pin can support multiple streams (e.g. 3 on Intel HSW/BDW/SKL).

There may be 2 options:
-#1: Let an HDMI codec specify number of substreams, same as the number of device entries on a pin. We can specify 3 for HSW/BDW/SKL. Other vendors can also specify a value according to actual HW capabilities.

Quoted 4 lines

So for HSW, we have 3x3 subtreams totally. But we only have 3 convertors (for 3 display pipelines), so we can open up to 3 substreams at the same time. When the audio driver finds all 3 convertors are used when opening a new substream, it will fail.

One thing I noticed is the number of devices on a PIN is only updated when the MST device is plugged in so normally pins 5,6,7 have 0 devices, and when I plug in MST device, I get the 3 devices on port 6. So it seems dynamic enough at this point, though I guess it'll always be 0 or 3.

Quoted 6 lines

- #2: Create PCM device dynamically. Only create a PCM devices for a device entry which connects to monitor with audio support. When the monitor is removed, the PCM device will be disconnected, closed and removed, similar to the USB case.

Quoted 7 lines

Quoted 3 lines

This will change ALSA core. But there will be less PCM devices and substreams, since the number of connected monitors Is decided by the actual GPU display pipeline.

I like this option more, since I think it should be more like USB, but I've no idea how much work it would be from the alsa side, this patch was probably as deep into alsa as I've gone.

Two things have to be considered for compatibility:
- ELD, channel map and jack detection: these are created per PCM device, and extending to substream would confuse user space a lot. In theory, it can be extended using subdevice number, but in anyway this won't work with PulseAudio as is.

- The per-pin assignment provides a more or less persistent route to a certain device. Changing the assignment method may break the previous setup.

Quoted 6 lines

Also, the dynamic PCM creation / removal is an issue that has been discussed many times. Unfortunately it won't work as is, at lest for PA. Currently PA does probing of devices only at the card probe time. The hotplug of USB-audio works because it's always tied with the card. But in this case, the card remains while only the PCM devices will be created / removed, thus the probe in PA won't be triggered.

Quoted 2 lines

I guess that means we either have to hotplug entire (fake) cards or fix up userspace to support mst audio properly?

It would work for HSW/BDW. But BYT/BSW and SKL share the same controller for both HDMI and analog codecs, thus the card can't be handled as hotplugged.

Quoted 8 lines

We had to do some minimal changes
to the ddx drivers too to make sure they're rescanning the connector list properly. Imo since this is all new I think we could require PA to rescan the PCM dev list on hotplug events too to support DP MST. And we kinda do need hotplug at that level since if we'd hotplug the entire card we'd kill a stream that's running on some other display.

And always registering all of them feels like a very bad hack too.

Yeah, I personally am for the PCM hotplug, too. It's a cleaner way.

(The current static assignment comes from the chips where they had no
ELD communication -- the hardware before the recent onboard Intel and AMD gfx but connected somehow externally. For such hardware, we still need the static assignment.)

OTOH, we have to keep some compatibility. And moving to the hotplug would break certainly some existing configuration that assumes the static port / device assignment.

So, a compromise would be:
- change the behavior via either Kconfig or module option.
- create many PCM devices statically as much as possible

The latter can be a problem in the case of AMD or Nvidia where 8 (or more?) ports may exist. The former is, of course, messy and confusing for users, too.

Quoted 91 lines · Takashi Iwai

At Mon, 22 Jun 2015 14:54:29 +0200,

Quoted 62 lines · Daniel Vetter

Quoted 53 lines · Takashi Iwai

At Fri, 19 Jun 2015 20:33:39 +1000,

Quoted 33 lines · Dave Airlie

Quoted 3 lines · Mengdong

Hi Takashi/Dave,

Shall we move or cc this discussion on audio driver side to ALSA ML?

Oops I thought I had cc'ed these patches to alsa-devel as well when I sent them.

Quoted 10 lines

I think we also need to decide how to manage PCM devices for DP MST. Now the HD-A driver create a PCM device for each pin, and the substream number is 1 for each PCM. Now with DP MST enabled, each pin can support multiple streams (e.g. 3 on Intel HSW/BDW/SKL).

There may be 2 options:
-#1: Let an HDMI codec specify number of substreams, same as the number of device entries on a pin. We can specify 3 for HSW/BDW/SKL. Other vendors can also specify a value according to actual HW capabilities.

Quoted 4 lines

So for HSW, we have 3x3 subtreams totally. But we only have 3 convertors (for 3 display pipelines), so we can open up to 3 substreams at the same time. When the audio driver finds all 3 convertors are used when opening a new substream, it will fail.

One thing I noticed is the number of devices on a PIN is only updated when the MST device is plugged in so normally pins 5,6,7 have 0 devices, and when I plug in MST device, I get the 3 devices on port 6. So it seems dynamic enough at this point, though I guess it'll always be 0 or 3.

Quoted 6 lines

- #2: Create PCM device dynamically. Only create a PCM devices for a device entry which connects to monitor with audio support. When the monitor is removed, the PCM device will be disconnected, closed and removed, similar to the USB case.

Quoted 7 lines

Quoted 3 lines

This will change ALSA core. But there will be less PCM devices and substreams, since the number of connected monitors Is decided by the actual GPU display pipeline.

I like this option more, since I think it should be more like USB, but I've no idea how much work it would be from the alsa side, this patch was probably as deep into alsa as I've gone.

Two things have to be considered for compatibility:
- ELD, channel map and jack detection: these are created per PCM device, and extending to substream would confuse user space a lot. In theory, it can be extended using subdevice number, but in anyway this won't work with PulseAudio as is.

- The per-pin assignment provides a more or less persistent route to a certain device. Changing the assignment method may break the previous setup.

Quoted 6 lines

Also, the dynamic PCM creation / removal is an issue that has been discussed many times. Unfortunately it won't work as is, at lest for PA. Currently PA does probing of devices only at the card probe time. The hotplug of USB-audio works because it's always tied with the card. But in this case, the card remains while only the PCM devices will be created / removed, thus the probe in PA won't be triggered.

Quoted 2 lines

I guess that means we either have to hotplug entire (fake) cards or fix up userspace to support mst audio properly?

It would work for HSW/BDW. But BYT/BSW and SKL share the same controller for both HDMI and analog codecs, thus the card can't be handled as hotplugged.

Quoted 8 lines

We had to do some minimal changes
to the ddx drivers too to make sure they're rescanning the connector list properly. Imo since this is all new I think we could require PA to rescan the PCM dev list on hotplug events too to support DP MST. And we kinda do need hotplug at that level since if we'd hotplug the entire card we'd kill a stream that's running on some other display.

And always registering all of them feels like a very bad hack too.

Yeah, I personally am for the PCM hotplug, too. It's a cleaner way.

(The current static assignment comes from the chips where they had no
ELD communication -- the hardware before the recent onboard Intel and AMD gfx but connected somehow externally. For such hardware, we still need the static assignment.)

OTOH, we have to keep some compatibility. And moving to the hotplug would break certainly some existing configuration that assumes the static port / device assignment.

Quoted 7 lines

So, a compromise would be:
- change the behavior via either Kconfig or module option.
- create many PCM devices statically as much as possible

The latter can be a problem in the case of AMD or Nvidia where 8 (or more?) ports may exist. The former is, of course, messy and confusing for users, too.

Fwiw, Mengdong has some patches that are work in progress for the dynamic PCM creation/removal as part of the topology work. The topology has to support DSP FW that can load/unload different services that may also include a dynamically registered PCM/compressed PCM device.

Tanu, what's your take on the effort for dynamically created PCMs support for pulseaudio ?

Btw, the topology core now also dynamically creates/removes mixer controls, can PA handle this atm ?

Quoted 100 lines · Liam Girdwood

Quoted 91 lines · Takashi Iwai

At Mon, 22 Jun 2015 14:54:29 +0200,

Quoted 62 lines · Daniel Vetter

Quoted 53 lines · Takashi Iwai

At Fri, 19 Jun 2015 20:33:39 +1000,

Quoted 33 lines · Dave Airlie

Quoted 3 lines · Mengdong

Hi Takashi/Dave,

Shall we move or cc this discussion on audio driver side to ALSA ML?

Oops I thought I had cc'ed these patches to alsa-devel as well when I sent them.

Quoted 10 lines

I think we also need to decide how to manage PCM devices for DP MST. Now the HD-A driver create a PCM device for each pin, and the substream number is 1 for each PCM. Now with DP MST enabled, each pin can support multiple streams (e.g. 3 on Intel HSW/BDW/SKL).

There may be 2 options:
-#1: Let an HDMI codec specify number of substreams, same as the number of device entries on a pin. We can specify 3 for HSW/BDW/SKL. Other vendors can also specify a value according to actual HW capabilities.

Quoted 4 lines

So for HSW, we have 3x3 subtreams totally. But we only have 3 convertors (for 3 display pipelines), so we can open up to 3 substreams at the same time. When the audio driver finds all 3 convertors are used when opening a new substream, it will fail.

One thing I noticed is the number of devices on a PIN is only updated when the MST device is plugged in so normally pins 5,6,7 have 0 devices, and when I plug in MST device, I get the 3 devices on port 6. So it seems dynamic enough at this point, though I guess it'll always be 0 or 3.

Quoted 6 lines

- #2: Create PCM device dynamically. Only create a PCM devices for a device entry which connects to monitor with audio support. When the monitor is removed, the PCM device will be disconnected, closed and removed, similar to the USB case.

Quoted 7 lines

Quoted 3 lines

This will change ALSA core. But there will be less PCM devices and substreams, since the number of connected monitors Is decided by the actual GPU display pipeline.

I like this option more, since I think it should be more like USB, but I've no idea how much work it would be from the alsa side, this patch was probably as deep into alsa as I've gone.

Two things have to be considered for compatibility:
- ELD, channel map and jack detection: these are created per PCM device, and extending to substream would confuse user space a lot. In theory, it can be extended using subdevice number, but in anyway this won't work with PulseAudio as is.

- The per-pin assignment provides a more or less persistent route to a certain device. Changing the assignment method may break the previous setup.

Quoted 6 lines

Also, the dynamic PCM creation / removal is an issue that has been discussed many times. Unfortunately it won't work as is, at lest for PA. Currently PA does probing of devices only at the card probe time. The hotplug of USB-audio works because it's always tied with the card. But in this case, the card remains while only the PCM devices will be created / removed, thus the probe in PA won't be triggered.

Quoted 2 lines

I guess that means we either have to hotplug entire (fake) cards or fix up userspace to support mst audio properly?

It would work for HSW/BDW. But BYT/BSW and SKL share the same controller for both HDMI and analog codecs, thus the card can't be handled as hotplugged.

Quoted 8 lines

We had to do some minimal changes
to the ddx drivers too to make sure they're rescanning the connector list properly. Imo since this is all new I think we could require PA to rescan the PCM dev list on hotplug events too to support DP MST. And we kinda do need hotplug at that level since if we'd hotplug the entire card we'd kill a stream that's running on some other display.

And always registering all of them feels like a very bad hack too.

Yeah, I personally am for the PCM hotplug, too. It's a cleaner way.

(The current static assignment comes from the chips where they had no
ELD communication -- the hardware before the recent onboard Intel and AMD gfx but connected somehow externally. For such hardware, we still need the static assignment.)

OTOH, we have to keep some compatibility. And moving to the hotplug would break certainly some existing configuration that assumes the static port / device assignment.

Quoted 7 lines

So, a compromise would be:
- change the behavior via either Kconfig or module option.
- create many PCM devices statically as much as possible

The latter can be a problem in the case of AMD or Nvidia where 8 (or more?) ports may exist. The former is, of course, messy and confusing for users, too.

Quoted 7 lines

Fwiw, Mengdong has some patches that are work in progress for the dynamic PCM creation/removal as part of the topology work. The topology has to support DSP FW that can load/unload different services that may also include a dynamically registered PCM/compressed PCM device.

Tanu, what's your take on the effort for dynamically created PCMs support for pulseaudio ?

It's a significant amount of work, but I think PulseAudio should be improved to support this in any case, if other approaches make life miserable for driver developers.

What would be the interface for getting notifications about new and removed PCM devices? udev?

PulseAudio (mostly) doesn't use the hw:X devices directly. Instead, it uses logical names like "front", "hdmi", "iec958", etc. Speaking of HDMI specifically, PulseAudio uses devices from "hdmi:X,0" to "hdmi:X,7". With this new dynamic PCM system, do these logical names still work?

When probing a new card, PulseAudio goes through the configured set of profiles for that card, and tries to open each configured device separately, and also each configured combination of devices (one profile may consist of multiple devices). This kind of probing gets messy, if devices appear dynamically when the card is already in use, because reliable results require other devices of the card to be closed before trying to open the new device. To avoid glitches in playing audio, the probing would have to be delayed until the card is idle... I suspect that isn't really feasible, so maybe we will just have to live with glitches.

It would be nice if PulseAudio didn't have to open the PCM devices when probing, but that would require some interface to find out which PCM devices can be opened simultaneously and which can not.

Another problem is that PulseAudio doesn't know which hw PCM devices are used by the logical devices. Without that information, PulseAudio has to re-probe every device that failed previously, when new hw PCM devices appear, and also re-probe every device that succeeded previously, when PCM devices are removed. I suspect that this information might actually be available, since "aplay -v -Dfront" shows the underlying hw device. Maybe PulseAudio should be using that information?

Quoted 2 lines

Btw, the topology core now also dynamically creates/removes mixer controls, can PA handle this atm ?

No, PA checks the mixer controls only when loading a new card. Dynamically added controls are ignored. Dynamically removed controls just cause silent failure, at least when setting volume (I didn't check other use cases). That is, changing the volume appears to succeed, but nothing actually happens.

At Mon, 22 Jun 2015 15:21:16 +0000,

Quoted 110 lines · Tanu

Quoted 100 lines · Liam Girdwood

Quoted 91 lines · Takashi Iwai

At Mon, 22 Jun 2015 14:54:29 +0200,

Quoted 62 lines · Daniel Vetter

Quoted 53 lines · Takashi Iwai

At Fri, 19 Jun 2015 20:33:39 +1000,

Quoted 33 lines · Dave Airlie

Quoted 3 lines · Mengdong

Hi Takashi/Dave,

Shall we move or cc this discussion on audio driver side to ALSA ML?

Oops I thought I had cc'ed these patches to alsa-devel as well when I sent them.

Quoted 10 lines

I think we also need to decide how to manage PCM devices for DP MST. Now the HD-A driver create a PCM device for each pin, and the substream number is 1 for each PCM. Now with DP MST enabled, each pin can support multiple streams (e.g. 3 on Intel HSW/BDW/SKL).

There may be 2 options:
-#1: Let an HDMI codec specify number of substreams, same as the number of device entries on a pin. We can specify 3 for HSW/BDW/SKL. Other vendors can also specify a value according to actual HW capabilities.

Quoted 4 lines

So for HSW, we have 3x3 subtreams totally. But we only have 3 convertors (for 3 display pipelines), so we can open up to 3 substreams at the same time. When the audio driver finds all 3 convertors are used when opening a new substream, it will fail.

One thing I noticed is the number of devices on a PIN is only updated when the MST device is plugged in so normally pins 5,6,7 have 0 devices, and when I plug in MST device, I get the 3 devices on port 6. So it seems dynamic enough at this point, though I guess it'll always be 0 or 3.

Quoted 6 lines

- #2: Create PCM device dynamically. Only create a PCM devices for a device entry which connects to monitor with audio support. When the monitor is removed, the PCM device will be disconnected, closed and removed, similar to the USB case.

Quoted 7 lines

Quoted 3 lines

This will change ALSA core. But there will be less PCM devices and substreams, since the number of connected monitors Is decided by the actual GPU display pipeline.

I like this option more, since I think it should be more like USB, but I've no idea how much work it would be from the alsa side, this patch was probably as deep into alsa as I've gone.

Two things have to be considered for compatibility:
- ELD, channel map and jack detection: these are created per PCM device, and extending to substream would confuse user space a lot. In theory, it can be extended using subdevice number, but in anyway this won't work with PulseAudio as is.

- The per-pin assignment provides a more or less persistent route to a certain device. Changing the assignment method may break the previous setup.

Quoted 6 lines

Also, the dynamic PCM creation / removal is an issue that has been discussed many times. Unfortunately it won't work as is, at lest for PA. Currently PA does probing of devices only at the card probe time. The hotplug of USB-audio works because it's always tied with the card. But in this case, the card remains while only the PCM devices will be created / removed, thus the probe in PA won't be triggered.

Quoted 2 lines

I guess that means we either have to hotplug entire (fake) cards or fix up userspace to support mst audio properly?

It would work for HSW/BDW. But BYT/BSW and SKL share the same controller for both HDMI and analog codecs, thus the card can't be handled as hotplugged.

Quoted 8 lines

We had to do some minimal changes
to the ddx drivers too to make sure they're rescanning the connector list properly. Imo since this is all new I think we could require PA to rescan the PCM dev list on hotplug events too to support DP MST. And we kinda do need hotplug at that level since if we'd hotplug the entire card we'd kill a stream that's running on some other display.

And always registering all of them feels like a very bad hack too.

Yeah, I personally am for the PCM hotplug, too. It's a cleaner way.

(The current static assignment comes from the chips where they had no
ELD communication -- the hardware before the recent onboard Intel and AMD gfx but connected somehow externally. For such hardware, we still need the static assignment.)

OTOH, we have to keep some compatibility. And moving to the hotplug would break certainly some existing configuration that assumes the static port / device assignment.

Quoted 7 lines

So, a compromise would be:
- change the behavior via either Kconfig or module option.
- create many PCM devices statically as much as possible

The latter can be a problem in the case of AMD or Nvidia where 8 (or more?) ports may exist. The former is, of course, messy and confusing for users, too.

Quoted 7 lines

Fwiw, Mengdong has some patches that are work in progress for the dynamic PCM creation/removal as part of the topology work. The topology has to support DSP FW that can load/unload different services that may also include a dynamically registered PCM/compressed PCM device.

Tanu, what's your take on the effort for dynamically created PCMs support for pulseaudio ?

Quoted 6 lines

It's a significant amount of work, but I think PulseAudio should be improved to support this in any case, if other approaches make life miserable for driver developers.

What would be the interface for getting notifications about new and removed PCM devices? udev?

In general, yes.

Quoted 4 lines

PulseAudio (mostly) doesn't use the hw:X devices directly. Instead, it uses logical names like "front", "hdmi", "iec958", etc. Speaking of HDMI specifically, PulseAudio uses devices from "hdmi:X,0" to "hdmi:X,7". With this new dynamic PCM system, do these logical names still work?

Unfortunately, this doesn't work for HDA as is, because of the terribly arcane secret. For keeping the compatibility with the old config, there is a static mapping of the hdmi:x,y and hw:x,z.

Maybe we should introduce a new device class for dynamic HDMI/DP device, something like dhdmi:x,y, to make things straightforward. (Just a concept -- I'm not good at naming.)

Alternatively, we may introduce a new argument to hdmi PCM to access like "hdmi:CARD=x,SYSDEV=y".

Quoted 11 lines

When probing a new card, PulseAudio goes through the configured set of profiles for that card, and tries to open each configured device separately, and also each configured combination of devices (one profile may consist of multiple devices). This kind of probing gets messy, if devices appear dynamically when the card is already in use, because reliable results require other devices of the card to be closed before trying to open the new device. To avoid glitches in playing audio, the probing would have to be delayed until the card is idle... I suspect that isn't really feasible, so maybe we will just have to live with glitches.

Quoted 3 lines

It would be nice if PulseAudio didn't have to open the PCM devices when probing, but that would require some interface to find out which PCM devices can be opened simultaneously and which can not.

Yeah, that's the missing information. We can give something in alsa-lib config to indicate the conflict, in theory...

Quoted 7 lines

Another problem is that PulseAudio doesn't know which hw PCM devices are used by the logical devices. Without that information, PulseAudio has to re-probe every device that failed previously, when new hw PCM devices appear, and also re-probe every device that succeeded previously, when PCM devices are removed. I suspect that this information might actually be available, since "aplay -v -Dfront" shows the underlying hw device. Maybe PulseAudio should be using that information?

aplay just uses an API function to dump the setup, and its text is not supposed to be parsed as an interface. We need a proper API function to provide that missing piece like above.

Quoted 8 lines

Quoted 2 lines

Btw, the topology core now also dynamically creates/removes mixer controls, can PA handle this atm ?

No, PA checks the mixer controls only when loading a new card. Dynamically added controls are ignored. Dynamically removed controls just cause silent failure, at least when setting volume (I didn't check other use cases). That is, changing the volume appears to succeed, but nothing actually happens.

Won't PA use ELD or other information? The corresponding controls to HDMI/DP will be created / deleted dynamically together with a PCM device, I suppose.

(Added pulseaudio-discuss to CC.)

Quoted 130 lines · Takashi Iwai

At Mon, 22 Jun 2015 15:21:16 +0000,

Quoted 110 lines · Tanu

Quoted 100 lines · Liam Girdwood

Quoted 91 lines · Takashi Iwai

At Mon, 22 Jun 2015 14:54:29 +0200,

Quoted 62 lines · Daniel Vetter

Quoted 53 lines · Takashi Iwai

At Fri, 19 Jun 2015 20:33:39 +1000,

Quoted 33 lines · Dave Airlie

Quoted 3 lines · Mengdong

Hi Takashi/Dave,

Shall we move or cc this discussion on audio driver side to ALSA ML?

Oops I thought I had cc'ed these patches to alsa-devel as well when I sent them.

Quoted 10 lines

I think we also need to decide how to manage PCM devices for DP MST. Now the HD-A driver create a PCM device for each pin, and the substream number is 1 for each PCM. Now with DP MST enabled, each pin can support multiple streams (e.g. 3 on Intel HSW/BDW/SKL).

There may be 2 options:
-#1: Let an HDMI codec specify number of substreams, same as the number of device entries on a pin. We can specify 3 for HSW/BDW/SKL. Other vendors can also specify a value according to actual HW capabilities.

Quoted 4 lines

So for HSW, we have 3x3 subtreams totally. But we only have 3 convertors (for 3 display pipelines), so we can open up to 3 substreams at the same time. When the audio driver finds all 3 convertors are used when opening a new substream, it will fail.

One thing I noticed is the number of devices on a PIN is only updated when the MST device is plugged in so normally pins 5,6,7 have 0 devices, and when I plug in MST device, I get the 3 devices on port 6. So it seems dynamic enough at this point, though I guess it'll always be 0 or 3.

Quoted 6 lines

- #2: Create PCM device dynamically. Only create a PCM devices for a device entry which connects to monitor with audio support. When the monitor is removed, the PCM device will be disconnected, closed and removed, similar to the USB case.

Quoted 7 lines

Quoted 3 lines

This will change ALSA core. But there will be less PCM devices and substreams, since the number of connected monitors Is decided by the actual GPU display pipeline.

I like this option more, since I think it should be more like USB, but I've no idea how much work it would be from the alsa side, this patch was probably as deep into alsa as I've gone.

Two things have to be considered for compatibility:
- ELD, channel map and jack detection: these are created per PCM device, and extending to substream would confuse user space a lot. In theory, it can be extended using subdevice number, but in anyway this won't work with PulseAudio as is.

- The per-pin assignment provides a more or less persistent route to a certain device. Changing the assignment method may break the previous setup.

Quoted 6 lines

Also, the dynamic PCM creation / removal is an issue that has been discussed many times. Unfortunately it won't work as is, at lest for PA. Currently PA does probing of devices only at the card probe time. The hotplug of USB-audio works because it's always tied with the card. But in this case, the card remains while only the PCM devices will be created / removed, thus the probe in PA won't be triggered.

Quoted 2 lines

I guess that means we either have to hotplug entire (fake) cards or fix up userspace to support mst audio properly?

It would work for HSW/BDW. But BYT/BSW and SKL share the same controller for both HDMI and analog codecs, thus the card can't be handled as hotplugged.

Quoted 8 lines

We had to do some minimal changes
to the ddx drivers too to make sure they're rescanning the connector list properly. Imo since this is all new I think we could require PA to rescan the PCM dev list on hotplug events too to support DP MST. And we kinda do need hotplug at that level since if we'd hotplug the entire card we'd kill a stream that's running on some other display.

And always registering all of them feels like a very bad hack too.

Yeah, I personally am for the PCM hotplug, too. It's a cleaner way.

(The current static assignment comes from the chips where they had no
ELD communication -- the hardware before the recent onboard Intel and AMD gfx but connected somehow externally. For such hardware, we still need the static assignment.)

OTOH, we have to keep some compatibility. And moving to the hotplug would break certainly some existing configuration that assumes the static port / device assignment.

Quoted 7 lines

So, a compromise would be:
- change the behavior via either Kconfig or module option.
- create many PCM devices statically as much as possible

The latter can be a problem in the case of AMD or Nvidia where 8 (or more?) ports may exist. The former is, of course, messy and confusing for users, too.

Quoted 7 lines

Fwiw, Mengdong has some patches that are work in progress for the dynamic PCM creation/removal as part of the topology work. The topology has to support DSP FW that can load/unload different services that may also include a dynamically registered PCM/compressed PCM device.

Tanu, what's your take on the effort for dynamically created PCMs support for pulseaudio ?

Quoted 6 lines

It's a significant amount of work, but I think PulseAudio should be improved to support this in any case, if other approaches make life miserable for driver developers.

What would be the interface for getting notifications about new and removed PCM devices? udev?

In general, yes.

Quoted 4 lines

PulseAudio (mostly) doesn't use the hw:X devices directly. Instead, it uses logical names like "front", "hdmi", "iec958", etc. Speaking of HDMI specifically, PulseAudio uses devices from "hdmi:X,0" to "hdmi:X,7". With this new dynamic PCM system, do these logical names still work?

Unfortunately, this doesn't work for HDA as is, because of the terribly arcane secret. For keeping the compatibility with the old config, there is a static mapping of the hdmi:x,y and hw:x,z.

Quoted 6 lines

Maybe we should introduce a new device class for dynamic HDMI/DP device, something like dhdmi:x,y, to make things straightforward. (Just a concept -- I'm not good at naming.)

Alternatively, we may introduce a new argument to hdmi PCM to access like "hdmi:CARD=x,SYSDEV=y".

What happens if PulseAudio tries to open "dhdmi:x,y" or "hdmi:CARD=x,SYSDEV=y", when y points to a non-HDMI device? If it fails, then PulseAudio can replace its current "hdmi:x,[0-7]" devices with "hdmi:CARD=X,SYSDEV=[0-13]" and blindly try every hw PCM device. But if opening a non-HDMI device through "hdmi:CARD=x,SYSDEV=y" succeeds, then how is PulseAudio supposed to know which hw PCM devices are HDMI devices?

At Tue, 23 Jun 2015 07:51:22 +0000,

Quoted 8 lines · Tanu

(Added pulseaudio-discuss to CC.)

Quoted 4 lines · Takashi Iwai

At Mon, 22 Jun 2015 15:21:16 +0000,

Quoted 2 lines · Tanu

On Mon, 2015-06-22 at 14:29 +0100, Liam Girdwood wrote:

Quoted 28 lines

Quoted 21 lines

Quoted 9 lines

Quoted 2 lines

Tanu, what's your take on the effort for dynamically created PCMs support for pulseaudio ?

It's a significant amount of work, but I think PulseAudio should be improved to support this in any case, if other approaches make life miserable for driver developers.

What would be the interface for getting notifications about new and removed PCM devices? udev?

In general, yes.

Quoted 4 lines

PulseAudio (mostly) doesn't use the hw:X devices directly. Instead, it uses logical names like "front", "hdmi", "iec958", etc. Speaking of HDMI specifically, PulseAudio uses devices from "hdmi:X,0" to "hdmi:X,7". With this new dynamic PCM system, do these logical names still work?

Unfortunately, this doesn't work for HDA as is, because of the terribly arcane secret. For keeping the compatibility with the old config, there is a static mapping of the hdmi:x,y and hw:x,z.

Quoted 6 lines

Maybe we should introduce a new device class for dynamic HDMI/DP device, something like dhdmi:x,y, to make things straightforward. (Just a concept -- I'm not good at naming.)

Alternatively, we may introduce a new argument to hdmi PCM to access like "hdmi:CARD=x,SYSDEV=y".

Quoted 7 lines

What happens if PulseAudio tries to open "dhdmi:x,y" or "hdmi:CARD=x,SYSDEV=y", when y points to a non-HDMI device? If it fails, then PulseAudio can replace its current "hdmi:x,[0-7]" devices with "hdmi:CARD=X,SYSDEV=[0-13]" and blindly try every hw PCM device. But if opening a non-HDMI device through "hdmi:CARD=x,SYSDEV=y" succeeds, then how is PulseAudio supposed to know which hw PCM devices are HDMI devices?

It's a good question. I think this is the core part of the missing pieces.

What I have in my mind is to extend SNDRV_PCM_CLASS_* definition for dedicated to HDMI/DP, e.g. SNDRV_PCM_CLASS_HDMI. The difference between DP and HDMI can be specified in subclass optionally.

Quoted 10 lines · Takashi Iwai

At Tue, 23 Jun 2015 07:51:22 +0000,

Quoted 8 lines · Tanu

(Added pulseaudio-discuss to CC.)

Quoted 4 lines · Takashi Iwai

At Mon, 22 Jun 2015 15:21:16 +0000,

Quoted 2 lines · Tanu

On Mon, 2015-06-22 at 14:29 +0100, Liam Girdwood wrote:

<snip>

Quoted 36 lines

Quoted 28 lines

Quoted 21 lines

Quoted 9 lines

Quoted 2 lines

Tanu, what's your take on the effort for dynamically created PCMs support for pulseaudio ?

It's a significant amount of work, but I think PulseAudio should be improved to support this in any case, if other approaches make life miserable for driver developers.

What would be the interface for getting notifications about new and removed PCM devices? udev?

In general, yes.

Quoted 4 lines

PulseAudio (mostly) doesn't use the hw:X devices directly. Instead, it uses logical names like "front", "hdmi", "iec958", etc. Speaking of HDMI specifically, PulseAudio uses devices from "hdmi:X,0" to "hdmi:X,7". With this new dynamic PCM system, do these logical names still work?

Unfortunately, this doesn't work for HDA as is, because of the terribly arcane secret. For keeping the compatibility with the old config, there is a static mapping of the hdmi:x,y and hw:x,z.

Quoted 6 lines

Maybe we should introduce a new device class for dynamic HDMI/DP device, something like dhdmi:x,y, to make things straightforward. (Just a concept -- I'm not good at naming.)

Alternatively, we may introduce a new argument to hdmi PCM to access like "hdmi:CARD=x,SYSDEV=y".

Quoted 7 lines

What happens if PulseAudio tries to open "dhdmi:x,y" or "hdmi:CARD=x,SYSDEV=y", when y points to a non-HDMI device? If it fails, then PulseAudio can replace its current "hdmi:x,[0-7]" devices with "hdmi:CARD=X,SYSDEV=[0-13]" and blindly try every hw PCM device. But if opening a non-HDMI device through "hdmi:CARD=x,SYSDEV=y" succeeds, then how is PulseAudio supposed to know which hw PCM devices are HDMI devices?

Quoted 6 lines

It's a good question. I think this is the core part of the missing pieces.

What I have in my mind is to extend SNDRV_PCM_CLASS_* definition for dedicated to HDMI/DP, e.g. SNDRV_PCM_CLASS_HDMI. The difference between DP and HDMI can be specified in subclass optionally.

Sounds good. Knowing the difference between HDMI, DisplayPort and Thunderbolt would be very nice too. It would enable better labelling in UIs.

I have one minor concern, though: what if a PCM device isn't statically assigned to one output connector? Is it possible that some DSP is able to dynamically change the PCM routing between different outputs? In those cases the PCM class would be useless information (unless there are notifications when the class changes). I don't have any better ideas, though.

Quoted 61 lines · Tanu

Quoted 10 lines · Takashi Iwai

At Tue, 23 Jun 2015 07:51:22 +0000,

Quoted 8 lines · Tanu

(Added pulseaudio-discuss to CC.)

Quoted 4 lines · Takashi Iwai

At Mon, 22 Jun 2015 15:21:16 +0000,

Quoted 2 lines · Tanu

On Mon, 2015-06-22 at 14:29 +0100, Liam Girdwood wrote:

<snip>

Quoted 36 lines

Quoted 28 lines

Quoted 21 lines

Quoted 9 lines

Quoted 2 lines

Tanu, what's your take on the effort for dynamically created PCMs support for pulseaudio ?

It's a significant amount of work, but I think PulseAudio should be improved to support this in any case, if other approaches make life miserable for driver developers.

What would be the interface for getting notifications about new and removed PCM devices? udev?

In general, yes.

Quoted 4 lines

PulseAudio (mostly) doesn't use the hw:X devices directly. Instead, it uses logical names like "front", "hdmi", "iec958", etc. Speaking of HDMI specifically, PulseAudio uses devices from "hdmi:X,0" to "hdmi:X,7". With this new dynamic PCM system, do these logical names still work?

Unfortunately, this doesn't work for HDA as is, because of the terribly arcane secret. For keeping the compatibility with the old config, there is a static mapping of the hdmi:x,y and hw:x,z.

Quoted 6 lines

Maybe we should introduce a new device class for dynamic HDMI/DP device, something like dhdmi:x,y, to make things straightforward. (Just a concept -- I'm not good at naming.)

Alternatively, we may introduce a new argument to hdmi PCM to access like "hdmi:CARD=x,SYSDEV=y".

Quoted 7 lines

What happens if PulseAudio tries to open "dhdmi:x,y" or "hdmi:CARD=x,SYSDEV=y", when y points to a non-HDMI device? If it fails, then PulseAudio can replace its current "hdmi:x,[0-7]" devices with "hdmi:CARD=X,SYSDEV=[0-13]" and blindly try every hw PCM device. But if opening a non-HDMI device through "hdmi:CARD=x,SYSDEV=y" succeeds, then how is PulseAudio supposed to know which hw PCM devices are HDMI devices?

Quoted 6 lines

It's a good question. I think this is the core part of the missing pieces.

What I have in my mind is to extend SNDRV_PCM_CLASS_* definition for dedicated to HDMI/DP, e.g. SNDRV_PCM_CLASS_HDMI. The difference between DP and HDMI can be specified in subclass optionally.

Sounds good. Knowing the difference between HDMI, DisplayPort and Thunderbolt would be very nice too. It would enable better labelling in UIs.

Quoted 7 lines

I have one minor concern, though: what if a PCM device isn't statically assigned to one output connector? Is it possible that some DSP is able to dynamically change the PCM routing between different outputs? In those cases the PCM class would be useless information (unless there are notifications when the class changes). I don't have any better ideas, though.

This is possible atm, we can route from one PCM to multiple outputs depending on DSP and codec mixer settings. The outputs can even be on different physical interfaces.

Quoted 70 lines · Liam Girdwood

Quoted 61 lines · Tanu

Quoted 10 lines · Takashi Iwai

At Tue, 23 Jun 2015 07:51:22 +0000,

Quoted 8 lines · Tanu

(Added pulseaudio-discuss to CC.)

Quoted 4 lines · Takashi Iwai

At Mon, 22 Jun 2015 15:21:16 +0000,

Quoted 2 lines · Tanu

On Mon, 2015-06-22 at 14:29 +0100, Liam Girdwood wrote:

<snip>

Quoted 36 lines

Quoted 28 lines

Quoted 21 lines

Quoted 9 lines

Quoted 2 lines

Tanu, what's your take on the effort for dynamically created PCMs support for pulseaudio ?

It's a significant amount of work, but I think PulseAudio should be improved to support this in any case, if other approaches make life miserable for driver developers.

What would be the interface for getting notifications about new and removed PCM devices? udev?

In general, yes.

Quoted 4 lines

PulseAudio (mostly) doesn't use the hw:X devices directly. Instead, it uses logical names like "front", "hdmi", "iec958", etc. Speaking of HDMI specifically, PulseAudio uses devices from "hdmi:X,0" to "hdmi:X,7". With this new dynamic PCM system, do these logical names still work?

Unfortunately, this doesn't work for HDA as is, because of the terribly arcane secret. For keeping the compatibility with the old config, there is a static mapping of the hdmi:x,y and hw:x,z.

Quoted 6 lines

Maybe we should introduce a new device class for dynamic HDMI/DP device, something like dhdmi:x,y, to make things straightforward. (Just a concept -- I'm not good at naming.)

Alternatively, we may introduce a new argument to hdmi PCM to access like "hdmi:CARD=x,SYSDEV=y".

Quoted 7 lines

What happens if PulseAudio tries to open "dhdmi:x,y" or "hdmi:CARD=x,SYSDEV=y", when y points to a non-HDMI device? If it fails, then PulseAudio can replace its current "hdmi:x,[0-7]" devices with "hdmi:CARD=X,SYSDEV=[0-13]" and blindly try every hw PCM device. But if opening a non-HDMI device through "hdmi:CARD=x,SYSDEV=y" succeeds, then how is PulseAudio supposed to know which hw PCM devices are HDMI devices?

Quoted 6 lines

It's a good question. I think this is the core part of the missing pieces.

What I have in my mind is to extend SNDRV_PCM_CLASS_* definition for dedicated to HDMI/DP, e.g. SNDRV_PCM_CLASS_HDMI. The difference between DP and HDMI can be specified in subclass optionally.

Sounds good. Knowing the difference between HDMI, DisplayPort and Thunderbolt would be very nice too. It would enable better labelling in UIs.

Quoted 7 lines

I have one minor concern, though: what if a PCM device isn't statically assigned to one output connector? Is it possible that some DSP is able to dynamically change the PCM routing between different outputs? In those cases the PCM class would be useless information (unless there are notifications when the class changes). I don't have any better ideas, though.

Quoted 3 lines

This is possible atm, we can route from one PCM to multiple outputs depending on DSP and codec mixer settings. The outputs can even be on different physical interfaces.

Hmm, my question was a bit silly. It has been possible forever to switch between e.g. headphones and speakers using just the mixer. And in this case that we're discussing, the question was irrelevant anyway, because surely the dynamically created PCM would be dedicated to the plugged-in HDMI port, otherwise the dynamic creation wouldn't really make sense. So, setting the PCM class should work at least in this case.

If there are cases where the DSP may route the same PCM to either HDMI or non-HDMI output depending on mixer settings, then that's when the PCM class would be less useful.

In any case, PulseAudio needs to figure out whether it should ignore the PCM class and use "hdmi:x,y" or whether it should check the PCM class and use "hdmi:CARD=x,SYSDEV=y". How should that be done? Is checking the kernel (and alsa-lib) version a good approach, or is some other mechanism needed? Checking the versions should work, if all HDMI audio drivers start to set the PCM class at the same time, but if some drivers set it and some don't, then version checking isn't a reliable solution.

Quoted 82 lines · Tanu

Quoted 70 lines · Liam Girdwood

Quoted 61 lines · Tanu

Quoted 10 lines · Takashi Iwai

At Tue, 23 Jun 2015 07:51:22 +0000,

Quoted 8 lines · Tanu

(Added pulseaudio-discuss to CC.)

Quoted 4 lines · Takashi Iwai

At Mon, 22 Jun 2015 15:21:16 +0000,

Quoted 2 lines · Tanu

On Mon, 2015-06-22 at 14:29 +0100, Liam Girdwood wrote:

<snip>

Quoted 36 lines

Quoted 28 lines

Quoted 21 lines

Quoted 9 lines

Quoted 2 lines

Tanu, what's your take on the effort for dynamically created PCMs support for pulseaudio ?

It's a significant amount of work, but I think PulseAudio should be improved to support this in any case, if other approaches make life miserable for driver developers.

What would be the interface for getting notifications about new and removed PCM devices? udev?

In general, yes.

Quoted 4 lines

PulseAudio (mostly) doesn't use the hw:X devices directly. Instead, it uses logical names like "front", "hdmi", "iec958", etc. Speaking of HDMI specifically, PulseAudio uses devices from "hdmi:X,0" to "hdmi:X,7". With this new dynamic PCM system, do these logical names still work?

Unfortunately, this doesn't work for HDA as is, because of the terribly arcane secret. For keeping the compatibility with the old config, there is a static mapping of the hdmi:x,y and hw:x,z.

Quoted 6 lines

Maybe we should introduce a new device class for dynamic HDMI/DP device, something like dhdmi:x,y, to make things straightforward. (Just a concept -- I'm not good at naming.)

Alternatively, we may introduce a new argument to hdmi PCM to access like "hdmi:CARD=x,SYSDEV=y".

Quoted 7 lines

What happens if PulseAudio tries to open "dhdmi:x,y" or "hdmi:CARD=x,SYSDEV=y", when y points to a non-HDMI device? If it fails, then PulseAudio can replace its current "hdmi:x,[0-7]" devices with "hdmi:CARD=X,SYSDEV=[0-13]" and blindly try every hw PCM device. But if opening a non-HDMI device through "hdmi:CARD=x,SYSDEV=y" succeeds, then how is PulseAudio supposed to know which hw PCM devices are HDMI devices?

Quoted 6 lines

It's a good question. I think this is the core part of the missing pieces.

What I have in my mind is to extend SNDRV_PCM_CLASS_* definition for dedicated to HDMI/DP, e.g. SNDRV_PCM_CLASS_HDMI. The difference between DP and HDMI can be specified in subclass optionally.

Sounds good. Knowing the difference between HDMI, DisplayPort and Thunderbolt would be very nice too. It would enable better labelling in UIs.

Quoted 7 lines

I have one minor concern, though: what if a PCM device isn't statically assigned to one output connector? Is it possible that some DSP is able to dynamically change the PCM routing between different outputs? In those cases the PCM class would be useless information (unless there are notifications when the class changes). I don't have any better ideas, though.

Quoted 3 lines

This is possible atm, we can route from one PCM to multiple outputs depending on DSP and codec mixer settings. The outputs can even be on different physical interfaces.

Hmm, my question was a bit silly. It has been possible forever to switch between e.g. headphones and speakers using just the mixer. And in this case that we're discussing, the question was irrelevant anyway, because surely the dynamically created PCM would be dedicated to the plugged-in HDMI port, otherwise the dynamic creation wouldn't really make sense. So, setting the PCM class should work at least in this case.

Quoted 4 lines

If there are cases where the DSP may route the same PCM to either HDMI or non-HDMI output depending on mixer settings, then that's when the PCM class would be less useful.

Yeah, we may need to provide other hints here. DSPs make it possible now to route one PCM to multiple HW endpoints.

Quoted 7 lines

In any case, PulseAudio needs to figure out whether it should ignore the PCM class and use "hdmi:x,y" or whether it should check the PCM class and use "hdmi:CARD=x,SYSDEV=y". How should that be done? Is checking the kernel (and alsa-lib) version a good approach, or is some other mechanism needed? Checking the versions should work, if all HDMI audio drivers start to set the PCM class at the same time, but if some drivers set it and some don't, then version checking isn't a reliable solution.

struct snd_pcm_info does have some reserved space (64 bytes currently set as 0) that could be used to differentiate and provide information on the PCM ?

(Added pulseaudio-discuss to CC.)

Quoted 13 lines · Takashi Iwai

At Mon, 22 Jun 2015 15:21:16 +0000,

Quoted 10 lines · Tanu

Quoted 2 lines · Liam Girdwood

Btw, the topology core now also dynamically creates/removes mixer controls, can PA handle this atm ?

No, PA checks the mixer controls only when loading a new card. Dynamically added controls are ignored. Dynamically removed controls just cause silent failure, at least when setting volume (I didn't check other use cases). That is, changing the volume appears to succeed, but nothing actually happens.

Quoted 3 lines

Won't PA use ELD or other information? The corresponding controls to HDMI/DP will be created / deleted dynamically together with a PCM device, I suppose.

Yes, PA uses ELD. If mixer controls become dynamic too, then that's another thing to implement.

Quoted 16 lines · Tanu Kaskinen

(Added pulseaudio-discuss to CC.)

Quoted 13 lines · Takashi Iwai

At Mon, 22 Jun 2015 15:21:16 +0000,

Quoted 10 lines · Tanu

Quoted 2 lines · Liam Girdwood

Btw, the topology core now also dynamically creates/removes mixer controls, can PA handle this atm ?

No, PA checks the mixer controls only when loading a new card. Dynamically added controls are ignored. Dynamically removed controls just cause silent failure, at least when setting volume (I didn't check other use cases). That is, changing the volume appears to succeed, but nothing actually happens.

Quoted 6 lines

Quoted 3 lines

Won't PA use ELD or other information? The corresponding controls to HDMI/DP will be created / deleted dynamically together with a PCM device, I suppose.

Yes, PA uses ELD. If mixer controls become dynamic too, then that's another thing to implement.

Liam, how does the userspace know when the new PCM is ready to use?

If
new mixer controls are added too, it has to be defined which gets created first: the PCM device or the mixer controls. I think it would be best if the mixer controls get created first, and when the new PCM device appears, it's immediately fully ready to be used.

At Mon, 22 Jun 2015 15:21:16 +0000,

Quoted 4 lines · Tanu

PulseAudio (mostly) doesn't use the hw:X devices directly. Instead, it uses logical names like "front", "hdmi", "iec958", etc. Speaking of HDMI specifically, PulseAudio uses devices from "hdmi:X,0" to "hdmi:X,7". With this new dynamic PCM system, do these logical names still work?

Unfortunately, this doesn't work for HDA as is, because of the terribly arcane secret. For keeping the compatibility with the old config, there is a static mapping of the hdmi:x,y and hw:x,z.

Quoted 4 lines

Quoted 2 lines

Shall we move or cc this discussion on audio driver side to ALSA ML?

Oops I thought I had cc'ed these patches to alsa-devel as well when I

Quoted 3 lines

Quoted 3 lines

I think we also need to decide how to manage PCM devices for DP MST. Now the HD-A driver create a PCM device for each pin, and the

Quoted 1 line

Quoted 1 line

number is 1 for each PCM. Now with DP MST enabled, each pin can

Quoted 4 lines

Quoted 4 lines

multiple streams (e.g. 3 on Intel HSW/BDW/SKL).

There may be 2 options:
-#1: Let an HDMI codec specify number of substreams, same as the

Quoted 4 lines

Quoted 4 lines

of device entries on a pin. We can specify 3 for HSW/BDW/SKL. Other vendors can also specify a value according to actual HW capabilities.

So for HSW, we have 3x3 subtreams totally. But we only have 3

Quoted 1 line

Quoted 1 line

(for 3 display pipelines), so we can open up to 3 substreams at the

Quoted 1 line

Quoted 1 line

time. When the audio driver finds all 3 convertors are used when

Quoted 3 lines

Quoted 1 line

a new substream, it will fail.

One thing I noticed is the number of devices on a PIN is only updated

Quoted 1 line

the MST device is plugged in so normally pins 5,6,7 have 0 devices, and

Quoted 4 lines

I plug in MST device, I get the 3 devices on port 6. So it seems dynamic enough at this point, though I guess it'll always be 0 or 3.

Quoted 2 lines

- #2: Create PCM device dynamically. Only create a PCM devices for a

Quoted 3 lines

Quoted 3 lines

entry which connects to monitor with audio support. When the monitor is removed, the PCM device will be disconnected, closed and removed, similar to the USB case.

Quoted 4 lines

Quoted 4 lines

Quoted 2 lines

Shall we move or cc this discussion on audio driver side to ALSA ML?

Oops I thought I had cc'ed these patches to alsa-devel as well when I

sent them.

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

I think we also need to decide how to manage PCM devices for DP MST. Now the HD-A driver create a PCM device for each pin, and the

substream

Quoted 1 line

Quoted 1 line

Quoted 1 line

number is 1 for each PCM. Now with DP MST enabled, each pin can

support

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

multiple streams (e.g. 3 on Intel HSW/BDW/SKL).

There may be 2 options:
-#1: Let an HDMI codec specify number of substreams, same as the

number

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

of device entries on a pin. We can specify 3 for HSW/BDW/SKL. Other vendors can also specify a value according to actual HW capabilities.

So for HSW, we have 3x3 subtreams totally. But we only have 3

convertors

Quoted 1 line

Quoted 1 line

Quoted 1 line

(for 3 display pipelines), so we can open up to 3 substreams at the

same

Quoted 1 line

Quoted 1 line

Quoted 1 line

time. When the audio driver finds all 3 convertors are used when

opening

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

a new substream, it will fail.

One thing I noticed is the number of devices on a PIN is only updated

when

Quoted 1 line

Quoted 1 line

the MST device is plugged in so normally pins 5,6,7 have 0 devices, and

when

Quoted 3 lines

Quoted 3 lines

I plug in MST device, I get the 3 devices on port 6. So it seems dynamic enough at this point, though I guess it'll always be 0 or 3.

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

- #2: Create PCM device dynamically. Only create a PCM devices for a

device

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

entry which connects to monitor with audio support. When the monitor is removed, the PCM device will be disconnected, closed and removed, similar to the USB case.

Do the driver really need dynamic PCM since display ports are dasiy chained and the locations within the dasiy chain are fixed ?

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Shall we move or cc this discussion on audio driver side to ALSA

Quoted 5 lines

Quoted 2 lines

Quoted 2 lines

Oops I thought I had cc'ed these patches to alsa-devel as well when I

sent them.

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

I think we also need to decide how to manage PCM devices for DP

Quoted 11 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

Now the HD-A driver create a PCM device for each pin, and the

substream

Quoted 1 line

Quoted 1 line

Quoted 1 line

number is 1 for each PCM. Now with DP MST enabled, each pin can

support

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

multiple streams (e.g. 3 on Intel HSW/BDW/SKL).

There may be 2 options:
-#1: Let an HDMI codec specify number of substreams, same as the

number

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

of device entries on a pin. We can specify 3 for HSW/BDW/SKL. Other vendors can also specify a value according to actual HW

Quoted 4 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

So for HSW, we have 3x3 subtreams totally. But we only have 3

convertors

Quoted 1 line

Quoted 1 line

Quoted 1 line

(for 3 display pipelines), so we can open up to 3 substreams at the

Quoted 8 lines

same

Quoted 1 line

Quoted 1 line

Quoted 1 line

time. When the audio driver finds all 3 convertors are used when

opening

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

a new substream, it will fail.

One thing I noticed is the number of devices on a PIN is only updated

when

Quoted 1 line

Quoted 1 line

the MST device is plugged in so normally pins 5,6,7 have 0 devices,

Quoted 2 lines

when

Quoted 1 line

Quoted 1 line

I plug in MST device, I get the 3 devices on port 6. So it seems

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

enough at this point, though I guess it'll always be 0 or 3.

Quoted 2 lines

- #2: Create PCM device dynamically. Only create a PCM devices for

Quoted 2 lines

device

Quoted 1 line

Quoted 1 line

Quoted 1 line

entry which connects to monitor with audio support. When the

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

is removed, the PCM device will be disconnected, closed and

Quoted 3 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

similar to the USB case.

Do the driver really need dynamic PCM since display ports are dasiy

Quoted 1 line

and the locations within the dasiy chain are fixed ?

2015-6-29 下午3:58於 "Jani Nikula" <[email protected]>寫道：

Quoted 4 lines · Raymond Yau

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Shall we move or cc this discussion on audio driver side to ALSA

ML?

Quoted 5 lines

Quoted 5 lines

Quoted 2 lines

Quoted 2 lines

Oops I thought I had cc'ed these patches to alsa-devel as well when I

sent them.

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

I think we also need to decide how to manage PCM devices for DP

MST.

Quoted 11 lines

Quoted 11 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

Now the HD-A driver create a PCM device for each pin, and the

substream

Quoted 1 line

Quoted 1 line

Quoted 1 line

number is 1 for each PCM. Now with DP MST enabled, each pin can

support

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

multiple streams (e.g. 3 on Intel HSW/BDW/SKL).

There may be 2 options:
-#1: Let an HDMI codec specify number of substreams, same as the

number

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

of device entries on a pin. We can specify 3 for HSW/BDW/SKL. Other vendors can also specify a value according to actual HW

capabilities.

Quoted 4 lines

Quoted 4 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

So for HSW, we have 3x3 subtreams totally. But we only have 3

convertors

Quoted 1 line

Quoted 1 line

Quoted 1 line

(for 3 display pipelines), so we can open up to 3 substreams at the

Quoted 8 lines

Quoted 8 lines

same

Quoted 1 line

Quoted 1 line

Quoted 1 line

time. When the audio driver finds all 3 convertors are used when

opening

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

a new substream, it will fail.

One thing I noticed is the number of devices on a PIN is only updated

when

Quoted 1 line

Quoted 1 line

the MST device is plugged in so normally pins 5,6,7 have 0 devices,

and

Quoted 2 lines

Quoted 2 lines

when

Quoted 1 line

Quoted 1 line

I plug in MST device, I get the 3 devices on port 6. So it seems

dynamic

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

enough at this point, though I guess it'll always be 0 or 3.

Quoted 2 lines

- #2: Create PCM device dynamically. Only create a PCM devices for

a

Quoted 2 lines

Quoted 2 lines

device

Quoted 1 line

Quoted 1 line

Quoted 1 line

entry which connects to monitor with audio support. When the

monitor

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

is removed, the PCM device will be disconnected, closed and

removed,

Quoted 2 lines

Quoted 2 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

similar to the USB case.

Quoted 1 line

Quoted 1 line

Do the driver really need dynamic PCM since display ports are dasiy

chained

Quoted 1 line

Quoted 1 line

and the locations within the dasiy chain are fixed ?

http://www.intel.com/support/motherboards/desktop/sb/CS-034199.htm

Do the first display always support sound since hdmi can connect to tv using dvi ?

As Pulseaudio does not support multi streaming ( playing different audio streams to headphone and rear panel speakers , how can pulseaudion support displayport MST ?

Quoted 2 lines

Shall we move or cc this discussion on audio driver side to ALSA ML?

Oops I thought I had cc'ed these patches to alsa-devel as well when I

Quoted 5 lines

I think we also need to decide how to manage PCM devices for DP MST. Now the HD-A driver create a PCM device for each pin, and the substream number is 1 for each PCM. Now with DP MST enabled, each pin can support multiple streams (e.g. 3 on Intel HSW/BDW/SKL).

Quoted 4 lines

Quoted 2 lines

Shall we move or cc this discussion on audio driver side to ALSA ML?

Oops I thought I had cc'ed these patches to alsa-devel as well when I

sent them.

Quoted 5 lines

Quoted 5 lines

I think we also need to decide how to manage PCM devices for DP MST. Now the HD-A driver create a PCM device for each pin, and the substream number is 1 for each PCM. Now with DP MST enabled, each pin can support multiple streams (e.g. 3 on Intel HSW/BDW/SKL).

What hardware feature are available ?

Only three monitors playing different audio streams or

With 1x3 video wall , can the center monitor playback stereo while the left monitor play left channel and right monitor play right channel ?