All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V4 1/2] dp/mst: add SDP stream support
@ 2015-12-02  6:09 libin.yang
  2015-12-02  6:09 ` [PATCH V4 2/2] drm/i915: start adding dp mst audio libin.yang
  2015-12-11  6:09 ` [PATCH V4 1/2] dp/mst: add SDP stream support Libin Yang
  0 siblings, 2 replies; 15+ messages in thread
From: libin.yang @ 2015-12-02  6:09 UTC (permalink / raw)
  To: intel-gfx, conselvan2, jani.nikula, ville.syrjala, daniel.vetter
  Cc: Dave Airlie, Libin Yang

From: Libin Yang <libin.yang@linux.intel.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.

Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 38 ++++++++++++++++++++++++++++++++---
 include/drm/drm_dp_mst_helper.h       |  7 +++++--
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 809959d..f50eb7b 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,9 @@ 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;
@@ -1562,6 +1567,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)
@@ -1573,10 +1580,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);
 
@@ -2259,6 +2269,27 @@ out:
 EXPORT_SYMBOL(drm_dp_mst_detect_port);
 
 /**
+ * drm_dp_mst_port_has_audio() - Check whether port has audio capability or not
+ * @mgr: manager for this port
+ * @port: unverified pointer to a port.
+ *
+ * This returns whether the port supports audio or not.
+ */
+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
@@ -2283,6 +2314,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;
 }
@@ -2566,7 +2598,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 5340099..74b5888 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 {
@@ -484,6 +485,8 @@ 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);
 
 
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V4 2/2] drm/i915: start adding dp mst audio
  2015-12-02  6:09 [PATCH V4 1/2] dp/mst: add SDP stream support libin.yang
@ 2015-12-02  6:09 ` libin.yang
  2015-12-08  8:01   ` Libin Yang
  2015-12-11  6:09 ` [PATCH V4 1/2] dp/mst: add SDP stream support Libin Yang
  1 sibling, 1 reply; 15+ messages in thread
From: libin.yang @ 2015-12-02  6:09 UTC (permalink / raw)
  To: intel-gfx, conselvan2, jani.nikula, ville.syrjala, daniel.vetter
  Cc: Dave Airlie, Libin Yang

From: Libin Yang <libin.yang@linux.intel.com>

This patch adds support for DP MST audio in i915.

Enable audio codec when DP MST is enabled if has_audio flag is set.
Disable audio codec when DP MST is disabled if has_audio flag is set.

Another separated patches to support DP MST audio will be implemented
in audio driver.

Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 16 ++++++++++++++++
 drivers/gpu/drm/i915/intel_audio.c  |  9 ++++++---
 drivers/gpu/drm/i915/intel_ddi.c    | 20 +++++++++++++++-----
 drivers/gpu/drm/i915/intel_dp_mst.c | 22 ++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h    |  2 ++
 5 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index bfd57fb..8387638 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2850,6 +2850,20 @@ 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;
+	bool has_audio = drm_dp_mst_port_has_audio(&intel_dp->mst_mgr,
+					intel_connector->port);
+
+	seq_printf(m, "\taudio support: %s\n", yesno(has_audio));
+}
+
 static void intel_hdmi_info(struct seq_file *m,
 			    struct intel_connector *intel_connector)
 {
@@ -2893,6 +2907,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 9aa83e7..5ad2e66 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -262,7 +262,8 @@ 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);
 
@@ -474,7 +475,8 @@ static void ilk_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(adjusted_mode);
@@ -512,7 +514,8 @@ 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, adjusted_mode) / 2;
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 76ce7c2..bc7fffb 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3108,6 +3108,19 @@ void intel_ddi_fdi_disable(struct drm_crtc *crtc)
 	I915_WRITE(FDI_RX_CTL(PIPE_A), val);
 }
 
+bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,
+				 struct intel_crtc *intel_crtc)
+{
+	u32 temp;
+
+	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(intel_crtc->pipe))
+			return true;
+	}
+	return false;
+}
+
 void intel_ddi_get_config(struct intel_encoder *encoder,
 			  struct intel_crtc_state *pipe_config)
 {
@@ -3168,11 +3181,8 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 		break;
 	}
 
-	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(intel_crtc->pipe))
-			pipe_config->has_audio = true;
-	}
+	pipe_config->has_audio =
+		intel_ddi_is_audio_enabled(dev_priv, intel_crtc);
 
 	if (encoder->type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp_bpp &&
 	    pipe_config->pipe_bpp > dev_priv->vbt.edp_bpp) {
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 8c4e7df..8b608c2 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -78,6 +78,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->crtc_clock, bpp);
 
 	pipe_config->pbn = mst_pbn;
@@ -102,6 +104,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);
@@ -112,6 +119,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)
@@ -208,6 +219,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;
 
@@ -220,6 +232,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,
@@ -245,6 +264,9 @@ static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
 
 	pipe_config->has_dp_encoder = true;
 
+	pipe_config->has_audio =
+		intel_ddi_is_audio_enabled(dev_priv, crtc);
+
 	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
 	if (temp & TRANS_DDI_PHSYNC)
 		flags |= DRM_MODE_FLAG_PHSYNC;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1ffd8d5..0d2c59f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1011,6 +1011,8 @@ void intel_ddi_set_pipe_settings(struct drm_crtc *crtc);
 void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp);
 bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector);
 void intel_ddi_fdi_disable(struct drm_crtc *crtc);
+bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,
+				 struct intel_crtc *intel_crtc);
 void intel_ddi_get_config(struct intel_encoder *encoder,
 			  struct intel_crtc_state *pipe_config);
 struct intel_encoder *
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH V4 2/2] drm/i915: start adding dp mst audio
  2015-12-02  6:09 ` [PATCH V4 2/2] drm/i915: start adding dp mst audio libin.yang
@ 2015-12-08  8:01   ` Libin Yang
  2015-12-10  9:02     ` Daniel Vetter
  0 siblings, 1 reply; 15+ messages in thread
From: Libin Yang @ 2015-12-08  8:01 UTC (permalink / raw)
  To: intel-gfx, conselvan2, jani.nikula, ville.syrjala, daniel.vetter
  Cc: Dave Airlie

Hi all,

Any comments on the patches?

Best Regards,
Libin

On 12/02/2015 02:09 PM, libin.yang@linux.intel.com wrote:
> From: Libin Yang <libin.yang@linux.intel.com>
>
> This patch adds support for DP MST audio in i915.
>
> Enable audio codec when DP MST is enabled if has_audio flag is set.
> Disable audio codec when DP MST is disabled if has_audio flag is set.
>
> Another separated patches to support DP MST audio will be implemented
> in audio driver.
>
> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c | 16 ++++++++++++++++
>   drivers/gpu/drm/i915/intel_audio.c  |  9 ++++++---
>   drivers/gpu/drm/i915/intel_ddi.c    | 20 +++++++++++++++-----
>   drivers/gpu/drm/i915/intel_dp_mst.c | 22 ++++++++++++++++++++++
>   drivers/gpu/drm/i915/intel_drv.h    |  2 ++
>   5 files changed, 61 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index bfd57fb..8387638 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2850,6 +2850,20 @@ 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;
> +	bool has_audio = drm_dp_mst_port_has_audio(&intel_dp->mst_mgr,
> +					intel_connector->port);
> +
> +	seq_printf(m, "\taudio support: %s\n", yesno(has_audio));
> +}
> +
>   static void intel_hdmi_info(struct seq_file *m,
>   			    struct intel_connector *intel_connector)
>   {
> @@ -2893,6 +2907,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 9aa83e7..5ad2e66 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -262,7 +262,8 @@ 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);
>
> @@ -474,7 +475,8 @@ static void ilk_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(adjusted_mode);
> @@ -512,7 +514,8 @@ 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, adjusted_mode) / 2;
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 76ce7c2..bc7fffb 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3108,6 +3108,19 @@ void intel_ddi_fdi_disable(struct drm_crtc *crtc)
>   	I915_WRITE(FDI_RX_CTL(PIPE_A), val);
>   }
>
> +bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,
> +				 struct intel_crtc *intel_crtc)
> +{
> +	u32 temp;
> +
> +	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(intel_crtc->pipe))
> +			return true;
> +	}
> +	return false;
> +}
> +
>   void intel_ddi_get_config(struct intel_encoder *encoder,
>   			  struct intel_crtc_state *pipe_config)
>   {
> @@ -3168,11 +3181,8 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>   		break;
>   	}
>
> -	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(intel_crtc->pipe))
> -			pipe_config->has_audio = true;
> -	}
> +	pipe_config->has_audio =
> +		intel_ddi_is_audio_enabled(dev_priv, intel_crtc);
>
>   	if (encoder->type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp_bpp &&
>   	    pipe_config->pipe_bpp > dev_priv->vbt.edp_bpp) {
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 8c4e7df..8b608c2 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -78,6 +78,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->crtc_clock, bpp);
>
>   	pipe_config->pbn = mst_pbn;
> @@ -102,6 +104,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);
> @@ -112,6 +119,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)
> @@ -208,6 +219,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;
>
> @@ -220,6 +232,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,
> @@ -245,6 +264,9 @@ static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
>
>   	pipe_config->has_dp_encoder = true;
>
> +	pipe_config->has_audio =
> +		intel_ddi_is_audio_enabled(dev_priv, crtc);
> +
>   	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
>   	if (temp & TRANS_DDI_PHSYNC)
>   		flags |= DRM_MODE_FLAG_PHSYNC;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 1ffd8d5..0d2c59f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1011,6 +1011,8 @@ void intel_ddi_set_pipe_settings(struct drm_crtc *crtc);
>   void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp);
>   bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector);
>   void intel_ddi_fdi_disable(struct drm_crtc *crtc);
> +bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,
> +				 struct intel_crtc *intel_crtc);
>   void intel_ddi_get_config(struct intel_encoder *encoder,
>   			  struct intel_crtc_state *pipe_config);
>   struct intel_encoder *
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V4 2/2] drm/i915: start adding dp mst audio
  2015-12-08  8:01   ` Libin Yang
@ 2015-12-10  9:02     ` Daniel Vetter
  2015-12-11  6:07       ` Libin Yang
  0 siblings, 1 reply; 15+ messages in thread
From: Daniel Vetter @ 2015-12-10  9:02 UTC (permalink / raw)
  To: Libin Yang; +Cc: intel-gfx, Dave Airlie, daniel.vetter

On Tue, Dec 08, 2015 at 04:01:20PM +0800, Libin Yang wrote:
> Hi all,
> 
> Any comments on the patches?

Sorry, simply fell through the cracks since Ander is on vacation. Takashi
is working on some cleanup patches to have a port->encoder mapping for the
audio side of i915. His patch cleans up all the existing audio code in
i915, but please work together with him to align mst code with the new
style.

Both patches queued for next.

Thanks, Daniel

> 
> Best Regards,
> Libin
> 
> On 12/02/2015 02:09 PM, libin.yang@linux.intel.com wrote:
> >From: Libin Yang <libin.yang@linux.intel.com>
> >
> >This patch adds support for DP MST audio in i915.
> >
> >Enable audio codec when DP MST is enabled if has_audio flag is set.
> >Disable audio codec when DP MST is disabled if has_audio flag is set.
> >
> >Another separated patches to support DP MST audio will be implemented
> >in audio driver.
> >
> >Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> >Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> >Signed-off-by: Dave Airlie <airlied@redhat.com>
> >---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 16 ++++++++++++++++
> >  drivers/gpu/drm/i915/intel_audio.c  |  9 ++++++---
> >  drivers/gpu/drm/i915/intel_ddi.c    | 20 +++++++++++++++-----
> >  drivers/gpu/drm/i915/intel_dp_mst.c | 22 ++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
> >  5 files changed, 61 insertions(+), 8 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> >index bfd57fb..8387638 100644
> >--- a/drivers/gpu/drm/i915/i915_debugfs.c
> >+++ b/drivers/gpu/drm/i915/i915_debugfs.c
> >@@ -2850,6 +2850,20 @@ 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;
> >+	bool has_audio = drm_dp_mst_port_has_audio(&intel_dp->mst_mgr,
> >+					intel_connector->port);
> >+
> >+	seq_printf(m, "\taudio support: %s\n", yesno(has_audio));
> >+}
> >+
> >  static void intel_hdmi_info(struct seq_file *m,
> >  			    struct intel_connector *intel_connector)
> >  {
> >@@ -2893,6 +2907,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 9aa83e7..5ad2e66 100644
> >--- a/drivers/gpu/drm/i915/intel_audio.c
> >+++ b/drivers/gpu/drm/i915/intel_audio.c
> >@@ -262,7 +262,8 @@ 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);
> >
> >@@ -474,7 +475,8 @@ static void ilk_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(adjusted_mode);
> >@@ -512,7 +514,8 @@ 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, adjusted_mode) / 2;
> >diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> >index 76ce7c2..bc7fffb 100644
> >--- a/drivers/gpu/drm/i915/intel_ddi.c
> >+++ b/drivers/gpu/drm/i915/intel_ddi.c
> >@@ -3108,6 +3108,19 @@ void intel_ddi_fdi_disable(struct drm_crtc *crtc)
> >  	I915_WRITE(FDI_RX_CTL(PIPE_A), val);
> >  }
> >
> >+bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,
> >+				 struct intel_crtc *intel_crtc)
> >+{
> >+	u32 temp;
> >+
> >+	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(intel_crtc->pipe))
> >+			return true;
> >+	}
> >+	return false;
> >+}
> >+
> >  void intel_ddi_get_config(struct intel_encoder *encoder,
> >  			  struct intel_crtc_state *pipe_config)
> >  {
> >@@ -3168,11 +3181,8 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >  		break;
> >  	}
> >
> >-	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(intel_crtc->pipe))
> >-			pipe_config->has_audio = true;
> >-	}
> >+	pipe_config->has_audio =
> >+		intel_ddi_is_audio_enabled(dev_priv, intel_crtc);
> >
> >  	if (encoder->type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp_bpp &&
> >  	    pipe_config->pipe_bpp > dev_priv->vbt.edp_bpp) {
> >diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> >index 8c4e7df..8b608c2 100644
> >--- a/drivers/gpu/drm/i915/intel_dp_mst.c
> >+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> >@@ -78,6 +78,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->crtc_clock, bpp);
> >
> >  	pipe_config->pbn = mst_pbn;
> >@@ -102,6 +104,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);
> >@@ -112,6 +119,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)
> >@@ -208,6 +219,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;
> >
> >@@ -220,6 +232,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,
> >@@ -245,6 +264,9 @@ static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
> >
> >  	pipe_config->has_dp_encoder = true;
> >
> >+	pipe_config->has_audio =
> >+		intel_ddi_is_audio_enabled(dev_priv, crtc);
> >+
> >  	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> >  	if (temp & TRANS_DDI_PHSYNC)
> >  		flags |= DRM_MODE_FLAG_PHSYNC;
> >diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >index 1ffd8d5..0d2c59f 100644
> >--- a/drivers/gpu/drm/i915/intel_drv.h
> >+++ b/drivers/gpu/drm/i915/intel_drv.h
> >@@ -1011,6 +1011,8 @@ void intel_ddi_set_pipe_settings(struct drm_crtc *crtc);
> >  void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp);
> >  bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector);
> >  void intel_ddi_fdi_disable(struct drm_crtc *crtc);
> >+bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,
> >+				 struct intel_crtc *intel_crtc);
> >  void intel_ddi_get_config(struct intel_encoder *encoder,
> >  			  struct intel_crtc_state *pipe_config);
> >  struct intel_encoder *
> >
> _______________________________________________
> 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] 15+ messages in thread

* Re: [PATCH V4 2/2] drm/i915: start adding dp mst audio
  2015-12-10  9:02     ` Daniel Vetter
