Re: [PATCH 1/3] drm: Widen vblank count to 64 bits. Change vblank time precision to ns
From: Keith Packard
Date: Thu Jul 06 2017 - 11:04:56 EST
Michel DÃnzer <michel@xxxxxxxxxxx> writes:
> Subtle breakage here: vblwait->request.sequence must still get updated
> for _DRM_VBLANK_RELATIVE, in case we're interrupted by a signal.
Thanks for finding this.
I think it might be better to just not modify the request.type field
instead, so that on re-entry it gets recomputed? That would mean that a
signal might cause the value to be different if the application takes a
long time processing the signal, but I'm not sure that's wrong?
>> @@ -317,6 +317,9 @@ int via_driver_irq_postinstall(struct drm_device *dev)
>> if (!dev_priv)
>> return -EINVAL;
>> + if (dev->driver->get_vblank_counter)
>> + dev->max_vblank_count = 0xffffffff;
> What's the purpose of this? All drivers providing get_vblank_counter
> should already initialize max_vblank_count correctly.
Yeah, I couldn't prove that this driver did that, and as Daniel says, we
haven't ever audited the drivers to make sure they do.
We have a check to see that they don't set max_vblank_count if they
don't provide a get function, but I can't find the matching check for
drivers that do provide a function and aren't setting max_vblank_count.
Do you have any thoughts on the wisdom of changing this API before we
have a driver that needs it?
And, of course, thanks for your review!
Description: PGP signature