Re: [PATCH 1/6] drm/connector: report IRQ_HPD events to drm_connector_oob_hotplug_event()

[Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>]

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.
>>>>> 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@oss.qualcomm.com>
>>>>> ---
>>>>>     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)
>>>> I find the "IRQ HPD" naming always confusing, even if I'm somewhat familiar
>>>> 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 bit
>>> 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.
>> We'd still need to define what exactly it means. I think it might be better
>> 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