On Thu, Jul 23, 2015 at 06:35:11PM +1200, Chris Forbes wrote:
> This fixes my HSW getting dropped back to 3.2 most of the time, and
> seems like the reasonable thing to do.
>
> Tested-and-acked-by: Chris Forbes <chr...@ijw.co.nz>
>
> On Tue, Jul 21, 2015 at 11:58 PM, Chris Wilson <ch...@chris-wilson.co.uk>
> wrote:
> > I was mistaken, I thought we already had fixed this in the kernel a
> > couple of years ago. We had not, and the broken read (the hardware
> > shifts the register output on 64bit kernels, but not on 32bit kernels) is
> > now enshrined into the ABI. I also had the buggy architecture reversed,
> > believing it to be 32bit that had the shifted results. On the basis of
> > those mistakes, I wrote
> >
> > commit c8d3ebaffc0d7d915c1c19d54dba61fd1e57b338
> > Author: Chris Wilson <ch...@chris-wilson.co.uk>
> > Date: Wed Apr 29 13:32:38 2015 +0100
> >
> > i965: Query whether we have kernel support for the TIMESTAMP register
> > once
> >
> > Now that we do have an extended register read interface for always
> > reporting the full 36bit TIMESTAMP (irrespective of whether the kernel
> > is buggy or not), make use of it and in the process fix my reversed
> > detection of the buggy reads for unpatched kernels.
> >
> > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> > Cc: Martin Peres <martin.pe...@linux.intel.com>
> > Cc: Kenneth Graunke <kenn...@whitecape.org>
> > Cc: Michał Winiarski <michal.winiar...@intel.com>
> > Cc: Daniel Vetter <dan...@ffwll.ch>
> > ---
> > src/mesa/drivers/dri/i965/brw_queryobj.c | 15 ++++++++--
> > src/mesa/drivers/dri/i965/intel_screen.c | 49
> > +++++++++++++++++++++++---------
> > src/mesa/drivers/dri/i965/intel_screen.h | 2 +-
> > 3 files changed, 49 insertions(+), 17 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_queryobj.c
> > b/src/mesa/drivers/dri/i965/brw_queryobj.c
> > index aea4d9b..6f1accd 100644
> > --- a/src/mesa/drivers/dri/i965/brw_queryobj.c
> > +++ b/src/mesa/drivers/dri/i965/brw_queryobj.c
> > @@ -497,13 +497,22 @@ brw_get_timestamp(struct gl_context *ctx)
> > struct brw_context *brw = brw_context(ctx);
> > uint64_t result = 0;
> >
> > - drm_intel_reg_read(brw->bufmgr, TIMESTAMP, &result);
> > + switch (brw->intelScreen->hw_has_timestamp) {
> > + case 3: /* New kernel, always full 36bit accuracy */
> > + drm_intel_reg_read(brw->bufmgr, TIMESTAMP | 1, &result);
> > + break;
> > + case 2: /* 64bit kernel, result is right-shifted by 32bits, losing
> > 4bits */
> > + drm_intel_reg_read(brw->bufmgr, TIMESTAMP, &result);
> > + result = result >> 32;
> > + break;
> > + case 1: /* 32bit kernel, result is 36bit wide but may be inaccurate! */
> > + drm_intel_reg_read(brw->bufmgr, TIMESTAMP, &result);
> > + break;
> > + }
Maybe do an enum with symbolic names instead of an int for
hw_has_timestamp, but that's kinda a bikeshed. With or without that
Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>
> >
> > /* See logic in brw_queryobj_get_results() */
> > - result = result >> 32;
> > result *= 80;
> > result &= (1ull << 36) - 1;
> > -
> > return result;
> > }
> >
> > diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
> > b/src/mesa/drivers/dri/i965/intel_screen.c
> > index 1470b05..65a1766 100644
> > --- a/src/mesa/drivers/dri/i965/intel_screen.c
> > +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> > @@ -1123,25 +1123,48 @@ intel_detect_swizzling(struct intel_screen *screen)
> > return true;
> > }
> >
> > -static bool
> > +static int
> > intel_detect_timestamp(struct intel_screen *screen)
> > {
> > - uint64_t dummy = 0;
> > - int loop = 10;
> > + uint64_t dummy = 0, last = 0;
> > + int upper, lower, loops;
> >
> > - /*
> > - * 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.).
> > + /* On 64bit systems, some old kernels trigger a hw bug resulting in the
> > + * TIMESTAMP register being shifted and the low 32bits always zero.
> > + *
> > + * More recent kernels offer an interface to read the full 36bits
> > + * everywhere.
> > */
> > - do {
> > + if (drm_intel_reg_read(screen->bufmgr, TIMESTAMP | 1, &dummy) == 0)
> > + return 3;
> > +
> > + /* Determine if we have a 32bit or 64bit kernel by inspecting the
> > + * upper 32bits for a rapidly changing timestamp.
> > + */
> > + if (drm_intel_reg_read(screen->bufmgr, TIMESTAMP, &last))
> > + return 0;
> > +
> > + upper = lower = 0;
> > + for (loops = 0; loops < 10; loops++) {
> > + /* The TIMESTAMP should change every 80ns, so several round trips
> > + * through the kernel should be enough to advance it.
> > + */
> > if (drm_intel_reg_read(screen->bufmgr, TIMESTAMP, &dummy))
> > - return false;
> > - } while ((dummy & 0xffffffff) == 0 && --loop);
> > + return 0;
> > +
> > + upper += (dummy >> 32) != (last >> 32);
> > + if (upper > 1) /* beware 32bit counter overflow */
> > + return 2; /* upper dword holds the low 32bits of the timestamp */
> > +
> > + lower += (dummy & 0xffffffff) != (last & 0xffffffff);
> > + if (lower > 1)
> > + return 1; /* timestamp is unshifted */
> > +
> > + last = dummy;
> > + }
> >
> > - return loop > 0;
> > + /* No advancement? No timestamp! */
> > + return 0;
> > }
> >
> > /**
> > diff --git a/src/mesa/drivers/dri/i965/intel_screen.h
> > b/src/mesa/drivers/dri/i965/intel_screen.h
> > index a741c94..fd5143e 100644
> > --- a/src/mesa/drivers/dri/i965/intel_screen.h
> > +++ b/src/mesa/drivers/dri/i965/intel_screen.h
> > @@ -52,7 +52,7 @@ struct intel_screen
> >
> > bool hw_has_swizzling;
> >
> > - bool hw_has_timestamp;
> > + int hw_has_timestamp;
> >
> > /**
> > * Does the kernel support resource streamer?
> > --
> > 2.1.4
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
