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@chris-wilson.co.uk -Chris