Re: [PATCH 1/3] drm: Widen vblank count to 64 bits. Change vblank time precision to ns

From: Keith Packard <keithp@keithp.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: linux-kernel@vger.kernel.org, Dave Airlie <airlied@redhat.com>,
	Daniel Vetter <daniel@ffwll.ch>,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 1/3] drm: Widen vblank count to 64 bits. Change vblank time precision to ns
Date: Thu, 06 Jul 2017 07:59:51 -0700	[thread overview]
Message-ID: <86tw2pd314.fsf@keithp.com> (raw)
In-Reply-To: <20170706071948.pphlrqjcyas3zdfy@phenom.ffwll.local>

[-- Attachment #1: Type: text/plain, Size: 1766 bytes --]

Daniel Vetter <daniel@ffwll.ch> writes:

> Extending the reported/sw vblank counter to u64 makes sense imo, but do we
> have to extend the driver interfaces too? If there's no 64 bit hw vblank
> currently I think I'd be good to postpone that part, simply because I'm
> too lazy to audit all the drivers for correctly setting max_vblank_count
> after your change :-)

As I said, it's easy enough to do that; I figured I'd do the obvious
part and let you decide if you wanted that or not. We could also
set max_vblank_count to 0xffffffff if it wasn't set by the driver,
and/or add a WARN_ON_ONCE if it wasn't set. Given that it takes over two
years to wrap this counter at 60Hz, we're never likely to hit a bug in
testing.

Let me know what you think; I'm not invested in any particular solution
at this point.

> Other thought on this, since you bother to change all the types: Afaik
> both timespec and timeval suffer from the 32bit issues.

I'm not sure what 32bit issues you're concerned about here? We don't
compare these values, just report them up to user space.

> If we bother with changing everything I think it'd be neat to switch
> all internal interfaces over to ktime, and only convert to the
> userspace types once when we generate the event. I think that's how
> cool hackers are supposed to do it, but not fully sure.

Yeah, I can definitely get behind that plan. A simple 64-bit value
instead of a struct with two semi-related values which are hard to do
arithmetic on.

> Otherwise looks all good, but haven't yet carefully hunted for fumbles in
> review before the above is clear.

Thanks. I'll switch over to ktime and wait to hear what your thoughts
are on the vblank count interface changes.

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]