<div dir="ltr">David - is the v3 patch acceptable to you? Thanks.</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Feb 12, 2016 at 11:50 AM, Haixia Shi <span dir="ltr"><<a href="mailto:hshi@chromium.org" target="_blank">hshi@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">v1: Remove the general drm_device_is_unplugged() checks, and move the unplugged<br>
flag handling logic into drm/udl. In general we want to keep driver-specific<br>
logic out of common drm code.<br>
<br>
v2: Based on discussion with Stephane and David, drop most of the unplugged<br>
flag handling logic in drm/udl except for udl_detect() and udl_fb_open().<br>
The intention is to treat the device removal as a connector-unplug, and kep<br>
the UDL device fully functional.<br>
<br>
v3: Based on feedback from David, entirely drop the "unplugged" flag and all<br>
related code. There is no need to check for the unplugged flag as the existing<br>
udl_usb_disconnect() behavior already ensures the controller is removed, and<br>
all code paths that uses the usb-device are not reachable after unplug.<br>
<span class=""><br>
When a UDL monitor is unplugged, we need to still treat it as a fully<br>
functional device which just happens to have its connector unplugged.<br>
This allows user-space to properly deallocate fbs and dumb buffers<br>
before closing the device.<br>
<br>
This drops the "unplugged" flag hack, which puts the device in a<br>
non-functional state after USB unplug and rejects most operations on<br>
the device such as ioctls with error -ENODEV.<br>
<br>
Signed-off-by: Haixia Shi <<a href="mailto:hshi@chromium.org">hshi@chromium.org</a>><br>
Reviewed-by: Stéphane Marchesin <<a href="mailto:marcheu@chromium.org">marcheu@chromium.org</a>><br>
Cc: David Herrmann <<a href="mailto:dh.herrmann@gmail.com">dh.herrmann@gmail.com</a>><br>
</span>Cc: Daniel Vetter <<a href="mailto:daniel@ffwll.ch">daniel@ffwll.ch</a>><br>
<div class="HOEnZb"><div class="h5">---<br>
 drivers/gpu/drm/drm_drv.c           |  6 ------<br>
 drivers/gpu/drm/drm_fops.c          |  2 --<br>
 drivers/gpu/drm/drm_gem.c           |  3 ---<br>
 drivers/gpu/drm/drm_ioctl.c         |  3 ---<br>
 drivers/gpu/drm/drm_vm.c            |  3 ---<br>
 drivers/gpu/drm/udl/udl_connector.c |  2 --<br>
 drivers/gpu/drm/udl/udl_fb.c        |  6 ------<br>
 include/drm/drmP.h                  | 14 --------------<br>
 8 files changed, 39 deletions(-)<br>
<br>
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c<br>
index 167c8d3..f93ee12 100644<br>
--- a/drivers/gpu/drm/drm_drv.c<br>
+++ b/drivers/gpu/drm/drm_drv.c<br>
@@ -376,9 +376,6 @@ struct drm_minor *drm_minor_acquire(unsigned int minor_id)<br>
<br>
        if (!minor) {<br>
                return ERR_PTR(-ENODEV);<br>
-       } else if (drm_device_is_unplugged(minor->dev)) {<br>
-               drm_dev_unref(minor->dev);<br>
-               return ERR_PTR(-ENODEV);<br>
        }<br>
<br>
        return minor;<br>
@@ -464,9 +461,6 @@ void drm_unplug_dev(struct drm_device *dev)<br>
        drm_minor_unregister(dev, DRM_MINOR_CONTROL);<br>
<br>
        mutex_lock(&drm_global_mutex);<br>
-<br>
-       drm_device_set_unplugged(dev);<br>
-<br>
        if (dev->open_count == 0) {<br>
                drm_put_dev(dev);<br>
        }<br>
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c<br>
index 1ea8790..b4332d4 100644<br>
--- a/drivers/gpu/drm/drm_fops.c<br>
+++ b/drivers/gpu/drm/drm_fops.c<br>
@@ -497,8 +497,6 @@ int drm_release(struct inode *inode, struct file *filp)<br>
<br>
        if (!--dev->open_count) {<br>
                retcode = drm_lastclose(dev);<br>
-               if (drm_device_is_unplugged(dev))<br>
-                       drm_put_dev(dev);<br>
        }<br>
        mutex_unlock(&drm_global_mutex);<br>
<br>
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c<br>
index 2e8c77e..c622e32 100644<br>
--- a/drivers/gpu/drm/drm_gem.c<br>
+++ b/drivers/gpu/drm/drm_gem.c<br>
@@ -900,9 +900,6 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)<br>
        struct drm_vma_offset_node *node;<br>
        int ret;<br>
