Re: [PATCH 1/3] drm: Widen vblank UAPI to 64 bits. Change vblank time to ktime_t [v2]

From: "Michel Dänzer" <michel@daenzer.net>
To: Keith Packard <keithp@keithp.com>, Dave Airlie <airlied@redhat.com>
Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 1/3] drm: Widen vblank UAPI to 64 bits. Change vblank time to ktime_t [v2]
Date: Wed, 2 Aug 2017 18:41:25 +0900	[thread overview]
Message-ID: <75407a09-e365-93c4-50d9-a0efa48447cf@daenzer.net> (raw)
In-Reply-To: <20170802085358.cipsolpgxlb2e323@phenom.ffwll.local>

On 02/08/17 05:53 PM, Daniel Vetter wrote:
> 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 <michel@daenzer.net>
>> Suggested-by: Daniel Vetter <daniel@ffwll.ch>
>> Signed-off-by: Keith Packard <keithp@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.vetter@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.

I don't think changing max_vblank_count to u64 is necessary/useful;
other than that, AFAICT the issues I raised before for this patch have
been addressed. I'm afraid I don't know if/when I'll get a chance to
review the whole patch in detail though.

-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer