[PATCH 0/4] drm unplugging docs

[PATCH 0/4] drm unplugging docs

[Daniel Vetter]

Hi all,

Just a small cleanup plus kerneldoc for the unplug support. There's still the
fundamental problem that most likely our lifetime stuff is entirely screwed up
for this, at least never export a dma-buf or dma-fence from a driver which can
be hot-unplugged.

It'd be sweet if we could properly fix this by attaching resources to either the
physical device (which should be drm_device->dev) or the sw device (aka
drm_device) depending upon whether it is needed after unplugging. There's also
the slight problem of synchronizing all that access somehow.

Either way this is a separate topic, I just thought about it a bit more while
cleaning this up.

On the patches themselves: I had to move struct drm_device into drm_device.h to
avoid a header include loop, since that's a bit invasive it'd be great if I can
land this fast.

Cheers, Daniel

Daniel Vetter (4):
  drm: Extract drm_device.h
  drm: Document device unplug infrastructure
  drm: Only lastclose on unload for legacy drivers
  drm: Clean up drm_dev_unplug

 drivers/gpu/drm/drm_drv.c                   |  40 +++---
 drivers/gpu/drm/drm_file.c                  |   2 +-
 drivers/gpu/drm/drm_gem.c                   |   2 +-
 drivers/gpu/drm/drm_gem_cma_helper.c        |   2 +-
 drivers/gpu/drm/drm_ioctl.c                 |   4 +-
 drivers/gpu/drm/drm_vm.c                    |   2 +-
 drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c |   2 +-
 drivers/gpu/drm/udl/udl_connector.c         |   2 +-
 drivers/gpu/drm/udl/udl_drv.c               |   2 +-
 drivers/gpu/drm/udl/udl_fb.c                |   2 +-
 include/drm/drmP.h                          | 188 +--------------------------
 include/drm/drm_device.h                    | 190 ++++++++++++++++++++++++++++
 include/drm/drm_drv.h                       |  22 +++-
 13 files changed, 246 insertions(+), 214 deletions(-)
 create mode 100644 include/drm/drm_device.h

-- 
2.13.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/4] drm: Extract drm_device.h

[Daniel Vetter]

I need this to untangle an include loop in the next patch.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 include/drm/drmP.h       | 175 +------------------------------------------
 include/drm/drm_device.h | 190 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 191 insertions(+), 174 deletions(-)
 create mode 100644 include/drm/drm_device.h

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 3aa3809ab524..3031c256105e 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -82,19 +82,10 @@
 #include <drm/drm_sysfs.h>
 #include <drm/drm_vblank.h>
 #include <drm/drm_irq.h>
-
+#include <drm/drm_device.h>
 
 struct module;
 
-struct drm_device;
-struct drm_agp_head;
-struct drm_local_map;
-struct drm_device_dma;
-struct drm_gem_object;
-struct drm_master;
-struct drm_vblank_crtc;
-struct drm_vma_offset_manager;
-
 struct device_node;
 struct videomode;
 struct reservation_object;
