[PATCH v6 4/7] drm/i915/vrr: Prepare to Use TRANS_PUSH mechanism for PSR frame change
Nautiyal, Ankit K
ankit.k.nautiyal at intel.com
Tue Aug 12 08:29:19 UTC 2025
On 7/21/2025 4:44 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.
>
> 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 at intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_crtc.c | 4 ++-
> drivers/gpu/drm/i915/display/intel_psr.c | 11 +++++--
> drivers/gpu/drm/i915/display/intel_vrr.c | 39 ++++++++++++++++-------
> drivers/gpu/drm/i915/display/intel_vrr.h | 1 +
> 4 files changed, 40 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
> index a187db6df2d3..5233970b95e5 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
> @@ -723,7 +723,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);
Currently with PSR, vrr.enable will not be concurrent.
Is this required for concurrent Panel Replay + VRR?
>
> local_irq_enable();
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index d7c19b053fcc..172b1070c7b7 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1950,6 +1950,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);
Not sure it it will be correct to set only this bit, when TRANS_PUSH is
not enabled. May be we can use a flag and use that when are
enabling/disabling TRANS_VRR_PUSH
> }
>
> static bool psr_interrupt_error_check(struct intel_dp *intel_dp)
> @@ -2336,9 +2339,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 || DISPLAY_VER(display) >= 20)
I think it would be better to use a macro HAS_PUSH_WITH_PSR_FRAME_CHANGE
or something similar.
> + 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 3eed37f271b0..9db6127abeaa 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -10,6 +10,7 @@
> #include "intel_display_regs.h"
> #include "intel_display_types.h"
> #include "intel_dp.h"
> +#include "intel_psr.h"
> #include "intel_vrr.h"
> #include "intel_vrr_regs.h"
>
> @@ -505,16 +506,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);
> @@ -609,9 +617,7 @@ void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
> crtc_state->vrr.vmax - 1);
> intel_de_write(display, TRANS_VRR_FLIPLINE(display, cpu_transcoder),
> crtc_state->vrr.flipline - 1);
> -
> - 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);
>
> if (!intel_vrr_always_use_vrr_tg(display)) {
> intel_vrr_set_db_point_and_transmission_line(crtc_state);
> @@ -627,6 +633,15 @@ void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
> }
> }
>
> +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_VRR_CTL(display, cpu_transcoder), 0,
> + LNL_TRANS_PUSH_PSR_PR_EN);
I think you mean TRANS_VR_PUSH here instead of TRAN_VRR_CTL.
But as I mentioned earlier, instead of setting this bit, it would be
better to check flag and set the bit in the existing place.
Regards,
Ankit
> +}
> +
> void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
> {
> struct intel_display *display = to_intel_display(old_crtc_state);
> @@ -641,7 +656,8 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
> intel_de_wait_for_clear(display,
> TRANS_VRR_STATUS(display, cpu_transcoder),
> VRR_STATUS_VRR_EN_LIVE, 1000);
> - intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), 0);
> + intel_de_rmw(display, TRANS_PUSH(display, cpu_transcoder),
> + TRANS_PUSH_EN, 0);
> }
>
> intel_vrr_set_fixed_rr_timings(old_crtc_state);
> @@ -664,8 +680,8 @@ void intel_vrr_transcoder_enable(const struct intel_crtc_state *crtc_state)
> return;
> }
>
> - 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);
>
> intel_vrr_set_db_point_and_transmission_line(crtc_state);
>
> @@ -688,7 +704,8 @@ void intel_vrr_transcoder_disable(const struct intel_crtc_state *crtc_state)
>
> intel_de_wait_for_clear(display, TRANS_VRR_STATUS(display, cpu_transcoder),
> VRR_STATUS_VRR_EN_LIVE, 1000);
> - intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), 0);
> + intel_de_rmw(display, TRANS_PUSH(display, cpu_transcoder),
> + TRANS_PUSH_EN, 0);
> }
>
> bool intel_vrr_is_fixed_rr(const struct intel_crtc_state *crtc_state)
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> index 38bf9996b883..6f815fd05c36 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);
More information about the Intel-xe
mailing list