Re: [PATCH v11 04/21] drm/bridge: synopsys: dw-dp: Document missing reset line deassert

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing completion reinitialization causes spurious AUX transfer successes
- [High] HPD event state overwrite race drops hotplug events
--

--- Patch [4]: [PATCH v11 04/21] drm/bridge: synopsys: dw-dp: Document missing reset line deassert ---
commit 34c619c6d4754597f7c81d207e5acfe482e8b36d
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

drm/bridge: synopsys: dw-dp: Document missing reset line deassert

This patch adds a comment explaining why the driver avoids using automatically
deasserting reset controls. Unbinding the device with the reset line deasserted
can cause the SoC to hang if the shared power-domain is still active. The
current reset handling remains unchanged.

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> index 21d60e89126f0..52e3b37259a69 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but does dw_dp_aux_transfer()
miss a completion reinitialization?

drivers/gpu/drm/bridge/synopsys/dw-dp.c:dw_dp_aux_transfer() {
...
	regmap_write(dp->regmap, DW_DP_AUX_CMD, value);

	status = wait_for_completion_timeout(&dp->complete, timeout);
...
}

If a previous AUX transfer times out, wait_for_completion_timeout() returns 0
without decrementing the completion's done counter. If the hardware later
generates an interrupt, the IRQ handler calls complete(&dp->complete).

Will the next call to dw_dp_aux_transfer() immediately pass its
wait_for_completion_timeout() without actually waiting for the new command to
complete? 

This could lead to reading stale or invalid hardware status.

[Severity: High]
This is a pre-existing issue, but is there a hotplug event state overwrite race
between dw_dp_handle_hpd_event() and dw_dp_hpd_work()?

drivers/gpu/drm/bridge/synopsys/dw-dp.c:dw_dp_handle_hpd_event() {
...
	if (value & HPD_IRQ) {
		dev_dbg(dp->dev, "IRQ from the HPD\n");
		dp->hotplug.long_hpd = false;
		regmap_write(dp->regmap, DW_DP_HPD_STATUS, HPD_IRQ);
	}

	if (value & HPD_HOT_PLUG) {
		dev_dbg(dp->dev, "Hot plug detected\n");
		dp->hotplug.long_hpd = true;
		regmap_write(dp->regmap, DW_DP_HPD_STATUS, HPD_HOT_PLUG);
	}
...
}

If a long HPD event occurs, long_hpd is set to true and work is scheduled.
If a short HPD event occurs before the workqueue runs, the interrupt handler
overwrites long_hpd to false and calls schedule_work(), which coalesces with
the pending work.

drivers/gpu/drm/bridge/synopsys/dw-dp.c:dw_dp_hpd_work() {
...
	if (!long_hpd) {
		if (dw_dp_needs_link_retrain(dp)) {
			ret = dw_dp_link_retrain(dp);
			if (ret)
				dev_warn(dp->dev, "Retrain link failed\n");
		}
	} else {
		drm_helper_hpd_irq_event(dp->bridge.dev);
	}
...
}

When the work finally executes, it sees long_hpd == false and fails to call
drm_helper_hpd_irq_event(). 

Can this drop hotplug events, leaving userspace unaware of display connections?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806-synopsys-dw-dp-improvements-v11-0-0d508505f383@collabora.com?part=4