@@ -306,170 +297,6 @@ struct pci_controller;
 
 
 /**
- * DRM device structure. This structure represent a complete card that
- * may contain multiple heads.
- */
-struct drm_device {
-	struct list_head legacy_dev_list;/**< list of devices per driver for stealth attach cleanup */
-	int if_version;			/**< Highest interface version set */
-
-	/** \name Lifetime Management */
-	/*@{ */
-	struct kref ref;		/**< Object ref-count */
-	struct device *dev;		/**< Device structure of bus-device */
-	struct drm_driver *driver;	/**< DRM driver managing the device */
-	void *dev_private;		/**< DRM driver private data */
-	struct drm_minor *control;		/**< Control node */
-	struct drm_minor *primary;		/**< Primary node */
-	struct drm_minor *render;		/**< Render node */
-	bool registered;
-
-	/* currently active master for this device. Protected by master_mutex */
-	struct drm_master *master;
-
-	atomic_t unplugged;			/**< Flag whether dev is dead */
-	struct inode *anon_inode;		/**< inode for private address-space */
-	char *unique;				/**< unique name of the device */
-	/*@} */
-
-	/** \name Locks */
-	/*@{ */
-	struct mutex struct_mutex;	/**< For others */
-	struct mutex master_mutex;      /**< For drm_minor::master and drm_file::is_master */
-	/*@} */
-
-	/** \name Usage Counters */
-	/*@{ */
-	int open_count;			/**< Outstanding files open, protected by drm_global_mutex. */
-	spinlock_t buf_lock;		/**< For drm_device::buf_use and a few other things. */
-	int buf_use;			/**< Buffers in use -- cannot alloc */
-	atomic_t buf_alloc;		/**< Buffer allocation in progress */
-	/*@} */
-
-	struct mutex filelist_mutex;
-	struct list_head filelist;
-
-	/** \name Memory management */
-	/*@{ */
-	struct list_head maplist;	/**< Linked list of regions */
-	struct drm_open_hash map_hash;	/**< User token hash table for maps */
-
-	/** \name Context handle management */
-	/*@{ */
-	struct list_head ctxlist;	/**< Linked list of context handles */
-	struct mutex ctxlist_mutex;	/**< For ctxlist */
-
-	struct idr ctx_idr;
-
-	struct list_head vmalist;	/**< List of vmas (for debugging) */
-
-	/*@} */
-
-	/** \name DMA support */
-	/*@{ */
-	struct drm_device_dma *dma;		/**< Optional pointer for DMA support */
-	/*@} */
-
-	/** \name Context support */
-	/*@{ */
-
-	__volatile__ long context_flag;	/**< Context swapping flag */
-	int last_context;		/**< Last current context */
-	/*@} */
-
-	/**
-	 * @irq_enabled:
-	 *
-	 * Indicates that interrupt handling is enabled, specifically vblank
-	 * handling. Drivers which don't use drm_irq_install() need to set this
-	 * to true manually.
-	 */
-	bool irq_enabled;
-	int irq;
-
-	/**
-	 * @vblank_disable_immediate:
-	 *
-	 * If true, vblank interrupt will be disabled immediately when the
-	 * refcount drops to zero, as opposed to via the vblank disable
-	 * timer.
-	 *
-	 * This can be set to true it the hardware has a working vblank counter
-	 * with high-precision timestamping (otherwise there are races) and the
-	 * driver uses drm_crtc_vblank_on() and drm_crtc_vblank_off()
-	 * appropriately. See also @max_vblank_count and
-	 * &drm_crtc_funcs.get_vblank_counter.
-	 */
-	bool vblank_disable_immediate;
-
-	/**
-	 * @vblank:
-	 *
-	 * Array of vblank tracking structures, one per &struct drm_crtc. For
-	 * historical reasons (vblank support predates kernel modesetting) this
-	 * is free-standing and not part of &struct drm_crtc itself. It must be
-	 * initialized explicitly by calling drm_vblank_init().
-	 */
-	struct drm_vblank_crtc *vblank;
-
-	spinlock_t vblank_time_lock;    /**< Protects vblank count and time updates during vblank enable/disable */
-	spinlock_t vbl_lock;
-
-	/**
-	 * @max_vblank_count:
-	 *
-	 * Maximum value of the vblank registers. This value +1 will result in a
-	 * wrap-around of the vblank register. It is used by the vblank core to
-	 * handle wrap-arounds.
-	 *
-	 * If set to zero the vblank core will try to guess the elapsed vblanks
-	 * between times when the vblank interrupt is disabled through
-	 * high-precision timestamps. That approach is suffering from small
-	 * races and imprecision over longer time periods, hence exposing a
-	 * hardware vblank counter is always recommended.
-	 *
-	 * If non-zeor, &drm_crtc_funcs.get_vblank_counter must be set.
-	 */
-	u32 max_vblank_count;           /**< size of vblank counter register */
-
-	/**
-	 * List of events
-	 */
-	struct list_head vblank_event_list;
-	spinlock_t event_lock;
-
-	/*@} */
-
-	struct drm_agp_head *agp;	/**< AGP data */
-
-	struct pci_dev *pdev;		/**< PCI device structure */
-#ifdef __alpha__
-	struct pci_controller *hose;
-#endif
-
-	struct drm_sg_mem *sg;	/**< Scatter gather memory */
-	unsigned int num_crtcs;                  /**< Number of CRTCs on this device */
-
-	struct {
-		int context;
-		struct drm_hw_lock *lock;
-	} sigdata;
-
-	struct drm_local_map *agp_buffer_map;
-	unsigned int agp_buffer_token;
-
-	struct drm_mode_config mode_config;	/**< Current mode config */
-
-	/** \name GEM information */
-	/*@{ */
-	struct mutex object_name_lock;
-	struct idr object_name_idr;
-	struct drm_vma_offset_manager *vma_offset_manager;
-	/*@} */
-	int switch_power_state;
-};
-
-/**
  * drm_drv_uses_atomic_modeset - check if the driver implements
  * atomic_commit()
  * @dev: DRM device
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
new file mode 100644
index 000000000000..e21af87a2f3c
--- /dev/null
+++ b/include/drm/drm_device.h
@@ -0,0 +1,190 @@
+#ifndef _DRM_DEVICE_H_
+#define _DRM_DEVICE_H_
+
+#include <linux/list.h>
+#include <linux/kref.h>
+#include <linux/mutex.h>
+#include <linux/idr.h>
+
+#include <drm/drm_hashtab.h>
+#include <drm/drm_mode_config.h>
+
+struct drm_driver;
+struct drm_minor;
+struct drm_master;
+struct drm_device_dma;
+struct drm_vblank_crtc;
+struct drm_sg_mem;
+struct drm_local_map;
+struct drm_vma_offset_manager;
+
+struct inode;
+
+struct pci_dev;
+struct pci_controller;
+
+/**
+ * DRM device structure. This structure represent a complete card that
+ * may contain multiple heads.
+ */
+struct drm_device {
+	struct list_head legacy_dev_list;/**< list of devices per driver for stealth attach cleanup */
+	int if_version;			/**< Highest interface version set */
+
+	/** \name Lifetime Management */
+	/*@{ */
+	struct kref ref;		/**< Object ref-count */
+	struct device *dev;		/**< Device structure of bus-device */
+	struct drm_driver *driver;	/**< DRM driver managing the device */
+	void *dev_private;		/**< DRM driver private data */
+	struct drm_minor *control;		/**< Control node */
+	struct drm_minor *primary;		/**< Primary node */
+	struct drm_minor *render;		/**< Render node */
+	bool registered;
+
+	/* currently active master for this device. Protected by master_mutex */
+	struct drm_master *master;
+
+	atomic_t unplugged;			/**< Flag whether dev is dead */
+	struct inode *anon_inode;		/**< inode for private address-space */
+	char *unique;				/**< unique name of the device */
+	/*@} */
+
+	/** \name Locks */
+	/*@{ */
+	struct mutex struct_mutex;	/**< For others */
+	struct mutex master_mutex;      /**< For drm_minor::master and drm_file::is_master */
+	/*@} */
+
+	/** \name Usage Counters */
+	/*@{ */
+	int open_count;			/**< Outstanding files open, protected by drm_global_mutex. */
+	spinlock_t buf_lock;		/**< For drm_device::buf_use and a few other things. */
+	int buf_use;			/**< Buffers in use -- cannot alloc */
+	atomic_t buf_alloc;		/**< Buffer allocation in progress */
+	/*@} */
+
+	struct mutex filelist_mutex;
+	struct list_head filelist;
+
+	/** \name Memory management */
+	/*@{ */
+	struct list_head maplist;	/**< Linked list of regions */
+	struct drm_open_hash map_hash;	/**< User token hash table for maps */
+
+	/** \name Context handle management */
+	/*@{ */
+	struct list_head ctxlist;	/**< Linked list of context handles */
+	struct mutex ctxlist_mutex;	/**< For ctxlist */
+
+	struct idr ctx_idr;
+
+	struct list_head vmalist;	/**< List of vmas (for debugging) */
+
+	/*@} */
+
+	/** \name DMA support */
+	/*@{ */
+	struct drm_device_dma *dma;		/**< Optional pointer for DMA support */
+	/*@} */
+
+	/** \name Context support */
+	/*@{ */
+
+	__volatile__ long context_flag;	/**< Context swapping flag */
+	int last_context;		/**< Last current context */
+	/*@} */
+
+	/**
+	 * @irq_enabled:
+	 *
+	 * Indicates that interrupt handling is enabled, specifically vblank
+	 * handling. Drivers which don't use drm_irq_install() need to set this
+	 * to true manually.
+	 */
+	bool irq_enabled;
+	int irq;
+
+	/**
+	 * @vblank_disable_immediate:
+	 *
+	 * If true, vblank interrupt will be disabled immediately when the
+	 * refcount drops to zero, as opposed to via the vblank disable
+	 * timer.
+	 *
+	 * This can be set to true it the hardware has a working vblank counter
+	 * with high-precision timestamping (otherwise there are races) and the
+	 * driver uses drm_crtc_vblank_on() and drm_crtc_vblank_off()
+	 * appropriately. See also @max_vblank_count and
+	 * &drm_crtc_funcs.get_vblank_counter.
+	 */
+	bool vblank_disable_immediate;
+
+	/**
+	 * @vblank:
+	 *
+	 * Array of vblank tracking structures, one per &struct drm_crtc. For
+	 * historical reasons (vblank support predates kernel modesetting) this
+	 * is free-standing and not part of &struct drm_crtc itself. It must be
+	 * initialized explicitly by calling drm_vblank_init().
+	 */
+	struct drm_vblank_crtc *vblank;
+
+	spinlock_t vblank_time_lock;    /**< Protects vblank count and time updates during vblank enable/disable */
+	spinlock_t vbl_lock;
+
+	/**
+	 * @max_vblank_count:
+	 *
+	 * Maximum value of the vblank registers. This value +1 will result in a
+	 * wrap-around of the vblank register. It is used by the vblank core to
+	 * handle wrap-arounds.
+	 *
+	 * If set to zero the vblank core will try to guess the elapsed vblanks
+	 * between times when the vblank interrupt is disabled through
+	 * high-precision timestamps. That approach is suffering from small
+	 * races and imprecision over longer time periods, hence exposing a
+	 * hardware vblank counter is always recommended.
+	 *
+	 * If non-zeor, &drm_crtc_funcs.get_vblank_counter must be set.
+	 */
+	u32 max_vblank_count;           /**< size of vblank counter register */
+
+	/**
+	 * List of events
+	 */
+	struct list_head vblank_event_list;
+	spinlock_t event_lock;
+
+	/*@} */
+
+	struct drm_agp_head *agp;	/**< AGP data */
+
+	struct pci_dev *pdev;		/**< PCI device structure */
+#ifdef __alpha__
+	struct pci_controller *hose;
+#endif
+
+	struct drm_sg_mem *sg;	/**< Scatter gather memory */
+	unsigned int num_crtcs;                  /**< Number of CRTCs on this device */
+
+	struct {
+		int context;
+		struct drm_hw_lock *lock;
+	} sigdata;
+
+	struct drm_local_map *agp_buffer_map;
+	unsigned int agp_buffer_token;
+
+	struct drm_mode_config mode_config;	/**< Current mode config */
+
+	/** \name GEM information */
+	/*@{ */
+	struct mutex object_name_lock;
+	struct idr object_name_idr;
+	struct drm_vma_offset_manager *vma_offset_manager;
+	/*@} */
+	int switch_power_state;
+};
+
+#endif
-- 
2.13.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/4] drm: Document device unplug infrastructure

