On Tue, 17 Mar 2015, Imre Deak <imre.deak@xxxxxxxxx> wrote:
> From: "A.Sunil Kamath" <sunil.kamath@xxxxxxxxx>
>
> This patch will WARN if unused gmbus ports gets accessed for
> BXT using gmbus_get_adapter also ensure that only valid ports
> of BXT gets used. For BXT its more important to do this as it
> has only 3 valid ports and structure has empty content otherwise.
>
> Because of additonal IS_BROXTON check an additional "dev"
> argument is added to intel_gmbus_is_port_valid. Also added
> related changes in other places from where this function is accessed.
>
> v1: This WARN patch is added as per review comments from
> Daniel Vetter on gmbus BXT patch
>
> v2: Changed get_adapter to have only one is_port_valid call
> according to review comments from M Satheeshakrishna.
>
> v3: Early bail out on errors according to review comments from
> Daniel Vetter.
>
> Issue: VIZ-3574
> Signed-off-by: A.Sunil Kamath <sunil.kamath@xxxxxxxxx>
> Signed-off-by: Damien Lespiau <damien.lespiau@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 8 ++++++--
> drivers/gpu/drm/i915/intel_bios.c | 3 ++-
> drivers/gpu/drm/i915/intel_dvo.c | 2 +-
> drivers/gpu/drm/i915/intel_i2c.c | 9 ++++++---
> drivers/gpu/drm/i915/intel_lvds.c | 2 +-
> drivers/gpu/drm/i915/intel_sdvo.c | 4 +++-
> 6 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8fb7cc0..52e5f18 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3037,9 +3037,13 @@ 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_port_valid(unsigned port)
> +static inline bool
> +intel_gmbus_is_port_valid(struct drm_device *dev, unsigned port)
It's probably better to pass dev_priv here, and fix all the call
sites. Removes a bunch of extra local vars down there.
With this fixed,
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> {
> - return (port >= GMBUS_PORT_SSC && port <= GMBUS_PORT_DPD);
> + if (IS_BROXTON(dev))
> + return port >= GMBUS_PORT_DPC && port <= GMBUS_PORT_DPD;
It's seriously confusing that _DPB is within that range... but not a
problem in this patch per se.
> + else
> + return port >= GMBUS_PORT_SSC && port <= GMBUS_PORT_DPD;
> }
>
> extern struct i2c_adapter *intel_gmbus_get_adapter(
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index c684085..e423cc8 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -431,6 +431,7 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
> struct bdb_header *bdb)
> {
> struct bdb_general_definitions *general;
> + struct drm_device *dev = dev_priv->dev;
>
> general = find_section(bdb, BDB_GENERAL_DEFINITIONS);
> if (general) {
> @@ -438,7 +439,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_port_valid(bus_pin))
> + if (intel_gmbus_is_port_valid(dev, 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 d857951..27d5b9a 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -499,7 +499,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_port_valid(dvo->gpio))
> + if (intel_gmbus_is_port_valid(dev, dvo->gpio))
> gpio = dvo->gpio;
> else if (dvo->type == INTEL_DVO_CHIP_LVDS)
> gpio = GMBUS_PORT_SSC;
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index 3aa31e1..06892b5 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -619,10 +619,13 @@ err:
> struct i2c_adapter *intel_gmbus_get_adapter(struct drm_i915_private *dev_priv,
> unsigned port)
> {
> - WARN_ON(!intel_gmbus_is_port_valid(port));
> + struct drm_device *dev = dev_priv->dev;
> +
> + if (WARN_ON(!intel_gmbus_is_port_valid(dev, port)))
> + return NULL;
> +
> /* -1 to map pin pair to gmbus index */
> - return (intel_gmbus_is_port_valid(port)) ?
> - &dev_priv->gmbus[port - 1].adapter : NULL;
> + return &dev_priv->gmbus[port - 1].adapter;
> }
>
> void intel_gmbus_set_speed(struct i2c_adapter *adapter, int speed)
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 24e8730..094b88e 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -780,7 +780,7 @@ static bool lvds_is_present_in_vbt(struct drm_device *dev,
> child->device_type != DEVICE_TYPE_LFP)
> continue;
>
> - if (intel_gmbus_is_port_valid(child->i2c_pin))
> + if (intel_gmbus_is_port_valid(dev, 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 9e554c2..d68936e 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -2283,6 +2283,7 @@ intel_sdvo_select_i2c_bus(struct drm_i915_private *dev_priv,
> struct intel_sdvo *sdvo, u32 reg)
> {
> struct sdvo_device_mapping *mapping;
> + struct drm_device *dev = dev_priv->dev;
> u8 pin;
>
> if (sdvo->is_sdvob)
> @@ -2290,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_port_valid(mapping->i2c_pin))
> + if (mapping->initialized &&
> + intel_gmbus_is_port_valid(dev, mapping->i2c_pin))
> pin = mapping->i2c_pin;
> else
> pin = GMBUS_PORT_DPB;
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
