Re: [PATCH v1 4/7] drm/bridge: analogix_dp: Extend clear_hotplug_interrupts to accept IRQ bitmask

[sashiko-bot@kernel.org]

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

New issues:
- [High] Data Race in Interrupt Mask Registers During RMW

Pre-existing issues:
- [High] Lost Hardware Hotplug Events Due to Late Status Clearing
--

--- Patch [4]: [PATCH v1 4/7] drm/bridge: analogix_dp: Extend clear_hotplug_interrupts to accept IRQ bitmask ---
commit f4ba8e259db1706905ae7d04afdea18e9d5a28d6
Author: Damon Ding <damon.ding@rock-chips.com>

drm/bridge: analogix_dp: Extend clear_hotplug_interrupts to accept IRQ bitmask

Update analogix_dp_clear_hotplug_interrupts() to take an irq_type
bitmask argument. Instead of clearing all hotplug interrupt bits
unconditionally, only clear the interrupt flags corresponding to the
triggered events.

> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -733,7 +733,7 @@ static irqreturn_t analogix_dp_irq_thread(int irq, void *arg)
>  	}
>  
>  	if (irq_type) {
> -		analogix_dp_clear_hotplug_interrupts(dp);
> +		analogix_dp_clear_hotplug_interrupts(dp, irq_type);

[Severity: High]
This is a pre-existing issue, but can this late clearing cause hardware
hotplug events to be silently lost?

Because the hardware interrupt status is cleared at the end of the threaded
handler, it executes after the time-consuming drm_helper_hpd_irq_event()
completes:

analogix_dp_irq_thread()
    if (hpd_detected) {
        ...
        drm_helper_hpd_irq_event(dp->drm_dev);
    }

If a fast unplug/plug cable bounce occurs exactly while
drm_helper_hpd_irq_event() is executing, the hardware sets the status bit
again. Clearing the write-1-to-clear (W1C) register after this will clear
the new event without triggering a new interrupt.

Should the W1C status registers be cleared immediately after reading their
state, before executing the event handling logic?

>  		analogix_dp_unmute_hpd_interrupt(dp, irq_type);

[Severity: High]
Does this code need a spinlock to protect the interrupt mask registers?

Since the IRQ is requested without IRQF_ONESHOT, the hardirq handler can
preempt the threaded handler. If analogix_dp_unmute_hpd_interrupt() reads
ANALOGIX_DP_COMMON_INT_MASK_4 and gets preempted by a new hardirq that
masks a new event, the resuming threaded handler might overwrite the
register with its stale read value.

Could this cause a hotplug interrupt type to become permanently masked
if the hardirq overwrites the threaded handler's unmute operation?

>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730032744.381566-1-damon.ding@rock-chips.com?part=4