Re: [PATCH 23/24] drm/msm: create the bridge connectors from common code

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Centralized `msm_kms_init_connectors` loop unconditionally calls `drm_bridge_connector_init` for all non-virtual encoders, failing the driver probe with `-EINVAL` if any encoder lacks an attached bridge.
--

--- Patch [23]: [PATCH 23/24] drm/msm: create the bridge connectors from common code ---
commit 04453784aafc591b434b0736383d9d31623d416b
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
drm/msm: create the bridge connectors from common code

This patch centralizes bridge connector creation in the msm backend by
having each sub-block only set up its bridge chain. A new function,
msm_kms_init_connectors(), then walks through all encoders to create the
connectors using common code.

> diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c
> index 1a081e15a0a89..71af3780b7b13 100644
> --- a/drivers/gpu/drm/msm/msm_kms.c
> +++ b/drivers/gpu/drm/msm/msm_kms.c
[ ... ]
> @@ -310,6 +313,35 @@ static int msm_kms_init_connectors(struct drm_device *ddev)
>  	return ret;
>  }
>  
> +/* Build a bridge connector for each encoder. */
> +static int msm_kms_init_connectors(struct drm_device *ddev)
> +{
> +	struct drm_encoder *encoder;
> +
> +	drm_for_each_encoder(encoder, ddev) {
> +		struct drm_connector *connector;
> +		int ret;
> +
> +		/* the virtual writeback encoder brings its own connector */
> +		if (encoder->encoder_type == DRM_MODE_ENCODER_VIRTUAL)
> +			continue;
> +
> +		connector = drm_bridge_connector_init(ddev, encoder);
> +		if (IS_ERR(connector)) {
> +			DRM_DEV_ERROR(ddev->dev,
> +				      "failed to init connector for %s: %ld\n",
> +				      encoder->name, PTR_ERR(connector));
> +			return PTR_ERR(connector);
> +		}

[Severity: High]
Will this fail the entire driver probe on hardware where some encoders are
left without an attached bridge?

For example, on MDP4 hardware, a TMDS encoder is registered unconditionally
in mdp4_modeset_init_intf(), but it will have an empty bridge chain if HDMI
is not present. Similarly, for bonded DSI, msm_dsi_modeset_init() skips
bridge creation for the slave DSI, leaving its encoder without a bridge.

When drm_bridge_connector_init() processes an encoder with an empty bridge
chain, it returns ERR_PTR(-EINVAL) because it cannot determine the connector
type. Since this loop treats any error as fatal, won't this break
display initialization on these configurations?

> +
> +		/* The DP subconnector property applies to DisplayPort only. */
> +		if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort)
> +			drm_connector_attach_dp_subconnector_property(connector);
> +	}
> +
> +	return 0;
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722-drm-msm-display-interface-v1-0-368c10fe62fd@oss.qualcomm.com?part=23