As a pair to already existing drm_connector_unplug_all() (which we'll rename in this series to drm_connector_unregister_all()) we're adding generic implementation of what is already done in some drivers for registering all connectors.
After implementation of that new helper we're updating 2 drivers that used to use it's own implementation: [1] atmel_hlcdc [2] rcar_du
Other drivers still use load() callback and so should be first modified so their load() gets called from their probe() explicitly.
Build- and run-tested on yet to be upstreamed ARC PGU (part of AXS10x board).
Changes v3 -> v4: * Based on current drm-intel/topic/drm-misc It's now on commit 6c87e5c3ec6db052f3744804a517b6fb003906e1 And since thet new base already has "drm: Rename drm_connector_unplug_all() to drm_connector_unregister_all()" this series now only includes 3 subsequent patches.
* 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: * Added acks for 1, 3 and 4 patches * Updated kerneldoc descriptins of both register_ and unregister_all() * Updated commit messages (mostly spellos and grammar issues)
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()
Alexey Brodkin (3): drm: Introduce drm_connector_register_all() helper drm: atmel_hldc: Use generic drm_connector_register_all() helper drm: rcar-du: Use generic drm_connector_register_all() helper
Cc: Daniel Vetter daniel@ffwll.ch Cc: David Airlie airlied@linux.ie Cc: Boris Brezillon boris.brezillon@free-electrons.com Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Cc: linux-renesas-soc@vger.kernel.org Cc: David Herrmann dh.herrmann@gmail.com
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 39 ++-------------------------- drivers/gpu/drm/drm_crtc.c | 39 ++++++++++++++++++++++++++++ drivers/gpu/drm/drm_drv.c | 6 ++++- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 9 +------ include/drm/drm_crtc.h | 3 ++- 5 files changed, 49 insertions(+), 47 deletions(-)
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 abrodkin@synopsys.com Cc: Daniel Vetter daniel@ffwll.ch Cc: David Airlie airlied@linux.ie Cc: David Herrmann dh.herrmann@gmail.com ---
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); + 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);
Hi
On Tue, Mar 29, 2016 at 12:02 PM, Alexey Brodkin Alexey.Brodkin@synopsys.com 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 abrodkin@synopsys.com Cc: Daniel Vetter daniel@ffwll.ch Cc: David Airlie airlied@linux.ie Cc: David Herrmann dh.herrmann@gmail.com
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
Hi David,
On Tue, 2016-03-29 at 12:08 +0200, David Herrmann wrote:
Hi
On Tue, Mar 29, 2016 at 12:02 PM, Alexey Brodkin Alexey.Brodkin@synopsys.com 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 abrodkin@synopsys.com Cc: Daniel Vetter daniel@ffwll.ch Cc: David Airlie airlied@linux.ie Cc: David Herrmann dh.herrmann@gmail.com
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.
So true!
BTW that's why I liked the previous solution - code was much cleaner with only 1 branch of execution.
Will resend v5 in a minute.
-Alexey
This driver used to have its own implementation of connector_register_all() which actually was taken as a prototype of drm_connector_register_all().
Now when drm_connector_register_all() exists reusing it here.
And while at it replace atmel_hlcdc_dc_connector_unplug_all() with generic drm_connector_unregister_all().
Signed-off-by: Alexey Brodkin abrodkin@synopsys.com Cc: Daniel Vetter daniel@ffwll.ch Cc: David Airlie airlied@linux.ie Acked-by: Boris Brezillon boris.brezillon@free-electrons.com ---
No changes v3 -> v4.
Changes v2 -> v3: * Updated title with capital after colon * Added ack from Boris
No changes v1 -> v2.
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 39 ++-------------------------- 1 file changed, 2 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c index 3d8d164..1c537e4 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c @@ -584,41 +584,6 @@ static void atmel_hlcdc_dc_unload(struct drm_device *dev) destroy_workqueue(dc->wq); }
-static int atmel_hlcdc_dc_connector_plug_all(struct drm_device *dev) -{ - struct drm_connector *connector, *failed; - int ret; - - mutex_lock(&dev->mode_config.mutex); - list_for_each_entry(connector, &dev->mode_config.connector_list, head) { - ret = drm_connector_register(connector); - if (ret) { - failed = connector; - goto err; - } - } - mutex_unlock(&dev->mode_config.mutex); - return 0; - -err: - list_for_each_entry(connector, &dev->mode_config.connector_list, head) { - if (failed == connector) - break; - - drm_connector_unregister(connector); - } - mutex_unlock(&dev->mode_config.mutex); - - return ret; -} - -static void atmel_hlcdc_dc_connector_unplug_all(struct drm_device *dev) -{ - mutex_lock(&dev->mode_config.mutex); - drm_connector_unplug_all(dev); - mutex_unlock(&dev->mode_config.mutex); -} - static void atmel_hlcdc_dc_lastclose(struct drm_device *dev) { struct atmel_hlcdc_dc *dc = dev->dev_private; @@ -736,7 +701,7 @@ static int atmel_hlcdc_dc_drm_probe(struct platform_device *pdev) if (ret) goto err_unload;
- ret = atmel_hlcdc_dc_connector_plug_all(ddev); + ret = drm_connector_register_all(ddev); if (ret) goto err_unregister;
@@ -758,7 +723,7 @@ static int atmel_hlcdc_dc_drm_remove(struct platform_device *pdev) { struct drm_device *ddev = platform_get_drvdata(pdev);
- atmel_hlcdc_dc_connector_unplug_all(ddev); + drm_connector_unregister_all(ddev); drm_dev_unregister(ddev); atmel_hlcdc_dc_unload(ddev); drm_dev_unref(ddev);
Now that a generic drm_connector_register_all() helper exists we may safely substitute it for the driver-specific implementation of connectors plugging in sysfs.
Signed-off-by: Alexey Brodkin abrodkin@synopsys.com Cc: Daniel Vetter daniel@ffwll.ch Cc: David Airlie airlied@linux.ie Cc: linux-renesas-soc@vger.kernel.org Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com ---
No changes v3 -> v4.
Changes v2 -> v3: * Updated title with capital after colon * Updated commit message with fixes of spellos and grammar issues * Added ack from Laurent
No changes v1 -> v2.
drivers/gpu/drm/rcar-du/rcar_du_drv.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 644db36..0f251dc 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c @@ -361,14 +361,7 @@ static int rcar_du_probe(struct platform_device *pdev) if (ret) goto error;
- mutex_lock(&ddev->mode_config.mutex); - drm_for_each_connector(connector, ddev) { - ret = drm_connector_register(connector); - if (ret < 0) - break; - } - mutex_unlock(&ddev->mode_config.mutex); - + ret = drm_connector_register_all(ddev); if (ret < 0) goto error;
dri-devel@lists.freedesktop.org
-
Alexey Brodkin -
David Herrmann