From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [Intel-gfx] [PATCH v3] drm/i915: Use two 32bit reads for select 64bit REG_READ ioctls Date: Tue, 21 Jul 2015 08:49:31 +0200 Message-ID: <20150721064931.GM16722@phenom.ffwll.local> References: <1437045549-15455-1-git-send-email-michal.winiarski@intel.com> <1437046676-31811-1-git-send-email-chris@chris-wilson.co.uk> <20150717151025.GA26539@mwiniars-desk1.igk.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Content-Disposition: inline In-Reply-To: <20150717151025.GA26539@mwiniars-desk1.igk.intel.com> Sender: stable-owner@vger.kernel.org To: =?utf-8?Q?Micha=C5=82?= Winiarski Cc: Chris Wilson , intel-gfx@lists.freedesktop.org, stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org On Fri, Jul 17, 2015 at 05:10:25PM +0200, Micha=C5=82 Winiarski wrote: > On Thu, Jul 16, 2015 at 12:37:56PM +0100, Chris Wilson wrote: > > Since the hardware sometimes mysteriously totally flummoxes the 64b= it > > read of a 64bit register when read using a single instruction, spli= t 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. > >=20 > > However, since userspace tried to workaround this issue and so ensh= rined > > this ABI for a broken hardware read and in the process neglected th= at > > the read only fails in some environments, we have to introduce a ne= w > > uABI flag for userspace to request the 2x32 bit accurate read of th= e > > timestamp. > >=20 > > v2: Fix alignment check and include details of the workaround for > > userspace. > >=20 > > Reported-by: Karol Herbst > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=3D91317 > > Testcase: igt/gem_reg_read > Tested-by: Micha=C5=82 Winiarski Where are the mesa/beignet/libva patches for this? -Daniel > > Signed-off-by: Chris Wilson > > Cc: Micha=C5=82 Winiarski > > Cc: stable@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(-) > >=20 > > 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 =3D dev->dev_private; > > struct drm_i915_reg_read *reg =3D data; > > struct register_whitelist const *entry =3D whitelist; > > + unsigned size; > > + u64 offset; > > int i, ret =3D 0; > > =20 > > for (i =3D 0; i < ARRAY_SIZE(whitelist); i++, entry++) { > > - if (entry->offset =3D=3D reg->offset && > > + if (entry->offset =3D=3D (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 =3D=3D ARRAY_SIZE(whitelist)) > > return -EINVAL; > > =20 > > + /* We use the low bits to encode extra flags as the register shou= ld > > + * be naturally aligned (and those that are not so aligned merely > > + * limit the available flags for that register). > > + */ > > + offset =3D entry->offset; > > + size =3D entry->size; > > + size |=3D reg->offset ^ offset; > > + > > intel_runtime_pm_get(dev_priv); > > =20 > > - switch (entry->size) { > > + switch (size) { > > + case 8 | 1: > > + reg->val =3D I915_READ64_2x32(offset, offset+4); > > + break; > > case 8: > > - reg->val =3D I915_READ64(reg->offset); > > + reg->val =3D I915_READ64(offset); > > break; > > case 4: > > - reg->val =3D I915_READ(reg->offset); > > + reg->val =3D I915_READ(offset); > > break; > > case 2: > > - reg->val =3D I915_READ16(reg->offset); > > + reg->val =3D I915_READ16(offset); > > break; > > case 1: > > - reg->val =3D I915_READ8(reg->offset); > > + reg->val =3D I915_READ8(offset); > > break; > > default: > > - MISSING_CASE(entry->size); > > ret =3D -EINVAL; > > goto out; > > } > > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_dr= m.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 defa= ult > > + * single instruction 8byte read, in order to workaround that us= e > > + * offset (0x2538 | 1) instead. > > + * > > + */ > > =20 > > struct drm_i915_reset_stats { > > __u32 ctx_id; > > --=20 > > 2.1.4 > >=20 > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx --=20 Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch