On Mon, Jul 31, 2017 at 10:03:04PM -0700, Keith Packard wrote:
> This modifies the datatypes used by the vblank code to provide both 64
> bits of vblank count and switch to using ktime_t for timestamps to
> increase resolution from microseconds to nanoseconds.
> 
> The driver interfaces have been left using 32 bits of vblank count;
> all of the code necessary to widen that value for the user API was
> already included to handle devices returning fewer than 32-bits.
> 
> This will provide the necessary datatypes for the Vulkan API.
> 
> v2:
> 
>  * Re-write wait_vblank ioctl to ABSOLUTE sequence
> 
>       When an application uses the WAIT_VBLANK ioctl with RELATIVE
>       or NEXTONMISS bits set, the target vblank interval is updated
>       within the kernel. We need to write that target back to the
>       ioctl buffer and update the flags bits so that if the wait is
>       interrupted by a signal, when it is re-started, it will target
>       precisely the same vblank count as before.
> 
>  * Leave driver API with 32-bit vblank count
> 
> Suggested-by:  Michel Dänzer <mic...@daenzer.net>
> Suggested-by: Daniel Vetter <dan...@ffwll.ch>
> Signed-off-by: Keith Packard <kei...@keithp.com>

Subject is a bit confusing since you say uapi, but this is just the
internal prep work. Dropping UAPI fixes that. With that fixed:

Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>

Two more optional comments below, feel free to adapt or ignore. I'll wait
for Michel's r-b before merging either way.

>  static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
> +                               u64 req_seq,
>                                 union drm_wait_vblank *vblwait,

Minor bikeshed: Since you pass the requested vblank number explicit, mabye
also pass the user_data explicit and remove the vblwait struct from the
parameter list? Restricts the old uapi cruft a bit.

>  /*
> + * Widen a 32-bit param to 64-bits.
> + *
> + * \param narrow 32-bit value (missing upper 32 bits)
> + * \param near 64-bit value that should be 'close' to near
> + *
> + * This function returns a 64-bit value using the lower 32-bits from
> + * 'narrow' and constructing the upper 32-bits so that the result is
> + * as close as possible to 'near'.
> + */
> + 
> +static u64 widen_32_to_64(u32 narrow, u64 near)
> +{
> +     u64 wide = narrow | (near & 0xffffffff00000000ULL);
> +     if ((int64_t) (wide - near) > 0x80000000LL)
> +             wide -= 0x100000000ULL;
> +     else if ((int64_t) (near - wide) > 0x80000000LL)
> +             wide += 0x100000000ULL;
> +     return wide;

return near + (int32_s) ((uint32_t)wide - near) ?

But then it took me way too long to think about this one, so maybe leave
it at that.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Reply via email to