[Daniel Vetter]

While at it, also ocd and give them a consistent drm_dev_ prefix, like
the other device instance functionality. Plus move the functions into
the right places.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_drv.c                   | 24 +++++++++++++++++++++---
 drivers/gpu/drm/drm_file.c                  |  2 +-
 drivers/gpu/drm/drm_gem.c                   |  2 +-
 drivers/gpu/drm/drm_gem_cma_helper.c        |  2 +-
 drivers/gpu/drm/drm_ioctl.c                 |  4 ++--
 drivers/gpu/drm/drm_vm.c                    |  2 +-
 drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c |  2 +-
 drivers/gpu/drm/udl/udl_connector.c         |  2 +-
 drivers/gpu/drm/udl/udl_drv.c               |  2 +-
 drivers/gpu/drm/udl/udl_fb.c                |  2 +-
 include/drm/drmP.h                          | 13 -------------
 include/drm/drm_drv.h                       | 22 ++++++++++++++++++++--
 12 files changed, 51 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 2ed2d919beae..39191e5c240e 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -291,7 +291,7 @@ struct drm_minor *drm_minor_acquire(unsigned int minor_id)
 
 	if (!minor) {
 		return ERR_PTR(-ENODEV);
-	} else if (drm_device_is_unplugged(minor->dev)) {
+	} else if (drm_dev_is_unplugged(minor->dev)) {
 		drm_dev_unref(minor->dev);
 		return ERR_PTR(-ENODEV);
 	}