@ 2015-12-11  6:07       ` Libin Yang
  2015-12-11 10:43         ` Takashi Iwai
  2015-12-11 14:05         ` Takashi Iwai
  0 siblings, 2 replies; 15+ messages in thread
From: Libin Yang @ 2015-12-11  6:07 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: alsa-devel, intel-gfx, Takashi Iwai, Dave Airlie, daniel.vetter

Add Takashi and ALSA mail list.

On 12/10/2015 05:02 PM, Daniel Vetter wrote:
> On Tue, Dec 08, 2015 at 04:01:20PM +0800, Libin Yang wrote:
>> Hi all,
>>
>> Any comments on the patches?
>
> Sorry, simply fell through the cracks since Ander is on vacation. Takashi
> is working on some cleanup patches to have a port->encoder mapping for the
> audio side of i915. His patch cleans up all the existing audio code in
> i915, but please work together with him to align mst code with the new
> style.
>
> Both patches queued for next.

Yes, I have seen Takashi's patches. I will check the patches.

Best Regards,
Libin

>
> Thanks, Daniel
>
>>
>> Best Regards,
>> Libin
>>
>> On 12/02/2015 02:09 PM, libin.yang@linux.intel.com wrote:
>>> From: Libin Yang <libin.yang@linux.intel.com>
>>>
>>> This patch adds support for DP MST audio in i915.
>>>
>>> Enable audio codec when DP MST is enabled if has_audio flag is set.
>>> Disable audio codec when DP MST is disabled if has_audio flag is set.
>>>
>>> Another separated patches to support DP MST audio will be implemented
>>> in audio driver.
>>>
>>> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
>>> Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
>>> Signed-off-by: Dave Airlie <airlied@redhat.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_debugfs.c | 16 ++++++++++++++++
>>>   drivers/gpu/drm/i915/intel_audio.c  |  9 ++++++---
>>>   drivers/gpu/drm/i915/intel_ddi.c    | 20 +++++++++++++++-----
>>>   drivers/gpu/drm/i915/intel_dp_mst.c | 22 ++++++++++++++++++++++
>>>   drivers/gpu/drm/i915/intel_drv.h    |  2 ++
>>>   5 files changed, 61 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>>> index bfd57fb..8387638 100644
>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>> @@ -2850,6 +2850,20 @@ 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;
>>> +	bool has_audio = drm_dp_mst_port_has_audio(&intel_dp->mst_mgr,
>>> +					intel_connector->port);
>>> +
>>> +	seq_printf(m, "\taudio support: %s\n", yesno(has_audio));
>>> +}
>>> +
>>>   static void intel_hdmi_info(struct seq_file *m,
>>>   			    struct intel_connector *intel_connector)
>>>   {
>>> @@ -2893,6 +2907,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 9aa83e7..5ad2e66 100644
>>> --- a/drivers/gpu/drm/i915/intel_audio.c
>>> +++ b/drivers/gpu/drm/i915/intel_audio.c
>>> @@ -262,7 +262,8 @@ 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);
>>>
>>> @@ -474,7 +475,8 @@ static void ilk_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(adjusted_mode);
>>> @@ -512,7 +514,8 @@ 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, adjusted_mode) / 2;
>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>>> index 76ce7c2..bc7fffb 100644
>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>>> @@ -3108,6 +3108,19 @@ void intel_ddi_fdi_disable(struct drm_crtc *crtc)
>>>   	I915_WRITE(FDI_RX_CTL(PIPE_A), val);
>>>   }
>>>
>>> +bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,
>>> +				 struct intel_crtc *intel_crtc)
>>> +{
>>> +	u32 temp;
>>> +
>>> +	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(intel_crtc->pipe))
>>> +			return true;
>>> +	}
>>> +	return false;
>>> +}
>>> +
>>>   void intel_ddi_get_config(struct intel_encoder *encoder,
>>>   			  struct intel_crtc_state *pipe_config)
>>>   {
>>> @@ -3168,11 +3181,8 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>>>   		break;
>>>   	}
>>>
>>> -	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(intel_crtc->pipe))
>>> -			pipe_config->has_audio = true;
>>> -	}
>>> +	pipe_config->has_audio =
>>> +		intel_ddi_is_audio_enabled(dev_priv, intel_crtc);
>>>
>>>   	if (encoder->type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp_bpp &&
>>>   	    pipe_config->pipe_bpp > dev_priv->vbt.edp_bpp) {
>>> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
>>> index 8c4e7df..8b608c2 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
>>> @@ -78,6 +78,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->crtc_clock, bpp);
>>>
>>>   	pipe_config->pbn = mst_pbn;
>>> @@ -102,6 +104,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);
>>> @@ -112,6 +119,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)
>>> @@ -208,6 +219,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;
>>>
>>> @@ -220,6 +232,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,
>>> @@ -245,6 +264,9 @@ static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
>>>
>>>   	pipe_config->has_dp_encoder = true;
>>>
>>> +	pipe_config->has_audio =
>>> +		intel_ddi_is_audio_enabled(dev_priv, crtc);
>>> +
>>>   	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
>>>   	if (temp & TRANS_DDI_PHSYNC)
>>>   		flags |= DRM_MODE_FLAG_PHSYNC;
>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>>> index 1ffd8d5..0d2c59f 100644
>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>> @@ -1011,6 +1011,8 @@ void intel_ddi_set_pipe_settings(struct drm_crtc *crtc);
>>>   void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp);
>>>   bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector);
>>>   void intel_ddi_fdi_disable(struct drm_crtc *crtc);
>>> +bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,
>>> +				 struct intel_crtc *intel_crtc);
>>>   void intel_ddi_get_config(struct intel_encoder *encoder,
>>>   			  struct intel_crtc_state *pipe_config);
>>>   struct intel_encoder *
>>>
>> _______________________________________________
>> 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] 15+ messages in thread

* Re: [PATCH V4 1/2] dp/mst: add SDP stream support
  2015-12-02  6:09 [PATCH V4 1/2] dp/mst: add SDP stream support libin.yang
  2015-12-02  6:09 ` [PATCH V4 2/2] drm/i915: start adding dp mst audio libin.yang
