Re: [PATCH RESEND v7 2/2] drm: Send per-connector hotplug events

Dmitry Baryshkov
2026-04-19T01:02:37Z

On Wed, Apr 15, 2026 at 07:59:40PM +0200, Nicolas Frattaroli wrote:
> From: Marius Vlad <marius.vlad@collabora.com>
>
> Use the new pending_hp member of drm_connector to always send
> per-connector hotplug events for those connectors that need it, rather
> than sending a global event, or only an event for one connector.
>
> On the HPD (Hot Plug Detect) path this change notifies all connectors,
> rather than just first changed connector.

I'd like to point out that there are enough places were the code calls
drm_kms_helper_hotplug_event() directly. Some of them are ridiculous and
can easily be sorted out (e.g. bridges or single connector drivers,
where we know exactly for which connector or bridge to report the HPD
event). Others might be not that obvious. So, if we are to promise to
userspace to always deliver connector-based events, there is more work
to be done (and it probably makes a nice TODO item).

>
> The polling path is changed to no longer send a connector-less hotplug
> event, but similarly send a hotplug event for each changed connector.
>
> Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
> drivers/gpu/drm/drm_probe_helper.c | 22 +++++++++++++++++-----
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index f8cbd6713960..5c39f27ada1d 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -860,8 +860,14 @@ static void output_poll_execute(struct work_struct *work)
> mutex_unlock(&dev->mode_config.mutex);
>
> out:
> - if (changed)
> - drm_kms_helper_hotplug_event(dev);
> + if (changed) {
> + drm_connector_list_iter_begin(dev, &conn_iter);
> + drm_for_each_connector_iter(connector, &conn_iter) {
> + if (connector->pending_hp)
> + drm_kms_helper_connector_hotplug_event(connector);
> + }
> + drm_connector_list_iter_end(&conn_iter);
> + }
>
> if (repoll)
> schedule_delayed_work(delayed_work, DRM_OUTPUT_POLL_PERIOD);
> @@ -1124,10 +1130,16 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
> drm_connector_list_iter_end(&conn_iter);
> mutex_unlock(&dev->mode_config.mutex);
>
> - if (changed == 1)
> + if (changed == 1) {
> drm_kms_helper_connector_hotplug_event(first_changed_connector);
> - else if (changed > 0)
> - drm_kms_helper_hotplug_event(dev);
> + } else if (changed > 0) {
> + drm_connector_list_iter_begin(dev, &conn_iter);
> + drm_for_each_connector_iter(connector, &conn_iter) {
> + if (connector->pending_hp)
> + drm_kms_helper_connector_hotplug_event(connector);

So, instead of sending one HPD event, we can now send a set of events in
a very short time, potentially triggering undesired events in the
userspace. At least this change of te behaviour must be documented in
the commit message and in the cover letter.

> + }
> + drm_connector_list_iter_end(&conn_iter);

Should this still be executed under the mode_config mutex?

> + }
>
> if (first_changed_connector)
> drm_connector_put(first_changed_connector);

With the code in place, is there a point in keeping a special case for a
single connector change?

>
> --
> 2.53.0
>

--
With best wishes
Dmitry