On Tue, 2022-03-15 at 15:27 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Let's just do a full DRRS disable/enable across all pipe updates.
> This guarantees that the DRRS work doesn't interfere with anything
> while the atomic commit is busy reprogramming the pipe.
>
> Needed so that we can start reprogramming M/N seamlessly during
> fastsets whenever possible. Also avoids the pre-bdw DRRS PIPECONF
> rmw racing with the potential PIPECONF write from the atomic
> commit (eg. due to GAMMA_MODE changes).
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 4 --
> drivers/gpu/drm/i915/display/intel_display.c | 8 ++--
> drivers/gpu/drm/i915/display/intel_drrs.c | 40 ++------------------
> drivers/gpu/drm/i915/display/intel_drrs.h | 3 --
> 4 files changed, 7 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index e2b297d2c295..dc208df829f1 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -45,7 +45,6 @@
> #include "intel_dp_link_training.h"
> #include "intel_dp_mst.h"
> #include "intel_dpio_phy.h"
> -#include "intel_drrs.h"
> #include "intel_dsi.h"
> #include "intel_fdi.h"
> #include "intel_fifo_underrun.h"
> @@ -3010,12 +3009,9 @@ static void intel_ddi_update_pipe_dp(struct intel_atomic_state *state,
> const struct intel_crtc_state *crtc_state,
> const struct drm_connector_state *conn_state)
> {
> - struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> -
> intel_ddi_set_dp_msa(crtc_state, conn_state);
>
> intel_dp_set_infoframes(encoder, true, crtc_state, conn_state);
> - intel_drrs_update(state, crtc);
>
> intel_backlight_update(state, encoder, crtc_state, conn_state);
> drm_connector_update_privacy_screen(conn_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index eb49973621f0..86fc8ddd0b8f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1229,7 +1229,6 @@ static void intel_post_plane_update(struct intel_atomic_state *state,
>
> hsw_ips_post_update(state, crtc);
> intel_fbc_post_update(state, crtc);
> - intel_drrs_page_flip(crtc);
>
> if (needs_async_flip_vtd_wa(old_crtc_state) &&
> !needs_async_flip_vtd_wa(new_crtc_state))
> @@ -1247,6 +1246,7 @@ static void intel_post_plane_update(struct intel_atomic_state *state,
> !needs_cursorclk_wa(new_crtc_state))
> icl_wa_cursorclkgating(dev_priv, pipe, false);
>
> + intel_drrs_enable(new_crtc_state);
> }
>
> static void intel_crtc_enable_flip_done(struct intel_atomic_state *state,
> @@ -1324,6 +1324,8 @@ static void intel_pre_plane_update(struct intel_atomic_state *state,
> intel_atomic_get_new_crtc_state(state, crtc);
> enum pipe pipe = crtc->pipe;
>
> + intel_drrs_disable(old_crtc_state);
> +
> intel_psr_pre_plane_update(state, crtc);
>
> if (hsw_ips_pre_update(state, crtc))
> @@ -8127,8 +8129,6 @@ static void intel_enable_crtc(struct intel_atomic_state *state,
> if (intel_crtc_is_bigjoiner_slave(new_crtc_state))
> return;
>
> - intel_drrs_enable(new_crtc_state);
> -
> /* vblanks work again, re-enable pipe CRC. */
> intel_crtc_enable_pipe_crc(crtc);
> }
> @@ -8198,8 +8198,6 @@ static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
> */
> intel_crtc_disable_pipe_crc(crtc);
>
> - intel_drrs_disable(old_crtc_state);
> -
> dev_priv->display->crtc_disable(state, crtc);
> crtc->active = false;
> intel_fbc_disable(crtc);
> diff --git a/drivers/gpu/drm/i915/display/intel_drrs.c b/drivers/gpu/drm/i915/display/intel_drrs.c
> index 8f9e0fde0c5a..44c9af8f8b9b 100644
> --- a/drivers/gpu/drm/i915/display/intel_drrs.c
> +++ b/drivers/gpu/drm/i915/display/intel_drrs.c
> @@ -189,13 +189,12 @@ static unsigned int intel_drrs_frontbuffer_bits(const struct intel_crtc_state *c
> void intel_drrs_enable(const struct intel_crtc_state *crtc_state)
> {
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>
> if (!crtc_state->has_drrs)
> return;
>
> - drm_dbg_kms(&dev_priv->drm, "[CRTC:%d:%s] Enabling DRRS\n",
> - crtc->base.base.id, crtc->base.name);
> + if (!crtc_state->hw.active)
> + return;
>
> mutex_lock(&crtc->drrs.mutex);
>
> @@ -217,13 +216,12 @@ void intel_drrs_enable(const struct intel_crtc_state *crtc_state)
> void intel_drrs_disable(const struct intel_crtc_state *old_crtc_state)
> {
> struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc);
> - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>
> if (!old_crtc_state->has_drrs)
> return;
>
> - drm_dbg_kms(&dev_priv->drm, "[CRTC:%d:%s] Disabling DRRS\n",
> - crtc->base.base.id, crtc->base.name);
> + if (!old_crtc_state->hw.active)
> + return;
Changes looks good but now there will not be any DRRS debug message, can you at least add debug message in intel_drrs_compute_config() when DRRS is
allowed?
With that:
Reviewed-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
>
> mutex_lock(&crtc->drrs.mutex);
>
> @@ -239,28 +237,6 @@ void intel_drrs_disable(const struct intel_crtc_state *old_crtc_state)
> cancel_delayed_work_sync(&crtc->drrs.work);
> }
>
> -/**
> - * intel_drrs_update - Update DRRS during fastset
> - * @state: atomic state
> - * @crtc: crtc
> - */
> -void intel_drrs_update(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 intel_crtc_state *new_crtc_state =
> - intel_atomic_get_new_crtc_state(state, crtc);
> -
> - if (old_crtc_state->has_drrs == new_crtc_state->has_drrs)
> - return;
> -
> - if (new_crtc_state->has_drrs)
> - intel_drrs_enable(new_crtc_state);
> - else
> - intel_drrs_disable(old_crtc_state);
> -}
> -
> static void intel_drrs_downclock_work(struct work_struct *work)
> {
> struct intel_crtc *crtc = container_of(work, typeof(*crtc), drrs.work.work);
> @@ -348,14 +324,6 @@ void intel_drrs_flush(struct drm_i915_private *dev_priv,
> intel_drrs_frontbuffer_update(dev_priv, frontbuffer_bits, false);
> }
>
> -void intel_drrs_page_flip(struct intel_crtc *crtc)
> -{
> - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> - unsigned int frontbuffer_bits = INTEL_FRONTBUFFER_ALL_MASK(crtc->pipe);
> -
> - intel_drrs_frontbuffer_update(dev_priv, frontbuffer_bits, false);
> -}
> -
> /**
> * intel_crtc_drrs_init - Init DRRS for CRTC
> * @crtc: crtc
> diff --git a/drivers/gpu/drm/i915/display/intel_drrs.h b/drivers/gpu/drm/i915/display/intel_drrs.h
> index c6e325a91552..6e84b8e800a6 100644
> --- a/drivers/gpu/drm/i915/display/intel_drrs.h
> +++ b/drivers/gpu/drm/i915/display/intel_drrs.h
> @@ -19,13 +19,10 @@ const char *intel_drrs_type_str(enum drrs_type drrs_type);
> bool intel_drrs_is_enabled(struct intel_crtc *crtc);
> void intel_drrs_enable(const struct intel_crtc_state *crtc_state);
> void intel_drrs_disable(const struct intel_crtc_state *crtc_state);
> -void intel_drrs_update(struct intel_atomic_state *state,
> - struct intel_crtc *crtc);
> void intel_drrs_invalidate(struct drm_i915_private *dev_priv,
> unsigned int frontbuffer_bits);
> void intel_drrs_flush(struct drm_i915_private *dev_priv,
> unsigned int frontbuffer_bits);
> -void intel_drrs_page_flip(struct intel_crtc *crtc);
> void intel_drrs_compute_config(struct intel_connector *connector,
> struct intel_crtc_state *pipe_config,
> int output_bpp, bool constant_n);