@ 2015-12-11  6:09 ` Libin Yang
  1 sibling, 0 replies; 15+ messages in thread
From: Libin Yang @ 2015-12-11  6:09 UTC (permalink / raw)
  To: intel-gfx, conselvan2, jani.nikula, ville.syrjala, daniel.vetter
  Cc: Dave Airlie, alsa-devel, tiwai

Add Takashi and ALSA mailing list

Best Regards,
Libin

On 12/02/2015 02:09 PM, libin.yang@linux.intel.com wrote:
> From: Libin Yang <libin.yang@linux.intel.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.
>
> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>   drivers/gpu/drm/drm_dp_mst_topology.c | 38 ++++++++++++++++++++++++++++++++---
>   include/drm/drm_dp_mst_helper.h       |  7 +++++--
>   2 files changed, 40 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 809959d..f50eb7b 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,9 @@ 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;
> @@ -1562,6 +1567,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)
> @@ -1573,10 +1580,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);
>
> @@ -2259,6 +2269,27 @@ out:
>   EXPORT_SYMBOL(drm_dp_mst_detect_port);
>
>   /**
> + * drm_dp_mst_port_has_audio() - Check whether port has audio capability or not
> + * @mgr: manager for this port
> + * @port: unverified pointer to a port.
> + *
> + * This returns whether the port supports audio or not.
> + */
> +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
> @@ -2283,6 +2314,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;
>   }
> @@ -2566,7 +2598,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 5340099..74b5888 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 {
> @@ -484,6 +485,8 @@ 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);
>
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V4 2/2] drm/i915: start adding dp mst audio
  2015-12-11  6:07       ` Libin Yang
