Re: [PATCH 4/5] drm/i915: base gmbus pin validity check on the gmbus pin map array

Daniel Vetter
2015-04-01T12:10:29Z

On Fri, Mar 27, 2015 at 12:20:22AM +0200, Jani Nikula wrote:
> This will be helpful for adding future platforms. It is better to keep
> the information in the single point of truth (the table) instead of
> duplicating it into the validity function.
>
> While at it, add dev_priv parameter to the function, also to prepare for
> adding future platform support.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Merged up to this patch, thanks.
-Daniel

> ---
> drivers/gpu/drm/i915/i915_drv.h | 6 ++----
> drivers/gpu/drm/i915/intel_bios.c | 2 +-
> drivers/gpu/drm/i915/intel_dvo.c | 2 +-
> drivers/gpu/drm/i915/intel_i2c.c | 14 ++++++++++----
> drivers/gpu/drm/i915/intel_lvds.c | 2 +-
> drivers/gpu/drm/i915/intel_sdvo.c | 3 ++-
> 6 files changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 0c6024101eb9..a1ba9bec8a6b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3048,10 +3048,8 @@ void i915_teardown_sysfs(struct drm_device *dev_priv);
> /* intel_i2c.c */
> extern int intel_setup_gmbus(struct drm_device *dev);
> extern void intel_teardown_gmbus(struct drm_device *dev);
> -static inline bool intel_gmbus_is_valid_pin(unsigned int pin)
> -{
> - return (pin >= GMBUS_PIN_SSC && pin <= GMBUS_PIN_DPD);
> -}
> +extern bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
> + unsigned int pin);
>
> extern struct i2c_adapter *
> intel_gmbus_get_adapter(struct drm_i915_private *dev_priv, unsigned int pin);
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 333f40793435..ad2f3b0d922f 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -438,7 +438,7 @@ 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 (intel_gmbus_is_valid_pin(bus_pin))
> + if (intel_gmbus_is_valid_pin(dev_priv, bus_pin))
> dev_priv->vbt.crt_ddc_pin = bus_pin;
> } else {
> DRM_DEBUG_KMS("BDB_GD too small (%d). Invalid.\n",
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 8d62272a3421..9670e3802939 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -500,7 +500,7 @@ void intel_dvo_init(struct drm_device *dev)
> * special cases, but otherwise default to what's defined
> * in the spec.
> */
> - if (intel_gmbus_is_valid_pin(dvo->gpio))
> + if (intel_gmbus_is_valid_pin(dev_priv, dvo->gpio))
> gpio = dvo->gpio;
> else if (dvo->type == INTEL_DVO_CHIP_LVDS)
> gpio = GMBUS_PIN_SSC;
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index ff47a8fdcb6d..ec9cc8cf642e 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -49,6 +49,12 @@ static const struct gmbus_pin gmbus_pins[] = {
> [GMBUS_PIN_DPD] = { "dpd", GPIOF },
> };
>
> +bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
> + unsigned int pin)
> +{
> + return pin < ARRAY_SIZE(gmbus_pins) && gmbus_pins[pin].reg;
> +}
> +
> /* Intel GPIO access functions */
>
> #define I2C_RISEFALL_TIME 10
> @@ -534,7 +540,7 @@ int intel_setup_gmbus(struct drm_device *dev)
> init_waitqueue_head(&dev_priv->gmbus_wait_queue);
>
> for (pin = 0; pin < ARRAY_SIZE(dev_priv->gmbus); pin++) {
> - if (!intel_gmbus_is_valid_pin(pin))
> + if (!intel_gmbus_is_valid_pin(dev_priv, pin))
> continue;
>
> bus = &dev_priv->gmbus[pin];
> @@ -571,7 +577,7 @@ int intel_setup_gmbus(struct drm_device *dev)
>
> err:
> while (--pin) {
> - if (!intel_gmbus_is_valid_pin(pin))
> + if (!intel_gmbus_is_valid_pin(dev_priv, pin))
> continue;
>
> bus = &dev_priv->gmbus[pin];
> @@ -583,7 +589,7 @@ err:
> struct i2c_adapter *intel_gmbus_get_adapter(struct drm_i915_private *dev_priv,
> unsigned int pin)
> {
> - if (WARN_ON(!intel_gmbus_is_valid_pin(pin)))
> + if (WARN_ON(!intel_gmbus_is_valid_pin(dev_priv, pin)))
> return NULL;
>
> return &dev_priv->gmbus[pin].adapter;
> @@ -613,7 +619,7 @@ void intel_teardown_gmbus(struct drm_device *dev)
> unsigned int pin;
>
> for (pin = 0; pin < ARRAY_SIZE(dev_priv->gmbus); pin++) {
> - if (!intel_gmbus_is_valid_pin(pin))
> + if (!intel_gmbus_is_valid_pin(dev_priv, pin))
> continue;
>
> bus = &dev_priv->gmbus[pin];
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index d61aa78ed7e3..314a5d56ace2 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -781,7 +781,7 @@ static bool lvds_is_present_in_vbt(struct drm_device *dev,
> child->device_type != DEVICE_TYPE_LFP)
> continue;
>
> - if (intel_gmbus_is_valid_pin(child->i2c_pin))
> + if (intel_gmbus_is_valid_pin(dev_priv, child->i2c_pin))
> *i2c_pin = child->i2c_pin;
>
> /* However, we cannot trust the BIOS writers to populate
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 124992e48abd..b121796c86aa 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -2291,7 +2291,8 @@ intel_sdvo_select_i2c_bus(struct drm_i915_private *dev_priv,
> else
> mapping = &dev_priv->sdvo_mappings[1];
>
> - if (mapping->initialized && intel_gmbus_is_valid_pin(mapping->i2c_pin))
> + if (mapping->initialized &&
> + intel_gmbus_is_valid_pin(dev_priv, mapping->i2c_pin))
> pin = mapping->i2c_pin;
> else
> pin = GMBUS_PIN_DPB;
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation