Message ID | 1353425264-3728-5-git-send-email-przanoni@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
> + 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); \ > + } \ Do we really have to DRM_ERROR here? this bit being set seems to be beyond things we can fix (BIOS leaving that behind?)
Hi 2012/11/20 Damien Lespiau <damien.lespiau@intel.com>: >> + 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); \ >> + } \ > > Do we really have to DRM_ERROR here? this bit being set seems to be > beyond things we can fix (BIOS leaving that behind?) If we do I915_WRITE_NOTRACE or I915_READ_NOTRACE to a register that does not exist I believe the unclaimed bit will be set. > > -- > Damien
On Tue, Nov 20, 2012 at 7:10 PM, Paulo Zanoni <przanoni@gmail.com> wrote: > Hi > > 2012/11/20 Damien Lespiau <damien.lespiau@intel.com>: >>> + 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); \ >>> + } \ >> >> Do we really have to DRM_ERROR here? this bit being set seems to be >> beyond things we can fix (BIOS leaving that behind?) > > If we do I915_WRITE_NOTRACE or I915_READ_NOTRACE to a register that > does not exist I believe the unclaimed bit will be set. Yup, that looks true, I guess that if we're seeing that BIOSes leave that behind and generate a false positive we can write that bit when cleaning up after the BIOS anyway. Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
2012/11/20 Damien Lespiau <damien.lespiau@intel.com>: > On Tue, Nov 20, 2012 at 7:10 PM, Paulo Zanoni <przanoni@gmail.com> wrote: >> Hi >> >> 2012/11/20 Damien Lespiau <damien.lespiau@intel.com>: >>>> + 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); \ >>>> + } \ >>> >>> Do we really have to DRM_ERROR here? this bit being set seems to be >>> beyond things we can fix (BIOS leaving that behind?) >> >> If we do I915_WRITE_NOTRACE or I915_READ_NOTRACE to a register that >> does not exist I believe the unclaimed bit will be set. > > Yup, that looks true, I guess that if we're seeing that BIOSes leave > that behind and generate a false positive we can write that bit when > cleaning up after the BIOS anyway. As far as I have investigated, we are also seeing leftovers from the BIOS that you're talking about. We need to do an initial clear in this bit at the driver initialization time (irq initialization?). I forgot to say, but the error message introduced by this patch will also be print when we I915_READ a register that does not exist (which is the case when we hang the gpu, for example). > > Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> > > -- > Damien
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 418d17c..88c44ad 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1251,6 +1251,10 @@ 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); \ + } \ if (IS_VALLEYVIEW(dev_priv->dev) && IS_DISPLAYREG(reg)) { \ write##y(val, dev_priv->regs + reg + 0x180000); \ } else { \