On Thu, 16 Sep 2021, "Lee, Shawn C" <shawn.c.lee@xxxxxxxxx> wrote: > On Thu, 16 Sep 2021, Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote: >>On Thu, 16 Sep 2021, Lee Shawn C <shawn.c.lee@xxxxxxxxx> 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@xxxxxxxxxxxxxxx> >>> Cc: Vandita Kulkarni <vandita.kulkarni@xxxxxxxxx> >>> Cc: Cooper Chiou <cooper.chiou@xxxxxxxxx> >>> Cc: William Tseng <william.tseng@xxxxxxxxx> >>> Signed-off-by: Lee Shawn C <shawn.c.lee@xxxxxxxxx> >>> --- >>> 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