Daniel Vetter <dan...@ffwll.ch> writes: > Subject is a bit confusing since you say uapi, but this is just the > internal prep work. Dropping UAPI fixes that. With that fixed:

## Advertising

Yeah, thanks. > Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch> Added. > 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. I also need to re-write the reply.sequence value in the queue function; seems like passing in the vblwait is a simpler plan. >> +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) ? Oh, yes, that makes perfect sense -- an int32_t will obviously hold the shortest distance between the two, whether negative or positive. Of course, '(uint32_t) wide' is just 'narrow'. > But then it took me way too long to think about this one, so maybe leave > it at that. Your version is a lot shorter, and I think it's actually clearer. How about static inline uint64_t widen_32_to_64(uint32_t narrow, uint64_t near) { return near + (int32_t) (narrow - (uint32_t) near); } Here's a test program which validates the widen function.

#include <stdint.h> #include <assert.h> #include <stdlib.h> #include <stdio.h> static inline uint64_t widen_32_to_64(uint32_t narrow, uint64_t near) { return near + (int32_t) (narrow - (uint32_t) near); } uint64_t random_u64(void) { assert(sizeof (long int) == 8); return (uint32_t) mrand48() | (((uint64_t) (uint32_t) mrand48()) << 32); } uint32_t random_u32(int bits) { return random_u64() & ((1UL << bits) - 1); } int32_t random_s32(int bits) { int32_t value = (int32_t) random_u32(bits); return value << (32 - bits) >> (32 - bits); } int main(int argc, char **argv) { int bits; int tries; /* Validate random number generators */ for (bits = 1; bits <= 32; bits++) { int64_t max = ((1L << (bits-1)) - 1); int32_t min = -max - 1; int32_t range_min = INT32_MAX; int32_t range_max = INT32_MIN; for (tries = 0; tries < 100000; tries++) { int32_t i = random_s32(bits); if (i < min || max < i) { printf ("min %d i %d max %d\n", min, i, max); exit(1); } if (i < range_min) range_min = i; if (i > range_max) range_max = i; } if (range_min >= min/2 || (range_max <= max/2 && max != 0)) { printf ("bits %d min %d max %d range min %d range max %d\n", bits, min, max, range_min, range_max); exit(1); } } /* Check to make sure the 'widen' function generates the right answer */ for (bits = 1; bits < 32; bits++) { for (tries = 0; tries < 100000; tries++) { /* A random 64-bit value */ uint64_t near = random_u64(); /* Compute a nearby value, within a 32-bit delta of the target*/ int32_t delta = random_s32(bits); uint64_t value = near + delta; /* Narrow the value to 32-bits */ uint32_t narrow = (uint32_t) (value); /* Use our 'widen' function to reconstruct the wider value */ uint64_t wide = widen_32_to_64(narrow, near); /* Make sure the reconstruction worked */ if (wide != value) printf ("widen failed near %ld value %ld wide %ld\n", near, value, wide); } } }

-- -keith

**
signature.asc**

*Description:* PGP signature