<br>
-       if (drm_device_is_unplugged(dev))<br>
-               return -ENODEV;<br>
-<br>
        drm_vma_offset_lock_lookup(dev->vma_offset_manager);<br>
        node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,<br>
                                                  vma->vm_pgoff,<br>
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c<br>
index 8ce2a0c..f959074 100644<br>
--- a/drivers/gpu/drm/drm_ioctl.c<br>
+++ b/drivers/gpu/drm/drm_ioctl.c<br>
@@ -695,9 +695,6 @@ long drm_ioctl(struct file *filp,<br>
<br>
        dev = file_priv->minor->dev;<br>
<br>
-       if (drm_device_is_unplugged(dev))<br>
-               return -ENODEV;<br>
-<br>
        is_driver_ioctl = nr >= DRM_COMMAND_BASE && nr < DRM_COMMAND_END;<br>
<br>
        if (is_driver_ioctl) {<br>
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c<br>
index f90bd5f..3a68be4 100644<br>
--- a/drivers/gpu/drm/drm_vm.c<br>
+++ b/drivers/gpu/drm/drm_vm.c<br>
@@ -657,9 +657,6 @@ int drm_legacy_mmap(struct file *filp, struct vm_area_struct *vma)<br>
        struct drm_device *dev = priv->minor->dev;<br>
        int ret;<br>
<br>
-       if (drm_device_is_unplugged(dev))<br>
-               return -ENODEV;<br>
-<br>
        mutex_lock(&dev->struct_mutex);<br>
        ret = drm_mmap_locked(filp, vma);<br>
        mutex_unlock(&dev->struct_mutex);<br>
diff --git a/drivers/gpu/drm/udl/udl_connector.c b/drivers/gpu/drm/udl/udl_connector.c<br>
index 4709b54..a6d5e21 100644<br>
--- a/drivers/gpu/drm/udl/udl_connector.c<br>
+++ b/drivers/gpu/drm/udl/udl_connector.c<br>
@@ -96,8 +96,6 @@ static int udl_mode_valid(struct drm_connector *connector,<br>
 static enum drm_connector_status<br>
 udl_detect(struct drm_connector *connector, bool force)<br>
 {<br>
-       if (drm_device_is_unplugged(connector->dev))<br>
-               return connector_status_disconnected;<br>
        return connector_status_connected;<br>
 }<br>
<br>
diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c<br>
index 200419d..29aca6c 100644<br>
--- a/drivers/gpu/drm/udl/udl_fb.c<br>
+++ b/drivers/gpu/drm/udl/udl_fb.c<br>
@@ -321,12 +321,6 @@ static void udl_fb_imageblit(struct fb_info *info, const struct fb_image *image)<br>
 static int udl_fb_open(struct fb_info *info, int user)<br>
 {<br>
        struct udl_fbdev *ufbdev = info->par;<br>
-       struct drm_device *dev = ufbdev->ufb.base.dev;<br>
-       struct udl_device *udl = dev->dev_private;<br>
-<br>
-       /* If the USB device is gone, we don't accept new opens */<br>
-       if (drm_device_is_unplugged(udl->ddev))<br>
-               return -ENODEV;<br>
<br>
        ufbdev->fb_count++;<br>
<br>
diff --git a/include/drm/drmP.h b/include/drm/drmP.h<br>
index d7162cf..40c6099 100644<br>
--- a/include/drm/drmP.h<br>
+++ b/include/drm/drmP.h<br>
@@ -748,7 +748,6 @@ struct drm_device {<br>
        struct drm_minor *control;              /**< Control node */<br>
        struct drm_minor *primary;              /**< Primary node */<br>
        struct drm_minor *render;               /**< Render node */<br>
-       atomic_t unplugged;                     /**< Flag whether dev is dead */<br>
        struct inode *anon_inode;               /**< inode for private address-space */<br>
        char *unique;                           /**< unique name of the device */<br>
        /*@} */<br>
@@ -879,19 +878,6 @@ static __inline__ int drm_core_check_feature(struct drm_device *dev,<br>
        return ((dev->driver->driver_features & feature) ? 1 : 0);<br>
 }<br>
<br>
-static inline void drm_device_set_unplugged(struct drm_device *dev)<br>
-{<br>
-       smp_wmb();<br>
-       atomic_set(&dev->unplugged, 1);<br>
-}<br>
-<br>
-static inline int drm_device_is_unplugged(struct drm_device *dev)<br>
-{<br>
-       int ret = atomic_read(&dev->unplugged);<br>
-       smp_rmb();<br>
-       return ret;<br>
-}<br>
-<br>
 static inline bool drm_is_render_client(const struct drm_file *file_priv)<br>
 {<br>
        return file_priv->minor->type == DRM_MINOR_RENDER;<br>
--<br>
2.7.0.rc3.207.g0ac5344<br>
<br>
</div></div></blockquote></div><br></div>