Re: [PATCH 1/6] drm/connector: report IRQ_HPD events to drm_connector_oob_hotplug_event()
From: Tomi Valkeinen
Date: Mon Apr 20 2026 - 07:07:25 EST
Hi,
On 20/04/2026 12:50, Dmitry Baryshkov wrote:
On Mon, Apr 20, 2026 at 07:50:46AM +0300, Tomi Valkeinen wrote:
Hi,
On 18/04/2026 01:32, Dmitry Baryshkov wrote:
On Thu, Apr 16, 2026 at 11:10:03AM +0300, Tomi Valkeinen wrote:
Hi,
On 16/04/2026 02:22, Dmitry Baryshkov wrote:
The DisplayPort standard defines a special kind of events called IRQ.I find the "IRQ HPD" naming always confusing, even if I'm somewhat familiar
These events are used to notify DP Source about the events on the Sink
side. It is extremely important for DP MST handling, where the MST
events are reported through this IRQ.
In case of the USB-C DP AltMode there is no actual HPD pulse, but the
events are ported through the bits in the AltMode VDOs.
Extend the drm_connector_oob_hotplug_event() interface and report IRQ
events to the DisplayPort Sink drivers.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/drm_connector.c | 4 +++-
drivers/usb/typec/altmodes/displayport.c | 12 ++++++++----
include/drm/drm_connector.h | 3 ++-
3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 47dc53c4a738..5fdacbd84bd7 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -3510,6 +3510,7 @@ struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode)
* drm_connector_oob_hotplug_event - Report out-of-band hotplug event to connector
* @connector_fwnode: fwnode_handle to report the event on
* @status: hot plug detect logical state
+ * @irq_hpd: HPD pulse detected
*
* On some hardware a hotplug event notification may come from outside the display
* driver / device. An example of this is some USB Type-C setups where the hardware
@@ -3520,7 +3521,8 @@ struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode)
* a drm_connector reference through calling drm_connector_find_by_fwnode().
*/
void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode,
- enum drm_connector_status status)
+ enum drm_connector_status status,
+ bool irq_hpd)
with DP, but if someone has mainly worked on HDMI, I'm sure it's even worse.
Can we define this a bit more precisely? Is 'irq_hpd' only for displayport?
If so, perhaps 'dp_irq_hpd' or 'displayport_irq_hpd'. I might even call it
'dp_hpd_pulse', but maybe that's not good as the spec talks about HPD pulse
for both short and long ones (although in the kernel doc you just write "HPD
pulse")... The kernel doc could be expanded a bit to make it clear what this
flag indicates.
I attempted to stay away from defining a DP-specific flag, keeping it
generic enough. HDMI is pretty close (IMO) to requiring separate flag in
If it's not specifically the DP IRQ HPD, then we need to define what it
means. I tried to think what it would mean with HDMI, but I didn't come up
with anything.
I might be mistaken, but I had someting like HEAC HPD / EDID status
changes in mind (or HDCP-triggered HPD status changes). But here I
admit, I hadn't checked if it is actually applicable or not.
Possibly, I'm not familiar with those.
Anyway, for e.g. DVI or VGA that means nothing. But, my point really is
to abstain from defining someting as DP-only in the top-level API.
I'm fine with that, but then it really has to be defined =).
Linux. Likewise I'd rather not use "pulse". The DP AltMode defines a bitWe'd still need to define what exactly it means. I think it might be better
in the VDO rather than a pulse.
Anyway, if irq_hpd doesn't sound precise enough, what about "bool
extra_irq"? This would convey that this is the extra hpd-related IRQ,
but it would also be obvious that it's not related to the HPD pin
itself.
to just define it as the DP IRQ HPD, as then the meaning is clear.
Also, would an enum flags parameter be better than a bool parameter?
Maybe not enum, but u32 param. Then it can become:
@extra_status: additional type-specific information provided by the sink
without changing the HPD state
void drm_connector_oob_hotplug_event(..., u32 extra_status);
/* DP short HPD pulse or corresponding AltMode flag */
#define DRM_CONNECTOR_OOB_DP_IRQ_HPD BIT(0)
/* DP long HPD pulse, debounced XXX: do we need this? */
#define DRM_CONNECTOR_OOB_DP_REPLUG BIT(1)
Why is u32 better than enum? So that we could e.g. pass short values inside the extra_status?
For HDMI we might want to define:
/* HDMI 1.4b 8.5, HPD pulse */
#define DRM_CONNECTOR_OOB_HDMI_REPLUG BIT(0)
Or might not, 100ms is long enough for all debouncers.
As I read the spec, there's no usable HPD pulse in HDMI as such. It just means that if HPD is low less than 100ms, it should be ignored, and if it's low more than 100ms, it should be handled. In other words, from spec perspective there's no difference between HPD being low 105ms or five days, there's no upper limit for the "pulse".
Still, we probably want to handle the case where the HPD is low only for a short period, so that we don't do a full disable/enable-cycle. We can interpret it as the same monitor still being connected, we just need to check the EDID again.
But isn't that just a drm_connector_hotplug_event with drm_connector_status staying connected? The callee can see that the connector was connected before, it's connected now, but we got an event, so let's read the EDID again.
Tomi