Re: [PATCH v2 15/23] drm/msm/dcp: Add DCP DSI host and panel support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:
- [High] drm/msm/dcp: off-by-one DSC chroma and color space enums in dcp_dsi_host_attach()
- [High] drm/msm/dcp: deadlock and unsafe teardown in dcp_dsi_bridge_cleanup()
- [Medium] drm/msm/dcp: OF node leak of dcp_np on error in dcp_dsi_bridge_init()
- [Medium] drm/msm/dcp: dcp_dsi_bridge_setup_dsc() never sets dsc_rc_override
- [Medium] drm/msm/dcp: duplicate dsi_host_dev name in dcp_dsi_bridge_init()
- [Medium] drm/msm/dcp: invalid session close and goto mixing in dcp_dsi_bridge_prepare_session()
- [Medium] drm/msm/dcp: missing session_lock in dcp_dsi_bridge_atomic_enable()
- [Medium] drm/msm/dcp: unsynchronized session_id and packet_id in power control
- [Medium] drm/msm/dcp: firmware session leak on dcp_hfi_display_close() early return
- [Medium] drm/msm/dcp: wrong byte count returned for DSI reads in dcp_dsi_host_transfer()
--

commit 7535bc53c50473e4189f52c83b74358ab514c8d0
Author: Jayant Shekhar <jayant.shekhar@oss.qualcomm.com>

drm/msm/dcp: Add DCP DSI host and panel support

This commit implements the MIPI DSI host, panel-device support, bridge
state, DSC configuration, DCS transfers, and DSI buffer management for the
DCP display path.

> diff --git a/drivers/gpu/drm/msm/dcp/dcp_dsi_bridge.c b/drivers/gpu/drm/msm/dcp/dcp_dsi_bridge.c
> new file mode 100644
> index 0000000000000..b3e782b8f0470
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/dcp/dcp_dsi_bridge.c
> @@ -0,0 +1,1110 @@

[ ... ]

> +static void dcp_dsi_power_control_work(struct work_struct *work)
> +{
> +	struct dcp_dsi_bridge *dsi_bridge =
> +		container_of(work, struct dcp_dsi_bridge, power_control_work);
> +	struct dcp_dpu_kms *kms = dsi_bridge->kms;
> +
> +	dcp_hfi_display_power_control_ack(kms->hfi,
> +					  dsi_bridge->session_id,
> +					  dsi_bridge->power_packet_id);
> +
> +	complete(&dsi_bridge->power_done);
> +}
> +
> +static void dcp_dsi_power_control_cb(void *priv, u32 display_id,
> +				     u32 power_mask, bool enable,
> +				     u32 packet_id)
> +{
> +	struct dcp_dpu_kms *kms = priv;
> +	struct dcp_dsi_bridge *dsi_bridge = NULL;
> +	int idx;
> +
> +	for (idx = 0; idx < ARRAY_SIZE(kms->dsi_bridge); idx++) {
> +		struct dcp_dsi_bridge *b = kms->dsi_bridge[idx];
> +
> +		if (b && b->session_id == display_id) {
> +			dsi_bridge = b;
> +			break;
> +		}
> +	}
> +
> +	if (!dsi_bridge) {
> +		dev_warn(kms->pm_dev,
> +			 "DSI: power_control for unknown display_id=%u\n", display_id);
> +		return;
> +	}
> +
> +	dsi_bridge->power_packet_id = packet_id;
> +	schedule_work(&dsi_bridge->power_control_work);
> +}

[Severity: Medium]
Are dsi_bridge->session_id and dsi_bridge->power_packet_id synchronized and
matched safely between dcp_dsi_power_control_cb() and
dcp_dsi_power_control_work()?

When dcp_hfi_dispatch_event() drops resp_drain_lock and invokes
dcp_dsi_power_control_cb(), the loop checks b->session_id == display_id
without checking b->session_open or holding session_lock. Since closed
bridges have session_id == 0, an unopened bridge matches when
display_id == 0, whereas a temporary session opened in
dcp_dsi_host_transfer() stores its session ID in a local variable and
leaves dsi_bridge->session_id == 0, failing to match a non-zero
display_id.

Furthermore, dcp_dsi_power_control_work() reads dsi_bridge->session_id and
dsi_bridge->power_packet_id without locking while
dcp_dsi_bridge_atomic_post_disable(), dcp_dsi_bridge_atomic_enable(), and
dcp_dsi_bridge_prepare_session() reset dsi_bridge->session_id = 0 without
flushing power_control_work. And in dcp_dsi_bridge_cleanup(),
dcp_hfi_clear_power_control_cb() does not wait for an in-flight callback
that already dropped resp_drain_lock, allowing
schedule_work(&dsi_bridge->power_control_work) to run after
cancel_work_sync().

[ ... ]

> +static int dcp_dsi_host_attach(struct mipi_dsi_host *host,
> +			       struct mipi_dsi_device *dsi)
> +{
> +	struct dcp_dsi_bridge *dsi_bridge = to_dcp_dsi_host(host);
> +	struct dcp_dsi_hfi_panel_config *cfg = &dsi_bridge->hfi_panel_config;
> +	int ret;

[ ... ]

> +	if (dsi->dsc) {
> +		struct drm_dsc_config *dsc = dsi->dsc;
> +		u32 chroma_format = 0;
> +		u32 color_space = 0;
> +
> +		if (dsc->native_422)
> +			chroma_format = 1;
> +		else if (dsc->native_420)
> +			chroma_format = 2;
> +		if (dsc->native_422 || dsc->native_420)
> +			color_space = 1;
> +
> +		cfg->dsc_enabled                 = true;
> +		cfg->dsc_version_major           = dsc->dsc_version_major;
> +		cfg->dsc_version_minor           = dsc->dsc_version_minor;
> +		cfg->dsc_scr_version              = 0;
> +		cfg->dsc_slice_height             = dsc->slice_height;
> +		cfg->dsc_slice_width              = dsc->slice_width;
> +		cfg->dsc_slices_per_pkt           = 1;
> +		cfg->dsc_bits_per_component       = dsc->bits_per_component;
> +		cfg->dsc_pps_delay_ms             = 0;
> +		cfg->dsc_bits_per_pixel           = dsc->bits_per_pixel;
> +		cfg->dsc_chroma_format            = chroma_format;
> +		cfg->dsc_color_space              = color_space;

[Severity: High]
Are chroma_format and color_space off by one here?

In drivers/gpu/drm/msm/dcp/hfi/hfi_defs_panel.h, the HFI protocol enums
packed into HFI_PROPERTY_PANEL_COMPRESSION_DATA by dcp_hfi_panel_init() are
1-based:

enum hfi_panel_chroma_format_type {
	HFI_PANEL_CHROMA_FORMAT_444     = 1,
	HFI_PANEL_CHROMA_FORMAT_422     = 2,
	HFI_PANEL_CHROMA_FORMAT_420     = 3,
};

enum hfi_panel_color_space_type {
	HFI_PANEL_COLOR_SPACE_RGB     = 1,
	HFI_PANEL_COLOR_SPACE_YUV     = 2,
};

Should dcp_dsi_host_attach() use the HFI_PANEL_CHROMA_FORMAT_* and
HFI_PANEL_COLOR_SPACE_* constants instead of 0-based values?

[ ... ]

> +static ssize_t dcp_dsi_host_transfer(struct mipi_dsi_host *host,
> +				     const struct mipi_dsi_msg *msg)
> +{
> +	struct dcp_dsi_bridge *dsi_bridge = to_dcp_dsi_host(host);
> +	struct dcp_dpu_kms *kms = dsi_bridge->kms;
> +	u32 session_id;
> +	bool temporary_session = false;
> +	ssize_t ret;
> +
> +	guard(mutex)(&dsi_bridge->session_lock);
> +	if (!dsi_bridge->dcs_tx_buf)
> +		return -ENODEV;
> +
> +	/* Serialize DCS transfers */
> +	guard(mutex)(&dsi_bridge->dcs_lock);
> +
> +	if (dsi_bridge->session_open) {
> +		session_id = dsi_bridge->session_id;
> +	} else {
> +		/*
> +		 * mipi_dsi_host_ops::transfer() may be called while the host is
> +		 * disabled. Open a transaction-only DCP session in that case;
> +		 * atomic_pre_enable() will create the full display session later.
> +		 */
> +		ret = dcp_hfi_display_enable(kms->hfi, dsi_bridge->intf_idx,
> +					     dsi_bridge->fw_display_id,
> +					     &session_id);
> +		if (ret)
> +			return ret;
> +
> +		temporary_session = true;
> +	}
> +
> +	ret = dcp_hfi_dcs_transfer(kms->hfi, session_id, msg,
> +				   dsi_bridge->dcs_tx_buf, dsi_bridge->dcs_tx_iova,
> +				   dsi_bridge->dcs_rx_buf, dsi_bridge->dcs_rx_iova, 0,
> +				   dsi_bridge->hfi_panel_config.ctrl_indices[0]);

[Severity: Medium]
Does dcp_dsi_host_transfer() return the wrong byte count for DSI read
transfers and reject 0-parameter generic transfers?

In drivers/gpu/drm/msm/dcp/dcp_hfi_dsi_panel.c, dcp_hfi_dcs_transfer() ends
with:

	if (msg->rx_len > 0)
		memcpy(msg->rx_buf, rx_buf, msg->rx_len);

	return msg->tx_len;

and rejects msg->tx_len == 0 at entry with -EINVAL.

For mipi_dsi_host_ops::transfer(), callers such as mipi_dsi_dcs_read() and
mipi_dsi_generic_read() expect the number of received bytes (msg->rx_len)
on read transfers, and 0-parameter generic transfers
(MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM and
MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM) have msg->tx_len == 0.

> +	if (temporary_session) {
> +		int close_ret = dcp_hfi_display_close(kms->hfi, session_id);
> +
> +		if (ret >= 0 && close_ret)
> +			ret = close_ret;
> +	}

[Severity: Medium]
Can calling dcp_hfi_display_close() here (and on the err_close path in
dcp_dsi_bridge_atomic_enable()) leak the firmware display session if
DISPLAY_DISABLE or DISPLAY_POST_DISABLE fails?

In drivers/gpu/drm/msm/dcp/dcp_hfi_dsi.c, dcp_hfi_display_close() returns
early before sending HFI_COMMAND_DISPLAY_CLOSE if either
dcp_hfi_display_disable() or dcp_hfi_display_post_disable() returns an
error:

int dcp_hfi_display_close(struct dcp_hfi *hfi, u32 session_id)
{
	int ret;

	ret = dcp_hfi_display_disable(hfi, session_id);
	if (ret) {
		dev_warn(hfi->dev, "HFI DISPLAY_DISABLE failed: %d\n", ret);
		return ret;
	}

	ret = dcp_hfi_display_post_disable(hfi, session_id);
	if (ret) {
		dev_warn(hfi->dev, "HFI DISPLAY_POST_DISABLE failed: %d\n", ret);
		return ret;
	}

When closing a temporary session in dcp_dsi_host_transfer() (which only
called dcp_hfi_display_enable() and never called
dcp_hfi_display_post_enable()) or rolling back a partially enabled session
in dcp_dsi_bridge_atomic_enable(), an error from DISPLAY_DISABLE or
DISPLAY_POST_DISABLE skips HFI_COMMAND_DISPLAY_CLOSE while the driver drops
session_id and marks session_open = false. In contrast,
dcp_dsi_bridge_atomic_disable() and dcp_dsi_bridge_atomic_post_disable()
still send HFI_COMMAND_DISPLAY_CLOSE even if the preceding disable commands
warn.

[ ... ]

> +static int dcp_dsi_bridge_setup_dsc(struct dcp_dsi_bridge *dsi_bridge,
> +				    struct dcp_dsi_hfi_panel_config *cfg,
> +				    const struct drm_display_mode *mode)
> +{

[ ... ]

> +	for (i = 0; i < ARRAY_SIZE(cfg->dsc_rc_min_qp); i++) {
> +		cfg->dsc_rc_min_qp[i] = dsc->rc_range_params[i].range_min_qp;
> +		cfg->dsc_rc_max_qp[i] = dsc->rc_range_params[i].range_max_qp;
> +		cfg->dsc_rc_bpg_offsets[i] = dsc->rc_range_params[i].range_bpg_offset;
> +	}
> +
> +	return 0;
> +}

[Severity: Medium]
Should dcp_dsi_bridge_setup_dsc() also set cfg->dsc_rc_override = true?

dcp_dsi_host_attach() initializes cfg->dsc_rc_override = false, and
dcp_dsi_bridge_prepare_session() passes a stack-local copy of cfg to
dcp_dsi_bridge_setup_dsc() and then to dcp_hfi_panel_init(). In
drivers/gpu/drm/msm/dcp/dcp_hfi_dsi_panel.c, dcp_hfi_panel_init() only packs
HFI_PROPERTY_PANEL_COMPRESSION_RC_OVERRIDE when
(cfg->dsc_enabled && cfg->dsc_rc_override) is true, so the rate-control
parameters populated in cfg->dsc_rc_min_qp, cfg->dsc_rc_max_qp, and
cfg->dsc_rc_bpg_offsets are never sent to the firmware.

> +
> +int dcp_dsi_bridge_prepare_session(struct dcp_dsi_bridge *dsi_bridge,
> +				   const struct drm_display_mode *mode)
> +{
> +	struct dcp_dpu_kms *kms = dsi_bridge->kms;
> +	struct dcp_dsi_bridge_state *state = dcp_dsi_bridge_get_state(dsi_bridge);
> +	struct dcp_dsi_hfi_panel_config cfg;
> +	int ret;
> +	u32 opened_session_id;
> +
> +	if (!mode)
> +		return -ENODEV;
> +
> +	guard(mutex)(&dsi_bridge->session_lock);
> +	if (dsi_bridge->session_open)
> +		return 0;
> +
> +	dsi_bridge->session_id = dsi_bridge->fw_display_id;
> +
> +	if (!dsi_bridge->dsi_device)
> +		return -ENODEV;

[ ... ]

> +	ret = dcp_hfi_display_enable(kms->hfi,
> +				     dsi_bridge->intf_idx,
> +				     dsi_bridge->fw_display_id,
> +				     &opened_session_id);
> +	if (ret) {
> +		drm_err(kms->dev, "DSI%u: HFI DISPLAY_ENABLE failed: %d\n",
> +			dsi_bridge->intf_idx, ret);
> +		goto err_close;
> +	}
> +	dsi_bridge->session_id   = opened_session_id;
> +	dsi_bridge->session_open = true;
> +	dcp_dsi_bridge_publish_session(dsi_bridge);
> +
> +	drm_dbg_driver(kms->dev, "DSI%u: session prepared (session_id=%u)\n",
> +		       dsi_bridge->intf_idx, dsi_bridge->session_id);
> +	return 0;
> +
> +err_close:
> +	dcp_hfi_clear_power_control_cb(kms->hfi, kms);
> +	dcp_hfi_display_close(kms->hfi, dsi_bridge->session_id);
> +	dsi_bridge->session_id   = 0;
> +	dsi_bridge->session_open = false;
> +	dcp_dsi_bridge_clear_session(dsi_bridge);
> +
> +	return ret;
> +}

[Severity: Medium]
There appear to be several issues with error handling and cleanup in
dcp_dsi_bridge_prepare_session():

1. It mixes guard(mutex)(&dsi_bridge->session_lock) with goto err_close,
   contrary to the cleanup.h guideline not to mix goto error paths and
   scope-based cleanup helpers in the same function.

2. If !dsi_bridge->dsi_device is true, returning -ENODEV early leaves
   dsi_bridge->session_id set to dsi_bridge->fw_display_id while
   session_open is false.

3. Every goto err_close path triggers before dcp_hfi_display_enable() has
   succeeded (even if dcp_dsi_bridge_setup_dsc() fails in software before
   any HFI command is sent), yet err_close calls dcp_hfi_display_close() to
   send DISPLAY_DISABLE, DISPLAY_POST_DISABLE, and DISPLAY_CLOSE for a
   session that was never enabled.

4. Calling dcp_hfi_clear_power_control_cb(kms->hfi, kms) in err_close clears
   the single shared power control callback on kms->hfi for all DSI bridges
   in kms->dsi_bridge[].

> +
> +static void dcp_dsi_bridge_atomic_pre_enable(struct drm_bridge *bridge,
> +					     struct drm_atomic_commit *state)
> +{
> +	struct dcp_dsi_bridge *dsi_bridge = to_dcp_dsi_bridge(bridge);
> +	int ret;
> +
> +	ret = dsi_bridge->session_open ? 0 : -ENODEV;
> +	dcp_dsi_bridge_get_state(dsi_bridge)->prepare_error = ret;
> +	if (ret)
> +		drm_err(bridge->dev, "DSI%u: session prepare failed: %d\n",
> +			dsi_bridge->intf_idx, ret);
> +}
> +
> +static void dcp_dsi_bridge_atomic_enable(struct drm_bridge *bridge,
> +					 struct drm_atomic_commit *state)
> +{
> +	struct dcp_dsi_bridge *dsi_bridge = to_dcp_dsi_bridge(bridge);
> +	struct dcp_dpu_kms *kms = dsi_bridge->kms;
> +	bool scan_start_registered = false;
> +	bool scan_complete_registered = false;
> +	bool autorefresh_enabled = false;
> +	int ret;
> +
> +	drm_dbg_driver(kms->dev,
> +		       "DSI%u: bridge atomic_enable (session_id=%u)\n",
> +		       dsi_bridge->intf_idx, dsi_bridge->session_id);
> +
> +	if (dcp_dsi_bridge_get_state(dsi_bridge)->prepare_error)
> +		return;
> +	if (!dsi_bridge->session_open)
> +		return;
> +
> +	ret = dcp_hfi_display_post_enable(kms->hfi, dsi_bridge->session_id);

[ ... ]

> +err_close:
> +	if (autorefresh_enabled)
> +		dcp_hfi_crtc_set_autorefresh(kms->hfi, dsi_bridge->session_id,
> +					     false, 0);
> +	if (scan_complete_registered)
> +		dcp_hfi_display_event_deregister(kms->hfi, dsi_bridge->session_id,
> +						 HFI_EVENT_FRAME_SCAN_COMPLETE);
> +	if (scan_start_registered)
> +		dcp_hfi_display_event_deregister(kms->hfi, dsi_bridge->session_id,
> +						 HFI_EVENT_FRAME_SCAN_START);
> +	drm_err(bridge->dev,
> +		"DSI%u: bridge enable failed; session closed\n",
> +		dsi_bridge->intf_idx);
> +	dcp_hfi_display_close(kms->hfi, dsi_bridge->session_id);
> +	{
> +		guard(mutex)(&dsi_bridge->session_lock);
> +		dsi_bridge->session_id   = 0;
> +		dsi_bridge->session_open = false;
> +	}
> +	dcp_dsi_bridge_clear_session(dsi_bridge);
> +}

[Severity: Medium]
Should dcp_dsi_bridge_atomic_enable() (and dcp_dsi_bridge_atomic_pre_enable())
hold dsi_bridge->session_lock across the session operations rather than only
around clearing session_id and session_open at the end of err_close?

In addition to mixing goto err_close with guard(mutex), reading
dsi_bridge->session_open and dsi_bridge->session_id and calling
dcp_hfi_display_close() without holding session_lock allows a concurrent
dcp_dsi_host_transfer() call to observe session_open == true and send DCS
commands on a session ID that is actively being closed or has already been
closed in firmware.

[ ... ]

> +struct drm_bridge *dcp_dsi_bridge_init(struct dcp_dpu_kms *kms,
> +				       struct device_node *np,
> +				       struct device_node *dcp_np,
> +				       u32 dcp_port, u32 intf_idx,
> +				       u32 fw_display_id)
> +{
> +	struct dcp_dsi_bridge *dsi_bridge;
> +	struct dcp_dsi_hfi_panel_config *cfg;
> +	struct device *dev = kms->pm_dev;
> +	struct device *dma_dev = kms->hfi->dma_dev;
> +	int ret;
> +
> +	dsi_bridge = devm_drm_bridge_alloc(dev, struct dcp_dsi_bridge,
> +					   bridge, &dcp_dsi_bridge_funcs);
> +	if (IS_ERR(dsi_bridge))
> +		return ERR_CAST(dsi_bridge);
> +
> +	dsi_bridge->dcp_np        = of_node_get(dcp_np);

[ ... ]

> +	device_initialize(&dsi_bridge->dsi_host_dev);
> +	dsi_bridge->dsi_host_dev.release = dcp_dsi_host_dev_release;
> +	dsi_bridge->dsi_host_dev.parent  = dev;
> +	dsi_bridge->dsi_host_dev.of_node = of_node_get(np);
> +	dev_set_name(&dsi_bridge->dsi_host_dev, "%pOFn", np);
> +	dsi_bridge->dsi_host.ops = &dcp_dsi_host_ops;
> +	dsi_bridge->dsi_host.dev = &dsi_bridge->dsi_host_dev;
> +
> +	ret = device_add(&dsi_bridge->dsi_host_dev);

[Severity: Medium]
Won't dev_set_name(&dsi_bridge->dsi_host_dev, "%pOFn", np) cause a device
name collision in device_add() when two DSI bridges are present?

The %pOFn specifier strips the @unit-address suffix from the node name, so
both dsi@0 and dsi@1 receive the name "dsi" under the same parent device
(kms->pm_dev), causing the second device_add() to fail with -EEXIST.

[ ... ]

> +err_unwind_dma:
> +	if (dsi_bridge->dcs_rx_buf) {
> +		dma_free_coherent(dma_dev, DCP_DSI_DCS_RX_BUF_SIZE,
> +				  dsi_bridge->dcs_rx_buf,
> +				  dsi_bridge->dcs_rx_iova);
> +	}
> +	if (dsi_bridge->dcs_tx_buf) {
> +		dma_free_coherent(dma_dev, DCP_DSI_DCS_TX_BUF_SIZE,
> +				  dsi_bridge->dcs_tx_buf,
> +				  dsi_bridge->dcs_tx_iova);
> +	}
> +	cancel_work_sync(&dsi_bridge->power_control_work);
> +	mutex_destroy(&dsi_bridge->session_lock);
> +	mutex_destroy(&dsi_bridge->dcs_lock);
> +	return ERR_PTR(ret);
> +}

[Severity: Medium]
Does the err_unwind_dma path in dcp_dsi_bridge_init() leak the
dsi_bridge->dcp_np device_node reference acquired via of_node_get(dcp_np) if
dma_alloc_coherent(), device_add(), or mipi_dsi_host_register() fails?

[ ... ]

> +void dcp_dsi_bridge_cleanup(struct dcp_dsi_bridge *dsi_bridge)
> +{
> +	struct device *dma_dev = dsi_bridge->kms->hfi->dma_dev;
> +
> +	dcp_hfi_clear_power_control_cb(dsi_bridge->kms->hfi, dsi_bridge->kms);
> +	cancel_work_sync(&dsi_bridge->power_control_work);
> +
> +	drm_bridge_remove(&dsi_bridge->bridge);
> +	of_node_put(dsi_bridge->bridge.of_node);
> +	of_node_put(dsi_bridge->dcp_np);
> +
> +	mipi_dsi_host_unregister(&dsi_bridge->dsi_host);
> +	device_del(&dsi_bridge->dsi_host_dev);
> +	put_device(&dsi_bridge->dsi_host_dev);
> +
> +	dcp_dsi_bridge_unmap_buffers(dsi_bridge);
> +	dma_free_coherent(dma_dev, DCP_DSI_DCS_TX_BUF_SIZE,
> +			  dsi_bridge->dcs_tx_buf, dsi_bridge->dcs_tx_iova);
> +
> +	dma_free_coherent(dma_dev, DCP_DSI_DCS_RX_BUF_SIZE,
> +			  dsi_bridge->dcs_rx_buf, dsi_bridge->dcs_rx_iova);
> +
> +	mutex_destroy(&dsi_bridge->session_lock);
> +	mutex_destroy(&dsi_bridge->dcs_lock);
> +}

[Severity: High]
Can calling mipi_dsi_host_unregister() from dcp_dsi_bridge_cleanup()
deadlock on component_mutex and trigger use-after-free during teardown?

1. When a DSI device is attached, mipi_dsi_host_unregister() removes the
   child device and invokes:

   mipi_dsi_remove_device_fn()
     -> mipi_dsi_detach()
       -> dcp_dsi_host_detach()
         -> component_del(&dsi_bridge->dsi_host_dev, &dcp_dsi_component_ops)

   component_del() acquires the global component_mutex. When
   dcp_dsi_bridge_cleanup() is called from aggregate device bringup failure
   or aggregate device takedown (for example via component_master_del() /
   take_down_aggregate_device() -> msm_drm_unbind() -> msm_drm_uninit() ->
   dcp_dpu_kms_destroy()), component_mutex is already held by the caller,
   causing a recursive deadlock.

2. struct dcp_dsi_bridge is allocated with devm_drm_bridge_alloc() on
   kms->pm_dev and embeds struct device dsi_host_dev, which is refcounted
   independently. Calling device_del() and put_device() on dsi_host_dev
   during kms->destroy() happens before component_unbind_all() finishes
   using component->dev (&dsi_bridge->dsi_host_dev). In addition,
   dcs_tx_buf and dcs_rx_buf are freed without holding session_lock/dcs_lock
   or clearing dsi_bridge->dcs_tx_buf to NULL, and session_lock and dcs_lock
   are destroyed while the bridge object is still alive.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026-jshekhar-dcp-hlos-v2-0-7cf014efdee1@oss.qualcomm.com?part=15