Re: [PATCH v4 10/14] drm/vblank: Add helper to get next vblank time

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

On Tue, Feb 21, 2023 at 03:11:33PM +0200, Pekka Paalanen wrote:
> On Tue, 21 Feb 2023 15:01:35 +0200
> Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> 
> > On Tue, Feb 21, 2023 at 10:45:51AM +0200, Pekka Paalanen wrote:
> > > On Mon, 20 Feb 2023 07:55:41 -0800
> > > Rob Clark <robdclark@gmail.com> wrote:
> > >   
> > > > On Mon, Feb 20, 2023 at 1:08 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:  
> > > > >
> > > > > On Sat, 18 Feb 2023 13:15:53 -0800
> > > > > Rob Clark <robdclark@gmail.com> wrote:
> > > > >    
> > > > > > From: Rob Clark <robdclark@chromium.org>
> > > > > >
> > > > > > Will be used in the next commit to set a deadline on fences that an
> > > > > > atomic update is waiting on.
> > > > > >
> > > > > > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > > > > > ---
> > > > > >  drivers/gpu/drm/drm_vblank.c | 32 ++++++++++++++++++++++++++++++++
> > > > > >  include/drm/drm_vblank.h     |  1 +
> > > > > >  2 files changed, 33 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> > > > > > index 2ff31717a3de..caf25ebb34c5 100644
> > > > > > --- a/drivers/gpu/drm/drm_vblank.c
> > > > > > +++ b/drivers/gpu/drm/drm_vblank.c
> > > > > > @@ -980,6 +980,38 @@ u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
> > > > > >  }
> > > > > >  EXPORT_SYMBOL(drm_crtc_vblank_count_and_time);
> > > > > >
> > > > > > +/**
> > > > > > + * drm_crtc_next_vblank_time - calculate the time of the next vblank
> > > > > > + * @crtc: the crtc for which to calculate next vblank time
> > > > > > + * @vblanktime: pointer to time to receive the next vblank timestamp.
> > > > > > + *
> > > > > > + * Calculate the expected time of the next vblank based on time of previous
> > > > > > + * vblank and frame duration    
> > > > >
> > > > > Hi,
> > > > >
> > > > > for VRR this targets the highest frame rate possible for the current
> > > > > VRR mode, right?
> > > > >    
> > > > 
> > > > It is based on vblank->framedur_ns which is in turn based on
> > > > mode->crtc_clock.  Presumably for VRR that ends up being a maximum?  
> > > 
> > > I don't know. :-)  
> > 
> > At least for i915 this will give you the maximum frame
> > duration.
> 
> Really maximum duration? So minimum VRR frequency?

Yes. Doing otherwise would complicate the actual
timestamp calculation even further.

The actual timestamps i915 generates will however match
the start of active video, regardless of how long vblank
was extended.

The only exception might be if you query the timestamp
during vblank but VRR exit has not yet been triggered,
ie. not commit has been made during the frame. In that
case the timestamp will correspond to the max frame
duration, which may or may not end up being the case.
Depends totally whether a commit will still happen
during the vblank to trigger an early vblank exit.

> 
> > Also this does not calculate the the start of vblank, it
> > calculates the start of active video.
> 
> Oh indeed, so it's too late. What one would actually need for the
> deadline is the driver's deadline to present for the immediately next
> start of active video.
> 
> And with VRR that should probably aim for the maximum frame frequency,
> not minimum?

Yeah, max frame rate seems like the easiest thing to use there.

The other option might be some average value based on recent
history, but figuring tht out would seem like a lot more work.

-- 
Ville Syrjälä
Intel