Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756718AbcC2KIN (ORCPT ); Tue, 29 Mar 2016 06:08:13 -0400 Received: from mail-wm0-f53.google.com ([74.125.82.53]:34362 "EHLO mail-wm0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751580AbcC2KIL (ORCPT ); Tue, 29 Mar 2016 06:08:11 -0400 MIME-Version: 1.0 In-Reply-To: <1459245734-2392-2-git-send-email-abrodkin@synopsys.com> References: <1459245734-2392-1-git-send-email-abrodkin@synopsys.com> <1459245734-2392-2-git-send-email-abrodkin@synopsys.com> Date: Tue, 29 Mar 2016 12:08:09 +0200 Message-ID: Subject: Re: [PATCH 1/3 v4] drm: Introduce drm_connector_register_all() helper From: David Herrmann To: Alexey Brodkin Cc: "dri-devel@lists.freedesktop.org" , linux-kernel , Daniel Vetter , David Airlie Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4612 Lines: 126 Hi On Tue, Mar 29, 2016 at 12:02 PM, Alexey Brodkin 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 > Cc: Daniel Vetter > Cc: David Airlie > Cc: David Herrmann > --- > > Changes v3 -> v4: > * In drm_connector_register_all() fail path which calls unregister_all() > is moved outside of loop&locked section (as suggested by Daniel) > > 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 | 39 +++++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/drm_drv.c | 6 +++++- > include/drm/drm_crtc.h | 3 ++- > 3 files changed, 46 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 7675826..3e4cdb1 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -1079,6 +1079,45 @@ 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) > + goto err; > + } > + > + mutex_unlock(&dev->mode_config.mutex); > + > + return 0; > + > +err: > + drm_connector_unregister_all(dev); You _must_ unlock the mutex before returning. Thanks David > + return ret; > +} > +EXPORT_SYMBOL(drm_connector_register_all); > + > +/** > * drm_connector_unregister_all - unregister connector userspace interfaces > * @dev: drm device > * > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 167c8d3..2c9a2b6 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -715,7 +715,11 @@ EXPORT_SYMBOL(drm_dev_unref); > * > * Register the DRM device @dev with the system, advertise device to user-space > * and start normal device operation. @dev must be allocated via drm_dev_alloc() > - * previously. > + * previously. Right after drm_dev_register() the driver should call > + * drm_connector_register_all() to register all connectors in sysfs. This is > + * a separate call for backward compatibility with drivers still using > + * the deprecated ->load() callback, where connectors are registered from within > + * the ->load() callback. > * > * Never call this twice on any device! > * > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 12f2bd4..6231f6c 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -2253,7 +2253,8 @@ void drm_connector_unregister(struct drm_connector *connector); > > extern void drm_connector_cleanup(struct drm_connector *connector); > extern unsigned int drm_connector_index(struct drm_connector *connector); > -/* helper to unregister all connectors from sysfs for device */ > +/* helpers to {un}register all connectors from sysfs for device */ > +extern int drm_connector_register_all(struct drm_device *dev); > extern void drm_connector_unregister_all(struct drm_device *dev); > > extern int drm_bridge_add(struct drm_bridge *bridge); > -- > 2.5.0 >