On Thu, Jul 16, 2015 at 12:37:56PM +0100, Chris Wilson wrote:
> Since the hardware sometimes mysteriously totally flummoxes the 64bit
> read of a 64bit register when read using a single instruction, split the
> read into two instructions. Since the read here is of automatically
> incrementing timestamp counters, we also have to be very careful in
> order to make sure that it does not increment between the two
> instructions.
>
> However, since userspace tried to workaround this issue and so enshrined
> this ABI for a broken hardware read and in the process neglected that
> the read only fails in some environments, we have to introduce a new
> uABI flag for userspace to request the 2x32 bit accurate read of the
> timestamp.
>
> v2: Fix alignment check and include details of the workaround for
> userspace.
>
> Reported-by: Karol Herbst <freedesk...@karolherbst.de>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91317
> Testcase: igt/gem_reg_read
Tested-by: Michał Winiarski <michal.winiar...@intel.com>
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Michał Winiarski <michal.winiar...@intel.com>
> Cc: sta...@vger.kernel.org
> ---
> drivers/gpu/drm/i915/intel_uncore.c | 26 +++++++++++++++++++-------
> include/uapi/drm/i915_drm.h | 8 ++++++++
> 2 files changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> b/drivers/gpu/drm/i915/intel_uncore.c
> index 2c477663d378..eb244b57b3fd 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1310,10 +1310,12 @@ int i915_reg_read_ioctl(struct drm_device *dev,
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_i915_reg_read *reg = data;
> struct register_whitelist const *entry = whitelist;
> + unsigned size;
> + u64 offset;
> int i, ret = 0;
>
> for (i = 0; i < ARRAY_SIZE(whitelist); i++, entry++) {
> - if (entry->offset == reg->offset &&
> + if (entry->offset == (reg->offset & -entry->size) &&
> (1 << INTEL_INFO(dev)->gen & entry->gen_bitmask))
> break;
> }
> @@ -1321,23 +1323,33 @@ int i915_reg_read_ioctl(struct drm_device *dev,
> if (i == ARRAY_SIZE(whitelist))
> return -EINVAL;
>
> + /* We use the low bits to encode extra flags as the register should
> + * be naturally aligned (and those that are not so aligned merely
> + * limit the available flags for that register).
> + */
> + offset = entry->offset;
> + size = entry->size;
> + size |= reg->offset ^ offset;
> +
> intel_runtime_pm_get(dev_priv);
>
> - switch (entry->size) {
> + switch (size) {
> + case 8 | 1:
> + reg->val = I915_READ64_2x32(offset, offset+4);
> + break;
> case 8:
> - reg->val = I915_READ64(reg->offset);
> + reg->val = I915_READ64(offset);
> break;
> case 4:
> - reg->val = I915_READ(reg->offset);
> + reg->val = I915_READ(offset);
> break;
> case 2:
> - reg->val = I915_READ16(reg->offset);
> + reg->val = I915_READ16(offset);
> break;
> case 1:
> - reg->val = I915_READ8(reg->offset);
> + reg->val = I915_READ8(offset);
> break;
> default:
> - MISSING_CASE(entry->size);
> ret = -EINVAL;
> goto out;
> }
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index b0f82ddab987..83f60f01dca2 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -1087,6 +1087,14 @@ struct drm_i915_reg_read {
> __u64 offset;
> __u64 val; /* Return value */
> };
> +/* Known registers:
> + *
> + * Render engine timestamp - 0x2358 + 64bit - gen7+
> + * - Note this register returns an invalid value if using the default
> + * single instruction 8byte read, in order to workaround that use
> + * offset (0x2538 | 1) instead.
> + *
> + */
>
> struct drm_i915_reset_stats {
> __u32 ctx_id;
> --
> 2.1.4
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
