[PATCH] drm/i915/vrr: Set vrr.vmin to min Vtotal to re-enable LRR for PTL

Nautiyal, Ankit K ankit.k.nautiyal at intel.com
Thu May 29 09:08:57 UTC 2025


On 5/28/2025 5:17 PM, Ville Syrjälä wrote:
> On Fri, May 09, 2025 at 08:41:07AM +0530, Ankit Nautiyal wrote:
>> Currently in intel_vrr_compute_config, we set vrr.vmin to crtc_vtotal for
>> all cases to avoid having vrr.vmin changed when we switch from
>> fixed refresh rate timings to variable refresh rate timings. This works for
>> all cases, except for LRR case where user can change the clock so as to
>> seamlessly switch to a lower refresh rate only for a VRR supporting panel.
>>
>> In LRR case the crtc_vtotal changes for the mode, and due to which vrr.vmin
>> changes and therefore the guardband also changes. Since we cannot change
>> the guardband on the fly when VRR Timing Generator is on, this gets
>> rejected.
>>
>> To overcome this, for panels that support VRR, instead of setting the
>> vrr.vmin to crtc_vtotal, we set that to the lowest Vtotal (for highest
>> Refresh rate supported by the panel). For non-vrr panels, the vrr.vmin
>> stays the same i.e. crtc_vtotal.
> The guardband must match the actual vblank length or else we'll end
> up completely changing where the double buffered registers get
> latched/vblank interrupts are generated. And nothing is prepared
> for that.

Hmm.. so how do we deal with the LRR case?

Alternatively can we go for a constant guardband (not matching the 
actual vblank length) that will fit in the smallest vblank?



>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_vrr.c | 64 ++++++++++++++++--------
>>   1 file changed, 42 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
>> index c6565baf815a..f0949a598f53 100644
>> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
>> @@ -278,6 +278,9 @@ int intel_vrr_fixed_rr_vmin(const struct intel_crtc_state *crtc_state)
>>   {
>>   	struct intel_display *display = to_intel_display(crtc_state);
>>   
>> +	if (crtc_state->vrr.in_range)
>> +		return crtc_state->vrr.vmin;
>> +
>>   	return intel_vrr_fixed_rr_vtotal(crtc_state) -
>>   		intel_vrr_flipline_offset(display);
>>   }
>> @@ -308,26 +311,37 @@ static
>>   void intel_vrr_compute_fixed_rr_timings(struct intel_crtc_state *crtc_state)
>>   {
>>   	/*
>> -	 * For fixed rr,  vmin = vmax = flipline.
>> -	 * vmin is already set to crtc_vtotal set vmax and flipline the same.
>> +	 * For fixed rr vmax = flipline.
>> +	 * set vmax and flipline same as vtotal.
>>   	 */
>>   	crtc_state->vrr.vmax = crtc_state->hw.adjusted_mode.crtc_vtotal;
>>   	crtc_state->vrr.flipline = crtc_state->hw.adjusted_mode.crtc_vtotal;
>>   }
>>   
>>   static
>> -int intel_vrr_compute_vmin(struct intel_crtc_state *crtc_state)
>> +int intel_vrr_compute_fixed_vmin(struct intel_crtc_state *crtc_state)
>>   {
>>   	/*
>> -	 * To make fixed rr and vrr work seamless the guardband/pipeline full
>> -	 * should be set such that it satisfies both the fixed and variable
>> -	 * timings.
>> -	 * For this set the vmin as crtc_vtotal. With this we never need to
>> -	 * change anything to do with the guardband.
>> +	 * For non VRR supporting panels/config, set the vmin to crtc_vtotal.
>> +	 * This will help the case where VRR TG is used even for non-vrr panels/config.
>>   	 */
>>   	return crtc_state->hw.adjusted_mode.crtc_vtotal;
>>   }
>>   
>> +static
>> +int intel_vrr_compute_vmin(struct intel_connector *connector,
>> +			   const struct drm_display_mode *adjusted_mode)
>> +{
>> +	const struct drm_display_info *info = &connector->base.display_info;
>> +	int vmin;
>> +
>> +	vmin = adjusted_mode->crtc_clock * 1000 /
>> +		(adjusted_mode->crtc_htotal * info->monitor_range.max_vfreq);
>> +	vmin = min_t(int, vmin, adjusted_mode->crtc_vtotal);
>> +
>> +	return vmin;
>> +}
>> +
>>   static
>>   int intel_vrr_compute_vmax(struct intel_connector *connector,
>>   			   const struct drm_display_mode *adjusted_mode)
>> @@ -374,13 +388,13 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>>   	if (crtc_state->joiner_pipes)
>>   		crtc_state->vrr.in_range = false;
>>   
>> -	vmin = intel_vrr_compute_vmin(crtc_state);
>> -
>>   	if (crtc_state->vrr.in_range) {
>>   		if (HAS_LRR(display))
>>   			crtc_state->update_lrr = true;
>>   		vmax = intel_vrr_compute_vmax(connector, adjusted_mode);
>> +		vmin = intel_vrr_compute_vmin(connector, adjusted_mode);
>>   	} else {
>> +		vmin = intel_vrr_compute_fixed_vmin(crtc_state);
>>   		vmax = vmin;
>>   	}
>>   
>> @@ -671,8 +685,7 @@ void intel_vrr_transcoder_disable(const struct intel_crtc_state *crtc_state)
>>   bool intel_vrr_is_fixed_rr(const struct intel_crtc_state *crtc_state)
>>   {
>>   	return crtc_state->vrr.flipline &&
>> -	       crtc_state->vrr.flipline == crtc_state->vrr.vmax &&
>> -	       crtc_state->vrr.flipline == intel_vrr_vmin_flipline(crtc_state);
>> +	       crtc_state->vrr.flipline == crtc_state->vrr.vmax;
>>   }
>>   
>>   void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
>> @@ -713,16 +726,6 @@ 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,
>> @@ -732,6 +735,23 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
>>   			crtc_state->vrr.vsync_end =
>>   				REG_FIELD_GET(VRR_VSYNC_END_MASK, trans_vrr_vsync);
>>   		}
>> +		/*
>> +		 * For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal
>> +		 * bits are not filled. For fixed timings the vmax vtotal can be used to
>> +		 * fill the VTOTAL. For variable timings, we need to compute the vtotal.
>> +		 */
>> +		if (intel_vrr_always_use_vrr_tg(display)) {
>> +			u32 crtc_vtotal;
>> +
>> +			if (intel_vrr_is_fixed_rr(crtc_state))
>> +				crtc_vtotal = intel_vrr_vmax_vtotal(crtc_state);
>> +			else
>> +				crtc_vtotal = intel_vrr_vmin_vtotal(crtc_state) +
>> +					      crtc_state->vrr.guardband +
>> +					      crtc_state->vrr.vsync_start +
>> +					      crtc_state->vrr.vsync_end + 1;
> This looks like nonsense to me. No idea what you're trying to do here.

I was trying to compute vtotal from other things, since we do not store 
the vtotal in Vtotal reg.

Earlier it was just same as vmin_vtotal , which I tried to change in 
this patch.


Regards,

Ankit

>
>> +			crtc_state->hw.adjusted_mode.crtc_vtotal = crtc_vtotal;
>> +		}
>>   	}
>>   
>>   	vrr_enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE;
>> -- 
>> 2.45.2


More information about the Intel-xe mailing list