Re: [PATCH 1/3] drm: Widen vblank UAPI to 64 bits. Change vblank time to ktime_t [v2]
From: Daniel Vetter
Date: Wed Aug 02 2017 - 04:54:59 EST
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.
> * 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 <michel@xxxxxxxxxxx>
> Suggested-by: Daniel Vetter <daniel@xxxxxxxx>
> Signed-off-by: Keith Packard <keithp@xxxxxxxxxx>
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.vetter@xxxxxxxx>
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.
Software Engineer, Intel Corporation