Re: [PATCH 16/22] drm/i915/vrr: Use fixed timings for platforms that support VRR

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 3/5/2025 12:23 AM, Ville Syrjälä wrote:
> On Tue, Mar 04, 2025 at 01:49:42PM +0530, Ankit Nautiyal wrote:
>> For fixed refresh rate use fixed timings for all platforms that support
>> VRR. For this add checks to avoid computing and reading VRR for
>> platforms that do not support VRR.
>>
>> v2: Avoid touching check for VRR_CTL_FLIP_LINE_EN. (Ville)
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_vrr.c | 11 ++++++++---
>>   1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
>> index 97040ab9ed86..11f23affd13a 100644
>> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
>> @@ -347,6 +347,9 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>>   	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
>>   	int vmin, vmax;
>>   
>> +	if (!HAS_VRR(display))
>> +		return;
>> +
>>   	/*
>>   	 * FIXME all joined pipes share the same transcoder.
>>   	 * Need to account for that during VRR toggle/push/etc.
>> @@ -370,8 +373,7 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>>   		vmax = vmin;
>>   	}
>>   
>> -	if (vmin >= vmax)
>> -		return;
>> +	vmin = intel_vrr_compute_vmin(crtc_state);
> Didn't we already do that?


(facepalm) Yes, indeed, this is already set in an earlier patch. It 
should be removed. Thanks for catching this.


>
>>   
>>   	crtc_state->vrr.vmin = vmin;
>>   	crtc_state->vrr.vmax = vmax;
>> @@ -385,7 +387,7 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>>   	 */
>>   	crtc_state->vrr.vmin -= intel_vrr_flipline_offset(display);
>>   
>> -	if (crtc_state->uapi.vrr_enabled)
>> +	if (crtc_state->uapi.vrr_enabled && vmin < vmax)
>>   		intel_vrr_compute_vrr_timings(crtc_state);
>>   	else if (is_cmrr_frac_required(crtc_state) && is_edp)
>>   		intel_vrr_compute_cmrr_timings(crtc_state);
>> @@ -659,6 +661,9 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
>>   	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>>   	u32 trans_vrr_ctl, trans_vrr_vsync;
>>   
>> +	if (!HAS_VRR(display))
>> +		return;
>> +
> The caller already checks that, so should not be needed here.

Will remove this check from here.


Regards,

Ankit

>
>>   	trans_vrr_ctl = intel_de_read(display,
>>   				      TRANS_VRR_CTL(display, cpu_transcoder));
>>   
>> -- 
>> 2.45.2