From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/msm/dp: rework HPD handling Date: Mon, 25 May 2026 17:13:08 +1000 Message-ID: In-Reply-To: <20260524-hpd-refactor-v6-7-cf3ab488dd7b@oss.qualcomm.com> References: <20260524-hpd-refactor-v6-0-cf3ab488dd7b@oss.qualcomm.com> <20260524-hpd-refactor-v6-7-cf3ab488dd7b@oss.qualcomm.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review This is the core patch (~600 lines changed). It drops the entire event queue/state machine and replaces it with a threaded IRQ handler that calls `drm_bridge_hpd_notify()` directly. **Concern (potential bug):** The IRQ thread handler does not check `DP_DP_HPD_REPLUG_INT_MASK`: ```c + if (hpd_isr_status & DP_DP_HPD_UNPLUG_INT_MASK) + drm_bridge_hpd_notify(..., connector_status_disconnected); + + if (hpd_isr_status & DP_DP_HPD_PLUG_INT_MASK) + drm_bridge_hpd_notify(..., connector_status_connected); + + if (hpd_isr_status & DP_DP_IRQ_HPD_INT_MASK) + drm_bridge_hpd_notify(..., connector_status_connected); ``` The old code explicitly handled `DP_DP_HPD_REPLUG_INT_MASK` with an unplug+delayed plug sequence. The new code relies on `msm_dp_bridge_hpd_notify()` detecting replug via `msm_dp_aux_is_link_connected()` returning `ISR_HPD_REPLUG_COUNT`. But if the hardware sets only the REPLUG bit (without PLUG or UNPLUG), the IRQ thread will not call `drm_bridge_hpd_notify()` at all and the replug event is silently dropped. The old code previously queued explicit events for this. If hardware always sets PLUG alongside REPLUG this is fine, but that's a hardware assumption that should be documented. **Observation:** The old replug handling had a 3-tick delay between unplug and plug events. The new `hpd_notify` handler does them back-to-back: ```c + if (hpd_link_status == ISR_HPD_REPLUG_COUNT) { + msm_dp_hpd_unplug_handle(dp); + msm_dp_hpd_plug_handle(dp); ``` This removes the debounce timing. If the display needs settling time between unplug and plug (which is common for DP), this could cause link training failures on replug events. **Observation:** `msm_dp_snapshot()` loses its `event_mutex` protection. The function reads `dp->power_on` and accesses MMIO registers. Without locking, there's a TOCTOU race where power_on is true at the check but the device gets powered off before the register reads. This was pre-existing but the mutex removal makes it slightly worse. --- --- Generated by Claude Code Patch Reviewer