Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] The 2ms HPD deglitch filter (0xbb80) on Rockchip platforms completely filters out valid DP short pulses, breaking link loss detection and PSR. Pre-existing issues: - [High] The threaded IRQ handler ignores DP_IRQ_TYPE_IRQ_HPD (short pulses), preventing drm_helper_hpd_irq_event() from processing link loss or PSR events. -- --- Patch [7]: [PATCH v2 07/10] drm/bridge: analogix_dp: Use platform-specific HPD detection scheme --- commit 9400555d3cb8cd686c3464c5b0945a71464ab506 Author: Damon Ding <[email protected]> drm/bridge: analogix_dp: Use platform-specific HPD detection scheme Hotplug detection can be implemented either via HOTPLUG_CHG interrupt, or the combination of PLUG and HPD_LOST interrupts. This patch configures HPD deglitch to 2ms and uses HOTPLUG_CHG for hotplug detection on Rockchip platforms. > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > index 608d8d098329c..2399febb539e7 100644 > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > @@ -721,13 +721,18 @@ static irqreturn_t analogix_dp_irq_thread(int irq, void > *arg) > { > struct analogix_dp_device *dp = arg; > u32 irq_type; > + bool hpd_detected; > > irq_type = analogix_dp_get_irq_type(dp); > if (irq_type) > analogix_dp_clear_hotplug_interrupts(dp, irq_type); > > - if (irq_type & DP_IRQ_TYPE_HP_CABLE_IN || > - irq_type & DP_IRQ_TYPE_HP_CABLE_OUT) { > + if (!dp->hpd_gpiod && analogix_dp_is_rockchip(dp->plat_data->dev_type)) > + hpd_detected = irq_type & DP_IRQ_TYPE_HP_CHANGE; > + else > + hpd_detected = (irq_type & DP_IRQ_TYPE_HP_CABLE_IN) || > + (irq_type & DP_IRQ_TYPE_HP_CABLE_OUT); > + [Severity: High] This is a pre-existing issue, but does the calculation of hpd_detected in analogix_dp_irq_thread() miss DP_IRQ_TYPE_IRQ_HPD? It appears that short pulse interrupts (DP_IRQ_TYPE_IRQ_HPD) are ignored in this threaded IRQ handler and never propagated to the DRM core via drm_helper_hpd_irq_event(). This could cause link loss events to go unhandled until a physical cable replug. > + if (hpd_detected) { > dev_dbg(dp->dev, "Detected cable status changed!\n"); > if (dp->drm_dev) > drm_helper_hpd_irq_event(dp->drm_dev); > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c > b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c > index 42463e18f392c..fa8e2f104d6c5 100644 > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c > @@ -149,8 +149,13 @@ void analogix_dp_reset(struct analogix_dp_device *dp) > writel(0x0, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL); > writel(0x0, dp->reg_base + ANALOGIX_DP_HDCP_CTL); > > - writel(0x5e, dp->reg_base + ANALOGIX_DP_HPD_DEGLITCH_L); > - writel(0x1a, dp->reg_base + ANALOGIX_DP_HPD_DEGLITCH_H); > + if (analogix_dp_is_rockchip(dp->plat_data->dev_type)) { > + writel(0x80, dp->reg_base + ANALOGIX_DP_HPD_DEGLITCH_L); > + writel(0xbb, dp->reg_base + ANALOGIX_DP_HPD_DEGLITCH_H); [Severity: High] Will configuring the HPD deglitch filter to 2ms (0xbb80) completely filter out valid DP short pulses? The DP specification requires short pulses to be between 0.5ms and 1.0ms. It seems a 2.0ms filter will unconditionally drop them before they can trigger an interrupt, potentially breaking link loss recovery and PSR sink events on Rockchip platforms. > + } else { > + writel(0x5e, dp->reg_base + ANALOGIX_DP_HPD_DEGLITCH_L); > + writel(0x1a, dp->reg_base + ANALOGIX_DP_HPD_DEGLITCH_H); > + } > [ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7
