Simon Ser
2023-Jun-20 18:15 UTC
[Nouveau] [PATCH] drm/nouveau/disp: use drm_kms_helper_connector_hotplug_event()
This adds more information to the hotplug uevent and lets user-space
know that it's about a particular connector only.
Signed-off-by: Simon Ser <contact at emersion.fr>
Cc: Ben Skeggs <bskeggs at redhat.com>
Cc: Lyude Paul <lyude at redhat.com>
Cc: David Airlie <airlied at linux.ie>
Cc: Karol Herbst <kherbst at redhat.com>
---
drivers/gpu/drm/nouveau/nouveau_display.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c
b/drivers/gpu/drm/nouveau/nouveau_display.c
index ec3ffff487fc..99977e5fe716 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -465,7 +465,8 @@ nouveau_display_hpd_work(struct work_struct *work)
struct drm_connector *connector;
struct drm_connector_list_iter conn_iter;
u32 pending;
- bool changed = false;
+ int changed = 0;
+ struct drm_connector *first_changed_connector = NULL;
pm_runtime_get_sync(dev->dev);
@@ -509,7 +510,12 @@ nouveau_display_hpd_work(struct work_struct *work)
if (old_epoch_counter == connector->epoch_counter)
continue;
- changed = true;
+ changed++;
+ if (!first_changed_connector) {
+ drm_connector_get(connector);
+ first_changed_connector = connector;
+ }
+
drm_dbg_kms(dev, "[CONNECTOR:%d:%s] status updated from %s to %s (epoch
counter %llu->%llu)\n",
connector->base.id, connector->name,
drm_get_connector_status_name(old_status),
@@ -520,9 +526,14 @@ nouveau_display_hpd_work(struct work_struct *work)
drm_connector_list_iter_end(&conn_iter);
mutex_unlock(&dev->mode_config.mutex);
- if (changed)
+ if (changed == 1)
+ drm_kms_helper_connector_hotplug_event(first_changed_connector);
+ else if (changed > 0)
drm_kms_helper_hotplug_event(dev);
+ if (first_changed_connector)
+ drm_connector_put(first_changed_connector);
+
pm_runtime_mark_last_busy(drm->dev->dev);
noop:
pm_runtime_put_autosuspend(dev->dev);
--
2.41.0
Lyude Paul
2023-Jun-21 21:56 UTC
[Nouveau] [PATCH] drm/nouveau/disp: use drm_kms_helper_connector_hotplug_event()
Looks alright! Some comments below On Tue, 2023-06-20 at 18:15 +0000, Simon Ser wrote:> This adds more information to the hotplug uevent and lets user-space > know that it's about a particular connector only. > > Signed-off-by: Simon Ser <contact at emersion.fr> > Cc: Ben Skeggs <bskeggs at redhat.com> > Cc: Lyude Paul <lyude at redhat.com> > Cc: David Airlie <airlied at linux.ie> > Cc: Karol Herbst <kherbst at redhat.com> > --- > drivers/gpu/drm/nouveau/nouveau_display.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c > index ec3ffff487fc..99977e5fe716 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_display.c > +++ b/drivers/gpu/drm/nouveau/nouveau_display.c > @@ -465,7 +465,8 @@ nouveau_display_hpd_work(struct work_struct *work) > struct drm_connector *connector; > struct drm_connector_list_iter conn_iter; > u32 pending; > - bool changed = false; > + int changed = 0; > + struct drm_connector *first_changed_connector = NULL; > > pm_runtime_get_sync(dev->dev); > > @@ -509,7 +510,12 @@ nouveau_display_hpd_work(struct work_struct *work) > if (old_epoch_counter == connector->epoch_counter) > continue; > > - changed = true; > + changed++; > + if (!first_changed_connector) { > + drm_connector_get(connector); > + first_changed_connector = connector; > + } > + > drm_dbg_kms(dev, "[CONNECTOR:%d:%s] status updated from %s to %s (epoch counter %llu->%llu)\n", > connector->base.id, connector->name, > drm_get_connector_status_name(old_status), > @@ -520,9 +526,14 @@ nouveau_display_hpd_work(struct work_struct *work) > drm_connector_list_iter_end(&conn_iter); > mutex_unlock(&dev->mode_config.mutex); > > - if (changed) > + if (changed == 1) > + drm_kms_helper_connector_hotplug_event(first_changed_connector); > + else if (changed > 0) > drm_kms_helper_hotplug_event(dev);I'm curious if you think there might be an advantage to doing this per- connector even with multiple connectors? Seems like we could do that if we stored changed connectors as a bitmask.> > + if (first_changed_connector) > + drm_connector_put(first_changed_connector); > + > pm_runtime_mark_last_busy(drm->dev->dev); > noop: > pm_runtime_put_autosuspend(dev->dev);-- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat