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@xxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Michel Dänzer <michel@xxxxxxxxxxx>
> Cc: Thierry Reding <treding@xxxxxxxxxx>
> Cc: Dave Airlie <airlied@xxxxxxxxxx>
> ---
> 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.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
