[Mesa-dev] [PATCH 01/18] i965: Query whether we have kernel support for the TIMESTAMP register once

[Kenneth Graunke]

On Monday, July 06, 2015 05:12:10 PM Chris Wilson wrote:
> On Mon, Jul 06, 2015 at 04:19:36PM +0300, Martin Peres wrote:
> > 
> > 
> > On 06/07/15 16:15, Martin Peres wrote:
> > >On 06/07/15 16:13, Chris Wilson wrote:
> > >>On Mon, Jul 06, 2015 at 03:10:48PM +0300, Martin Peres wrote:
> > >>>On 06/07/15 13:33, Chris Wilson wrote:
> > >>>>Move the query for the TIMESTAMP register from context init to the
> > >>>>screen, so that it is only queried once for all contexts.
> > >>>>
> > >>>>On 32bit systems, some old kernels trigger a hw bug resulting in the
> > >>>>TIMESTAMP register being shifted and the low bits always zero. Detect
> > >>>>this by repeating the read a few times and check the register is
> > >>>>incrementing.
> > >>>You do not do the latter. You only check for the low bits.
> > >>>
> > >>>I guess the counter is supposed to be monotonically increasing and
> > >>>with a resolution of a few microseconds which would make this
> > >>>perfectly valid. Could you confirm and make sure to add this
> > >>>information in the commit message please?
> > >>The counter should increment every 80ns. What's misleading in what I
> > >>wrote? It describes the hw bug and how to detect it.
> > >
> > >Well, it is not misleading, it just lacks this information.
> > >
> > >If it incremented every seconds, the patch would be stupid because
> > >the timestamp could be at 0 and polling 10 times at a few us of
> > >interval would always yield the same result. That's all :)
> > 
> > Oh, forgot to say: With this information added in the commit message
> > and the commit message duplicated as a comment in
> > intel_detect_timestamp(), the patch is:
> 
> How about:
> 
> On 32bit systems, some old kernels trigger a hw bug resulting in the
> TIMESTAMP register being shifted and the low 32bits always zero. Detect
> this by repeating the read a few times and check the register is
> incrementing every 80ns as expected and not stuck on zero (as would be 
> the case with the buggy kernel/hw.).
> -Chris
> 
> 

It would be great to put this in a comment above the loop rather than
only in the commit message.

Also, moving the check to screen-time and fixing the check to use a loop
are really two separate things - but they're so trivial I'm inclined to
not be picky. :)

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150706/4d2dac06/attachment.sig>