On Mon, Feb 20, 2012 at 04:13:48PM +0000, Dave Airlie wrote:
> From: Dave Airlie <airlied@xxxxxxxxxx>
>
> Two parts to this, one is simple unplug from sysfs for the device node.
>
> The second adds an unplugged state, if we have device opens, we
> just set the unplugged state and return, if we have no device
> opens we drop the drm device.
>
> If after a lastclose we discover we are unplugged we then
> drop the drm device.
>
> Signed-off-by: Dave Airlie <airlied@xxxxxxxxxx>
I like the unplugged state to handle whole-device removal, but I think
this needs a few extensions:
- I think we should unconditionally unregister the drm dev minors. Simply
checking dev->unplugged should do the trick because we can't just tear
down the minor structs (we can only unregister the dev node).
- writing to dev->unplugged needs to be protected by the same mutex (i.e.
drm_global_mutex) as the actual unregister step.
- I think we should add a simply drm_device_is_unplugged so that drivers
can check the unplugged state in a race-free manner. Adding an smp_wmb()
after writing to dev->unplugged and an smb_rmb() before reading it in
that helper (plus ensuring that dev->unplugged is a separate word in
drm_device) should do the trick without overhead.
- I think we wan't the drm core to return -ENODEV for any ioctls an open
calls and return a SIGBUS on the fault handler. Currently you roll that
yourself in udl, but I can't think of a use-case where we want to stick
the interfaces around after a hotremove of the entire device.
Cheers, Daniel
> ---
> drivers/gpu/drm/drm_fops.c | 2 ++
> drivers/gpu/drm/drm_stub.c | 22 ++++++++++++++++++++++
> include/drm/drmP.h | 2 ++
> 3 files changed, 26 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> index 6263b01..154cfac 100644
> --- a/drivers/gpu/drm/drm_fops.c
> +++ b/drivers/gpu/drm/drm_fops.c
> @@ -579,6 +579,8 @@ int drm_release(struct inode *inode, struct file *filp)
> retcode = -EBUSY;
> } else
> retcode = drm_lastclose(dev);
> + if (dev->unplugged)
> + drm_put_dev(dev);
> }
> mutex_unlock(&drm_global_mutex);
>
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index 6d7b083..b04c92d 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -429,6 +429,11 @@ int drm_put_minor(struct drm_minor **minor_p)
> return 0;
> }
>
> +static void drm_unplug_minor(struct drm_minor *minor)
> +{
> + drm_sysfs_device_remove(minor);
> +}
> +
> /**
> * Called via drm_exit() at module unload time or when pci device is
> * unplugged.
> @@ -492,3 +497,20 @@ void drm_put_dev(struct drm_device *dev)
> kfree(dev);
> }
> EXPORT_SYMBOL(drm_put_dev);
> +
> +void drm_unplug_dev(struct drm_device *dev)
> +{
> + /* for a USB device */
> + if (drm_core_check_feature(dev, DRIVER_MODESET))
> + drm_unplug_minor(dev->control);
> + drm_unplug_minor(dev->primary);
> +
> + dev->unplugged = true;
> +
> + mutex_lock(&drm_global_mutex);
> + if (dev->open_count == 0) {
> + drm_put_dev(dev);
> + }
> + mutex_unlock(&drm_global_mutex);
> +}
> +EXPORT_SYMBOL(drm_unplug_dev);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 92f0981..8c11797 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1170,6 +1170,7 @@ struct drm_device {
> struct idr object_name_idr;
> /*@} */
> int switch_power_state;
> + bool unplugged; /* device has been unplugged or gone away */
> };
>
> #define DRM_SWITCH_POWER_ON 0
> @@ -1464,6 +1465,7 @@ extern void drm_master_put(struct drm_master **master);
>
> extern void drm_put_dev(struct drm_device *dev);
> extern int drm_put_minor(struct drm_minor **minor);
> +extern void drm_unplug_dev(struct drm_device *dev);
> extern unsigned int drm_debug;
>
> extern unsigned int drm_vblank_offdelay;
> --
> 1.7.6
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Mail: daniel@xxxxxxxx
Mobile: +41 (0)79 365 57 48
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
