Hi Simon,
On Fri, Oct 15, 2021 at 04:33:45PM +0000, Simon Ser wrote:
If an hotplug event only updates a single connector, use drm_kms_helper_connector_hotplug_event instead of drm_kms_helper_hotplug_event.
Signed-off-by: Simon Ser contact@emersion.fr
drivers/gpu/drm/drm_probe_helper.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 3aef3b188c99..6049dc92324b 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -927,7 +927,7 @@ EXPORT_SYMBOL(drm_connector_helper_hpd_irq_event); */ bool drm_helper_hpd_irq_event(struct drm_device *dev) {
- struct drm_connector *connector;
- struct drm_connector *connector, *changed_connector = NULL; struct drm_connector_list_iter conn_iter; bool changed = false;
@@ -941,15 +941,27 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev) if (!(connector->polled & DRM_CONNECTOR_POLL_HPD)) continue;
if (check_connector_changed(connector))
if (check_connector_changed(connector)) {if (changed) {if (changed_connector)drm_connector_put(changed_connector);changed_connector = NULL;} else {drm_connector_get(connector);changed_connector = connector;}
This code is a little confusing to read.
In case we have only one connector with a change we hit the else part. What we really want to find out is if we have one or more connectors with a change. We could do something like:
struct drm_connector *changed_connector = NULL; int changes = 0;
...
/* Find if we have one or more changed connectors */ drm_for_each_connector_iter(connector, &conn_iter) { if (!(connector->polled & DRM_CONNECTOR_POLL_HPD)) continue;
if (check_connector_changed(connector)) { if (!changes) { changed_connector = connector; drm_connector_get(changed_connector); }
changes++; } } drm_connector_list_iter_end(&conn_iter); mutex_unlock(&dev->mode_config.mutex);
if (changes == 1) drm_kms_helper_connector_hotplug_event(changed_connector); else if (changes > 1) drm_kms_helper_hotplug_event(dev);
if (changed_connector) drm_connector_put(changed_connector);
Maybe the only reason why I think this is better is bc I wrote it?!?
Sam