| Message ID | 20170315204027.20160-3-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 15, 2017 at 08:40:27PM +0000, Chris Wilson wrote: > Bypass all the spinlocks and return the last timestamp and counter from > the last vblank if the driver delcares that it is accurate (and stable > across on/off), and the vblank is currently enabled. > > This is dependent upon the both the hardware and driver to provide the > proper barriers to facilitate reading our bookkeeping outside of the > vblank interrupt and outside of the explicit vblank locks. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Daniel Vetter <daniel@ffwll.ch> > Cc: Michel Dänzer <michel@daenzer.net> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Dave Airlie <airlied@redhat.com>, > Cc: Mario Kleiner <mario.kleiner.de@gmail.com> > --- > drivers/gpu/drm/drm_irq.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > index 53a526c7b24d..c55abce152a2 100644 > --- a/drivers/gpu/drm/drm_irq.c > +++ b/drivers/gpu/drm/drm_irq.c > @@ -1559,6 +1559,17 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe, > return ret; > } > > +static bool drm_wait_vblank_is_query(union drm_wait_vblank *vblwait) > +{ > + if (vblwait->request.sequence) > + return false; > + > + return _DRM_VBLANK_RELATIVE == > + (vblwait->request.type & (_DRM_VBLANK_TYPES_MASK | > + _DRM_VBLANK_EVENT | > + _DRM_VBLANK_NEXTONMISS)); > +} > + > /* > * Wait for VBLANK. > * > @@ -1608,6 +1619,21 @@ int drm_wait_vblank(struct drm_device *dev, void *data, > > vblank = &dev->vblank[pipe]; > > + /* If the counter is currently enabled and accurate, short-circuit queries > + * to return the cached timestamp of the last vblank. > + */ > + if (dev->vblank_disable_immediate && > + drm_wait_vblank_is_query(vblwait) && > + vblank->enabled) { Hmm. I'm wondering if this could give us something stale if we're just about to enable the vblank interrupt. We seem to set enabled=true before we update the count/ts. So I'm thinking we'd need to flip those around and make sure proper barriers are in place. > + struct timeval now; > + > + vblwait->reply.sequence = > + drm_vblank_count_and_time(dev, pipe, &now); > + vblwait->reply.tval_sec = now.tv_sec; > + vblwait->reply.tval_usec = now.tv_usec; > + return 0; > + } > + > ret = drm_vblank_get(dev, pipe); > if (ret) { > DRM_DEBUG("crtc %d failed to acquire vblank counter, %d\n", pipe, ret); > -- > 2.11.0
On Wed, Mar 15, 2017 at 11:06:32PM +0200, Ville Syrjälä wrote: > > @@ -1608,6 +1619,21 @@ int drm_wait_vblank(struct drm_device *dev, void *data, > > > > vblank = &dev->vblank[pipe]; > > > > + /* If the counter is currently enabled and accurate, short-circuit queries > > + * to return the cached timestamp of the last vblank. > > + */ > > + if (dev->vblank_disable_immediate && > > + drm_wait_vblank_is_query(vblwait) && > > + vblank->enabled) { > > Hmm. I'm wondering if this could give us something stale if we're just > about to enable the vblank interrupt. > > We seem to set enabled=true before we update the count/ts. So I'm > thinking we'd need to flip those around and make sure proper barriers > are in place. Yes, that is a valid concern. -Chris
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 53a526c7b24d..c55abce152a2 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -1559,6 +1559,17 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe, return ret; } +static bool drm_wait_vblank_is_query(union drm_wait_vblank *vblwait) +{ + if (vblwait->request.sequence) + return false; + + return _DRM_VBLANK_RELATIVE == + (vblwait->request.type & (_DRM_VBLANK_TYPES_MASK | + _DRM_VBLANK_EVENT | + _DRM_VBLANK_NEXTONMISS)); +} + /* * Wait for VBLANK. * @@ -1608,6 +1619,21 @@ int drm_wait_vblank(struct drm_device *dev, void *data, vblank = &dev->vblank[pipe]; + /* If the counter is currently enabled and accurate, short-circuit queries + * to return the cached timestamp of the last vblank. + */ + if (dev->vblank_disable_immediate && + drm_wait_vblank_is_query(vblwait) && + vblank->enabled) { + struct timeval now; + + vblwait->reply.sequence = + drm_vblank_count_and_time(dev, pipe, &now); + vblwait->reply.tval_sec = now.tv_sec; + vblwait->reply.tval_usec = now.tv_usec; + return 0; + } + ret = drm_vblank_get(dev, pipe); if (ret) { DRM_DEBUG("crtc %d failed to acquire vblank counter, %d\n", pipe, ret);
Bypass all the spinlocks and return the last timestamp and counter from the last vblank if the driver delcares that it is accurate (and stable across on/off), and the vblank is currently enabled. This is dependent upon the both the hardware and driver to provide the proper barriers to facilitate reading our bookkeeping outside of the vblank interrupt and outside of the explicit vblank locks. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Michel Dänzer <michel@daenzer.net> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Dave Airlie <airlied@redhat.com>, Cc: Mario Kleiner <mario.kleiner.de@gmail.com> --- drivers/gpu/drm/drm_irq.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)