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

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

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@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.

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.

> 
> 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.
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel