| Message ID | 1470260019-6173-6-git-send-email-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Aug 03, 2016 at 02:33:38PM -0700, Rodrigo Vivi wrote: > In modern systems there are situations that you can let the screen > enabled and sleep or shut off a most of the display controler. > > In situations like this the vblank hw counter can be reset. > When this happens everything in the system gets crazy by > the big count. > > So, the right approach is to make sure we are avoiding any > runtime suspend when vblank is enabled but also restore > drm crtc vblank counter to a state we can rely. > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/drm_irq.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ > include/drm/drm_irq.h | 1 + > 2 files changed, 47 insertions(+) > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > index d0d1dde..7320836 100644 > --- a/drivers/gpu/drm/drm_irq.c > +++ b/drivers/gpu/drm/drm_irq.c > @@ -265,6 +265,52 @@ u32 drm_accurate_vblank_count(struct drm_crtc *crtc) > } > EXPORT_SYMBOL(drm_accurate_vblank_count); > > +/** > + * drm_crtc_vblank_sanitize_counter - Sanitize vblank counter > + * @crtc: which counter to sanitize > + * > + * This function returns drm crtc vblank counter to the latest > + * known counter. > + * > + * This function is useful for runtime suspend where the hw > + * counter or timer might reset. > + * > + * Use this only when there is no risk of the runtime suspend > + * reseting hardware counter and before enabling vblank irq. I think this needs to be a bit more precise, and should reference the prepare/unprepare hooks. > + */ > +void drm_crtc_vblank_sanitize_counter(struct drm_crtc *crtc) sanitize_counter is a bit an unclear function name. I think resync_counter would be better, but still not good really. > +{ > + struct drm_device *dev = crtc->dev; > + unsigned int pipe = drm_crtc_index(crtc); > + struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; > + unsigned long irqflags; > + struct timeval t_vblank; > + int count = DRM_TIMESTAMP_MAXRETRIES; > + u32 cur_vblank; > + bool rc; > + > + spin_lock_irqsave(&dev->vblank_time_lock, irqflags); > + > + if (vblank->enabled) { If we put the sanitize call like I suggested, this would be a driver bug. We don't want to sanitize it when it's already sanitized and nothing could have corrupted it meanwhile, and especially not when the vblank counter is in use. Should be a WARN_ON, not debug level. > + DRM_DEBUG_VBL("Skip sanitize - Vblank is enabled on pipe %d\n", > + pipe); > + goto unlock; > + } > + > + do { > + cur_vblank = dev->driver->get_vblank_counter(dev, pipe); > + rc = drm_get_last_vbltimestamp(dev, pipe, &t_vblank, 0); > + } while (cur_vblank != dev->driver->get_vblank_counter(dev, pipe) && --count > 0); > + > + if (!rc) > + t_vblank = (struct timeval) {0, 0}; > + > + store_vblank(dev, pipe, 0, &t_vblank, cur_vblank); We also need to sufficiently fake the missed vblanks by comparing the last timestamp with the new one and dividing the elapsed time by the frame time. Ville has added such logic to already to make sure we correctly account for the time between drm_vblank_on and drm_vblank_off when the vblank counter is not running. Please reuse that code to make that adjustment, instead of open-code it. > +unlock: > + spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags); > +} > +EXPORT_SYMBOL(drm_crtc_vblank_sanitize_counter); > + > /* > * Disable vblank irq's on crtc, make sure that last vblank count > * of hardware and corresponding consistent software vblank counter > diff --git a/include/drm/drm_irq.h b/include/drm/drm_irq.h > index 93a9e9d..55c419a 100644 > --- a/include/drm/drm_irq.h > +++ b/include/drm/drm_irq.h > @@ -160,6 +160,7 @@ extern u32 drm_vblank_count(struct drm_device *dev, unsigned int pipe); > extern u32 drm_crtc_vblank_count(struct drm_crtc *crtc); > extern u32 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc, > struct timeval *vblanktime); > +extern void drm_crtc_vblank_sanitize_counter(struct drm_crtc *crtc); > extern void drm_crtc_send_vblank_event(struct drm_crtc *crtc, > struct drm_pending_vblank_event *e); > extern void drm_crtc_arm_vblank_event(struct drm_crtc *crtc, > -- > 2.4.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index d0d1dde..7320836 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -265,6 +265,52 @@ u32 drm_accurate_vblank_count(struct drm_crtc *crtc) } EXPORT_SYMBOL(drm_accurate_vblank_count); +/** + * drm_crtc_vblank_sanitize_counter - Sanitize vblank counter + * @crtc: which counter to sanitize + * + * This function returns drm crtc vblank counter to the latest + * known counter. + * + * This function is useful for runtime suspend where the hw + * counter or timer might reset. + * + * Use this only when there is no risk of the runtime suspend + * reseting hardware counter and before enabling vblank irq. + */ +void drm_crtc_vblank_sanitize_counter(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + unsigned int pipe = drm_crtc_index(crtc); + struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; + unsigned long irqflags; + struct timeval t_vblank; + int count = DRM_TIMESTAMP_MAXRETRIES; + u32 cur_vblank; + bool rc; + + spin_lock_irqsave(&dev->vblank_time_lock, irqflags); + + if (vblank->enabled) { + DRM_DEBUG_VBL("Skip sanitize - Vblank is enabled on pipe %d\n", + pipe); + goto unlock; + } + + do { + cur_vblank = dev->driver->get_vblank_counter(dev, pipe); + rc = drm_get_last_vbltimestamp(dev, pipe, &t_vblank, 0); + } while (cur_vblank != dev->driver->get_vblank_counter(dev, pipe) && --count > 0); + + if (!rc) + t_vblank = (struct timeval) {0, 0}; + + store_vblank(dev, pipe, 0, &t_vblank, cur_vblank); +unlock: + spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags); +} +EXPORT_SYMBOL(drm_crtc_vblank_sanitize_counter); + /* * Disable vblank irq's on crtc, make sure that last vblank count * of hardware and corresponding consistent software vblank counter diff --git a/include/drm/drm_irq.h b/include/drm/drm_irq.h index 93a9e9d..55c419a 100644 --- a/include/drm/drm_irq.h +++ b/include/drm/drm_irq.h @@ -160,6 +160,7 @@ extern u32 drm_vblank_count(struct drm_device *dev, unsigned int pipe); extern u32 drm_crtc_vblank_count(struct drm_crtc *crtc); extern u32 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc, struct timeval *vblanktime); +extern void drm_crtc_vblank_sanitize_counter(struct drm_crtc *crtc); extern void drm_crtc_send_vblank_event(struct drm_crtc *crtc, struct drm_pending_vblank_event *e); extern void drm_crtc_arm_vblank_event(struct drm_crtc *crtc,
In modern systems there are situations that you can let the screen enabled and sleep or shut off a most of the display controler. In situations like this the vblank hw counter can be reset. When this happens everything in the system gets crazy by the big count. So, the right approach is to make sure we are avoiding any runtime suspend when vblank is enabled but also restore drm crtc vblank counter to a state we can rely. Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/drm_irq.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_irq.h | 1 + 2 files changed, 47 insertions(+)