Re: [PATCH 1/2] drm/i915/display: Introduce transcoder_has_vrr() helper

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Mar 26, 2025 at 09:33:20PM +0530, Ankit Nautiyal wrote:
> To avoid having VRR read/write for DSI transcoders, we currently use
> !transcoder_is_dsi() in many places.
> Instead introduce a new helper to check transcoder_has_vrr() and use
> that to exclude transcoders which do not support VRR.
> 
> v2: Include HAS_VRR into the helper. (Ville)
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index ee7812126129..0db1cd4fc963 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2625,6 +2625,15 @@ void intel_cpu_transcoder_set_m2_n2(struct intel_crtc *crtc,
>  		      PIPE_LINK_N2(display, transcoder));
>  }
>  
> +static bool
> +transcoder_has_vrr(const struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_display *display = to_intel_display(crtc_state);
> +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> +
> +	return HAS_VRR(display) && !transcoder_is_dsi(cpu_transcoder);
> +}
> +
>  static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_display *display = to_intel_display(crtc_state);
> @@ -2635,7 +2644,7 @@ static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_sta
>  	u32 crtc_vdisplay, crtc_vtotal, crtc_vblank_start, crtc_vblank_end;
>  	int vsyncshift = 0;
>  
> -	drm_WARN_ON(display->drm, transcoder_is_dsi(cpu_transcoder));
> +	drm_WARN_ON(display->drm, !transcoder_has_vrr(crtc_state));

Actually, this one should stay as is. We don't use this for
DSI even on hardware that lacks VRR support.

>  
>  	/* We need to be careful not to changed the adjusted mode, for otherwise
>  	 * the hw state checker will get angry at the mismatch. */
> @@ -2717,7 +2726,7 @@ static void intel_set_transcoder_timings_lrr(const struct intel_crtc_state *crtc
>  	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
>  	u32 crtc_vdisplay, crtc_vtotal, crtc_vblank_start, crtc_vblank_end;
>  
> -	drm_WARN_ON(display->drm, transcoder_is_dsi(cpu_transcoder));
> +	drm_WARN_ON(display->drm, !transcoder_has_vrr(crtc_state));

same here.

>  
>  	crtc_vdisplay = adjusted_mode->crtc_vdisplay;
>  	crtc_vtotal = adjusted_mode->crtc_vtotal;
> @@ -3908,7 +3917,7 @@ static bool hsw_get_pipe_config(struct intel_crtc *crtc,
>  	    DISPLAY_VER(display) >= 11)
>  		intel_get_transcoder_timings(crtc, pipe_config);
>  
> -	if (HAS_VRR(display) && !transcoder_is_dsi(pipe_config->cpu_transcoder))
> +	if (transcoder_has_vrr(pipe_config))
>  		intel_vrr_get_config(pipe_config);

This one is fine.

>  
>  	intel_get_pipe_src_size(crtc, pipe_config);
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel