Re: [PATCH 03/23] drm/i915/vrr: Introduce new field for VRR mode

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

On 11/11/2024 11:03 PM, Garg, Nemesa wrote:
>
>> -----Original Message-----
>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ankit
>> Nautiyal
>> Sent: Monday, November 11, 2024 2:42 PM
>> To: intel-gfx@lists.freedesktop.org
>> Cc: intel-xe@lists.freedesktop.org; jani.nikula@linux.intel.com;
>> ville.syrjala@linux.intel.com; Golani, Mitulkumar Ajitkumar
>> <mitulkumar.ajitkumar.golani@intel.com>
>> Subject: [PATCH 03/23] drm/i915/vrr: Introduce new field for VRR mode
>>
>> VRR timing generator can be used in multiple modes: dynamic vrr, fixed refresh
>> rate and content matched refresh rate (cmrr).
>> Currently we support dynamic vrr mode and cmrr mode, so add a new member to
>> track in which mode the VRR timing generator is used.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_display_types.h | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
>> b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index d3a1aa7c919f..a1b67e76d91c 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -913,6 +913,12 @@ void intel_io_mmio_fw_write(void *ctx, i915_reg_t reg,
>> u32 val);
>>
>>   typedef void (*intel_io_reg_write)(void *ctx, i915_reg_t reg, u32 val);
>>
>> +enum intel_vrrtg_mode {
>> +	INTEL_VRRTG_MODE_NONE,
>> +	INTEL_VRRTG_MODE_VRR,
>> +	INTEL_VRRTG_MODE_CMRR,
>> +};
>> +
> Here INTEL_VRRTG_MODE_NONE mode means fixed refresh rate mode ?
> And if not should we add this as member for future purpose?

Hi Nemesa,


INTEL_VRRTG_MODE_NONE means that VRR timing generator is not used.

For fixed refresh rate with VRR timing generator, INTEL_VRRTG_MODE_FIXED_RR is added in later patches.
Perhaps I should document the meaning of these in comments.

Thanks & Regards,
Ankit

>
> Thanks,
> Nemesa
>>   struct intel_crtc_state {
>>   	/*
>>   	 * uapi (drm) state. This is the software state shown to userspace.
>> @@ -1286,6 +1292,7 @@ struct intel_crtc_state {
>>   		u8 pipeline_full;
>>   		u16 flipline, vmin, vmax, guardband;
>>   		u32 vsync_end, vsync_start;
>> +		enum intel_vrrtg_mode mode;
>>   	} vrr;
>>
>>   	/* Content Match Refresh Rate state */
>> --
>> 2.45.2