@ 2015-12-11 10:43         ` Takashi Iwai
  2015-12-11 13:58           ` Takashi Iwai
  2015-12-14  9:33           ` Jani Nikula
  2015-12-11 14:05         ` Takashi Iwai
  1 sibling, 2 replies; 15+ messages in thread
From: Takashi Iwai @ 2015-12-11 10:43 UTC (permalink / raw)
  To: Libin Yang; +Cc: alsa-devel, intel-gfx, Dave Airlie, daniel.vetter

On Fri, 11 Dec 2015 07:07:53 +0100,
Libin Yang wrote:
> 
> >>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> >>> index 9aa83e7..5ad2e66 100644
> >>> --- a/drivers/gpu/drm/i915/intel_audio.c
> >>> +++ b/drivers/gpu/drm/i915/intel_audio.c
> >>> @@ -262,7 +262,8 @@ 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;

The same check is missing in hsw_audio_codec_enable()?

> >>>   	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> >>>
> >>> @@ -474,7 +475,8 @@ static void ilk_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;

... and missing for ilk_audio_codec_disable()?


> >>>   	else
> >>>   		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
> >>> @@ -512,7 +514,8 @@ 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))

IMO, it's better to have a macro to cover this two-line check instead
of open-coding at each place.  We'll have 5 places in the end.


thanks,

Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V4 2/2] drm/i915: start adding dp mst audio
  2015-12-11 10:43         ` Takashi Iwai
@ 2015-12-11 13:58           ` Takashi Iwai
  2015-12-21  8:31             ` [alsa-devel] " Libin Yang
  2015-12-14  9:33           ` Jani Nikula
  1 sibling, 1 reply; 15+ messages in thread
From: Takashi Iwai @ 2015-12-11 13:58 UTC (permalink / raw)
  To: Libin Yang; +Cc: alsa-devel, intel-gfx, Dave Airlie, daniel.vetter

On Fri, 11 Dec 2015 11:43:51 +0100,
Takashi Iwai wrote:
> 
> On Fri, 11 Dec 2015 07:07:53 +0100,
> Libin Yang wrote:
> > 
> > >>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> > >>> index 9aa83e7..5ad2e66 100644
> > >>> --- a/drivers/gpu/drm/i915/intel_audio.c
> > >>> +++ b/drivers/gpu/drm/i915/intel_audio.c
> > >>> @@ -262,7 +262,8 @@ 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;
> 
> The same check is missing in hsw_audio_codec_enable()?
> 
> > >>>   	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> > >>>
> > >>> @@ -474,7 +475,8 @@ static void ilk_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;
> 
> ... and missing for ilk_audio_codec_disable()?
> 
> 
> > >>>   	else
> > >>>   		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
> > >>> @@ -512,7 +514,8 @@ 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))
> 
> IMO, it's better to have a macro to cover this two-line check instead
> of open-coding at each place.  We'll have 5 places in the end.

Also, this patch still has an issue about the encoder type, namely, it
passes intel_encoder from MST, where you can't apply
enc_to_dig_port().  We need another help to get the digital port
depending on the encoder type, e.g.

static struct intel_digital_port *
intel_encoder_to_dig_port(struct intel_encoder *intel_encoder)
{
	struct drm_encoder *encoder = &intel_encoder->base;

	if (intel_encoder->type == INTEL_OUTPUT_DP_MST)
		return enc_to_mst(encoder)->primary;
	return enc_to_dig_port(encoder);
}


Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V4 2/2] drm/i915: start adding dp mst audio
  2015-12-11  6:07       ` Libin Yang
  2015-12-11 10:43         ` Takashi Iwai
@ 2015-12-11 14:05         ` Takashi Iwai
  2015-12-13  8:27           ` Takashi Iwai
  1 sibling, 1 reply; 15+ messages in thread
From: Takashi Iwai @ 2015-12-11 14:05 UTC (permalink / raw)
  To: Libin Yang; +Cc: alsa-devel, intel-gfx, Dave Airlie, daniel.vetter

On Fri, 11 Dec 2015 07:07:53 +0100,
Libin Yang wrote:
> 
> Add Takashi and ALSA mail list.
> 
> On 12/10/2015 05:02 PM, Daniel Vetter wrote:
> > On Tue, Dec 08, 2015 at 04:01:20PM +0800, Libin Yang wrote:
> >> Hi all,
> >>
> >> Any comments on the patches?
> >
> > Sorry, simply fell through the cracks since Ander is on vacation. Takashi
> > is working on some cleanup patches to have a port->encoder mapping for the
> > audio side of i915. His patch cleans up all the existing audio code in
> > i915, but please work together with him to align mst code with the new
> > style.
> >
> > Both patches queued for next.
> 
> Yes, I have seen Takashi's patches. I will check the patches.

The patch like below should work; it sets/clears the reverse mapping
dynamically for the MST encoder.

At least, now I could get a proper ELD from a docking station.  But
the audio itself doesn't seem working yet, missing something...

FWIW, the fixed patches are found in my test/hdmi-jack branch.
It contains my previous get_eld patchset, HD-audio side changes,
Libin's this patchset, plus Libin's HD-audio MST patchset and some
fixes.


Takashi

---
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 8b608c2cd070..87dad62fd10b 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -108,6 +108,7 @@ static void intel_mst_disable_dp(struct intel_encoder *encoder)
 	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);
+	enum port port = intel_dig_port->port;
 
 	int ret;
 
@@ -122,6 +123,9 @@ static void intel_mst_disable_dp(struct intel_encoder *encoder)
 	if (intel_crtc->config->has_audio) {
 		intel_audio_codec_disable(encoder);
 		intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
+		mutex_lock(&dev_priv->av_mutex);
+		dev_priv->dig_port_map[port] = NULL;
+		mutex_unlock(&dev_priv->av_mutex);
 	}
 }
 
@@ -236,6 +240,9 @@ static void intel_mst_enable_dp(struct intel_encoder *encoder)
 	if (crtc->config->has_audio) {
 		DRM_DEBUG_DRIVER("Enabling DP audio on pipe %c\n",
 				 pipe_name(crtc->pipe));
+		mutex_lock(&dev_priv->av_mutex);
+		dev_priv->dig_port_map[port] = encoder;
+		mutex_unlock(&dev_priv->av_mutex);
 		intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
 		intel_audio_codec_enable(encoder);
 	}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH V4 2/2] drm/i915: start adding dp mst audio
  2015-12-11 14:05         ` Takashi Iwai
@ 2015-12-13  8:27           ` Takashi Iwai
  0 siblings, 0 replies; 15+ messages in thread
From: Takashi Iwai @ 2015-12-13  8:27 UTC (permalink / raw)
  To: Libin Yang; +Cc: alsa-devel, intel-gfx, Dave Airlie, daniel.vetter

On Fri, 11 Dec 2015 15:05:08 +0100,
Takashi Iwai wrote:
> 
> On Fri, 11 Dec 2015 07:07:53 +0100,
> Libin Yang wrote:
> > 
> > Add Takashi and ALSA mail list.
> > 
> > On 12/10/2015 05:02 PM, Daniel Vetter wrote:
> > > On Tue, Dec 08, 2015 at 04:01:20PM +0800, Libin Yang wrote:
> > >> Hi all,
> > >>
> > >> Any comments on the patches?
> > >
> > > Sorry, simply fell through the cracks since Ander is on vacation. Takashi
> > > is working on some cleanup patches to have a port->encoder mapping for the
> > > audio side of i915. His patch cleans up all the existing audio code in
> > > i915, but please work together with him to align mst code with the new
> > > style.
> > >
> > > Both patches queued for next.
> > 
> > Yes, I have seen Takashi's patches. I will check the patches.
> 
> The patch like below should work; it sets/clears the reverse mapping
> dynamically for the MST encoder.

This might be too shortsighted and broken -- if multiple devices are
hook on the same port...

Now I wonder which graphics part actually corresponds to the HD-audio
widget.  So far, we supposed that each pin widget 0x05-0x07
corresponds to the digital port B-D in the fixed manner.  If so, how
is the case where two devices on the same port?  In the codec level,
we get an unsolicited event on the same pin but check with the
different device index.  What would be a unique index instead that can
be passed via the component ops?

Then, another question is which audio converter widget is used and how
is set up, if multiple devices were on the same pin.

Libin, do you have any git branch where MST really works?  I should
test it at first to confirm that it actually works on my test system.


thanks,

Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V4 2/2] drm/i915: start adding dp mst audio
  2015-12-11 10:43         ` Takashi Iwai
  2015-12-11 13:58           ` Takashi Iwai
@ 2015-12-14  9:33           ` Jani Nikula
  2015-12-14  9:40             ` Takashi Iwai
  1 sibling, 1 reply; 15+ messages in thread
From: Jani Nikula @ 2015-12-14  9:33 UTC (permalink / raw)
  To: Takashi Iwai, Libin Yang
  Cc: alsa-devel, intel-gfx, Dave Airlie, daniel.vetter

