| Message ID | 20210811235253.924867-3-robdclark@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| 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)
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? 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
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)