From: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
To: "José Roberto de Souza" <jose.souza@intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Disable audio, DRRS and PSR before planes
Date: Tue, 27 Jul 2021 17:36:27 +0300 [thread overview]
Message-ID: <133203bd-66eb-7a2b-79da-8abf8953774f@intel.com> (raw)
In-Reply-To: <20210726181559.80855-1-jose.souza@intel.com>
Looks good to me.
Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
On 7/26/21 9:15 PM, José Roberto de Souza wrote:
> HDMI and DisplayPort sequences states that audio and PSR should be
> disabled before planes are disabled.
> Not following it did not caused any problems up to Alderlake-P but
> for this platform it causes underruns during the PSR2 disable
> sequence.
>
> Specification don't mention that DRRS should be disabled before planes
> but it looks safer to switch back to the default refresh rate before
> following with the rest of the pipe disable sequence.
>
> BSpec: 49191
> BSpec: 49190
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 30 ++++++++++++-------
> drivers/gpu/drm/i915/display/intel_display.c | 24 +++++++++++++++
> .../drm/i915/display/intel_display_types.h | 4 +++
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 14 +++++++--
> 4 files changed, 59 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 26a3aa73fcc43..061a663f43b84 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3204,12 +3204,6 @@ static void intel_disable_ddi_dp(struct intel_atomic_state *state,
>
> intel_dp->link_trained = false;
>
> - if (old_crtc_state->has_audio)
> - intel_audio_codec_disable(encoder,
> - old_crtc_state, old_conn_state);
> -
> - intel_edp_drrs_disable(intel_dp, old_crtc_state);
> - intel_psr_disable(intel_dp, old_crtc_state);
> intel_edp_backlight_off(old_conn_state);
> /* Disable the decompression in DP Sink */
> intel_dp_sink_set_decompression_state(intel_dp, old_crtc_state,
> @@ -3227,10 +3221,6 @@ static void intel_disable_ddi_hdmi(struct intel_atomic_state *state,
> struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> struct drm_connector *connector = old_conn_state->connector;
>
> - if (old_crtc_state->has_audio)
> - intel_audio_codec_disable(encoder,
> - old_crtc_state, old_conn_state);
> -
> if (!intel_hdmi_handle_sink_scrambling(encoder, connector,
> false, false))
> drm_dbg_kms(&i915->drm,
> @@ -3238,6 +3228,25 @@ static void intel_disable_ddi_hdmi(struct intel_atomic_state *state,
> connector->base.id, connector->name);
> }
>
> +static void intel_pre_disable_ddi(struct intel_atomic_state *state,
> + struct intel_encoder *encoder,
> + const struct intel_crtc_state *old_crtc_state,
> + const struct drm_connector_state *old_conn_state)
> +{
> + struct intel_dp *intel_dp;
> +
> + if (old_crtc_state->has_audio)
> + intel_audio_codec_disable(encoder, old_crtc_state,
> + old_conn_state);
> +
> + if (intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_HDMI))
> + return;
> +
> + intel_dp = enc_to_intel_dp(encoder);
> + intel_edp_drrs_disable(intel_dp, old_crtc_state);
> + intel_psr_disable(intel_dp, old_crtc_state);
> +}
> +
> static void intel_disable_ddi(struct intel_atomic_state *state,
> struct intel_encoder *encoder,
> const struct intel_crtc_state *old_crtc_state,
> @@ -4590,6 +4599,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> encoder->enable = intel_enable_ddi;
> encoder->pre_pll_enable = intel_ddi_pre_pll_enable;
> encoder->pre_enable = intel_ddi_pre_enable;
> + encoder->pre_disable = intel_pre_disable_ddi;
> encoder->disable = intel_disable_ddi;
> encoder->post_disable = intel_ddi_post_disable;
> encoder->update_pipe = intel_ddi_update_pipe;
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index bb0aebcc3ecd3..cf58df9132748 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -3195,6 +3195,28 @@ static void intel_encoders_enable(struct intel_atomic_state *state,
> }
> }
>
> +static void intel_encoders_pre_disable(struct intel_atomic_state *state,
> + struct intel_crtc *crtc)
> +{
> + const struct intel_crtc_state *old_crtc_state =
> + intel_atomic_get_old_crtc_state(state, crtc);
> + const struct drm_connector_state *old_conn_state;
> + struct drm_connector *conn;
> + int i;
> +
> + for_each_old_connector_in_state(&state->base, conn, old_conn_state, i) {
> + struct intel_encoder *encoder =
> + to_intel_encoder(old_conn_state->best_encoder);
> +
> + if (old_conn_state->crtc != &crtc->base)
> + continue;
> +
> + if (encoder->pre_disable)
> + encoder->pre_disable(state, encoder, old_crtc_state,
> + old_conn_state);
> + }
> +}
> +
> static void intel_encoders_disable(struct intel_atomic_state *state,
> struct intel_crtc *crtc)
> {
> @@ -10327,6 +10349,8 @@ static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
>
> drm_WARN_ON(&dev_priv->drm, old_crtc_state->bigjoiner_slave);
>
> + intel_encoders_pre_disable(state, crtc);
> +
> intel_crtc_disable_planes(state, crtc);
>
> /*
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 19d8d3eefbc27..34ec83fc71670 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -196,6 +196,10 @@ struct intel_encoder {
> void (*update_complete)(struct intel_atomic_state *,
> struct intel_encoder *,
> struct intel_crtc *);
> + void (*pre_disable)(struct intel_atomic_state *,
> + struct intel_encoder *,
> + const struct intel_crtc_state *,
> + const struct drm_connector_state *);
> void (*disable)(struct intel_atomic_state *,
> struct intel_encoder *,
> const struct intel_crtc_state *,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index e2eb46337369e..8d13d7b26a25b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -348,6 +348,16 @@ static void wait_for_act_sent(struct intel_encoder *encoder,
> drm_dp_check_act_status(&intel_dp->mst_mgr);
> }
>
> +static void intel_mst_pre_disable_dp(struct intel_atomic_state *state,
> + struct intel_encoder *encoder,
> + const struct intel_crtc_state *old_crtc_state,
> + const struct drm_connector_state *old_conn_state)
> +{
> + if (old_crtc_state->has_audio)
> + intel_audio_codec_disable(encoder, old_crtc_state,
> + old_conn_state);
> +}
> +
> static void intel_mst_disable_dp(struct intel_atomic_state *state,
> struct intel_encoder *encoder,
> const struct intel_crtc_state *old_crtc_state,
> @@ -372,9 +382,6 @@ static void intel_mst_disable_dp(struct intel_atomic_state *state,
> if (ret) {
> drm_dbg_kms(&i915->drm, "failed to update payload %d\n", ret);
> }
> - if (old_crtc_state->has_audio)
> - intel_audio_codec_disable(encoder,
> - old_crtc_state, old_conn_state);
> }
>
> static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
> @@ -903,6 +910,7 @@ intel_dp_create_fake_mst_encoder(struct intel_digital_port *dig_port, enum pipe
>
> intel_encoder->compute_config = intel_dp_mst_compute_config;
> intel_encoder->compute_config_late = intel_dp_mst_compute_config_late;
> + intel_encoder->pre_disable = intel_mst_pre_disable_dp;
> intel_encoder->disable = intel_mst_disable_dp;
> intel_encoder->post_disable = intel_mst_post_disable_dp;
> intel_encoder->update_pipe = intel_ddi_update_pipe;
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
prev parent reply other threads:[~2021-07-27 14:36 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-26 18:15 [Intel-gfx] [PATCH] drm/i915/display: Disable audio, DRRS and PSR before planes José Roberto de Souza
2021-07-26 18:29 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2021-07-26 18:58 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-07-26 22:43 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2021-07-27 14:36 ` Gwan-gyeong Mun [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=133203bd-66eb-7a2b-79da-8abf8953774f@intel.com \
--to=gwan-gyeong.mun@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jose.souza@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).