@@ -364,7 +364,22 @@ void drm_put_dev(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_put_dev);
 
-void drm_unplug_dev(struct drm_device *dev)
+static void drm_device_set_unplugged(struct drm_device *dev)
+{
+	smp_wmb();
+	atomic_set(&dev->unplugged, 1);
+}
+
+/**
+ * drm_dev_unplug - unplug a DRM device
+ * @dev: DRM device
+ *
+ * This unplugs a hotpluggable DRM device, which makes it inaccessible to
+ * userspace operations. Entry-points can use drm_dev_is_unplugged(). This
+ * essentially unregisters the device like drm_dev_unregister(), but can be
+ * called while there are still open users of @dev.
+ */
+void drm_dev_unplug(struct drm_device *dev)
 {
 	/* for a USB device */
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
@@ -383,7 +398,7 @@ void drm_unplug_dev(struct drm_device *dev)
 	}
 	mutex_unlock(&drm_global_mutex);
 }
-EXPORT_SYMBOL(drm_unplug_dev);
+EXPORT_SYMBOL(drm_dev_unplug);
 
 /*
  * DRM internal mount
@@ -835,6 +850,9 @@ EXPORT_SYMBOL(drm_dev_register);
  * drm_dev_register() but does not deallocate the device. The caller must call
  * drm_dev_unref() 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 should be called first in the device teardown code to make sure
  * userspace can't access the device instance any more.
  */
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 59b75a974357..b3c6e997ccdb 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -436,7 +436,7 @@ int drm_release(struct inode *inode, struct file *filp)
 
 	if (!--dev->open_count) {
 		drm_lastclose(dev);
-		if (drm_device_is_unplugged(dev))
+		if (drm_dev_is_unplugged(dev))
 			drm_put_dev(dev);
 	}
 	mutex_unlock(&drm_global_mutex);
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index a8d396bed6a4..ad4e9cfe48a2 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1001,7 +1001,7 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
 	struct drm_vma_offset_node *node;
 	int ret;
 
-	if (drm_device_is_unplugged(dev))
+	if (drm_dev_is_unplugged(dev))
 		return -ENODEV;
 
 	drm_vma_offset_lock_lookup(dev->vma_offset_manager);
diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
index 275ab872b34f..f4e00571839d 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -390,7 +390,7 @@ unsigned long drm_gem_cma_get_unmapped_area(struct file *filp,
 	struct drm_device *dev = priv->minor->dev;
 	struct drm_vma_offset_node *node;
 
-	if (drm_device_is_unplugged(dev))
+	if (drm_dev_is_unplugged(dev))
 		return -ENODEV;
 
 	drm_vma_offset_lock_lookup(dev->vma_offset_manager);
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 1aad32fe6cda..f7aaa9331243 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -716,7 +716,7 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t *func, void *kdata,
 	struct drm_device *dev = file_priv->minor->dev;
 	int retcode;
 
-	if (drm_device_is_unplugged(dev))
+	if (drm_dev_is_unplugged(dev))
 		return -ENODEV;
 
 	retcode = drm_ioctl_permit(flags, file_priv);
@@ -765,7 +765,7 @@ long drm_ioctl(struct file *filp,
 
 	dev = file_priv->minor->dev;
 
-	if (drm_device_is_unplugged(dev))
+	if (drm_dev_is_unplugged(dev))
 		return -ENODEV;
 
 	is_driver_ioctl = nr >= DRM_COMMAND_BASE && nr < DRM_COMMAND_END;
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index 1170b3209a12..13a59ed2afbc 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -631,7 +631,7 @@ int drm_legacy_mmap(struct file *filp, struct vm_area_struct *vma)
 	struct drm_device *dev = priv->minor->dev;
 	int ret;
 
-	if (drm_device_is_unplugged(dev))
+	if (drm_dev_is_unplugged(dev))
 		return -ENODEV;
 
 	mutex_lock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
index f224b54a30f6..177e9d861001 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
@@ -56,7 +56,7 @@ static const struct drm_connector_helper_funcs tinydrm_connector_hfuncs = {
 static enum drm_connector_status
 tinydrm_connector_detect(struct drm_connector *connector, bool force)
 {
-	if (drm_device_is_unplugged(connector->dev))
+	if (drm_dev_is_unplugged(connector->dev))
 		return connector_status_disconnected;
 
 	return connector->status;
diff --git a/drivers/gpu/drm/udl/udl_connector.c b/drivers/gpu/drm/udl/udl_connector.c
index d2f57c52f7db..9f9a49748d17 100644
--- a/drivers/gpu/drm/udl/udl_connector.c
+++ b/drivers/gpu/drm/udl/udl_connector.c
@@ -96,7 +96,7 @@ static int udl_mode_valid(struct drm_connector *connector,
 static enum drm_connector_status
 udl_detect(struct drm_connector *connector, bool force)
 {
-	if (drm_device_is_unplugged(connector->dev))
+	if (drm_dev_is_unplugged(connector->dev))
 		return connector_status_disconnected;
 	return connector_status_connected;
 }
diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index 0f02e1acf0ba..96c44ede5d69 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -102,7 +102,7 @@ static void udl_usb_disconnect(struct usb_interface *interface)
 	drm_kms_helper_poll_disable(dev);
 	udl_fbdev_unplug(dev);
 	udl_drop_usb(dev);
-	drm_unplug_dev(dev);
+	drm_dev_unplug(dev);
 }
 
 /*
diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
index a5c54dc60def..77a2c59c56a1 100644
--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -198,7 +198,7 @@ static int udl_fb_open(struct fb_info *info, int user)
 	struct udl_device *udl = dev->dev_private;
 
 	/* If the USB device is gone, we don't accept new opens */
-	if (drm_device_is_unplugged(udl->ddev))
+	if (drm_dev_is_unplugged(udl->ddev))
 		return -ENODEV;
 
 	ufbdev->fb_count++;
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 3031c256105e..7277783a4ff0 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -320,19 +320,6 @@ static __inline__ int drm_core_check_feature(struct drm_device *dev,
 	return ((dev->driver->driver_features & feature) ? 1 : 0);
 }
 
-static inline void drm_device_set_unplugged(struct drm_device *dev)
-{
-	smp_wmb();
-	atomic_set(&dev->unplugged, 1);
-}
-
-static inline int drm_device_is_unplugged(struct drm_device *dev)
-{
-	int ret = atomic_read(&dev->unplugged);
-	smp_rmb();
-	return ret;
-}
-
 /******************************************************************/
 /** \name Internal function definitions */
 /*@{*/
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 505c91354802..71bbaaec836d 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -30,7 +30,8 @@
 #include <linux/list.h>
 #include <linux/irqreturn.h>
 
-struct drm_device;
+#include <drm/drm_device.h>
+
 struct drm_file;
 struct drm_gem_object;
 struct drm_master;
@@ -613,7 +614,24 @@ void drm_dev_unregister(struct drm_device *dev);
 void drm_dev_ref(struct drm_device *dev);
 void drm_dev_unref(struct drm_device *dev);
 void drm_put_dev(struct drm_device *dev);
-void drm_unplug_dev(struct drm_device *dev);
+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
+ */
+static inline int drm_dev_is_unplugged(struct drm_device *dev)
+{
+	int ret = atomic_read(&dev->unplugged);
+	smp_rmb();
+	return ret;
+}
+
 
 int drm_dev_set_unique(struct drm_device *dev, const char *name);
 
-- 
2.13.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/4] drm: Only lastclose on unload for legacy drivers

[Daniel Vetter]

The only thing modern drivers are supposed to do in lastclose is
restore the fb emulation state. Which is entirely optional, and
there's really no reason to do that. So restrict it to legacy drivers
(where the driver cleanup essentially happens in lastclose).

This will also allow us to share the unregister function with
drm_dev_unplug().

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 39191e5c240e..694040a240af 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -860,7 +860,8 @@ void drm_dev_unregister(struct drm_device *dev)
 {
 	struct drm_map_list *r_list, *list_temp;
 
-	drm_lastclose(dev);
+	if (drm_core_check_feature(dev, DRIVER_LEGACY))
+		drm_lastclose(dev);
 
 	dev->registered = false;
 
-- 
2.13.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/4] drm: Clean up drm_dev_unplug

[Daniel Vetter]

Use drm_dev_unregister to unregister the interfaces, which also allows
us to simplify the open_count == 0 case.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_drv.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 694040a240af..be38ac7050d4 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -381,21 +381,12 @@ static void drm_device_set_unplugged(struct drm_device *dev)
  */
 void drm_dev_unplug(struct drm_device *dev)
 {
-	/* for a USB device */
-	if (drm_core_check_feature(dev, DRIVER_MODESET))
-		drm_modeset_unregister_all(dev);
-
-	drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
-	drm_minor_unregister(dev, DRM_MINOR_RENDER);
-	drm_minor_unregister(dev, DRM_MINOR_CONTROL);
+	drm_dev_unregister(dev);
 
 	mutex_lock(&drm_global_mutex);
-
 	drm_device_set_unplugged(dev);
-
-	if (dev->open_count == 0) {
-		drm_put_dev(dev);
-	}
+	if (dev->open_count == 0)
+		drm_dev_unref(dev);
 	mutex_unlock(&drm_global_mutex);
 }
 EXPORT_SYMBOL(drm_dev_unplug);
-- 
2.13.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✓ Fi.CI.BAT: success for drm unplugging docs

[Patchwork]

== Series Details ==

Series: drm unplugging docs
URL   : https://patchwork.freedesktop.org/series/28249/
State : success

== Summary ==

Series 28249v1 drm unplugging docs
https://patchwork.freedesktop.org/api/1.0/series/28249/revisions/1/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                skip       -> PASS       (fi-skl-x1585l) fdo#101781

fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781

fi-bdw-5557u     total:280  pass:269  dwarn:0   dfail:0   fail:0   skip:11  time:445s
fi-bdw-gvtdvm    total:280  pass:266  dwarn:0   dfail:0   fail:0   skip:14  time:431s
fi-blb-e6850     total:280  pass:225  dwarn:1   dfail:0   fail:0   skip:54  time:354s
fi-bsw-n3050     total:280  pass:244  dwarn:0   dfail:0   fail:0   skip:36  time:537s
fi-bxt-j4205     total:280  pass:261  dwarn:0   dfail:0   fail:0   skip:19  time:515s
fi-byt-n2820     total:280  pass:251  dwarn:1   dfail:0   fail:0   skip:28  time:504s
fi-glk-2a        total:280  pass:261  dwarn:0   dfail:0   fail:0   skip:19  time:602s
fi-hsw-4770      total:280  pass:264  dwarn:0   dfail:0   fail:0   skip:16  time:445s
fi-hsw-4770r     total:280  pass:264  dwarn:0   dfail:0   fail:0   skip:16  time:419s
fi-ilk-650       total:280  pass:230  dwarn:0   dfail:0   fail:0   skip:50  time:416s
fi-ivb-3520m     total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  time:500s
fi-ivb-3770      total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  time:476s
fi-kbl-7500u     total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  time:466s
fi-kbl-7560u     total:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  time:575s
fi-kbl-r         total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  time:582s
fi-pnv-d510      total:280  pass:224  dwarn:1   dfail:0   fail:0   skip:55  time:567s
fi-skl-6260u     total:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  time:464s
fi-skl-6700hq    total:280  pass:263  dwarn:0   dfail:0   fail:0   skip:17  time:588s
fi-skl-6700k     total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  time:471s
fi-skl-6770hq    total:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  time:483s
fi-skl-gvtdvm    total:280  pass:267  dwarn:0   dfail:0   fail:0   skip:13  time:439s
fi-skl-x1585l    total:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  time:490s
fi-snb-2520m     total:280  pass:252  dwarn:0   dfail:0   fail:0   skip:28  time:546s
fi-snb-2600      total:280  pass:251  dwarn:0   dfail:0   fail:0   skip:29  time:407s

fcb630a80579faf6d12ee62cb49bd7a4acff41e6 drm-tip: 2017y-08m-01d-17h-14m-57s UTC integration manifest
d160a07a2f69 drm: Clean up drm_dev_unplug
3257bf6664f4 drm: Only lastclose on unload for legacy drivers
be7243902433 drm: Document device unplug infrastructure
1acb33f12d14 drm: Extract drm_device.h

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5310/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm: Only lastclose on unload for legacy drivers

[Alex Deucher]

On Wed, Aug 2, 2017 at 7:56 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> The only thing modern drivers are supposed to do in lastclose is
> restore the fb emulation state. Which is entirely optional, and
> there's really no reason to do that. So restrict it to legacy drivers
> (where the driver cleanup essentially happens in lastclose).

vga_switcheroo_process_delayed_switch() gets called in lastclose.
Won't that need to get moved elsewhere for this to work?

Alex

>
> This will also allow us to share the unregister function with
> drm_dev_unplug().
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_drv.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 39191e5c240e..694040a240af 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -860,7 +860,8 @@ void drm_dev_unregister(struct drm_device *dev)
>  {
>         struct drm_map_list *r_list, *list_temp;
>
> -       drm_lastclose(dev);
> +       if (drm_core_check_feature(dev, DRIVER_LEGACY))
> +               drm_lastclose(dev);
>
>         dev->registered = false;
>
> --
> 2.13.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/4] drm: Only lastclose on unload for legacy drivers

[Daniel Vetter]

On Wed, Aug 2, 2017 at 10:50 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Wed, Aug 2, 2017 at 7:56 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> The only thing modern drivers are supposed to do in lastclose is
>> restore the fb emulation state. Which is entirely optional, and
>> there's really no reason to do that. So restrict it to legacy drivers
>> (where the driver cleanup essentially happens in lastclose).
>
> vga_switcheroo_process_delayed_switch() gets called in lastclose.
> Won't that need to get moved elsewhere for this to work?

Hm right, I forgot the lazy way to do runtime pm by keeping the device
alive as long as anyone has an open fd for it ... This shouldn't be a
problem, since you need to unregister from vgaswitcheroo anyway on
unload. Maybe that blows up, I'll check the code and augment the patch
as needed.
-Daniel

>
> Alex
>
>>
>> This will also allow us to share the unregister function with
>> drm_dev_unplug().
>>
>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>> ---
>>  drivers/gpu/drm/drm_drv.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> index 39191e5c240e..694040a240af 100644
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>> @@ -860,7 +860,8 @@ void drm_dev_unregister(struct drm_device *dev)
>>  {
>>         struct drm_map_list *r_list, *list_temp;
>>
>> -       drm_lastclose(dev);
>> +       if (drm_core_check_feature(dev, DRIVER_LEGACY))
>> +               drm_lastclose(dev);
>>
>>         dev->registered = false;
>>
>> --
>> 2.13.3
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm: Only lastclose on unload for legacy drivers

[Daniel Vetter]

On Thu, Aug 3, 2017 at 1:17 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Wed, Aug 2, 2017 at 10:50 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
>> On Wed, Aug 2, 2017 at 7:56 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>>> The only thing modern drivers are supposed to do in lastclose is
>>> restore the fb emulation state. Which is entirely optional, and
>>> there's really no reason to do that. So restrict it to legacy drivers
>>> (where the driver cleanup essentially happens in lastclose).
>>
>> vga_switcheroo_process_delayed_switch() gets called in lastclose.
>> Won't that need to get moved elsewhere for this to work?
>
> Hm right, I forgot the lazy way to do runtime pm by keeping the device
> alive as long as anyone has an open fd for it ... This shouldn't be a
> problem, since you need to unregister from vgaswitcheroo anyway on
> unload. Maybe that blows up, I'll check the code and augment the patch
> as needed.

So I think there's 3 cases:
- Trying to unload the module. You can't do that while anyone has the
fd still open, so lastclose is guaranteeed to run.
- Forcefully unbinding the driver through sysfs. Not any better, since
the can_switch stuff checks for the open count, and so will delay the
delayed switch no matter what.
- Actual hotremoval: a) not implemented since none of the drivers
taking part in vgaswitcheroo correctly unplug the drm device and b)
you can't hotremove a chip from a laptop.

So I think I'm not breaking this anymore than it probably already is :-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/4] drm: Only lastclose on unload for legacy drivers

[Alex Deucher]

On Thu, Aug 3, 2017 at 9:54 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Thu, Aug 3, 2017 at 1:17 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> On Wed, Aug 2, 2017 at 10:50 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
>>> On Wed, Aug 2, 2017 at 7:56 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>>>> The only thing modern drivers are supposed to do in lastclose is
>>>> restore the fb emulation state. Which is entirely optional, and
>>>> there's really no reason to do that. So restrict it to legacy drivers
>>>> (where the driver cleanup essentially happens in lastclose).
>>>
>>> vga_switcheroo_process_delayed_switch() gets called in lastclose.
>>> Won't that need to get moved elsewhere for this to work?
>>
>> Hm right, I forgot the lazy way to do runtime pm by keeping the device
>> alive as long as anyone has an open fd for it ... This shouldn't be a
>> problem, since you need to unregister from vgaswitcheroo anyway on
>> unload. Maybe that blows up, I'll check the code and augment the patch
>> as needed.
>
> So I think there's 3 cases:
> - Trying to unload the module. You can't do that while anyone has the
> fd still open, so lastclose is guaranteeed to run.
> - Forcefully unbinding the driver through sysfs. Not any better, since
> the can_switch stuff checks for the open count, and so will delay the
> delayed switch no matter what.
> - Actual hotremoval: a) not implemented since none of the drivers
> taking part in vgaswitcheroo correctly unplug the drm device and b)
> you can't hotremove a chip from a laptop.
>
> So I think I'm not breaking this anymore than it probably already is :-)

