* haswell displayport MST audio support
@ 2015-06-17 4:01 Dave Airlie
2015-06-17 4:01 ` [PATCH 1/4] dp/mst: add SDP stream support Dave Airlie
` (3 more replies)
0 siblings, 4 replies; 34+ messages in thread
From: Dave Airlie @ 2015-06-17 4:01 UTC (permalink / raw)
To: intel-gfx
Hi guys,
Since the audio folks have ignored me mostly, I'm sending these patches
as-is, I'd like to get the GPU side merged, and hopefully provoke
some action from the Intel audio people, otherwise I guess I get to
ship these in Fedora/RHEL and everyone else can do what they like.
Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 1/4] dp/mst: add SDP stream support
2015-06-17 4:01 haswell displayport MST audio support Dave Airlie
@ 2015-06-17 4:01 ` Dave Airlie
2015-06-17 4:01 ` [PATCH 2/4] i915: add support for GPU side of MST audio Dave Airlie
` (2 subsequent siblings)
3 siblings, 0 replies; 34+ messages in thread
From: Dave Airlie @ 2015-06-17 4:01 UTC (permalink / raw)
To: intel-gfx
From: Dave Airlie <airlied@redhat.com>
This adds code to initialise the SDP streams
for a sink in the simplest ordering.
I've no idea how you'd want to control the
ordering at this level, so don't bother
until someone comes up with a use case.
Signed-off-by: Dave Airlie <airlied@redhat.com>
---
drivers/gpu/drm/drm_dp_mst_topology.c | 31 ++++++++++++++++++++++++++++---
include/drm/drm_dp_mst_helper.h | 6 ++++--
2 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index ca9d7f1..fb65f5d 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -666,7 +666,9 @@ static int build_enum_path_resources(struct drm_dp_sideband_msg_tx *msg, int por
}
static int build_allocate_payload(struct drm_dp_sideband_msg_tx *msg, int port_num,
- u8 vcpi, uint16_t pbn)
+ u8 vcpi, uint16_t pbn,
+ u8 number_sdp_streams,
+ u8 *sdp_stream_sink)
{
struct drm_dp_sideband_msg_req_body req;
memset(&req, 0, sizeof(req));
@@ -674,6 +676,8 @@ static int build_allocate_payload(struct drm_dp_sideband_msg_tx *msg, int port_n
req.u.allocate_payload.port_number = port_num;
req.u.allocate_payload.vcpi = vcpi;
req.u.allocate_payload.pbn = pbn;
+ req.u.allocate_payload.number_sdp_streams = number_sdp_streams;
+ memcpy(req.u.allocate_payload.sdp_stream_sink, sdp_stream_sink, number_sdp_streams);
drm_dp_encode_sideband_req(&req, msg);
msg->path_msg = true;
return 0;
@@ -1531,6 +1535,8 @@ static int drm_dp_payload_send_msg(struct drm_dp_mst_topology_mgr *mgr,
struct drm_dp_sideband_msg_tx *txmsg;
struct drm_dp_mst_branch *mstb;
int len, ret;
+ u8 sinks[DRM_DP_MAX_SDP_STREAMS];
+ int i;
mstb = drm_dp_get_validated_mstb_ref(mgr, port->parent);
if (!mstb)
@@ -1542,10 +1548,13 @@ static int drm_dp_payload_send_msg(struct drm_dp_mst_topology_mgr *mgr,
goto fail_put;
}
+ for (i = 0; i < port->num_sdp_streams; i++)
+ sinks[i] = i;
+
txmsg->dst = mstb;
len = build_allocate_payload(txmsg, port->port_num,
id,
- pbn);
+ pbn, port->num_sdp_streams, sinks);
drm_dp_queue_down_tx(mgr, txmsg);
@@ -2226,6 +2235,21 @@ out:
EXPORT_SYMBOL(drm_dp_mst_detect_port);
/**
+ * drm_dp_mst_port_has_audio()
+ */
+bool drm_dp_mst_port_has_audio(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port)
+{
+ bool ret = false;
+ port = drm_dp_get_validated_port_ref(mgr, port);
+ if (!port)
+ return ret;
+ ret = port->has_audio;
+ drm_dp_put_port(port);
+ return ret;
+}
+EXPORT_SYMBOL(drm_dp_mst_port_has_audio);
+
+/**
* drm_dp_mst_get_edid() - get EDID for an MST port
* @connector: toplevel connector to get EDID for
* @mgr: manager for this port
@@ -2250,6 +2274,7 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_
edid = drm_get_edid(connector, &port->aux.ddc);
drm_mode_connector_set_tile_property(connector);
+ port->has_audio = drm_detect_monitor_audio(edid);
drm_dp_put_port(port);
return edid;
}
@@ -2533,7 +2558,7 @@ static void drm_dp_mst_dump_mstb(struct seq_file *m,
seq_printf(m, "%smst: %p, %d\n", prefix, mstb, mstb->num_ports);
list_for_each_entry(port, &mstb->ports, next) {
- seq_printf(m, "%sport: %d: ddps: %d ldps: %d, %p, conn: %p\n", prefix, port->port_num, port->ddps, port->ldps, port, port->connector);
+ seq_printf(m, "%sport: %d: ddps: %d ldps: %d, sdp: %d/%d, %p, conn: %p\n", prefix, port->port_num, port->ddps, port->ldps, port->num_sdp_streams, port->num_sdp_stream_sinks, port, port->connector);
if (port->mstb)
drm_dp_mst_dump_mstb(m, port->mstb);
}
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 86d0b25..d8d3799 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -94,6 +94,7 @@ struct drm_dp_mst_port {
struct drm_dp_mst_topology_mgr *mgr;
struct edid *cached_edid; /* for DP logical ports - make tiling work */
+ bool has_audio;
};
/**
@@ -215,13 +216,13 @@ struct drm_dp_sideband_msg_rx {
struct drm_dp_sideband_msg_hdr initial_hdr;
};
-
+#define DRM_DP_MAX_SDP_STREAMS 16
struct drm_dp_allocate_payload {
u8 port_number;
u8 number_sdp_streams;
u8 vcpi;
u16 pbn;
- u8 sdp_stream_sink[8];
+ u8 sdp_stream_sink[DRM_DP_MAX_SDP_STREAMS];
};
struct drm_dp_allocate_payload_ack_reply {
@@ -482,6 +483,7 @@ int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl
enum drm_connector_status drm_dp_mst_detect_port(struct drm_connector *connector, struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port);
+bool drm_dp_mst_port_has_audio(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port);
struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port);
--
2.4.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 2/4] i915: add support for GPU side of MST audio
2015-06-17 4:01 haswell displayport MST audio support Dave Airlie
2015-06-17 4:01 ` [PATCH 1/4] dp/mst: add SDP stream support Dave Airlie
@ 2015-06-17 4:01 ` Dave Airlie
2015-06-17 8:07 ` Chris Wilson
2015-06-17 11:27 ` Daniel Vetter
2015-06-17 4:01 ` [PATCH 3/4] snd: add support for displayport multi-stream to hda codec Dave Airlie
2015-06-17 4:01 ` [PATCH 4/4] snd/hdmi: hack out haswell codec workaround Dave Airlie
3 siblings, 2 replies; 34+ messages in thread
From: Dave Airlie @ 2015-06-17 4:01 UTC (permalink / raw)
To: intel-gfx
From: Dave Airlie <airlied@redhat.com>
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 <airlied@redhat.com>
---
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(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index dc55c51..a109a21 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -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");
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 2396cc7..77d2a01 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -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;
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 5cb4748..8a39e99 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -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;
--
2.4.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-17 4:01 haswell displayport MST audio support Dave Airlie
2015-06-17 4:01 ` [PATCH 1/4] dp/mst: add SDP stream support Dave Airlie
2015-06-17 4:01 ` [PATCH 2/4] i915: add support for GPU side of MST audio Dave Airlie
@ 2015-06-17 4:01 ` Dave Airlie
2015-06-19 9:54 ` Lin, Mengdong
2015-06-17 4:01 ` [PATCH 4/4] snd/hdmi: hack out haswell codec workaround Dave Airlie
3 siblings, 1 reply; 34+ messages in thread
From: Dave Airlie @ 2015-06-17 4:01 UTC (permalink / raw)
To: intel-gfx
From: Dave Airlie <airlied@redhat.com>
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 <airlied@redhat.com>
---
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(-)
diff --git a/include/sound/hda_verbs.h b/include/sound/hda_verbs.h
index d0509db..3b62ac5 100644
--- a/include/sound/hda_verbs.h
+++ b/include/sound/hda_verbs.h
@@ -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
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 5645481..3981c63 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -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
diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c
index baaf7ed0..39fac53 100644
--- a/sound/pci/hda/hda_proc.c
+++ b/sound/pci/hda/hda_proc.c
@@ -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;
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 5f44f60..8272656 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -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
- 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);
- 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, 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);
+ 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,
+ 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)
@@ -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);
@@ -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);
- 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;
+ 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",
- 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,
+ 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);
+ if (pin_eld->monitor_present)
+ eld->eld_valid = !!(per_pin->dev_list[device_num] & AC_DE_ELDV);
+ else
+ eld->eld_valid = false;
+ if (eld->eld_valid)
+ snd_hda_codec_write(codec, pin_nid, 0, AC_VERB_SET_DEVICE_SEL, device_num);
+ } 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",
+ 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;
+ 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);
}
}
@@ -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)
@@ -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) {
@@ -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;
@@ -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;
- 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);
- 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);
+ }
+ } 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;
}
generic_hdmi_init_per_pins(codec);
--
2.4.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 4/4] snd/hdmi: hack out haswell codec workaround
2015-06-17 4:01 haswell displayport MST audio support Dave Airlie
` (2 preceding siblings ...)
2015-06-17 4:01 ` [PATCH 3/4] snd: add support for displayport multi-stream to hda codec Dave Airlie
@ 2015-06-17 4:01 ` Dave Airlie
3 siblings, 0 replies; 34+ messages in thread
From: Dave Airlie @ 2015-06-17 4:01 UTC (permalink / raw)
To: intel-gfx
From: Dave Airlie <airlied@redhat.com>
This breaks MST audio, as it appears the hw doesn't
operate like the code believes.
What seems to happen if we have 3 pins, 5, 6, 7
and 6 is connected to 3 devices and 5/7 are connected
to 0 devices, then devices 5/7 act always as if they
are equivalent to pin 6 device 0.
So I connected pin 6 device 0 to channel 1, then
this code goes and connects pin 5/7 to channel 0
but this actually reprograms pin 6 device 0 to channel
0 at the same time and breaks audio.
Signed-off-by: Dave Airlie <airlied@redhat.com>
---
sound/pci/hda/patch_hdmi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 8272656..7a4aeac 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1411,6 +1411,7 @@ static void intel_not_share_assigned_cvt(struct hda_codec *codec,
int cvt_idx, curr;
struct hdmi_spec_per_cvt *per_cvt;
+ return;
/* configure all pins, including "no physical connection" ones */
for_each_hda_codec_node(nid, codec) {
unsigned int wid_caps = get_wcaps(codec, nid);
--
2.4.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 2/4] i915: add support for GPU side of MST audio
2015-06-17 4:01 ` [PATCH 2/4] i915: add support for GPU side of MST audio Dave Airlie
@ 2015-06-17 8:07 ` Chris Wilson
2015-06-17 11:54 ` Jani Nikula
2015-06-17 11:27 ` Daniel Vetter
1 sibling, 1 reply; 34+ messages in thread
From: Chris Wilson @ 2015-06-17 8:07 UTC (permalink / raw)
To: Dave Airlie; +Cc: intel-gfx
On Wed, Jun 17, 2015 at 02:01:57PM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
>
> 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 <airlied@redhat.com>
> ---
> @@ -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
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/4] i915: add support for GPU side of MST audio
2015-06-17 4:01 ` [PATCH 2/4] i915: add support for GPU side of MST audio Dave Airlie
2015-06-17 8:07 ` Chris Wilson
@ 2015-06-17 11:27 ` Daniel Vetter
1 sibling, 0 replies; 34+ messages in thread
From: Daniel Vetter @ 2015-06-17 11:27 UTC (permalink / raw)
To: Dave Airlie; +Cc: intel-gfx
On Wed, Jun 17, 2015 at 02:01:57PM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
>
> 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 <airlied@redhat.com>
Might be time to refactor all the ddi code a bit since this duplicates a
lot. Otoh the code in intel_ddi.c is full of presonality confusion
if-ladders which ain't pretty either.
Anyway this duplicates what we do for sst audio, and I guess that makes
sense. Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> since I lack clue
for a real r-b.
Feel free to merge through whatever tree you want if the audio folks keep
on being offline. But perhaps first resend again with alsa-devel on cc.
-Daniel
> ---
> 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(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index dc55c51..a109a21 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -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");
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 2396cc7..77d2a01 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -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;
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 5cb4748..8a39e99 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -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;
> --
> 2.4.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/4] i915: add support for GPU side of MST audio
2015-06-17 8:07 ` Chris Wilson
@ 2015-06-17 11:54 ` Jani Nikula
2015-06-17 11:56 ` Jani Nikula
2015-06-19 6:27 ` Lin, Mengdong
0 siblings, 2 replies; 34+ messages in thread
From: Jani Nikula @ 2015-06-17 11:54 UTC (permalink / raw)
To: Chris Wilson, Dave Airlie; +Cc: intel-gfx
On Wed, 17 Jun 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Wed, Jun 17, 2015 at 02:01:57PM +1000, Dave Airlie wrote:
>> From: Dave Airlie <airlied@redhat.com>
>>
>> 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 <airlied@redhat.com>
>> ---
>> @@ -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.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/4] i915: add support for GPU side of MST audio
2015-06-17 11:54 ` Jani Nikula
@ 2015-06-17 11:56 ` Jani Nikula
2015-06-19 6:27 ` Lin, Mengdong
1 sibling, 0 replies; 34+ messages in thread
From: Jani Nikula @ 2015-06-17 11:56 UTC (permalink / raw)
To: Chris Wilson, Dave Airlie; +Cc: intel-gfx
On Wed, 17 Jun 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Wed, 17 Jun 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> On Wed, Jun 17, 2015 at 02:01:57PM +1000, Dave Airlie wrote:
>>> From: Dave Airlie <airlied@redhat.com>
>>>
>>> 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 <airlied@redhat.com>
>>> ---
>>> @@ -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. :(
Also, should mask out anything that was there before!
BR,
Jani.
>
> BR,
> Jani.
>
>
>
>
>
>
>
>> -Chris
>>
>> --
>> Chris Wilson, Intel Open Source Technology Centre
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/4] i915: add support for GPU side of MST audio
2015-06-17 11:54 ` Jani Nikula
2015-06-17 11:56 ` Jani Nikula
@ 2015-06-19 6:27 ` Lin, Mengdong
1 sibling, 0 replies; 34+ messages in thread
From: Lin, Mengdong @ 2015-06-19 6:27 UTC (permalink / raw)
To: Jani Nikula, Chris Wilson, Dave Airlie
Cc: Takashi Iwai (tiwai@suse.de), Yang, Libin, intel-gfx, Lu, Han
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Jani Nikula
> On Wed, 17 Jun 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Wed, Jun 17, 2015 at 02:01:57PM +1000, Dave Airlie wrote:
> >> From: Dave Airlie <airlied@redhat.com>
> >>
> >> 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 <airlied@redhat.com>
> >> ---
> >> @@ -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);
I think we should not add this line.
DIP Port Select [Bit 30:29 ]of register AUD_DIP_ELD_CTRL_ST is ReadOnly.
For DP MST, I think the device select happens in both i915 driver and
HD-Audio driver:
- For i915 driver, it should select which transcoder/pipe is connected to a
device on a port. MST allows 3 devices on a port.
Could someone guide me to the code?
The Bit 30:29 just reflect the selection, although I think this is not enough
because it only tell the port number, but now we hope know the device
entry on a port.
- For audio driver, it should select which convertor is connected to a device
of a codec pin (pin is a mapping for a DDI port). We need to add the support
in HD-A driver. The default device entry is 0 for a pin. Current HD-A driver
does not handle other 2 device entries on a pin.
Thanks
Mengdong
> >> 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.
>
>
>
>
>
>
>
> > -Chris
> >
> > --
> > Chris Wilson, Intel Open Source Technology Centre
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-17 4:01 ` [PATCH 3/4] snd: add support for displayport multi-stream to hda codec Dave Airlie
@ 2015-06-19 9:54 ` Lin, Mengdong
2015-06-19 10:33 ` Dave Airlie
0 siblings, 1 reply; 34+ messages in thread
From: Lin, Mengdong @ 2015-06-19 9:54 UTC (permalink / raw)
To: Dave Airlie, intel-gfx
Cc: Takashi Iwai (tiwai@suse.de), Girdwood, Liam R, Lu, Han
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.
Could you share your preference on these 2 options or other suggestions?
Thanks
Mengdong
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Dave Airlie
> Sent: Wednesday, June 17, 2015 12:02 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 3/4] snd: add support for displayport multi-stream
> to hda codec.
>
> From: Dave Airlie <airlied@redhat.com>
>
> 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 <airlied@redhat.com>
> ---
> 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(-)
>
> diff --git a/include/sound/hda_verbs.h b/include/sound/hda_verbs.h index
> d0509db..3b62ac5 100644
> --- a/include/sound/hda_verbs.h
> +++ b/include/sound/hda_verbs.h
> @@ -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
> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index
> 5645481..3981c63 100644
> --- a/sound/pci/hda/hda_codec.c
> +++ b/sound/pci/hda/hda_codec.c
> @@ -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
> diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c index
> baaf7ed0..39fac53 100644
> --- a/sound/pci/hda/hda_proc.c
> +++ b/sound/pci/hda/hda_proc.c
> @@ -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;
>
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index
> 5f44f60..8272656 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -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
> - 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);
> - 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,
> +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);
> + 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,
> + 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)
> @@ -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);
> @@ -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);
> - 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;
> + 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",
> - 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,
> + 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);
> + if (pin_eld->monitor_present)
> + eld->eld_valid = !!(per_pin->dev_list[device_num] &
> AC_DE_ELDV);
> + else
> + eld->eld_valid = false;
> + if (eld->eld_valid)
> + snd_hda_codec_write(codec, pin_nid, 0,
> AC_VERB_SET_DEVICE_SEL, device_num);
> + } 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",
> + 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;
> + 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);
> }
> }
> @@ -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)
> @@ -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) {
> @@ -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;
> @@ -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;
> - 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);
> - 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);
> + }
> + } 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;
> }
>
> generic_hdmi_init_per_pins(codec);
> --
> 2.4.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-19 9:54 ` Lin, Mengdong
@ 2015-06-19 10:33 ` Dave Airlie
2015-06-19 11:15 ` Takashi Iwai
2015-06-25 8:22 ` Raymond Yau
0 siblings, 2 replies; 34+ messages in thread
From: Dave Airlie @ 2015-06-19 10:33 UTC (permalink / raw)
To: Lin, Mengdong
Cc: alsa-devel, Takashi Iwai (tiwai@suse.de),
intel-gfx, Girdwood, Liam R, Lu, Han
On 19 June 2015 at 19:54, Lin, Mengdong <mengdong.lin@intel.com> wrote:
> 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.
> 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.
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.
>
> - #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.
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.
>> Signed-off-by: Dave Airlie <airlied@redhat.com>
>> ---
>> 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(-)
>>
>> diff --git a/include/sound/hda_verbs.h b/include/sound/hda_verbs.h index
>> d0509db..3b62ac5 100644
>> --- a/include/sound/hda_verbs.h
>> +++ b/include/sound/hda_verbs.h
>> @@ -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
>> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index
>> 5645481..3981c63 100644
>> --- a/sound/pci/hda/hda_codec.c
>> +++ b/sound/pci/hda/hda_codec.c
>> @@ -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
>> diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c index
>> baaf7ed0..39fac53 100644
>> --- a/sound/pci/hda/hda_proc.c
>> +++ b/sound/pci/hda/hda_proc.c
>> @@ -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;
>>
>> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index
>> 5f44f60..8272656 100644
>> --- a/sound/pci/hda/patch_hdmi.c
>> +++ b/sound/pci/hda/patch_hdmi.c
>> @@ -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
>> - 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);
>> - 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,
>> +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);
>> + 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,
>> + 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)
>> @@ -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);
>> @@ -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);
>> - 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;
>> + 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",
>> - 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,
>> + 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);
>> + if (pin_eld->monitor_present)
>> + eld->eld_valid = !!(per_pin->dev_list[device_num] &
>> AC_DE_ELDV);
>> + else
>> + eld->eld_valid = false;
>> + if (eld->eld_valid)
>> + snd_hda_codec_write(codec, pin_nid, 0,
>> AC_VERB_SET_DEVICE_SEL, device_num);
>> + } 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",
>> + 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;
>> + 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);
>> }
>> }
>> @@ -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)
>> @@ -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) {
>> @@ -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;
>> @@ -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;
>> - 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);
>> - 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);
>> + }
>> + } 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;
>> }
>>
>> generic_hdmi_init_per_pins(codec);
>> --
>> 2.4.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-19 10:33 ` Dave Airlie
@ 2015-06-19 11:15 ` Takashi Iwai
2015-06-22 12:54 ` Daniel Vetter
2015-06-27 6:09 ` [alsa-devel] " Raymond Yau
2015-06-25 8:22 ` Raymond Yau
1 sibling, 2 replies; 34+ messages in thread
From: Takashi Iwai @ 2015-06-19 11:15 UTC (permalink / raw)
To: Dave Airlie; +Cc: alsa-devel, intel-gfx, Girdwood, Liam R, Lu, Han
At Fri, 19 Jun 2015 20:33:39 +1000,
Dave Airlie wrote:
>
> On 19 June 2015 at 19:54, Lin, Mengdong <mengdong.lin@intel.com> wrote:
> > 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.
>
> > 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.
>
> 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.
> >
> > - #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.
>
> 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.
Takashi
>
> Dave.
>
> >> Signed-off-by: Dave Airlie <airlied@redhat.com>
> >> ---
> >> 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(-)
> >>
> >> diff --git a/include/sound/hda_verbs.h b/include/sound/hda_verbs.h index
> >> d0509db..3b62ac5 100644
> >> --- a/include/sound/hda_verbs.h
> >> +++ b/include/sound/hda_verbs.h
> >> @@ -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
> >> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index
> >> 5645481..3981c63 100644
> >> --- a/sound/pci/hda/hda_codec.c
> >> +++ b/sound/pci/hda/hda_codec.c
> >> @@ -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
> >> diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c index
> >> baaf7ed0..39fac53 100644
> >> --- a/sound/pci/hda/hda_proc.c
> >> +++ b/sound/pci/hda/hda_proc.c
> >> @@ -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;
> >>
> >> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index
> >> 5f44f60..8272656 100644
> >> --- a/sound/pci/hda/patch_hdmi.c
> >> +++ b/sound/pci/hda/patch_hdmi.c
> >> @@ -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
> >> - 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);
> >> - 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,
> >> +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);
> >> + 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,
> >> + 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)
> >> @@ -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);
> >> @@ -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);
> >> - 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;
> >> + 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",
> >> - 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,
> >> + 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);
> >> + if (pin_eld->monitor_present)
> >> + eld->eld_valid = !!(per_pin->dev_list[device_num] &
> >> AC_DE_ELDV);
> >> + else
> >> + eld->eld_valid = false;
> >> + if (eld->eld_valid)
> >> + snd_hda_codec_write(codec, pin_nid, 0,
> >> AC_VERB_SET_DEVICE_SEL, device_num);
> >> + } 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",
> >> + 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;
> >> + 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);
> >> }
> >> }
> >> @@ -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)
> >> @@ -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) {
> >> @@ -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;
> >> @@ -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;
> >> - 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);
> >> - 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);
> >> + }
> >> + } 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;
> >> }
> >>
> >> generic_hdmi_init_per_pins(codec);
> >> --
> >> 2.4.1
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-19 11:15 ` Takashi Iwai
@ 2015-06-22 12:54 ` Daniel Vetter
2015-06-22 13:23 ` Takashi Iwai
2015-06-27 6:09 ` [alsa-devel] " Raymond Yau
1 sibling, 1 reply; 34+ messages in thread
From: Daniel Vetter @ 2015-06-22 12:54 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Lu, Han, alsa-devel, intel-gfx, Girdwood, Liam R
On Fri, Jun 19, 2015 at 01:15:57PM +0200, Takashi Iwai wrote:
> At Fri, 19 Jun 2015 20:33:39 +1000,
> Dave Airlie wrote:
> >
> > On 19 June 2015 at 19:54, Lin, Mengdong <mengdong.lin@intel.com> wrote:
> > > 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.
> >
> > > 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.
> >
> > 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.
> > >
> > > - #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.
> >
> > 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.
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.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-22 12:54 ` Daniel Vetter
@ 2015-06-22 13:23 ` Takashi Iwai
2015-06-22 13:29 ` Liam Girdwood
0 siblings, 1 reply; 34+ messages in thread
From: Takashi Iwai @ 2015-06-22 13:23 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Lu, Han, alsa-devel, intel-gfx, Girdwood, Liam R
At Mon, 22 Jun 2015 14:54:29 +0200,
Daniel Vetter wrote:
>
> On Fri, Jun 19, 2015 at 01:15:57PM +0200, Takashi Iwai wrote:
> > At Fri, 19 Jun 2015 20:33:39 +1000,
> > Dave Airlie wrote:
> > >
> > > On 19 June 2015 at 19:54, Lin, Mengdong <mengdong.lin@intel.com> wrote:
> > > > 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.
> > >
> > > > 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.
> > >
> > > 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.
> > > >
> > > > - #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.
> > >
> > > 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.
>
> 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.
> 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.
Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-22 13:23 ` Takashi Iwai
@ 2015-06-22 13:29 ` Liam Girdwood
2015-06-22 15:21 ` Kaskinen, Tanu
0 siblings, 1 reply; 34+ messages in thread
From: Liam Girdwood @ 2015-06-22 13:29 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel, Kaskinen, Tanu, intel-gfx, Lu, Han
On Mon, 2015-06-22 at 15:23 +0200, Takashi Iwai wrote:
> At Mon, 22 Jun 2015 14:54:29 +0200,
> Daniel Vetter wrote:
> >
> > On Fri, Jun 19, 2015 at 01:15:57PM +0200, Takashi Iwai wrote:
> > > At Fri, 19 Jun 2015 20:33:39 +1000,
> > > Dave Airlie wrote:
> > > >
> > > > On 19 June 2015 at 19:54, Lin, Mengdong <mengdong.lin@intel.com> wrote:
> > > > > 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.
> > > >
> > > > > 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.
> > > >
> > > > 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.
> > > > >
> > > > > - #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.
> > > >
> > > > 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.
> >
> > 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.
>
> > 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.
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 ?
Thanks
Liam
---------------------------------------------------------------------
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-22 13:29 ` Liam Girdwood
@ 2015-06-22 15:21 ` Kaskinen, Tanu
2015-06-22 15:44 ` Takashi Iwai
0 siblings, 1 reply; 34+ messages in thread
From: Kaskinen, Tanu @ 2015-06-22 15:21 UTC (permalink / raw)
To: Girdwood, Liam R; +Cc: alsa-devel, tiwai, intel-gfx, Lu, Han
On Mon, 2015-06-22 at 14:29 +0100, Liam Girdwood wrote:
> On Mon, 2015-06-22 at 15:23 +0200, Takashi Iwai wrote:
> > At Mon, 22 Jun 2015 14:54:29 +0200,
> > Daniel Vetter wrote:
> > >
> > > On Fri, Jun 19, 2015 at 01:15:57PM +0200, Takashi Iwai wrote:
> > > > At Fri, 19 Jun 2015 20:33:39 +1000,
> > > > Dave Airlie wrote:
> > > > >
> > > > > On 19 June 2015 at 19:54, Lin, Mengdong <mengdong.lin@intel.com> wrote:
> > > > > > 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.
> > > > >
> > > > > > 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.
> > > > >
> > > > > 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.
> > > > > >
> > > > > > - #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.
> > > > >
> > > > > 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.
> > >
> > > 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.
> >
> > > 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.
>
> 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?
> 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.
--
Tanu
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-22 15:21 ` Kaskinen, Tanu
@ 2015-06-22 15:44 ` Takashi Iwai
2015-06-23 7:51 ` [Intel-gfx] " Kaskinen, Tanu
2015-06-26 16:27 ` [Intel-gfx] " Kaskinen, Tanu
0 siblings, 2 replies; 34+ messages in thread
From: Takashi Iwai @ 2015-06-22 15:44 UTC (permalink / raw)
To: Kaskinen, Tanu; +Cc: alsa-devel, Girdwood, Liam R, intel-gfx, Lu, Han
At Mon, 22 Jun 2015 15:21:16 +0000,
Kaskinen, Tanu wrote:
>
> On Mon, 2015-06-22 at 14:29 +0100, Liam Girdwood wrote:
> > On Mon, 2015-06-22 at 15:23 +0200, Takashi Iwai wrote:
> > > At Mon, 22 Jun 2015 14:54:29 +0200,
> > > Daniel Vetter wrote:
> > > >
> > > > On Fri, Jun 19, 2015 at 01:15:57PM +0200, Takashi Iwai wrote:
> > > > > At Fri, 19 Jun 2015 20:33:39 +1000,
> > > > > Dave Airlie wrote:
> > > > > >
> > > > > > On 19 June 2015 at 19:54, Lin, Mengdong <mengdong.lin@intel.com> wrote:
> > > > > > > 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.
> > > > > >
> > > > > > > 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.
> > > > > >
> > > > > > 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.
> > > > > > >
> > > > > > > - #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.
> > > > > >
> > > > > > 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.
> > > >
> > > > 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.
> > >
> > > > 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.
> >
> > 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?
In general, yes.
> 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".
> 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.
Yeah, that's the missing information. We can give something in
alsa-lib config to indicate the conflict, in theory...
> 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.
> > 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.
Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-22 15:44 ` Takashi Iwai
@ 2015-06-23 7:51 ` Kaskinen, Tanu
2015-06-23 8:06 ` Takashi Iwai
2015-06-26 10:50 ` Kaskinen, Tanu
2015-06-26 16:27 ` [Intel-gfx] " Kaskinen, Tanu
1 sibling, 2 replies; 34+ messages in thread
From: Kaskinen, Tanu @ 2015-06-23 7:51 UTC (permalink / raw)
To: tiwai
Cc: alsa-devel, pulseaudio-discuss, Girdwood, Liam R, intel-gfx,
daniel, airlied
(Added pulseaudio-discuss to CC.)
On Mon, 2015-06-22 at 17:44 +0200, Takashi Iwai wrote:
> At Mon, 22 Jun 2015 15:21:16 +0000,
> Kaskinen, Tanu wrote:
> >
> > On Mon, 2015-06-22 at 14:29 +0100, Liam Girdwood wrote:
> > > On Mon, 2015-06-22 at 15:23 +0200, Takashi Iwai wrote:
> > > > At Mon, 22 Jun 2015 14:54:29 +0200,
> > > > Daniel Vetter wrote:
> > > > >
> > > > > On Fri, Jun 19, 2015 at 01:15:57PM +0200, Takashi Iwai wrote:
> > > > > > At Fri, 19 Jun 2015 20:33:39 +1000,
> > > > > > Dave Airlie wrote:
> > > > > > >
> > > > > > > On 19 June 2015 at 19:54, Lin, Mengdong <mengdong.lin@intel.com> wrote:
> > > > > > > > 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.
> > > > > > >
> > > > > > > > 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.
> > > > > > >
> > > > > > > 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.
> > > > > > > >
> > > > > > > > - #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.
> > > > > > >
> > > > > > > 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.
> > > > >
> > > > > 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.
> > > >
> > > > > 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.
> > >
> > > 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?
>
> In general, yes.
>
> > 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".
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?
> > 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.
>
> Yeah, that's the missing information. We can give something in
> alsa-lib config to indicate the conflict, in theory...
>
> > 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.
>
>
> > > 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.
Yes, PA uses ELD. If mixer controls become dynamic too, then that's
another thing to implement.
--
Tanu
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-23 7:51 ` [Intel-gfx] " Kaskinen, Tanu
@ 2015-06-23 8:06 ` Takashi Iwai
2015-06-24 12:41 ` [Intel-gfx] " Kaskinen, Tanu
2015-06-26 10:50 ` Kaskinen, Tanu
1 sibling, 1 reply; 34+ messages in thread
From: Takashi Iwai @ 2015-06-23 8:06 UTC (permalink / raw)
To: Kaskinen, Tanu
Cc: alsa-devel, pulseaudio-discuss, Girdwood, Liam R, intel-gfx, Lu, Han
At Tue, 23 Jun 2015 07:51:22 +0000,
Kaskinen, Tanu wrote:
>
> (Added pulseaudio-discuss to CC.)
>
> On Mon, 2015-06-22 at 17:44 +0200, Takashi Iwai wrote:
> > At Mon, 22 Jun 2015 15:21:16 +0000,
> > Kaskinen, Tanu wrote:
> > >
> > > On Mon, 2015-06-22 at 14:29 +0100, Liam Girdwood wrote:
> > > > On Mon, 2015-06-22 at 15:23 +0200, Takashi Iwai wrote:
> > > > > At Mon, 22 Jun 2015 14:54:29 +0200,
> > > > > Daniel Vetter wrote:
> > > > > >
> > > > > > On Fri, Jun 19, 2015 at 01:15:57PM +0200, Takashi Iwai wrote:
> > > > > > > At Fri, 19 Jun 2015 20:33:39 +1000,
> > > > > > > Dave Airlie wrote:
> > > > > > > >
> > > > > > > > On 19 June 2015 at 19:54, Lin, Mengdong <mengdong.lin@intel.com> wrote:
> > > > > > > > > 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.
> > > > > > > >
> > > > > > > > > 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.
> > > > > > > >
> > > > > > > > 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.
> > > > > > > > >
> > > > > > > > > - #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.
> > > > > > > >
> > > > > > > > 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.
> > > > > >
> > > > > > 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.
> > > > >
> > > > > > 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.
> > > >
> > > > 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?
> >
> > In general, yes.
> >
> > > 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".
>
> 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.
We may add also SNDRV_PCM_CLASS_SPDIF, too.
This can be taken from snd_pcm_info_get_class(). Also it's exposed in
sysfs, too. (Oh the sysfs interface looks buggy, I have to fix...)
Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-23 8:06 ` Takashi Iwai
@ 2015-06-24 12:41 ` Kaskinen, Tanu
2015-06-24 13:06 ` [alsa-devel] " Liam Girdwood
0 siblings, 1 reply; 34+ messages in thread
From: Kaskinen, Tanu @ 2015-06-24 12:41 UTC (permalink / raw)
To: tiwai
Cc: alsa-devel, pulseaudio-discuss, Girdwood, Liam R, intel-gfx,
daniel, airlied
On Tue, 2015-06-23 at 10:06 +0200, Takashi Iwai wrote:
> At Tue, 23 Jun 2015 07:51:22 +0000,
> Kaskinen, Tanu wrote:
> >
> > (Added pulseaudio-discuss to CC.)
> >
> > On Mon, 2015-06-22 at 17:44 +0200, Takashi Iwai wrote:
> > > At Mon, 22 Jun 2015 15:21:16 +0000,
> > > Kaskinen, Tanu wrote:
> > > >
> > > > On Mon, 2015-06-22 at 14:29 +0100, Liam Girdwood wrote:
<snip>
> > > > > 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.
> > >
> > > > 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".
> >
> > 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.
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.
I also wonder about compatibility between various combinations of
kernel, alsa-lib and PA versions. An old kernel doesn't have dynamic PCM
devices, and doesn't know about the HDMI PCM class. An old alsa-lib
doesn't support the SYSDEV parameter, and doesn't know about the HDMI
PCM class. An old PA version will only use the old-style "hdmi:x,y"
device strings, and won't check the PCM class. There are 8 cases:
1: old kernel, old alsa-lib, old PA: No chance for regressions.
2: old kernel, old alsa-lib, new PA: The old alsa-lib version won't
report the PCM class correctly. This can be handled by falling back to
the old PA behaviour when an old alsa-lib version is detected at PA
build time.
3: old kernel, new alsa-lib, old PA: No chance for regressions.
4: old kernel, new alsa-lib, new PA: The old kernel won't report the PCM
class correctly. How should this be handled? Should PA do kernel version
checking, and fall back to the old behaviour with old kernels?
5: new kernel, old alsa-lib, old PA: Do the kernel changes cause
regressions on hardware that used to work, or do the changes only affect
new hardware that has never worked?
6: new kernel, old alsa-lib, new PA: If PA uses the fallback strategy
described in case 2, then this is equivalent to case 5.
7: new kernel, new alsa-lib, old PA: Equivalent to case 5.
8: new kernel, new alsa-lib, new PA: Everything works perfectly.
--
Tanu
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [alsa-devel] [Intel-gfx] [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-24 12:41 ` [Intel-gfx] " Kaskinen, Tanu
@ 2015-06-24 13:06 ` Liam Girdwood
2015-06-24 20:47 ` Kaskinen, Tanu
0 siblings, 1 reply; 34+ messages in thread
From: Liam Girdwood @ 2015-06-24 13:06 UTC (permalink / raw)
To: Kaskinen, Tanu
Cc: alsa-devel, pulseaudio-discuss, tiwai, intel-gfx, daniel, airlied
On Wed, 2015-06-24 at 12:41 +0000, Kaskinen, Tanu wrote:
> On Tue, 2015-06-23 at 10:06 +0200, Takashi Iwai wrote:
> > At Tue, 23 Jun 2015 07:51:22 +0000,
> > Kaskinen, Tanu wrote:
> > >
> > > (Added pulseaudio-discuss to CC.)
> > >
> > > On Mon, 2015-06-22 at 17:44 +0200, Takashi Iwai wrote:
> > > > At Mon, 22 Jun 2015 15:21:16 +0000,
> > > > Kaskinen, Tanu wrote:
> > > > >
> > > > > On Mon, 2015-06-22 at 14:29 +0100, Liam Girdwood wrote:
>
> <snip>
>
> > > > > > 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.
> > > >
> > > > > 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".
> > >
> > > 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.
>
> 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.
>
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.
Liam
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-24 13:06 ` [alsa-devel] " Liam Girdwood
@ 2015-06-24 20:47 ` Kaskinen, Tanu
2015-06-25 9:44 ` [alsa-devel] " Liam Girdwood
0 siblings, 1 reply; 34+ messages in thread
From: Kaskinen, Tanu @ 2015-06-24 20:47 UTC (permalink / raw)
To: liam.r.girdwood
Cc: alsa-devel, pulseaudio-discuss, tiwai, Lin, Mengdong, intel-gfx,
daniel, Lu, Han, airlied
On Wed, 2015-06-24 at 14:06 +0100, Liam Girdwood wrote:
> On Wed, 2015-06-24 at 12:41 +0000, Kaskinen, Tanu wrote:
> > On Tue, 2015-06-23 at 10:06 +0200, Takashi Iwai wrote:
> > > At Tue, 23 Jun 2015 07:51:22 +0000,
> > > Kaskinen, Tanu wrote:
> > > >
> > > > (Added pulseaudio-discuss to CC.)
> > > >
> > > > On Mon, 2015-06-22 at 17:44 +0200, Takashi Iwai wrote:
> > > > > At Mon, 22 Jun 2015 15:21:16 +0000,
> > > > > Kaskinen, Tanu wrote:
> > > > > >
> > > > > > On Mon, 2015-06-22 at 14:29 +0100, Liam Girdwood wrote:
> >
> > <snip>
> >
> > > > > > > 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.
> > > > >
> > > > > > 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".
> > > >
> > > > 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.
> >
> > 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.
> >
>
> 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.
--
Tanu
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [alsa-devel] [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-19 10:33 ` Dave Airlie
2015-06-19 11:15 ` Takashi Iwai
@ 2015-06-25 8:22 ` Raymond Yau
2015-06-25 12:52 ` David Weinehall
1 sibling, 1 reply; 34+ messages in thread
From: Raymond Yau @ 2015-06-25 8:22 UTC (permalink / raw)
To: Dave Airlie
Cc: alsa-devel, Takashi Iwai (tiwai@suse.de),
intel-gfx, Girdwood, Liam R, Lu, Han
[-- Attachment #1.1: Type: text/plain, Size: 678 bytes --]
> >
> > 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.
>
> > 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 ?
[-- Attachment #1.2: Type: text/html, Size: 802 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [alsa-devel] [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-24 20:47 ` Kaskinen, Tanu
@ 2015-06-25 9:44 ` Liam Girdwood
2015-06-26 10:14 ` [alsa-devel] [Intel-gfx] " Kaskinen, Tanu
0 siblings, 1 reply; 34+ messages in thread
From: Liam Girdwood @ 2015-06-25 9:44 UTC (permalink / raw)
To: Kaskinen, Tanu; +Cc: alsa-devel, pulseaudio-discuss, tiwai, intel-gfx, Lu, Han
On Wed, 2015-06-24 at 20:47 +0000, Kaskinen, Tanu wrote:
> On Wed, 2015-06-24 at 14:06 +0100, Liam Girdwood wrote:
> > On Wed, 2015-06-24 at 12:41 +0000, Kaskinen, Tanu wrote:
> > > On Tue, 2015-06-23 at 10:06 +0200, Takashi Iwai wrote:
> > > > At Tue, 23 Jun 2015 07:51:22 +0000,
> > > > Kaskinen, Tanu wrote:
> > > > >
> > > > > (Added pulseaudio-discuss to CC.)
> > > > >
> > > > > On Mon, 2015-06-22 at 17:44 +0200, Takashi Iwai wrote:
> > > > > > At Mon, 22 Jun 2015 15:21:16 +0000,
> > > > > > Kaskinen, Tanu wrote:
> > > > > > >
> > > > > > > On Mon, 2015-06-22 at 14:29 +0100, Liam Girdwood wrote:
> > >
> > > <snip>
> > >
> > > > > > > > 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.
> > > > > >
> > > > > > > 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".
> > > > >
> > > > > 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.
> > >
> > > 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.
> > >
> >
> > 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.
>
Yeah, we may need to provide other hints here. DSPs make it possible now
to route one PCM to multiple HW endpoints.
> 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 ?
Liam
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [alsa-devel] [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-25 8:22 ` Raymond Yau
@ 2015-06-25 12:52 ` David Weinehall
0 siblings, 0 replies; 34+ messages in thread
From: David Weinehall @ 2015-06-25 12:52 UTC (permalink / raw)
To: Raymond Yau
Cc: alsa-devel, Takashi Iwai (tiwai@suse.de),
intel-gfx, Girdwood, Liam R, Lu, Han
On Thu, Jun 25, 2015 at 04:22:38PM +0800, Raymond Yau wrote:
> > >
> > > 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.
> >
> > > 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 ?
Ohhh, clever idea :) If it isn't support already it'd make an awesome
feature at least.
Kind regards, David
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [alsa-devel] [Intel-gfx] [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-25 9:44 ` [alsa-devel] " Liam Girdwood
@ 2015-06-26 10:14 ` Kaskinen, Tanu
0 siblings, 0 replies; 34+ messages in thread
From: Kaskinen, Tanu @ 2015-06-26 10:14 UTC (permalink / raw)
To: liam.r.girdwood
Cc: alsa-devel, pulseaudio-discuss, tiwai, intel-gfx, daniel, airlied
On Thu, 2015-06-25 at 10:44 +0100, Liam Girdwood wrote:
> On Wed, 2015-06-24 at 20:47 +0000, Kaskinen, Tanu wrote:
> > On Wed, 2015-06-24 at 14:06 +0100, Liam Girdwood wrote:
> > > On Wed, 2015-06-24 at 12:41 +0000, Kaskinen, Tanu wrote:
> > > > On Tue, 2015-06-23 at 10:06 +0200, Takashi Iwai wrote:
> > > > > At Tue, 23 Jun 2015 07:51:22 +0000,
> > > > > Kaskinen, Tanu wrote:
> > > > > >
> > > > > > (Added pulseaudio-discuss to CC.)
> > > > > >
> > > > > > On Mon, 2015-06-22 at 17:44 +0200, Takashi Iwai wrote:
> > > > > > > At Mon, 22 Jun 2015 15:21:16 +0000,
> > > > > > > Kaskinen, Tanu wrote:
> > > > > > > >
> > > > > > > > On Mon, 2015-06-22 at 14:29 +0100, Liam Girdwood wrote:
> > > >
> > > > <snip>
> > > >
> > > > > > > > > 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.
> > > > > > >
> > > > > > > > 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".
> > > > > >
> > > > > > 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.
> > > >
> > > > 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.
> > > >
> > >
> > > 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.
> >
>
> Yeah, we may need to provide other hints here. DSPs make it possible now
> to route one PCM to multiple HW endpoints.
Getting information to userspace about the hw endpoints and how each hw
PCM and the endpoints are related would probably also solve this dynamic
HDMI PCM issue, but it would probably be much more complicated than just
adding a couple of bits to the PCM info. I suggest that we leave the
dynamically routed PCM problem alone for now, and only focus on the
dynamic HDMI PCM issue, even if that means that we devise a solution for
the HDMI PCM issue that will become obsolete later.
> > 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 ?
Yes, snd_pcm_info could have e.g. a version field. If the version is 0,
then PulseAudio should handle HDMI devices as before (blindly try
"hdmi:x,[0-7]"), and if it's 1, then PulseAudio should check the PCM
class, and use "hdmi:CARD=x,SYSDEV=y" if the class is HDMI.
--
Tanu
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-23 7:51 ` [Intel-gfx] " Kaskinen, Tanu
2015-06-23 8:06 ` Takashi Iwai
@ 2015-06-26 10:50 ` Kaskinen, Tanu
2015-06-26 18:45 ` Liam Girdwood
1 sibling, 1 reply; 34+ messages in thread
From: Kaskinen, Tanu @ 2015-06-26 10:50 UTC (permalink / raw)
To: tiwai
Cc: alsa-devel, pulseaudio-discuss, Girdwood, Liam R, intel-gfx,
daniel, airlied
On Tue, 2015-06-23 at 10:51 +0300, Tanu Kaskinen wrote:
> (Added pulseaudio-discuss to CC.)
>
> On Mon, 2015-06-22 at 17:44 +0200, Takashi Iwai wrote:
> > At Mon, 22 Jun 2015 15:21:16 +0000,
> > Kaskinen, Tanu wrote:
> > >
> > > On Mon, 2015-06-22 at 14:29 +0100, Liam Girdwood wrote:
> > > > 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.
>
> 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.
--
Tanu
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-22 15:44 ` Takashi Iwai
2015-06-23 7:51 ` [Intel-gfx] " Kaskinen, Tanu
@ 2015-06-26 16:27 ` Kaskinen, Tanu
1 sibling, 0 replies; 34+ messages in thread
From: Kaskinen, Tanu @ 2015-06-26 16:27 UTC (permalink / raw)
To: tiwai
Cc: alsa-devel, pulseaudio-discuss, Girdwood, Liam R, Lin, Mengdong,
intel-gfx, daniel, Lu, Han, airlied
On Mon, 2015-06-22 at 17:44 +0200, Takashi Iwai wrote:
> At Mon, 22 Jun 2015 15:21:16 +0000,
> Kaskinen, Tanu wrote:
> > 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.
Yes, the mapping is like this:
y = 0, z = 3
y = 1, z = 7
y = 2, z = 8
y = 3, z = 9
etc.
Now I'm wondering if this is HDA specific (your wording suggests that it
is)? For HDMI ELD information and jack detection support, PulseAudio has
been assuming this particular mapping. If other drivers have been using
a different mapping, then ELD and jack detection support has been and
continues to be broken for other drivers in PA.
--
Tanu
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-26 10:50 ` Kaskinen, Tanu
@ 2015-06-26 18:45 ` Liam Girdwood
0 siblings, 0 replies; 34+ messages in thread
From: Liam Girdwood @ 2015-06-26 18:45 UTC (permalink / raw)
To: Kaskinen, Tanu; +Cc: alsa-devel, pulseaudio-discuss, tiwai, intel-gfx, Lu, Han
On Fri, 2015-06-26 at 11:50 +0100, Kaskinen, Tanu wrote:
> On Tue, 2015-06-23 at 10:51 +0300, Tanu Kaskinen wrote:
> > (Added pulseaudio-discuss to CC.)
> >
> > On Mon, 2015-06-22 at 17:44 +0200, Takashi Iwai wrote:
> > > At Mon, 22 Jun 2015 15:21:16 +0000,
> > > Kaskinen, Tanu wrote:
> > > >
> > > > On Mon, 2015-06-22 at 14:29 +0100, Liam Girdwood wrote:
> > > > > 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.
> >
> > 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?
It's generally ready when the PCM is created iirc.
> 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.
>
I agree, adding mixers first make sense.
Liam
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [alsa-devel] [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-19 11:15 ` Takashi Iwai
2015-06-22 12:54 ` Daniel Vetter
@ 2015-06-27 6:09 ` Raymond Yau
2015-06-29 8:00 ` Jani Nikula
1 sibling, 1 reply; 34+ messages in thread
From: Raymond Yau @ 2015-06-27 6:09 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel, intel-gfx, Girdwood, Liam R, Lu, Han
[-- Attachment #1.1: Type: text/plain, Size: 1708 bytes --]
> > >
> > > 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.
> >
> > > 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.
> >
> > 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.
> > >
> > > - #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.
Do the driver really need dynamic PCM since display ports are dasiy chained
and the locations within the dasiy chain are fixed ?
[-- Attachment #1.2: Type: text/html, Size: 2095 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [alsa-devel] [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-27 6:09 ` [alsa-devel] " Raymond Yau
@ 2015-06-29 8:00 ` Jani Nikula
2015-07-03 3:45 ` Raymond Yau
0 siblings, 1 reply; 34+ messages in thread
From: Jani Nikula @ 2015-06-29 8:00 UTC (permalink / raw)
To: Raymond Yau, Takashi Iwai
Cc: Lu, Han, alsa-devel, intel-gfx, Girdwood, Liam R
On Sat, 27 Jun 2015, Raymond Yau <superquad.vortex2@gmail.com> wrote:
>> > >
>> > > 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.
>> >
>> > > 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.
>> >
>> > 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.
>> > >
>> > > - #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.
>
> Do the driver really need dynamic PCM since display ports are dasiy chained
> and the locations within the dasiy chain are fixed ?
Please elaborate.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [alsa-devel] [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-06-29 8:00 ` Jani Nikula
@ 2015-07-03 3:45 ` Raymond Yau
2015-07-03 8:54 ` Jani Nikula
0 siblings, 1 reply; 34+ messages in thread
From: Raymond Yau @ 2015-07-03 3:45 UTC (permalink / raw)
To: Jani Nikula
Cc: Takashi Iwai, Lu, Han, alsa-devel, intel-gfx, Girdwood, Liam R
[-- Attachment #1.1: Type: text/plain, Size: 2379 bytes --]
2015-6-29 下午3:58於 "Jani Nikula" <jani.nikula@linux.intel.com>寫道：
>
> On Sat, 27 Jun 2015, Raymond Yau <superquad.vortex2@gmail.com> wrote:
> >> > >
> >> > > 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.
> >> >
> >> > > 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.
> >> >
> >> > 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.
> >> > >
> >> > > - #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.
> >
> > Do the driver really need dynamic PCM since display ports are dasiy
chained
> > 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 ?
[-- Attachment #1.2: Type: text/html, Size: 3285 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [alsa-devel] [PATCH 3/4] snd: add support for displayport multi-stream to hda codec.
2015-07-03 3:45 ` Raymond Yau
@ 2015-07-03 8:54 ` Jani Nikula
0 siblings, 0 replies; 34+ messages in thread
From: Jani Nikula @ 2015-07-03 8:54 UTC (permalink / raw)
To: Raymond Yau
Cc: Takashi Iwai, Lu, Han, alsa-devel, intel-gfx, Girdwood, Liam R
On Fri, 03 Jul 2015, Raymond Yau <superquad.vortex2@gmail.com> wrote:
> 2015-6-29 下午3:58於 "Jani Nikula" <jani.nikula@linux.intel.com>寫道：
>>
>> On Sat, 27 Jun 2015, Raymond Yau <superquad.vortex2@gmail.com> wrote:
>> >> > >
>> >> > > 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.
>> >> >
>> >> > > 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.
>> >> >
>> >> > 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.
>> >> > >
>> >> > > - #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.
>> >
>> > Do the driver really need dynamic PCM since display ports are dasiy
> chained
>> > 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 ?
I don't see the relevance.
> As Pulseaudio does not support multi streaming ( playing different audio
> streams to headphone and rear panel speakers , how can pulseaudion support
> displayport MST ?
Even if it couldn't support multiple streams at the same time, there has
to be a way to choose one of the audio capable sinks in the DP MST
topology (which will change dynamically).
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2015-07-03 8:54 UTC | newest]
Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-17 4:01 haswell displayport MST audio support Dave Airlie
2015-06-17 4:01 ` [PATCH 1/4] dp/mst: add SDP stream support Dave Airlie
2015-06-17 4:01 ` [PATCH 2/4] i915: add support for GPU side of MST audio Dave Airlie
2015-06-17 8:07 ` Chris Wilson
2015-06-17 11:54 ` Jani Nikula
2015-06-17 11:56 ` Jani Nikula
2015-06-19 6:27 ` Lin, Mengdong
2015-06-17 11:27 ` Daniel Vetter
2015-06-17 4:01 ` [PATCH 3/4] snd: add support for displayport multi-stream to hda codec Dave Airlie
2015-06-19 9:54 ` Lin, Mengdong
2015-06-19 10:33 ` Dave Airlie
2015-06-19 11:15 ` Takashi Iwai
2015-06-22 12:54 ` Daniel Vetter
2015-06-22 13:23 ` Takashi Iwai
2015-06-22 13:29 ` Liam Girdwood
2015-06-22 15:21 ` Kaskinen, Tanu
2015-06-22 15:44 ` Takashi Iwai
2015-06-23 7:51 ` [Intel-gfx] " Kaskinen, Tanu
2015-06-23 8:06 ` Takashi Iwai
2015-06-24 12:41 ` [Intel-gfx] " Kaskinen, Tanu
2015-06-24 13:06 ` [alsa-devel] " Liam Girdwood
2015-06-24 20:47 ` Kaskinen, Tanu
2015-06-25 9:44 ` [alsa-devel] " Liam Girdwood
2015-06-26 10:14 ` [alsa-devel] [Intel-gfx] " Kaskinen, Tanu
2015-06-26 10:50 ` Kaskinen, Tanu
2015-06-26 18:45 ` Liam Girdwood
2015-06-26 16:27 ` [Intel-gfx] " Kaskinen, Tanu
2015-06-27 6:09 ` [alsa-devel] " Raymond Yau
2015-06-29 8:00 ` Jani Nikula
2015-07-03 3:45 ` Raymond Yau
2015-07-03 8:54 ` Jani Nikula
2015-06-25 8:22 ` Raymond Yau
2015-06-25 12:52 ` David Weinehall
2015-06-17 4:01 ` [PATCH 4/4] snd/hdmi: hack out haswell codec workaround Dave Airlie
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.