Re: [PATCH 02/16] drm/i915/vrr: Avoid reading vrr.enable based on fixed_rr check

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

On 3/21/2025 11:04 PM, Ville Syrjälä wrote:
> On Tue, Mar 18, 2025 at 01:05:26PM +0530, Ankit Nautiyal wrote:
>> Currently, vrr.enable is intended only for variable refresh rate timings.
>> At this point, we do not set fixed refresh rate timings, but the GOP can,
>> which creates a problem during the readback of vrr.enable.
>>
>> The GOP enables the VRR timing generator with fixed timings, while the
>> driver only recognizes the VRR timing generator as enabled with
>> variable timings. This discrepancy causes an issue due to the
>> fixed refresh rate check during readback. Since the VRR timing generator
>> is enabled and we do not support fixed timings, the readback should set
>> vrr.enable so that the driver can disable the VRR timing generator.
>> However, the current check does not allow this.
>>
>> Therefore, remove the fixed refresh rate check during readback.
>>
>> Fixes: 27217f9d1856 ("drm/i915/vrr: Track vrr.enable only for variable timing")
>> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_vrr.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
>> index aa65a6933ddb..6bdcdfed4b9b 100644
>> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
>> @@ -657,8 +657,7 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
>>   		}
>>   	}
>>   
>> -	crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE &&
>> -				 !intel_vrr_is_fixed_rr(crtc_state);
>> +	crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE;
> Doesn't this break the state checker when we use the VRR timing
> generator for fixed refresh modes?

Yes right, but currently those changes are not merged yet.

I have something like this [1], when we add support for VRR TG with 
fixed refresh rate.

[1] https://patchwork.freedesktop.org/patch/643470/?series=134383&rev=16


Regards,

Ankit


>
>>   
>>   	/*
>>   	 * #TODO: For Both VRR and CMRR the flag I915_MODE_FLAG_VRR is set for mode_flags.
>> -- 
>> 2.45.2