On Tue, Mar 19, 2013 at 09:56:57AM +0100, Jiri Kosina wrote:
> On Mon, 18 Mar 2013, Chris Wilson wrote:
>
> > > +#define HAS_GMBUS_IRQ(dev) (INTEL_INFO(dev)->gen >= 5)
> > > void
> > > intel_i2c_reset(struct drm_device *dev)
> > > {
> > > struct drm_i915_private *dev_priv = dev->dev_private;
> > > I915_WRITE(dev_priv->gpio_mmio_base + GMBUS0, 0);
> > > - I915_WRITE(dev_priv->gpio_mmio_base + GMBUS4, 0);
> > > + if (HAS_GMBUS_IRQ(dev))
> > > + I915_WRITE(dev_priv->gpio_mmio_base + GMBUS4, 0);
> >
> > There should not be any harm in always clearing GMBUS4, it exists on all
> > platforms.
> >
> > > }
> > >
> > > static void intel_i2c_quirk_set(struct drm_i915_private *dev_priv, bool enable)
> > > @@ -203,7 +205,6 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin)
> > > algo->data = bus;
> > > }
> > >
> > > -#define HAS_GMBUS_IRQ(dev) (INTEL_INFO(dev)->gen >= 4)
> > > static int
> > > gmbus_wait_hw_status(struct drm_i915_private *dev_priv,
> > > u32 gmbus2_status,
> > > @@ -214,6 +215,13 @@ gmbus_wait_hw_status(struct drm_i915_private *dev_priv,
> > > u32 gmbus2 = 0;
> > > DEFINE_WAIT(wait);
> > >
> > > + if (!HAS_GMBUS_IRQ(dev_priv->dev)) {
> > > + int ret;
> > > + ret = wait_for((gmbus2 = I915_READ(GMBUS2 + reg_offset)) &
> > > + (GMBUS_SATOER | gmbus2_status),
> > > + 50);
> >
> > This should couple up to the normal return code that chooses the
> > appropriate return value based on gmbus2.
> >
> > How about just using:
> > if (!HAS_GMBUS_IRQ(dev_priv->dev)) gmbus4_irq_en = 0;
> > and the existing wait loop?
>
> I explicitly wanted to avoid touching GMBUS4 register, as the real cause
> of the failure is not clear.
>
> But, as Yinghai Lu points out, the problem is most likely caused by
> interrupt disabling not working properly (see his very good point
> regarding DisINTx+ and INTx+ discrepancy), so zeroing the register out
> should work .... and it indeed does in my case, hence the (tested) patch
> below.
>
> I think it's a 3.9-rc material, and I am all open to debug this further
> for 3.10 so that the race is closed and gmbus irqs can be used on Gen4
> platform properly.
Agreed. Using the IRQ for GMBUS is just a performance feature that can
be deferred until after we determine the root cause - and hope that the
failure is somehow peculiar to GMBUS.
Acked-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
