| Message ID | 1429033302-16353-2-git-send-email-mario.kleiner.de@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 15.04.2015 02:41, Mario Kleiner wrote: > For a kms driver to support immediate disable of vblank > irq's reliably without introducing off by one errors or > other mayhem for clients, it must not only support a > hardware vblank counter query, but also high precision > vblank timestamping, so vblank count and timestamp can be > instantaneously reinitialzed to valid values. Additionally > the exposed hardware counter must behave as if it is > incrementing at leading edge of vblank to avoid off by > one errors during reinitialization of the counter while > the display happens to be inside or close to vblank. > > Check during drm_vblank_init that a driver which claims to > be capable of vblank_disable_immediate at least supports > high precision timestamping and prevent use of instant > disable if that isn't present as a minimum requirement. > > Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Michel Dänzer <michel@daenzer.net> > Cc: Thierry Reding <treding@nvidia.com> > Cc: Dave Airlie <airlied@redhat.com> > --- > drivers/gpu/drm/drm_irq.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > index af9662e..6efe822 100644 > --- a/drivers/gpu/drm/drm_irq.c > +++ b/drivers/gpu/drm/drm_irq.c > @@ -336,6 +336,12 @@ int drm_vblank_init(struct drm_device *dev, int num_crtcs) > else > DRM_INFO("No driver support for vblank timestamp query.\n"); > > + /* Must have precise timestamping for reliable vblank instant disable */ > + if (dev->vblank_disable_immediate && !dev->driver->get_vblank_timestamp) { > + dev->vblank_disable_immediate = false; > + DRM_ERROR("Set vblank_disable_immediate, but not supported.\n"); > + } I think DRM_ERROR is kind of a bad compromise for this. If this is considered a driver bug, something like WARN_ONCE would be better to draw attention to the culprit. Otherwise, maybe something like DRM_INFO("Setting vblank_disable_immediate to false because " "get_vblank_timestamp == NULL\n"); would be both clearer and less alarming. Other than that, looks good to me.
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index af9662e..6efe822 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -336,6 +336,12 @@ int drm_vblank_init(struct drm_device *dev, int num_crtcs) else DRM_INFO("No driver support for vblank timestamp query.\n"); + /* Must have precise timestamping for reliable vblank instant disable */ + if (dev->vblank_disable_immediate && !dev->driver->get_vblank_timestamp) { + dev->vblank_disable_immediate = false; + DRM_ERROR("Set vblank_disable_immediate, but not supported.\n"); + } + dev->vblank_disable_allowed = false; return 0;
For a kms driver to support immediate disable of vblank irq's reliably without introducing off by one errors or other mayhem for clients, it must not only support a hardware vblank counter query, but also high precision vblank timestamping, so vblank count and timestamp can be instantaneously reinitialzed to valid values. Additionally the exposed hardware counter must behave as if it is incrementing at leading edge of vblank to avoid off by one errors during reinitialization of the counter while the display happens to be inside or close to vblank. Check during drm_vblank_init that a driver which claims to be capable of vblank_disable_immediate at least supports high precision timestamping and prevent use of instant disable if that isn't present as a minimum requirement. Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Michel Dänzer <michel@daenzer.net> Cc: Thierry Reding <treding@nvidia.com> Cc: Dave Airlie <airlied@redhat.com> --- drivers/gpu/drm/drm_irq.c | 6 ++++++ 1 file changed, 6 insertions(+)