[4/4] drm/i915: use gmbus irq to wait for gmbus idle

Commit Message

Daniel Vetter Sept. 6, 2012, 7:10 a.m. UTC

GMBUS_ACTIVE has inverted sense and so doesn't fit into the
wait_hw_status helper, hence create a new gmbus_wait_idle functions.
Also, we only care about the idle irq event and nothing else, which
allows us to use the wait_event_timeout helper directly without
jumping through hoops to catch NAKs.

Since gen2/3 don't have gmbus interrupts, handle them separately with
the old wait_for macro.

This shaves another few ms off reading EDID from a hdmi screen on my
testbox here. EDID reading with interrupt driven gmbus is now as fast
as with busy-looping gmbus at 28 ms here (with negligible cpu
overhead).

Signed-off-by: Daniel Vetter <[e-mail]>
---
 drivers/gpu/drm/i915/intel_i2c.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

Comments

Chris Wilson Sept. 6, 2012, 1:06 p.m. UTC | #1

On Thu,  6 Sep 2012 09:10:02 +0200, Daniel Vetter <[e-mail]> wrote:
> GMBUS_ACTIVE has inverted sense and so doesn't fit into the
> wait_hw_status helper, hence create a new gmbus_wait_idle functions.
> Also, we only care about the idle irq event and nothing else, which
> allows us to use the wait_event_timeout helper directly without
> jumping through hoops to catch NAKs.
> 
> Since gen2/3 don't have gmbus interrupts, handle them separately with
> the old wait_for macro.
> 
> This shaves another few ms off reading EDID from a hdmi screen on my
> testbox here. EDID reading with interrupt driven gmbus is now as fast
> as with busy-looping gmbus at 28 ms here (with negligible cpu
> overhead).

I'll put my neck on the line and say I can't spot any other mistakes:
Reviewed-by: Chris Wilson <[e-mail]>

A couple of triffling things, you could use whitespace more uniformly
and the comment for only enabling one interrupt source could do with
the explanation that then means we also have to poll for NAK.
-Chris

Patch

diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index 86f2b8c..faf85b3 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -204,6 +204,7 @@  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,
@@ -239,6 +240,31 @@  gmbus_wait_hw_status(struct drm_i915_private *dev_priv,
 }
 
 static int
+gmbus_wait_idle(struct drm_i915_private *dev_priv)
+{
+	int ret;
+	int reg_offset = dev_priv->gpio_mmio_base;
+
+#define C ((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0)
+
+	if (!HAS_GMBUS_IRQ(dev_priv->dev))
+		return wait_for(C, 10);
+
+	/* Important: The hw handles only the first bit, so set only one! */
+	I915_WRITE(GMBUS4 + reg_offset, GMBUS_IDLE_EN);
+
+	ret = wait_event_timeout(dev_priv->gmbus_wait_queue, C, 10);
+
+	I915_WRITE(GMBUS4 + reg_offset, 0);
+
+	if (ret)
+		return 0;
+	else
+		return -ETIMEDOUT;
+#undef C
+}
+
+static int
 gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
 		u32 gmbus1_index)
 {
@@ -405,8 +431,7 @@  gmbus_xfer(struct i2c_adapter *adapter,
 	 * We will re-enable it at the start of the next xfer,
 	 * till then let it sleep.
 	 */
-	if (wait_for((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0,
-		     10)) {
+	if (gmbus_wait_idle(dev_priv)) {
 		DRM_DEBUG_KMS("GMBUS [%s] timed out waiting for idle\n",
 			 adapter->name);
 		ret = -ETIMEDOUT;
@@ -430,8 +455,7 @@  clear_err:
 	 * it's slow responding and only answers on the 2nd retry.
 	 */
 	ret = -ENXIO;
-	if (wait_for((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0,
-		     10)) {
+	if (gmbus_wait_idle(dev_priv)) {
 		DRM_DEBUG_KMS("GMBUS [%s] timed out after NAK\n",
 			      adapter->name);
 		ret = -ETIMEDOUT;