On Friday, October 15th, 2021 at 22:03, Sam Ravnborg <sam@xxxxxxxxxxxx> wrote: > 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?!? Ah, it's not just you, this version is much better. Thanks for the suggestion, will do that in the next version!