Re: [PATCH 11/13] drm/i915/vrr: Handle joiner with vrr

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

On Mon, Sep 02, 2024 at 01:36:32PM +0530, Ankit Nautiyal wrote:
> Do not program transcoder registers for VRR for the secondary pipe of
> the joiner. Remove check to skip VRR for joiner case.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_vrr.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 5e947465c6e0..e01d4b4b8fa7 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -169,13 +169,6 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>  	const struct drm_display_info *info = &connector->base.display_info;
>  	int vmin, vmax;
>  
> -	/*
> -	 * FIXME all joined pipes share the same transcoder.
> -	 * Need to account for that during VRR toggle/push/etc.
> -	 */
> -	if (crtc_state->joiner_pipes)
> -		return;

There's more to this than just sprinkling the secondary checks.
Namely, we need to make sure the timing changes happen in the
correct spot in the sequence for both primary and secondary pipes.

> -
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
>  		return;
>  
> @@ -272,6 +265,9 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state)
>  	struct intel_display *display = to_intel_display(crtc_state);
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  
> +	if (intel_crtc_is_joiner_secondary(crtc_state))
> +		return;
> +
>  	/*
>  	 * This bit seems to have two meanings depending on the platform:
>  	 * TGL: generate VRR "safe window" for DSB vblank waits
> @@ -313,6 +309,9 @@ void intel_vrr_send_push(const struct intel_crtc_state *crtc_state)
>  	struct intel_display *display = to_intel_display(crtc_state);
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  
> +	if (intel_crtc_is_joiner_secondary(crtc_state))
> +		return;
> +
>  	if (!crtc_state->vrr.enable || crtc_state->vrr.fixed_rr)
>  		return;
>  
> @@ -336,6 +335,9 @@ void intel_vrr_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;
>  
> +	if (intel_crtc_is_joiner_secondary(crtc_state))
> +		return;
> +
>  	if (!crtc_state->vrr.enable)
>  		return;
>  
> @@ -364,6 +366,9 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
>  	struct intel_display *display = to_intel_display(old_crtc_state);
>  	enum transcoder cpu_transcoder = old_crtc_state->cpu_transcoder;
>  
> +	if (intel_crtc_is_joiner_secondary(old_crtc_state))
> +		return;
> +
>  	if (!old_crtc_state->vrr.enable)
>  		return;
>  
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel