Re: [PATCH 25/43] drm/i915: Include gpio_mmio_base in GMBUS reg defines

Jesse Barnes
2015-10-12T15:55:28Z

On 09/18/2015 10:03 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 12 ++++-----
> drivers/gpu/drm/i915/intel_i2c.c | 54 +++++++++++++++++-----------------------
> 2 files changed, 29 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b35e24f..0216771 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2114,7 +2114,7 @@ enum skl_disp_power_wells {
> # define GPIO_DATA_VAL_IN (1 << 12)
> # define GPIO_DATA_PULLUP_DISABLE (1 << 13)
>
> -#define GMBUS0 0x5100 /* clock/port select */
> +#define GMBUS0 (dev_priv->gpio_mmio_base + 0x5100) /* clock/port select */
> #define GMBUS_RATE_100KHZ (0<<8)
> #define GMBUS_RATE_50KHZ (1<<8)
> #define GMBUS_RATE_400KHZ (2<<8) /* reserved on Pineview */
> @@ -2133,7 +2133,7 @@ enum skl_disp_power_wells {
> #define GMBUS_PIN_2_BXT 2
> #define GMBUS_PIN_3_BXT 3
> #define GMBUS_NUM_PINS 7 /* including 0 */
> -#define GMBUS1 0x5104 /* command/status */
> +#define GMBUS1 (dev_priv->gpio_mmio_base + 0x5104) /* command/status */
> #define GMBUS_SW_CLR_INT (1<<31)
> #define GMBUS_SW_RDY (1<<30)
> #define GMBUS_ENT (1<<29) /* enable timeout */
> @@ -2147,7 +2147,7 @@ enum skl_disp_power_wells {
> #define GMBUS_SLAVE_ADDR_SHIFT 1
> #define GMBUS_SLAVE_READ (1<<0)
> #define GMBUS_SLAVE_WRITE (0<<0)
> -#define GMBUS2 0x5108 /* status */
> +#define GMBUS2 (dev_priv->gpio_mmio_base + 0x5108) /* status */
> #define GMBUS_INUSE (1<<15)
> #define GMBUS_HW_WAIT_PHASE (1<<14)
> #define GMBUS_STALL_TIMEOUT (1<<13)
> @@ -2155,14 +2155,14 @@ enum skl_disp_power_wells {
> #define GMBUS_HW_RDY (1<<11)
> #define GMBUS_SATOER (1<<10)
> #define GMBUS_ACTIVE (1<<9)
> -#define GMBUS3 0x510c /* data buffer bytes 3-0 */
> -#define GMBUS4 0x5110 /* interrupt mask (Pineview+) */
> +#define GMBUS3 (dev_priv->gpio_mmio_base + 0x510c) /* data buffer bytes 3-0 */
> +#define GMBUS4 (dev_priv->gpio_mmio_base + 0x5110) /* interrupt mask (Pineview+) */
> #define GMBUS_SLAVE_TIMEOUT_EN (1<<4)
> #define GMBUS_NAK_EN (1<<3)
> #define GMBUS_IDLE_EN (1<<2)
> #define GMBUS_HW_WAIT_EN (1<<1)
> #define GMBUS_HW_RDY_EN (1<<0)
> -#define GMBUS5 0x5120 /* byte index */
> +#define GMBUS5 (dev_priv->gpio_mmio_base + 0x5120) /* byte index */
> #define GMBUS_2BYTE_INDEX_EN (1<<31)
>
> /*
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index a64f26c..1369fc4 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -114,8 +114,8 @@ 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);
> + I915_WRITE(GMBUS0, 0);
> + I915_WRITE(GMBUS4, 0);
> }
>
> static void intel_i2c_quirk_set(struct drm_i915_private *dev_priv, bool enable)
> @@ -261,7 +261,6 @@ gmbus_wait_hw_status(struct drm_i915_private *dev_priv,
> u32 gmbus4_irq_en)
> {
> int i;
> - int reg_offset = dev_priv->gpio_mmio_base;
> u32 gmbus2 = 0;
> DEFINE_WAIT(wait);
>
> @@ -271,13 +270,13 @@ gmbus_wait_hw_status(struct drm_i915_private *dev_priv,
> /* Important: The hw handles only the first bit, so set only one! Since
> * we also need to check for NAKs besides the hw ready/idle signal, we
> * need to wake up periodically and check that ourselves. */
> - I915_WRITE(GMBUS4 + reg_offset, gmbus4_irq_en);
> + I915_WRITE(GMBUS4, gmbus4_irq_en);
>
> for (i = 0; i < msecs_to_jiffies_timeout(50); i++) {
> prepare_to_wait(&dev_priv->gmbus_wait_queue, &wait,
> TASK_UNINTERRUPTIBLE);
>
> - gmbus2 = I915_READ_NOTRACE(GMBUS2 + reg_offset);
> + gmbus2 = I915_READ_NOTRACE(GMBUS2);
> if (gmbus2 & (GMBUS_SATOER | gmbus2_status))
> break;
>
> @@ -285,7 +284,7 @@ gmbus_wait_hw_status(struct drm_i915_private *dev_priv,
> }
> finish_wait(&dev_priv->gmbus_wait_queue, &wait);
>
> - I915_WRITE(GMBUS4 + reg_offset, 0);
> + I915_WRITE(GMBUS4, 0);
>
> if (gmbus2 & GMBUS_SATOER)
> return -ENXIO;
> @@ -298,20 +297,19 @@ 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_NOTRACE(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0)
> +#define C ((I915_READ_NOTRACE(GMBUS2) & 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);
> + I915_WRITE(GMBUS4, GMBUS_IDLE_EN);
>
> ret = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> msecs_to_jiffies_timeout(10));
>
> - I915_WRITE(GMBUS4 + reg_offset, 0);
> + I915_WRITE(GMBUS4, 0);
>
> if (ret)
> return 0;
> @@ -325,9 +323,7 @@ gmbus_xfer_read_chunk(struct drm_i915_private *dev_priv,
> unsigned short addr, u8 *buf, unsigned int len,
> u32 gmbus1_index)
> {
> - int reg_offset = dev_priv->gpio_mmio_base;
> -
> - I915_WRITE(GMBUS1 + reg_offset,
> + I915_WRITE(GMBUS1,
> gmbus1_index |
> GMBUS_CYCLE_WAIT |
> (len << GMBUS_BYTE_COUNT_SHIFT) |
> @@ -342,7 +338,7 @@ gmbus_xfer_read_chunk(struct drm_i915_private *dev_priv,
> if (ret)
> return ret;
>
> - val = I915_READ(GMBUS3 + reg_offset);
> + val = I915_READ(GMBUS3);
> do {
> *buf++ = val & 0xff;
> val >>= 8;
> @@ -380,7 +376,6 @@ static int
> gmbus_xfer_write_chunk(struct drm_i915_private *dev_priv,
> unsigned short addr, u8 *buf, unsigned int len)
> {
> - int reg_offset = dev_priv->gpio_mmio_base;
> unsigned int chunk_size = len;
> u32 val, loop;
>
> @@ -390,8 +385,8 @@ gmbus_xfer_write_chunk(struct drm_i915_private *dev_priv,
> len -= 1;
> }
>
> - I915_WRITE(GMBUS3 + reg_offset, val);
> - I915_WRITE(GMBUS1 + reg_offset,
> + I915_WRITE(GMBUS3, val);
> + I915_WRITE(GMBUS1,
> GMBUS_CYCLE_WAIT |
> (chunk_size << GMBUS_BYTE_COUNT_SHIFT) |
> (addr << GMBUS_SLAVE_ADDR_SHIFT) |
> @@ -404,7 +399,7 @@ gmbus_xfer_write_chunk(struct drm_i915_private *dev_priv,
> val |= *buf++ << (8 * loop);
> } while (--len && ++loop < 4);
>
> - I915_WRITE(GMBUS3 + reg_offset, val);
> + I915_WRITE(GMBUS3, val);
>
> ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_RDY,
> GMBUS_HW_RDY_EN);
> @@ -452,7 +447,6 @@ gmbus_is_index_read(struct i2c_msg *msgs, int i, int num)
> 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;
> @@ -466,13 +460,13 @@ gmbus_xfer_index_read(struct drm_i915_private *dev_priv, struct i2c_msg *msgs)
>
> /* GMBUS5 holds 16-bit index */
> if (gmbus5)
> - I915_WRITE(GMBUS5 + reg_offset, gmbus5);
> + I915_WRITE(GMBUS5, 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);
> + I915_WRITE(GMBUS5, 0);
>
> return ret;
> }
> @@ -486,7 +480,7 @@ gmbus_xfer(struct i2c_adapter *adapter,
> struct intel_gmbus,
> adapter);
> struct drm_i915_private *dev_priv = bus->dev_priv;
> - int i = 0, inc, try = 0, reg_offset;
> + int i = 0, inc, try = 0;
> int ret = 0;
>
> intel_aux_display_runtime_get(dev_priv);
> @@ -497,10 +491,8 @@ gmbus_xfer(struct i2c_adapter *adapter,
> goto out;
> }
>
> - reg_offset = dev_priv->gpio_mmio_base;
> -
> retry:
> - I915_WRITE(GMBUS0 + reg_offset, bus->reg0);
> + I915_WRITE(GMBUS0, bus->reg0);
>
> for (; i < num; i += inc) {
> inc = 1;
> @@ -530,7 +522,7 @@ retry:
> * 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);
> + I915_WRITE(GMBUS1, 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,
> @@ -541,7 +533,7 @@ retry:
> adapter->name);
> ret = -ETIMEDOUT;
> }
> - I915_WRITE(GMBUS0 + reg_offset, 0);
> + I915_WRITE(GMBUS0, 0);
> ret = ret ?: i;
> goto out;
>
> @@ -570,9 +562,9 @@ clear_err:
> * 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);
> + I915_WRITE(GMBUS1, GMBUS_SW_CLR_INT);
> + I915_WRITE(GMBUS1, 0);
> + I915_WRITE(GMBUS0, 0);
>
> DRM_DEBUG_KMS("GMBUS [%s] NAK for addr: %04x %c(%d)\n",
> adapter->name, msgs[i].addr,
> @@ -595,7 +587,7 @@ clear_err:
> timeout:
> 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);
> + I915_WRITE(GMBUS0, 0);
>
> /* Hardware may not support GMBUS over these pins? Try GPIO bitbanging instead. */
> bus->force_bit = 1;
>

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>