Re: [PATCH 10/13] drm/i915/vrr: Avoid sending PUSH when VRR TG is used with Fixed refresh rate

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

On Mon, Sep 02, 2024 at 01:36:31PM +0530, Ankit Nautiyal wrote:
> As per Bspec:68925: Push enable must be set if not configuring for a
> fixed refresh rate (i.e Vmin == Flipline == Vmax is not true).
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> Reviewed-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_vrr.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 03af50b9f9eb..5e947465c6e0 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -313,7 +313,7 @@ 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 (!crtc_state->vrr.enable)
> +	if (!crtc_state->vrr.enable || crtc_state->vrr.fixed_rr)

Hmm. I wonder if we should keep vrr.enable meaning VRR actually
enabled... Maybe not as that would complicate the readout/check
a bit too much perhaps.

But we could have some kind of helper for these "do we need to use
push?" checks (eg. intel_vrr_use_push()).

>  		return;
>  
>  	intel_de_write(display, TRANS_PUSH(display, cpu_transcoder),
> @@ -325,7 +325,7 @@ bool intel_vrr_is_push_sent(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 (!crtc_state->vrr.enable)
> +	if (!crtc_state->vrr.enable || crtc_state->vrr.fixed_rr)
>  		return false;
>  
>  	return intel_de_read(display, TRANS_PUSH(display, cpu_transcoder)) & TRANS_PUSH_SEND;
> @@ -339,8 +339,9 @@ void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
>  	if (!crtc_state->vrr.enable)
>  		return;
>  
> -	intel_de_write(display, TRANS_PUSH(display, cpu_transcoder),
> -		       TRANS_PUSH_EN);
> +	if (!crtc_state->vrr.fixed_rr)
> +		intel_de_write(display, TRANS_PUSH(display, cpu_transcoder),
> +			       TRANS_PUSH_EN);
>  
>  	if (HAS_AS_SDP(display))
>  		intel_de_write(display,
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel