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;
> + }
>
> /* 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
