Re: [PATCH v10 04/10] drm/i915/vrr: Prepare to Use TRANS_PUSH mechanism for PSR frame change

Nautiyal, Ankit K
2026-01-27T05:19:13Z

On 1/26/2026 1:29 PM, Jouni Högander wrote:
> On Lunarlake and onwards it is possible to generate PSR "frame change"
> event using TRANS_PUSH mechanism. Implement function to enable this and
> take PSR into account in intel_vrr_send_push.
>
> v7:
> - HAS_PSR_FRAME_CHANGE macro moved to separate patch and renamed as
> HAS_PSR_TRANS_PUSH_FRAME_CHANGE
> - use intel_psr_use_trans_push instead of HAS_PSR_FRAME_CHANGE in
> intel_psr_trigger_frame_change
> - moved calling intel_vrr_psr_frame_change_enable away from this patch
> v6:
> - add HAS_PSR_FRAME_CHANGE macro
> - use TRANS_PUSH in instead of TRAN_VRR_CTL
> v5: use intel_psr_use_trans_push for intel_vrr_psr_frame_change_enable
> v4:
> - use rmw when enabling/disabling transcoder
> - set TRANS_PUSH_EN conditionally in intel_vrr_send_push
> - do not call intel_vrr_send_push from intel_psr_trigger_frame_change
> - do not enable using TRANS_PUSH mechanism for PSR "Frame Change"
> v3:
> - use rmw when enabling/disabling
> - keep LNL_TRANS_PUSH_PSR_PR_EN set always on LunarLake and onwards
> v2: use intel_vrr_trans_push_enabled_set_clear instead of rmw
>
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_crtc.c | 4 +++-
> drivers/gpu/drm/i915/display/intel_psr.c | 8 +++++---
> drivers/gpu/drm/i915/display/intel_vrr.c | 14 ++++++++++++--
> drivers/gpu/drm/i915/display/intel_vrr.h | 1 +
> 4 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
> index 778ebc5095c3..ed3c6c4ce025 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
> @@ -747,7 +747,9 @@ void intel_pipe_update_end(struct intel_atomic_state *state,
> * which would cause the next frame to terminate already at vmin
> * vblank start instead of vmax vblank start.
> */
> - if (!state->base.legacy_cursor_update)
> + if (!state->base.legacy_cursor_update ||
> + (intel_psr_use_trans_push(new_crtc_state) &&
> + !new_crtc_state->vrr.enable))
> intel_vrr_send_push(NULL, new_crtc_state);
>
> local_irq_enable();
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index b0d72c04db45..9613c50623dc 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -2516,9 +2516,11 @@ void intel_psr_trigger_frame_change_event(struct intel_dsb *dsb,
> intel_pre_commit_crtc_state(state, crtc);
> struct intel_display *display = to_intel_display(crtc);
>
> - if (crtc_state->has_psr)
> - intel_de_write_dsb(display, dsb,
> - CURSURFLIVE(display, crtc->pipe), 0);
> + if (!crtc_state->has_psr || intel_psr_use_trans_push(crtc_state))
> + return;
> +
> + intel_de_write_dsb(display, dsb,
> + CURSURFLIVE(display, crtc->pipe), 0);
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index f26989c74268..8a072f90049f 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -698,7 +698,7 @@ void intel_vrr_send_push(struct intel_dsb *dsb,
> struct intel_display *display = to_intel_display(crtc_state);
> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>
> - if (!crtc_state->vrr.enable)
> + if (!crtc_state->vrr.enable && !intel_psr_use_trans_push(crtc_state))
> return;
>
> if (dsb)
> @@ -920,7 +920,8 @@ static void intel_vrr_tg_disable(const struct intel_crtc_state *old_crtc_state)
> VRR_STATUS_VRR_EN_LIVE, 1000))
> drm_err(display->drm, "Timed out waiting for VRR live status to clear\n");
>
> - intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), 0);
> + intel_de_rmw(display, TRANS_PUSH(display, cpu_transcoder),
> + TRANS_PUSH_EN, 0);
> }
>
> void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
> @@ -973,6 +974,15 @@ void intel_vrr_transcoder_disable(const struct intel_crtc_state *old_crtc_state)
> intel_vrr_tg_disable(old_crtc_state);
> }
>
> +void intel_vrr_psr_frame_change_enable(const struct intel_crtc_state *crtc_state)

I think introduction to this helper can be a separate patch.

So in the current patch, we are essentially preparing PSR HW to hook to
the new frame change enable based on intel_psr_use_trans_push().

IMO, the helper to enable this new functionality can be a separate thing.

Overall I agree with the changes.

Regards,

Ankit

> +{
> + struct intel_display *display = to_intel_display(crtc_state);
> + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> +
> + intel_de_write(display, TRANS_PUSH(display, cpu_transcoder),
> + trans_vrr_push(crtc_state, false));
> +}
> +
> bool intel_vrr_is_fixed_rr(const struct intel_crtc_state *crtc_state)
> {
> return crtc_state->vrr.flipline &&
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> index bedcc8c4bff2..4f16ca4af91f 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.h
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> @@ -33,6 +33,7 @@ void intel_vrr_dcb_increment_flip_count(struct intel_crtc_state *crtc_state,
> struct intel_crtc *crtc);
> bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state);
> void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state);
> +void intel_vrr_psr_frame_change_enable(const struct intel_crtc_state *crtc_state);
> void intel_vrr_get_config(struct intel_crtc_state *crtc_state);
> int intel_vrr_vmax_vtotal(const struct intel_crtc_state *crtc_state);
> int intel_vrr_vmin_vtotal(const struct intel_crtc_state *crtc_state);