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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230221151133.6392c521@eldfell>
Date:   Tue, 21 Feb 2023 15:11:33 +0200
From:   Pekka Paalanen <ppaalanen@...il.com>
To:     Ville Syrjälä <ville.syrjala@...ux.intel.com>
Cc:     Rob Clark <robdclark@...il.com>,
        Rob Clark <robdclark@...omium.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Tvrtko Ursulin <tvrtko.ursulin@...el.com>,
        Christian König <ckoenig.leichtzumerken@...il.com>,
        Michel Dänzer <michel@...nzer.net>,
        open list <linux-kernel@...r.kernel.org>,
        dri-devel@...ts.freedesktop.org,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        Alex Deucher <alexander.deucher@....com>,
        freedreno@...ts.freedesktop.org
Subject: Re: [PATCH v4 10/14] drm/vblank: Add helper to get next vblank time

On Tue, 21 Feb 2023 15:01:35 +0200
Ville Syrjälä <ville.syrjala@...ux.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@...il.com> wrote:
> >   
> > > On Mon, Feb 20, 2023 at 1:08 AM Pekka Paalanen <ppaalanen@...il.com> wrote:  
> > > >
> > > > On Sat, 18 Feb 2023 13:15:53 -0800
> > > > Rob Clark <robdclark@...il.com> wrote:
> > > >    
> > > > > From: Rob Clark <robdclark@...omium.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@...omium.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?

> 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?



Thanks,
pq

> 
> > 
> > You need a number of clock cycles in addition to the clock frequency,
> > and that could still be minimum, maximum, the last realized one, ...
> > 
> > VRR works by adjusting the front porch length IIRC.
> > 
> > 
> > Thanks,
> > pq
> >   
> > > BR,
> > > -R
> > > 
> > >   
> > > >
> > > > Thanks,
> > > > pq
> > > >    
> > > > > + */
> > > > > +int drm_crtc_next_vblank_time(struct drm_crtc *crtc, ktime_t *vblanktime)
> > > > > +{
> > > > > +     unsigned int pipe = drm_crtc_index(crtc);
> > > > > +     struct drm_vblank_crtc *vblank = &crtc->dev->vblank[pipe];
> > > > > +     u64 count;
> > > > > +
> > > > > +     if (!vblank->framedur_ns)
> > > > > +             return -EINVAL;
> > > > > +
> > > > > +     count = drm_vblank_count_and_time(crtc->dev, pipe, vblanktime);
> > > > > +
> > > > > +     /*
> > > > > +      * If we don't get a valid count, then we probably also don't
> > > > > +      * have a valid time:
> > > > > +      */
> > > > > +     if (!count)
> > > > > +             return -EINVAL;
> > > > > +
> > > > > +     *vblanktime = ktime_add(*vblanktime, ns_to_ktime(vblank->framedur_ns));
> > > > > +
> > > > > +     return 0;
> > > > > +}
> > > > > +EXPORT_SYMBOL(drm_crtc_next_vblank_time);
> > > > > +
> > > > >  static void send_vblank_event(struct drm_device *dev,
> > > > >               struct drm_pending_vblank_event *e,
> > > > >               u64 seq, ktime_t now)
> > > > > diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> > > > > index 733a3e2d1d10..a63bc2c92f3c 100644
> > > > > --- a/include/drm/drm_vblank.h
> > > > > +++ b/include/drm/drm_vblank.h
> > > > > @@ -230,6 +230,7 @@ bool drm_dev_has_vblank(const struct drm_device *dev);
> > > > >  u64 drm_crtc_vblank_count(struct drm_crtc *crtc);
> > > > >  u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
> > > > >                                  ktime_t *vblanktime);
> > > > > +int drm_crtc_next_vblank_time(struct drm_crtc *crtc, ktime_t *vblanktime);
> > > > >  void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
> > > > >                              struct drm_pending_vblank_event *e);
> > > > >  void drm_crtc_arm_vblank_event(struct drm_crtc *crtc,    
> > > >    
> >   
> 
> 
> 


Content of type "application/pgp-signature" skipped