Re: [PATCH v3 3/6] drm/connector: use drm_sysfs_connector_hotplug_event

[Pekka Paalanen <ppaalanen@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2833 bytes --]

On Wed, 27 Oct 2021 13:26:45 +0000
Simon Ser <contact@emersion.fr> wrote:

> On Wednesday, October 27th, 2021 at 15:15, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> 
> > On Fri, 15 Oct 2021 16:33:43 +0000
> > Simon Ser <contact@emersion.fr> wrote:
> >  
> > > In drm_connector_register, use drm_sysfs_connector_hotplug_event
> > > instead of drm_sysfs_hotplug_event, because the hotplug event
> > > only updates a single connector.
> > >
> > > Signed-off-by: Simon Ser <contact@emersion.fr>
> > > ---
> > >  drivers/gpu/drm/drm_connector.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > index ec3973e8963c..a50c82bc2b2f 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -547,7 +547,7 @@ int drm_connector_register(struct drm_connector *connector)
> > >  	connector->registration_state = DRM_CONNECTOR_REGISTERED;
> > >
> > >  	/* Let userspace know we have a new connector */
> > > -	drm_sysfs_hotplug_event(connector->dev);
> > > +	drm_sysfs_connector_hotplug_event(connector);
> > >
> > >  	if (connector->privacy_screen)
> > >  		drm_privacy_screen_register_notifier(connector->privacy_screen,  
> >
> > Hi Simon,
> >
> > this might not work for Weston if I understand this right. Kernel is
> > adding a new connector, which means userspace does not recognise the
> > connector id in the uevent. Weston as it is right now would ignore the
> > event rather than add the connector.
> >
> > The missing piece is for Weston to revert to the old fashioned "recheck
> > everything" behaviour when hotplug uevent carries anything
> > unrecognised. Grep for drm_backend_update_conn_props if you want to see
> > for yourself.
> >
> > However, I wouldn't NAK this patch just for Weston, but I wonder if
> > other software would ignore events because of this as well.
> >
> > A whole another question is, would anyone notice. I guess this can only
> > be an issue with MST.  
> 
> I think Weston should be fine: udev_event_is_conn_prop_change returns false
> if there's no PROPERTY in the uevent. An uevent with just a CONNECTOR and no
> PROPERTY is something new. Weston already falls back to the old "reprobe the
> world" approach in this case.
> 
> So far the CONNECTOR+PROPERTY uevent fields have only been used for content
> protection stuff. I'm not aware of other user-space using it (checked Kodi
> just in case, it doesn't do content protection nor handles uevents at all).
> 
> > All the other changes in this series look fine to me, so them I can give
> > Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>  

Hi Simon,

you're right! Therefore my Ack applies to this patch too.


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]