Re: [PATCH v9 3/8] drm/i915/vrr: Prepare to Use TRANS_PUSH mechanism for PSR frame change

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

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 <jouni.hogander@intel.com>
> ---
>   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.


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);