Re: [PATCH 19/21] drm/i915/vrr: Allow fixed_rr with pipe joiner

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

On Mon, Mar 10, 2025 at 05:46:13PM +0530, Ankit Nautiyal wrote:
> VRR with joiner is currently disabled as it still needs some work to
> correctly sequence the primary and secondary transcoders. However, we can
> still use VRR Timing generator in fixed refresh rate for joiner and since
> it just need to program vrr timings once and does not involve changing
> timings on the fly. We still need to skip the VRR and LRR for joiner.
> 
> To achieve this set vrr.in_range to 0 for joiner case, so that we do not
> try VRR and LRR for the joiner case.
> 
> v2: Avoid checks for secondary pipes, where not required. (Ville)
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_vrr.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index b83b1c66c242..76398b3a9679 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -353,19 +353,23 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>  	if (!HAS_VRR(display))
>  		return;
>  
> -	/*
> -	 * FIXME all joined pipes share the same transcoder.
> -	 * Need to account for that during VRR toggle/push/etc.
> -	 */
> -	if (crtc_state->joiner_pipes)
> -		return;
> -
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
>  		return;
>  
>  	crtc_state->vrr.in_range =
>  		intel_vrr_is_in_range(connector, drm_mode_vrefresh(adjusted_mode));
>  
> +	/*
> +	 * Allow fixed refresh rate with VRR Timing Generator.
> +	 * For now set the vrr.in_range to 0, to allow fixed_rr but skip actual
> +	 * VRR and LRR.
> +	 * #TODO For actual VRR with joiner, we need to figure out how to
> +	 * correctly sequence transcoder level stuff vs. pipe level stuff
> +	 * in the commit.
> +	 */
> +	if (crtc_state->joiner_pipes)
> +		crtc_state->vrr.in_range = 0;

s/0/false/

> +
>  	vmin = intel_vrr_compute_vmin(crtc_state);
>  
>  	if (crtc_state->vrr.in_range) {
> @@ -488,6 +492,9 @@ void intel_vrr_send_push(struct intel_dsb *dsb,
>  	if (!crtc_state->vrr.enable)
>  		return;
>  
> +	if (intel_crtc_is_joiner_secondary(crtc_state))
> +		return;

This check is redundant due to the vrr.enable check above.

With those two issues sorted
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +
>  	if (dsb)
>  		intel_dsb_nonpost_start(dsb);
>  
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel