* [PATCH v3] drm/i915: start adding dp mst audio
@ 2016-11-14 5:32 libin.yang
2016-11-14 5:40 ` Yang, Libin
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: libin.yang @ 2016-11-14 5:32 UTC (permalink / raw)
To: intel-gfx, jani.nikula, ville.syrjala, daniel.vetter, tiwai
Cc: Libin Yang, Dhinakaran Pandiyan, Rodrigo Vivi
From: Libin Yang <libin.yang@linux.intel.com>
(This patch is developed by Dave Airlie <airlied@redhat.com> originally)
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.
v2:
Rebased.
v3:
refine to support updated intel_audio_codec_enable()
Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Lyude <cpaul@redhat.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1474334681-22690-6-git-send-email-dhinakaran.pandiyan@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 19 ++++++++++++++++++-
drivers/gpu/drm/i915/intel_ddi.c | 20 +++++++++++++++-----
drivers/gpu/drm/i915/intel_dp_mst.c | 18 ++++++++++++++++++
drivers/gpu/drm/i915/intel_drv.h | 2 ++
4 files changed, 53 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index bce3880..3442381 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2893,6 +2893,20 @@ static void intel_dp_info(struct seq_file *m,
&intel_dp->aux);
}
+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)
{
@@ -2935,7 +2949,10 @@ static void intel_connector_info(struct seq_file *m,
switch (connector->connector_type) {
case DRM_MODE_CONNECTOR_DisplayPort:
case DRM_MODE_CONNECTOR_eDP:
- intel_dp_info(m, intel_connector);
+ if (intel_encoder->type == INTEL_OUTPUT_DP_MST)
+ intel_dp_mst_info(m, intel_connector);
+ else
+ intel_dp_info(m, intel_connector);
break;
case DRM_MODE_CONNECTOR_LVDS:
if (intel_encoder->type == INTEL_OUTPUT_LVDS)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 0ad4e16..b1e3442 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1951,6 +1951,19 @@ void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
udelay(600);
}
+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)
{
@@ -2016,11 +2029,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 3ffbd69..96ff6a9 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -37,6 +37,8 @@ static bool intel_dp_mst_compute_config(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 intel_connector *connector =
+ to_intel_connector(conn_state->connector);
struct drm_atomic_state *state;
int bpp;
int lane_count, slots;
@@ -59,6 +61,8 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
state = pipe_config->base.state;
+ if (drm_dp_mst_port_has_audio(&intel_dp->mst_mgr, connector->port))
+ pipe_config->has_audio = true;
mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);
pipe_config->pbn = mst_pbn;
@@ -84,6 +88,7 @@ static void intel_mst_disable_dp(struct intel_encoder *encoder,
struct intel_dp *intel_dp = &intel_dig_port->dp;
struct intel_connector *connector =
to_intel_connector(old_conn_state->connector);
+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
int ret;
DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
@@ -94,6 +99,10 @@ static void intel_mst_disable_dp(struct intel_encoder *encoder,
if (ret) {
DRM_ERROR("failed to update payload %d\n", ret);
}
+ if (old_crtc_state->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,
@@ -206,6 +215,12 @@ 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 (pipe_config->has_audio) {
+ DRM_DEBUG_DRIVER("Enabling DP audio on pipe %c\n",
+ pipe_name(intel_mst->pipe));
+ intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
+ intel_audio_codec_enable(encoder, pipe_config, conn_state);
+ }
}
static bool intel_dp_mst_enc_get_hw_state(struct intel_encoder *encoder,
@@ -228,6 +243,9 @@ static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
u32 temp, flags = 0;
+ 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 003afb8..8c9aa0a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1166,6 +1166,8 @@ bool intel_ddi_pll_select(struct intel_crtc *crtc,
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);
+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 *
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] drm/i915: start adding dp mst audio
2016-11-14 5:32 [PATCH v3] drm/i915: start adding dp mst audio libin.yang
@ 2016-11-14 5:40 ` Yang, Libin
2016-11-17 5:10 ` Yang, Libin
2016-11-14 6:15 ` ✓ Fi.CI.BAT: success for " Patchwork
2016-11-25 14:22 ` [PATCH v3] " Jani Nikula
2 siblings, 1 reply; 6+ messages in thread
From: Yang, Libin @ 2016-11-14 5:40 UTC (permalink / raw)
To: intel-gfx, jani.nikula, ville.syrjala, Vetter, Daniel, tiwai, Kp, Jeeja
Cc: Libin Yang, Pandiyan, Dhinakaran, Vivi, Rodrigo
As the dp-mst flicker issue has been fixed by Dhinakaran, let's resend this patch to gfx driver to support DP-MST audio.
This patch is also refined to support the new intel_audio_codec_enable() parameters.
Regards,
Libin
> -----Original Message-----
> From: Yang, Libin
> Sent: Monday, November 14, 2016 1:32 PM
> To: intel-gfx@lists.freedesktop.org; jani.nikula@linux.intel.com;
> ville.syrjala@linux.intel.com; Vetter, Daniel <daniel.vetter@intel.com>;
> tiwai@suse.de
> Cc: Yang, Libin <libin.yang@intel.com>; Libin Yang
> <libin.yang@linux.intel.com>; Pandiyan, Dhinakaran
> <dhinakaran.pandiyan@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Subject: [PATCH v3] drm/i915: start adding dp mst audio
>
> From: Libin Yang <libin.yang@linux.intel.com>
>
> (This patch is developed by Dave Airlie <airlied@redhat.com> originally)
>
> 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.
>
> v2:
> Rebased.
> v3:
> refine to support updated intel_audio_codec_enable()
>
> Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Reviewed-by: Lyude <cpaul@redhat.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Link: http://patchwork.freedesktop.org/patch/msgid/1474334681-22690-6-
> git-send-email-dhinakaran.pandiyan@intel.com
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 19 ++++++++++++++++++-
> drivers/gpu/drm/i915/intel_ddi.c | 20 +++++++++++++++-----
> drivers/gpu/drm/i915/intel_dp_mst.c | 18 ++++++++++++++++++
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> 4 files changed, 53 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index bce3880..3442381 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2893,6 +2893,20 @@ static void intel_dp_info(struct seq_file *m,
> &intel_dp->aux);
> }
>
> +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) { @@ -
> 2935,7 +2949,10 @@ static void intel_connector_info(struct seq_file *m,
> switch (connector->connector_type) {
> case DRM_MODE_CONNECTOR_DisplayPort:
> case DRM_MODE_CONNECTOR_eDP:
> - intel_dp_info(m, intel_connector);
> + if (intel_encoder->type == INTEL_OUTPUT_DP_MST)
> + intel_dp_mst_info(m, intel_connector);
> + else
> + intel_dp_info(m, intel_connector);
> break;
> case DRM_MODE_CONNECTOR_LVDS:
> if (intel_encoder->type == INTEL_OUTPUT_LVDS) diff --git
> a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 0ad4e16..b1e3442 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1951,6 +1951,19 @@ void intel_ddi_prepare_link_retrain(struct
> intel_dp *intel_dp)
> udelay(600);
> }
>
> +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) { @@ -2016,11
> +2029,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 3ffbd69..96ff6a9 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -37,6 +37,8 @@ static bool intel_dp_mst_compute_config(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 intel_connector *connector =
> + to_intel_connector(conn_state->connector);
> struct drm_atomic_state *state;
> int bpp;
> int lane_count, slots;
> @@ -59,6 +61,8 @@ static bool intel_dp_mst_compute_config(struct
> intel_encoder *encoder,
>
> state = pipe_config->base.state;
>
> + if (drm_dp_mst_port_has_audio(&intel_dp->mst_mgr, connector-
> >port))
> + pipe_config->has_audio = true;
> mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock,
> bpp);
>
> pipe_config->pbn = mst_pbn;
> @@ -84,6 +88,7 @@ static void intel_mst_disable_dp(struct intel_encoder
> *encoder,
> struct intel_dp *intel_dp = &intel_dig_port->dp;
> struct intel_connector *connector =
> to_intel_connector(old_conn_state->connector);
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> int ret;
>
> DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links); @@ -94,6
> +99,10 @@ static void intel_mst_disable_dp(struct intel_encoder *encoder,
> if (ret) {
> DRM_ERROR("failed to update payload %d\n", ret);
> }
> + if (old_crtc_state->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, @@ -
> 206,6 +215,12 @@ 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 (pipe_config->has_audio) {
> + DRM_DEBUG_DRIVER("Enabling DP audio on pipe %c\n",
> + pipe_name(intel_mst->pipe));
> + intel_display_power_get(dev_priv,
> POWER_DOMAIN_AUDIO);
> + intel_audio_codec_enable(encoder, pipe_config, conn_state);
> + }
> }
>
> static bool intel_dp_mst_enc_get_hw_state(struct intel_encoder *encoder,
> @@ -228,6 +243,9 @@ static void intel_dp_mst_enc_get_config(struct
> intel_encoder *encoder,
> enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> u32 temp, flags = 0;
>
> + 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 003afb8..8c9aa0a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1166,6 +1166,8 @@ bool intel_ddi_pll_select(struct intel_crtc *crtc,
> 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);
> +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 *
> --
> 2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: start adding dp mst audio
2016-11-14 5:32 [PATCH v3] drm/i915: start adding dp mst audio libin.yang
2016-11-14 5:40 ` Yang, Libin
@ 2016-11-14 6:15 ` Patchwork
2016-11-25 14:22 ` [PATCH v3] " Jani Nikula
2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2016-11-14 6:15 UTC (permalink / raw)
To: libin.yang; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: start adding dp mst audio
URL : https://patchwork.freedesktop.org/series/15245/
State : success
== Summary ==
Series 15245v1 drm/i915: start adding dp mst audio
https://patchwork.freedesktop.org/api/1.0/series/15245/revisions/1/mbox/
fi-bdw-5557u total:244 pass:229 dwarn:0 dfail:0 fail:0 skip:15
fi-bsw-n3050 total:244 pass:204 dwarn:0 dfail:0 fail:0 skip:40
fi-bxt-t5700 total:244 pass:216 dwarn:0 dfail:0 fail:0 skip:28
fi-byt-j1900 total:244 pass:216 dwarn:0 dfail:0 fail:0 skip:28
fi-byt-n2820 total:244 pass:212 dwarn:0 dfail:0 fail:0 skip:32
fi-hsw-4770 total:244 pass:224 dwarn:0 dfail:0 fail:0 skip:20
fi-hsw-4770r total:244 pass:224 dwarn:0 dfail:0 fail:0 skip:20
fi-ilk-650 total:244 pass:191 dwarn:0 dfail:0 fail:0 skip:53
fi-ivb-3520m total:244 pass:222 dwarn:0 dfail:0 fail:0 skip:22
fi-ivb-3770 total:244 pass:222 dwarn:0 dfail:0 fail:0 skip:22
fi-skl-6260u total:244 pass:230 dwarn:0 dfail:0 fail:0 skip:14
fi-skl-6700hq total:244 pass:223 dwarn:0 dfail:0 fail:0 skip:21
fi-skl-6700k total:244 pass:222 dwarn:1 dfail:0 fail:0 skip:21
fi-snb-2520m total:244 pass:212 dwarn:0 dfail:0 fail:0 skip:32
fi-snb-2600 total:244 pass:211 dwarn:0 dfail:0 fail:0 skip:33
48ac4d084b5911d7c88a6b54175731d6a87f1769 drm-intel-nightly: 2016y-11m-13d-16h-50m-41s UTC integration manifest
46ec54b drm/i915: start adding dp mst audio
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_2975/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] drm/i915: start adding dp mst audio
2016-11-14 5:40 ` Yang, Libin
@ 2016-11-17 5:10 ` Yang, Libin
0 siblings, 0 replies; 6+ messages in thread
From: Yang, Libin @ 2016-11-17 5:10 UTC (permalink / raw)
To: Yang, Libin, intel-gfx, jani.nikula, ville.syrjala, Vetter,
Daniel, tiwai, Kp, Jeeja
Cc: Libin Yang, Pandiyan, Dhinakaran, Vivi, Rodrigo
Hi all,
Could anyone help review the patches? Thanks.
Regards,
Libin
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Yang, Libin
> Sent: Monday, November 14, 2016 1:41 PM
> To: intel-gfx@lists.freedesktop.org; jani.nikula@linux.intel.com;
> ville.syrjala@linux.intel.com; Vetter, Daniel <daniel.vetter@intel.com>;
> tiwai@suse.de; Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>;
> Kp, Jeeja <jeeja.kp@intel.com>
> Cc: Libin Yang <libin.yang@linux.intel.com>; Pandiyan, Dhinakaran
> <dhinakaran.pandiyan@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Subject: Re: [Intel-gfx] [PATCH v3] drm/i915: start adding dp mst audio
>
> As the dp-mst flicker issue has been fixed by Dhinakaran, let's resend this
> patch to gfx driver to support DP-MST audio.
>
> This patch is also refined to support the new intel_audio_codec_enable()
> parameters.
>
> Regards,
> Libin
>
>
> > -----Original Message-----
> > From: Yang, Libin
> > Sent: Monday, November 14, 2016 1:32 PM
> > To: intel-gfx@lists.freedesktop.org; jani.nikula@linux.intel.com;
> > ville.syrjala@linux.intel.com; Vetter, Daniel
> > <daniel.vetter@intel.com>; tiwai@suse.de
> > Cc: Yang, Libin <libin.yang@intel.com>; Libin Yang
> > <libin.yang@linux.intel.com>; Pandiyan, Dhinakaran
> > <dhinakaran.pandiyan@intel.com>; Vivi, Rodrigo
> > <rodrigo.vivi@intel.com>
> > Subject: [PATCH v3] drm/i915: start adding dp mst audio
> >
> > From: Libin Yang <libin.yang@linux.intel.com>
> >
> > (This patch is developed by Dave Airlie <airlied@redhat.com>
> > originally)
> >
> > 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.
> >
> > v2:
> > Rebased.
> > v3:
> > refine to support updated intel_audio_codec_enable()
> >
> > Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Reviewed-by: Lyude <cpaul@redhat.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Link: http://patchwork.freedesktop.org/patch/msgid/1474334681-22690-6-
> > git-send-email-dhinakaran.pandiyan@intel.com
> > ---
> > drivers/gpu/drm/i915/i915_debugfs.c | 19 ++++++++++++++++++-
> > drivers/gpu/drm/i915/intel_ddi.c | 20 +++++++++++++++-----
> > drivers/gpu/drm/i915/intel_dp_mst.c | 18 ++++++++++++++++++
> > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > 4 files changed, 53 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index bce3880..3442381 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2893,6 +2893,20 @@ static void intel_dp_info(struct seq_file *m,
> > &intel_dp->aux);
> > }
> >
> > +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) { @@ -
> > 2935,7 +2949,10 @@ static void intel_connector_info(struct seq_file *m,
> > switch (connector->connector_type) {
> > case DRM_MODE_CONNECTOR_DisplayPort:
> > case DRM_MODE_CONNECTOR_eDP:
> > - intel_dp_info(m, intel_connector);
> > + if (intel_encoder->type == INTEL_OUTPUT_DP_MST)
> > + intel_dp_mst_info(m, intel_connector);
> > + else
> > + intel_dp_info(m, intel_connector);
> > break;
> > case DRM_MODE_CONNECTOR_LVDS:
> > if (intel_encoder->type == INTEL_OUTPUT_LVDS) diff --git
> > a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 0ad4e16..b1e3442 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1951,6 +1951,19 @@ void intel_ddi_prepare_link_retrain(struct
> > intel_dp *intel_dp)
> > udelay(600);
> > }
> >
> > +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) { @@ -2016,11
> > +2029,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 3ffbd69..96ff6a9 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > @@ -37,6 +37,8 @@ static bool intel_dp_mst_compute_config(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 intel_connector *connector =
> > + to_intel_connector(conn_state->connector);
> > struct drm_atomic_state *state;
> > int bpp;
> > int lane_count, slots;
> > @@ -59,6 +61,8 @@ static bool intel_dp_mst_compute_config(struct
> > intel_encoder *encoder,
> >
> > state = pipe_config->base.state;
> >
> > + if (drm_dp_mst_port_has_audio(&intel_dp->mst_mgr, connector-
> > >port))
> > + pipe_config->has_audio = true;
> > mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock,
> > bpp);
> >
> > pipe_config->pbn = mst_pbn;
> > @@ -84,6 +88,7 @@ static void intel_mst_disable_dp(struct
> > intel_encoder *encoder,
> > struct intel_dp *intel_dp = &intel_dig_port->dp;
> > struct intel_connector *connector =
> > to_intel_connector(old_conn_state->connector);
> > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > int ret;
> >
> > DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links); @@ -94,6
> > +99,10 @@ static void intel_mst_disable_dp(struct intel_encoder
> > +*encoder,
> > if (ret) {
> > DRM_ERROR("failed to update payload %d\n", ret);
> > }
> > + if (old_crtc_state->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,
> > @@ -
> > 206,6 +215,12 @@ 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 (pipe_config->has_audio) {
> > + DRM_DEBUG_DRIVER("Enabling DP audio on pipe %c\n",
> > + pipe_name(intel_mst->pipe));
> > + intel_display_power_get(dev_priv,
> > POWER_DOMAIN_AUDIO);
> > + intel_audio_codec_enable(encoder, pipe_config, conn_state);
> > + }
> > }
> >
> > static bool intel_dp_mst_enc_get_hw_state(struct intel_encoder
> > *encoder, @@ -228,6 +243,9 @@ static void
> > intel_dp_mst_enc_get_config(struct
> > intel_encoder *encoder,
> > enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> > u32 temp, flags = 0;
> >
> > + 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 003afb8..8c9aa0a 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1166,6 +1166,8 @@ bool intel_ddi_pll_select(struct intel_crtc
> > *crtc, 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);
> > +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 *
> > --
> > 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] drm/i915: start adding dp mst audio
2016-11-14 5:32 [PATCH v3] drm/i915: start adding dp mst audio libin.yang
2016-11-14 5:40 ` Yang, Libin
2016-11-14 6:15 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2016-11-25 14:22 ` Jani Nikula
2016-11-27 1:33 ` Yang, Libin
2 siblings, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2016-11-25 14:22 UTC (permalink / raw)
To: libin.yang, intel-gfx, ville.syrjala, daniel.vetter, tiwai
Cc: Libin Yang, Dhinakaran Pandiyan, Rodrigo Vivi
On Mon, 14 Nov 2016, libin.yang@intel.com wrote:
> From: Libin Yang <libin.yang@linux.intel.com>
>
> (This patch is developed by Dave Airlie <airlied@redhat.com> originally)
>
> 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.
>
> v2:
> Rebased.
> v3:
> refine to support updated intel_audio_codec_enable()
I view this patch different from usual because this regressed and was
reverted before. Please split this up to three patches:
1) Do intel_dp_mst_info() in i915_debugfs.c. The information printed
looks at the sink, not pipe config, so this is not dependent on the
rest.
2) Abstract intel_ddi_is_audio_enabled(). It's a nice clean
non-functional change.
3) The rest. The last patch *must* reference the original commit and the
revert:
3708d5e082c3 ("drm/i915: start adding dp mst audio")
be754b101f70 ("Revert "drm/i915: start adding dp mst audio"")
Please rename the patch title to something like, "enable dp mst
audio" to distinguish.
If the split had been done originally, we would only have reverted part
3, not all of it. Please do it now.
More comments inline.
>
> Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Reviewed-by: Lyude <cpaul@redhat.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Link: http://patchwork.freedesktop.org/patch/msgid/1474334681-22690-6-git-send-email-dhinakaran.pandiyan@intel.com
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 19 ++++++++++++++++++-
> drivers/gpu/drm/i915/intel_ddi.c | 20 +++++++++++++++-----
> drivers/gpu/drm/i915/intel_dp_mst.c | 18 ++++++++++++++++++
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> 4 files changed, 53 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index bce3880..3442381 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2893,6 +2893,20 @@ static void intel_dp_info(struct seq_file *m,
> &intel_dp->aux);
> }
>
> +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)
> {
> @@ -2935,7 +2949,10 @@ static void intel_connector_info(struct seq_file *m,
> switch (connector->connector_type) {
> case DRM_MODE_CONNECTOR_DisplayPort:
> case DRM_MODE_CONNECTOR_eDP:
> - intel_dp_info(m, intel_connector);
> + if (intel_encoder->type == INTEL_OUTPUT_DP_MST)
> + intel_dp_mst_info(m, intel_connector);
> + else
> + intel_dp_info(m, intel_connector);
The above two hunks make up patch 1/3.
> break;
> case DRM_MODE_CONNECTOR_LVDS:
> if (intel_encoder->type == INTEL_OUTPUT_LVDS)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 0ad4e16..b1e3442 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1951,6 +1951,19 @@ void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
> udelay(600);
> }
>
> +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)
> {
> @@ -2016,11 +2029,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);
The above two hunks + declaration in intel_drv.h make up patch 2/3.
>
> 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 3ffbd69..96ff6a9 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -37,6 +37,8 @@ static bool intel_dp_mst_compute_config(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 intel_connector *connector =
> + to_intel_connector(conn_state->connector);
> struct drm_atomic_state *state;
> int bpp;
> int lane_count, slots;
> @@ -59,6 +61,8 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>
> state = pipe_config->base.state;
>
> + if (drm_dp_mst_port_has_audio(&intel_dp->mst_mgr, connector->port))
> + pipe_config->has_audio = true;
> mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);
>
> pipe_config->pbn = mst_pbn;
> @@ -84,6 +88,7 @@ static void intel_mst_disable_dp(struct intel_encoder *encoder,
> struct intel_dp *intel_dp = &intel_dig_port->dp;
> struct intel_connector *connector =
> to_intel_connector(old_conn_state->connector);
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> int ret;
>
> DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
> @@ -94,6 +99,10 @@ static void intel_mst_disable_dp(struct intel_encoder *encoder,
> if (ret) {
> DRM_ERROR("failed to update payload %d\n", ret);
> }
> + if (old_crtc_state->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,
> @@ -206,6 +215,12 @@ 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 (pipe_config->has_audio) {
> + DRM_DEBUG_DRIVER("Enabling DP audio on pipe %c\n",
> + pipe_name(intel_mst->pipe));
Not thrilled about the debug logging here. Is the information in
intel_audio_codec_enable() not enough? Can that be amended instead?
BR,
Jani.
> + intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
> + intel_audio_codec_enable(encoder, pipe_config, conn_state);
> + }
> }
>
> static bool intel_dp_mst_enc_get_hw_state(struct intel_encoder *encoder,
> @@ -228,6 +243,9 @@ static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
> enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> u32 temp, flags = 0;
>
> + 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 003afb8..8c9aa0a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1166,6 +1166,8 @@ bool intel_ddi_pll_select(struct intel_crtc *crtc,
> 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);
> +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 *
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] drm/i915: start adding dp mst audio
2016-11-25 14:22 ` [PATCH v3] " Jani Nikula
@ 2016-11-27 1:33 ` Yang, Libin
0 siblings, 0 replies; 6+ messages in thread
From: Yang, Libin @ 2016-11-27 1:33 UTC (permalink / raw)
To: Jani Nikula, intel-gfx, ville.syrjala, Vetter, Daniel, tiwai
Cc: Libin Yang, Pandiyan, Dhinakaran, Vivi, Rodrigo
Hi Jani,
Thanks for review. I will split this patch.
Regards,
Libin
> -----Original Message-----
> From: Jani Nikula [mailto:jani.nikula@linux.intel.com]
> Sent: Friday, November 25, 2016 10:23 PM
> To: Yang, Libin <libin.yang@intel.com>; intel-gfx@lists.freedesktop.org;
> ville.syrjala@linux.intel.com; Vetter, Daniel <daniel.vetter@intel.com>;
> tiwai@suse.de
> Cc: Yang, Libin <libin.yang@intel.com>; Libin Yang
> <libin.yang@linux.intel.com>; Pandiyan, Dhinakaran
> <dhinakaran.pandiyan@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Subject: Re: [PATCH v3] drm/i915: start adding dp mst audio
>
> On Mon, 14 Nov 2016, libin.yang@intel.com wrote:
> > From: Libin Yang <libin.yang@linux.intel.com>
> >
> > (This patch is developed by Dave Airlie <airlied@redhat.com>
> > originally)
> >
> > 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.
> >
> > v2:
> > Rebased.
> > v3:
> > refine to support updated intel_audio_codec_enable()
>
> I view this patch different from usual because this regressed and was
> reverted before. Please split this up to three patches:
>
> 1) Do intel_dp_mst_info() in i915_debugfs.c. The information printed
> looks at the sink, not pipe config, so this is not dependent on the
> rest.
>
> 2) Abstract intel_ddi_is_audio_enabled(). It's a nice clean
> non-functional change.
>
> 3) The rest. The last patch *must* reference the original commit and the
> revert:
>
> 3708d5e082c3 ("drm/i915: start adding dp mst audio")
> be754b101f70 ("Revert "drm/i915: start adding dp mst audio"")
>
> Please rename the patch title to something like, "enable dp mst
> audio" to distinguish.
>
> If the split had been done originally, we would only have reverted part 3, not
> all of it. Please do it now.
>
> More comments inline.
>
> >
> > Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Reviewed-by: Lyude <cpaul@redhat.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Link:
> > http://patchwork.freedesktop.org/patch/msgid/1474334681-22690-6-git-se
> > nd-email-dhinakaran.pandiyan@intel.com
> > ---
> > drivers/gpu/drm/i915/i915_debugfs.c | 19 ++++++++++++++++++-
> > drivers/gpu/drm/i915/intel_ddi.c | 20 +++++++++++++++-----
> > drivers/gpu/drm/i915/intel_dp_mst.c | 18 ++++++++++++++++++
> > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > 4 files changed, 53 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index bce3880..3442381 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2893,6 +2893,20 @@ static void intel_dp_info(struct seq_file *m,
> > &intel_dp->aux);
> > }
> >
> > +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) { @@ -
> 2935,7
> > +2949,10 @@ static void intel_connector_info(struct seq_file *m,
> > switch (connector->connector_type) {
> > case DRM_MODE_CONNECTOR_DisplayPort:
> > case DRM_MODE_CONNECTOR_eDP:
> > - intel_dp_info(m, intel_connector);
> > + if (intel_encoder->type == INTEL_OUTPUT_DP_MST)
> > + intel_dp_mst_info(m, intel_connector);
> > + else
> > + intel_dp_info(m, intel_connector);
>
> The above two hunks make up patch 1/3.
>
> > break;
> > case DRM_MODE_CONNECTOR_LVDS:
> > if (intel_encoder->type == INTEL_OUTPUT_LVDS) diff --git
> > a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 0ad4e16..b1e3442 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1951,6 +1951,19 @@ void intel_ddi_prepare_link_retrain(struct
> intel_dp *intel_dp)
> > udelay(600);
> > }
> >
> > +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) { @@ -2016,11
> +2029,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);
>
> The above two hunks + declaration in intel_drv.h make up patch 2/3.
>
> >
> > 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 3ffbd69..96ff6a9 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > @@ -37,6 +37,8 @@ static bool intel_dp_mst_compute_config(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 intel_connector *connector =
> > + to_intel_connector(conn_state->connector);
> > struct drm_atomic_state *state;
> > int bpp;
> > int lane_count, slots;
> > @@ -59,6 +61,8 @@ static bool intel_dp_mst_compute_config(struct
> > intel_encoder *encoder,
> >
> > state = pipe_config->base.state;
> >
> > + if (drm_dp_mst_port_has_audio(&intel_dp->mst_mgr, connector-
> >port))
> > + pipe_config->has_audio = true;
> > mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock,
> bpp);
> >
> > pipe_config->pbn = mst_pbn;
> > @@ -84,6 +88,7 @@ static void intel_mst_disable_dp(struct intel_encoder
> *encoder,
> > struct intel_dp *intel_dp = &intel_dig_port->dp;
> > struct intel_connector *connector =
> > to_intel_connector(old_conn_state->connector);
> > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > int ret;
> >
> > DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links); @@ -94,6
> +99,10
> > @@ static void intel_mst_disable_dp(struct intel_encoder *encoder,
> > if (ret) {
> > DRM_ERROR("failed to update payload %d\n", ret);
> > }
> > + if (old_crtc_state->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,
> > @@ -206,6 +215,12 @@ 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 (pipe_config->has_audio) {
> > + DRM_DEBUG_DRIVER("Enabling DP audio on pipe %c\n",
> > + pipe_name(intel_mst->pipe));
>
> Not thrilled about the debug logging here. Is the information in
> intel_audio_codec_enable() not enough? Can that be amended instead?
>
> BR,
> Jani.
>
> > + intel_display_power_get(dev_priv,
> POWER_DOMAIN_AUDIO);
> > + intel_audio_codec_enable(encoder, pipe_config, conn_state);
> > + }
> > }
> >
> > static bool intel_dp_mst_enc_get_hw_state(struct intel_encoder
> > *encoder, @@ -228,6 +243,9 @@ static void
> intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
> > enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> > u32 temp, flags = 0;
> >
> > + 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 003afb8..8c9aa0a 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1166,6 +1166,8 @@ bool intel_ddi_pll_select(struct intel_crtc
> > *crtc, 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);
> > +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 *
>
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-11-27 1:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-14 5:32 [PATCH v3] drm/i915: start adding dp mst audio libin.yang
2016-11-14 5:40 ` Yang, Libin
2016-11-17 5:10 ` Yang, Libin
2016-11-14 6:15 ` ✓ Fi.CI.BAT: success for " Patchwork
2016-11-25 14:22 ` [PATCH v3] " Jani Nikula
2016-11-27 1:33 ` Yang, Libin
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.