On Wed, Mar 26, 2025 at 09:33:21PM +0530, Ankit Nautiyal wrote:
> For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal
> bits are not required. Since the support for these bits is going to
> be deprecated in upcoming platforms, avoid writing these bits for the
> platforms that do not use legacy Timing Generator.
>
> Since for these platforms TRAN_VMIN is always filled with crtc_vtotal,
> use TRAN_VRR_VMIN to get the vtotal for adjusted_mode.
>
> v2: Avoid having a helper for manipulating VTOTAL register, and instead
> just make the change where required. (Ville)
> v3: Set `crtc_vtotal` instead of working with the bits directly (Ville).
> Use intel_vrr_vmin_vtotal() to set the vtotal during readout. (Ville)
>
> Signed-off-by: Ankit Nautiyal <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 31 +++++++++++++++++++-
> drivers/gpu/drm/i915/display/intel_vrr.c | 10 +++++++
> 2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 0db1cd4fc963..6796dd0307a6 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2698,9 +2698,19 @@ static void intel_set_transcoder_timings(const struct
> intel_crtc_state *crtc_sta
> HSYNC_START(adjusted_mode->crtc_hsync_start - 1) |
> HSYNC_END(adjusted_mode->crtc_hsync_end - 1));
>
> + /*
> + * For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal
> + * bits are not required. Since the support for these bits is going to
> + * be deprecated in upcoming platforms, avoid writing these bits for the
> + * platforms that do not use legacy Timing Generator.
> + */
> + if (intel_vrr_always_use_vrr_tg(display))
> + crtc_vtotal = 1;
> +
> intel_de_write(display, TRANS_VTOTAL(display, cpu_transcoder),
> VACTIVE(crtc_vdisplay - 1) |
> VTOTAL(crtc_vtotal - 1));
> +
spurious whitespace change
> intel_de_write(display, TRANS_VBLANK(display, cpu_transcoder),
> VBLANK_START(crtc_vblank_start - 1) |
> VBLANK_END(crtc_vblank_end - 1));
> @@ -2758,6 +2768,15 @@ static void intel_set_transcoder_timings_lrr(const
> struct intel_crtc_state *crtc
> intel_de_write(display, TRANS_VBLANK(display, cpu_transcoder),
> VBLANK_START(crtc_vblank_start - 1) |
> VBLANK_END(crtc_vblank_end - 1));
> + /*
> + * For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal
> + * bits are not required. Since the support for these bits is going to
> + * be deprecated in upcoming platforms, avoid writing these bits for the
> + * platforms that do not use legacy Timing Generator.
> + */
> + if (intel_vrr_always_use_vrr_tg(display))
> + crtc_vtotal = 1;
> +
> /*
> * The double buffer latch point for TRANS_VTOTAL
> * is the transcoder's undelayed vblank.
> @@ -2827,7 +2846,17 @@ static void intel_get_transcoder_timings(struct
> intel_crtc *crtc,
>
> tmp = intel_de_read(display, TRANS_VTOTAL(display, cpu_transcoder));
> adjusted_mode->crtc_vdisplay = REG_FIELD_GET(VACTIVE_MASK, tmp) + 1;
> - adjusted_mode->crtc_vtotal = REG_FIELD_GET(VTOTAL_MASK, tmp) + 1;
> +
> + /*
> + * For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal
> + * bits are not filled. The value for adjusted_mode->crtc_vtotal is read
> + * from VRR_VMIN register in intel_vrr_get_config.
> + * Just set this to 0 here.
> + */
> + if (intel_vrr_always_use_vrr_tg(display))
This one either needs the transcoder_has_vrr() check, or we could just
keep on blindly reading this anyway, and let intel_vrr_get_config()
overwrite it afterwards. That's kinda how we deal with
TRANS_SET_CONTEXT_LATENCY as well.
> + adjusted_mode->crtc_vtotal = 0;
> + else
> + adjusted_mode->crtc_vtotal = REG_FIELD_GET(VTOTAL_MASK, tmp) +
> 1;
>
> /* FIXME TGL+ DSI transcoders have this! */
> if (!transcoder_is_dsi(cpu_transcoder)) {
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c
> b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 414f93851059..7359d66fc091 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -708,6 +708,16 @@ void intel_vrr_get_config(struct intel_crtc_state
> *crtc_state)
> crtc_state->vrr.vmin = intel_de_read(display,
> TRANS_VRR_VMIN(display,
> cpu_transcoder)) + 1;
>
> + /*
> + * For platforms that always use VRR Timing Generator, the
> VTOTAL.Vtotal
> + * bits are not filled. Since for these platforms TRAN_VMIN is
> always
> + * filled with crtc_vtotal, use TRAN_VRR_VMIN to get the vtotal
> for
> + * adjusted_mode.
> + */
> + if (intel_vrr_always_use_vrr_tg(display))
> + crtc_state->hw.adjusted_mode.crtc_vtotal =
> + intel_vrr_vmin_vtotal(crtc_state);
> +
> if (HAS_AS_SDP(display)) {
> trans_vrr_vsync =
> intel_de_read(display,
> --
> 2.45.2
--
Ville Syrjälä
Intel
