Re: [Intel-gfx] [PATCH] drm/i915/dsi: unregister gmbus if LFP display was MIPI panel

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

On Thu, 16 Sep 2021, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
> On Thu, 16 Sep 2021, Jani Nikula <jani.nikula@linux.intel.com> wrote:
>>On Thu, 16 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.
>>
>>Just a quick reply:
>>
>>So I don't know off the bat what the right solution is, but it's very obvious to me that we absolute can't go deleting gmbus adapters from DSI code.
>>
>>
>>BR,
>>Jani.
>>
>
> Create a new function in gmbus driver that allow to remove a given gmbus adapter. And DSI driver used it to unregister particular gmbus.
> It looks to me more reasonable for DSI and gmbus driver. What do you think?

No, I don't think that's good enough either. We probably need to figure
this out up front in intel_bios.c. You see, if a non-DSI port is already
using the pin, you'll end up removing it under it. So I think you'd need
to prevent the gmbus pin from being registered and the non-DSI port
being used.

BR,
Jani.

>
> Best regards,
> Shawn
>
>>>
>>> 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/icl_dsi.c | 14 ++++++++++++++
>>>  1 file changed, 14 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c 
>>> b/drivers/gpu/drm/i915/display/icl_dsi.c
>>> index 060bc8fb0d30..d2504e291fcb 100644
>>> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
>>> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
>>> @@ -1999,6 +1999,7 @@ void icl_dsi_init(struct drm_i915_private *dev_priv)
>>>  	struct intel_connector *intel_connector;
>>>  	struct drm_connector *connector;
>>>  	struct drm_display_mode *fixed_mode;
>>> +	struct intel_gmbus *bus;
>>>  	enum port port;
>>>  
>>>  	if (!intel_bios_is_dsi_present(dev_priv, &port)) @@ -2092,6 +2093,19 
>>> @@ void icl_dsi_init(struct drm_i915_private *dev_priv)
>>>  	icl_dphy_param_init(intel_dsi);
>>>  
>>>  	icl_dsi_add_properties(intel_connector);
>>> +
>>> +	/*
>>> +	 * DDC bus may configured as gpio and reserved for MIPI driver
>>> +	 * to control panel power on/off sequence. so, unregister gmbus
>>> +	 * if MIPI was LFP display.
>>> +	 */
>>> +	bus = &dev_priv->gmbus[GMBUS_PIN_1_BXT];
>>> +	i2c_del_adapter(&bus->adapter);
>>> +
>>> +	if (dev_priv->vbt.dsi.config->dual_link) {
>>> +		bus = &dev_priv->gmbus[GMBUS_PIN_2_BXT];
>>> +		i2c_del_adapter(&bus->adapter);
>>> +	}
>>>  	return;
>>>  
>>>  err:
>>
>>--
>>Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center