Re: [PATCH v1 6/9] drm/bridge: ti-sn65dsi86: Add NO_CONNECTOR support

Hi Sam,

Quoting Sam Ravnborg (2022-02-06 15:44:02)

From: Rob Clark robdclark@chromium.org

Slightly awkward to fish out the display_info when we aren't creating own connector. But I don't see an obvious better way.

v3:

• Rebased and dropped the ti_sn_bridge_get_bpp() patch as this was solved in a different way (Sam)

v2:

• Remove error return with NO_CONNECTOR flag (Rob)

Signed-off-by: Rob Clark robdclark@chromium.org Signed-off-by: Sam Ravnborg sam@ravnborg.org Cc: Rob Clark robdclark@chromium.org Cc: Douglas Anderson dianders@chromium.org Cc: Andrzej Hajda a.hajda@samsung.com Cc: Neil Armstrong narmstrong@baylibre.com Cc: Robert Foss robert.foss@linaro.org Cc: Laurent Pinchart Laurent.pinchart@ideasonboard.com Cc: Jonas Karlman jonas@kwiboo.se Cc: Jernej Skrabec jernej.skrabec@gmail.com

---

drivers/gpu/drm/bridge/ti-sn65dsi86.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c index dc6ec40bc1ef..a9041dfd2ae5 100644 --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c @@ -746,11 +746,6 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge, struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge); int ret;

•   if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
  

•           DRM_ERROR("Fix bridge driver to make connector optional!");
  

•           return -EINVAL;
  

•   }
  

•   pdata->aux.drm_dev = bridge->dev;
    ret = drm_dp_aux_register(&pdata->aux);
    if (ret < 0) {
  

@@ -758,12 +753,14 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge, return ret; }

•   ret = ti_sn_bridge_connector_init(pdata);
  

•   if (ret < 0)
  

•           goto err_conn_init;
  

•   if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
  

•           ret = ti_sn_bridge_connector_init(pdata);
  

•           if (ret < 0)
  

•                   goto err_conn_init;
  
  

•   /* We never want the next bridge to *also* create a connector: */
  

•   flags |= DRM_BRIDGE_ATTACH_NO_CONNECTOR;
  

•           /* We never want the next bridge to *also* create a connector: */
  

•           flags |= DRM_BRIDGE_ATTACH_NO_CONNECTOR;
  

•   }
  
    /* Attach the next bridge */
    ret = drm_bridge_attach(bridge->encoder, pdata->next_bridge,
  

@@ -774,7 +771,8 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge, return 0;

err_dsi_host:

•   drm_connector_cleanup(&pdata->connector);
  

•   if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR))
  

I think this is unreachable / always false due to the flags |= DRM_BRIDGE_ATTACH_NO_CONNECTOR.

I've solved this locally by doing:

- /* Attach the next bridge */ + /* + * Attach the next bridge We never want the next bridge to *also* create + * a connector: + */ ret = drm_bridge_attach(bridge->encoder, pdata->next_bridge, - &pdata->bridge, flags); + &pdata->bridge, + flags | DRM_BRIDGE_ATTACH_NO_CONNECTOR); if (ret < 0) goto err_initted_aux;

+ if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) + return 0; + pdata->connector = drm_bridge_connector_init(pdata->bridge.dev, pdata->bridge.encoder); if (IS_ERR(pdata->connector)) { ret = PTR_ERR(pdata->connector); goto err_initted_aux; }

drm_connector_attach_encoder(pdata->connector, pdata->bridge.encoder);

return 0;

Which also fixes the support for flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR

-- Regards

Kieran

•           drm_connector_cleanup(&pdata->connector);
  

err_conn_init: drm_dp_aux_unregister(&pdata->aux); return ret; -- 2.32.0