| Message ID | 20210916102118.17356-1-shawn.c.lee@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/dsi: unregister gmbus if LFP display was MIPI panel | expand |
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. > > 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:
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? 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
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
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:
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. 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(+)