[Intel-gfx] [PATCH v7 09/11] drm: uevent for connector status change
Daniel Vetter
daniel.vetter at ffwll.ch
Tue May 14 11:12:53 UTC 2019
On Mon, May 13, 2019 at 11:20 PM Lyude Paul <lyude at redhat.com> wrote:
>
> Hi-just wanted to give some general thoughts here.
>
> First off I'm 100% behind the epoch idea, that was one of the ideas I had been
> thinking of proposing here in the first place but probably forgot at some
> point down the road.
>
> A couple of other things:
> * I think it would also probably be good to have events for when connectors
> are added or removed from the system (mainly for MST)
Current uevent + userspace looks at the connector list from
drmGetResources? That should be enough to figure this out I think ...
> * Have we considered having any sort of SYNC event, like what evdev uses for
> signaling the end of a frame of events for input devices?
If we go with just 1 event, then that's the natural sync marker
stating "everything we updated for this connector is now updated". For
more global events the global uevent could serve that role (I guess
this is useful for hotpluggin/unpluggin entire mst chains - we might
want to coalesce these indeed).
I also don't think we need to make this an uapi guarantee, just best
effort kernel implementation - userspace needs to always be able to
deal with an event right after the one it's just processing.
-Daniel
>
> On Fri, 2019-05-10 at 14:12 +0200, Paul Kocialkowski wrote:
> > Hi,
> >
> > On Tue, 2019-05-07 at 21:57 +0530, Ramalingam C wrote:
> > > DRM API for generating uevent for a status changes of connector's
> > > property.
> > >
> > > This uevent will have following details related to the status change:
> > >
> > > HOTPLUG=1, CONNECTOR=<connector_id> and PROPERTY=<property_id>
> > >
> > > Need ACK from this uevent from userspace consumer.
> >
> > So we just had some discussions over on IRC and at about the hotplug
> > issue and came up with similar ideas:
> > https://lists.freedesktop.org/archives/dri-devel/2019-May/217408.html
> >
> > The conclusions of these discussions so far would be to have a more or
> > less fine grain of uevent reporting depending on what happened. The
> > point is that we need to cover different cases:
> > - one or more properties changed;
> > - the connector status changed;
> > - something else about the connector changed (e.g. EDID/modes)
> >
> > For the first case, we can send out:
> > HOTPLUG=1
> > CONNECTOR=<id>
> > PROPERTY=<id>
> >
> > and no reprobe is required.
> >
> > For the second one, something like:
> > HOTPLUG=1
> > CONNECTOR=<id>
> > STATUS=Connected/Disconnected
> >
> > and a connector probe is needed for connected, but not for
> > disconnected;
> >
> > For the third one, we can only indicate the connector:
> > HOTPLUG=1
> > CONNECTOR=<id>
> >
> > and a reprobe of the connector is always needed
> >
> > Then we still have the legacy case:
> > HOTPLUG=1
> >
> > where userspace is expected to reprobe all the connectors.
> >
> > I think this would deserve to be a separate series on its own. So I am
> > proposing to take this one off your plate and come up with another
> > seres implementing this proposal. What do you think?
> >
> > Cheers,
> >
> > Paul
> >
> > > v2:
> > > Minor fixes at KDoc comments [Daniel]
> > > v3:
> > > Check the property is really attached with connector [Daniel]
> > >
> > > Signed-off-by: Ramalingam C <ramalingam.c at intel.com>
> > > Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> > > ---
> > > drivers/gpu/drm/drm_sysfs.c | 35 +++++++++++++++++++++++++++++++++++
> > > include/drm/drm_sysfs.h | 5 ++++-
> > > 2 files changed, 39 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> > > index 18b1ac442997..63fa951a20db 100644
> > > --- a/drivers/gpu/drm/drm_sysfs.c
> > > +++ b/drivers/gpu/drm/drm_sysfs.c
> > > @@ -21,6 +21,7 @@
> > > #include <drm/drm_sysfs.h>
> > > #include <drm/drmP.h>
> > > #include "drm_internal.h"
> > > +#include "drm_crtc_internal.h"
> > >
> > > #define to_drm_minor(d) dev_get_drvdata(d)
> > > #define to_drm_connector(d) dev_get_drvdata(d)
> > > @@ -320,6 +321,9 @@ void drm_sysfs_lease_event(struct drm_device *dev)
> > > * Send a uevent for the DRM device specified by @dev. Currently we only
> > > * set HOTPLUG=1 in the uevent environment, but this could be expanded to
> > > * deal with other types of events.
> > > + *
> > > + * Any new uapi should be using the drm_sysfs_connector_status_event()
> > > + * for uevents on connector status change.
> > > */
> > > void drm_sysfs_hotplug_event(struct drm_device *dev)
> > > {
> > > @@ -332,6 +336,37 @@ void drm_sysfs_hotplug_event(struct drm_device *dev)
> > > }
> > > EXPORT_SYMBOL(drm_sysfs_hotplug_event);
> > >
> > > +/**
> > > + * drm_sysfs_connector_status_event - generate a DRM uevent for connector
> > > + * property status change
> > > + * @connector: connector on which property status changed
> > > + * @property: connector property whoes status changed.
> > > + *
> > > + * Send a uevent for the DRM device specified by @dev. Currently we
> > > + * set HOTPLUG=1 and connector id along with the attached property id
> > > + * related to the status change.
> > > + */
> > > +void drm_sysfs_connector_status_event(struct drm_connector *connector,
> > > + struct drm_property *property)
> > > +{
> > > + struct drm_device *dev = connector->dev;
> > > + char hotplug_str[] = "HOTPLUG=1", conn_id[30], prop_id[30];
> > > + char *envp[4] = { hotplug_str, conn_id, prop_id, NULL };
> > > +
> > > + WARN_ON(!drm_mode_obj_find_prop_id(&connector->base,
> > > + property->base.id));
> > > +
> > > + snprintf(conn_id, ARRAY_SIZE(conn_id),
> > > + "CONNECTOR=%u", connector->base.id);
> > > + snprintf(prop_id, ARRAY_SIZE(prop_id),
> > > + "PROPERTY=%u", property->base.id);
> > > +
> > > + DRM_DEBUG("generating connector status event\n");
> > > +
> > > + kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp);
> > > +}
> > > +EXPORT_SYMBOL(drm_sysfs_connector_status_event);
> > > +
> > > static void drm_sysfs_release(struct device *dev)
> > > {
> > > kfree(dev);
> > > diff --git a/include/drm/drm_sysfs.h b/include/drm/drm_sysfs.h
> > > index 4f311e836cdc..d454ef617b2c 100644
> > > --- a/include/drm/drm_sysfs.h
> > > +++ b/include/drm/drm_sysfs.h
> > > @@ -4,10 +4,13 @@
> > >
> > > struct drm_device;
> > > struct device;
> > > +struct drm_connector;
> > > +struct drm_property;
> > >
> > > int drm_class_device_register(struct device *dev);
> > > void drm_class_device_unregister(struct device *dev);
> > >
> > > void drm_sysfs_hotplug_event(struct drm_device *dev);
> > > -
> > > +void drm_sysfs_connector_status_event(struct drm_connector *connector,
> > > + struct drm_property *property);
> > > #endif
> --
> Cheers,
> Lyude Paul
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the Intel-gfx
mailing list