| Message ID | 20241111091221.2992818-4-ankit.k.nautiyal@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Use VRR timing generator for fixed refresh rate modes | expand |
> -----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? 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
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
On Mon, 11 Nov 2024, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote: > 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, I couldn't quickly conclude whether this is in fact redundant with tg_enable. Would it be possible to ditch this in favor of using mode != NONE? BR, Jani. > + INTEL_VRRTG_MODE_VRR, > + INTEL_VRRTG_MODE_CMRR, > +}; > + > 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 */
On Tue, 12 Nov 2024, Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Mon, 11 Nov 2024, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote: >> 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, > > I couldn't quickly conclude whether this is in fact redundant with > tg_enable. > > Would it be possible to ditch this in favor of using mode != NONE? Hrmh, I meant s/this/tg_enable/. > > BR, > Jani. > > >> + INTEL_VRRTG_MODE_VRR, >> + INTEL_VRRTG_MODE_CMRR, >> +}; >> + >> 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 */
On 11/12/2024 5:03 PM, Jani Nikula wrote: > On Tue, 12 Nov 2024, Jani Nikula <jani.nikula@linux.intel.com> wrote: >> On Mon, 11 Nov 2024, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote: >>> 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, >> I couldn't quickly conclude whether this is in fact redundant with >> tg_enable. >> >> Would it be possible to ditch this in favor of using mode != NONE? > Hrmh, I meant s/this/tg_enable/. You are right, tg_enable is indeed redundant now, and can be replaced with check for mode. Thanks & Regards, Ankit > >> BR, >> Jani. >> >> >>> + INTEL_VRRTG_MODE_VRR, >>> + INTEL_VRRTG_MODE_CMRR, >>> +}; >>> + >>> 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 */
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, +}; + 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 */
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(+)