Thanks.  This series is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm: Only lastclose on unload for legacy drivers

[Michel Dänzer]

On 03/08/17 10:54 PM, Daniel Vetter wrote:
> On Thu, Aug 3, 2017 at 1:17 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> On Wed, Aug 2, 2017 at 10:50 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
>>> On Wed, Aug 2, 2017 at 7:56 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>>>> The only thing modern drivers are supposed to do in lastclose is
>>>> restore the fb emulation state. Which is entirely optional, and
>>>> there's really no reason to do that. So restrict it to legacy drivers
>>>> (where the driver cleanup essentially happens in lastclose).
>>>
>>> vga_switcheroo_process_delayed_switch() gets called in lastclose.
>>> Won't that need to get moved elsewhere for this to work?
>>
>> Hm right, I forgot the lazy way to do runtime pm by keeping the device
>> alive as long as anyone has an open fd for it ... This shouldn't be a
>> problem, since you need to unregister from vgaswitcheroo anyway on
>> unload. Maybe that blows up, I'll check the code and augment the patch
>> as needed.
> 
> So I think there's 3 cases:
> - Trying to unload the module. You can't do that while anyone has the
> fd still open, so lastclose is guaranteeed to run.
> - Forcefully unbinding the driver through sysfs. Not any better, since
> the can_switch stuff checks for the open count, and so will delay the
> delayed switch no matter what.

Are you sure that this is working as intended?
https://bugs.freedesktop.org/show_bug.cgi?id=100399 sounds like
unbinding works even while Xorg is using the DRM device.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm: Only lastclose on unload for legacy drivers

[Daniel Vetter]

On Fri, Aug 4, 2017 at 6:12 AM, Michel Dänzer <michel@daenzer.net> wrote:
> On 03/08/17 10:54 PM, Daniel Vetter wrote:
>> On Thu, Aug 3, 2017 at 1:17 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>>> On Wed, Aug 2, 2017 at 10:50 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
>>>> On Wed, Aug 2, 2017 at 7:56 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>>>>> The only thing modern drivers are supposed to do in lastclose is
>>>>> restore the fb emulation state. Which is entirely optional, and
>>>>> there's really no reason to do that. So restrict it to legacy drivers
>>>>> (where the driver cleanup essentially happens in lastclose).
>>>>
>>>> vga_switcheroo_process_delayed_switch() gets called in lastclose.
>>>> Won't that need to get moved elsewhere for this to work?
>>>
>>> Hm right, I forgot the lazy way to do runtime pm by keeping the device
>>> alive as long as anyone has an open fd for it ... This shouldn't be a
>>> problem, since you need to unregister from vgaswitcheroo anyway on
>>> unload. Maybe that blows up, I'll check the code and augment the patch
>>> as needed.
>>
>> So I think there's 3 cases:
>> - Trying to unload the module. You can't do that while anyone has the
>> fd still open, so lastclose is guaranteeed to run.
>> - Forcefully unbinding the driver through sysfs. Not any better, since
>> the can_switch stuff checks for the open count, and so will delay the
>> delayed switch no matter what.
>
> Are you sure that this is working as intended?
> https://bugs.freedesktop.org/show_bug.cgi?id=100399 sounds like
> unbinding works even while Xorg is using the DRM device.

