Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754749AbcCWLNe (ORCPT ); Wed, 23 Mar 2016 07:13:34 -0400 Received: from mail-wm0-f67.google.com ([74.125.82.67]:33668 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752573AbcCWLNZ (ORCPT ); Wed, 23 Mar 2016 07:13:25 -0400 MIME-Version: 1.0 In-Reply-To: <1458722577-20283-3-git-send-email-abrodkin@synopsys.com> References: <1458722577-20283-1-git-send-email-abrodkin@synopsys.com> <1458722577-20283-3-git-send-email-abrodkin@synopsys.com> Date: Wed, 23 Mar 2016 12:13:24 +0100 Message-ID: Subject: Re: [PATCH 2/4 v3] 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: 4941 Lines: 131 Hi On Wed, Mar 23, 2016 at 9:42 AM, 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 > --- > > 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. Thanks David > + drm_connector_unregister_all(dev); > + return ret; > + } > + } > + > + mutex_unlock(&dev->mode_config.mutex); > + > + return 0; > +} > +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 42d9f4d..6a34117 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -2214,7 +2214,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 >