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

[Kieran Bingham]

Hi Sam,

Quoting Sam Ravnborg (2022-02-06 15:44:02)
> From: Rob Clark <robdclark at 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 at chromium.org>
> Signed-off-by: Sam Ravnborg <sam at ravnborg.org>
> Cc: Rob Clark <robdclark at chromium.org>
> Cc: Douglas Anderson <dianders at chromium.org>
> Cc: Andrzej Hajda <a.hajda at samsung.com>
> Cc: Neil Armstrong <narmstrong at baylibre.com>
> Cc: Robert Foss <robert.foss at linaro.org>
> Cc: Laurent Pinchart <Laurent.pinchart at ideasonboard.com>
> Cc: Jonas Karlman <jonas at kwiboo.se>
> Cc: Jernej Skrabec <jernej.skrabec at 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
>