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:57:38 EST
Hi,
On 20/04/2026 14:45, Dmitry Baryshkov wrote:
On Mon, Apr 20, 2026 at 02:01:57PM +0300, Tomi Valkeinen wrote:
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?
No, my thought was to be able to define values specific to the
particular connector types and to be able to combine those values.
After sending the email I started thinking about the bridged and
corresponding notifications. There having overlapping values will not
work becasue bridges in the chanin don't easily know the final connector
type.
An enum can have overlapping values. I don't think there's much difference between u32 and an enum in C. I just like enum because 1) it groups the possible values in the header file, and 2) the function parameters can use the enum type, making it obvious what flags you are supposed to use there.
I think you are correct here, it should be the enum. With the first
iteration defined as:
/**
* enum drm_connector_status_extra - additional events sent by the sink
* together or in replacement of the HPD status changes
/
enum drm_connector_status_extra {
/**
* @DRM_CONNECTOR_DP_IRQ_HPD: DisplayPort Sink has sent the
* IRQ_HPD (either by the HPD short pulse or via the AltMode event).
*/
DRM_CONNECTOR_DP_IRQ_HPD = BIT(0),
};
/**
* @extra_status: additional information provided by the sink without
* changing the HPD state (or in addition to such a change). It is an
* OR of the values defined in the drm_connector_status_extra enum.
*/
void drm_connector_oob_hotplug_event(..., u32 extra_status);
Looks good to me, except I'd use "enum drm_connector_status_extra" instead of u32 there in the function parameters.
Tomi