Re: [PATCH 03/24] drm/msm: create the display connectors from common code

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional calls to modeset_init functions with potentially NULL encoders causes kernel crash or probe failure.
--

--- Patch [3]: [PATCH 03/24] drm/msm: create the display connectors from common code ---
commit a0a4ea009e59812e9ecb6b60251e9294d0b779b8
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

drm/msm: create the display connectors from common code

This commit moves the bridge and connector setup logic from the individual
KMS backends into a common msm_kms_init_connectors() function. The backend
is now only responsible for creating the encoder, which is then used by the
common code.

> diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c
> index e5d0ea6294484..87530145e82b0 100644
> --- a/drivers/gpu/drm/msm/msm_kms.c
> +++ b/drivers/gpu/drm/msm/msm_kms.c
[ ... ]
> @@ -260,6 +260,57 @@ void msm_drm_kms_uninit(struct device *dev)
>  		kms->funcs->destroy(kms);
>  }
>  
> +/*
> + * Set up the bridges and connectors for the display sub-blocks, using the
> + * encoders the backend created in ->kms_init().
> + */
> +static int msm_kms_init_connectors(struct drm_device *ddev)
> +{
> +	struct msm_drm_private *priv = ddev->dev_private;
> +	struct msm_kms *kms = priv->kms;
> +	int i, ret;
[ ... ]
> +	if (kms->hdmi) {
> +		ret = msm_hdmi_modeset_init(kms->hdmi, ddev, kms->hdmi_encoder);

[Severity: High]
Will this cause a NULL pointer dereference if the encoder was not created?

If the device tree defines an HDMI node (so kms->hdmi is populated), but the
KMS backend's hardware configuration doesn't support the interface, encoder
creation is skipped and kms->hdmi_encoder remains NULL.

If msm_hdmi_modeset_init() receives a NULL encoder and there is no external
bridge, it eventually calls drm_bridge_connector_init():

drivers/gpu/drm/msm/hdmi/hdmi.c:msm_hdmi_modeset_init() {
    ...
    hdmi->connector = drm_bridge_connector_init(hdmi->dev, encoder);
    ...
}

This unconditionally dereferences the encoder to access the bridge chain:

drivers/gpu/drm/display/drm_bridge_connector.c:drm_bridge_connector_init() {
    ...
    drm_for_each_bridge_in_chain(encoder, bridge) {
    ...
}

Should there be a check ensuring the encoder was actually populated before
calling msm_hdmi_modeset_init() (and similarly for DP and DSI above this)?

> +		if (ret) {
> +			DRM_DEV_ERROR(ddev->dev,
> +				      "modeset_init failed for HDMI: %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}

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