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 <freedesktop@xxxxxxxxxxxxxx> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91317 > Testcase: igt/gem_reg_read Tested-by: Michał Winiarski <michal.winiarski@xxxxxxxxx> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Cc: Michał Winiarski <michal.winiarski@xxxxxxxxx> > Cc: stable@xxxxxxxxxxxxxxx > --- > 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@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
