Re: [PATCH 1/3 v4] drm: Introduce drm_connector_register_all() helper

From: Alexey Brodkin
Date: Tue Mar 29 2016 - 06:10:22 EST

Hi David,

On Tue, 2016-03-29 at 12:08 +-0200, David Herrmann wrote:
+AD4- Hi
+AD4- On Tue, Mar 29, 2016 at 12:02 PM, Alexey Brodkin
+AD4- wrote:
+AD4- +AD4-
+AD4- +AD4- As a pair to already existing drm+AF8-connector+AF8-unregister+AF8-all() we're adding
+AD4- +AD4- generic implementation of what is already done in some drivers.
+AD4- +AD4-
+AD4- +AD4- Once this helper is implemented we'll be ready to switch existing
+AD4- +AD4- driver-specific implementations with the generic one.
+AD4- +AD4-
+AD4- +AD4- Signed-off-by: Alexey Brodkin
+AD4- +AD4- Cc: Daniel Vetter
+AD4- +AD4- Cc: David Airlie
+AD4- +AD4- Cc: David Herrmann
+AD4- +AD4- ---
+AD4- +AD4-
+AD4- +AD4- Changes v3 -+AD4- v4:
+AD4- +AD4- +AKAAKg- In drm+AF8-connector+AF8-register+AF8-all() fail path which calls unregister+AF8-all()
+AD4- +AD4- +AKAAoACg-is moved outside of loop+ACY-locked section (as suggested by Daniel)
+AD4- +AD4-
+AD4- +AD4- Changes v2 -+AD4- v3:
+AD4- +AD4- +AKAAKg- Updated title with capital after colon
+AD4- +AD4- +AKAAKg- Simplified failure path with direct and unconditional invocation of
+AD4- +AD4- +AKAAoACg-unregister+AF8-all()
+AD4- +AD4- +AKAAKg- Updated kerneldoc description of the drm+AF8-connector+AF8-register+AF8-all()
+AD4- +AD4-
+AD4- +AD4- Changes v1 -+AD4- v2:
+AD4- +AD4- +AKAAKg- Rename drm+AF8-connector+AF8-unplug+AF8-all() to drm+AF8-connector+AF8-unregister+AF8-all()
+AD4- +AD4- +AKAAKg- Use drm+AF8-for+AF8-each+AF8-connector() instead of list+AF8-for+AF8-each+AF8-entry()
+AD4- +AD4- +AKAAKg- Updated kerneldoc for drm+AF8-dev+AF8-register()
+AD4- +AD4-
+AD4- +AD4- +AKA-drivers/gpu/drm/drm+AF8-crtc.c +AHw- 39 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-
+AD4- +AD4- +AKA-drivers/gpu/drm/drm+AF8-drv.c+AKAAoAB8AKAAoA-6 +-+-+-+-+--
+AD4- +AD4- +AKA-include/drm/drm+AF8-crtc.h+AKAAoACgAKAAoAB8AKAAoA-3 +-+--
+AD4- +AD4- +AKA-3 files changed, 46 insertions(+-), 2 deletions(-)
+AD4- +AD4-
+AD4- +AD4- diff --git a/drivers/gpu/drm/drm+AF8-crtc.c b/drivers/gpu/drm/drm+AF8-crtc.c
+AD4- +AD4- index 7675826..3e4cdb1 100644
+AD4- +AD4- --- a/drivers/gpu/drm/drm+AF8-crtc.c
+AD4- +AD4- +-+-+- b/drivers/gpu/drm/drm+AF8-crtc.c
+AD4- +AD4- +AEAAQA- -1079,6 +-1079,45 +AEAAQA- void drm+AF8-connector+AF8-unregister(struct drm+AF8-connector +ACo-connector)
+AD4- +AD4- +AKA-EXPORT+AF8-SYMBOL(drm+AF8-connector+AF8-unregister)+ADs-
+AD4- +AD4-
+AD4- +AD4- +AKA-/+ACoAKg-
+AD4- +AD4- +- +ACo- drm+AF8-connector+AF8-register+AF8-all - register all connectors
+AD4- +AD4- +- +ACo- +AEA-dev: drm device
+AD4- +AD4- +- +ACo-
+AD4- +AD4- +- +ACo- This function registers all connectors in sysfs and other places so that
+AD4- +AD4- +- +ACo- userspace can start to access them. Drivers can call it after calling
+AD4- +AD4- +- +ACo- drm+AF8-dev+AF8-register() to complete the device registration, if they don't call
+AD4- +AD4- +- +ACo- drm+AF8-connector+AF8-register() on each connector individually.
+AD4- +AD4- +- +ACo-
+AD4- +AD4- +- +ACo- When a device is unplugged and should be removed from userspace access,
+AD4- +AD4- +- +ACo- call drm+AF8-connector+AF8-unregister+AF8-all(), which is the inverse of this
+AD4- +AD4- +- +ACo- function.
+AD4- +AD4- +- +ACo-
+AD4- +AD4- +- +ACo- Returns:
+AD4- +AD4- +- +ACo- Zero on success, error code on failure.
+AD4- +AD4- +- +ACo-/
+AD4- +AD4- +-int drm+AF8-connector+AF8-register+AF8-all(struct drm+AF8-device +ACo-dev)
+AD4- +AD4- +-+AHs-
+AD4- +AD4- +-+AKAAoACgAKAAoACgAKA-struct drm+AF8-connector +ACo-connector+ADs-
+AD4- +AD4- +-+AKAAoACgAKAAoACgAKA-int ret+ADs-
+AD4- +AD4- +-
+AD4- +AD4- +-+AKAAoACgAKAAoACgAKA-mutex+AF8-lock(+ACY-dev-+AD4-mode+AF8-config.mutex)+ADs-
+AD4- +AD4- +-
+AD4- +AD4- +-+AKAAoACgAKAAoACgAKA-drm+AF8-for+AF8-each+AF8-connector(connector, dev) +AHs-
+AD4- +AD4- +-+AKAAoACgAKAAoACgAKAAoACgAKAAoACgAKAAoACg-ret +AD0- drm+AF8-connector+AF8-register(connector)+ADs-
+AD4- +AD4- +-
+AD4- +AD4- +-+AKAAoACgAKAAoACgAKA-mutex+AF8-unlock(+ACY-dev-+AD4-mode+AF8-config.mutex)+ADs-
+AD4- +AD4- +-
+AD4- +AD4- +-+AKAAoACgAKAAoACgAKA-return 0+ADs-
+AD4- +AD4- +-
+AD4- +AD4- +-err:
+AD4- +AD4- +-+AKAAoACgAKAAoACgAKA-drm+AF8-connector+AF8-unregister+AF8-all(dev)+ADs-
+AD4- You +AF8-must+AF8- unlock the mutex before returning.

So true+ACE-

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.