Re: [PATCH v6 17/29] drm/msm/dp: add msm_dp_display_get_panel() to initialize DP panel

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] msm_dp_display_get_panel() blindly allocates a duplicate panel structure for a given stream ID, leading to desynchronized internal state.
- [High] Missing NULL pointer check for optional pixel_base resource leads to a kernel panic during stream setup.
--

commit 09c5d687d7add705558b0ff0b93169b01fc6922e
Author: Abhinav Kumar <quic_abhinavk@quicinc.com>

drm/msm/dp: add msm_dp_display_get_panel() to initialize DP panel

This commit adds the msm_dp_display_get_panel API to initialize and
return a DP panel structure for use by the DP MST module.

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 90e3d2c4ef478..6952f83db0065 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c

[ ... ]

> @@ -529,6 +529,33 @@ static int msm_dp_irq_hpd_handle(struct msm_dp_display_private *dp)
>  	return rc;
>  }
>  
> +struct msm_dp_panel *msm_dp_display_get_panel(struct msm_dp *msm_dp_display,
> +					      enum msm_dp_stream_id stream_id)
> +{
> +	struct msm_dp_display_private *dp;
> +	struct msm_dp_panel *dp_panel;
> +
> +	dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
> +
> +	if (stream_id >= msm_dp_ctrl_get_stream_cnt(dp->ctrl)) {
> +		DRM_ERROR("invalid stream_id %d\n", stream_id);
> +		return NULL;
> +	}
> +
> +	dp_panel = msm_dp_panel_get(&dp->msm_dp_display.pdev->dev, dp->aux, dp->link,
> +				    dp->link_base, dp->mst2link_base, dp->mst3link_base,
> +				    dp->pixel_base[stream_id]);

[Severity: High]
Does this inadvertently create a duplicate panel structure that lacks
initialized state?

When an MST stream is enabled, this dynamically allocates a new panel struct
where link_info is zeroed. Since the primary panel usually populates dpcd and
link_info (via msm_dp_panel_read_link_caps()), bandwidth calculations in
msm_dp_panel_get_supported_bpp() will evaluate to 0 for this secondary panel.
Will this permanently degrade the display to the minimum 18bpp fallback?

[Severity: High]
Can dp->pixel_base[stream_id] be NULL here?

The pixel_base memory regions are explicitly handled as optional in
msm_dp_display_get_io(). If the device tree specifies a pixel clock but
omits the corresponding register region, passing dp->pixel_base[stream_id]
as NULL to msm_dp_panel_get() stores a NULL panel->pixel_base. 

When the display pipeline attempts to configure the panel timings later
(e.g., via msm_dp_write_pn()), could it dereference this NULL pointer and
cause a panic?

> +
> +	if (IS_ERR(dp_panel)) {
> +		DRM_ERROR("failed to initialize panel\n");
> +		return NULL;
> +	}
> +
> +	dp_panel->stream_id = stream_id;
> +
> +	return dp_panel;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-msm-dp-mst-v6-0-c91d35d6fb9e@oss.qualcomm.com?part=17