| Message ID | 1436950242-4738-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 15, 2015 at 09:50:42AM +0100, Chris Wilson wrote: > Since we may conceivably encounter situations where the upper part of the > 64bit register changes between reads, for example when a timestamp > counter overflows, change the WARN into a retry loop. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Micha? Winiarski <michal.winiarski@intel.com> Micha?, as you correctly predicted this WARN will be hit in the wild after adjusting reg_read_ioctl(TIMESTAMP). Anyone care to review this patch? > --- > drivers/gpu/drm/i915/i915_drv.h | 17 ++++++++--------- > 1 file changed, 8 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index ee53873ab252..cfddd69bd06c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3484,15 +3484,14 @@ int intel_freq_opcode(struct drm_i915_private *dev_priv, int val); > #define I915_READ64(reg) dev_priv->uncore.funcs.mmio_readq(dev_priv, (reg), true) > > #define I915_READ64_2x32(lower_reg, upper_reg) ({ \ > - u32 upper = I915_READ(upper_reg); \ > - u32 lower = I915_READ(lower_reg); \ > - u32 tmp = I915_READ(upper_reg); \ > - if (upper != tmp) { \ > - upper = tmp; \ > - lower = I915_READ(lower_reg); \ > - WARN_ON(I915_READ(upper_reg) != upper); \ > - } \ > - (u64)upper << 32 | lower; }) > + u32 upper, lower, tmp; \ > + tmp = I915_READ(upper_reg); \ > + do { \ > + upper = tmp; \ > + lower = I915_READ(lower_reg); \ > + tmp = I915_READ(upper_reg); \ > + } while (upper != tmp); \ > + (u64)upper << 32 | lower; }) > > #define POSTING_READ(reg) (void)I915_READ_NOTRACE(reg) > #define POSTING_READ16(reg) (void)I915_READ16_NOTRACE(reg)
On Tue, Jul 28, 2015 at 02:53:16PM +0100, Chris Wilson wrote: > On Wed, Jul 15, 2015 at 09:50:42AM +0100, Chris Wilson wrote: > > Since we may conceivably encounter situations where the upper part of the > > 64bit register changes between reads, for example when a timestamp > > counter overflows, change the WARN into a retry loop. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Micha? Winiarski <michal.winiarski@intel.com> > > Micha?, as you correctly predicted this WARN will be hit in the wild > after adjusting reg_read_ioctl(TIMESTAMP). > > Anyone care to review this patch? Picked up for -fixes, thanks for the patch. -Daniel > > > --- > > drivers/gpu/drm/i915/i915_drv.h | 17 ++++++++--------- > > 1 file changed, 8 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index ee53873ab252..cfddd69bd06c 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -3484,15 +3484,14 @@ int intel_freq_opcode(struct drm_i915_private *dev_priv, int val); > > #define I915_READ64(reg) dev_priv->uncore.funcs.mmio_readq(dev_priv, (reg), true) > > > > #define I915_READ64_2x32(lower_reg, upper_reg) ({ \ > > - u32 upper = I915_READ(upper_reg); \ > > - u32 lower = I915_READ(lower_reg); \ > > - u32 tmp = I915_READ(upper_reg); \ > > - if (upper != tmp) { \ > > - upper = tmp; \ > > - lower = I915_READ(lower_reg); \ > > - WARN_ON(I915_READ(upper_reg) != upper); \ > > - } \ > > - (u64)upper << 32 | lower; }) > > + u32 upper, lower, tmp; \ > > + tmp = I915_READ(upper_reg); \ > > + do { \ > > + upper = tmp; \ > > + lower = I915_READ(lower_reg); \ > > + tmp = I915_READ(upper_reg); \ > > + } while (upper != tmp); \ > > + (u64)upper << 32 | lower; }) > > > > #define POSTING_READ(reg) (void)I915_READ_NOTRACE(reg) > > #define POSTING_READ16(reg) (void)I915_READ16_NOTRACE(reg) > > -- > Chris Wilson, Intel Open Source Technology Centre > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index ee53873ab252..cfddd69bd06c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3484,15 +3484,14 @@ int intel_freq_opcode(struct drm_i915_private *dev_priv, int val); #define I915_READ64(reg) dev_priv->uncore.funcs.mmio_readq(dev_priv, (reg), true) #define I915_READ64_2x32(lower_reg, upper_reg) ({ \ - u32 upper = I915_READ(upper_reg); \ - u32 lower = I915_READ(lower_reg); \ - u32 tmp = I915_READ(upper_reg); \ - if (upper != tmp) { \ - upper = tmp; \ - lower = I915_READ(lower_reg); \ - WARN_ON(I915_READ(upper_reg) != upper); \ - } \ - (u64)upper << 32 | lower; }) + u32 upper, lower, tmp; \ + tmp = I915_READ(upper_reg); \ + do { \ + upper = tmp; \ + lower = I915_READ(lower_reg); \ + tmp = I915_READ(upper_reg); \ + } while (upper != tmp); \ + (u64)upper << 32 | lower; }) #define POSTING_READ(reg) (void)I915_READ_NOTRACE(reg) #define POSTING_READ16(reg) (void)I915_READ16_NOTRACE(reg)
Since we may conceivably encounter situations where the upper part of the 64bit register changes between reads, for example when a timestamp counter overflows, change the WARN into a retry loop. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Micha? Winiarski <michal.winiarski@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)