Re: [Intel-gfx] [PATCH] drm/i915/dsi: do not register gmbus if it was reserved for MIPI display

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 17 Sep 2021, Lee Shawn C <shawn.c.lee@intel.com> wrote:
> Gmbus driver would setup all Intel i2c GMBuses. But DDC bus
> may configured as gpio and reserved for MIPI driver to control
> panel power on/off sequence.
>
> Using i2c tool to communicate to peripherals via i2c interface
> reversed for gmbus(DDC). There will be some high/low pulse
> appear on DDC SCL and SDA (might be host sent out i2c slave
> address). MIPI panel would be impacted due to unexpected signal
> then caused abnormal display or shut down issue.
>
> v2: gmbus driver should not add i2c adapter for DDC interface
>     if LFP display was configured to support MIPI panel.
> v3: fix sparse warning
>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Vandita Kulkarni <vandita.kulkarni@intel.com>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
> Cc: William Tseng <william.tseng@intel.com>
> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_gmbus.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c
> index ceb1bf8a8c3c..51d2b6bf2ed2 100644
> --- a/drivers/gpu/drm/i915/display/intel_gmbus.c
> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
> @@ -141,6 +141,21 @@ bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
>  	return pin < size && get_gmbus_pin(dev_priv, pin)->name;
>  }
>  
> +static bool intel_gmbus_ddc_reserve_for_mipi(struct drm_i915_private *dev_priv,
> +					     unsigned int pin)
> +{
> +	if (intel_bios_is_dsi_present(dev_priv, NULL)) {
> +		if (DISPLAY_VER(dev_priv) >= 11) {
> +			if ((pin == GMBUS_PIN_2_BXT && dev_priv->vbt.dsi.config->dual_link) ||
> +			     pin == GMBUS_PIN_1_BXT) {
> +				return true;
> +			}
> +		}
> +	}
> +
> +	return false;
> +}
> +
>  /* Intel GPIO access functions */
>  
>  #define I2C_RISEFALL_TIME 10
> @@ -859,7 +874,8 @@ int intel_gmbus_setup(struct drm_i915_private *dev_priv)
>  	init_waitqueue_head(&dev_priv->gmbus_wait_queue);
>  
>  	for (pin = 0; pin < ARRAY_SIZE(dev_priv->gmbus); pin++) {
> -		if (!intel_gmbus_is_valid_pin(dev_priv, pin))
> +		if (!intel_gmbus_is_valid_pin(dev_priv, pin) ||
> +		     intel_gmbus_ddc_reserve_for_mipi(dev_priv, pin))
>  			continue;
>  
>  		bus = &dev_priv->gmbus[pin];

This does not prevent the pin from being used for e.g. HDMI, and things
are probably going to go awfully wrong with intel_gmbus_get_adapter()
when the adapter hasn't been registered. In that sense, this is no
different from v1.

Sure, the VBT probably shouldn't do that, but that's not an excuse for
us to not take it into account.

Cc: Ville in case he has some clever ideas off the top of his head. I
know I'd have to spend time I don't have to figure this out.


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center