Hi David, On Wed, 2016-03-23 at 12:13 +0100, David Herrmann wrote: > Hi > > On Wed, Mar 23, 2016 at 9:42 AM, Alexey Brodkin > <[email protected]> wrote: > > > > As a pair to already existing drm_connector_unregister_all() we're adding > > generic implementation of what is already done in some drivers. > > > > Once this helper is implemented we'll be ready to switch existing > > driver-specific implementations with the generic one. > > > > Signed-off-by: Alexey Brodkin <[email protected]> > > Cc: Daniel Vetter <[email protected]> > > Cc: David Airlie <[email protected]> > > --- > > > > Changes v2 -> v3: > > * Updated title with capital after colon > > * Simplified failure path with direct and unconditional invocation of > > unregister_all() > > * Updated kerneldoc description of the drm_connector_register_all() > > > > Changes v1 -> v2: > > * Rename drm_connector_unplug_all() to drm_connector_unregister_all() > > * Use drm_for_each_connector() instead of list_for_each_entry() > > * Updated kerneldoc for drm_dev_register() > > > > drivers/gpu/drm/drm_crtc.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/drm_drv.c | 6 +++++- > > include/drm/drm_crtc.h | 3 ++- > > 3 files changed, 50 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > > index 65488a6..21eea11 100644 > > --- a/drivers/gpu/drm/drm_crtc.c > > +++ b/drivers/gpu/drm/drm_crtc.c > > @@ -1081,6 +1081,49 @@ void drm_connector_unregister(struct drm_connector > > *connector) > > EXPORT_SYMBOL(drm_connector_unregister); > > > > /** > > + * drm_connector_register_all - register all connectors > > + * @dev: drm device > > + * > > + * This function registers all connectors in sysfs and other places so that > > + * userspace can start to access them. Drivers can call it after calling > > + * drm_dev_register() to complete the device registration, if they don't > > call > > + * drm_connector_register() on each connector individually. > > + * > > + * When a device is unplugged and should be removed from userspace access, > > + * call drm_connector_unregister_all(), which is the inverse of this > > + * function. > > + * > > + * Returns: > > + * Zero on success, error code on failure. > > + */ > > +int drm_connector_register_all(struct drm_device *dev) > > +{ > > + struct drm_connector *connector; > > + int ret; > > + > > + mutex_lock(&dev->mode_config.mutex); > > + > > + drm_for_each_connector(connector, dev) { > > + ret = drm_connector_register(connector); > > + if (ret) { > > + /* > > + * We may safely call unregister_all() here within > > + * area locked with mutex because unregister_all() > > + * doesn't use locks inside (see a comment in that > > + * function). > > + */ > Ugh? unregister_all() says: > > /* FIXME: taking the mode config mutex ends up in a clash with sysfs */ > > This strongly contradicts your comment. Anyway, regardless how you > want to fix it: You better unlock the mode-config mutex before > returning below.
So good catch. But what I really meant since we didn't get any further after registering all "good" connections (see we're not releasing mutex still) the will be no clashes with sysfs. Still I;d like Daniel to comment on that separately. -Alexey
