| Message ID | 20210811235253.924867-3-robdclark@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | drm/msm+ti-sn65dsi86: Fix NO_CONNECTOR fallout | expand |
Hi Rob Thank you for the patch. On Wed, Aug 11, 2021 at 04:52:48PM -0700, Rob Clark wrote: > From: Rob Clark <robdclark@chromium.org> > > For now, since we have a mix of bridges which support this flag, which > which do *not* support this flag, or work both ways, try it once with > NO_CONNECTOR and then fall back to the old way if that doesn't work. > Eventually we can drop the fallback path. > > Signed-off-by: Rob Clark <robdclark@chromium.org> > --- > drivers/gpu/drm/msm/Kconfig | 2 ++ > drivers/gpu/drm/msm/dsi/dsi_manager.c | 41 ++++++++++++++++++--------- > 2 files changed, 30 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig > index e9c6af78b1d7..36e5ba3ccc28 100644 > --- a/drivers/gpu/drm/msm/Kconfig > +++ b/drivers/gpu/drm/msm/Kconfig > @@ -14,6 +14,8 @@ config DRM_MSM > select REGULATOR > select DRM_KMS_HELPER > select DRM_PANEL > + select DRM_BRIDGE > + select DRM_PANEL_BRIDGE > select DRM_SCHED > select SHMEM > select TMPFS > diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c > index c41d39f5b7cf..1fd1cf93abbf 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c > @@ -3,6 +3,8 @@ > * Copyright (c) 2015, The Linux Foundation. All rights reserved. > */ > > +#include "drm/drm_bridge_connector.h" > + > #include "msm_kms.h" > #include "dsi.h" > > @@ -690,8 +692,7 @@ struct drm_connector *msm_dsi_manager_ext_bridge_init(u8 id) > struct drm_device *dev = msm_dsi->dev; > struct drm_encoder *encoder; > struct drm_bridge *int_bridge, *ext_bridge; > - struct drm_connector *connector; > - struct list_head *connector_list; > + int ret; > > int_bridge = msm_dsi->bridge; > ext_bridge = msm_dsi->external_bridge = > @@ -699,22 +700,36 @@ struct drm_connector *msm_dsi_manager_ext_bridge_init(u8 id) > > encoder = msm_dsi->encoder; > > - /* link the internal dsi bridge to the external bridge */ > - drm_bridge_attach(encoder, ext_bridge, int_bridge, 0); > - > /* > - * we need the drm_connector created by the external bridge > - * driver (or someone else) to feed it to our driver's > - * priv->connector[] list, mainly for msm_fbdev_init() > + * Try first to create the bridge without it creating it's own s/it's/its/ > + * connector.. currently some bridges support this, and others > + * do not (and some support both modes) > */ > - connector_list = &dev->mode_config.connector_list; > + ret = drm_bridge_attach(encoder, ext_bridge, int_bridge, > + DRM_BRIDGE_ATTACH_NO_CONNECTOR); Should all this be moved one layer up, to the code that attaches to the mem_dsi->bridge ? I suppose we can start here, but as part of a global move to bridges and DRM_BRIDGE_ATTACH_NO_CONNECTOR, I think the top-level would make more sense in the long term. If you want to start here, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + if (ret == -EINVAL) { > + struct drm_connector *connector; > + struct list_head *connector_list; > + > + /* link the internal dsi bridge to the external bridge */ > + drm_bridge_attach(encoder, ext_bridge, int_bridge, 0); > + > + /* > + * we need the drm_connector created by the external bridge > + * driver (or someone else) to feed it to our driver's > + * priv->connector[] list, mainly for msm_fbdev_init() > + */ > + connector_list = &dev->mode_config.connector_list; > + > + list_for_each_entry(connector, connector_list, head) { > + if (drm_connector_has_possible_encoder(connector, encoder)) > + return connector; > + } > > - list_for_each_entry(connector, connector_list, head) { > - if (drm_connector_has_possible_encoder(connector, encoder)) > - return connector; > + return ERR_PTR(-ENODEV); > } > > - return ERR_PTR(-ENODEV); > + return drm_bridge_connector_init(dev, encoder); > } > > void msm_dsi_manager_bridge_destroy(struct drm_bridge *bridge) -- Regards, Laurent Pinchart
On Thu, Aug 12, 2021 at 10:31 AM Sam Ravnborg <sam@ravnborg.org> wrote: > > Hi Rob, > > On Wed, Aug 11, 2021 at 04:52:48PM -0700, Rob Clark wrote: > > From: Rob Clark <robdclark@chromium.org> > > > > For now, since we have a mix of bridges which support this flag, which > > which do *not* support this flag, or work both ways, try it once with > > NO_CONNECTOR and then fall back to the old way if that doesn't work. > > Eventually we can drop the fallback path. > > Which bridges are missing support for the NO_CONNECTOR flags that you > are using? Currently (as far as things that I am aware of) it is just ti-sn65dsi86, which the last two patches in this series convert. But who knows what someone somewhere decides to build ;-) BR, -R > Background for the question is that if you are using one of the bridges > missing a conversion then we could do the conversion and maybe you could > help with some tests. > > We in the above sentence is likely me, so it may take some weeks, > but known there is a chance for testing is a good motivator. > > Sam
diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig index e9c6af78b1d7..36e5ba3ccc28 100644 --- a/drivers/gpu/drm/msm/Kconfig +++ b/drivers/gpu/drm/msm/Kconfig @@ -14,6 +14,8 @@ config DRM_MSM select REGULATOR select DRM_KMS_HELPER select DRM_PANEL + select DRM_BRIDGE + select DRM_PANEL_BRIDGE select DRM_SCHED select SHMEM select TMPFS diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c index c41d39f5b7cf..1fd1cf93abbf 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c @@ -3,6 +3,8 @@ * Copyright (c) 2015, The Linux Foundation. All rights reserved. */ +#include "drm/drm_bridge_connector.h" + #include "msm_kms.h" #include "dsi.h" @@ -690,8 +692,7 @@ struct drm_connector *msm_dsi_manager_ext_bridge_init(u8 id) struct drm_device *dev = msm_dsi->dev; struct drm_encoder *encoder; struct drm_bridge *int_bridge, *ext_bridge; - struct drm_connector *connector; - struct list_head *connector_list; + int ret; int_bridge = msm_dsi->bridge; ext_bridge = msm_dsi->external_bridge = @@ -699,22 +700,36 @@ struct drm_connector *msm_dsi_manager_ext_bridge_init(u8 id) encoder = msm_dsi->encoder; - /* link the internal dsi bridge to the external bridge */ - drm_bridge_attach(encoder, ext_bridge, int_bridge, 0); - /* - * we need the drm_connector created by the external bridge - * driver (or someone else) to feed it to our driver's - * priv->connector[] list, mainly for msm_fbdev_init() + * Try first to create the bridge without it creating it's own + * connector.. currently some bridges support this, and others + * do not (and some support both modes) */ - connector_list = &dev->mode_config.connector_list; + ret = drm_bridge_attach(encoder, ext_bridge, int_bridge, + DRM_BRIDGE_ATTACH_NO_CONNECTOR); + if (ret == -EINVAL) { + struct drm_connector *connector; + struct list_head *connector_list; + + /* link the internal dsi bridge to the external bridge */ + drm_bridge_attach(encoder, ext_bridge, int_bridge, 0); + + /* + * we need the drm_connector created by the external bridge + * driver (or someone else) to feed it to our driver's + * priv->connector[] list, mainly for msm_fbdev_init() + */ + connector_list = &dev->mode_config.connector_list; + + list_for_each_entry(connector, connector_list, head) { + if (drm_connector_has_possible_encoder(connector, encoder)) + return connector; + } - list_for_each_entry(connector, connector_list, head) { - if (drm_connector_has_possible_encoder(connector, encoder)) - return connector; + return ERR_PTR(-ENODEV); } - return ERR_PTR(-ENODEV); + return drm_bridge_connector_init(dev, encoder); } void msm_dsi_manager_bridge_destroy(struct drm_bridge *bridge)