On Thu, Feb 07, 2019 at 04:07:33PM -0500, Sean Paul wrote:
On Sun, Feb 03, 2019 at 04:41:56PM +0100, Noralf Trønnes wrote:
The only thing now that makes drm_dev_unplug() special is that it sets drm_device->unplugged. Move this code to drm_dev_unregister() so that we can remove drm_dev_unplug().
Signed-off-by: Noralf Trønnes noralf@tronnes.org
Maybe s/unplugged/unregistered/ ?
I looked at drm_device->registered, but using that would mean that drm_dev_is_unplugged() would return before drm_device is registered. And given that its current purpose is to prevent race against connector registration, I stayed away from it.
Makes sense. I think having unregistered along with registered might cause some confusion unless it's really clearly documented. Perhaps s/unplugged/unregistered/ and s/registered/initialized/ but init has its own meaning...
I'd hate to hold up the patchset over naming bikeshed, so maybe this would be better off as a follow-on series where everyone can pile on opinions. So,
Reviewed-by: Sean Paul sean@poorly.run
Looks like I was dropped from Cc, so I didn't see the replies after Oleksandr's review. So go ahead and disregard this :-)
Sean
Noralf.
drivers/gpu/drm/drm_drv.c | 27 +++++++++++++++------------ include/drm/drm_drv.h | 10 ++++------ 2 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 05bbc2b622fc..e0941200edc6 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -366,15 +366,6 @@ EXPORT_SYMBOL(drm_dev_exit); */ void drm_dev_unplug(struct drm_device *dev) {
- /*
* After synchronizing any critical read section is guaranteed to see* the new value of ->unplugged, and any critical section which might* still have seen the old value of ->unplugged is guaranteed to have* finished.*/- dev->unplugged = true;
- synchronize_srcu(&drm_unplug_srcu);
- drm_dev_unregister(dev); drm_dev_put(dev);
} @@ -832,11 +823,14 @@ EXPORT_SYMBOL(drm_dev_register);
- drm_dev_register() but does not deallocate the device. The caller must call
- drm_dev_put() to drop their final reference.
- A special form of unregistering for hotpluggable devices is drm_dev_unplug(),
- which can be called while there are still open users of @dev.
- This function can be called while there are still open users of @dev as long
- as the driver protects its device resources using drm_dev_enter() and
- drm_dev_exit().
- This should be called first in the device teardown code to make sure
- userspace can't access the device instance any more.
- userspace can't access the device instance any more. Drivers that support
- device unplug will probably want to call drm_atomic_helper_shutdown() first
*/
- in order to disable the hardware on regular driver module unload.
void drm_dev_unregister(struct drm_device *dev) { @@ -845,6 +839,15 @@ void drm_dev_unregister(struct drm_device *dev) if (drm_core_check_feature(dev, DRIVER_LEGACY)) drm_lastclose(dev);
/*
* After synchronizing any critical read section is guaranteed to see* the new value of ->unplugged, and any critical section which might* still have seen the old value of ->unplugged is guaranteed to have* finished.*/dev->unplugged = true;
synchronize_srcu(&drm_unplug_srcu);
dev->registered = false;
drm_client_dev_unregister(dev);
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index ca46a45a9cce..c50696c82a42 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -736,13 +736,11 @@ void drm_dev_unplug(struct drm_device *dev);
- drm_dev_is_unplugged - is a DRM device unplugged
- @dev: DRM device
- This function can be called to check whether a hotpluggable is unplugged.
- Unplugging itself is singalled through drm_dev_unplug(). If a device is
- unplugged, these two functions guarantee that any store before calling
- drm_dev_unplug() is visible to callers of this function after it completes
- This function can be called to check whether @dev is unregistered. This can
- be used to detect that the underlying parent device is gone.
- WARNING: This function fundamentally races against drm_dev_unplug(). It is
- recommended that drivers instead use the underlying drm_dev_enter() and
- WARNING: This function fundamentally races against drm_dev_unregister(). It
*/
- is recommended that drivers instead use the underlying drm_dev_enter() and
- drm_dev_exit() function pairs.
static inline bool drm_dev_is_unplugged(struct drm_device *dev)
2.20.1
-- Sean Paul, Software Engineer, Google / Chromium OS