Unbinding the gpu while someone has a file open (like X) is very much
possible, it's how it's designed. That's the forced unplug case. But:
a) amdgpu doesn't implement actual hotremoval support and blows up
b) even in the unload path we still check for can_switch, which will
notice that there's still open fds and not do the delayed switch, so
the code I've removed is already dead in that case.

But yeah, unbinding while X is running is very much possible, it's
exactly what will happen if you hotremove the gpu (from the driver
model pov) physically. The only thing I tried to show is that my patch
doesn't make anything worse :-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm: Only lastclose on unload for legacy drivers

[Daniel Vetter]

On Thu, Aug 03, 2017 at 01:52:55PM -0400, Alex Deucher wrote:
> On Thu, Aug 3, 2017 at 9:54 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > On Thu, Aug 3, 2017 at 1:17 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >> On Wed, Aug 2, 2017 at 10:50 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
> >>> On Wed, Aug 2, 2017 at 7:56 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >>>> The only thing modern drivers are supposed to do in lastclose is
> >>>> restore the fb emulation state. Which is entirely optional, and
> >>>> there's really no reason to do that. So restrict it to legacy drivers
> >>>> (where the driver cleanup essentially happens in lastclose).
> >>>
> >>> vga_switcheroo_process_delayed_switch() gets called in lastclose.
> >>> Won't that need to get moved elsewhere for this to work?
> >>
> >> Hm right, I forgot the lazy way to do runtime pm by keeping the device
> >> alive as long as anyone has an open fd for it ... This shouldn't be a
> >> problem, since you need to unregister from vgaswitcheroo anyway on
> >> unload. Maybe that blows up, I'll check the code and augment the patch
> >> as needed.
> >
> > So I think there's 3 cases:
> > - Trying to unload the module. You can't do that while anyone has the
> > fd still open, so lastclose is guaranteeed to run.
> > - Forcefully unbinding the driver through sysfs. Not any better, since
> > the can_switch stuff checks for the open count, and so will delay the
> > delayed switch no matter what.
> > - Actual hotremoval: a) not implemented since none of the drivers
> > taking part in vgaswitcheroo correctly unplug the drm device and b)
> > you can't hotremove a chip from a laptop.
> >
> > So I think I'm not breaking this anymore than it probably already is :-)

Added the above to the commit message ...
> 
> Thanks.  This series is:
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

... and pulled the entire series in.

Thanks for your review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel