On Thu, 2026-01-22 at 16:34 +0530, Nautiyal, Ankit K wrote:
>
> On 12/23/2025 4:21 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.
> >
> > 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 <[email protected]>
> > ---
> > drivers/gpu/drm/i915/display/intel_crtc.c | 4 +++-
> > drivers/gpu/drm/i915/display/intel_psr.c | 13 +++++++---
> > drivers/gpu/drm/i915/display/intel_vrr.c | 29
> > ++++++++++++++++++-----
> > drivers/gpu/drm/i915/display/intel_vrr.h | 1 +
> > 4 files changed, 37 insertions(+), 10 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 170d65999ccd..4336ba188aa7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -207,6 +207,8 @@
> > #define CAN_PSR(intel_dp) ((intel_dp)->psr.sink_support && \
> > (intel_dp)->psr.source_support)
> >
> > +#define HAS_PSR_FRAME_CHANGE(display) (DISPLAY_VER(display) >=
> > 20)
> > +
> > bool intel_encoder_can_psr(struct intel_encoder *encoder)
> > {
> > if (intel_encoder_is_dp(encoder) || encoder->type ==
> > INTEL_OUTPUT_DP_MST)
> > @@ -2120,6 +2122,9 @@ static void intel_psr_enable_source(struct
> > intel_dp *intel_dp,
> > intel_dmc_block_pkgc(display, intel_dp->psr.pipe,
> > true);
> >
> > intel_alpm_configure(intel_dp, crtc_state);
> > +
> > + if (intel_psr_use_trans_push(crtc_state))
> > + intel_vrr_psr_frame_change_enable(crtc_state);
> > }
> >
> > static bool psr_interrupt_error_check(struct intel_dp *intel_dp)
> > @@ -2511,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 || HAS_PSR_FRAME_CHANGE(display))
> > + 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 b92c42fde937..aaf0f6cf3cfe 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> > @@ -584,16 +584,23 @@ 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;
> > + u32 trans_push;
> >
> > - if (!crtc_state->vrr.enable)
> > + if (!crtc_state->vrr.enable &&
> > !intel_psr_use_trans_push(crtc_state))
> > return;
> >
> > if (dsb)
> > intel_dsb_nonpost_start(dsb);
> >
> > - intel_de_write_dsb(display, dsb,
> > - TRANS_PUSH(display, cpu_transcoder),
> > - TRANS_PUSH_EN | TRANS_PUSH_SEND);
> > + trans_push = TRANS_PUSH_SEND;
> > +
> > + if (crtc_state->vrr.enable)
> > + trans_push |= TRANS_PUSH_EN;
> > + if (intel_psr_use_trans_push(crtc_state))
> > + trans_push |= LNL_TRANS_PUSH_PSR_PR_EN;
> > +
> > + intel_de_write_dsb(display, dsb, TRANS_PUSH(display,
> > cpu_transcoder),
> > + trans_push);
> >
> > if (dsb)
> > intel_dsb_nonpost_end(dsb);
> > @@ -693,7 +700,7 @@ static void intel_vrr_tg_enable(const struct
> > intel_crtc_state *crtc_state,
> > enum transcoder cpu_transcoder = crtc_state-
> > >cpu_transcoder;
> > u32 vrr_ctl;
> >
> > - intel_de_write(display, TRANS_PUSH(display,
> > cpu_transcoder), TRANS_PUSH_EN);
> > + intel_de_rmw(display, TRANS_PUSH(display, cpu_transcoder),
> > 0, TRANS_PUSH_EN);
> >
> > vrr_ctl = VRR_CTL_VRR_ENABLE | trans_vrr_ctl(crtc_state);
> >
> > @@ -721,7 +728,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)
> > @@ -737,6 +745,15 @@ void intel_vrr_enable(const struct
> > intel_crtc_state *crtc_state)
> > intel_vrr_tg_enable(crtc_state, crtc_state-
> > >cmrr.enable);
> > }
> >
> > +void intel_vrr_psr_frame_change_enable(const struct
> > intel_crtc_state *crtc_state)
> > +{
> > + struct intel_display *display =
> > to_intel_display(crtc_state);
> > + enum transcoder cpu_transcoder = crtc_state-
> > >cpu_transcoder;
> > +
> > + intel_de_rmw(display, TRANS_PUSH(display, cpu_transcoder),
> > 0,
> > + LNL_TRANS_PUSH_PSR_PR_EN);
> > +}
>
>
> Can we have a function that tells us what to write in the
> TRANS_VRR_PUSH
> reg, instead or rmw.
>
> static u32 trans_vrr_push(const struct intel_crtc_state *crtc_state,
> bool send_push)
> {
> struct intel_display *display =
> to_intel_display(crtc_state);
> u32 trans_vrr_push = 0;
>
> if (intel_vrr_always_use_vrr_tg(display) ||
> crtc_state->vrr.enable)
> trans_vrr_push |= TRANS_PUSH_EN;
>
> if (send_push)
> trans_vrr_push |= TRANS_PUSH_SEND;
>
> if (intel_psr_use_trans_push(crtc_state))
> trans_vrr_push |= LNL_TRANS_PUSH_PSR_PR_EN;
>
> return trans_vrr_push;
> }
>
> Then we can just use this in different place.
>
> I think that will make things easier.
>
> We can have first a preparatory patch with changes without the bit
> : LNL_TRANS_PUSH_PSR_PR_EN;
>
> The current patch will then just add the lines to set
> the LNL_TRANS_PUSH_PSR_PR_EN and thats it.
This is good idea. I will try/implement that.
BR,
Jouni Högander
>
>
> Regards,
>
> Ankit
>
>
> > +
> > void intel_vrr_disable(const struct intel_crtc_state
> > *old_crtc_state)
> > {
> > struct intel_display *display =
> > to_intel_display(old_crtc_state);
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h
> > b/drivers/gpu/drm/i915/display/intel_vrr.h
> > index bc9044621635..4dc5bb3f6f28 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vrr.h
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> > @@ -30,6 +30,7 @@ void intel_vrr_check_push_sent(struct intel_dsb
> > *dsb,
> > const struct intel_crtc_state
> > *crtc_state);
> > 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);
