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 <kenn...@whitecape.org>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
