i965: Use updated kernel interface for accurate TIMESTAMP reads

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 <chris@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 hardware
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 <chris@chris-wilson.co.uk>
Cc: Martin Peres <martin.peres@linux.intel.com>
Cc: Kenneth Graunke <kenneth@whitecape.org>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Tested-and-acked-by: Chris Forbes <chrisf@ijw.co.nz>
Reviewed-by: Daniel Vetter <daniel@ffwll.ch>

diff --git a/src/mesa/drivers/dri/i965/brw_queryobj.c b/src/mesa/drivers/dri/i965/brw_queryobj.c
index aea4d9b77d340d894768bef6fca5f447a0bc72dd..d6b012c392e2811ae347aafaa76094b5e5113162 100644 (file)
--- 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 left-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 1470b059d9b314b73fb5fe517210942dc8dcba38..65a17664188fc9bf118e640ce699a8a525aaeed8 100644 (file)
--- 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 a741c94e0ff5721bcb0537138ea87846cd3c4f2e..fd5143eecba3b0f4045ce10883547e01b376f831 100644 (file)
--- 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?