From patchwork Thu Sep 6 07:10:01 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 1412491 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork1.kernel.org (Postfix) with ESMTP id 9908E3FC85 for ; Thu, 6 Sep 2012 08:19:12 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 7C9B19EDD9 for ; Thu, 6 Sep 2012 01:19:12 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-bk0-f49.google.com (mail-bk0-f49.google.com [209.85.214.49]) by gabe.freedesktop.org (Postfix) with ESMTP id 5E44D9ECDA for ; Thu, 6 Sep 2012 01:17:27 -0700 (PDT) Received: by mail-bk0-f49.google.com with SMTP id ji2so643778bkc.36 for ; Thu, 06 Sep 2012 01:17:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references; bh=BBquYFUsJxL9LNojuHc0p3nmoEkD+RXXR+DOeEncoRM=; b=FyLpCFq16pP9vyrRxPmqZIlTqmsAfe/PvcvYN4S0x0BeLtucj2rEdSLqU2lszWN54q mnbvooMuI1KdSbqzjAstUeT1TrSKtwkIvwTjhHGRBSz9N9PUBSql0dOCWWvmeTpAS5Tb 9+Q3A6RnLAy6MKOMTBL3QMEeZDp7Gyc1K/WMo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references :x-gm-message-state; bh=BBquYFUsJxL9LNojuHc0p3nmoEkD+RXXR+DOeEncoRM=; b=fPrgZgdkyo9X9kbQI6lb/AwB4UhpVJntExKBL+2hHwBGOS24p9knzSDnCdyOyG2/2P NwvkV4eb/m5LvWIkAZn/3VgeJHKZCtgCLy+XsIvRwcCw44tfH3CV09ricvJvL9SGzZoB hbtHhOG1qjbkX77CLzrgLv3TPwWq2H45Adtw8RzYNArFROZ0myQMWyxwwCTwgFdu6TFF x4sSN3Gd1ZpOs59PVXOjxqvl4FxmfQ8Vjo/GWjnC5cVwV2aGHdpitmsN1ra2YLpLcnLD TPF5g2eN5EfdhZfjhCs5IFhjCbusQ+s2NkglTbw44Pmt+15IDc1Om0KE9vgq5Ojq766V LtDw== Received: by 10.205.135.146 with SMTP id ig18mr334777bkc.80.1346919446880; Thu, 06 Sep 2012 01:17:26 -0700 (PDT) Received: from wespe.ffwll.local (178-83-130-250.dynamic.hispeed.ch. [178.83.130.250]) by mx.google.com with ESMTPS id hg13sm485182bkc.7.2012.09.06.01.17.25 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 06 Sep 2012 01:17:26 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Thu, 6 Sep 2012 09:10:01 +0200 Message-Id: <1346915402-9399-3-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 1.7.11.2 In-Reply-To: <1346915402-9399-1-git-send-email-daniel.vetter@ffwll.ch> References: <1346915402-9399-1-git-send-email-daniel.vetter@ffwll.ch> X-Gm-Message-State: ALoCoQm40Cuev5so2HEmdIv5Kh1ho00jJPoGhiCJg7GfrRRaMBSKChLrYhUf5725QjMNSFpEJjPs Cc: Daniel Vetter Subject: [Intel-gfx] [PATCH 3/4] drm/i915: use the gmbus irq for waits X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org We need two special things to properly wire this up: - Add another argument to gmbus_wait_hw_status to pass in the correct interrupt bit in gmbus4. - Since we can only get an irq for one of the two events we want, hand-roll the wait_event_timeout code so that we wake up every jiffie and can check for NAKs. This way we also subsume gmbus support for platforms without interrupts (or where those are not yet enabled). The important bit really is to only enable one gmbus interrupt source at the same time - with that piece of lore figured out, this seems to work flawlessly. Ben Widawsky rightfully complained the lack of measurements for the claimed benefits (especially since the first version was actually broken and fell back to bit-banging). Previously reading the 256 byte hdmi EDID takes about 72 ms here. With this patch it's down to 33 ms. Given that transfering the 256 bytes over i2c at wire speed takes 20.5ms alone, the reduction in additional overhead is rather nice. v2: Chris Wilson wondered whether GMBUS4 might contain some set bits when booting up an hence result in some spurious interrupts. Since we clear GMBUS4 after every wait and we do gmbus transfer really early in the setup sequence to detect displays the window is small, but still be paranoid and clear it properly. Cc: Daniel Kurtz Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_irq.c | 4 ++++ drivers/gpu/drm/i915/intel_i2c.c | 43 ++++++++++++++++++++++++++++++---------- 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 9fce782..d93ce14 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -418,6 +418,8 @@ typedef struct drm_i915_private { */ uint32_t gpio_mmio_base; + wait_queue_head_t gmbus_wait_queue; + struct pci_dev *bridge_dev; struct intel_ring_buffer ring[I915_NUM_RINGS]; uint32_t next_seqno; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 8415fa6..dadc86b 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -598,7 +598,11 @@ out: static void gmbus_irq_handler(struct drm_device *dev) { + struct drm_i915_private *dev_priv = (drm_i915_private_t *) dev->dev_private; + DRM_DEBUG_DRIVER("GMBUS interrupt\n"); + + wake_up_all(&dev_priv->gmbus_wait_queue); } static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir) diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index 57decac..86f2b8c 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -64,6 +64,7 @@ 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); } static void intel_i2c_quirk_set(struct drm_i915_private *dev_priv, bool enable) @@ -205,20 +206,36 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin) static int gmbus_wait_hw_status(struct drm_i915_private *dev_priv, - u32 gmbus2_status) + u32 gmbus2_status, + u32 gmbus4_irq_en) { - int ret; + int i; int reg_offset = dev_priv->gpio_mmio_base; - u32 gmbus2; + u32 gmbus2 = 0; + DEFINE_WAIT(wait); + + /* Important: The hw handles only the first bit, so set only one! */ + I915_WRITE(GMBUS4 + reg_offset, gmbus4_irq_en); - ret = wait_for((gmbus2 = I915_READ(GMBUS2 + reg_offset)) & - (GMBUS_SATOER | gmbus2_status), - 50); + for (i = 0; i < msecs_to_jiffies(50) + 1; i++) { + prepare_to_wait(&dev_priv->gmbus_wait_queue, &wait, + TASK_UNINTERRUPTIBLE); + + gmbus2 = I915_READ(GMBUS2 + reg_offset); + if (gmbus2 & (GMBUS_SATOER | gmbus2_status)) + break; + + schedule_timeout(1); + } + finish_wait(&dev_priv->gmbus_wait_queue, &wait); + + I915_WRITE(GMBUS4 + reg_offset, 0); if (gmbus2 & GMBUS_SATOER) return -ENXIO; - - return ret; + if (gmbus2 & gmbus2_status) + return 0; + return -ETIMEDOUT; } static int @@ -239,7 +256,8 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg, int ret; u32 val, loop = 0; - ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_RDY); + ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_RDY, + GMBUS_HW_RDY_EN); if (ret) return ret; @@ -283,7 +301,8 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg) I915_WRITE(GMBUS3 + reg_offset, val); - ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_RDY); + ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_RDY, + GMBUS_HW_RDY_EN); if (ret) return ret; } @@ -368,7 +387,8 @@ gmbus_xfer(struct i2c_adapter *adapter, if (ret == -ENXIO) goto clear_err; - ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_WAIT_PHASE); + ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_WAIT_PHASE, + GMBUS_HW_WAIT_EN); if (ret == -ENXIO) goto clear_err; if (ret) @@ -474,6 +494,7 @@ int intel_setup_gmbus(struct drm_device *dev) dev_priv->gpio_mmio_base = 0; mutex_init(&dev_priv->gmbus_mutex); + init_waitqueue_head(&dev_priv->gmbus_wait_queue); for (i = 0; i < GMBUS_NUM_PORTS; i++) { struct intel_gmbus *bus = &dev_priv->gmbus[i];