On Tue, Sep 23, 2025 at 11:42:42AM +0300, Marius Vlad wrote: > Hi Dmitry, > > On Sat, Aug 23, 2025 at 12:30:27AM +0300, Dmitry Baryshkov wrote: > > On Tue, Jul 29, 2025 at 07:57:07PM +0300, Marius Vlad wrote: > > > This patch introduces a new boolean variable used to track connector's > > > connect/disconnect status and it is being used on both polling and > > > the HPD (Hot Plug Detect) paths. > > > > > > A subsequent patch would make use of this connector status to propagate > > > per-connector udev hot plug events. This allows user-space to receive > > > the connector's ID, rather than having a generic hot-plug event for all > > > connectors, or in the HPD path, just the first one found with a > > > connection status change. > > > > This looks good, the main question would be, what prevents a races > > during modifications of this field? I think it should be toggled under > > the dev->mode_config.mutex. > Thanks the review (been away for awhile and just saw your replies). > > Sent out a v3 update: > https://lore.kernel.org/dri-devel/[email protected]/ Err, this one points to the correct version: https://lore.kernel.org/dri-devel/[email protected]/T/#t > > > > > > > > Signed-off-by: Marius Vlad <[email protected]> > > > --- > > > drivers/gpu/drm/drm_connector.c | 1 + > > > drivers/gpu/drm/drm_probe_helper.c | 12 ++++++++++++ > > > drivers/gpu/drm/drm_sysfs.c | 1 + > > > include/drm/drm_connector.h | 3 +++ > > > 4 files changed, 17 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_connector.c > > > b/drivers/gpu/drm/drm_connector.c > > > index 272d6254ea47..3c6628ee3096 100644 > > > --- a/drivers/gpu/drm/drm_connector.c > > > +++ b/drivers/gpu/drm/drm_connector.c > > > @@ -274,6 +274,7 @@ static int drm_connector_init_only(struct drm_device > > > *dev, > > > > > > /* provide ddc symlink in sysfs */ > > > connector->ddc = ddc; > > > + connector->status_changed = false; > > > > > > INIT_LIST_HEAD(&connector->head); > > > INIT_LIST_HEAD(&connector->global_connector_list_entry); > > > diff --git a/drivers/gpu/drm/drm_probe_helper.c > > > b/drivers/gpu/drm/drm_probe_helper.c > > > index 6b3541159c0f..761766181e99 100644 > > > --- a/drivers/gpu/drm/drm_probe_helper.c > > > +++ b/drivers/gpu/drm/drm_probe_helper.c > > > @@ -628,6 +628,7 @@ int drm_helper_probe_single_connector_modes(struct > > > drm_connector *connector, > > > mod_delayed_work(system_wq, > > > &dev->mode_config.output_poll_work, > > > 0); > > > + connector->status_changed = true; > > > } > > > > > > /* > > > @@ -731,6 +732,15 @@ > > > EXPORT_SYMBOL(drm_helper_probe_single_connector_modes); > > > */ > > > void drm_kms_helper_hotplug_event(struct drm_device *dev) > > > { > > > + struct drm_connector *connector; > > > + struct drm_connector_list_iter conn_iter; > > > + > > > + drm_connector_list_iter_begin(dev, &conn_iter); > > > + drm_for_each_connector_iter(connector, &conn_iter) { > > > + connector->status_changed = false; > > > + } > > > + drm_connector_list_iter_end(&conn_iter); > > > + > > > drm_sysfs_hotplug_event(dev); > > > drm_client_dev_hotplug(dev); > > > } > > > @@ -747,6 +757,7 @@ void drm_kms_helper_connector_hotplug_event(struct > > > drm_connector *connector) > > > { > > > struct drm_device *dev = connector->dev; > > > > > > + connector->status_changed = false; > > > drm_sysfs_connector_hotplug_event(connector); > > > drm_client_dev_hotplug(dev); > > > > > > What would be the rule? Should it be unset before or after calling all > > the notifiers? Otherwise it's really strange. In the chunk below you set > > the flag, then it calls this function and the flags gets immediately > > unset. > Yeah, saw that was well. I've removed it entirely and just kept the unset bit > in drm_kms_helper_hotplug_event/drm_kms_helper_connector_hotplug_event. > > > > > } > > > @@ -1041,6 +1052,7 @@ bool drm_connector_helper_hpd_irq_event(struct > > > drm_connector *connector) > > > mutex_unlock(&dev->mode_config.mutex); > > > > > > if (changed) { > > > + connector->status_changed = true; > > > drm_kms_helper_connector_hotplug_event(connector); > > > drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Sent hotplug event\n", > > > connector->base.id, > > > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c > > > index 60c1f26edb6f..77f880654d6a 100644 > > > --- a/drivers/gpu/drm/drm_sysfs.c > > > +++ b/drivers/gpu/drm/drm_sysfs.c > > > @@ -196,6 +196,7 @@ static ssize_t status_store(struct device *device, > > > return ret; > > > > > > old_force = connector->force; > > > + connector->status_changed = true; > > > > > > if (sysfs_streq(buf, "detect")) > > > connector->force = 0; > > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > > > index 8f34f4b8183d..e4310df3d55c 100644 > > > --- a/include/drm/drm_connector.h > > > +++ b/include/drm/drm_connector.h > > > @@ -2146,6 +2146,9 @@ struct drm_connector { > > > /** @force: a DRM_FORCE_<foo> state for forced mode sets */ > > > enum drm_connector_force force; > > > > > > + /** @status_changed: if the old status doesn't match current connection > > > status */ > > > + bool status_changed; > > > + > > > /** > > > * @edid_override: Override EDID set via debugfs. > > > * > > > -- > > > 2.47.2 > > > > > > > -- > > With best wishes > > Dmitry
signature.asc
Description: PGP signature
