On Wed, Aug 06, 2025 at 08:22:12AM +0300, Jouni Högander wrote:
> We are currently observing crc failures after we started using dsb for PSR
> updates as well. This seems to happen because PSR HW is still sending
> couple of updates using old framebuffers on wake-up.
>
> This patch is preparing to fix that by adding interface which can be used
> to add poll ensuring PSR HW is idle into dsb commit.
>
> v2: add pass crtc_state->dsb_commit as parameter
>
> Signed-off-by: Jouni Högander <jouni.hogan...@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_psr.c | 40 +++++++++++++++++++++---
> drivers/gpu/drm/i915/display/intel_psr.h | 1 +
> 2 files changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 172bc0c39968..2254dd5a3ac4 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -42,6 +42,7 @@
> #include "intel_dmc.h"
> #include "intel_dp.h"
> #include "intel_dp_aux.h"
> +#include "intel_dsb.h"
> #include "intel_frontbuffer.h"
> #include "intel_hdmi.h"
> #include "intel_psr.h"
> @@ -2991,7 +2992,8 @@ void intel_psr_post_plane_update(struct
> intel_atomic_state *state,
> #define PSR_IDLE_TIMEOUT_MS 50
>
> static int
> -_psr2_ready_for_pipe_update_locked(const struct intel_crtc_state
> *new_crtc_state)
> +_psr2_ready_for_pipe_update_locked(const struct intel_crtc_state
> *new_crtc_state,
> + struct intel_dsb *dsb)
> {
> struct intel_display *display = to_intel_display(new_crtc_state);
> enum transcoder cpu_transcoder = new_crtc_state->cpu_transcoder;
> @@ -3001,6 +3003,13 @@ _psr2_ready_for_pipe_update_locked(const struct
> intel_crtc_state *new_crtc_state
> * As all higher states has bit 4 of PSR2 state set we can just wait for
> * EDP_PSR2_STATUS_STATE_DEEP_SLEEP to be cleared.
> */
> + if (dsb) {
> + intel_dsb_poll(dsb, EDP_PSR2_STATUS(display, cpu_transcoder),
> + EDP_PSR2_STATUS_STATE_DEEP_SLEEP, 0, 200,
> + PSR_IDLE_TIMEOUT_MS * 1000 / 200);
The paramters look like they'll fit in the register. We should probably
add some warns to intel_dsb_poll() to validate that though...
> + return true;
> + }
> +
> return intel_de_wait_for_clear(display,
> EDP_PSR2_STATUS(display, cpu_transcoder),
> EDP_PSR2_STATUS_STATE_DEEP_SLEEP,
> @@ -3008,11 +3017,19 @@ _psr2_ready_for_pipe_update_locked(const struct
> intel_crtc_state *new_crtc_state
> }
>
> static int
> -_psr1_ready_for_pipe_update_locked(const struct intel_crtc_state
> *new_crtc_state)
> +_psr1_ready_for_pipe_update_locked(const struct intel_crtc_state
> *new_crtc_state,
> + struct intel_dsb *dsb)
> {
> struct intel_display *display = to_intel_display(new_crtc_state);
> enum transcoder cpu_transcoder = new_crtc_state->cpu_transcoder;
>
> + if (dsb) {
> + intel_dsb_poll(dsb, psr_status_reg(display, cpu_transcoder),
> + EDP_PSR_STATUS_STATE_MASK, 0, 200,
> + PSR_IDLE_TIMEOUT_MS * 1000 / 200);
> + return true;
> + }
> +
> return intel_de_wait_for_clear(display,
> psr_status_reg(display, cpu_transcoder),
> EDP_PSR_STATUS_STATE_MASK,
> @@ -3045,9 +3062,11 @@ void intel_psr_wait_for_idle_locked(const struct
> intel_crtc_state *new_crtc_stat
> continue;
>
> if (intel_dp->psr.sel_update_enabled)
> - ret =
> _psr2_ready_for_pipe_update_locked(new_crtc_state);
> + ret = _psr2_ready_for_pipe_update_locked(new_crtc_state,
> + NULL);
> else
> - ret =
> _psr1_ready_for_pipe_update_locked(new_crtc_state);
> + ret = _psr1_ready_for_pipe_update_locked(new_crtc_state,
> + NULL);
>
> if (ret)
> drm_err(display->drm,
> @@ -3055,6 +3074,19 @@ void intel_psr_wait_for_idle_locked(const struct
> intel_crtc_state *new_crtc_stat
> }
> }
>
> +void intel_psr_wait_for_idle_dsb(const struct intel_crtc_state
> *new_crtc_state)
> +{
> + if (!new_crtc_state->has_psr || new_crtc_state->has_panel_replay)
> + return;
> +
> + if (new_crtc_state->has_sel_update)
> + _psr2_ready_for_pipe_update_locked(new_crtc_state,
> + new_crtc_state->dsb_commit);
Please pass the dsb all the way from the top so that it's easier to
change the DSB usage model if needed.
Otherwise lgtm
Reviewed-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> + else
> + _psr1_ready_for_pipe_update_locked(new_crtc_state,
> + new_crtc_state->dsb_commit);
> +}
> +
> static bool __psr_wait_for_idle_locked(struct intel_dp *intel_dp)
> {
> struct intel_display *display = to_intel_display(intel_dp);
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
> b/drivers/gpu/drm/i915/display/intel_psr.h
> index 9b061a22361f..0cd0542b2450 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> @@ -52,6 +52,7 @@ void intel_psr_get_config(struct intel_encoder *encoder,
> void intel_psr_irq_handler(struct intel_dp *intel_dp, u32 psr_iir);
> void intel_psr_short_pulse(struct intel_dp *intel_dp);
> void intel_psr_wait_for_idle_locked(const struct intel_crtc_state
> *new_crtc_state);
> +void intel_psr_wait_for_idle_dsb(const struct intel_crtc_state
> *new_crtc_state);
> bool intel_psr_enabled(struct intel_dp *intel_dp);
> int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> struct intel_crtc *crtc);
> --
> 2.43.0
--
Ville Syrjälä
Intel
