On Fri, Mar 18, 2016 at 09:58:49PM +0000, Alexey Brodkin wrote:
Hi Daniel,
On Fri, 2016-03-18 at 19:06 +0100, Daniel Vetter wrote:
On Fri, Mar 18, 2016 at 01:01:42PM +0300, Alexey Brodkin wrote:
As a pair to already existing drm_connector_unplug_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 generic one.
Signed-off-by: Alexey Brodkin abrodkin@synopsys.com Cc: Daniel Vetter daniel@ffwll.ch Cc: David Airlie airlied@linux.ie
drivers/gpu/drm/drm_crtc.c | 44 +++++++++++++++++++++++++++++++++++++++++++- drivers/gpu/drm/drm_drv.c | 3 ++- include/drm/drm_crtc.h | 3 ++- 3 files changed, 47 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 65258ac..ce27420 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1080,6 +1080,46 @@ void drm_connector_unregister(struct drm_connector *connector) } EXPORT_SYMBOL(drm_connector_unregister); +/**
- drm_connector_plug_all - register connector userspace interfaces
- @dev: drm device
- This function registers all connector userspace interfaces in sysfs. Should
- be call when the device is disconnected, e.g. from an usb driver's
Still talks about disconnect ;-) Please also mention that this just calls drm_connector_register() exactly like this including () to generate a kerneldoc hyperlink.
Well I intentionally left in description of drm_connector_register_all(): "Should be call when the device is disconnected, e.g. from an usb driver's ->connect callback."
You use "disconnected" for connecting stuff. That doesn't make sense to me at all - register_all is for when you want to publish something, not for unpublishing when the device disappears. Or maybe this is a case of lost in translation, and you mean something else? -Daniel
I did mean it. Or is this statement is incorrect and example of invocation of drm_connector_register_all() should be different? Which one works better then?
- ->connect callback.
Returns: section is missing, specifying how this can fail. Just copy the one from connector_register().
Yeah, correct. Blind copy-paste doesn't work equally good always :(
- */
+int drm_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) {
for_each_connector here please. And the s/plug/register/ naming discussion we've had.
Ok.
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;
+} +EXPORT_SYMBOL(drm_connector_plug_all); /** * drm_connector_unplug_all - unregister connector userspace interfaces @@ -1093,10 +1133,12 @@ void drm_connector_unplug_all(struct drm_device *dev) { struct drm_connector *connector;
- /* FIXME: taking the mode config mutex ends up in a clash with sysfs */
- mutex_lock(&dev->mode_config.mutex);
You can't drop that FIXME, the bug is still there.
That's clear given your explanation in the previous email.
list_for_each_entry(connector, &dev->mode_config.connector_list, head) drm_connector_unregister(connector);
- mutex_unlock(&dev->mode_config.mutex);
} EXPORT_SYMBOL(drm_connector_unplug_all); diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 167c8d3..4a559c6 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -715,7 +715,8 @@ 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 and right after drm_dev_register() driver should call
It'd do 2 sentences here for simplicity, not connect them with and. Also "... _the_ driver should ..."
Ok.
- drm_connector_plug_all() to register all connectors in sysfs.
Maybe mention why this is separate: "This is a separate call for backwards compatibility with drivers still using the deprecated ->load() callback, where connectors are registered from within the ->load() callback."
Ok.
-Alexey