Re: [Intel-gfx] [PATCH] drm/i915: Fix VLV/CHV HDMI/DP audio enable
On Thu, 08 Dec 2022, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Despite what I claimed in commit c3c5dc1d9224
> ("drm/i915/audio: Do the vblank waits") the vblank
> interrupts are in fact not enabled yet when we do the
> audio enable sequence on VLV/CHV (all other platforms are
> fine).
>
> Reorder the enable sequence on VLV/CHV to match that of the
> other platforms so that the audio enable happens after the
> pipe has been enabled.
>
> Fixes: c3c5dc1d9224 ("drm/i915/audio: Do the vblank waits")
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/display/g4x_dp.c | 4 +--
> drivers/gpu/drm/i915/display/g4x_hdmi.c | 41 -
> 2 files changed, 29 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c
> b/drivers/gpu/drm/i915/display/g4x_dp.c
> index 3593938dcd87..24ef36ec2d3d 100644
> --- a/drivers/gpu/drm/i915/display/g4x_dp.c
> +++ b/drivers/gpu/drm/i915/display/g4x_dp.c
> @@ -673,8 +673,6 @@ static void intel_enable_dp(struct intel_atomic_state
> *state,
> intel_dp_pcon_dsc_configure(intel_dp, pipe_config);
> intel_dp_start_link_train(intel_dp, pipe_config);
> intel_dp_stop_link_train(intel_dp, pipe_config);
> -
> - intel_audio_codec_enable(encoder, pipe_config, conn_state);
> }
>
> static void g4x_enable_dp(struct intel_atomic_state *state,
> @@ -683,6 +681,7 @@ static void g4x_enable_dp(struct intel_atomic_state
> *state,
> const struct drm_connector_state *conn_state)
> {
> intel_enable_dp(state, encoder, pipe_config, conn_state);
> + intel_audio_codec_enable(encoder, pipe_config, conn_state);
> intel_edp_backlight_on(pipe_config, conn_state);
> }
>
> @@ -691,6 +690,7 @@ static void vlv_enable_dp(struct intel_atomic_state
> *state,
> const struct intel_crtc_state *pipe_config,
> const struct drm_connector_state *conn_state)
> {
> + intel_audio_codec_enable(encoder, pipe_config, conn_state);
> intel_edp_backlight_on(pipe_config, conn_state);
> }
>
> diff --git a/drivers/gpu/drm/i915/display/g4x_hdmi.c
> b/drivers/gpu/drm/i915/display/g4x_hdmi.c
> index 121caeaa409b..c3580d96765c 100644
> --- a/drivers/gpu/drm/i915/display/g4x_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/g4x_hdmi.c
> @@ -157,24 +157,32 @@ static void intel_hdmi_get_config(struct intel_encoder
> *encoder,
>_config->infoframes.hdmi);
> }
>
> +static void g4x_hdmi_enable_port(struct intel_encoder *encoder,
> + const struct intel_crtc_state *pipe_config)
> +{
> + struct drm_device *dev = encoder->base.dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
Could clean up some of the drm_device and dev_priv on top.
Reviewed-by: Jani Nikula
> + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> + u32 temp;
> +
> + temp = intel_de_read(dev_priv, intel_hdmi->hdmi_reg);
> +
> + temp |= SDVO_ENABLE;
> + if (pipe_config->has_audio)
> + temp |= HDMI_AUDIO_ENABLE;
> +
> + intel_de_write(dev_priv, intel_hdmi->hdmi_reg, temp);
> + intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg);
> +}
> +
> static void g4x_enable_hdmi(struct intel_atomic_state *state,
> struct intel_encoder *encoder,
> const struct intel_crtc_state *pipe_config,
> const struct drm_connector_state *conn_state)
> {
> - struct drm_device *dev = encoder->base.dev;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> - struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> - u32 temp;
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>
> - temp = intel_de_read(dev_priv, intel_hdmi->hdmi_reg);
> -
> - temp |= SDVO_ENABLE;
> - if (pipe_config->has_audio)
> - temp |= HDMI_AUDIO_ENABLE;
> -
> - intel_de_write(dev_priv, intel_hdmi->hdmi_reg, temp);
> - intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg);
> + g4x_hdmi_enable_port(encoder, pipe_config);
>
> drm_WARN_ON(_priv->drm, pipe_config->has_audio &&
> !pipe_config->has_hdmi_sink);
> @@ -294,6 +302,11 @@ static void vlv_enable_hdmi(struct intel_atomic_state
> *state,
> const struct intel_crtc_state *pipe_config,
> const struct drm_connector_state *conn_state)
> {
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +
> + drm_WARN_ON(_priv->drm, pipe_config->has_audio &&
> + !pipe_config->has_hdmi_sink);
> + intel_audio_codec_enable(encoder, pipe_config, conn_state);
> }
>
> static void intel_disable_hdmi(struct intel_atomic_state *state,
> @@ -415,7 +428,7 @@ static void vlv_hdmi_pre_enable(struct intel_atomic_state
> *state,
> pipe_config->has_infoframe,
>
[Intel-gfx] [PATCH] drm/i915: Fix VLV/CHV HDMI/DP audio enable
From: Ville Syrjälä
Despite what I claimed in commit c3c5dc1d9224
("drm/i915/audio: Do the vblank waits") the vblank
interrupts are in fact not enabled yet when we do the
audio enable sequence on VLV/CHV (all other platforms are
fine).
Reorder the enable sequence on VLV/CHV to match that of the
other platforms so that the audio enable happens after the
pipe has been enabled.
Fixes: c3c5dc1d9224 ("drm/i915/audio: Do the vblank waits")
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/display/g4x_dp.c | 4 +--
drivers/gpu/drm/i915/display/g4x_hdmi.c | 41 -
2 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c
b/drivers/gpu/drm/i915/display/g4x_dp.c
index 3593938dcd87..24ef36ec2d3d 100644
--- a/drivers/gpu/drm/i915/display/g4x_dp.c
+++ b/drivers/gpu/drm/i915/display/g4x_dp.c
@@ -673,8 +673,6 @@ static void intel_enable_dp(struct intel_atomic_state
*state,
intel_dp_pcon_dsc_configure(intel_dp, pipe_config);
intel_dp_start_link_train(intel_dp, pipe_config);
intel_dp_stop_link_train(intel_dp, pipe_config);
-
- intel_audio_codec_enable(encoder, pipe_config, conn_state);
}
static void g4x_enable_dp(struct intel_atomic_state *state,
@@ -683,6 +681,7 @@ static void g4x_enable_dp(struct intel_atomic_state *state,
const struct drm_connector_state *conn_state)
{
intel_enable_dp(state, encoder, pipe_config, conn_state);
+ intel_audio_codec_enable(encoder, pipe_config, conn_state);
intel_edp_backlight_on(pipe_config, conn_state);
}
@@ -691,6 +690,7 @@ static void vlv_enable_dp(struct intel_atomic_state *state,
const struct intel_crtc_state *pipe_config,
const struct drm_connector_state *conn_state)
{
+ intel_audio_codec_enable(encoder, pipe_config, conn_state);
intel_edp_backlight_on(pipe_config, conn_state);
}
diff --git a/drivers/gpu/drm/i915/display/g4x_hdmi.c
b/drivers/gpu/drm/i915/display/g4x_hdmi.c
index 121caeaa409b..c3580d96765c 100644
--- a/drivers/gpu/drm/i915/display/g4x_hdmi.c
+++ b/drivers/gpu/drm/i915/display/g4x_hdmi.c
@@ -157,24 +157,32 @@ static void intel_hdmi_get_config(struct intel_encoder
*encoder,
_config->infoframes.hdmi);
}
+static void g4x_hdmi_enable_port(struct intel_encoder *encoder,
+const struct intel_crtc_state *pipe_config)
+{
+ struct drm_device *dev = encoder->base.dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+ u32 temp;
+
+ temp = intel_de_read(dev_priv, intel_hdmi->hdmi_reg);
+
+ temp |= SDVO_ENABLE;
+ if (pipe_config->has_audio)
+ temp |= HDMI_AUDIO_ENABLE;
+
+ intel_de_write(dev_priv, intel_hdmi->hdmi_reg, temp);
+ intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg);
+}
+
static void g4x_enable_hdmi(struct intel_atomic_state *state,
struct intel_encoder *encoder,
const struct intel_crtc_state *pipe_config,
const struct drm_connector_state *conn_state)
{
- struct drm_device *dev = encoder->base.dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
- u32 temp;
+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
- temp = intel_de_read(dev_priv, intel_hdmi->hdmi_reg);
-
- temp |= SDVO_ENABLE;
- if (pipe_config->has_audio)
- temp |= HDMI_AUDIO_ENABLE;
-
- intel_de_write(dev_priv, intel_hdmi->hdmi_reg, temp);
- intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg);
+ g4x_hdmi_enable_port(encoder, pipe_config);
drm_WARN_ON(_priv->drm, pipe_config->has_audio &&
!pipe_config->has_hdmi_sink);
@@ -294,6 +302,11 @@ static void vlv_enable_hdmi(struct intel_atomic_state
*state,
const struct intel_crtc_state *pipe_config,
const struct drm_connector_state *conn_state)
{
+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+
+ drm_WARN_ON(_priv->drm, pipe_config->has_audio &&
+ !pipe_config->has_hdmi_sink);
+ intel_audio_codec_enable(encoder, pipe_config, conn_state);
}
static void intel_disable_hdmi(struct intel_atomic_state *state,
@@ -415,7 +428,7 @@ static void vlv_hdmi_pre_enable(struct intel_atomic_state
*state,
pipe_config->has_infoframe,
pipe_config, conn_state);
- g4x_enable_hdmi(state, encoder, pipe_config, conn_state);
+ g4x_hdmi_enable_port(encoder, pipe_config);
vlv_wait_port_ready(dev_priv, dig_port, 0x0);
}
@@ -492,7 +505,7 @@ static void chv_hdmi_pre_enable(struct
