* [PATCH v1 0/7] Add HPD support for Rockchip Analogix DP
@ 2026-07-30 3:27 Damon Ding
2026-07-30 3:27 ` [PATCH v1 1/7] drm/bridge: analogix_dp: Manage pm runtime and IRQ for native HPD pin detection Damon Ding
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Damon Ding @ 2026-07-30 3:27 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Heiko Stuebner, Andy Yan
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
Dmitry Baryshkov, Marek Szyprowski, Sebastian Reichel, dri-devel,
devicetree, linux-kernel, linux-rockchip, linux-arm-kernel,
Damon Ding
This series improves the HPD (Hotplug Detect) interrupt handling in
the Analogix DP driver to enable reliable native HPD pin detection on
Rockchip platforms, and introduces platform-specific HPD detection
schemes with fine-grained interrupt control.
On Rockchip platforms, the Analogix DP native HPD pin IRQ requires the
DP controller to remain powered, clocked and initialized to generate
plug/unplug interrupts. The previous driver enabled/disabled IRQ during
bridge enable/disable, which left no HPD detection when the display
pipeline was inactive. Additionally, the interrupt mute/unmute/clear
routines operated on all HPD interrupt bits unconditionally, lacking
the granularity needed for per-event control.
The series reorganizes IRQ and pm_runtime management into bind/unbind,
converts the interrupt type detection to a bitmask-based scheme for
fine-grained mute/unmute/clear operations, and configures Rockchip
platforms to use the HOTPLUG_CHG interrupt with a 2ms HPD deglitch
setting, which provides better stability than the PLUG + HPD_LOST
pair.
Tested on RK3576 with both native HPD pin and GPIO HPD configurations.
Native HPD pin mode:
&edp {
status = "okay";
pinctrl-names = "default";
pinctrl-0 = <&edp_txm0_pins>;
};
GPIO HPD mode:
&edp {
status = "okay";
pinctrl-names = "default";
pinctrl-0 = <&edp0_hpd>;
hpd-gpios = <&gpio4 RK_PC1 GPIO_ACTIVE_HIGH>;
};
&pinctrl {
edp {
edp0_hpd: edp0-hpd {
rockchip,pins = <4 RK_PC1 0 &pcfg_pull_none>;
};
};
};
Both configurations detect cable plug/unplug events correctly.
Patch 1: Move enable_irq()/disable_irq() to bind/unbind and hold a
pm_runtime reference for Rockchip native HPD pin mode.
Patch 2: Convert analogix_dp_get_irq_type() to return a u32 bitmask
instead of an enum, accumulating all pending interrupt flags.
Patch 3: Extend mute/unmute helpers to accept an irq_type bitmask for
per-event interrupt masking.
Patch 4: Extend clear_hotplug_interrupts() to accept an irq_type
bitmask for per-event pending interrupt clearing.
Patch 5: Simplify analogix_dp_config_interrupt() by removing redundant
local macros and leveraging the unmute helper.
Patch 6: Configure Rockchip platforms to use HOTPLUG_CHG interrupt with
2ms HPD deglitch; other platforms keep PLUG + HPD_LOST pair.
Patch 7: Skip native HPD interrupt register operations for GPIO HPD
mode, where hotplug is detected through an external GPIO.
Damon Ding (7):
drm/bridge: analogix_dp: Manage pm runtime and IRQ for native HPD pin
detection
drm/bridge: analogix_dp: Return bitmask from
analogix_dp_get_irq_type()
drm/bridge: analogix_dp: Extend mute/unmute HPD interrupts to accept
irq bitmask
drm/bridge: analogix_dp: Extend clear_hotplug_interrupts to accept IRQ
bitmask
drm/bridge: analogix_dp: Simplify analogix_dp_config_interrupt()
drm/bridge: analogix_dp: Use platform-specific HPD detection scheme
drm/bridge: analogix_dp: Skip native HPD interrupt ops for GPIO HPD
.../drm/bridge/analogix/analogix_dp_core.c | 46 ++++--
.../drm/bridge/analogix/analogix_dp_core.h | 10 +-
.../gpu/drm/bridge/analogix/analogix_dp_reg.c | 146 ++++++++++++------
3 files changed, 134 insertions(+), 68 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v1 1/7] drm/bridge: analogix_dp: Manage pm runtime and IRQ for native HPD pin detection
2026-07-30 3:27 [PATCH v1 0/7] Add HPD support for Rockchip Analogix DP Damon Ding
@ 2026-07-30 3:27 ` Damon Ding
2026-07-30 3:39 ` sashiko-bot
2026-07-30 3:27 ` [PATCH v1 2/7] drm/bridge: analogix_dp: Return bitmask from analogix_dp_get_irq_type() Damon Ding
` (4 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Damon Ding @ 2026-07-30 3:27 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Heiko Stuebner, Andy Yan
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
Dmitry Baryshkov, Marek Szyprowski, Sebastian Reichel, dri-devel,
devicetree, linux-kernel, linux-rockchip, linux-arm-kernel,
Damon Ding
On Rockchip platforms, the Analogix DP native HPD pin IRQ functionality
requires the DP controller to be powered, clocked and initialized to
generate plug/unplug interrupts correctly.
To keep HPD detection active, move enable_irq()/disable_irq() from
bridge enabling/disabling handlers into analogix_dp_bind() and
analogix_dp_unbind(). Call pm_runtime_resume_and_get() before enabling
IRQ and pm_runtime_put_sync() after disabling IRQ for symmetric power
management.
Persistent power is only necessary for native HPD pin mode. It is not
required for force-HPD or GPIO HPD modes. Add helper to handle this
special case exclusively for Rockchip native HPD pin configurations.
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
.../drm/bridge/analogix/analogix_dp_core.c | 22 ++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 8d3d4a6e6ca2..d414f4ff40c8 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -40,6 +40,12 @@
static const bool verify_fast_training;
+static bool analogix_dp_require_pm_for_hpd_irq(struct analogix_dp_device *dp)
+{
+ return analogix_dp_is_rockchip(dp->plat_data->dev_type) && !dp->hpd_gpiod &&
+ !dp->force_hpd;
+}
+
static void analogix_dp_init_dp(struct analogix_dp_device *dp)
{
analogix_dp_reset(dp);
@@ -1014,7 +1020,6 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
goto out_dp_init;
}
- enable_irq(dp->irq);
return 0;
out_dp_init:
@@ -1156,8 +1161,6 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
if (dp->dpms_mode != DRM_MODE_DPMS_ON)
return;
- disable_irq(dp->irq);
-
analogix_dp_set_analog_power_down(dp, POWER_ALL, 1);
pm_runtime_put_sync(dp->dev);
@@ -1504,6 +1507,14 @@ int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
goto err_unregister_aux;
}
+ if (analogix_dp_require_pm_for_hpd_irq(dp)) {
+ ret = pm_runtime_resume_and_get(dp->dev);
+ if (ret)
+ goto err_unregister_aux;
+ }
+
+ enable_irq(dp->irq);
+
return 0;
err_unregister_aux:
@@ -1515,6 +1526,11 @@ EXPORT_SYMBOL_GPL(analogix_dp_bind);
void analogix_dp_unbind(struct analogix_dp_device *dp)
{
+ disable_irq(dp->irq);
+
+ if (analogix_dp_require_pm_for_hpd_irq(dp))
+ pm_runtime_put_sync(dp->dev);
+
drm_dp_aux_unregister(&dp->aux);
}
EXPORT_SYMBOL_GPL(analogix_dp_unbind);
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v1 2/7] drm/bridge: analogix_dp: Return bitmask from analogix_dp_get_irq_type()
2026-07-30 3:27 [PATCH v1 0/7] Add HPD support for Rockchip Analogix DP Damon Ding
2026-07-30 3:27 ` [PATCH v1 1/7] drm/bridge: analogix_dp: Manage pm runtime and IRQ for native HPD pin detection Damon Ding
@ 2026-07-30 3:27 ` Damon Ding
2026-07-30 3:41 ` sashiko-bot
2026-07-30 3:27 ` [PATCH v1 3/7] drm/bridge: analogix_dp: Extend mute/unmute HPD interrupts to accept irq bitmask Damon Ding
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Damon Ding @ 2026-07-30 3:27 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Heiko Stuebner, Andy Yan
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
Dmitry Baryshkov, Marek Szyprowski, Sebastian Reichel, dri-devel,
devicetree, linux-kernel, linux-rockchip, linux-arm-kernel,
Damon Ding
Analogix DP controllers support two sets of interrupts for hotplug
detection: HOTPLUG_CHG, and the pair PLUG / HPD_LOST. The current
driver logic relies on PLUG/HPD_LOST and does not consume HOTPLUG_CHG,
nor does it check for INT_HPD for IRQ_HPD events.
The existing analogix_dp_get_irq_type() returns on the first matched
interrupt flag. This causes the hardirq handler to unconditionally mute
all HPD interrupts, including HOTPLUG_CHG and INT_HPD, creating a
limitation for future extensions.
To prepare fine-grained interrupt handling, convert the return type
from enum dp_irq_type to a u32 bitmask. Accumulate all pending
interrupt flags instead of returning early, and add detection for
DP_IRQ_TYPE_IRQ_HPD. Remove DP_IRQ_TYPE_UNKNOWN sentinel; use zero
to indicate no pending interrupts, which simplifies code and
facilitates future extension for additional interrupt types.
This prepares subsequent changes to pass specific irq flags into
mute/unmute helpers for selective interrupt control.
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
.../gpu/drm/bridge/analogix/analogix_dp_core.c | 8 ++++----
.../gpu/drm/bridge/analogix/analogix_dp_core.h | 4 ++--
.../gpu/drm/bridge/analogix/analogix_dp_reg.c | 18 ++++++++++--------
3 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index d414f4ff40c8..c04af9fd4092 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -704,10 +704,10 @@ static irqreturn_t analogix_dp_hardirq(int irq, void *arg)
{
struct analogix_dp_device *dp = arg;
irqreturn_t ret = IRQ_NONE;
- enum dp_irq_type irq_type;
+ u32 irq_type;
irq_type = analogix_dp_get_irq_type(dp);
- if (irq_type != DP_IRQ_TYPE_UNKNOWN) {
+ if (irq_type) {
analogix_dp_mute_hpd_interrupt(dp);
ret = IRQ_WAKE_THREAD;
}
@@ -718,7 +718,7 @@ static irqreturn_t analogix_dp_hardirq(int irq, void *arg)
static irqreturn_t analogix_dp_irq_thread(int irq, void *arg)
{
struct analogix_dp_device *dp = arg;
- enum dp_irq_type irq_type;
+ u32 irq_type;
irq_type = analogix_dp_get_irq_type(dp);
if (irq_type & DP_IRQ_TYPE_HP_CABLE_IN ||
@@ -728,7 +728,7 @@ static irqreturn_t analogix_dp_irq_thread(int irq, void *arg)
drm_helper_hpd_irq_event(dp->drm_dev);
}
- if (irq_type != DP_IRQ_TYPE_UNKNOWN) {
+ if (irq_type) {
analogix_dp_clear_hotplug_interrupts(dp);
analogix_dp_unmute_hpd_interrupt(dp);
}
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index c7997677a286..c2eba77f9a81 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -120,7 +120,7 @@ enum dp_irq_type {
DP_IRQ_TYPE_HP_CABLE_IN = BIT(0),
DP_IRQ_TYPE_HP_CABLE_OUT = BIT(1),
DP_IRQ_TYPE_HP_CHANGE = BIT(2),
- DP_IRQ_TYPE_UNKNOWN = BIT(3),
+ DP_IRQ_TYPE_IRQ_HPD = BIT(3),
};
struct video_info {
@@ -193,7 +193,7 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
int analogix_dp_init_analog_func(struct analogix_dp_device *dp);
void analogix_dp_init_hpd(struct analogix_dp_device *dp);
void analogix_dp_force_hpd(struct analogix_dp_device *dp);
-enum dp_irq_type analogix_dp_get_irq_type(struct analogix_dp_device *dp);
+u32 analogix_dp_get_irq_type(struct analogix_dp_device *dp);
void analogix_dp_clear_hotplug_interrupts(struct analogix_dp_device *dp);
void analogix_dp_reset_aux(struct analogix_dp_device *dp);
void analogix_dp_init_aux(struct analogix_dp_device *dp);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index ea8401293a23..f4f859cb2936 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -413,8 +413,9 @@ void analogix_dp_force_hpd(struct analogix_dp_device *dp)
writel(reg, dp->reg_base + ANALOGIX_DP_SYS_CTL_3);
}
-enum dp_irq_type analogix_dp_get_irq_type(struct analogix_dp_device *dp)
+u32 analogix_dp_get_irq_type(struct analogix_dp_device *dp)
{
+ u32 irq_type = 0;
u32 reg;
if (dp->hpd_gpiod) {
@@ -426,17 +427,18 @@ enum dp_irq_type analogix_dp_get_irq_type(struct analogix_dp_device *dp)
} else {
/* Parse hotplug interrupt status register */
reg = readl(dp->reg_base + ANALOGIX_DP_COMMON_INT_STA_4);
-
if (reg & PLUG)
- return DP_IRQ_TYPE_HP_CABLE_IN;
-
+ irq_type |= DP_IRQ_TYPE_HP_CABLE_IN;
if (reg & HPD_LOST)
- return DP_IRQ_TYPE_HP_CABLE_OUT;
-
+ irq_type |= DP_IRQ_TYPE_HP_CABLE_OUT;
if (reg & HOTPLUG_CHG)
- return DP_IRQ_TYPE_HP_CHANGE;
+ irq_type |= DP_IRQ_TYPE_HP_CHANGE;
+
+ reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
+ if (reg & INT_HPD)
+ irq_type |= DP_IRQ_TYPE_IRQ_HPD;
- return DP_IRQ_TYPE_UNKNOWN;
+ return irq_type;
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v1 3/7] drm/bridge: analogix_dp: Extend mute/unmute HPD interrupts to accept irq bitmask
2026-07-30 3:27 [PATCH v1 0/7] Add HPD support for Rockchip Analogix DP Damon Ding
2026-07-30 3:27 ` [PATCH v1 1/7] drm/bridge: analogix_dp: Manage pm runtime and IRQ for native HPD pin detection Damon Ding
2026-07-30 3:27 ` [PATCH v1 2/7] drm/bridge: analogix_dp: Return bitmask from analogix_dp_get_irq_type() Damon Ding
@ 2026-07-30 3:27 ` Damon Ding
2026-07-30 3:41 ` sashiko-bot
2026-07-30 3:27 ` [PATCH v1 5/7] drm/bridge: analogix_dp: Simplify analogix_dp_config_interrupt() Damon Ding
` (2 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Damon Ding @ 2026-07-30 3:27 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Heiko Stuebner, Andy Yan
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
Dmitry Baryshkov, Marek Szyprowski, Sebastian Reichel, dri-devel,
devicetree, linux-kernel, linux-rockchip, linux-arm-kernel,
Damon Ding
Update the global HPD interrupt mute/unmute routines to accept an IRQ
bitmask parameter. Instead of masking and restoring all HPD interrupts
unconditionally, only operate on flags triggered by the current IRQ
event.
Pass the detected interrupt bitmask from hardirq and thread handler
into the updated per-event interrupt control helpers.
This implements fine-grained per-interrupt masking logic, eliminates
the limitations of global interrupt mute, and prepares subsequent
improvements for accurate HPD event handling.
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
.../drm/bridge/analogix/analogix_dp_core.c | 4 +-
.../drm/bridge/analogix/analogix_dp_core.h | 4 +-
.../gpu/drm/bridge/analogix/analogix_dp_reg.c | 62 ++++++++++++++-----
3 files changed, 51 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index c04af9fd4092..1671f388ef30 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -708,7 +708,7 @@ static irqreturn_t analogix_dp_hardirq(int irq, void *arg)
irq_type = analogix_dp_get_irq_type(dp);
if (irq_type) {
- analogix_dp_mute_hpd_interrupt(dp);
+ analogix_dp_mute_hpd_interrupt(dp, irq_type);
ret = IRQ_WAKE_THREAD;
}
@@ -730,7 +730,7 @@ static irqreturn_t analogix_dp_irq_thread(int irq, void *arg)
if (irq_type) {
analogix_dp_clear_hotplug_interrupts(dp);
- analogix_dp_unmute_hpd_interrupt(dp);
+ analogix_dp_unmute_hpd_interrupt(dp, irq_type);
}
return IRQ_HANDLED;
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index c2eba77f9a81..bc13ae45be69 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -183,8 +183,8 @@ void analogix_dp_init_interrupt(struct analogix_dp_device *dp);
void analogix_dp_reset(struct analogix_dp_device *dp);
void analogix_dp_swreset(struct analogix_dp_device *dp);
void analogix_dp_config_interrupt(struct analogix_dp_device *dp);
-void analogix_dp_mute_hpd_interrupt(struct analogix_dp_device *dp);
-void analogix_dp_unmute_hpd_interrupt(struct analogix_dp_device *dp);
+void analogix_dp_mute_hpd_interrupt(struct analogix_dp_device *dp, u32 irq_type);
+void analogix_dp_unmute_hpd_interrupt(struct analogix_dp_device *dp, u32 irq_type);
int analogix_dp_wait_pll_locked(struct analogix_dp_device *dp);
void analogix_dp_set_pll_power_down(struct analogix_dp_device *dp, bool enable);
void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index f4f859cb2936..42c1da160ad0 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -24,6 +24,9 @@
#define COMMON_INT_MASK_4 (HOTPLUG_CHG | HPD_LOST | PLUG)
#define INT_STA_MASK INT_HPD
+#define COMMON_INT_4_HPD_IRQ (DP_IRQ_TYPE_HP_CABLE_IN | DP_IRQ_TYPE_HP_CABLE_OUT | \
+ DP_IRQ_TYPE_HP_CHANGE)
+
void analogix_dp_enable_video_mute(struct analogix_dp_device *dp, bool enable)
{
u32 reg;
@@ -192,30 +195,59 @@ void analogix_dp_config_interrupt(struct analogix_dp_device *dp)
writel(reg, dp->reg_base + ANALOGIX_DP_INT_STA_MASK);
}
-void analogix_dp_mute_hpd_interrupt(struct analogix_dp_device *dp)
+void analogix_dp_mute_hpd_interrupt(struct analogix_dp_device *dp, u32 irq_type)
{
- u32 reg;
+ u32 reg, mask = 0;
- /* 0: mask, 1: unmask */
- reg = readl(dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_4);
- reg &= ~COMMON_INT_MASK_4;
- writel(reg, dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_4);
+ if (!irq_type)
+ return;
- reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA_MASK);
- reg &= ~INT_STA_MASK;
- writel(reg, dp->reg_base + ANALOGIX_DP_INT_STA_MASK);
+ if (irq_type & COMMON_INT_4_HPD_IRQ) {
+ /* 0: mask, 1: unmask */
+ reg = readl(dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_4);
+ if (irq_type & DP_IRQ_TYPE_HP_CABLE_IN)
+ mask |= PLUG;
+ if (irq_type & DP_IRQ_TYPE_HP_CABLE_OUT)
+ mask |= HPD_LOST;
+ if (irq_type & DP_IRQ_TYPE_HP_CHANGE)
+ mask |= HOTPLUG_CHG;
+ reg &= ~mask;
+ writel(reg, dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_4);
+ }
+
+ if (irq_type & DP_IRQ_TYPE_IRQ_HPD) {
+ /* 0: mask, 1: unmask */
+ reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA_MASK);
+ reg &= ~INT_HPD;
+ writel(reg, dp->reg_base + ANALOGIX_DP_INT_STA_MASK);
+ }
}
-void analogix_dp_unmute_hpd_interrupt(struct analogix_dp_device *dp)
+void analogix_dp_unmute_hpd_interrupt(struct analogix_dp_device *dp, u32 irq_type)
{
u32 reg;
- /* 0: mask, 1: unmask */
- reg = COMMON_INT_MASK_4;
- writel(reg, dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_4);
+ if (!irq_type)
+ return;
- reg = INT_STA_MASK;
- writel(reg, dp->reg_base + ANALOGIX_DP_INT_STA_MASK);
+ if (irq_type & COMMON_INT_4_HPD_IRQ) {
+ /* 0: mask, 1: unmask */
+ reg = readl(dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_4);
+ if (irq_type & DP_IRQ_TYPE_HP_CABLE_IN)
+ reg |= PLUG;
+ if (irq_type & DP_IRQ_TYPE_HP_CABLE_OUT)
+ reg |= HPD_LOST;
+ if (irq_type & DP_IRQ_TYPE_HP_CHANGE)
+ reg |= HOTPLUG_CHG;
+ writel(reg, dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_4);
+ }
+
+ if (irq_type & DP_IRQ_TYPE_IRQ_HPD) {
+ /* 0: mask, 1: unmask */
+ reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA_MASK);
+ reg |= INT_HPD;
+ writel(reg, dp->reg_base + ANALOGIX_DP_INT_STA_MASK);
+ }
}
int analogix_dp_wait_pll_locked(struct analogix_dp_device *dp)
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v1 5/7] drm/bridge: analogix_dp: Simplify analogix_dp_config_interrupt()
2026-07-30 3:27 [PATCH v1 0/7] Add HPD support for Rockchip Analogix DP Damon Ding
` (2 preceding siblings ...)
2026-07-30 3:27 ` [PATCH v1 3/7] drm/bridge: analogix_dp: Extend mute/unmute HPD interrupts to accept irq bitmask Damon Ding
@ 2026-07-30 3:27 ` Damon Ding
2026-07-30 3:27 ` [PATCH v1 6/7] drm/bridge: analogix_dp: Use platform-specific HPD detection scheme Damon Ding
[not found] ` <20260730032744.381566-5-damon.ding@rock-chips.com>
5 siblings, 0 replies; 11+ messages in thread
From: Damon Ding @ 2026-07-30 3:27 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Heiko Stuebner, Andy Yan
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
Dmitry Baryshkov, Marek Szyprowski, Sebastian Reichel, dri-devel,
devicetree, linux-kernel, linux-rockchip, linux-arm-kernel,
Damon Ding
Remove local COMMON_INT_MASK and INT_STA_MASK macros. These constants
are only used once inside this function and bring no reuse benefit.
Replace open-coded register writes for COMMON_INT_MASK_1~3 with
direct writel(0) calls.
Leverage analogix_dp_unmute_hpd_interrupt() using the full HPD_IRQ
mask to initialize HPD interrupt state, removing duplicated register
handling.
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
.../gpu/drm/bridge/analogix/analogix_dp_reg.c | 25 +++----------------
1 file changed, 4 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index 816be79a8da6..42463e18f392 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -18,12 +18,6 @@
#include "analogix_dp_core.h"
#include "analogix_dp_reg.h"
-#define COMMON_INT_MASK_1 0
-#define COMMON_INT_MASK_2 0
-#define COMMON_INT_MASK_3 0
-#define COMMON_INT_MASK_4 (HOTPLUG_CHG | HPD_LOST | PLUG)
-#define INT_STA_MASK INT_HPD
-
#define HPD_IRQ (DP_IRQ_TYPE_HP_CABLE_IN | DP_IRQ_TYPE_HP_CABLE_OUT | \
DP_IRQ_TYPE_HP_CHANGE | DP_IRQ_TYPE_IRQ_HPD)
#define COMMON_INT_4_HPD_IRQ (DP_IRQ_TYPE_HP_CABLE_IN | DP_IRQ_TYPE_HP_CABLE_OUT | \
@@ -178,23 +172,12 @@ void analogix_dp_swreset(struct analogix_dp_device *dp)
void analogix_dp_config_interrupt(struct analogix_dp_device *dp)
{
- u32 reg;
-
/* 0: mask, 1: unmask */
- reg = COMMON_INT_MASK_1;
- writel(reg, dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_1);
-
- reg = COMMON_INT_MASK_2;
- writel(reg, dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_2);
-
- reg = COMMON_INT_MASK_3;
- writel(reg, dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_3);
-
- reg = COMMON_INT_MASK_4;
- writel(reg, dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_4);
+ writel(0, dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_1);
+ writel(0, dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_2);
+ writel(0, dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_3);
- reg = INT_STA_MASK;
- writel(reg, dp->reg_base + ANALOGIX_DP_INT_STA_MASK);
+ analogix_dp_unmute_hpd_interrupt(dp, HPD_IRQ);
}
void analogix_dp_mute_hpd_interrupt(struct analogix_dp_device *dp, u32 irq_type)
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v1 6/7] drm/bridge: analogix_dp: Use platform-specific HPD detection scheme
2026-07-30 3:27 [PATCH v1 0/7] Add HPD support for Rockchip Analogix DP Damon Ding
` (3 preceding siblings ...)
2026-07-30 3:27 ` [PATCH v1 5/7] drm/bridge: analogix_dp: Simplify analogix_dp_config_interrupt() Damon Ding
@ 2026-07-30 3:27 ` Damon Ding
2026-07-30 3:41 ` sashiko-bot
[not found] ` <20260730032744.381566-5-damon.ding@rock-chips.com>
5 siblings, 1 reply; 11+ messages in thread
From: Damon Ding @ 2026-07-30 3:27 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Heiko Stuebner, Andy Yan
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
Dmitry Baryshkov, Marek Szyprowski, Sebastian Reichel, dri-devel,
devicetree, linux-kernel, linux-rockchip, linux-arm-kernel,
Damon Ding
Hotplug detection can be implemented either via HOTPLUG_CHG interrupt,
or the combination of PLUG and HPD_LOST interrupts.
For Rockchip platforms, configure HPD deglitch to 2ms and rely on
HOTPLUG_CHG interrupt for hotplug events, which is verified as the
optimal solution through engineering tests. Other platforms continue
using PLUG + HPD_LOST pair.
Adjust analogix_dp_config_interrupt() to apply platform-specific
interrupt masking and deglitch settings. Update threaded irq handler
to check corresponding interrupt flags according to platform type.
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
.../gpu/drm/bridge/analogix/analogix_dp_core.c | 9 +++++++--
.../gpu/drm/bridge/analogix/analogix_dp_reg.c | 16 +++++++++++++++-
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 5af4150f0e7e..a292bc881b6b 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -719,10 +719,15 @@ 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 & 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);
+ 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 42463e18f392..981ce3810e90 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -177,7 +177,21 @@ void analogix_dp_config_interrupt(struct analogix_dp_device *dp)
writel(0, dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_2);
writel(0, dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_3);
- analogix_dp_unmute_hpd_interrupt(dp, HPD_IRQ);
+ if (analogix_dp_is_rockchip(dp->plat_data->dev_type)) {
+ /*
+ * Either HOTPLUG_CHG interrupt or PLUG + HPD_LOST interrupt
+ * pair can be used to implement hotplug detection.
+ *
+ * On Rockchip platforms, configuring HPD deglitch to 2ms and
+ * using HOTPLUG_CHG interrupt for hotplug detection is proven
+ * as a better solution via engineering verification.
+ */
+ writel(0x80, dp->reg_base + ANALOGIX_DP_HPD_DEGLITCH_L);
+ writel(0xbb, dp->reg_base + ANALOGIX_DP_HPD_DEGLITCH_H);
+ analogix_dp_unmute_hpd_interrupt(dp, DP_IRQ_TYPE_HP_CHANGE);
+ } else {
+ analogix_dp_unmute_hpd_interrupt(dp, HPD_IRQ);
+ }
}
void analogix_dp_mute_hpd_interrupt(struct analogix_dp_device *dp, u32 irq_type)
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v1 4/7] drm/bridge: analogix_dp: Extend clear_hotplug_interrupts to accept IRQ bitmask
[not found] ` <20260730032744.381566-5-damon.ding@rock-chips.com>
@ 2026-07-30 3:38 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-30 3:38 UTC (permalink / raw)
To: Damon Ding; +Cc: dri-devel, conor+dt, robh, devicetree
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/7] drm/bridge: analogix_dp: Manage pm runtime and IRQ for native HPD pin detection
2026-07-30 3:27 ` [PATCH v1 1/7] drm/bridge: analogix_dp: Manage pm runtime and IRQ for native HPD pin detection Damon Ding
@ 2026-07-30 3:39 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-30 3:39 UTC (permalink / raw)
To: Damon Ding; +Cc: devicetree, dri-devel, conor+dt, robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The HPD IRQ handler unconditionally accesses unclocked DP controller registers during runtime suspend on GPIO HPD and non-Rockchip configurations, causing a synchronous external abort (system crash).
--
--- Patch [1]: [PATCH v1 1/7] drm/bridge: analogix_dp: Manage pm runtime and IRQ for native HPD pin detection ---
commit 0b74430c48be5d81c8cb19f2cd5f520d6c7a696a
Author: Damon Ding <damon.ding@rock-chips.com>
drm/bridge: analogix_dp: Manage pm runtime and IRQ for native HPD pin detection
This commit moves enable_irq() and disable_irq() from bridge enable/disable
handlers to analogix_dp_bind() and analogix_dp_unbind() to keep HPD detection
active. It also acquires a runtime PM reference exclusively for Rockchip
native HPD pin configurations to ensure the controller remains powered.
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 30c0c0b41d5aa..1c98f92e001d8 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
[ ... ]
> @@ -1508,6 +1511,14 @@ int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
> goto err_unregister_aux;
> }
>
> + if (analogix_dp_require_pm_for_hpd_irq(dp)) {
> + ret = pm_runtime_resume_and_get(dp->dev);
> + if (ret)
> + goto err_unregister_aux;
> + }
> +
> + enable_irq(dp->irq);
> +
[Severity: High]
Since pm_runtime_resume_and_get() is only called for Rockchip native HPD,
other configurations (like GPIO HPD or Exynos) can enter runtime suspend
while the IRQ remains enabled here.
If the HPD interrupt fires (e.g. via an active GPIO pin) while the device is
in runtime suspend, will this cause a crash?
The hardirq handler executes and performs unconditional MMIO accesses to the
unclocked DP controller:
analogix_dp_hardirq()
analogix_dp_mute_hpd_interrupt()
readl(dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_4);
Accessing memory-mapped registers of a clock-gated peripheral typically
causes a synchronous external abort.
> return 0;
>
> err_unregister_aux:
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260730032744.381566-1-damon.ding@rock-chips.com?part=1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 2/7] drm/bridge: analogix_dp: Return bitmask from analogix_dp_get_irq_type()
2026-07-30 3:27 ` [PATCH v1 2/7] drm/bridge: analogix_dp: Return bitmask from analogix_dp_get_irq_type() Damon Ding
@ 2026-07-30 3:41 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-30 3:41 UTC (permalink / raw)
To: Damon Ding; +Cc: dri-devel, robh, devicetree, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional DP register access in hardirq/thread handlers causes bus faults when using GPIO HPD with PM runtime suspended.
--
--- Patch [2]: [PATCH v1 2/7] drm/bridge: analogix_dp: Return bitmask from analogix_dp_get_irq_type() ---
commit 23b7b1cae6716042fa037a50c31f18ad40263d93
Author: Damon Ding <damon.ding@rock-chips.com>
drm/bridge: analogix_dp: Return bitmask from analogix_dp_get_irq_type()
This commit converts analogix_dp_get_irq_type() to return a bitmask of pending
interrupt flags instead of just the first matched flag. It also updates the
handlers to check for a non-zero return value instead of DP_IRQ_TYPE_UNKNOWN.
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 1c98f92e001d8..ccb8a374b40b7 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -708,10 +708,10 @@ static irqreturn_t analogix_dp_hardirq(int irq, void *arg)
> {
[ ... ]
> irq_type = analogix_dp_get_irq_type(dp);
> - if (irq_type != DP_IRQ_TYPE_UNKNOWN) {
> + if (irq_type) {
> analogix_dp_mute_hpd_interrupt(dp);
[Severity: High]
Could this unconditional call to analogix_dp_mute_hpd_interrupt() cause a
regression (bus fault) when using GPIO HPD while the device is PM runtime
suspended?
Since an earlier patch in this series moved enable_irq() to
analogix_dp_bind() without keeping the device PM resumed for GPIO HPD
configurations, a GPIO interrupt can fire while the DP controller is suspended.
If that happens, analogix_dp_mute_hpd_interrupt() will directly read/write
MMIO registers (like ANALOGIX_DP_COMMON_INT_MASK_4) on an unclocked or
powered-off hardware block.
> ret = IRQ_WAKE_THREAD;
> }
[ ... ]
> @@ -722,7 +722,7 @@ static irqreturn_t analogix_dp_hardirq(int irq, void *arg)
> static irqreturn_t analogix_dp_irq_thread(int irq, void *arg)
> {
[ ... ]
> - if (irq_type != DP_IRQ_TYPE_UNKNOWN) {
> + if (irq_type) {
> analogix_dp_clear_hotplug_interrupts(dp);
> analogix_dp_unmute_hpd_interrupt(dp);
[Severity: High]
Similarly, could these function calls cause a regression for the same reason
if called during a GPIO HPD event while PM runtime suspended?
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260730032744.381566-1-damon.ding@rock-chips.com?part=2
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 3/7] drm/bridge: analogix_dp: Extend mute/unmute HPD interrupts to accept irq bitmask
2026-07-30 3:27 ` [PATCH v1 3/7] drm/bridge: analogix_dp: Extend mute/unmute HPD interrupts to accept irq bitmask Damon Ding
@ 2026-07-30 3:41 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-30 3:41 UTC (permalink / raw)
To: Damon Ding; +Cc: dri-devel, conor+dt, robh, devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Selective unmasking combined with unconditional status clearing permanently masks concurrently triggered hotplug interrupts, causing lost events.
- [High] Accessing DP controller mask registers in mute/unmute functions when using GPIO HPD causes a bus hang during runtime suspend.
--
--- Patch [3]: [PATCH v1 3/7] drm/bridge: analogix_dp: Extend mute/unmute HPD interrupts to accept irq bitmask ---
commit 09d81009e86378eb20f782e0e10f313daaa0f8d2
Author: Damon Ding <damon.ding@rock-chips.com>
drm/bridge: analogix_dp: Extend mute/unmute HPD interrupts to accept irq bitmask
This commit updates the global HPD interrupt mute/unmute routines to accept
an IRQ bitmask parameter. It passes the bitmask from the hardirq and thread
handlers to selectively mask/unmask flags triggered by the current IRQ event,
preparing for subsequent accurate HPD event handling.
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index ccb8a374b40b7..a3dc5eac91b8f 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
[ ... ]
> @@ -734,7 +734,7 @@ static irqreturn_t analogix_dp_irq_thread(int irq, void *arg)
>
> if (irq_type) {
> analogix_dp_clear_hotplug_interrupts(dp);
> - analogix_dp_unmute_hpd_interrupt(dp);
> + analogix_dp_unmute_hpd_interrupt(dp, irq_type);
[Severity: High]
Could this selective unmasking combined with the unconditional status clearing
permanently mask concurrently triggered hotplug interrupts?
Since analogix_dp_irq_thread() samples irq_type earlier via
analogix_dp_get_irq_type(), what happens if a new native hotplug interrupt
(like HOTPLUG_CHG) fires immediately after that read?
The hardirq would preempt the thread, read the updated status, and mask the
new interrupt. When the thread resumes, analogix_dp_clear_hotplug_interrupts()
unconditionally clears all hotplug status bits:
reg = HOTPLUG_CHG | HPD_LOST | PLUG;
writel(reg, dp->reg_base + ANALOGIX_DP_COMMON_INT_STA_4);
destroying the new interrupt's status. Then, this selective
analogix_dp_unmute_hpd_interrupt() call uses the stale irq_type snapshot that
lacks the new interrupt, leaving the newly triggered interrupt permanently
masked in ANALOGIX_DP_COMMON_INT_MASK_4 and lost until a driver reload.
> }
>
> return IRQ_HANDLED;
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index f4f859cb29366..42c1da160ad02 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
[ ... ]
> -void analogix_dp_mute_hpd_interrupt(struct analogix_dp_device *dp)
> +void analogix_dp_mute_hpd_interrupt(struct analogix_dp_device *dp, u32 irq_type)
> {
> - u32 reg;
> + u32 reg, mask = 0;
>
> - /* 0: mask, 1: unmask */
> - reg = readl(dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_4);
> - reg &= ~COMMON_INT_MASK_4;
> - writel(reg, dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_4);
> + if (!irq_type)
> + return;
>
> - reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA_MASK);
> - reg &= ~INT_STA_MASK;
> - writel(reg, dp->reg_base + ANALOGIX_DP_INT_STA_MASK);
> + if (irq_type & COMMON_INT_4_HPD_IRQ) {
> + /* 0: mask, 1: unmask */
> + reg = readl(dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_4);
[Severity: High]
Does this code perform an unprotected MMIO read on the DP controller when
using a GPIO for HPD?
If dp->hpd_gpiod is active, the DP controller might be in runtime suspend
(with power/clocks off) while the GPIO HPD IRQ remains enabled. A cable plug
would trigger the GPIO interrupt, and the call to
analogix_dp_mute_hpd_interrupt() in analogix_dp_hardirq() will attempt to
read the DP controller's mask registers here, potentially causing a bus hang.
> + if (irq_type & DP_IRQ_TYPE_HP_CABLE_IN)
> + mask |= PLUG;
> + if (irq_type & DP_IRQ_TYPE_HP_CABLE_OUT)
> + mask |= HPD_LOST;
> + if (irq_type & DP_IRQ_TYPE_HP_CHANGE)
> + mask |= HOTPLUG_CHG;
> + reg &= ~mask;
> + writel(reg, dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_4);
> + }
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260730032744.381566-1-damon.ding@rock-chips.com?part=3
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 6/7] drm/bridge: analogix_dp: Use platform-specific HPD detection scheme
2026-07-30 3:27 ` [PATCH v1 6/7] drm/bridge: analogix_dp: Use platform-specific HPD detection scheme Damon Ding
@ 2026-07-30 3:41 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-30 3:41 UTC (permalink / raw)
To: Damon Ding; +Cc: devicetree, robh, conor+dt, dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] DisplayPort short HPD pulses are completely filtered out by the 2ms hardware deglitch filter, breaking link loss detection and PSR.
- [High] Hotplug interrupt unmasking logic unintentionally re-enables PLUG and HPD_LOST interrupts on Rockchip platforms, defeating the patch's purpose.
--
--- Patch [6]: [PATCH v1 6/7] drm/bridge: analogix_dp: Use platform-specific HPD detection scheme ---
commit af35433f355c1427bbfdea612a689613337f11ab
Author: Damon Ding <damon.ding@rock-chips.com>
Date: Thu Jul 30 03:27:43 2026 +0000
drm/bridge: analogix_dp: Use platform-specific HPD detection scheme
This commit configures the HPD deglitch filter to 2ms on Rockchip platforms
and updates the interrupt handling to rely exclusively on the HOTPLUG_CHG
interrupt instead of the PLUG and HPD_LOST combination. The threaded IRQ
handler is also adjusted to evaluate platform-specific interrupt flags.
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 9f6ecaa1aeb5e..b7aa10daae2f2 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -723,10 +723,15 @@ 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 & 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);
> + if (hpd_detected) {
[Severity: High]
Could this logic inadvertently re-enable the PLUG and HPD_LOST interrupts
that were intentionally masked during initialization?
When analogix_dp_get_irq_type() is called, it reads the raw interrupt
status register without checking against the active mask:
analogix_dp_get_irq_type() {
...
reg = readl(dp->reg_base + ANALOGIX_DP_COMMON_INT_STA_4);
if (reg & PLUG)
irq_type |= DP_IRQ_TYPE_HP_CABLE_IN;
if (reg & HPD_LOST)
irq_type |= DP_IRQ_TYPE_HP_CABLE_OUT;
...
}
If the hardware continues to set the PLUG and HPD_LOST status bits on cable
events even while they are masked, analogix_dp_get_irq_type() will still
return them in irq_type.
Later in analogix_dp_irq_thread(), this irq_type mask is passed to
analogix_dp_unmute_hpd_interrupt():
if (!dp->hpd_gpiod && irq_type) {
analogix_dp_clear_hotplug_interrupts(dp, irq_type);
analogix_dp_unmute_hpd_interrupt(dp, irq_type);
}
Would passing the unintentionally detected PLUG and HPD_LOST bits into
analogix_dp_unmute_hpd_interrupt() permanently unmask those interrupts,
defeating the platform-specific interrupt scheme on the very first event?
Additionally, because DP_IRQ_TYPE_IRQ_HPD is no longer included in the
hpd_detected calculation for Rockchip, will this prevent
drm_helper_hpd_irq_event() from being called to process link retraining
and Panel Self Refresh (PSR) events?
> 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..981ce3810e90d 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
[ ... ]
> @@ -177,7 +177,21 @@ void analogix_dp_config_interrupt(struct analogix_dp_device *dp)
> writel(0, dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_2);
> writel(0, dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_3);
>
> - analogix_dp_unmute_hpd_interrupt(dp, HPD_IRQ);
> + if (analogix_dp_is_rockchip(dp->plat_data->dev_type)) {
> + /*
> + * Either HOTPLUG_CHG interrupt or PLUG + HPD_LOST interrupt
> + * pair can be used to implement hotplug detection.
> + *
> + * On Rockchip platforms, configuring HPD deglitch to 2ms and
> + * using HOTPLUG_CHG interrupt for hotplug detection is proven
> + * as a better solution via engineering verification.
> + */
> + writel(0x80, dp->reg_base + ANALOGIX_DP_HPD_DEGLITCH_L);
> + writel(0xbb, dp->reg_base + ANALOGIX_DP_HPD_DEGLITCH_H);
[Severity: High]
Does configuring the deglitch filter to 2ms risk filtering out valid
DisplayPort short HPD pulses?
The DP specification defines short HPD pulses (which are used for sink
events like PSR and link retraining) as a low pulse lasting between 0.5ms
and 1.0ms.
Because the new 2ms filter configuration exceeds the maximum short pulse
duration of 1.0ms, will the hardware treat all valid DP short pulses as
noise and silently drop them before they can trigger an interrupt?
> + analogix_dp_unmute_hpd_interrupt(dp, DP_IRQ_TYPE_HP_CHANGE);
> + } else {
> + analogix_dp_unmute_hpd_interrupt(dp, HPD_IRQ);
> + }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260730032744.381566-1-damon.ding@rock-chips.com?part=6
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-07-30 6:57 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-30 3:27 [PATCH v1 0/7] Add HPD support for Rockchip Analogix DP Damon Ding
2026-07-30 3:27 ` [PATCH v1 1/7] drm/bridge: analogix_dp: Manage pm runtime and IRQ for native HPD pin detection Damon Ding
2026-07-30 3:39 ` sashiko-bot
2026-07-30 3:27 ` [PATCH v1 2/7] drm/bridge: analogix_dp: Return bitmask from analogix_dp_get_irq_type() Damon Ding
2026-07-30 3:41 ` sashiko-bot
2026-07-30 3:27 ` [PATCH v1 3/7] drm/bridge: analogix_dp: Extend mute/unmute HPD interrupts to accept irq bitmask Damon Ding
2026-07-30 3:41 ` sashiko-bot
2026-07-30 3:27 ` [PATCH v1 5/7] drm/bridge: analogix_dp: Simplify analogix_dp_config_interrupt() Damon Ding
2026-07-30 3:27 ` [PATCH v1 6/7] drm/bridge: analogix_dp: Use platform-specific HPD detection scheme Damon Ding
2026-07-30 3:41 ` sashiko-bot
[not found] ` <20260730032744.381566-5-damon.ding@rock-chips.com>
2026-07-30 3:38 ` [PATCH v1 4/7] drm/bridge: analogix_dp: Extend clear_hotplug_interrupts to accept IRQ bitmask sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox