Hi Sam,
Quoting Sam Ravnborg (2022-02-06 15:44:02)
> From: Rob Clark <robdclark@xxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxx>
> Signed-off-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
> Cc: Rob Clark <robdclark@xxxxxxxxxxxx>
> Cc: Douglas Anderson <dianders@xxxxxxxxxxxx>
> Cc: Andrzej Hajda <a.hajda@xxxxxxxxxxx>
> Cc: Neil Armstrong <narmstrong@xxxxxxxxxxxx>
> Cc: Robert Foss <robert.foss@xxxxxxxxxx>
> Cc: Laurent Pinchart <Laurent.pinchart@xxxxxxxxxxxxxxxx>
> Cc: Jonas Karlman <jonas@xxxxxxxxx>
> Cc: Jernej Skrabec <jernej.skrabec@xxxxxxxxx>
> ---
> 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
>
