On Fri, Mar 27, 2015 at 12:20:21AM +0200, Jani Nikula wrote:
> Index the gmbus tables directly using the pin instead of having a
> confusing "port = i + 1" mapping. This finishes off removing the "gmbus
> port" as a notion, and leaves us with just the "gmbus pin".
>
> As pin 0 is invalid by definition and the gmbus tables will have a gap
> at that index, add pin validity check to all the loops. This will be
> benefitial for supporting platforms that have different numbers of pins,
> or gaps.
>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +-
> drivers/gpu/drm/i915/i915_reg.h | 2 +-
> drivers/gpu/drm/i915/intel_i2c.c | 65 +++++++++++++++++++++++-----------------
> 3 files changed, 40 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3ba5de19e039..0c6024101eb9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1574,8 +1574,7 @@ struct drm_i915_private {
>
> struct i915_virtual_gpu vgpu;
>
> - struct intel_gmbus gmbus[GMBUS_NUM_PORTS];
> -
> + struct intel_gmbus gmbus[GMBUS_PIN_MAX];
>
> /** gmbus_mutex protects against concurrent usage of the single hw gmbus
> * controller on different i2c buses. */
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b6113c9a803b..cdc071cff001 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1797,7 +1797,7 @@ enum skl_disp_power_wells {
> #define GMBUS_PIN_DPB 5 /* SDVO, HDMIB */
> #define GMBUS_PIN_DPD 6 /* HDMID */
> #define GMBUS_PIN_RESERVED 7 /* 7 reserved */
> -#define GMBUS_NUM_PORTS (GMBUS_PIN_DPD - GMBUS_PIN_SSC + 1)
> +#define GMBUS_PIN_MAX 7 /* not inclusive */
PIN_MAX makes me think it's inclusive. NUM_PINS maybe?
w/ or w/o that particular bikeshed patches 1-4 look fine to me, so:
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> #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 b0003a2bd854..ff47a8fdcb6d 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -34,18 +34,19 @@
> #include <drm/i915_drm.h>
> #include "i915_drv.h"
>
> -struct gmbus_port {
> +struct gmbus_pin {
> const char *name;
> int reg;
> };
>
> -static const struct gmbus_port gmbus_ports[] = {
> - { "ssc", GPIOB },
> - { "vga", GPIOA },
> - { "panel", GPIOC },
> - { "dpc", GPIOD },
> - { "dpb", GPIOE },
> - { "dpd", GPIOF },
> +/* Map gmbus pin pairs to names and registers. */
> +static const struct gmbus_pin gmbus_pins[] = {
> + [GMBUS_PIN_SSC] = { "ssc", GPIOB },
> + [GMBUS_PIN_VGADDC] = { "vga", GPIOA },
> + [GMBUS_PIN_PANEL] = { "panel", GPIOC },
> + [GMBUS_PIN_DPC] = { "dpc", GPIOD },
> + [GMBUS_PIN_DPB] = { "dpb", GPIOE },
> + [GMBUS_PIN_DPD] = { "dpd", GPIOF },
> };
>
> /* Intel GPIO access functions */
> @@ -182,15 +183,14 @@ intel_gpio_post_xfer(struct i2c_adapter *adapter)
> }
>
> static void
> -intel_gpio_setup(struct intel_gmbus *bus, u32 pin)
> +intel_gpio_setup(struct intel_gmbus *bus, unsigned int pin)
> {
> struct drm_i915_private *dev_priv = bus->dev_priv;
> struct i2c_algo_bit_data *algo;
>
> algo = &bus->bit_algo;
>
> - /* -1 to map pin pair to gmbus index */
> - bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_ports[pin - 1].reg;
> + bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_pins[pin].reg;
>
> bus->adapter.algo_data = algo;
> algo->setsda = set_data;
> @@ -517,7 +517,9 @@ static const struct i2c_algorithm gmbus_algorithm = {
> int intel_setup_gmbus(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> - int ret, i;
> + struct intel_gmbus *bus;
> + unsigned int pin;
> + int ret;
>
> if (HAS_PCH_NOP(dev))
> return 0;
> @@ -531,16 +533,18 @@ int intel_setup_gmbus(struct drm_device *dev)
> 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];
> - u32 port = i + 1; /* +1 to map gmbus index to pin pair */
> + for (pin = 0; pin < ARRAY_SIZE(dev_priv->gmbus); pin++) {
> + if (!intel_gmbus_is_valid_pin(pin))
> + continue;
> +
> + bus = &dev_priv->gmbus[pin];
>
> bus->adapter.owner = THIS_MODULE;
> bus->adapter.class = I2C_CLASS_DDC;
> snprintf(bus->adapter.name,
> sizeof(bus->adapter.name),
> "i915 gmbus %s",
> - gmbus_ports[i].name);
> + gmbus_pins[pin].name);
>
> bus->adapter.dev.parent = &dev->pdev->dev;
> bus->dev_priv = dev_priv;
> @@ -548,13 +552,13 @@ int intel_setup_gmbus(struct drm_device *dev)
> bus->adapter.algo = &gmbus_algorithm;
>
> /* By default use a conservative clock rate */
> - bus->reg0 = port | GMBUS_RATE_100KHZ;
> + bus->reg0 = pin | GMBUS_RATE_100KHZ;
>
> /* gmbus seems to be broken on i830 */
> if (IS_I830(dev))
> bus->force_bit = 1;
>
> - intel_gpio_setup(bus, port);
> + intel_gpio_setup(bus, pin);
>
> ret = i2c_add_adapter(&bus->adapter);
> if (ret)
> @@ -566,8 +570,11 @@ int intel_setup_gmbus(struct drm_device *dev)
> return 0;
>
> err:
> - while (--i) {
> - struct intel_gmbus *bus = &dev_priv->gmbus[i];
> + while (--pin) {
> + if (!intel_gmbus_is_valid_pin(pin))
> + continue;
> +
> + bus = &dev_priv->gmbus[pin];
> i2c_del_adapter(&bus->adapter);
> }
> return ret;
> @@ -576,10 +583,10 @@ err:
> struct i2c_adapter *intel_gmbus_get_adapter(struct drm_i915_private *dev_priv,
> unsigned int pin)
> {
> - WARN_ON(!intel_gmbus_is_valid_pin(pin));
> - /* -1 to map pin pair to gmbus index */
> - return (intel_gmbus_is_valid_pin(pin)) ?
> - &dev_priv->gmbus[pin - 1].adapter : NULL;
> + if (WARN_ON(!intel_gmbus_is_valid_pin(pin)))
> + return NULL;
> +
> + return &dev_priv->gmbus[pin].adapter;
> }
>
> void intel_gmbus_set_speed(struct i2c_adapter *adapter, int speed)
> @@ -602,10 +609,14 @@ void intel_gmbus_force_bit(struct i2c_adapter *adapter, bool force_bit)
> void intel_teardown_gmbus(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> - int i;
> + struct intel_gmbus *bus;
> + unsigned int pin;
> +
> + for (pin = 0; pin < ARRAY_SIZE(dev_priv->gmbus); pin++) {
> + if (!intel_gmbus_is_valid_pin(pin))
> + continue;
>
> - for (i = 0; i < GMBUS_NUM_PORTS; i++) {
> - struct intel_gmbus *bus = &dev_priv->gmbus[i];
> + bus = &dev_priv->gmbus[pin];
> i2c_del_adapter(&bus->adapter);
> }
> }
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
