Message ID | 1359147462-3902-2-git-send-email-przanoni@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Fri, 25 Jan 2013 18:57:36 -0200 Paulo Zanoni <przanoni@gmail.com> wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > This avoids polluting i915_write##x and also allows us to reuse code > on i915_read##x. > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 24 ++++++++++++++++-------- > 1 file changed, 16 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 9cc8f87..ae0a55a 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1224,6 +1224,20 @@ ilk_dummy_write(struct drm_i915_private *dev_priv) > I915_WRITE_NOTRACE(MI_MODE, 0); > } > > +#define UNCLAIMED_REG_CLEAR(dev_priv, reg) \ > + if (IS_HASWELL(dev_priv->dev) && \ > + (I915_READ_NOTRACE(GEN7_ERR_INT) & ERR_INT_MMIO_UNCLAIMED)) { \ > + DRM_ERROR("Unknown unclaimed register before writing to %x\n", reg); \ > + I915_WRITE_NOTRACE(GEN7_ERR_INT, ERR_INT_MMIO_UNCLAIMED); \ > + } > + You don't really need the if here, do you? I think the way it worked originally was fine. > +#define UNCLAIMED_REG_CHECK(dev_priv, reg) \ > + if (IS_HASWELL(dev_priv->dev) && \ > + (I915_READ_NOTRACE(GEN7_ERR_INT) & ERR_INT_MMIO_UNCLAIMED)) { \ > + DRM_ERROR("Unclaimed write to %x\n", reg); \ > + writel(ERR_INT_MMIO_UNCLAIMED, dev_priv->regs + GEN7_ERR_INT); \ > + } > + > #define __i915_read(x, y) \ > u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \ > u##x val = 0; \ I suppose you could add some 'unlikely' predictions in another patch if you wanted. I really don't know what the consensus is on using those these day. > @@ -1262,10 +1276,7 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \ > } \ > if (IS_GEN5(dev_priv->dev)) \ > ilk_dummy_write(dev_priv); \ > - if (IS_HASWELL(dev_priv->dev) && (I915_READ_NOTRACE(GEN7_ERR_INT) & ERR_INT_MMIO_UNCLAIMED)) { \ > - DRM_ERROR("Unknown unclaimed register before writing to %x\n", reg); \ > - I915_WRITE_NOTRACE(GEN7_ERR_INT, ERR_INT_MMIO_UNCLAIMED); \ > - } \ > + UNCLAIMED_REG_CLEAR(dev_priv, reg); \ > if (IS_VALLEYVIEW(dev_priv->dev) && IS_DISPLAYREG(reg)) { \ > write##y(val, dev_priv->regs + reg + 0x180000); \ > } else { \ Here's what I meant above, you replace 2 lines of code with 4 from the macro. (Including an extra MMIO read). > @@ -1274,10 +1285,7 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \ > if (unlikely(__fifo_ret)) { \ > gen6_gt_check_fifodbg(dev_priv); \ > } \ > - if (IS_HASWELL(dev_priv->dev) && (I915_READ_NOTRACE(GEN7_ERR_INT) & ERR_INT_MMIO_UNCLAIMED)) { \ > - DRM_ERROR("Unclaimed write to %x\n", reg); \ > - writel(ERR_INT_MMIO_UNCLAIMED, dev_priv->regs + GEN7_ERR_INT); \ > - } \ > + UNCLAIMED_REG_CHECK(dev_priv, reg); \ > } > __i915_write(8, b) > __i915_write(16, w)
On Fri, Jan 25, 2013 at 05:02:01PM -0800, Ben Widawsky wrote: > On Fri, 25 Jan 2013 18:57:36 -0200 > Paulo Zanoni <przanoni@gmail.com> wrote: > > > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > This avoids polluting i915_write##x and also allows us to reuse code > > on i915_read##x. > > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: Ben Widawsky <ben@bwidawsk.net> > > --- > > drivers/gpu/drm/i915/i915_drv.c | 24 ++++++++++++++++-------- > > 1 file changed, 16 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > index 9cc8f87..ae0a55a 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -1224,6 +1224,20 @@ ilk_dummy_write(struct drm_i915_private *dev_priv) > > I915_WRITE_NOTRACE(MI_MODE, 0); > > } > > > > +#define UNCLAIMED_REG_CLEAR(dev_priv, reg) \ > > + if (IS_HASWELL(dev_priv->dev) && \ > > + (I915_READ_NOTRACE(GEN7_ERR_INT) & ERR_INT_MMIO_UNCLAIMED)) { \ > > + DRM_ERROR("Unknown unclaimed register before writing to %x\n", reg); \ > > + I915_WRITE_NOTRACE(GEN7_ERR_INT, ERR_INT_MMIO_UNCLAIMED); \ > > + } > > + > > You don't really need the if here, do you? I think the way it worked > originally was fine. I misread this the first time around. Please ignore this comment. Apologies. > > > +#define UNCLAIMED_REG_CHECK(dev_priv, reg) \ > > + if (IS_HASWELL(dev_priv->dev) && \ > > + (I915_READ_NOTRACE(GEN7_ERR_INT) & ERR_INT_MMIO_UNCLAIMED)) { \ > > + DRM_ERROR("Unclaimed write to %x\n", reg); \ > > + writel(ERR_INT_MMIO_UNCLAIMED, dev_priv->regs + GEN7_ERR_INT); \ > > + } > > + > > #define __i915_read(x, y) \ > > u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \ > > u##x val = 0; \ > > I suppose you could add some 'unlikely' predictions in another patch if > you wanted. I really don't know what the consensus is on using those > these day. > > > @@ -1262,10 +1276,7 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \ > > } \ > > if (IS_GEN5(dev_priv->dev)) \ > > ilk_dummy_write(dev_priv); \ > > - if (IS_HASWELL(dev_priv->dev) && (I915_READ_NOTRACE(GEN7_ERR_INT) & ERR_INT_MMIO_UNCLAIMED)) { \ > > - DRM_ERROR("Unknown unclaimed register before writing to %x\n", reg); \ > > - I915_WRITE_NOTRACE(GEN7_ERR_INT, ERR_INT_MMIO_UNCLAIMED); \ > > - } \ > > + UNCLAIMED_REG_CLEAR(dev_priv, reg); \ > > if (IS_VALLEYVIEW(dev_priv->dev) && IS_DISPLAYREG(reg)) { \ > > write##y(val, dev_priv->regs + reg + 0x180000); \ > > } else { \ > > Here's what I meant above, you replace 2 lines of code with 4 from the > macro. (Including an extra MMIO read). > I misread this the first time around. Please ignore this comment. Apologies. > > @@ -1274,10 +1285,7 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \ > > if (unlikely(__fifo_ret)) { \ > > gen6_gt_check_fifodbg(dev_priv); \ > > } \ > > - if (IS_HASWELL(dev_priv->dev) && (I915_READ_NOTRACE(GEN7_ERR_INT) & ERR_INT_MMIO_UNCLAIMED)) { \ > > - DRM_ERROR("Unclaimed write to %x\n", reg); \ > > - writel(ERR_INT_MMIO_UNCLAIMED, dev_priv->regs + GEN7_ERR_INT); \ > > - } \ > > + UNCLAIMED_REG_CHECK(dev_priv, reg); \ > > } > > __i915_write(8, b) > > __i915_write(16, w) > > > > -- > Ben Widawsky, Intel Open Source Technology Center > _______________________________________________ > 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.c b/drivers/gpu/drm/i915/i915_drv.c index 9cc8f87..ae0a55a 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1224,6 +1224,20 @@ ilk_dummy_write(struct drm_i915_private *dev_priv) I915_WRITE_NOTRACE(MI_MODE, 0); } +#define UNCLAIMED_REG_CLEAR(dev_priv, reg) \ + if (IS_HASWELL(dev_priv->dev) && \ + (I915_READ_NOTRACE(GEN7_ERR_INT) & ERR_INT_MMIO_UNCLAIMED)) { \ + DRM_ERROR("Unknown unclaimed register before writing to %x\n", reg); \ + I915_WRITE_NOTRACE(GEN7_ERR_INT, ERR_INT_MMIO_UNCLAIMED); \ + } + +#define UNCLAIMED_REG_CHECK(dev_priv, reg) \ + if (IS_HASWELL(dev_priv->dev) && \ + (I915_READ_NOTRACE(GEN7_ERR_INT) & ERR_INT_MMIO_UNCLAIMED)) { \ + DRM_ERROR("Unclaimed write to %x\n", reg); \ + writel(ERR_INT_MMIO_UNCLAIMED, dev_priv->regs + GEN7_ERR_INT); \ + } + #define __i915_read(x, y) \ u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \ u##x val = 0; \ @@ -1262,10 +1276,7 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \ } \ if (IS_GEN5(dev_priv->dev)) \ ilk_dummy_write(dev_priv); \ - if (IS_HASWELL(dev_priv->dev) && (I915_READ_NOTRACE(GEN7_ERR_INT) & ERR_INT_MMIO_UNCLAIMED)) { \ - DRM_ERROR("Unknown unclaimed register before writing to %x\n", reg); \ - I915_WRITE_NOTRACE(GEN7_ERR_INT, ERR_INT_MMIO_UNCLAIMED); \ - } \ + UNCLAIMED_REG_CLEAR(dev_priv, reg); \ if (IS_VALLEYVIEW(dev_priv->dev) && IS_DISPLAYREG(reg)) { \ write##y(val, dev_priv->regs + reg + 0x180000); \ } else { \ @@ -1274,10 +1285,7 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \ if (unlikely(__fifo_ret)) { \ gen6_gt_check_fifodbg(dev_priv); \ } \ - if (IS_HASWELL(dev_priv->dev) && (I915_READ_NOTRACE(GEN7_ERR_INT) & ERR_INT_MMIO_UNCLAIMED)) { \ - DRM_ERROR("Unclaimed write to %x\n", reg); \ - writel(ERR_INT_MMIO_UNCLAIMED, dev_priv->regs + GEN7_ERR_INT); \ - } \ + UNCLAIMED_REG_CHECK(dev_priv, reg); \ } __i915_write(8, b) __i915_write(16, w)