On Fri, 11 Dec 2015, Takashi Iwai <tiwai@suse.de> wrote:
> On Fri, 11 Dec 2015 07:07:53 +0100,
> Libin Yang wrote:
>> 
>> >>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
>> >>> index 9aa83e7..5ad2e66 100644
>> >>> --- a/drivers/gpu/drm/i915/intel_audio.c
>> >>> +++ b/drivers/gpu/drm/i915/intel_audio.c
>> >>> @@ -262,7 +262,8 @@ 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;
>
> The same check is missing in hsw_audio_codec_enable()?
>
>> >>>   	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>> >>>
>> >>> @@ -474,7 +475,8 @@ static void ilk_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;
>
> ... and missing for ilk_audio_codec_disable()?
>
>
>> >>>   	else
>> >>>   		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
>> >>> @@ -512,7 +514,8 @@ 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))
>
> IMO, it's better to have a macro to cover this two-line check instead
> of open-coding at each place.  We'll have 5 places in the end.

I don't think that's necessary. Only the hsw_* functions need it, the
other platforms (using the other functions) don't support DP MST.

BR,
Jani.


>
>
> thanks,
>
> Takashi

-- 
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] 15+ messages in thread

* Re: [PATCH V4 2/2] drm/i915: start adding dp mst audio
  2015-12-14  9:33           ` Jani Nikula
@ 2015-12-14  9:40             ` Takashi Iwai
  2015-12-14 10:34               ` Jani Nikula
  0 siblings, 1 reply; 15+ messages in thread
From: Takashi Iwai @ 2015-12-14  9:40 UTC (permalink / raw)
  To: Jani Nikula; +Cc: alsa-devel, intel-gfx, Dave Airlie, daniel.vetter, Libin Yang

On Mon, 14 Dec 2015 10:33:44 +0100,
Jani Nikula wrote:
> 
> On Fri, 11 Dec 2015, Takashi Iwai <tiwai@suse.de> wrote:
> > On Fri, 11 Dec 2015 07:07:53 +0100,
> > Libin Yang wrote:
> >> 
> >> >>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> >> >>> index 9aa83e7..5ad2e66 100644
> >> >>> --- a/drivers/gpu/drm/i915/intel_audio.c
> >> >>> +++ b/drivers/gpu/drm/i915/intel_audio.c
> >> >>> @@ -262,7 +262,8 @@ 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;
> >
> > The same check is missing in hsw_audio_codec_enable()?
> >
> >> >>>   	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> >> >>>
> >> >>> @@ -474,7 +475,8 @@ static void ilk_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;
> >
> > ... and missing for ilk_audio_codec_disable()?
> >
> >
> >> >>>   	else
> >> >>>   		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
> >> >>> @@ -512,7 +514,8 @@ 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))
> >
> > IMO, it's better to have a macro to cover this two-line check instead
> > of open-coding at each place.  We'll have 5 places in the end.
> 
> I don't think that's necessary. Only the hsw_* functions need it, the
> other platforms (using the other functions) don't support DP MST.

Then the patch changes the functions.  And still three places have
this check.


thanks,

Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH V4 2/2] drm/i915: start adding dp mst audio
  2015-12-14  9:40             ` Takashi Iwai
@ 2015-12-14 10:34               ` Jani Nikula
  2015-12-14 10:41                 ` Takashi Iwai
  0 siblings, 1 reply; 15+ messages in thread
From: Jani Nikula @ 2015-12-14 10:34 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: alsa-devel, intel-gfx, Dave Airlie, daniel.vetter, Libin Yang

On Mon, 14 Dec 2015, Takashi Iwai <tiwai@suse.de> wrote:
> On Mon, 14 Dec 2015 10:33:44 +0100,
> Jani Nikula wrote:
>> 
>> On Fri, 11 Dec 2015, Takashi Iwai <tiwai@suse.de> wrote:
>> > On Fri, 11 Dec 2015 07:07:53 +0100,
>> > Libin Yang wrote:
>> >> 
>> >> >>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
>> >> >>> index 9aa83e7..5ad2e66 100644
>> >> >>> --- a/drivers/gpu/drm/i915/intel_audio.c
>> >> >>> +++ b/drivers/gpu/drm/i915/intel_audio.c
>> >> >>> @@ -262,7 +262,8 @@ 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;
>> >
>> > The same check is missing in hsw_audio_codec_enable()?
>> >
>> >> >>>   	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>> >> >>>
>> >> >>> @@ -474,7 +475,8 @@ static void ilk_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;
>> >
>> > ... and missing for ilk_audio_codec_disable()?
>> >
>> >
>> >> >>>   	else
>> >> >>>   		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
>> >> >>> @@ -512,7 +514,8 @@ 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))
>> >
>> > IMO, it's better to have a macro to cover this two-line check instead
>> > of open-coding at each place.  We'll have 5 places in the end.
>> 
>> I don't think that's necessary. Only the hsw_* functions need it, the
>> other platforms (using the other functions) don't support DP MST.
>
> Then the patch changes the functions.  And still three places have
> this check.

Probably the mistake in the patch was to add the check to
ilk_audio_codec_enable when it should have been added to
hsw_audio_codec_enable.

BR,
Jani.



>
>
> thanks,
>
> Takashi

-- 
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] 15+ messages in thread

* Re: [PATCH V4 2/2] drm/i915: start adding dp mst audio
  2015-12-14 10:34               ` Jani Nikula
@ 2015-12-14 10:41                 ` Takashi Iwai
  0 siblings, 0 replies; 15+ messages in thread
From: Takashi Iwai @ 2015-12-14 10:41 UTC (permalink / raw)
  To: Jani Nikula; +Cc: alsa-devel, intel-gfx, Dave Airlie, daniel.vetter, Libin Yang

On Mon, 14 Dec 2015 11:34:53 +0100,
Jani Nikula wrote:
> 
> On Mon, 14 Dec 2015, Takashi Iwai <tiwai@suse.de> wrote:
> > On Mon, 14 Dec 2015 10:33:44 +0100,
> > Jani Nikula wrote:
> >> 
> >> On Fri, 11 Dec 2015, Takashi Iwai <tiwai@suse.de> wrote:
> >> > On Fri, 11 Dec 2015 07:07:53 +0100,
> >> > Libin Yang wrote:
> >> >> 
> >> >> >>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> >> >> >>> index 9aa83e7..5ad2e66 100644
> >> >> >>> --- a/drivers/gpu/drm/i915/intel_audio.c
> >> >> >>> +++ b/drivers/gpu/drm/i915/intel_audio.c
> >> >> >>> @@ -262,7 +262,8 @@ 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;
> >> >
> >> > The same check is missing in hsw_audio_codec_enable()?
> >> >
> >> >> >>>   	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> >> >> >>>
> >> >> >>> @@ -474,7 +475,8 @@ static void ilk_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;
> >> >
> >> > ... and missing for ilk_audio_codec_disable()?
> >> >
> >> >
> >> >> >>>   	else
> >> >> >>>   		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
> >> >> >>> @@ -512,7 +514,8 @@ 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))
> >> >
> >> > IMO, it's better to have a macro to cover this two-line check instead
> >> > of open-coding at each place.  We'll have 5 places in the end.
> >> 
> >> I don't think that's necessary. Only the hsw_* functions need it, the
> >> other platforms (using the other functions) don't support DP MST.
> >
> > Then the patch changes the functions.  And still three places have
> > this check.
> 
> Probably the mistake in the patch was to add the check to
> ilk_audio_codec_enable when it should have been added to
> hsw_audio_codec_enable.

Yeah, I wanted to write "the patch changes the wrong functions", too.


Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [alsa-devel] [PATCH V4 2/2] drm/i915: start adding dp mst audio
  2015-12-11 13:58           ` Takashi Iwai
@ 2015-12-21  8:31             ` Libin Yang
  0 siblings, 0 replies; 15+ messages in thread
From: Libin Yang @ 2015-12-21  8:31 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, intel-gfx, daniel.vetter, Dave Airlie


Sorry, I missed the emails.

On 12/11/2015 09:58 PM, Takashi Iwai wrote:
> On Fri, 11 Dec 2015 11:43:51 +0100,
> Takashi Iwai wrote:
>>
>> On Fri, 11 Dec 2015 07:07:53 +0100,
>> Libin Yang wrote:
>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
>>>>>> index 9aa83e7..5ad2e66 100644
>>>>>> --- a/drivers/gpu/drm/i915/intel_audio.c
>>>>>> +++ b/drivers/gpu/drm/i915/intel_audio.c
>>>>>> @@ -262,7 +262,8 @@ 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;
>>
>> The same check is missing in hsw_audio_codec_enable()?
>>
>>>>>>    	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>>>>>>
>>>>>> @@ -474,7 +475,8 @@ static void ilk_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;
>>
>> ... and missing for ilk_audio_codec_disable()?

Based on the discussion, I think you and Jani are right. It should be 
in hsw_audio_codec_enable() and not in ild_audio_codec_disable().

>>
>>
>>>>>>    	else
>>>>>>    		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
>>>>>> @@ -512,7 +514,8 @@ 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))
>>
>> IMO, it's better to have a macro to cover this two-line check instead
>> of open-coding at each place.  We'll have 5 places in the end.

OK, I will use a macro.

>
> Also, this patch still has an issue about the encoder type, namely, it
> passes intel_encoder from MST, where you can't apply
> enc_to_dig_port().  We need another help to get the digital port
> depending on the encoder type, e.g.
>
> static struct intel_digital_port *
> intel_encoder_to_dig_port(struct intel_encoder *intel_encoder)
> {
> 	struct drm_encoder *encoder = &intel_encoder->base;
>
> 	if (intel_encoder->type == INTEL_OUTPUT_DP_MST)
> 		return enc_to_mst(encoder)->primary;
> 	return enc_to_dig_port(encoder);
> }

Yes, I will fix it in the new version patch.

Best Regards,
Libin

>
>
> Takashi
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2015-12-21  8:31 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-02  6:09 [PATCH V4 1/2] dp/mst: add SDP stream support libin.yang
2015-12-02  6:09 ` [PATCH V4 2/2] drm/i915: start adding dp mst audio libin.yang
2015-12-08  8:01   ` Libin Yang
2015-12-10  9:02     ` Daniel Vetter
2015-12-11  6:07       ` Libin Yang
2015-12-11 10:43         ` Takashi Iwai
2015-12-11 13:58           ` Takashi Iwai
2015-12-21  8:31             ` [alsa-devel] " Libin Yang
2015-12-14  9:33           ` Jani Nikula
2015-12-14  9:40             ` Takashi Iwai
2015-12-14 10:34               ` Jani Nikula
2015-12-14 10:41                 ` Takashi Iwai
2015-12-11 14:05         ` Takashi Iwai
2015-12-13  8:27           ` Takashi Iwai
2015-12-11  6:09 ` [PATCH V4 1/2] dp/mst: add SDP stream support Libin Yang

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.