Re: [PATCH 1/3] drm: introduce drm_connector_plug_all() helper

[Alexey Brodkin]

Hi Daniel,

On Fri, 2016-03-18 at 19:06 +-0100, Daniel Vetter wrote:
+AD4- On Fri, Mar 18, 2016 at 01:01:42PM +-0300, Alexey Brodkin wrote:
+AD4- +AD4- 
+AD4- +AD4- As a pair to already existing drm+AF8-connector+AF8-unplug+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 generic one.
+AD4- +AD4- 
+AD4- +AD4- Signed-off-by: Alexey Brodkin +ADw-abrodkin+AEA-synopsys.com+AD4-
+AD4- +AD4- Cc: Daniel Vetter +ADw-daniel+AEA-ffwll.ch+AD4-
+AD4- +AD4- Cc: David Airlie +ADw-airlied+AEA-linux.ie+AD4-
+AD4- +AD4- ---
+AD4- +AD4- +AKA-drivers/gpu/drm/drm+AF8-crtc.c +AHw- 44 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+--
+AD4- +AD4- +AKA-drivers/gpu/drm/drm+AF8-drv.c+AKAAoAB8AKAAoA-3 +-+--
+AD4- +AD4- +AKA-include/drm/drm+AF8-crtc.h+AKAAoACgAKAAoAB8AKAAoA-3 +-+--
+AD4- +AD4- +AKA-3 files changed, 47 insertions(+-), 3 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 65258ac..ce27420 100644
+AD4- +AD4- --- a/drivers/gpu/drm/drm+AF8-crtc.c
+AD4- +AD4- +-+-+- b/drivers/gpu/drm/drm+AF8-crtc.c
+AD4- +AD4- +AEAAQA- -1080,6 +-1080,46 +AEAAQA- void drm+AF8-connector+AF8-unregister(struct drm+AF8-connector +ACo-connector)
+AD4- +AD4- +AKAAfQ-
+AD4- +AD4- +AKA-EXPORT+AF8-SYMBOL(drm+AF8-connector+AF8-unregister)+ADs-
+AD4- +AD4- +AKA-
+AD4- +AD4- +-/+ACoAKg-
+AD4- +AD4- +- +ACo- drm+AF8-connector+AF8-plug+AF8-all - register connector userspace interfaces
+AD4- +AD4- +- +ACo- +AEA-dev: drm device
+AD4- +AD4- +- +ACo-
+AD4- +AD4- +- +ACo- This function registers all connector userspace interfaces in sysfs. Should
+AD4- +AD4- +- +ACo- be call when the device is disconnected, e.g. from an usb driver's
+AD4- Still talks about disconnect +ADs--) Please also mention that this just calls
+AD4- drm+AF8-connector+AF8-register() exactly like this including () to generate a
+AD4- kerneldoc hyperlink.

Well I intentionally left in description of drm+AF8-connector+AF8-register+AF8-all():
+ACI-Should be call when the device is disconnected, e.g. from an usb driver's
+AKA--+AD4-connect callback.+ACI-

I did mean it. Or is this statement is incorrect and example of invocation of
drm+AF8-connector+AF8-register+AF8-all() should be different? Which one works better then?

+AD4- +AD4- 
+AD4- +AD4- +- +ACo- -+AD4-connect callback.
+AD4- Returns: section is missing, specifying how this can fail. Just copy the
+AD4- one from connector+AF8-register().

Yeah, correct. Blind copy-paste doesn't work equally good always :(

+AD4- +AD4- 
+AD4- +AD4- +- +ACo-/
+AD4- +AD4- +-int drm+AF8-connector+AF8-plug+AF8-all(struct drm+AF8-device +ACo-dev)
+AD4- +AD4- +-+AHs-
+AD4- +AD4- +-	struct drm+AF8-connector +ACo-connector, +ACo-failed+ADs-
+AD4- +AD4- +-	int ret+ADs-
+AD4- +AD4- +-
+AD4- +AD4- +-	mutex+AF8-lock(+ACY-dev-+AD4-mode+AF8-config.mutex)+ADs-
+AD4- +AD4- +-
+AD4- +AD4- +-	list+AF8-for+AF8-each+AF8-entry(connector, +ACY-dev-+AD4-mode+AF8-config.connector+AF8-list, head) +AHs-
+AD4- for+AF8-each+AF8-connector here please. And the s/plug/register/ naming discussion
+AD4- we've had.

Ok.

+AD4- +AD4- 
+AD4- +AD4- +-		ret +AD0- drm+AF8-connector+AF8-register(connector)+ADs-
+AD4- +AD4- +-		if (ret) +AHs-
+AD4- +AD4- +-			failed +AD0- connector+ADs-
+AD4- +AD4- +-			goto err+ADs-
+AD4- +AD4- +-		+AH0-
+AD4- +AD4- +-	+AH0-
+AD4- +AD4- +-
+AD4- +AD4- +-	mutex+AF8-unlock(+ACY-dev-+AD4-mode+AF8-config.mutex)+ADs-
+AD4- +AD4- +-
+AD4- +AD4- +-	return 0+ADs-
+AD4- +AD4- +-
+AD4- +AD4- +-err:
+AD4- +AD4- +-	list+AF8-for+AF8-each+AF8-entry(connector, +ACY-dev-+AD4-mode+AF8-config.connector+AF8-list, head) +AHs-
+AD4- +AD4- +-		if (failed +AD0APQ- connector)
+AD4- +AD4- +-			break+ADs-
+AD4- +AD4- +-
+AD4- +AD4- +-		drm+AF8-connector+AF8-unregister(connector)+ADs-
+AD4- +AD4- +-	+AH0-
+AD4- +AD4- +-
+AD4- +AD4- +-	mutex+AF8-unlock(+ACY-dev-+AD4-mode+AF8-config.mutex)+ADs-
+AD4- +AD4- +-
+AD4- +AD4- +-	return ret+ADs-
+AD4- +AD4- +-+AH0-
+AD4- +AD4- +-EXPORT+AF8-SYMBOL(drm+AF8-connector+AF8-plug+AF8-all)+ADs-
+AD4- +AD4- +AKA-
+AD4- +AD4- +AKA-/+ACoAKg-
+AD4- +AD4- +AKA- +ACo- drm+AF8-connector+AF8-unplug+AF8-all - unregister connector userspace interfaces
+AD4- +AD4- +AEAAQA- -1093,10 +-1133,12 +AEAAQA- void drm+AF8-connector+AF8-unplug+AF8-all(struct drm+AF8-device +ACo-dev)
+AD4- +AD4- +AKAAew-
+AD4- +AD4- +AKA-	struct drm+AF8-connector +ACo-connector+ADs-
+AD4- +AD4- +AKA-
+AD4- +AD4- -	/+ACo- FIXME: taking the mode config mutex ends up in a clash with sysfs +ACo-/
+AD4- +AD4- +-	mutex+AF8-lock(+ACY-dev-+AD4-mode+AF8-config.mutex)+ADs-
+AD4- You can't drop that FIXME, the bug is still there.

That's clear given your explanation in the previous email.

+AD4- +AD4- 
+AD4- +AD4- +-
+AD4- +AD4- +AKA-	list+AF8-for+AF8-each+AF8-entry(connector, +ACY-dev-+AD4-mode+AF8-config.connector+AF8-list, head)
+AD4- +AD4- +AKA-		drm+AF8-connector+AF8-unregister(connector)+ADs-
+AD4- +AD4- +AKA-
+AD4- +AD4- +-	mutex+AF8-unlock(+ACY-dev-+AD4-mode+AF8-config.mutex)+ADs-
+AD4- +AD4- +AKAAfQ-
+AD4- +AD4- +AKA-EXPORT+AF8-SYMBOL(drm+AF8-connector+AF8-unplug+AF8-all)+ADs-
+AD4- +AD4- +AKA-
+AD4- +AD4- diff --git a/drivers/gpu/drm/drm+AF8-drv.c b/drivers/gpu/drm/drm+AF8-drv.c
+AD4- +AD4- index 167c8d3..4a559c6 100644
+AD4- +AD4- --- a/drivers/gpu/drm/drm+AF8-drv.c
+AD4- +AD4- +-+-+- b/drivers/gpu/drm/drm+AF8-drv.c
+AD4- +AD4- +AEAAQA- -715,7 +-715,8 +AEAAQA- EXPORT+AF8-SYMBOL(drm+AF8-dev+AF8-unref)+ADs-
+AD4- +AD4- +AKA- +ACo-
+AD4- +AD4- +AKA- +ACo- Register the DRM device +AEA-dev with the system, advertise device to user-space
+AD4- +AD4- +AKA- +ACo- and start normal device operation. +AEA-dev must be allocated via drm+AF8-dev+AF8-alloc()
+AD4- +AD4- - +ACo- previously.
+AD4- +AD4- +- +ACo- previously and right after drm+AF8-dev+AF8-register() driver should call
+AD4- It'd do 2 sentences here for simplicity, not connect them with and. Also
+AD4- +ACI-... +AF8-the+AF8- driver should ...+ACI-

Ok.

+AD4- +AD4- 
+AD4- +AD4- +- +ACo- drm+AF8-connector+AF8-plug+AF8-all() to register all connectors in sysfs.
+AD4- Maybe mention why this is separate: +ACI-This is a separate call for backwards
+AD4- compatibility with drivers still using the deprecated -+AD4-load() callback,
+AD4- where connectors are registered from within the -+AD4-load() callback.+ACI-

Ok.

-Alexey