Re: [Intel-gfx] [PATCH v2 5/6] drm/i915/vrr: Relocate VRR enable/disable

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Apr 12, 2023 at 02:11:29PM +0000, Golani, Mitulkumar Ajitkumar wrote:
> Hi Ville,
> 
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: 11 April 2023 11:11
> > To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH v2 5/6] drm/i915/vrr: Relocate VRR
> > enable/disable
> > 
> > On Mon, Mar 27, 2023 at 08:05:49PM +0000, Golani, Mitulkumar Ajitkumar
> > wrote:
> > > Hi Ville,
> > >
> > > > -----Original Message-----
> > > > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
> > > > Of Ville Syrjala
> > > > Sent: 21 March 2023 19:26
> > > > To: intel-gfx@lists.freedesktop.org
> > > > Subject: [Intel-gfx] [PATCH v2 5/6] drm/i915/vrr: Relocate VRR
> > > > enable/disable
> > > >
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > >
> > > > Move VRR enabling/disabling into a place where it also works for fastsets.
> > > >
> > > > With this we always start the transcoder up in non-VRR mode.
> > > > Granted  we already did that but for a very short period of time.
> > > > But now that we might end up doing a bit more with the transcoder in
> > > > non-VRR mode it seems prudent to also update the active timings as
> > > > the transcoder changes its operating mode.
> > > >
> > > > crtc_state->vrr.enable still tracks whether VRR is actually enabled
> > > > or not, but now we configure all the other VRR timing registers
> > > > whenever VRR is possible (whether we actually enable it or not).
> > > > crtc_state->vrr.flipline can now serve as our "is VRR possible" bit of state.
> > >
> > > Understood the change. I was thinking if it is possible to make
> > > distinguish between is VRR "possible" and is VRR "enabled" by adding a
> > > new param ? Although changes looks good to me but using Flipline value as
> > "is VRR Possible" makes it bit confusing.
> > 
> > I suppose we could think about adding a knob for it. It would just reflect the
> > flipline enable bit state in the current scheme.
> > 
> > Another thing I was pondering is whether we should even care about this in
> > intel_dp_prepare_link_train() or if we should just set the MSA ingore bit any
> > time we have a VRR capable display. But I suppose that could have some
> > implicatations eg. for interlaces displays modes.
> 
> In that case to avoid implications should we add additional check for vrr.enable
> as well ?

The idea was to not have to do any DPCD frobbery when enabling/disabling
VRR. That would further complicate the fastset sequence.

-- 
Ville Syrjälä
Intel