From 110447fc2f30c22dcaa1936828cb33cec5b72272 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Fri, 23 Mar 2012 23:43:36 +0100 Subject: drm/i915: add an explict mmio base for gpio/gmbus io Again, Valleyview modes these around, so make the mmio base more explicit to consolidate the base address computations to one HAS_PCH_SPLIT check. v2: Fix up the PCH_SPLIT braino ... it actually works that way round. Reviewed-by: Chris Wilson Signed-Off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_i2c.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index 601c86e664af..1486c76e51ac 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -49,10 +49,7 @@ void intel_i2c_reset(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - if (HAS_PCH_SPLIT(dev)) - I915_WRITE(PCH_GMBUS0, 0); - else - I915_WRITE(GMBUS0, 0); + I915_WRITE(dev_priv->gpio_mmio_base + GMBUS0, 0); } static void intel_i2c_quirk_set(struct drm_i915_private *dev_priv, bool enable) @@ -162,8 +159,7 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin) algo = &bus->bit_algo; bus->gpio_reg = map_pin_to_reg[pin]; - if (HAS_PCH_SPLIT(dev_priv->dev)) - bus->gpio_reg += PCH_GPIOA - GPIOA; + bus->gpio_reg += dev_priv->gpio_mmio_base; bus->adapter.algo_data = algo; algo->setsda = set_data; @@ -219,7 +215,7 @@ gmbus_xfer(struct i2c_adapter *adapter, goto out; } - reg_offset = HAS_PCH_SPLIT(dev_priv->dev) ? PCH_GMBUS0 - GMBUS0 : 0; + reg_offset = dev_priv->gpio_mmio_base; I915_WRITE(GMBUS0 + reg_offset, bus->reg0); @@ -359,6 +355,11 @@ int intel_setup_gmbus(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; int ret, i; + if (HAS_PCH_SPLIT(dev)) + dev_priv->gpio_mmio_base = PCH_GPIOA - GPIOA; + else + dev_priv->gpio_mmio_base = 0; + dev_priv->gmbus = kcalloc(GMBUS_NUM_PORTS, sizeof(struct intel_gmbus), GFP_KERNEL); if (dev_priv->gmbus == NULL) -- cgit v1.2.3 From 0fb3f969c8683505fb7323c06bf8a999a5a45a15 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Fri, 2 Mar 2012 19:38:30 +0100 Subject: drm/i915: enable gmbus on gen2 With the recent set of gmbus fixes, this seems to work on my i855gm. Acked-by: Chris Wilson Signed-Off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_i2c.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index 1486c76e51ac..0713cc289119 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -389,10 +389,6 @@ int intel_setup_gmbus(struct drm_device *dev) bus->reg0 = i | GMBUS_RATE_100KHZ; bus->has_gpio = intel_gpio_setup(bus, i); - - /* XXX force bit banging until GMBUS is fully debugged */ - if (bus->has_gpio && IS_GEN2(dev)) - bus->force_bit = true; } intel_i2c_reset(dev_priv->dev); -- cgit v1.2.3 From 924a93edc96b7f7e08a0af7a2f9afe4827bab103 Mon Sep 17 00:00:00 2001 From: Daniel Kurtz Date: Wed, 28 Mar 2012 02:36:10 +0800 Subject: drm/i915/intel_i2c: refactor gmbus_xfer Split out gmbus_xfer_read/write() helper functions. Signed-off-by: Daniel Kurtz Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_i2c.c | 151 ++++++++++++++++++++++++--------------- 1 file changed, 92 insertions(+), 59 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index 0713cc289119..30675ce18056 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -197,6 +197,82 @@ intel_i2c_quirk_xfer(struct intel_gmbus *bus, return ret; } +static int +gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg, + bool last) +{ + int reg_offset = dev_priv->gpio_mmio_base; + u16 len = msg->len; + u8 *buf = msg->buf; + + I915_WRITE(GMBUS1 + reg_offset, + GMBUS_CYCLE_WAIT | + (last ? GMBUS_CYCLE_STOP : 0) | + (len << GMBUS_BYTE_COUNT_SHIFT) | + (msg->addr << GMBUS_SLAVE_ADDR_SHIFT) | + GMBUS_SLAVE_READ | GMBUS_SW_RDY); + POSTING_READ(GMBUS2 + reg_offset); + do { + u32 val, loop = 0; + + if (wait_for(I915_READ(GMBUS2 + reg_offset) & + (GMBUS_SATOER | GMBUS_HW_RDY), + 50)) + return -ETIMEDOUT; + if (I915_READ(GMBUS2 + reg_offset) & GMBUS_SATOER) + return -ENXIO; + + val = I915_READ(GMBUS3 + reg_offset); + do { + *buf++ = val & 0xff; + val >>= 8; + } while (--len && ++loop < 4); + } while (len); + + return 0; +} + +static int +gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg, + bool last) +{ + int reg_offset = dev_priv->gpio_mmio_base; + u16 len = msg->len; + u8 *buf = msg->buf; + u32 val, loop; + + val = loop = 0; + do { + val |= *buf++ << (8 * loop); + } while (--len && ++loop < 4); + + I915_WRITE(GMBUS3 + reg_offset, val); + I915_WRITE(GMBUS1 + reg_offset, + GMBUS_CYCLE_WAIT | + (last ? GMBUS_CYCLE_STOP : 0) | + (msg->len << GMBUS_BYTE_COUNT_SHIFT) | + (msg->addr << GMBUS_SLAVE_ADDR_SHIFT) | + GMBUS_SLAVE_WRITE | GMBUS_SW_RDY); + POSTING_READ(GMBUS2 + reg_offset); + while (len) { + if (wait_for(I915_READ(GMBUS2 + reg_offset) & + (GMBUS_SATOER | GMBUS_HW_RDY), + 50)) + return -ETIMEDOUT; + if (I915_READ(GMBUS2 + reg_offset) & GMBUS_SATOER) + return -ENXIO; + + val = loop = 0; + do { + val |= *buf++ << (8 * loop); + } while (--len && ++loop < 4); + + I915_WRITE(GMBUS3 + reg_offset, val); + POSTING_READ(GMBUS2 + reg_offset); + } + return 0; +} + static int gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, @@ -220,65 +296,22 @@ gmbus_xfer(struct i2c_adapter *adapter, I915_WRITE(GMBUS0 + reg_offset, bus->reg0); for (i = 0; i < num; i++) { - u16 len = msgs[i].len; - u8 *buf = msgs[i].buf; - - if (msgs[i].flags & I2C_M_RD) { - I915_WRITE(GMBUS1 + reg_offset, - GMBUS_CYCLE_WAIT | - (i + 1 == num ? GMBUS_CYCLE_STOP : 0) | - (len << GMBUS_BYTE_COUNT_SHIFT) | - (msgs[i].addr << GMBUS_SLAVE_ADDR_SHIFT) | - GMBUS_SLAVE_READ | GMBUS_SW_RDY); - POSTING_READ(GMBUS2+reg_offset); - do { - u32 val, loop = 0; - - if (wait_for(I915_READ(GMBUS2 + reg_offset) & (GMBUS_SATOER | GMBUS_HW_RDY), 50)) - goto timeout; - if (I915_READ(GMBUS2 + reg_offset) & GMBUS_SATOER) - goto clear_err; - - val = I915_READ(GMBUS3 + reg_offset); - do { - *buf++ = val & 0xff; - val >>= 8; - } while (--len && ++loop < 4); - } while (len); - } else { - u32 val, loop; - - val = loop = 0; - do { - val |= *buf++ << (8 * loop); - } while (--len && ++loop < 4); - - I915_WRITE(GMBUS3 + reg_offset, val); - I915_WRITE(GMBUS1 + reg_offset, - GMBUS_CYCLE_WAIT | - (i + 1 == num ? GMBUS_CYCLE_STOP : 0) | - (msgs[i].len << GMBUS_BYTE_COUNT_SHIFT) | - (msgs[i].addr << GMBUS_SLAVE_ADDR_SHIFT) | - GMBUS_SLAVE_WRITE | GMBUS_SW_RDY); - POSTING_READ(GMBUS2+reg_offset); - - while (len) { - if (wait_for(I915_READ(GMBUS2 + reg_offset) & (GMBUS_SATOER | GMBUS_HW_RDY), 50)) - goto timeout; - if (I915_READ(GMBUS2 + reg_offset) & GMBUS_SATOER) - goto clear_err; - - val = loop = 0; - do { - val |= *buf++ << (8 * loop); - } while (--len && ++loop < 4); - - I915_WRITE(GMBUS3 + reg_offset, val); - POSTING_READ(GMBUS2+reg_offset); - } - } - - if (i + 1 < num && wait_for(I915_READ(GMBUS2 + reg_offset) & (GMBUS_SATOER | GMBUS_HW_WAIT_PHASE), 50)) + bool last = i + 1 == num; + + if (msgs[i].flags & I2C_M_RD) + ret = gmbus_xfer_read(dev_priv, &msgs[i], last); + else + ret = gmbus_xfer_write(dev_priv, &msgs[i], last); + + if (ret == -ETIMEDOUT) + goto timeout; + if (ret == -ENXIO) + goto clear_err; + + if (!last && + wait_for(I915_READ(GMBUS2 + reg_offset) & + (GMBUS_SATOER | GMBUS_HW_WAIT_PHASE), + 50)) goto timeout; if (I915_READ(GMBUS2 + reg_offset) & GMBUS_SATOER) goto clear_err; -- cgit v1.2.3 From 874e3cc90ba2fe1420945c1ac93781aa8dd33b53 Mon Sep 17 00:00:00 2001 From: Daniel Kurtz Date: Wed, 28 Mar 2012 02:36:11 +0800 Subject: drm/i915/intel_i2c: cleanup error messages and comments Signed-off-by: Daniel Kurtz Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_i2c.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index 30675ce18056..e6c090bff9f2 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -333,17 +333,20 @@ done: * till then let it sleep. */ if (wait_for((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0, 10)) - DRM_INFO("GMBUS timed out waiting for idle\n"); + DRM_INFO("GMBUS [%s] timed out waiting for idle\n", + bus->adapter.name); I915_WRITE(GMBUS0 + reg_offset, 0); ret = i; goto out; timeout: - DRM_INFO("GMBUS timed out, falling back to bit banging on pin %d [%s]\n", - bus->reg0 & 0xff, bus->adapter.name); + DRM_INFO("GMBUS [%s] timed out, falling back to bit banging on pin %d\n", + bus->adapter.name, bus->reg0 & 0xff); I915_WRITE(GMBUS0 + reg_offset, 0); - /* Hardware may not support GMBUS over these pins? Try GPIO bitbanging instead. */ + /* Hardware may not support GMBUS over these pins? + * Try GPIO bitbanging instead. + */ if (!bus->has_gpio) { ret = -EIO; } else { -- cgit v1.2.3 From e4fd17af6156b46fae3f9115830e1a35a62cd8e7 Mon Sep 17 00:00:00 2001 From: Daniel Kurtz Date: Wed, 28 Mar 2012 02:36:12 +0800 Subject: drm/i915/intel_i2c: assign HDMI port D to pin pair 6 According to i915 documentation [1], "Port D" (DP/HDMI Port D) is actually gmbus pin pair 6 (gmbus0.2:0 == 110b GPIOF), not 7 (111b). Pin pair 7 is a reserved pair. [1] Documentation for [DevSNB+] and [DevIBX], as found on http://intellinuxgraphics.org: [DevSNB+]: http://intellinuxgraphics.org/documentation/SNB/IHD_OS_Vol3_Part3.pdf Section 2.2.2 lists the 6 gmbus ports (gpio pin pairs): [ 5: HDMI/DPD, 4: HDMIB, 3: HDMI/DPC, 2: LVDS, 1: SSC, 0: VGA ] 2.2.2.1 lists the GPIO registers to control these 6 ports. 2.2.3.1 lists the mapping between 5 of these gmbus ports and the 3 Pin_Pair_Select bits (of the GMBUS0 register). This table is missing HDMIB (port 101). [DevIBX]: http://intellinuxgraphics.org/IHD_OS_Vol3_Part3r2.pdf Section 2.2.2 lists the same 6 gmbus ports plus two 'reserved' gpio ports. 2.2.2.1 lists 8 GPIO registers... however, it says the size of the block is 6x32, which implies that those 2 reserved GPIO registers (GPIO_6 & GPIO_7) don't actually exist (or are irrelevant). 2.2.3.1 lists the mapping between the 6 named gmbus ports and the 3 Pin_Pair_Select bits (of the GMBUS0 register). This table has HDMIB. Note: the "reserved" and "disabled" pairs do not actually map to a physical pair of pins, nor GPIO regs and shouldn't be initialized or used. Fixing this is left for a later patch. This bug had not been noticed earlier for two reasons: 1) Until recently, "gmbus" mode was disabled - all transfers actually used "bit-bang" mode on GPIO port 5 (the "HDMI/DPD CTLDATA/CLK" pair), at register 0x5024 (defined as GPIOF i915_reg.h). Since this is the correct pair of pins for HDMI1, transfers succeed. 2) Even if gmbus mode is re-enabled, the first attempted transaction will fail because it tries to use the wrong ("Reserved") pin pair. However, the driver immediately falls back again to the bit-bang method, which correctly uses GPIOF, so again, transfers succeed. However, if gmbus mode is re-enabled and the GPIO fall-back mode is disabled, then reading an attached monitor's EDID fail. Signed-off-by: Daniel Kurtz Reviewed-by: Daniel Vetter Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_reg.h | 6 +++--- drivers/gpu/drm/i915/intel_i2c.c | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index f3609f260e5f..accd8ee48f9d 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -742,9 +742,9 @@ #define GMBUS_PORT_PANEL 3 #define GMBUS_PORT_DPC 4 /* HDMIC */ #define GMBUS_PORT_DPB 5 /* SDVO, HDMIB */ - /* 6 reserved */ -#define GMBUS_PORT_DPD 7 /* HDMID */ -#define GMBUS_NUM_PORTS 8 +#define GMBUS_PORT_DPD 6 /* HDMID */ +#define GMBUS_PORT_RESERVED 7 /* 7 reserved */ +#define GMBUS_NUM_PORTS 8 #define GMBUS1 0x5104 /* command/status */ #define GMBUS_SW_CLR_INT (1<<31) #define GMBUS_SW_RDY (1<<30) diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index e6c090bff9f2..9347281633f5 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -148,8 +148,8 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin) GPIOC, GPIOD, GPIOE, - 0, GPIOF, + 0, }; struct i2c_algo_bit_data *algo; @@ -385,8 +385,8 @@ int intel_setup_gmbus(struct drm_device *dev) "panel", "dpc", "dpb", - "reserved", "dpd", + "reserved", }; struct drm_i915_private *dev_priv = dev->dev_private; int ret, i; -- cgit v1.2.3 From 489fbc107f5fb041d598f30f367ea3ca714ff133 Mon Sep 17 00:00:00 2001 From: Daniel Kurtz Date: Wed, 28 Mar 2012 02:36:13 +0800 Subject: drm/i915/intel_i2c: use i2c pre/post_xfer functions to setup gpio xfers Instead of rolling our own custom quirk_xfer function, use the bit_algo pre_xfer and post_xfer functions to setup and teardown bit-banged i2c transactions. Signed-off-by: Daniel Kurtz Reviewed-by: Daniel Vetter Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_i2c.c | 60 +++++++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 26 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index 9347281633f5..1bb63624a793 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -137,6 +137,35 @@ static void set_data(void *data, int state_high) POSTING_READ(bus->gpio_reg); } +static int +intel_gpio_pre_xfer(struct i2c_adapter *adapter) +{ + struct intel_gmbus *bus = container_of(adapter, + struct intel_gmbus, + adapter); + struct drm_i915_private *dev_priv = bus->dev_priv; + + intel_i2c_reset(dev_priv->dev); + intel_i2c_quirk_set(dev_priv, true); + set_data(bus, 1); + set_clock(bus, 1); + udelay(I2C_RISEFALL_TIME); + return 0; +} + +static void +intel_gpio_post_xfer(struct i2c_adapter *adapter) +{ + struct intel_gmbus *bus = container_of(adapter, + struct intel_gmbus, + adapter); + struct drm_i915_private *dev_priv = bus->dev_priv; + + set_data(bus, 1); + set_clock(bus, 1); + intel_i2c_quirk_set(dev_priv, false); +} + static bool intel_gpio_setup(struct intel_gmbus *bus, u32 pin) { @@ -166,6 +195,8 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin) algo->setscl = set_clock; algo->getsda = get_data; algo->getscl = get_clock; + algo->pre_xfer = intel_gpio_pre_xfer; + algo->post_xfer = intel_gpio_post_xfer; algo->udelay = I2C_RISEFALL_TIME; algo->timeout = usecs_to_jiffies(2200); algo->data = bus; @@ -173,30 +204,6 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin) return true; } -static int -intel_i2c_quirk_xfer(struct intel_gmbus *bus, - struct i2c_msg *msgs, - int num) -{ - struct drm_i915_private *dev_priv = bus->dev_priv; - int ret; - - intel_i2c_reset(dev_priv->dev); - - intel_i2c_quirk_set(dev_priv, true); - set_data(bus, 1); - set_clock(bus, 1); - udelay(I2C_RISEFALL_TIME); - - ret = i2c_bit_algo.master_xfer(&bus->adapter, msgs, num); - - set_data(bus, 1); - set_clock(bus, 1); - intel_i2c_quirk_set(dev_priv, false); - - return ret; -} - static int gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg, bool last) @@ -287,7 +294,7 @@ gmbus_xfer(struct i2c_adapter *adapter, mutex_lock(&dev_priv->gmbus_mutex); if (bus->force_bit) { - ret = intel_i2c_quirk_xfer(bus, msgs, num); + ret = i2c_bit_algo.master_xfer(adapter, msgs, num); goto out; } @@ -351,8 +358,9 @@ timeout: ret = -EIO; } else { bus->force_bit = true; - ret = intel_i2c_quirk_xfer(bus, msgs, num); + ret = i2c_bit_algo.master_xfer(adapter, msgs, num); } + out: mutex_unlock(&dev_priv->gmbus_mutex); return ret; -- cgit v1.2.3 From 3bd7d90938f1fe77de5991dc4b727843c4980b2a Mon Sep 17 00:00:00 2001 From: Daniel Kurtz Date: Wed, 28 Mar 2012 02:36:14 +0800 Subject: drm/i915/intel_i2c: refactor using intel_gmbus_get_adapter Instead of letting other modules directly access the ->gmbus array, introduce intel_gmbus_get_adapter() for looking up an i2c_adapter for a given gmbus port identifier. This will enable later refactoring of the gmbus port list. Note: Before requesting an adapter for a given gmbus port number, the driver must first check its validity using i2c_intel_gmbus_is_port_valid(). If this check fails, a call to intel_gmbus_get_adapter() will WARN_ON and return NULL. This is relevant for parts of the driver that read a port from VBIOS, which might be improperly initialized and contain an invalid port. In these cases, the driver must fall back to using a safer default port. Signed-off-by: Daniel Kurtz Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.h | 7 +++++++ drivers/gpu/drm/i915/intel_bios.c | 4 ++-- drivers/gpu/drm/i915/intel_crt.c | 14 ++++++++------ drivers/gpu/drm/i915/intel_dvo.c | 6 +++--- drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++--- drivers/gpu/drm/i915/intel_i2c.c | 8 ++++++++ drivers/gpu/drm/i915/intel_lvds.c | 7 ++++--- drivers/gpu/drm/i915/intel_modes.c | 3 ++- drivers/gpu/drm/i915/intel_sdvo.c | 9 +++++---- 9 files changed, 45 insertions(+), 22 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index e11fcb09aea2..44e6430af3d4 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1342,6 +1342,13 @@ extern int i915_restore_state(struct drm_device *dev); /* intel_i2c.c */ extern int intel_setup_gmbus(struct drm_device *dev); extern void intel_teardown_gmbus(struct drm_device *dev); +extern inline bool intel_gmbus_is_port_valid(unsigned port) +{ + return (port >= GMBUS_PORT_DISABLED && port <= GMBUS_PORT_RESERVED); +} + +extern struct i2c_adapter *intel_gmbus_get_adapter( + struct drm_i915_private *dev_priv, unsigned port); extern void intel_gmbus_set_speed(struct i2c_adapter *adapter, int speed); extern void intel_gmbus_force_bit(struct i2c_adapter *adapter, bool force_bit); extern inline bool intel_gmbus_is_forced_bit(struct i2c_adapter *adapter) diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index e4317da848d5..871aa272bfef 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -372,11 +372,11 @@ parse_general_definitions(struct drm_i915_private *dev_priv, if (block_size >= sizeof(*general)) { int bus_pin = general->crt_ddc_gmbus_pin; DRM_DEBUG_KMS("crt_ddc_bus_pin: %d\n", bus_pin); - if (bus_pin >= 1 && bus_pin <= 6) + if (intel_gmbus_is_port_valid(bus_pin)) dev_priv->crt_ddc_pin = bus_pin; } else { DRM_DEBUG_KMS("BDB_GD too small (%d). Invalid.\n", - block_size); + block_size); } } } diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index 4d3d736a4f56..d54d2327a1b3 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -278,9 +278,10 @@ static bool intel_crt_detect_ddc(struct drm_connector *connector) if (intel_ddc_probe(&crt->base, dev_priv->crt_ddc_pin)) { struct edid *edid; bool is_digital = false; + struct i2c_adapter *i2c; - edid = drm_get_edid(connector, - &dev_priv->gmbus[dev_priv->crt_ddc_pin].adapter); + i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->crt_ddc_pin); + edid = drm_get_edid(connector, i2c); /* * This may be a DVI-I connector with a shared DDC * link between analog and digital outputs, so we @@ -483,15 +484,16 @@ static int intel_crt_get_modes(struct drm_connector *connector) struct drm_device *dev = connector->dev; struct drm_i915_private *dev_priv = dev->dev_private; int ret; + struct i2c_adapter *i2c; - ret = intel_ddc_get_modes(connector, - &dev_priv->gmbus[dev_priv->crt_ddc_pin].adapter); + i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->crt_ddc_pin); + ret = intel_ddc_get_modes(connector, i2c); if (ret || !IS_G4X(dev)) return ret; /* Try to probe digital port for output in DVI-I -> VGA mode. */ - return intel_ddc_get_modes(connector, - &dev_priv->gmbus[GMBUS_PORT_DPB].adapter); + i2c = intel_gmbus_get_adapter(dev_priv, GMBUS_PORT_DPB); + return intel_ddc_get_modes(connector, i2c); } static int intel_crt_set_property(struct drm_connector *connector, diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c index 020a7d7f744d..60ba50b956f2 100644 --- a/drivers/gpu/drm/i915/intel_dvo.c +++ b/drivers/gpu/drm/i915/intel_dvo.c @@ -243,7 +243,7 @@ static int intel_dvo_get_modes(struct drm_connector *connector) * that's not the case. */ intel_ddc_get_modes(connector, - &dev_priv->gmbus[GMBUS_PORT_DPC].adapter); + intel_gmbus_get_adapter(dev_priv, GMBUS_PORT_DPC)); if (!list_empty(&connector->probed_modes)) return 1; @@ -375,7 +375,7 @@ void intel_dvo_init(struct drm_device *dev) * special cases, but otherwise default to what's defined * in the spec. */ - if (dvo->gpio != 0) + if (intel_gmbus_is_port_valid(dvo->gpio)) gpio = dvo->gpio; else if (dvo->type == INTEL_DVO_CHIP_LVDS) gpio = GMBUS_PORT_SSC; @@ -386,7 +386,7 @@ void intel_dvo_init(struct drm_device *dev) * It appears that everything is on GPIOE except for panels * on i830 laptops, which are on GPIOB (DVOA). */ - i2c = &dev_priv->gmbus[gpio].adapter; + i2c = intel_gmbus_get_adapter(dev_priv, gpio); intel_dvo->dev = *dvo; if (!dvo->dev_ops->init(&intel_dvo->dev, i2c)) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index cae3e5f17a49..1d00f61adce6 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -334,7 +334,8 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) intel_hdmi->has_hdmi_sink = false; intel_hdmi->has_audio = false; edid = drm_get_edid(connector, - &dev_priv->gmbus[intel_hdmi->ddc_bus].adapter); + intel_gmbus_get_adapter(dev_priv, + intel_hdmi->ddc_bus)); if (edid) { if (edid->input & DRM_EDID_INPUT_DIGITAL) { @@ -367,7 +368,8 @@ static int intel_hdmi_get_modes(struct drm_connector *connector) */ return intel_ddc_get_modes(connector, - &dev_priv->gmbus[intel_hdmi->ddc_bus].adapter); + intel_gmbus_get_adapter(dev_priv, + intel_hdmi->ddc_bus)); } static bool @@ -379,7 +381,8 @@ intel_hdmi_detect_audio(struct drm_connector *connector) bool has_audio = false; edid = drm_get_edid(connector, - &dev_priv->gmbus[intel_hdmi->ddc_bus].adapter); + intel_gmbus_get_adapter(dev_priv, + intel_hdmi->ddc_bus)); if (edid) { if (edid->input & DRM_EDID_INPUT_DIGITAL) has_audio = drm_detect_monitor_audio(edid); diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index 1bb63624a793..2f65d01340ff 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -449,6 +449,14 @@ err: return ret; } +struct i2c_adapter *intel_gmbus_get_adapter(struct drm_i915_private *dev_priv, + unsigned port) +{ + WARN_ON(!intel_gmbus_is_port_valid(port)); + return (intel_gmbus_is_port_valid(port)) ? + &dev_priv->gmbus[port].adapter : NULL; +} + void intel_gmbus_set_speed(struct i2c_adapter *adapter, int speed) { struct intel_gmbus *bus = to_intel_gmbus(adapter); diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index c5c0973af8a1..4f92a11fc29a 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -837,8 +837,8 @@ static bool lvds_is_present_in_vbt(struct drm_device *dev, child->device_type != DEVICE_TYPE_LFP) continue; - if (child->i2c_pin) - *i2c_pin = child->i2c_pin; + if (intel_gmbus_is_port_valid(child->i2c_pin)) + *i2c_pin = child->i2c_pin; /* However, we cannot trust the BIOS writers to populate * the VBT correctly. Since LVDS requires additional @@ -979,7 +979,8 @@ bool intel_lvds_init(struct drm_device *dev) * preferred mode is the right one. */ intel_lvds->edid = drm_get_edid(connector, - &dev_priv->gmbus[pin].adapter); + intel_gmbus_get_adapter(dev_priv, + pin)); if (intel_lvds->edid) { if (drm_add_edid_modes(connector, intel_lvds->edid)) { diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c index 2978a3f61b58..cc682a02fa2f 100644 --- a/drivers/gpu/drm/i915/intel_modes.c +++ b/drivers/gpu/drm/i915/intel_modes.c @@ -55,7 +55,8 @@ bool intel_ddc_probe(struct intel_encoder *intel_encoder, int ddc_bus) } }; - return i2c_transfer(&dev_priv->gmbus[ddc_bus].adapter, msgs, 2) == 2; + return i2c_transfer(intel_gmbus_get_adapter(dev_priv, ddc_bus), + msgs, 2) == 2; } /** diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 70fb275cd205..830c8b5d90b8 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -1254,7 +1254,8 @@ intel_sdvo_get_analog_edid(struct drm_connector *connector) struct drm_i915_private *dev_priv = connector->dev->dev_private; return drm_get_edid(connector, - &dev_priv->gmbus[dev_priv->crt_ddc_pin].adapter); + intel_gmbus_get_adapter(dev_priv, + dev_priv->crt_ddc_pin)); } enum drm_connector_status @@ -1922,12 +1923,12 @@ intel_sdvo_select_i2c_bus(struct drm_i915_private *dev_priv, if (mapping->initialized) pin = mapping->i2c_pin; - if (pin < GMBUS_NUM_PORTS) { - sdvo->i2c = &dev_priv->gmbus[pin].adapter; + if (intel_gmbus_is_port_valid(pin)) { + sdvo->i2c = intel_gmbus_get_adapter(dev_priv, pin); intel_gmbus_set_speed(sdvo->i2c, GMBUS_RATE_1MHZ); intel_gmbus_force_bit(sdvo->i2c, true); } else { - sdvo->i2c = &dev_priv->gmbus[GMBUS_PORT_DPB].adapter; + sdvo->i2c = intel_gmbus_get_adapter(dev_priv, GMBUS_PORT_DPB); } } -- cgit v1.2.3 From 2ed06c93a1fce057808894d73167aae03c76deaf Mon Sep 17 00:00:00 2001 From: Daniel Kurtz Date: Wed, 28 Mar 2012 02:36:15 +0800 Subject: drm/i915/intel_i2c: gmbus disabled and reserved ports are invalid There is no GMBUS "disabled" port 0, nor "reserved" port 7. For the other 6 ports there is a fixed 1:1 mapping between pin pairs and gmbus ports, which means every real gmbus port has a gpio pin. Given these realizations, clean up gmbus initialization. Tested on Sandybridge (gen 6, PCH == CougarPoint) hardware. Signed-off-by: Daniel Kurtz Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.h | 3 +- drivers/gpu/drm/i915/i915_reg.h | 2 +- drivers/gpu/drm/i915/intel_i2c.c | 70 ++++++++++++++++------------------------ 3 files changed, 29 insertions(+), 46 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 44e6430af3d4..c5ad7b96b065 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -303,7 +303,6 @@ struct intel_fbc_work; struct intel_gmbus { struct i2c_adapter adapter; bool force_bit; - bool has_gpio; u32 reg0; u32 gpio_reg; struct i2c_algo_bit_data bit_algo; @@ -1344,7 +1343,7 @@ extern int intel_setup_gmbus(struct drm_device *dev); extern void intel_teardown_gmbus(struct drm_device *dev); extern inline bool intel_gmbus_is_port_valid(unsigned port) { - return (port >= GMBUS_PORT_DISABLED && port <= GMBUS_PORT_RESERVED); + return (port >= GMBUS_PORT_SSC && port <= GMBUS_PORT_DPD); } extern struct i2c_adapter *intel_gmbus_get_adapter( diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index accd8ee48f9d..a8d218ca7d1d 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -744,7 +744,7 @@ #define GMBUS_PORT_DPB 5 /* SDVO, HDMIB */ #define GMBUS_PORT_DPD 6 /* HDMID */ #define GMBUS_PORT_RESERVED 7 /* 7 reserved */ -#define GMBUS_NUM_PORTS 8 +#define GMBUS_NUM_PORTS (GMBUS_PORT_DPD - GMBUS_PORT_SSC + 1) #define GMBUS1 0x5104 /* command/status */ #define GMBUS_SW_CLR_INT (1<<31) #define GMBUS_SW_RDY (1<<30) diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index 2f65d01340ff..dcde6f64777a 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -35,6 +35,20 @@ #include "i915_drm.h" #include "i915_drv.h" +struct gmbus_port { + const char *name; + int reg; +}; + +static const struct gmbus_port gmbus_ports[] = { + { "ssc", GPIOB }, + { "vga", GPIOA }, + { "panel", GPIOC }, + { "dpc", GPIOD }, + { "dpb", GPIOE }, + { "dpd", GPIOF }, +}; + /* Intel GPIO access functions */ #define I2C_RISEFALL_TIME 10 @@ -166,29 +180,16 @@ intel_gpio_post_xfer(struct i2c_adapter *adapter) intel_i2c_quirk_set(dev_priv, false); } -static bool +static void intel_gpio_setup(struct intel_gmbus *bus, u32 pin) { struct drm_i915_private *dev_priv = bus->dev_priv; - static const int map_pin_to_reg[] = { - 0, - GPIOB, - GPIOA, - GPIOC, - GPIOD, - GPIOE, - GPIOF, - 0, - }; struct i2c_algo_bit_data *algo; - if (pin >= ARRAY_SIZE(map_pin_to_reg) || !map_pin_to_reg[pin]) - return false; - algo = &bus->bit_algo; - bus->gpio_reg = map_pin_to_reg[pin]; - bus->gpio_reg += dev_priv->gpio_mmio_base; + /* -1 to map pin pair to gmbus index */ + bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_ports[pin - 1].reg; bus->adapter.algo_data = algo; algo->setsda = set_data; @@ -200,8 +201,6 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin) algo->udelay = I2C_RISEFALL_TIME; algo->timeout = usecs_to_jiffies(2200); algo->data = bus; - - return true; } static int @@ -351,15 +350,9 @@ timeout: bus->adapter.name, bus->reg0 & 0xff); I915_WRITE(GMBUS0 + reg_offset, 0); - /* Hardware may not support GMBUS over these pins? - * Try GPIO bitbanging instead. - */ - if (!bus->has_gpio) { - ret = -EIO; - } else { - bus->force_bit = true; - ret = i2c_bit_algo.master_xfer(adapter, msgs, num); - } + /* Hardware may not support GMBUS over these pins? Try GPIO bitbanging instead. */ + bus->force_bit = true; + ret = i2c_bit_algo.master_xfer(adapter, msgs, num); out: mutex_unlock(&dev_priv->gmbus_mutex); @@ -386,16 +379,6 @@ static const struct i2c_algorithm gmbus_algorithm = { */ int intel_setup_gmbus(struct drm_device *dev) { - static const char *names[GMBUS_NUM_PORTS] = { - "disabled", - "ssc", - "vga", - "panel", - "dpc", - "dpb", - "dpd", - "reserved", - }; struct drm_i915_private *dev_priv = dev->dev_private; int ret, i; @@ -413,13 +396,14 @@ int intel_setup_gmbus(struct drm_device *dev) for (i = 0; i < GMBUS_NUM_PORTS; i++) { struct intel_gmbus *bus = &dev_priv->gmbus[i]; + u32 port = i + 1; /* +1 to map gmbus index to pin pair */ bus->adapter.owner = THIS_MODULE; bus->adapter.class = I2C_CLASS_DDC; snprintf(bus->adapter.name, sizeof(bus->adapter.name), "i915 gmbus %s", - names[i]); + gmbus_ports[i].name); bus->adapter.dev.parent = &dev->pdev->dev; bus->dev_priv = dev_priv; @@ -430,9 +414,9 @@ int intel_setup_gmbus(struct drm_device *dev) goto err; /* By default use a conservative clock rate */ - bus->reg0 = i | GMBUS_RATE_100KHZ; + bus->reg0 = port | GMBUS_RATE_100KHZ; - bus->has_gpio = intel_gpio_setup(bus, i); + intel_gpio_setup(bus, port); } intel_i2c_reset(dev_priv->dev); @@ -453,8 +437,9 @@ struct i2c_adapter *intel_gmbus_get_adapter(struct drm_i915_private *dev_priv, unsigned port) { WARN_ON(!intel_gmbus_is_port_valid(port)); + /* -1 to map pin pair to gmbus index */ return (intel_gmbus_is_port_valid(port)) ? - &dev_priv->gmbus[port].adapter : NULL; + &dev_priv->gmbus[port - 1].adapter : NULL; } void intel_gmbus_set_speed(struct i2c_adapter *adapter, int speed) @@ -468,8 +453,7 @@ void intel_gmbus_force_bit(struct i2c_adapter *adapter, bool force_bit) { struct intel_gmbus *bus = to_intel_gmbus(adapter); - if (bus->has_gpio) - bus->force_bit = force_bit; + bus->force_bit = force_bit; } void intel_teardown_gmbus(struct drm_device *dev) -- cgit v1.2.3 From f2c9677be3158c31ba19f527e2be0f7a519e19d1 Mon Sep 17 00:00:00 2001 From: Daniel Kurtz Date: Wed, 28 Mar 2012 02:36:16 +0800 Subject: drm/i915/intel_i2c: allocate gmbus array as part of drm_i915_private This memory is always allocated, and it is always a fixed size, so just allocate it along with the rest of the driver state. Signed-off-by: Daniel Kurtz Reviewed-by: Daniel Vetter Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/intel_i2c.c | 10 ---------- 2 files changed, 1 insertion(+), 11 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index c5ad7b96b065..6983b4bcbdea 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -326,7 +326,7 @@ typedef struct drm_i915_private { /** gt_lock is also taken in irq contexts. */ struct spinlock gt_lock; - struct intel_gmbus *gmbus; + struct intel_gmbus gmbus[GMBUS_NUM_PORTS]; /** gmbus_mutex protects against concurrent usage of the single hw gmbus * controller on different i2c buses. */ diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index dcde6f64777a..c12db7265893 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -387,11 +387,6 @@ int intel_setup_gmbus(struct drm_device *dev) else dev_priv->gpio_mmio_base = 0; - dev_priv->gmbus = kcalloc(GMBUS_NUM_PORTS, sizeof(struct intel_gmbus), - GFP_KERNEL); - if (dev_priv->gmbus == NULL) - return -ENOMEM; - mutex_init(&dev_priv->gmbus_mutex); for (i = 0; i < GMBUS_NUM_PORTS; i++) { @@ -428,8 +423,6 @@ err: struct intel_gmbus *bus = &dev_priv->gmbus[i]; i2c_del_adapter(&bus->adapter); } - kfree(dev_priv->gmbus); - dev_priv->gmbus = NULL; return ret; } @@ -468,7 +461,4 @@ void intel_teardown_gmbus(struct drm_device *dev) struct intel_gmbus *bus = &dev_priv->gmbus[i]; i2c_del_adapter(&bus->adapter); } - - kfree(dev_priv->gmbus); - dev_priv->gmbus = NULL; } -- cgit v1.2.3 From 26883c31b0799e76edf8f0ea8be48b64e09b2a7d Mon Sep 17 00:00:00 2001 From: Daniel Kurtz Date: Fri, 30 Mar 2012 19:46:36 +0800 Subject: drm/i915/intel_i2c: handle zero-length writes A common method of probing an i2c bus is trying to do a zero-length write. Handle this case by checking the length first before decrementing it. This is actually important, since attempting a zero-length write is one of the ways that i2cdetect and i2c_new_probed_device detect whether there is device present on the bus with a given address. Signed-off-by: Daniel Kurtz Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_i2c.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index c12db7265893..99a04f8bcb1f 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -248,9 +248,10 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg, u32 val, loop; val = loop = 0; - do { - val |= *buf++ << (8 * loop); - } while (--len && ++loop < 4); + while (len && loop < 4) { + val |= *buf++ << (8 * loop++); + len -= 1; + } I915_WRITE(GMBUS3 + reg_offset, val); I915_WRITE(GMBUS1 + reg_offset, -- cgit v1.2.3 From 7a39a9d4767e8d22d60f2c4bf5eece4f4398c274 Mon Sep 17 00:00:00 2001 From: Daniel Kurtz Date: Fri, 30 Mar 2012 19:46:37 +0800 Subject: drm/i915/intel_i2c: use double-buffered writes The GMBUS controller GMBUS3 register is double-buffered. Take advantage of this by writing two 4-byte words before the first wait for HW_RDY. This helps keep the GMBUS controller from becoming idle during long writes. In fact, during experiments using the GMBUS interrupts, the HW_RDY interrupt would only trigger for transactions >4 bytes after 2 writes to GMBUS3. Signed-off-by: Daniel Kurtz Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_i2c.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index 99a04f8bcb1f..f02e52aca746 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -262,13 +262,6 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg, GMBUS_SLAVE_WRITE | GMBUS_SW_RDY); POSTING_READ(GMBUS2 + reg_offset); while (len) { - if (wait_for(I915_READ(GMBUS2 + reg_offset) & - (GMBUS_SATOER | GMBUS_HW_RDY), - 50)) - return -ETIMEDOUT; - if (I915_READ(GMBUS2 + reg_offset) & GMBUS_SATOER) - return -ENXIO; - val = loop = 0; do { val |= *buf++ << (8 * loop); @@ -276,6 +269,13 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg, I915_WRITE(GMBUS3 + reg_offset, val); POSTING_READ(GMBUS2 + reg_offset); + + if (wait_for(I915_READ(GMBUS2 + reg_offset) & + (GMBUS_SATOER | GMBUS_HW_RDY), + 50)) + return -ETIMEDOUT; + if (I915_READ(GMBUS2 + reg_offset) & GMBUS_SATOER) + return -ENXIO; } return 0; } -- cgit v1.2.3 From e646d5773572bf52017983d758bdf05777dc5600 Mon Sep 17 00:00:00 2001 From: Daniel Kurtz Date: Fri, 30 Mar 2012 19:46:38 +0800 Subject: drm/i915/intel_i2c: always wait for IDLE before clearing NAK The GMBUS controller can report a NAK condition while a transaction is still active. If the driver is fast enough, and the bus is slow enough, the driver may clear the NAK condition while the controller is still busy, resulting in a confused GMBUS controller. This will leave the controller in a bad state such that the next transaction may fail. Also, return -ENXIO if a device NAKs a transaction. Note: this patch also refactors gmbus_xfer to remove the "done" label. Signed-off-by: Daniel Kurtz Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_i2c.c | 41 ++++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index f02e52aca746..9707868881ec 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -324,26 +324,47 @@ gmbus_xfer(struct i2c_adapter *adapter, goto clear_err; } - goto done; + /* Mark the GMBUS interface as disabled after waiting for idle. + * 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)) + DRM_INFO("GMBUS [%s] timed out waiting for idle\n", + adapter->name); + I915_WRITE(GMBUS0 + reg_offset, 0); + ret = i; + goto out; clear_err: + /* + * Wait for bus to IDLE before clearing NAK. + * If we clear the NAK while bus is still active, then it will stay + * active and the next transaction may fail. + */ + if (wait_for((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0, + 10)) + DRM_INFO("GMBUS [%s] timed out after NAK\n", adapter->name); + /* Toggle the Software Clear Interrupt bit. This has the effect * of resetting the GMBUS controller and so clearing the * BUS_ERROR raised by the slave's NAK. */ I915_WRITE(GMBUS1 + reg_offset, GMBUS_SW_CLR_INT); I915_WRITE(GMBUS1 + reg_offset, 0); + I915_WRITE(GMBUS0 + reg_offset, 0); -done: - /* Mark the GMBUS interface as disabled after waiting for idle. - * We will re-enable it at the start of the next xfer, - * till then let it sleep. + DRM_DEBUG_DRIVER("GMBUS [%s] NAK for addr: %04x %c(%d)\n", + adapter->name, msgs[i].addr, + (msgs[i].flags & I2C_M_RD) ? 'r' : 'w', msgs[i].len); + + /* + * If no ACK is received during the address phase of a transaction, + * the adapter must report -ENXIO. + * It is not clear what to return if no ACK is received at other times. + * So, we always return -ENXIO in all NAK cases, to ensure we send + * it at least during the one case that is specified. */ - if (wait_for((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0, 10)) - DRM_INFO("GMBUS [%s] timed out waiting for idle\n", - bus->adapter.name); - I915_WRITE(GMBUS0 + reg_offset, 0); - ret = i; + ret = -ENXIO; goto out; timeout: -- cgit v1.2.3 From 72d66afd1461effb143784a0f6cde2a9f9908b70 Mon Sep 17 00:00:00 2001 From: Daniel Kurtz Date: Fri, 30 Mar 2012 19:46:39 +0800 Subject: drm/i915/intel_i2c: use WAIT cycle, not STOP The i915 is only able to generate a STOP cycle (i.e. finalize an i2c transaction) during a DATA or WAIT phase. In other words, the controller rejects a STOP requested as part of the first transaction in a sequence. Thus, for the first transaction we must always use a WAIT cycle, detect when the device has finished (and is in a WAIT phase), and then either start the next transaction, or, if there are no more transactions, generate a STOP cycle. Note: Theoretically, the last transaction of a multi-transaction sequence could initiate a STOP cycle. However, this slight optimization is left for another patch. We return -ETIMEDOUT if the hardware doesn't deactivate after the STOP cycle. Signed-off-by: Daniel Kurtz [danvet: added comment to the code that gmbus can't generate STOP on the very first cycle.] Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_i2c.c | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index 9707868881ec..291e51ec309b 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -204,8 +204,7 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin) } static int -gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg, - bool last) +gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg) { int reg_offset = dev_priv->gpio_mmio_base; u16 len = msg->len; @@ -213,7 +212,6 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg, I915_WRITE(GMBUS1 + reg_offset, GMBUS_CYCLE_WAIT | - (last ? GMBUS_CYCLE_STOP : 0) | (len << GMBUS_BYTE_COUNT_SHIFT) | (msg->addr << GMBUS_SLAVE_ADDR_SHIFT) | GMBUS_SLAVE_READ | GMBUS_SW_RDY); @@ -239,8 +237,7 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg, } static int -gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg, - bool last) +gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg) { int reg_offset = dev_priv->gpio_mmio_base; u16 len = msg->len; @@ -256,7 +253,6 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg, I915_WRITE(GMBUS3 + reg_offset, val); I915_WRITE(GMBUS1 + reg_offset, GMBUS_CYCLE_WAIT | - (last ? GMBUS_CYCLE_STOP : 0) | (msg->len << GMBUS_BYTE_COUNT_SHIFT) | (msg->addr << GMBUS_SLAVE_ADDR_SHIFT) | GMBUS_SLAVE_WRITE | GMBUS_SW_RDY); @@ -289,7 +285,8 @@ gmbus_xfer(struct i2c_adapter *adapter, struct intel_gmbus, adapter); struct drm_i915_private *dev_priv = bus->dev_priv; - int i, reg_offset, ret; + int i, reg_offset; + int ret = 0; mutex_lock(&dev_priv->gmbus_mutex); @@ -303,20 +300,17 @@ gmbus_xfer(struct i2c_adapter *adapter, I915_WRITE(GMBUS0 + reg_offset, bus->reg0); for (i = 0; i < num; i++) { - bool last = i + 1 == num; - if (msgs[i].flags & I2C_M_RD) - ret = gmbus_xfer_read(dev_priv, &msgs[i], last); + ret = gmbus_xfer_read(dev_priv, &msgs[i]); else - ret = gmbus_xfer_write(dev_priv, &msgs[i], last); + ret = gmbus_xfer_write(dev_priv, &msgs[i]); if (ret == -ETIMEDOUT) goto timeout; if (ret == -ENXIO) goto clear_err; - if (!last && - wait_for(I915_READ(GMBUS2 + reg_offset) & + if (wait_for(I915_READ(GMBUS2 + reg_offset) & (GMBUS_SATOER | GMBUS_HW_WAIT_PHASE), 50)) goto timeout; @@ -324,15 +318,24 @@ gmbus_xfer(struct i2c_adapter *adapter, goto clear_err; } + /* Generate a STOP condition on the bus. Note that gmbus can't generata + * a STOP on the very first cycle. To simplify the code we + * unconditionally generate the STOP condition with an additional gmbus + * cycle. */ + I915_WRITE(GMBUS1 + reg_offset, GMBUS_CYCLE_STOP | GMBUS_SW_RDY); + /* Mark the GMBUS interface as disabled after waiting for idle. * 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 (wait_for((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0, + 10)) { DRM_INFO("GMBUS [%s] timed out waiting for idle\n", adapter->name); + ret = -ETIMEDOUT; + } I915_WRITE(GMBUS0 + reg_offset, 0); - ret = i; + ret = ret ?: i; goto out; clear_err: -- cgit v1.2.3 From 56f9eac05489912ac0165ffc0ebff0f8588f77d2 Mon Sep 17 00:00:00 2001 From: Daniel Kurtz Date: Fri, 30 Mar 2012 19:46:40 +0800 Subject: drm/i915/intel_i2c: use INDEX cycles for i2c read transactions It is very common for an i2c device to require a small 1 or 2 byte write followed by a read. For example, when reading from an i2c EEPROM it is common to write and address, offset or index followed by a reading some values. The i915 gmbus controller provides a special "INDEX" cycle for performing such a small write followed by a read. The INDEX can be either one or two bytes long. The advantage of using such a cycle is that the CPU has slightly less work to do once the read with INDEX cycle is started. Signed-off-by: Daniel Kurtz Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_i2c.c | 54 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 50 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index 291e51ec309b..5e0912a4a737 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -204,13 +204,15 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin) } static int -gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg) +gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg, + u32 gmbus1_index) { int reg_offset = dev_priv->gpio_mmio_base; u16 len = msg->len; u8 *buf = msg->buf; I915_WRITE(GMBUS1 + reg_offset, + gmbus1_index | GMBUS_CYCLE_WAIT | (len << GMBUS_BYTE_COUNT_SHIFT) | (msg->addr << GMBUS_SLAVE_ADDR_SHIFT) | @@ -276,6 +278,46 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg) return 0; } +/* + * The gmbus controller can combine a 1 or 2 byte write with a read that + * immediately follows it by using an "INDEX" cycle. + */ +static bool +gmbus_is_index_read(struct i2c_msg *msgs, int i, int num) +{ + return (i + 1 < num && + !(msgs[i].flags & I2C_M_RD) && msgs[i].len <= 2 && + (msgs[i + 1].flags & I2C_M_RD)); +} + +static int +gmbus_xfer_index_read(struct drm_i915_private *dev_priv, struct i2c_msg *msgs) +{ + int reg_offset = dev_priv->gpio_mmio_base; + u32 gmbus1_index = 0; + u32 gmbus5 = 0; + int ret; + + if (msgs[0].len == 2) + gmbus5 = GMBUS_2BYTE_INDEX_EN | + msgs[0].buf[1] | (msgs[0].buf[0] << 8); + if (msgs[0].len == 1) + gmbus1_index = GMBUS_CYCLE_INDEX | + (msgs[0].buf[0] << GMBUS_SLAVE_INDEX_SHIFT); + + /* GMBUS5 holds 16-bit index */ + if (gmbus5) + I915_WRITE(GMBUS5 + reg_offset, gmbus5); + + ret = gmbus_xfer_read(dev_priv, &msgs[1], gmbus1_index); + + /* Clear GMBUS5 after each index transfer */ + if (gmbus5) + I915_WRITE(GMBUS5 + reg_offset, 0); + + return ret; +} + static int gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, @@ -300,10 +342,14 @@ gmbus_xfer(struct i2c_adapter *adapter, I915_WRITE(GMBUS0 + reg_offset, bus->reg0); for (i = 0; i < num; i++) { - if (msgs[i].flags & I2C_M_RD) - ret = gmbus_xfer_read(dev_priv, &msgs[i]); - else + if (gmbus_is_index_read(msgs, i, num)) { + ret = gmbus_xfer_index_read(dev_priv, &msgs[i]); + i += 1; /* set i to the index of the read xfer */ + } else if (msgs[i].flags & I2C_M_RD) { + ret = gmbus_xfer_read(dev_priv, &msgs[i], 0); + } else { ret = gmbus_xfer_write(dev_priv, &msgs[i]); + } if (ret == -ETIMEDOUT) goto timeout; -- cgit v1.2.3 From 90e6b26d6b28ade684a4b16b856a74f27bc644bc Mon Sep 17 00:00:00 2001 From: Daniel Kurtz Date: Fri, 30 Mar 2012 19:46:41 +0800 Subject: drm/i915/intel_i2c: reuse GMBUS2 value read in polling loop Save the GMBUS2 value read while polling for state changes, and then reuse this value when determining for which reason the loops were exited. This is a small optimization which saves a couple of bus accesses for memory mapped IO registers. To avoid "assigning in if clause" checkpatch errors", use a ret variable to store the wait_for macro return value. Signed-off-by: Daniel Kurtz Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_i2c.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index 5e0912a4a737..c56cc350c9cf 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -219,13 +219,16 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg, GMBUS_SLAVE_READ | GMBUS_SW_RDY); POSTING_READ(GMBUS2 + reg_offset); do { + int ret; u32 val, loop = 0; + u32 gmbus2; - if (wait_for(I915_READ(GMBUS2 + reg_offset) & - (GMBUS_SATOER | GMBUS_HW_RDY), - 50)) + ret = wait_for((gmbus2 = I915_READ(GMBUS2 + reg_offset)) & + (GMBUS_SATOER | GMBUS_HW_RDY), + 50); + if (ret) return -ETIMEDOUT; - if (I915_READ(GMBUS2 + reg_offset) & GMBUS_SATOER) + if (gmbus2 & GMBUS_SATOER) return -ENXIO; val = I915_READ(GMBUS3 + reg_offset); @@ -260,6 +263,9 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg) GMBUS_SLAVE_WRITE | GMBUS_SW_RDY); POSTING_READ(GMBUS2 + reg_offset); while (len) { + int ret; + u32 gmbus2; + val = loop = 0; do { val |= *buf++ << (8 * loop); @@ -268,11 +274,12 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg) I915_WRITE(GMBUS3 + reg_offset, val); POSTING_READ(GMBUS2 + reg_offset); - if (wait_for(I915_READ(GMBUS2 + reg_offset) & - (GMBUS_SATOER | GMBUS_HW_RDY), - 50)) + ret = wait_for((gmbus2 = I915_READ(GMBUS2 + reg_offset)) & + (GMBUS_SATOER | GMBUS_HW_RDY), + 50); + if (ret) return -ETIMEDOUT; - if (I915_READ(GMBUS2 + reg_offset) & GMBUS_SATOER) + if (gmbus2 & GMBUS_SATOER) return -ENXIO; } return 0; @@ -342,6 +349,8 @@ gmbus_xfer(struct i2c_adapter *adapter, I915_WRITE(GMBUS0 + reg_offset, bus->reg0); for (i = 0; i < num; i++) { + u32 gmbus2; + if (gmbus_is_index_read(msgs, i, num)) { ret = gmbus_xfer_index_read(dev_priv, &msgs[i]); i += 1; /* set i to the index of the read xfer */ @@ -356,11 +365,12 @@ gmbus_xfer(struct i2c_adapter *adapter, if (ret == -ENXIO) goto clear_err; - if (wait_for(I915_READ(GMBUS2 + reg_offset) & - (GMBUS_SATOER | GMBUS_HW_WAIT_PHASE), - 50)) + ret = wait_for((gmbus2 = I915_READ(GMBUS2 + reg_offset)) & + (GMBUS_SATOER | GMBUS_HW_WAIT_PHASE), + 50); + if (ret) goto timeout; - if (I915_READ(GMBUS2 + reg_offset) & GMBUS_SATOER) + if (gmbus2 & GMBUS_SATOER) goto clear_err; } -- cgit v1.2.3 From e2ba4fb313d91c9e888f973e65a736f8f55b12b6 Mon Sep 17 00:00:00 2001 From: Daniel Kurtz Date: Fri, 30 Mar 2012 19:46:42 +0800 Subject: drm/i915/intel_i2c: remove POSTING_READ() from gmbus transfers The POSTING_READ() calls were originally added to make sure the writes were flushed before any timing delays and across loops. Now that the code has settled a bit, let's remove them. Signed-off-by: Daniel Kurtz Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_i2c.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index c56cc350c9cf..cab879fedf3c 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -217,7 +217,6 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg, (len << GMBUS_BYTE_COUNT_SHIFT) | (msg->addr << GMBUS_SLAVE_ADDR_SHIFT) | GMBUS_SLAVE_READ | GMBUS_SW_RDY); - POSTING_READ(GMBUS2 + reg_offset); do { int ret; u32 val, loop = 0; @@ -261,7 +260,6 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg) (msg->len << GMBUS_BYTE_COUNT_SHIFT) | (msg->addr << GMBUS_SLAVE_ADDR_SHIFT) | GMBUS_SLAVE_WRITE | GMBUS_SW_RDY); - POSTING_READ(GMBUS2 + reg_offset); while (len) { int ret; u32 gmbus2; @@ -272,7 +270,6 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg) } while (--len && ++loop < 4); I915_WRITE(GMBUS3 + reg_offset, val); - POSTING_READ(GMBUS2 + reg_offset); ret = wait_for((gmbus2 = I915_READ(GMBUS2 + reg_offset)) & (GMBUS_SATOER | GMBUS_HW_RDY), -- cgit v1.2.3 From 79985eee842ef146ed6307a29fdc2fa008036421 Mon Sep 17 00:00:00 2001 From: Daniel Kurtz Date: Fri, 13 Apr 2012 19:47:53 +0800 Subject: drm/i915/intel_i2c: handle zero-length reads A common method of probing an i2c bus is trying to do a zero-length read. Handle this case by checking the length first waiting for data to be read. This is actually important, since attempting a zero-length read is one of the ways that i2cdetect and i2c_new_probed_device detect whether there is device present on the bus with a given address. Reviewed-by: Chris Wilson Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=48269 Signed-off-by: Daniel Kurtz Signed-Off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_i2c.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index cab879fedf3c..e24916042550 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -217,7 +217,7 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg, (len << GMBUS_BYTE_COUNT_SHIFT) | (msg->addr << GMBUS_SLAVE_ADDR_SHIFT) | GMBUS_SLAVE_READ | GMBUS_SW_RDY); - do { + while (len) { int ret; u32 val, loop = 0; u32 gmbus2; @@ -235,7 +235,7 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg, *buf++ = val & 0xff; val >>= 8; } while (--len && ++loop < 4); - } while (len); + } return 0; } -- cgit v1.2.3 From 56fa6d6ff76c7700f8dd131bee9ffa6c3c06dcd4 Mon Sep 17 00:00:00 2001 From: Daniel Kurtz Date: Fri, 13 Apr 2012 19:47:54 +0800 Subject: drm/i915/intel_i2c: reduce verbosity of some messages Some of these messages can be hit when userspace tries to probe the i2c with nothing connected or if the driver code tries to do the same. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=48248 Signed-off-by: Daniel Kurtz Signed-Off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_i2c.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index e24916042550..e04255edc801 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -383,7 +383,7 @@ gmbus_xfer(struct i2c_adapter *adapter, */ if (wait_for((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0, 10)) { - DRM_INFO("GMBUS [%s] timed out waiting for idle\n", + DRM_DEBUG_KMS("GMBUS [%s] timed out waiting for idle\n", adapter->name); ret = -ETIMEDOUT; } @@ -399,7 +399,8 @@ clear_err: */ if (wait_for((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0, 10)) - DRM_INFO("GMBUS [%s] timed out after NAK\n", adapter->name); + DRM_DEBUG_KMS("GMBUS [%s] timed out after NAK\n", + adapter->name); /* Toggle the Software Clear Interrupt bit. This has the effect * of resetting the GMBUS controller and so clearing the @@ -409,7 +410,7 @@ clear_err: I915_WRITE(GMBUS1 + reg_offset, 0); I915_WRITE(GMBUS0 + reg_offset, 0); - DRM_DEBUG_DRIVER("GMBUS [%s] NAK for addr: %04x %c(%d)\n", + DRM_DEBUG_KMS("GMBUS [%s] NAK for addr: %04x %c(%d)\n", adapter->name, msgs[i].addr, (msgs[i].flags & I2C_M_RD) ? 'r' : 'w', msgs[i].len); -- cgit v1.2.3 From 83ee9e645846f0c56bd9b33ee28ead03b416bb25 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sun, 13 May 2012 14:44:20 +0200 Subject: drm/i915: disable gmbus on i830 The hw just returns garbage. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=49838 Reported-and-tested-by: Vladyslav Acked-by: Chris Wilson Signed-Off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_i2c.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index e04255edc801..4a9707dd0f9c 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -490,6 +490,10 @@ int intel_setup_gmbus(struct drm_device *dev) /* By default use a conservative clock rate */ bus->reg0 = port | GMBUS_RATE_100KHZ; + /* gmbus seems to be broken on i830 */ + if (IS_I830(dev)) + bus->force_bit = true; + intel_gpio_setup(bus, port); } -- cgit v1.2.3 From 65e818660275ecda3702a4245f308923e3813a85 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Mon, 21 May 2012 20:19:48 +0200 Subject: drm/i915: be more careful when returning -ENXIO in gmbus transfer ... flaky ddc hardware can cause a spurious NAK, resulting in the i2c core and drm edid functions not trying to retry the edid transfer. Luckily the gmbus quiescenting also times out for these cases, so we can get out of this mess by returning -ETIMEDOUT for this specific case. This way we keep the fast-fail of returning -ENXIO if there is no device present, speeding up the boot process. This regression has been introduced in commit e646d5773572bf52017983d758bdf05777dc5600 Author: Daniel Kurtz Date: Fri Mar 30 19:46:38 2012 +0800 drm/i915/intel_i2c: always wait for IDLE before clearing NAK v2: Return -ETIMEDOUT for this case and keep the -ENXIO for real NAKs, suggested by Daniel Kurtz. Cc: Daniel Kurtz Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=49518 Reported-and-Tested-by: Julian Simioni Acked-by: Chris Wilson Reviewed-by: Daniel Kurtz Signed-Off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_i2c.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index 4a9707dd0f9c..1991a4408cf9 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -396,11 +396,22 @@ clear_err: * Wait for bus to IDLE before clearing NAK. * If we clear the NAK while bus is still active, then it will stay * active and the next transaction may fail. + * + * If no ACK is received during the address phase of a transaction, the + * adapter must report -ENXIO. It is not clear what to return if no ACK + * is received at other times. But we have to be careful to not return + * spurious -ENXIO because that will prevent i2c and drm edid functions + * from retrying. So return -ENXIO only when gmbus properly quiescents - + * timing out seems to happen when there _is_ a ddc chip present, but + * 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)) + 10)) { DRM_DEBUG_KMS("GMBUS [%s] timed out after NAK\n", adapter->name); + ret = -ETIMEDOUT; + } /* Toggle the Software Clear Interrupt bit. This has the effect * of resetting the GMBUS controller and so clearing the @@ -414,14 +425,6 @@ clear_err: adapter->name, msgs[i].addr, (msgs[i].flags & I2C_M_RD) ? 'r' : 'w', msgs[i].len); - /* - * If no ACK is received during the address phase of a transaction, - * the adapter must report -ENXIO. - * It is not clear what to return if no ACK is received at other times. - * So, we always return -ENXIO in all NAK cases, to ensure we send - * it at least during the one case that is specified. - */ - ret = -ENXIO; goto out; timeout: -- cgit v1.2.3