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 v5 -> v6: * In atmel_hlcdc only substitute its own atmel_hlcdc_dc_connector_plug_all(). drm_connector_unregister_all() is already used there since 222b90943446 "drm/atmel: Fixup drm_connector_/unplug/unregister/_all"
Changes v4 -> v5: * Added missing mutex unlock on a fail path in drm_connector_register_all() Thanks David for his attention and patience!
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()
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
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
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 30 +-------------------- drivers/gpu/drm/drm_crtc.c | 40 ++++++++++++++++++++++++++++ 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(+), 39 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: Boris Brezillon boris.brezillon@free-electrons.com ---
No changes v5 -> v6.
Changes v4 -> v5: * Added missing mutex unlock on a fail path in drm_connector_register_all(). Thanks David for his attention and patience!
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 | 40 ++++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_drv.c | 6 +++++- include/drm/drm_crtc.h | 3 ++- 3 files changed, 47 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index f7fe9e1..ee549a3 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1068,6 +1068,46 @@ 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: + mutex_unlock(&dev->mode_config.mutex); + 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 6d46842..f9d2274 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -2249,7 +2249,8 @@ static inline unsigned drm_connector_index(struct drm_connector *connector) return connector->connector_id; }
-/* 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);
On Tue, Apr 19, 2016 at 03:24:51PM +0300, 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 abrodkin@synopsys.com Cc: Daniel Vetter daniel@ffwll.ch Cc: David Airlie airlied@linux.ie Cc: Boris Brezillon boris.brezillon@free-electrons.com
No changes v5 -> v6.
Changes v4 -> v5:
- Added missing mutex unlock on a fail path in drm_connector_register_all(). Thanks David for his attention and patience!
When resending, please add everyone who commmented on previous versions of your patches to the cc: list. Just to make sure they have a chance to look at the next version. -Daniel
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 | 40 ++++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_drv.c | 6 +++++- include/drm/drm_crtc.h | 3 ++- 3 files changed, 47 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index f7fe9e1..ee549a3 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1068,6 +1068,46 @@ 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:
- mutex_unlock(&dev->mode_config.mutex);
- 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 6d46842..f9d2274 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -2249,7 +2249,8 @@ static inline unsigned drm_connector_index(struct drm_connector *connector) return connector->connector_id; }
-/* 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.5
Hi Daniel,
On Wed, 2016-04-20 at 13:28 +0200, Daniel Vetter wrote:
On Tue, Apr 19, 2016 at 03:24:51PM +0300, 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 abrodkin@synopsys.com Cc: Daniel Vetter daniel@ffwll.ch Cc: David Airlie airlied@linux.ie Cc: Boris Brezillon boris.brezillon@free-electrons.com
No changes v5 -> v6.
Changes v4 -> v5: * Added missing mutex unlock on a fail path in drm_connector_register_all(). Thanks David for his attention and patience!
When resending, please add everyone who commmented on previous versions of your patches to the cc: list. Just to make sure they have a chance to look at the next version.
Sure I usually do that but looks like this time I completely forgot to add more people in Cc.
-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.
Signed-off-by: Alexey Brodkin abrodkin@synopsys.com Cc: Daniel Vetter daniel@ffwll.ch Cc: David Airlie airlied@linux.ie Cc: Boris Brezillon boris.brezillon@free-electrons.com ---
Changes v5 -> v6: * Only substitute its own atmel_hlcdc_dc_connector_plug_all(). drm_connector_unregister_all() is already used there since 222b90943446 "drm/atmel: Fixup drm_connector_/unplug/unregister/_all"
No changes v4 -> v5.
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 | 30 +--------------------------- 1 file changed, 1 insertion(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c index 8ab4318..9907dd1 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c @@ -584,34 +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); @@ -736,7 +708,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;
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 v5 -> v6.
No changes v4 -> v5.
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;
On Tue, Apr 19, 2016 at 03:24:50PM +0300, Alexey Brodkin wrote:
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 v5 -> v6:
- In atmel_hlcdc only substitute its own atmel_hlcdc_dc_connector_plug_all(). drm_connector_unregister_all() is already used there since 222b90943446 "drm/atmel: Fixup drm_connector_/unplug/unregister/_all"
Changes v4 -> v5:
- Added missing mutex unlock on a fail path in drm_connector_register_all() Thanks David for his attention and patience!
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()
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
All three applied to drm-misc, thanks. -Daniel
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
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 30 +-------------------- drivers/gpu/drm/drm_crtc.c | 40 ++++++++++++++++++++++++++++ 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(+), 39 deletions(-)
-- 2.5.5
dri-devel@lists.freedesktop.org
-
Alexey Brodkin -
Daniel Vetter