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

[Golani, Mitulkumar Ajitkumar]

Hi Ville,

> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Sent: 12 April 2023 13:15
> To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani at intel.com>
> Cc: intel-gfx at lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v2 5/6] drm/i915/vrr: Relocate VRR
> enable/disable
> 
> On Tue, Apr 11, 2023 at 08:41:11AM +0300, Ville Syrjälä wrote:
> > On Mon, Mar 27, 2023 at 08:05:49PM +0000, Golani, Mitulkumar Ajitkumar
> wrote:
> > > Hi Ville,
> > >
> > > > -----Original Message-----
> > > > From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On
> > > > Behalf Of Ville Syrjala
> > > > Sent: 21 March 2023 19:26
> > > > To: intel-gfx at lists.freedesktop.org
> > > > Subject: [Intel-gfx] [PATCH v2 5/6] drm/i915/vrr: Relocate VRR
> > > > enable/disable
> > > >
> > > > From: Ville Syrjälä <ville.syrjala at 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.
> 
> After further pondering I think I'm leaning towards just adding a tiny
> intel_vrr_possible()/etc. helper instead of adding a separate knob for this
> into the crtc state. That seems like somehting we can trivially do as a
> followup.

Yes. That also sounds good.

Regards,
Mitul
> 
> >
> > 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 ?
> >
> > --
> > Ville Syrjälä
> > Intel
> 
> --
> Ville Syrjälä
> Intel