[PATCH 11/35] drm/i915/vrr: Avoid prepare vrr timings for cmrr

[Nautiyal, Ankit K]

On 1/25/2025 2:39 AM, Ville Syrjälä wrote:
> On Fri, Jan 24, 2025 at 08:29:56PM +0530, Ankit Nautiyal wrote:
>> CMRR has a separate logic for computing vrr timings and so it
>> overwrites the timings prepared for vrr.
>>
>> Avoid prepare vrr timings for cmrr. This will help to separate the
>> helpers for timings for vrr, cmrr and the forthcoming
>> fixed_rr.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal at 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 7e69e30b2076..90fd6fe58fce 100644
>> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
>> @@ -257,8 +257,9 @@ void intel_vrr_compute_cmrr_timings(struct intel_crtc_state *crtc_state)
>>   }
>>   
>>   static
>> -void intel_vrr_compute_vrr_timings(struct intel_crtc_state *crtc_state)
>> +void intel_vrr_compute_vrr_timings(struct intel_crtc_state *crtc_state, int vmin, int vmax)
>>   {
>> +	intel_vrr_prepare_vrr_timings(crtc_state, vmin, vmax);
>>   	crtc_state->vrr.enable = true;
>>   	crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
>>   }
>> @@ -328,12 +329,12 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>>   	if (vmin >= vmax)
>>   		return;
>>   
>> -	intel_vrr_prepare_vrr_timings(crtc_state, vmin, vmax);
>> -
>>   	if (crtc_state->uapi.vrr_enabled)
>> -		intel_vrr_compute_vrr_timings(crtc_state);
>> +		intel_vrr_compute_vrr_timings(crtc_state, vmin, vmax);
>>   	else if (is_cmrr_frac_required(crtc_state) && is_edp)
>>   		intel_vrr_compute_cmrr_timings(crtc_state);
>> +	else
>> +		intel_vrr_prepare_vrr_timings(crtc_state, vmin, vmax);
> I don't understand why the caller is calculating the vrr vmin/vmax
> and then passing them in to everyone. Looks to me like each of those
> guys should just calculate the vmin/vmax on their own.

We are computing vmin and vmax early to avoid computing variable timings 
for non vrr panels, based on the condition vmin < vmax.

But I get your point. Will simplify this as mentioned below.

Thanks & Regards,

Ankit

>
> The
>   crtc_state->vrr.flipline = crtc_state->vrr.vmin;
>   crtc_state->vrr.vmin -= intel_vrr_min_flipline_offset(display);
> part could stay in the caller since it's always needed regardless
> of what kind of timings we use.
>
>
>
>
>>   
>>   	if (HAS_AS_SDP(display)) {
>>   		crtc_state->vrr.vsync_start =
>> -- 
>> 2.45.2