Daniel Vetter <daniel@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:

Yeah, thanks.

> Reviewed-by: Daniel Vetter <daniel.vetter@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.