[PATCH v2 00/11] drm/tinydrm: Support device unplug

[PATCH v2 00/11] drm/tinydrm: Support device unplug

[Noralf Trønnes]

This adds device unplug support to drm_fb_helper, drm_fb_cma_helper
(fbdev) and tinydrm.

There are several changes in this version:

I've used Daniel's idea of protecting drm_device.unplugged with srcu to
provide race free drm_dev_unplug().

The fbdev helper unplug patch has become very small with Daniel's help.
Ref is now taken and dropped in the existing helpers, so I could drop
drm_fb_helper_simple_init().

I has annoyed me that fbdev is restored in drm_driver.last_close even if
fbdev isn't used. I've added a patch to fix that. This means I can drop
calling drm_atomic_helper_shutdown() in tinydrm_unregister(), since I
now can easily disable the pipeline from userspace by just closing the
users. Disabled pipeline means balanced regulator_enable/disable.

The 'Embed drm_device in tinydrm_device' patch has gained
drm_driver.release functions after a discussion with Laurent. My
previous version relied on obscure freeing in tinydrm_release().
This means that I didn't retain the ack's.

Laurent also caught an ugly devm_kmalloc() in
tinydrm_display_pipe_init() that I've fixed.

Noralf.

Noralf Trønnes (11):
  drm: Use srcu to protect drm_device.unplugged
  drm/fb-helper: Support device unplug
  drm/fb-helper: Ensure driver module is pinned in fb_open()
  drm/fb-helper: Don't restore if fbdev is not in use
  drm/fb-cma-helper: Make struct drm_fbdev_cma public
  drm/fb-cma-helper: Support device unplug
  drm/tinydrm: Drop driver registered message
  drm/tinydrm: Embed drm_device in tinydrm_device
  drm/tinydrm: Support device unplug in core
  drm/tinydrm/mi0283qt: Let the display pipe handle power
  drm/tinydrm: Support device unplug in drivers

 drivers/gpu/drm/drm_drv.c                   |  54 +++++++++--
 drivers/gpu/drm/drm_fb_cma_helper.c         |  13 +--
 drivers/gpu/drm/drm_fb_helper.c             | 108 ++++++++++++++++++++--
 drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 135 +++++++++++++++++++---------
 drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c |  28 +++---
 drivers/gpu/drm/tinydrm/mi0283qt.c          |  81 +++++------------
 drivers/gpu/drm/tinydrm/mipi-dbi.c          |  58 +++++++++---
 drivers/gpu/drm/tinydrm/repaper.c           |  62 ++++++++-----
 drivers/gpu/drm/tinydrm/st7586.c            |  54 ++++++-----
 include/drm/drm_device.h                    |   9 +-
 include/drm/drm_drv.h                       |  15 +++-
 include/drm/drm_fb_cma_helper.h             |  11 ++-
 include/drm/drm_fb_helper.h                 |  28 ++++++
 include/drm/tinydrm/mipi-dbi.h              |   1 +
 include/drm/tinydrm/tinydrm.h               |  10 ++-
 15 files changed, 469 insertions(+), 198 deletions(-)

-- 
2.7.4

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

[PATCH v2 01/11] drm: Use srcu to protect drm_device.unplugged

[Noralf Trønnes]

Use srcu to protect drm_device.unplugged in a race free manner.
Drivers can use drm_dev_enter()/drm_dev_exit() to protect and mark
sections preventing access to device resources that are not available
after the device is gone.

Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_drv.c | 54 ++++++++++++++++++++++++++++++++++++++++++-----
 include/drm/drm_device.h  |  9 +++++++-
 include/drm/drm_drv.h     | 15 +++++++++----
 3 files changed, 68 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index be38ac7..66ceb0e 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -32,6 +32,7 @@
 #include <linux/moduleparam.h>
 #include <linux/mount.h>
 #include <linux/slab.h>
+#include <linux/srcu.h>
 
 #include <drm/drm_drv.h>
 #include <drm/drmP.h>
@@ -74,6 +75,8 @@ static bool drm_core_init_complete = false;
 
 static struct dentry *drm_debugfs_root;
 
+DEFINE_STATIC_SRCU(drm_unplug_srcu);
+
 #define DRM_PRINTK_FMT "[" DRM_NAME ":%s]%s %pV"
 
 void drm_dev_printk(const struct device *dev, const char *level,
@@ -364,18 +367,51 @@ void drm_put_dev(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_put_dev);
 
-static void drm_device_set_unplugged(struct drm_device *dev)
+/**
+ * drm_dev_enter - Enter device critical section
+ * @dev: DRM device
+ * @idx: Pointer to index that will be passed to the matching drm_dev_exit()
+ *
+ * This function marks and protects the beginning of a section that should not
+ * be entered after the device has been unplugged. The section end is marked
+ * with drm_dev_exit(). Calls to this function can be nested.
+ *
+ * Returns:
+ * True if it is OK to enter the section, false otherwise.
+ */
+bool drm_dev_enter(struct drm_device *dev, int *idx)
+{
+	*idx = srcu_read_lock(&drm_unplug_srcu);
+
+	if (dev->unplugged) {
+		srcu_read_unlock(&drm_unplug_srcu, *idx);
+		return false;
+	}
+
+	return true;
+}
+EXPORT_SYMBOL(drm_dev_enter);
+
+/**
+ * drm_dev_exit - Exit device critical section
+ * @idx: index returned from drm_dev_enter()
+ *
+ * This function marks the end of a section that should not be entered after
+ * the device has been unplugged.
+ */
+void drm_dev_exit(int idx)
 {
-	smp_wmb();
-	atomic_set(&dev->unplugged, 1);
+	srcu_read_unlock(&drm_unplug_srcu, idx);
 }
+EXPORT_SYMBOL(drm_dev_exit);
 
 /**
  * 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
+ * userspace operations. Entry-points can use drm_dev_enter() and
+ * drm_dev_exit() to protect device resources in a race free manner. This
  * essentially unregisters the device like drm_dev_unregister(), but can be
  * called while there are still open users of @dev.
  */
@@ -384,10 +420,18 @@ void drm_dev_unplug(struct drm_device *dev)
 	drm_dev_unregister(dev);
 
 	mutex_lock(&drm_global_mutex);
-	drm_device_set_unplugged(dev);
 	if (dev->open_count == 0)
 		drm_dev_unref(dev);
 	mutex_unlock(&drm_global_mutex);
+
+	/*
+	 * 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);
 }
 EXPORT_SYMBOL(drm_dev_unplug);
 
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index e21af87..b0cabbb 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -45,7 +45,14 @@ struct drm_device {
 	/* currently active master for this device. Protected by master_mutex */
 	struct drm_master *master;
 
-	atomic_t unplugged;			/**< Flag whether dev is dead */
+	/**
+	 * @unplugged:
+	 *
+	 * Flag to tell if the device has been unplugged.
+	 * See drm_dev_enter() and drm_dev_is_unplugged().
+	 */
+	bool unplugged;
+
 	struct inode *anon_inode;		/**< inode for private address-space */
 	char *unique;				/**< unique name of the device */
 	/*@} */
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 71bbaae..101c824 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -614,6 +614,8 @@ 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);
+bool drm_dev_enter(struct drm_device *dev, int *idx);
+void drm_dev_exit(int idx);
 void drm_dev_unplug(struct drm_device *dev);
 
 /**
@@ -625,11 +627,16 @@ void drm_dev_unplug(struct drm_device *dev);
  * 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)
+static inline bool drm_dev_is_unplugged(struct drm_device *dev)
 {
-	int ret = atomic_read(&dev->unplugged);
-	smp_rmb();
-	return ret;
+	int idx;
+
+	if (drm_dev_enter(dev, &idx)) {
+		drm_dev_exit(idx);
+		return false;
+	}
+
+	return true;
 }
 
 
-- 
2.7.4

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

[PATCH v2 02/11] drm/fb-helper: Support device unplug

[Noralf Trønnes]

Support device unplug by taking a ref on drm_device during probe, drop
it in fbops.destroy = drm_fb_helper_fb_destroy().
Use drm_dev_is_unplugged() to block futile operations.

drm_fb_helper_unregister_fbi() can now be called on device removal and
drm_fb_helper_fini() in drm_driver.release.

It turns out that fbdev has the necessary ref counting, so
unregister_framebuffer() together with fb_ops->destroy handles unplug
with open fd's. The ref counting doesn't apply to the fb_info structure
itself, but to use of the fbdev framebuffer.

Analysis of entry points after unregister_framebuffer():
- fbcon has been unbound (through notifier)
- sysfs files removed

First we look at possible operations on open fbdev file handles:

static const struct file_operations fb_fops = {
	.read =		fb_read,
	.write =	fb_write,
	.unlocked_ioctl = fb_ioctl,
	.compat_ioctl = fb_compat_ioctl,
	.mmap =		fb_mmap,
	.open =		fb_open,
	.release =	fb_release,
	.get_unmapped_area = get_fb_unmapped_area,
	.fsync =	fb_deferred_io_fsync,
};

Protected by file_fb_info() (-ENODEV):
fb_read() -> fb_ops.fb_read = drm_fb_helper_sys_read()
fb_write() -> fb_ops.fb_write = drm_fb_helper_sys_write()
fb_ioctl() -> fb_ops.fb_ioctl = drm_fb_helper_ioctl()
fb_compat_ioctl() -> fb_ops.fb_compat_ioctl
fb_mmap() -> fb_ops.fb_mmap

Safe:
fb_release() -> fb_ops.fb_release
get_fb_unmapped_area() : info->screen_base
fb_deferred_io_fsync() : if (info->fbdefio) schedule info->deferred_work

Active mmap's will need the backing buffer to be present.
If deferred IO is used, mmap writes will via a worker generate calls to
drm_fb_helper_deferred_io() which in turn via a worker calls into
drm_fb_helper_dirty_work().

The remaining struct fb_ops operations are safe since they're either
called through fb_ioctl(), fbcon or sysfs.

Next we look at other call paths:

drm_fb_helper_set_suspend{_unlocked}() and
drm_fb_helper_resume_worker():
Calls into fb_set_suspend(), but that's fine since it just uses the
fbdev notifier.

drm_fb_helper_restore_fbdev_mode_unlocked():
Called from drm_driver.last_close.

drm_fb_helper_force_kernel_mode():
Triggered by sysrq.

drm_fb_helper_hotplug_event():
Called by drm_kms_helper_hotplug_event().

Based on this analysis the following functions gets a check:
- drm_fb_helper_restore_fbdev_mode_unlocked()
- drm_fb_helper_force_kernel_mode()
- drm_fb_helper_hotplug_event()
- drm_fb_helper_dirty_work()

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_fb_helper.c | 53 +++++++++++++++++++++++++++++++++++++----
 include/drm/drm_fb_helper.h     |  7 ++++++
 2 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 6a31d13..74c1053 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -498,7 +498,7 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
 	bool do_delayed;
 	int ret;
 
-	if (!drm_fbdev_emulation)
+	if (!drm_fbdev_emulation || drm_dev_is_unplugged(fb_helper->dev))
 		return -ENODEV;
 
 	if (READ_ONCE(fb_helper->deferred_setup))
@@ -563,6 +563,9 @@ static bool drm_fb_helper_force_kernel_mode(void)
 	list_for_each_entry(helper, &kernel_fb_helper_list, kernel_fb_list) {
 		struct drm_device *dev = helper->dev;
 
+		if (drm_dev_is_unplugged(dev))
+			continue;
+
 		if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 			continue;
 
@@ -735,6 +738,9 @@ static void drm_fb_helper_dirty_work(struct work_struct *work)
 	struct drm_clip_rect clip_copy;
 	unsigned long flags;
 
+	if (drm_dev_is_unplugged(helper->dev))
+		return;
+
 	spin_lock_irqsave(&helper->dirty_lock, flags);
 	clip_copy = *clip;
 	clip->x1 = clip->y1 = ~0;
@@ -886,13 +892,24 @@ EXPORT_SYMBOL(drm_fb_helper_alloc_fbi);
  * @fb_helper: driver-allocated fbdev helper
  *
  * A wrapper around unregister_framebuffer, to release the fb_info
- * framebuffer device. This must be called before releasing all resources for
- * @fb_helper by calling drm_fb_helper_fini().
+ * framebuffer device. Unless drm_fb_helper_fb_destroy() set by
+ * DRM_FB_HELPER_DEFAULT_OPS() is used, the ref taken on &drm_device in
+ * drm_fb_helper_initial_config() is dropped. This function must be called
+ * before releasing all resources for @fb_helper by calling
+ * drm_fb_helper_fini().
  */
 void drm_fb_helper_unregister_fbi(struct drm_fb_helper *fb_helper)
 {
-	if (fb_helper && fb_helper->fbdev)
-		unregister_framebuffer(fb_helper->fbdev);
+	struct fb_info *info;
+
+	if (!fb_helper || !fb_helper->fbdev)
+		return;
+
+	info = fb_helper->fbdev;
+	unregister_framebuffer(info);
+	if (!(info->fbops &&
+	      info->fbops->fb_destroy == drm_fb_helper_fb_destroy))
+		drm_dev_unref(fb_helper->dev);
 }
 EXPORT_SYMBOL(drm_fb_helper_unregister_fbi);
 
@@ -1002,6 +1019,24 @@ void drm_fb_helper_deferred_io(struct fb_info *info,
 EXPORT_SYMBOL(drm_fb_helper_deferred_io);
 
 /**
+ * drm_fb_helper_fb_destroy - implementation for &fb_ops.fb_destroy
+ * @info: fbdev registered by the helper
+ *
+ * Drop ref taken on &drm_device in drm_fb_helper_initial_config().
+ *
+ * &fb_ops.fb_destroy is called during unregister_framebuffer() or the last
+ * fb_release() which ever comes last.
+ */
+void drm_fb_helper_fb_destroy(struct fb_info *info)
+{
+	struct drm_fb_helper *fb_helper = info->par;
+
+	DRM_DEBUG("\n");
+	drm_dev_unref(fb_helper->dev);
+}
+EXPORT_SYMBOL(drm_fb_helper_fb_destroy);
+
+/**
  * drm_fb_helper_sys_read - wrapper around fb_sys_read
  * @info: fb_info struct pointer
  * @buf: userspace buffer to read from framebuffer memory
@@ -2496,6 +2531,8 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper,
 	if (ret < 0)
 		return ret;
 
+	drm_dev_ref(dev);
+
 	dev_info(dev->dev, "fb%d: %s frame buffer device\n",
 		 info->node, info->fix.id);
 
@@ -2527,6 +2564,9 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper,
  * drm_fb_helper_fill_fix() are provided as helpers to setup simple default
  * values for the fbdev info structure.
  *
+ * A ref is taken on &drm_device if the framebuffer is registered. This ref is
+ * dropped in drm_fb_helper_unregister_fbi() or drm_fb_helper_fb_destroy().
+ *
  * HANG DEBUGGING:
  *
  * When you have fbcon support built-in or already loaded, this function will do
@@ -2593,6 +2633,9 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
 	if (!drm_fbdev_emulation)
 		return 0;
 
+	if (drm_dev_is_unplugged(fb_helper->dev))
+		return -ENODEV;
+
 	mutex_lock(&fb_helper->lock);
 	if (fb_helper->deferred_setup) {
 		err = __drm_fb_helper_initial_config_and_unlock(fb_helper,
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 33fe959..dd78eb3 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -232,6 +232,7 @@ struct drm_fb_helper {
  * functions. To be used in struct fb_ops of drm drivers.
  */
 #define DRM_FB_HELPER_DEFAULT_OPS \
+	.fb_destroy	= drm_fb_helper_fb_destroy, \
 	.fb_check_var	= drm_fb_helper_check_var, \
 	.fb_set_par	= drm_fb_helper_set_par, \
 	.fb_setcmap	= drm_fb_helper_setcmap, \
@@ -268,6 +269,8 @@ void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper);
 void drm_fb_helper_deferred_io(struct fb_info *info,
 			       struct list_head *pagelist);
 
+void drm_fb_helper_fb_destroy(struct fb_info *info);
+
 ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf,
 			       size_t count, loff_t *ppos);
 ssize_t drm_fb_helper_sys_write(struct fb_info *info, const char __user *buf,
@@ -398,6 +401,10 @@ static inline void drm_fb_helper_deferred_io(struct fb_info *info,
 {
 }
 
+static inline void drm_fb_helper_fb_destroy(struct fb_info *info)
+{
+}
+
 static inline ssize_t drm_fb_helper_sys_read(struct fb_info *info,
 					     char __user *buf, size_t count,
 					     loff_t *ppos)
-- 
2.7.4

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

[PATCH v2 03/11] drm/fb-helper: Ensure driver module is pinned in fb_open()

[Noralf Trønnes]

If struct fb_ops is defined in a library like cma, fb_open() takes a
ref on the library instead of the driver module. Use
fb_ops.fb_open/fb_release to ensure that the driver module is pinned.

Add drm_fb_helper_fb_open() and drm_fb_helper_fb_release() to
DRM_FB_HELPER_DEFAULT_OPS().

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_fb_helper.c | 41 +++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_fb_helper.h     | 14 ++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 74c1053..b080004 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1037,6 +1037,47 @@ void drm_fb_helper_fb_destroy(struct fb_info *info)
 EXPORT_SYMBOL(drm_fb_helper_fb_destroy);
 
 /**
+ * drm_fb_helper_fb_open - implementation for &fb_ops.fb_open
+ * @info: fbdev registered by the helper
+ * @user: 1=userspace, 0=fbcon
+ *
+ * If &fb_ops is wrapped in a library, pin the driver module.
+ */
+int drm_fb_helper_fb_open(struct fb_info *info, int user)
+{
+	struct drm_fb_helper *fb_helper = info->par;
+	struct drm_device *dev = fb_helper->dev;
+
+	/* Skip fbcon, it detaches itself in unregister_framebuffer() */
+	if (user && info->fbops->owner != dev->driver->fops->owner) {
+		if (!try_module_get(dev->driver->fops->owner))
+			return -ENODEV;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_fb_helper_fb_open);
+
+/**
+ * drm_fb_helper_fb_release - implementation for &fb_ops.fb_release
+ * @info: fbdev registered by the helper
+ * @user: 1=userspace, 0=fbcon
+ *
+ * If &fb_ops is wrapped in a library, unpin the driver module.
+ */
+int drm_fb_helper_fb_release(struct fb_info *info, int user)
+{
+	struct drm_fb_helper *fb_helper = info->par;
+	struct drm_device *dev = fb_helper->dev;
+
+	if (user && info->fbops->owner != dev->driver->fops->owner)
+		module_put(dev->driver->fops->owner);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_fb_helper_fb_release);
+
+/**
  * drm_fb_helper_sys_read - wrapper around fb_sys_read
  * @info: fb_info struct pointer
  * @buf: userspace buffer to read from framebuffer memory
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index dd78eb3..b44fc62 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -233,6 +233,8 @@ struct drm_fb_helper {
  */
 #define DRM_FB_HELPER_DEFAULT_OPS \
 	.fb_destroy	= drm_fb_helper_fb_destroy, \
+	.fb_open	= drm_fb_helper_fb_open, \
+	.fb_release	= drm_fb_helper_fb_release, \
 	.fb_check_var	= drm_fb_helper_check_var, \
 	.fb_set_par	= drm_fb_helper_set_par, \
 	.fb_setcmap	= drm_fb_helper_setcmap, \
@@ -270,6 +272,8 @@ void drm_fb_helper_deferred_io(struct fb_info *info,
 			       struct list_head *pagelist);
 
 void drm_fb_helper_fb_destroy(struct fb_info *info);
+int drm_fb_helper_fb_open(struct fb_info *info, int user);
+int drm_fb_helper_fb_release(struct fb_info *info, int user);
 
 ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf,
 			       size_t count, loff_t *ppos);
@@ -405,6 +409,16 @@ static inline void drm_fb_helper_fb_destroy(struct fb_info *info)
 {
 }
 
+static inline int drm_fb_helper_fb_open(struct fb_info *info, int user)
+{
+	return -ENODEV;
+}
+
+static inline int drm_fb_helper_fb_release(struct fb_info *info, int user)
+{
+	return -ENODEV;
+}
+
 static inline ssize_t drm_fb_helper_sys_read(struct fb_info *info,
 					     char __user *buf, size_t count,
 					     loff_t *ppos)
-- 
2.7.4

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

[PATCH v2 04/11] drm/fb-helper: Don't restore if fbdev is not in use

[Noralf Trønnes]

Keep track of fbdev users and only restore fbdev in
drm_fb_helper_restore_fbdev_mode_unlocked() when in use. This avoids
fbdev being restored in drm_driver.last_close when nothing uses it.
Additionally fbdev is turned off when the last user is closing.
fbcon is a user in this context.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_fb_helper.c | 14 ++++++++++++++
 include/drm/drm_fb_helper.h     |  7 +++++++
 2 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index b080004..12a7675 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -504,6 +504,12 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
 	if (READ_ONCE(fb_helper->deferred_setup))
 		return 0;
 
+	/* Don't restore if we track opens and it's zero */
+	if (fb_helper->fbdev && fb_helper->fbdev->fbops &&
+	    fb_helper->fbdev->fbops->fb_open == drm_fb_helper_fb_open &&
+	    !atomic_read(&fb_helper->open_count))
+		return 0;
+
 	mutex_lock(&fb_helper->lock);
 	ret = restore_fbdev_mode(fb_helper);
 
@@ -1041,6 +1047,7 @@ EXPORT_SYMBOL(drm_fb_helper_fb_destroy);
  * @info: fbdev registered by the helper
  * @user: 1=userspace, 0=fbcon
  *
+ * Increase fbdev use count.
  * If &fb_ops is wrapped in a library, pin the driver module.
  */
 int drm_fb_helper_fb_open(struct fb_info *info, int user)
@@ -1054,6 +1061,8 @@ int drm_fb_helper_fb_open(struct fb_info *info, int user)
 			return -ENODEV;
 	}
 
+	atomic_inc(&fb_helper->open_count);
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_fb_helper_fb_open);
@@ -1063,6 +1072,7 @@ EXPORT_SYMBOL(drm_fb_helper_fb_open);
  * @info: fbdev registered by the helper
  * @user: 1=userspace, 0=fbcon
  *
+ * Decrease fbdev use count and turn off if there are no users left.
  * If &fb_ops is wrapped in a library, unpin the driver module.
  */
 int drm_fb_helper_fb_release(struct fb_info *info, int user)
@@ -1070,6 +1080,10 @@ int drm_fb_helper_fb_release(struct fb_info *info, int user)
 	struct drm_fb_helper *fb_helper = info->par;
 	struct drm_device *dev = fb_helper->dev;
 
+	if (atomic_dec_and_test(&fb_helper->open_count) &&
+	    !drm_dev_is_unplugged(fb_helper->dev))
+		drm_fb_helper_blank(FB_BLANK_POWERDOWN, info);
+
 	if (user && info->fbops->owner != dev->driver->fops->owner)
 		module_put(dev->driver->fops->owner);
 
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index b44fc62..5a5509c 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -223,6 +223,13 @@ struct drm_fb_helper {
 	 * See also: @deferred_setup
 	 */
 	int preferred_bpp;
+
+	/**
+	 * @open_count:
+	 *
+	 * Keeps track of fbdev users to know when to restore fbdev.
+	 */
+	atomic_t open_count;
 };
 
 /**
-- 
2.7.4

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

[PATCH v2 05/11] drm/fb-cma-helper: Make struct drm_fbdev_cma public

[Noralf Trønnes]

Make struct drm_fbdev_cma public so we don't have to make more wrappers
to call the drm_fb_helper fbdev wrappers.

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_fb_cma_helper.c |  5 -----
 include/drm/drm_fb_cma_helper.h     | 11 +++++++----
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index f2ee883..a5dc586 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -27,11 +27,6 @@
 
 #define DEFAULT_FBDEFIO_DELAY_MS 50
 
-struct drm_fbdev_cma {
-	struct drm_fb_helper	fb_helper;
-	const struct drm_framebuffer_funcs *fb_funcs;
-};
-
 /**
  * DOC: framebuffer cma helper functions
  *
diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h
index a323781..1a70017 100644
--- a/include/drm/drm_fb_cma_helper.h
+++ b/include/drm/drm_fb_cma_helper.h
@@ -1,20 +1,23 @@
 #ifndef __DRM_FB_CMA_HELPER_H__
 #define __DRM_FB_CMA_HELPER_H__
 
-struct drm_fbdev_cma;
+#include <drm/drm_fb_helper.h>
+
 struct drm_gem_cma_object;
 
-struct drm_fb_helper_surface_size;
 struct drm_framebuffer_funcs;
-struct drm_fb_helper_funcs;
 struct drm_framebuffer;
-struct drm_fb_helper;
 struct drm_device;
 struct drm_file;
 struct drm_mode_fb_cmd2;
 struct drm_plane;
 struct drm_plane_state;
 
+struct drm_fbdev_cma {
+	struct drm_fb_helper fb_helper;
+	const struct drm_framebuffer_funcs *fb_funcs;
+};
+
 struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
 	unsigned int preferred_bpp, unsigned int max_conn_count,
 	const struct drm_framebuffer_funcs *funcs);
-- 
2.7.4

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

[PATCH v2 06/11] drm/fb-cma-helper: Support device unplug

[Noralf Trønnes]

Support using drm_fb_helper_unregister_fbi() in driver.remove prior to
calling drm_fbdev_cma_fini() in drm_driver.release.
Also make drm_fbdev_cma_fini() NULL tolerant.

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_fb_cma_helper.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index a5dc586..2030a6f 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -466,7 +466,13 @@ EXPORT_SYMBOL_GPL(drm_fbdev_cma_init);
  */
 void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma)
 {
-	drm_fb_helper_unregister_fbi(&fbdev_cma->fb_helper);
+	if (!fbdev_cma)
+		return;
+
+	/* Make sure it hasn't been unregistered already */
+	if (fbdev_cma->fb_helper.fbdev && fbdev_cma->fb_helper.fbdev->dev)
+		drm_fb_helper_unregister_fbi(&fbdev_cma->fb_helper);
+
 	if (fbdev_cma->fb_helper.fbdev)
 		drm_fbdev_cma_defio_fini(fbdev_cma->fb_helper.fbdev);
 
-- 
2.7.4

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

[PATCH v2 07/11] drm/tinydrm: Drop driver registered message

[Noralf Trønnes]

No need to put out a driver registered message since drm_dev_register()
does that now. SPI speed is an important metric when dealing with
display problems, so retain that info.

Cc: David Lechner <david@lechnology.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/tinydrm/mi0283qt.c | 14 +-------------
 drivers/gpu/drm/tinydrm/mipi-dbi.c |  2 ++
 drivers/gpu/drm/tinydrm/repaper.c  | 12 ++----------
 drivers/gpu/drm/tinydrm/st7586.c   | 14 +-------------
 4 files changed, 6 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index 7e5bb7d..7fd2691 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -163,7 +163,6 @@ MODULE_DEVICE_TABLE(spi, mi0283qt_id);
 static int mi0283qt_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
-	struct tinydrm_device *tdev;
 	struct mipi_dbi *mipi;
 	struct gpio_desc *dc;
 	u32 rotation = 0;
@@ -215,20 +214,9 @@ static int mi0283qt_probe(struct spi_device *spi)
 		return ret;
 	}
 
-	tdev = &mipi->tinydrm;
-
-	ret = devm_tinydrm_register(tdev);
-	if (ret)
-		return ret;
-
 	spi_set_drvdata(spi, mipi);
 
-	DRM_DEBUG_DRIVER("Initialized %s:%s @%uMHz on minor %d\n",
-			 tdev->drm->driver->name, dev_name(dev),
-			 spi->max_speed_hz / 1000000,
-			 tdev->drm->primary->index);
-
-	return 0;
+	return devm_tinydrm_register(&mipi->tinydrm);
 }
 
 static void mi0283qt_shutdown(struct spi_device *spi)
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index 2caeabc..f0dedc2 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -842,6 +842,8 @@ int mipi_dbi_spi_init(struct spi_device *spi, struct mipi_dbi *mipi,
 			return -ENOMEM;
 	}
 
+	DRM_DEBUG_DRIVER("SPI speed: %uMHz\n", spi->max_speed_hz / 1000000);
+
 	return 0;
 }
 EXPORT_SYMBOL(mipi_dbi_spi_init);
diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c
index 30dc97b..5fbe147 100644
--- a/drivers/gpu/drm/tinydrm/repaper.c
+++ b/drivers/gpu/drm/tinydrm/repaper.c
@@ -1078,19 +1078,11 @@ static int repaper_probe(struct spi_device *spi)
 		return ret;
 
 	drm_mode_config_reset(tdev->drm);
-
-	ret = devm_tinydrm_register(tdev);
-	if (ret)
-		return ret;
-
 	spi_set_drvdata(spi, tdev);
 
-	DRM_DEBUG_DRIVER("Initialized %s:%s @%uMHz on minor %d\n",
-			 tdev->drm->driver->name, dev_name(dev),
-			 spi->max_speed_hz / 1000000,
-			 tdev->drm->primary->index);
+	DRM_DEBUG_DRIVER("SPI speed: %uMHz\n", spi->max_speed_hz / 1000000);
 
-	return 0;
+	return devm_tinydrm_register(tdev);
 }
 
 static void repaper_shutdown(struct spi_device *spi)
diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c
index b439956..07b4d31 100644
--- a/drivers/gpu/drm/tinydrm/st7586.c
+++ b/drivers/gpu/drm/tinydrm/st7586.c
@@ -343,7 +343,6 @@ MODULE_DEVICE_TABLE(spi, st7586_id);
 static int st7586_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
-	struct tinydrm_device *tdev;
 	struct mipi_dbi *mipi;
 	struct gpio_desc *a0;
 	u32 rotation = 0;
@@ -388,20 +387,9 @@ static int st7586_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	tdev = &mipi->tinydrm;
-
-	ret = devm_tinydrm_register(tdev);
-	if (ret)
-		return ret;
-
 	spi_set_drvdata(spi, mipi);
 
-	DRM_DEBUG_DRIVER("Initialized %s:%s @%uMHz on minor %d\n",
-			 tdev->drm->driver->name, dev_name(dev),
-			 spi->max_speed_hz / 1000000,
-			 tdev->drm->primary->index);
-
-	return 0;
+	return devm_tinydrm_register(&mipi->tinydrm);
 }
 
 static void st7586_shutdown(struct spi_device *spi)
-- 
2.7.4

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

Re: [PATCH v2 07/11] drm/tinydrm: Drop driver registered message

[David Lechner]

On 09/08/2017 10:07 AM, Noralf Trønnes wrote:
> No need to put out a driver registered message since drm_dev_register()
> does that now. SPI speed is an important metric when dealing with
> display problems, so retain that info.
> 
> Cc: David Lechner <david@lechnology.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---

Acked-by: David Lechner <david@lechnology.com>

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

Re: [PATCH v2 07/11] drm/tinydrm: Drop driver registered message

[Noralf Trønnes]

Den 09.09.2017 23.33, skrev David Lechner:
> On 09/08/2017 10:07 AM, Noralf Trønnes wrote:
>> No need to put out a driver registered message since drm_dev_register()
>> does that now. SPI speed is an important metric when dealing with
>> display problems, so retain that info.
>>
>> Cc: David Lechner <david@lechnology.com>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>
> Acked-by: David Lechner <david@lechnology.com>
>

Thanks David, applied to drm-misc.

Noralf.

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

[PATCH v2 08/11] drm/tinydrm: Embed drm_device in tinydrm_device

[Noralf Trønnes]

Might as well embed drm_device since tinydrm_device (embeds pipe struct
and fbdev pointer) needs to stick around after driver-device unbind to
handle open fd's after device removal.

tinydrm_release() will be expanded in a later patch.

Cc: David Lechner <david@lechnology.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---

Changes since version 1:
- Add drm_driver.release callbacks
- Free structure if devm_tinydrm_init() fails.
- Probe message has been removed in the previous patch, so no need to
  fix up those variables.

 drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 61 +++++++++++++++++------------
 drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c |  2 +-
 drivers/gpu/drm/tinydrm/mi0283qt.c          |  9 +++--
 drivers/gpu/drm/tinydrm/mipi-dbi.c          | 30 +++++++++++---
 drivers/gpu/drm/tinydrm/repaper.c           | 20 ++++++++--
 drivers/gpu/drm/tinydrm/st7586.c            | 17 ++++----
 include/drm/tinydrm/mipi-dbi.h              |  1 +
 include/drm/tinydrm/tinydrm.h               | 10 ++++-
 8 files changed, 102 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
index 551709e..c8ae2e9 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
@@ -44,7 +44,7 @@
  */
 void tinydrm_lastclose(struct drm_device *drm)
 {
-	struct tinydrm_device *tdev = drm->dev_private;
+	struct tinydrm_device *tdev = drm_to_tinydrm(drm);
 
 	DRM_DEBUG_KMS("\n");
 	drm_fbdev_cma_restore_mode(tdev->fbdev_cma);
@@ -126,7 +126,7 @@ static struct drm_framebuffer *
 tinydrm_fb_create(struct drm_device *drm, struct drm_file *file_priv,
 		  const struct drm_mode_fb_cmd2 *mode_cmd)
 {
-	struct tinydrm_device *tdev = drm->dev_private;
+	struct tinydrm_device *tdev = drm_to_tinydrm(drm);
 
 	return drm_fb_cma_create_with_funcs(drm, file_priv, mode_cmd,
 					    tdev->fb_funcs);
@@ -138,27 +138,37 @@ static const struct drm_mode_config_funcs tinydrm_mode_config_funcs = {
 	.atomic_commit = drm_atomic_helper_commit,
 };
 
+/**
+ * tinydrm_release - DRM driver release helper
+ * @drm: DRM device
+ *
+ * This function finalizes &drm_device. The caller is responsible for freeing
+ * &tinydrm_device.
+ *
+ * Drivers must use this in their &drm_driver->release callback.
+ */
+void tinydrm_release(struct drm_device *drm)
+{
+	DRM_DEBUG_DRIVER("\n");
+
+	drm_dev_fini(drm);
+}
+EXPORT_SYMBOL(tinydrm_release);
+
 static int tinydrm_init(struct device *parent, struct tinydrm_device *tdev,
 			const struct drm_framebuffer_funcs *fb_funcs,
 			struct drm_driver *driver)
 {
-	struct drm_device *drm;
+	struct drm_device *drm = &tdev->drm;
+	int ret;
 
 	mutex_init(&tdev->dirty_lock);
 	tdev->fb_funcs = fb_funcs;
 
-	/*
-	 * We don't embed drm_device, because that prevent us from using
-	 * devm_kzalloc() to allocate tinydrm_device in the driver since
-	 * drm_dev_unref() frees the structure. The devm_ functions provide
-	 * for easy error handling.
-	 */
-	drm = drm_dev_alloc(driver, parent);
-	if (IS_ERR(drm))
-		return PTR_ERR(drm);
-
-	tdev->drm = drm;
-	drm->dev_private = tdev;
+	ret = drm_dev_init(drm, driver, parent);
+	if (ret)
+		return ret;
+
 	drm_mode_config_init(drm);
 	drm->mode_config.funcs = &tinydrm_mode_config_funcs;
 
@@ -167,10 +177,9 @@ static int tinydrm_init(struct device *parent, struct tinydrm_device *tdev,
 
 static void tinydrm_fini(struct tinydrm_device *tdev)
 {
-	drm_mode_config_cleanup(tdev->drm);
+	drm_mode_config_cleanup(&tdev->drm);
 	mutex_destroy(&tdev->dirty_lock);
-	tdev->drm->dev_private = NULL;
-	drm_dev_unref(tdev->drm);
+	drm_dev_unref(&tdev->drm);
 }
 
 static void devm_tinydrm_release(void *data)
@@ -212,12 +221,12 @@ EXPORT_SYMBOL(devm_tinydrm_init);
 
 static int tinydrm_register(struct tinydrm_device *tdev)
 {
-	struct drm_device *drm = tdev->drm;
+	struct drm_device *drm = &tdev->drm;
 	int bpp = drm->mode_config.preferred_depth;
 	struct drm_fbdev_cma *fbdev;
 	int ret;
 
-	ret = drm_dev_register(tdev->drm, 0);
+	ret = drm_dev_register(drm, 0);
 	if (ret)
 		return ret;
 
@@ -236,10 +245,10 @@ static void tinydrm_unregister(struct tinydrm_device *tdev)
 {
 	struct drm_fbdev_cma *fbdev_cma = tdev->fbdev_cma;
 
-	drm_atomic_helper_shutdown(tdev->drm);
+	drm_atomic_helper_shutdown(&tdev->drm);
 	/* don't restore fbdev in lastclose, keep pipeline disabled */
 	tdev->fbdev_cma = NULL;
-	drm_dev_unregister(tdev->drm);
+	drm_dev_unregister(&tdev->drm);
 	if (fbdev_cma)
 		drm_fbdev_cma_fini(fbdev_cma);
 }
@@ -262,7 +271,7 @@ static void devm_tinydrm_register_release(void *data)
  */
 int devm_tinydrm_register(struct tinydrm_device *tdev)
 {
-	struct device *dev = tdev->drm->dev;
+	struct device *dev = tdev->drm.dev;
 	int ret;
 
 	ret = tinydrm_register(tdev);
@@ -287,7 +296,7 @@ EXPORT_SYMBOL(devm_tinydrm_register);
  */
 void tinydrm_shutdown(struct tinydrm_device *tdev)
 {
-	drm_atomic_helper_shutdown(tdev->drm);
+	drm_atomic_helper_shutdown(&tdev->drm);
 }
 EXPORT_SYMBOL(tinydrm_shutdown);
 
@@ -312,7 +321,7 @@ int tinydrm_suspend(struct tinydrm_device *tdev)
 	}
 
 	drm_fbdev_cma_set_suspend_unlocked(tdev->fbdev_cma, 1);
-	state = drm_atomic_helper_suspend(tdev->drm);
+	state = drm_atomic_helper_suspend(&tdev->drm);
 	if (IS_ERR(state)) {
 		drm_fbdev_cma_set_suspend_unlocked(tdev->fbdev_cma, 0);
 		return PTR_ERR(state);
@@ -346,7 +355,7 @@ int tinydrm_resume(struct tinydrm_device *tdev)
 
 	tdev->suspend_state = NULL;
 
-	ret = drm_atomic_helper_resume(tdev->drm, state);
+	ret = drm_atomic_helper_resume(&tdev->drm, state);
 	if (ret) {
 		DRM_ERROR("Error resuming state: %d\n", ret);
 		return ret;
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
index 177e9d8..1bcb43a 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
@@ -198,7 +198,7 @@ tinydrm_display_pipe_init(struct tinydrm_device *tdev,
 			  const struct drm_display_mode *mode,
 			  unsigned int rotation)
 {
-	struct drm_device *drm = tdev->drm;
+	struct drm_device *drm = &tdev->drm;
 	struct drm_display_mode *mode_copy;
 	struct drm_connector *connector;
 	int ret;
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index 7fd2691..d61f987 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -23,7 +23,7 @@
 static int mi0283qt_init(struct mipi_dbi *mipi)
 {
 	struct tinydrm_device *tdev = &mipi->tinydrm;
-	struct device *dev = tdev->drm->dev;
+	struct device *dev = tdev->drm.dev;
 	u8 addr_mode;
 	int ret;
 
@@ -139,6 +139,7 @@ static struct drm_driver mi0283qt_driver = {
 				  DRIVER_ATOMIC,
 	.fops			= &mi0283qt_fops,
 	TINYDRM_GEM_DRIVER_OPS,
+	.release		= mipi_dbi_release,
 	.lastclose		= tinydrm_lastclose,
 	.debugfs_init		= mipi_dbi_debugfs_init,
 	.name			= "mi0283qt",
@@ -168,7 +169,7 @@ static int mi0283qt_probe(struct spi_device *spi)
 	u32 rotation = 0;
 	int ret;
 
-	mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
+	mipi = kzalloc(sizeof(*mipi), GFP_KERNEL);
 	if (!mipi)
 		return -ENOMEM;
 
@@ -200,8 +201,10 @@ static int mi0283qt_probe(struct spi_device *spi)
 
 	ret = mipi_dbi_init(&spi->dev, mipi, &mi0283qt_pipe_funcs,
 			    &mi0283qt_driver, &mi0283qt_mode, rotation);
-	if (ret)
+	if (ret) {
+		kfree(mipi);
 		return ret;
+	}
 
 	ret = mi0283qt_init(mipi);
 	if (ret)
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index f0dedc2..ec8f001 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -199,7 +199,7 @@ static int mipi_dbi_fb_dirty(struct drm_framebuffer *fb,
 			     unsigned int num_clips)
 {
 	struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
-	struct tinydrm_device *tdev = fb->dev->dev_private;
+	struct tinydrm_device *tdev = drm_to_tinydrm(fb->dev);
 	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
 	bool swap = mipi->swap_bytes;
 	struct drm_clip_rect clip;
@@ -285,7 +285,7 @@ EXPORT_SYMBOL(mipi_dbi_pipe_enable);
 
 static void mipi_dbi_blank(struct mipi_dbi *mipi)
 {
-	struct drm_device *drm = mipi->tinydrm.drm;
+	struct drm_device *drm = &mipi->tinydrm.drm;
 	u16 height = drm->mode_config.min_height;
 	u16 width = drm->mode_config.min_width;
 	size_t len = width * height * 2;
@@ -324,6 +324,24 @@ void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe)
 }
 EXPORT_SYMBOL(mipi_dbi_pipe_disable);
 
+/**
+ * mipi_dbi_release - DRM driver release helper
+ * @drm: DRM device
+ *
+ * This function finalizes and frees &mipi_dbi.
+ *
+ * Drivers can use this as their &drm_driver->release callback.
+ */
+void mipi_dbi_release(struct drm_device *drm)
+{
+	struct tinydrm_device *tdev = drm_to_tinydrm(drm);
+	struct mipi_dbi *dbi = mipi_dbi_from_tinydrm(tdev);
+
+	tinydrm_release(drm);
+	kfree(dbi);
+}
+EXPORT_SYMBOL(mipi_dbi_release);
+
 static const uint32_t mipi_dbi_formats[] = {
 	DRM_FORMAT_RGB565,
 	DRM_FORMAT_XRGB8888,
@@ -380,13 +398,13 @@ int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi,
 	if (ret)
 		return ret;
 
-	tdev->drm->mode_config.preferred_depth = 16;
+	tdev->drm.mode_config.preferred_depth = 16;
 	mipi->rotation = rotation;
 
-	drm_mode_config_reset(tdev->drm);
+	drm_mode_config_reset(&tdev->drm);
 
 	DRM_DEBUG_KMS("preferred_depth=%u, rotation = %u\n",
-		      tdev->drm->mode_config.preferred_depth, rotation);
+		      tdev->drm.mode_config.preferred_depth, rotation);
 
 	return 0;
 }
@@ -977,7 +995,7 @@ static const struct drm_info_list mipi_dbi_debugfs_list[] = {
  */
 int mipi_dbi_debugfs_init(struct drm_minor *minor)
 {
-	struct tinydrm_device *tdev = minor->dev->dev_private;
+	struct tinydrm_device *tdev = drm_to_tinydrm(minor->dev);
 	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
 	umode_t mode = S_IFREG | S_IWUSR;
 
diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c
index 5fbe147..9236efb 100644
--- a/drivers/gpu/drm/tinydrm/repaper.c
+++ b/drivers/gpu/drm/tinydrm/repaper.c
@@ -528,7 +528,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb,
 {
 	struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
 	struct dma_buf_attachment *import_attach = cma_obj->base.import_attach;
-	struct tinydrm_device *tdev = fb->dev->dev_private;
+	struct tinydrm_device *tdev = drm_to_tinydrm(fb->dev);
 	struct repaper_epd *epd = epd_from_tinydrm(tdev);
 	struct drm_clip_rect clip;
 	u8 *buf = NULL;
@@ -855,6 +855,15 @@ static const struct drm_simple_display_pipe_funcs repaper_pipe_funcs = {
 	.prepare_fb = tinydrm_display_pipe_prepare_fb,
 };
 
+static void repaper_release(struct drm_device *drm)
+{
+	struct tinydrm_device *tdev = drm_to_tinydrm(drm);
+	struct repaper_epd *epd = epd_from_tinydrm(tdev);
+
+	tinydrm_release(drm);
+	kfree(epd);
+}
+
 static const uint32_t repaper_formats[] = {
 	DRM_FORMAT_XRGB8888,
 };
@@ -894,6 +903,7 @@ static struct drm_driver repaper_driver = {
 				  DRIVER_ATOMIC,
 	.fops			= &repaper_fops,
 	TINYDRM_GEM_DRIVER_OPS,
+	.release		= repaper_release,
 	.name			= "repaper",
 	.desc			= "Pervasive Displays RePaper e-ink panels",
 	.date			= "20170405",
@@ -949,7 +959,7 @@ static int repaper_probe(struct spi_device *spi)
 		}
 	}
 
-	epd = devm_kzalloc(dev, sizeof(*epd), GFP_KERNEL);
+	epd = kzalloc(sizeof(*epd), GFP_KERNEL);
 	if (!epd)
 		return -ENOMEM;
 
@@ -1067,8 +1077,10 @@ static int repaper_probe(struct spi_device *spi)
 	tdev = &epd->tinydrm;
 
 	ret = devm_tinydrm_init(dev, tdev, &repaper_fb_funcs, &repaper_driver);
-	if (ret)
+	if (ret) {
+		kfree(epd);
 		return ret;
+	}
 
 	ret = tinydrm_display_pipe_init(tdev, &repaper_pipe_funcs,
 					DRM_MODE_CONNECTOR_VIRTUAL,
@@ -1077,7 +1089,7 @@ static int repaper_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	drm_mode_config_reset(tdev->drm);
+	drm_mode_config_reset(&tdev->drm);
 	spi_set_drvdata(spi, tdev);
 
 	DRM_DEBUG_DRIVER("SPI speed: %uMHz\n", spi->max_speed_hz / 1000000);
diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c
index 07b4d31..67f5c83 100644
--- a/drivers/gpu/drm/tinydrm/st7586.c
+++ b/drivers/gpu/drm/tinydrm/st7586.c
@@ -112,7 +112,7 @@ static int st7586_fb_dirty(struct drm_framebuffer *fb,
 			   unsigned int color, struct drm_clip_rect *clips,
 			   unsigned int num_clips)
 {
-	struct tinydrm_device *tdev = fb->dev->dev_private;
+	struct tinydrm_device *tdev = drm_to_tinydrm(fb->dev);
 	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
 	struct drm_clip_rect clip;
 	int start, end;
@@ -178,7 +178,7 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
 	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
 	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
 	struct drm_framebuffer *fb = pipe->plane.fb;
-	struct device *dev = tdev->drm->dev;
+	struct device *dev = tdev->drm.dev;
 	int ret;
 	u8 addr_mode;
 
@@ -290,13 +290,13 @@ static int st7586_init(struct device *dev, struct mipi_dbi *mipi,
 	if (ret)
 		return ret;
 
-	tdev->drm->mode_config.preferred_depth = 32;
+	tdev->drm.mode_config.preferred_depth = 32;
 	mipi->rotation = rotation;
 
-	drm_mode_config_reset(tdev->drm);
+	drm_mode_config_reset(&tdev->drm);
 
 	DRM_DEBUG_KMS("preferred_depth=%u, rotation = %u\n",
-		      tdev->drm->mode_config.preferred_depth, rotation);
+		      tdev->drm.mode_config.preferred_depth, rotation);
 
 	return 0;
 }
@@ -319,6 +319,7 @@ static struct drm_driver st7586_driver = {
 				  DRIVER_ATOMIC,
 	.fops			= &st7586_fops,
 	TINYDRM_GEM_DRIVER_OPS,
+	.release		= mipi_dbi_release,
 	.lastclose		= tinydrm_lastclose,
 	.debugfs_init		= mipi_dbi_debugfs_init,
 	.name			= "st7586",
@@ -348,7 +349,7 @@ static int st7586_probe(struct spi_device *spi)
 	u32 rotation = 0;
 	int ret;
 
-	mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
+	mipi = kzalloc(sizeof(*mipi), GFP_KERNEL);
 	if (!mipi)
 		return -ENOMEM;
 
@@ -384,8 +385,10 @@ static int st7586_probe(struct spi_device *spi)
 
 	ret = st7586_init(&spi->dev, mipi, &st7586_pipe_funcs, &st7586_driver,
 			  &st7586_mode, rotation);
-	if (ret)
+	if (ret) {
+		kfree(mipi);
 		return ret;
+	}
 
 	spi_set_drvdata(spi, mipi);
 
diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h
index 83346dd..f03b586 100644
--- a/include/drm/tinydrm/mipi-dbi.h
+++ b/include/drm/tinydrm/mipi-dbi.h
@@ -63,6 +63,7 @@ mipi_dbi_from_tinydrm(struct tinydrm_device *tdev)
 
 int mipi_dbi_spi_init(struct spi_device *spi, struct mipi_dbi *mipi,
 		      struct gpio_desc *dc);
+void mipi_dbi_release(struct drm_device *drm);
 int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi,
 		  const struct drm_simple_display_pipe_funcs *pipe_funcs,
 		  struct drm_driver *driver,
diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h
index 4774fe3..a895ad1 100644
--- a/include/drm/tinydrm/tinydrm.h
+++ b/include/drm/tinydrm/tinydrm.h
@@ -10,6 +10,7 @@
 #ifndef __LINUX_TINYDRM_H
 #define __LINUX_TINYDRM_H
 
+#include <drm/drm_device.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_simple_kms_helper.h>
@@ -24,7 +25,7 @@
  * @fb_funcs: Framebuffer functions used when creating framebuffers
  */
 struct tinydrm_device {
-	struct drm_device *drm;
+	struct drm_device drm;
 	struct drm_simple_display_pipe pipe;
 	struct mutex dirty_lock;
 	struct drm_fbdev_cma *fbdev_cma;
@@ -33,6 +34,12 @@ struct tinydrm_device {
 };
 
 static inline struct tinydrm_device *
+drm_to_tinydrm(struct drm_device *drm)
+{
+	return container_of(drm, struct tinydrm_device, drm);
+}
+
+static inline struct tinydrm_device *
 pipe_to_tinydrm(struct drm_simple_display_pipe *pipe)
 {
 	return container_of(pipe, struct tinydrm_device, pipe);
@@ -87,6 +94,7 @@ struct drm_gem_object *
 tinydrm_gem_cma_prime_import_sg_table(struct drm_device *drm,
 				      struct dma_buf_attachment *attach,
 				      struct sg_table *sgt);
+void tinydrm_release(struct drm_device *drm);
 int devm_tinydrm_init(struct device *parent, struct tinydrm_device *tdev,
 		      const struct drm_framebuffer_funcs *fb_funcs,
 		      struct drm_driver *driver);
-- 
2.7.4

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

[PATCH v2 09/11] drm/tinydrm: Support device unplug in core

[Noralf Trønnes]

Support device unplugging to make tinydrm suitable for USB devices.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 84 +++++++++++++++++++++--------
 drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 26 ++++-----
 2 files changed, 72 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
index c8ae2e9..082b347 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
@@ -32,6 +32,36 @@
  * The driver allocates &tinydrm_device, initializes it using
  * devm_tinydrm_init(), sets up the pipeline using tinydrm_display_pipe_init()
  * and registers the DRM device using devm_tinydrm_register().
+ *
+ * Device unplug
+ * -------------
+ *
+ * tinydrm supports device unplugging when there are still open DRM or fbdev
+ * file handles.
+ *
+ * There are 3 ways for driver-device unbinding to happen:
+ *
+ * - The driver module is unloaded causing the driver to be unregistered.
+ *   This can't happen as long as there are open file handles because a
+ *   reference is taken on the module.
+ *
+ * - The device is removed (USB, Device Tree overlay).
+ *   This can happen at any time.
+ *
+ * - The driver sysfs _unbind_ file can be used to unbind the driver from the
+ *   device. This can happen any time.
+ *
+ * The driver needs to protect device resources from access after the device is
+ * gone. This is done marking the region with drm_dev_enter() and
+ * drm_dev_exit(), typically in &drm_framebuffer_funcs.dirty,
+ * &drm_simple_display_pipe_funcs.enable and \.disable.
+ *
+ * Resources that don't face userspace and are only used with the
+ * device can be setup using devm\_ functions, but &tinydrm_device must be
+ * allocated using plain kzalloc() since it's lifetime can exceed that of the
+ * device.
+ *
+ * The structure should be freed in the &drm_driver->release callback.
  */
 
 /**
@@ -149,9 +179,16 @@ static const struct drm_mode_config_funcs tinydrm_mode_config_funcs = {
  */
 void tinydrm_release(struct drm_device *drm)
 {
+	struct tinydrm_device *tdev = drm_to_tinydrm(drm);
+
 	DRM_DEBUG_DRIVER("\n");
 
+	drm_fbdev_cma_fini(tdev->fbdev_cma);
+
+	drm_mode_config_cleanup(&tdev->drm);
 	drm_dev_fini(drm);
+
+	mutex_destroy(&tdev->dirty_lock);
 }
 EXPORT_SYMBOL(tinydrm_release);
 
@@ -175,16 +212,11 @@ static int tinydrm_init(struct device *parent, struct tinydrm_device *tdev,
 	return 0;
 }
 
-static void tinydrm_fini(struct tinydrm_device *tdev)
-{
-	drm_mode_config_cleanup(&tdev->drm);
-	mutex_destroy(&tdev->dirty_lock);
-	drm_dev_unref(&tdev->drm);
-}
-
 static void devm_tinydrm_release(void *data)
 {
-	tinydrm_fini(data);
+	struct tinydrm_device *tdev = data;
+
+	drm_dev_unref(&tdev->drm);
 }
 
 /**
@@ -194,9 +226,10 @@ static void devm_tinydrm_release(void *data)
  * @fb_funcs: Framebuffer functions
  * @driver: DRM driver
  *
- * This function initializes @tdev, the underlying DRM device and it's
- * mode_config. Resources will be automatically freed on driver detach (devres)
- * using drm_mode_config_cleanup() and drm_dev_unref().
+ * This function initializes @tdev, the underlying DRM device and its
+ * mode_config. drm_dev_unref() is called on driver detach (devres) and when
+ * all refs are dropped, the &drm_driver->release callback is called which in
+ * turn calls tinydrm_release().
  *
  * Returns:
  * Zero on success, negative error code on failure.
@@ -213,7 +246,7 @@ int devm_tinydrm_init(struct device *parent, struct tinydrm_device *tdev,
 
 	ret = devm_add_action(parent, devm_tinydrm_release, tdev);
 	if (ret)
-		tinydrm_fini(tdev);
+		devm_tinydrm_release(tdev);
 
 	return ret;
 }
@@ -243,14 +276,9 @@ static int tinydrm_register(struct tinydrm_device *tdev)
 
 static void tinydrm_unregister(struct tinydrm_device *tdev)
 {
-	struct drm_fbdev_cma *fbdev_cma = tdev->fbdev_cma;
-
-	drm_atomic_helper_shutdown(&tdev->drm);
-	/* don't restore fbdev in lastclose, keep pipeline disabled */
-	tdev->fbdev_cma = NULL;
-	drm_dev_unregister(&tdev->drm);
-	if (fbdev_cma)
-		drm_fbdev_cma_fini(fbdev_cma);
+	if (tdev->fbdev_cma)
+		drm_fb_helper_unregister_fbi(&tdev->fbdev_cma->fb_helper);
+	drm_dev_unplug(&tdev->drm);
 }
 
 static void devm_tinydrm_register_release(void *data)
@@ -279,10 +307,20 @@ int devm_tinydrm_register(struct tinydrm_device *tdev)
 		return ret;
 
 	ret = devm_add_action(dev, devm_tinydrm_register_release, tdev);
-	if (ret)
-		tinydrm_unregister(tdev);
+	if (ret) {
+		devm_tinydrm_register_release(tdev);
+		return ret;
+	}
 
-	return ret;
+	/*
+	 * Take a ref that will be put in devm_tinydrm_release().
+	 * It's done like this so devres cleanup can happen if there's an error
+	 * in the probe function between calling devm_tinydrm_init() and
+	 * devm_tinydrm_register().
+	 */
+	drm_dev_ref(&tdev->drm);
+
+	return 0;
 }
 EXPORT_SYMBOL(devm_tinydrm_register);
 
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
index 1bcb43a..22ef9d9 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
@@ -14,7 +14,7 @@
 
 struct tinydrm_connector {
 	struct drm_connector base;
-	const struct drm_display_mode *mode;
+	struct drm_display_mode mode;
 };
 
 static inline struct tinydrm_connector *
@@ -28,7 +28,7 @@ static int tinydrm_connector_get_modes(struct drm_connector *connector)
 	struct tinydrm_connector *tconn = to_tinydrm_connector(connector);
 	struct drm_display_mode *mode;
 
-	mode = drm_mode_duplicate(connector->dev, tconn->mode);
+	mode = drm_mode_duplicate(connector->dev, &tconn->mode);
 	if (!mode) {
 		DRM_ERROR("Failed to duplicate mode\n");
 		return 0;
@@ -92,7 +92,7 @@ tinydrm_connector_create(struct drm_device *drm,
 	if (!tconn)
 		return ERR_PTR(-ENOMEM);
 
-	tconn->mode = mode;
+	tconn->mode = *mode;
 	connector = &tconn->base;
 
 	drm_connector_helper_add(connector, &tinydrm_connector_hfuncs);
@@ -199,27 +199,23 @@ tinydrm_display_pipe_init(struct tinydrm_device *tdev,
 			  unsigned int rotation)
 {
 	struct drm_device *drm = &tdev->drm;
-	struct drm_display_mode *mode_copy;
+	struct drm_display_mode mode_copy;
 	struct drm_connector *connector;
 	int ret;
 
-	mode_copy = devm_kmalloc(drm->dev, sizeof(*mode_copy), GFP_KERNEL);
-	if (!mode_copy)
-		return -ENOMEM;
-
-	*mode_copy = *mode;
-	ret = tinydrm_rotate_mode(mode_copy, rotation);
+	mode_copy = *mode;
+	ret = tinydrm_rotate_mode(&mode_copy, rotation);
 	if (ret) {
 		DRM_ERROR("Illegal rotation value %u\n", rotation);
 		return -EINVAL;
 	}
 
-	drm->mode_config.min_width = mode_copy->hdisplay;
-	drm->mode_config.max_width = mode_copy->hdisplay;
-	drm->mode_config.min_height = mode_copy->vdisplay;
-	drm->mode_config.max_height = mode_copy->vdisplay;
+	drm->mode_config.min_width = mode_copy.hdisplay;
+	drm->mode_config.max_width = mode_copy.hdisplay;
+	drm->mode_config.min_height = mode_copy.vdisplay;
+	drm->mode_config.max_height = mode_copy.vdisplay;
 
-	connector = tinydrm_connector_create(drm, mode_copy, connector_type);
+	connector = tinydrm_connector_create(drm, &mode_copy, connector_type);
 	if (IS_ERR(connector))
 		return PTR_ERR(connector);
 
-- 
2.7.4

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

[PATCH v2 10/11] drm/tinydrm/mi0283qt: Let the display pipe handle power

[Noralf Trønnes]

It's better to leave power handling and controller init to the
modesetting machinery using the simple pipe .enable and .disable
callbacks.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/tinydrm/mi0283qt.c | 55 +++++++++-----------------------------
 drivers/gpu/drm/tinydrm/mipi-dbi.c | 10 ++++---
 2 files changed, 19 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index d61f987..9495c9a 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -20,9 +20,11 @@
 #include <linux/spi/spi.h>
 #include <video/mipi_display.h>
 
-static int mi0283qt_init(struct mipi_dbi *mipi)
+static void mi0283qt_enable(struct drm_simple_display_pipe *pipe,
+			    struct drm_crtc_state *crtc_state)
 {
-	struct tinydrm_device *tdev = &mipi->tinydrm;
+	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
+	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
 	struct device *dev = tdev->drm.dev;
 	u8 addr_mode;
 	int ret;
@@ -31,20 +33,19 @@ static int mi0283qt_init(struct mipi_dbi *mipi)
 
 	ret = regulator_enable(mipi->regulator);
 	if (ret) {
-		dev_err(dev, "Failed to enable regulator %d\n", ret);
-		return ret;
+		dev_err(dev, "Failed to enable regulator (%d)\n", ret);
+		return;
 	}
 
-	/* Avoid flicker by skipping setup if the bootloader has done it */
 	if (mipi_dbi_display_is_on(mipi))
-		return 0;
+		goto out_enable;
 
 	mipi_dbi_hw_reset(mipi);
 	ret = mipi_dbi_command(mipi, MIPI_DCS_SOFT_RESET);
 	if (ret) {
-		dev_err(dev, "Error sending command %d\n", ret);
+		dev_err(dev, "Error sending command (%d)\n", ret);
 		regulator_disable(mipi->regulator);
-		return ret;
+		return;
 	}
 
 	msleep(20);
@@ -110,19 +111,12 @@ static int mi0283qt_init(struct mipi_dbi *mipi)
 	mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON);
 	msleep(100);
 
-	return 0;
-}
-
-static void mi0283qt_fini(void *data)
-{
-	struct mipi_dbi *mipi = data;
-
-	DRM_DEBUG_KMS("\n");
-	regulator_disable(mipi->regulator);
+out_enable:
+	mipi_dbi_pipe_enable(pipe, crtc_state);
 }
 
 static const struct drm_simple_display_pipe_funcs mi0283qt_pipe_funcs = {
-	.enable = mipi_dbi_pipe_enable,
+	.enable = mi0283qt_enable,
 	.disable = mipi_dbi_pipe_disable,
 	.update = tinydrm_display_pipe_update,
 	.prepare_fb = tinydrm_display_pipe_prepare_fb,
@@ -206,17 +200,6 @@ static int mi0283qt_probe(struct spi_device *spi)
 		return ret;
 	}
 
-	ret = mi0283qt_init(mipi);
-	if (ret)
-		return ret;
-
-	/* use devres to fini after drm unregister (drv->remove is before) */
-	ret = devm_add_action(dev, mi0283qt_fini, mipi);
-	if (ret) {
-		mi0283qt_fini(mipi);
-		return ret;
-	}
-
 	spi_set_drvdata(spi, mipi);
 
 	return devm_tinydrm_register(&mipi->tinydrm);
@@ -232,25 +215,13 @@ static void mi0283qt_shutdown(struct spi_device *spi)
 static int __maybe_unused mi0283qt_pm_suspend(struct device *dev)
 {
 	struct mipi_dbi *mipi = dev_get_drvdata(dev);
-	int ret;
-
-	ret = tinydrm_suspend(&mipi->tinydrm);
-	if (ret)
-		return ret;
 
-	mi0283qt_fini(mipi);
-
-	return 0;
+	return tinydrm_suspend(&mipi->tinydrm);
 }
 
 static int __maybe_unused mi0283qt_pm_resume(struct device *dev)
 {
 	struct mipi_dbi *mipi = dev_get_drvdata(dev);
-	int ret;
-
-	ret = mi0283qt_init(mipi);
-	if (ret)
-		return ret;
 
 	return tinydrm_resume(&mipi->tinydrm);
 }
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index ec8f001..8498b49 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -263,8 +263,9 @@ static const struct drm_framebuffer_funcs mipi_dbi_fb_funcs = {
  * @pipe: Display pipe
  * @crtc_state: CRTC state
  *
- * This function enables backlight. Drivers can use this as their
- * &drm_simple_display_pipe_funcs->enable callback.
+ * This function flushes the whole framebuffer and enables the backlight.
+ * Drivers can use this in their &drm_simple_display_pipe_funcs->enable
+ * callback.
  */
 void mipi_dbi_pipe_enable(struct drm_simple_display_pipe *pipe,
 			  struct drm_crtc_state *crtc_state)
@@ -273,8 +274,6 @@ void mipi_dbi_pipe_enable(struct drm_simple_display_pipe *pipe,
 	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
 	struct drm_framebuffer *fb = pipe->plane.fb;
 
-	DRM_DEBUG_KMS("\n");
-
 	mipi->enabled = true;
 	if (fb)
 		fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0);
@@ -321,6 +320,9 @@ void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe)
 		tinydrm_disable_backlight(mipi->backlight);
 	else
 		mipi_dbi_blank(mipi);
+
+	if (mipi->regulator)
+		regulator_disable(mipi->regulator);
 }
 EXPORT_SYMBOL(mipi_dbi_pipe_disable);
 
-- 
2.7.4

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

[PATCH v2 11/11] drm/tinydrm: Support device unplug in drivers

[Noralf Trønnes]

Protect device resource by adding drm_dev_enter() and drm_dev_exit().

Cc: David Lechner <david@lechnology.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/tinydrm/mi0283qt.c | 11 ++++++++---
 drivers/gpu/drm/tinydrm/mipi-dbi.c | 18 +++++++++++++++++-
 drivers/gpu/drm/tinydrm/repaper.c  | 30 +++++++++++++++++++++++-------
 drivers/gpu/drm/tinydrm/st7586.c   | 23 +++++++++++++++++++----
 4 files changed, 67 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index 9495c9a..f85a404 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -27,14 +27,17 @@ static void mi0283qt_enable(struct drm_simple_display_pipe *pipe,
 	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
 	struct device *dev = tdev->drm.dev;
 	u8 addr_mode;
-	int ret;
+	int ret, idx;
+
+	if (!drm_dev_enter(&tdev->drm, &idx))
+		return;
 
 	DRM_DEBUG_KMS("\n");
 
 	ret = regulator_enable(mipi->regulator);
 	if (ret) {
 		dev_err(dev, "Failed to enable regulator (%d)\n", ret);
-		return;
+		goto exit;
 	}
 
 	if (mipi_dbi_display_is_on(mipi))
@@ -45,7 +48,7 @@ static void mi0283qt_enable(struct drm_simple_display_pipe *pipe,
 	if (ret) {
 		dev_err(dev, "Error sending command (%d)\n", ret);
 		regulator_disable(mipi->regulator);
-		return;
+		goto exit;
 	}
 
 	msleep(20);
@@ -113,6 +116,8 @@ static void mi0283qt_enable(struct drm_simple_display_pipe *pipe,
 
 out_enable:
 	mipi_dbi_pipe_enable(pipe, crtc_state);
+exit:
+	drm_dev_exit(idx);
 }
 
 static const struct drm_simple_display_pipe_funcs mi0283qt_pipe_funcs = {
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index 8498b49..8b72cfb 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -203,10 +203,13 @@ static int mipi_dbi_fb_dirty(struct drm_framebuffer *fb,
 	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
 	bool swap = mipi->swap_bytes;
 	struct drm_clip_rect clip;
-	int ret = 0;
+	int ret = 0, idx;
 	bool full;
 	void *tr;
 
+	if (!drm_dev_enter(fb->dev, &idx))
+		return -ENODEV;
+
 	mutex_lock(&tdev->dirty_lock);
 
 	if (!mipi->enabled)
@@ -244,6 +247,7 @@ static int mipi_dbi_fb_dirty(struct drm_framebuffer *fb,
 
 out_unlock:
 	mutex_unlock(&tdev->dirty_lock);
+	drm_dev_exit(idx);
 
 	if (ret)
 		dev_err_once(fb->dev->dev, "Failed to update display %d\n",
@@ -273,12 +277,18 @@ void mipi_dbi_pipe_enable(struct drm_simple_display_pipe *pipe,
 	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
 	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
 	struct drm_framebuffer *fb = pipe->plane.fb;
+	int idx;
+
+	if (!drm_dev_enter(&tdev->drm, &idx))
+		return;
 
 	mipi->enabled = true;
 	if (fb)
 		fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0);
 
 	tinydrm_enable_backlight(mipi->backlight);
+
+	drm_dev_exit(idx);
 }
 EXPORT_SYMBOL(mipi_dbi_pipe_enable);
 
@@ -311,6 +321,10 @@ void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe)
 {
 	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
 	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+	int idx;
+
+	if (!drm_dev_enter(&tdev->drm, &idx))
+		return;
 
 	DRM_DEBUG_KMS("\n");
 
@@ -323,6 +337,8 @@ void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe)
 
 	if (mipi->regulator)
 		regulator_disable(mipi->regulator);
+
+	drm_dev_exit(idx);
 }
 EXPORT_SYMBOL(mipi_dbi_pipe_disable);
 
diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c
index 9236efb..207a031 100644
--- a/drivers/gpu/drm/tinydrm/repaper.c
+++ b/drivers/gpu/drm/tinydrm/repaper.c
@@ -531,8 +531,11 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb,
 	struct tinydrm_device *tdev = drm_to_tinydrm(fb->dev);
 	struct repaper_epd *epd = epd_from_tinydrm(tdev);
 	struct drm_clip_rect clip;
+	int ret = 0, idx;
 	u8 *buf = NULL;
-	int ret = 0;
+
+	if (!drm_dev_enter(fb->dev, &idx))
+		return -ENODEV;
 
 	/* repaper can't do partial updates */
 	clip.x1 = 0;
@@ -632,6 +635,8 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb,
 		dev_err(fb->dev->dev, "Failed to update display (%d)\n", ret);
 	kfree(buf);
 
+	drm_dev_exit(idx);
+
 	return ret;
 }
 
@@ -666,7 +671,10 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe,
 	struct spi_device *spi = epd->spi;
 	struct device *dev = &spi->dev;
 	bool dc_ok = false;
-	int i, ret;
+	int i, ret, idx;
+
+	if (!drm_dev_enter(&tdev->drm, &idx))
+		return;
 
 	DRM_DEBUG_DRIVER("\n");
 
@@ -705,7 +713,7 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe,
 	if (!i) {
 		dev_err(dev, "timeout waiting for panel to become ready.\n");
 		power_off(epd);
-		return;
+		goto exit;
 	}
 
 	repaper_read_id(spi);
@@ -716,7 +724,7 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe,
 		else
 			dev_err(dev, "wrong COG ID 0x%02x\n", ret);
 		power_off(epd);
-		return;
+		goto exit;
 	}
 
 	/* Disable OE */
@@ -729,7 +737,7 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe,
 		else
 			dev_err(dev, "panel is reported broken\n");
 		power_off(epd);
-		return;
+		goto exit;
 	}
 
 	/* Power saving mode */
@@ -769,7 +777,7 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe,
 		if (ret < 0) {
 			dev_err(dev, "failed to read chip (%d)\n", ret);
 			power_off(epd);
-			return;
+			goto exit;
 		}
 
 		if (ret & 0x40) {
@@ -781,7 +789,7 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe,
 	if (!dc_ok) {
 		dev_err(dev, "dc/dc failed\n");
 		power_off(epd);
-		return;
+		goto exit;
 	}
 
 	/*
@@ -792,6 +800,8 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe,
 
 	epd->enabled = true;
 	epd->partial = false;
+exit:
+	drm_dev_exit(idx);
 }
 
 static void repaper_pipe_disable(struct drm_simple_display_pipe *pipe)
@@ -800,6 +810,10 @@ static void repaper_pipe_disable(struct drm_simple_display_pipe *pipe)
 	struct repaper_epd *epd = epd_from_tinydrm(tdev);
 	struct spi_device *spi = epd->spi;
 	unsigned int line;
+	int idx;
+
+	if (!drm_dev_enter(&tdev->drm, &idx))
+		return;
 
 	DRM_DEBUG_DRIVER("\n");
 
@@ -846,6 +860,8 @@ static void repaper_pipe_disable(struct drm_simple_display_pipe *pipe)
 	msleep(50);
 
 	power_off(epd);
+
+	drm_dev_exit(idx);
 }
 
 static const struct drm_simple_display_pipe_funcs repaper_pipe_funcs = {
diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c
index 67f5c83..874429c 100644
--- a/drivers/gpu/drm/tinydrm/st7586.c
+++ b/drivers/gpu/drm/tinydrm/st7586.c
@@ -115,8 +115,11 @@ static int st7586_fb_dirty(struct drm_framebuffer *fb,
 	struct tinydrm_device *tdev = drm_to_tinydrm(fb->dev);
 	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
 	struct drm_clip_rect clip;
+	int ret = 0, idx;
 	int start, end;
-	int ret = 0;
+
+	if (!drm_dev_enter(fb->dev, &idx))
+		return -ENODEV;
 
 	mutex_lock(&tdev->dirty_lock);
 
@@ -158,6 +161,7 @@ static int st7586_fb_dirty(struct drm_framebuffer *fb,
 
 out_unlock:
 	mutex_unlock(&tdev->dirty_lock);
+	drm_dev_exit(idx);
 
 	if (ret)
 		dev_err_once(fb->dev->dev, "Failed to update display %d\n",
@@ -179,16 +183,19 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
 	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
 	struct drm_framebuffer *fb = pipe->plane.fb;
 	struct device *dev = tdev->drm.dev;
-	int ret;
+	int ret, idx;
 	u8 addr_mode;
 
+	if (!drm_dev_enter(&tdev->drm, &idx))
+		return;
+
 	DRM_DEBUG_KMS("\n");
 
 	mipi_dbi_hw_reset(mipi);
 	ret = mipi_dbi_command(mipi, ST7586_AUTO_READ_CTRL, 0x9f);
 	if (ret) {
 		dev_err(dev, "Error sending command %d\n", ret);
-		return;
+		goto exit;
 	}
 
 	mipi_dbi_command(mipi, ST7586_OTP_RW_CTRL, 0x00);
@@ -243,20 +250,28 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
 
 	if (fb)
 		fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0);
+exit:
+	drm_dev_exit(idx);
 }
 
 static void st7586_pipe_disable(struct drm_simple_display_pipe *pipe)
 {
 	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
 	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+	int idx;
+
+	if (!drm_dev_enter(&tdev->drm, &idx))
+		return;
 
 	DRM_DEBUG_KMS("\n");
 
 	if (!mipi->enabled)
-		return;
+		goto exit;
 
 	mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_OFF);
 	mipi->enabled = false;
+exit:
+	drm_dev_exit(idx);
 }
 
 static const u32 st7586_formats[] = {
-- 
2.7.4

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

Re: [PATCH v2 00/11] drm/tinydrm: Support device unplug

[Daniel Vetter]

Hi Noralf,

On Fri, Sep 08, 2017 at 05:07:19PM +0200, Noralf Trønnes wrote:
> This adds device unplug support to drm_fb_helper, drm_fb_cma_helper
> (fbdev) and tinydrm.
> 
> There are several changes in this version:
> 
> I've used Daniel's idea of protecting drm_device.unplugged with srcu to
> provide race free drm_dev_unplug().
> 
> The fbdev helper unplug patch has become very small with Daniel's help.
> Ref is now taken and dropped in the existing helpers, so I could drop
> drm_fb_helper_simple_init().
> 
> I has annoyed me that fbdev is restored in drm_driver.last_close even if
> fbdev isn't used. I've added a patch to fix that. This means I can drop
> calling drm_atomic_helper_shutdown() in tinydrm_unregister(), since I
> now can easily disable the pipeline from userspace by just closing the
> users. Disabled pipeline means balanced regulator_enable/disable.
> 
> The 'Embed drm_device in tinydrm_device' patch has gained
> drm_driver.release functions after a discussion with Laurent. My
> previous version relied on obscure freeing in tinydrm_release().
> This means that I didn't retain the ack's.
> 
> Laurent also caught an ugly devm_kmalloc() in
> tinydrm_display_pipe_init() that I've fixed.

I'm practically packing for my 2 weeks of conference travel already, so
only very cursory read of the initial patches for core&fb-helpers. I think
this looks really splendid now.

But I won't have time for review for the next few week, would be good if
you could drag some others into this discussions. Iirc there's recently
been a few different people interested in udl (even some patches I think),
they might be able to help out with testing&review.

Also, would be great if you can submit this to intel-gfx on the next
round, so that our CI can pick it up and give it a solid beating. Touching
critical core paths like in patch 1 is always a bit scary.

Thanks, Daniel

> 
> Noralf.
> 
> Noralf Trønnes (11):
>   drm: Use srcu to protect drm_device.unplugged
>   drm/fb-helper: Support device unplug
>   drm/fb-helper: Ensure driver module is pinned in fb_open()
>   drm/fb-helper: Don't restore if fbdev is not in use
>   drm/fb-cma-helper: Make struct drm_fbdev_cma public
>   drm/fb-cma-helper: Support device unplug
>   drm/tinydrm: Drop driver registered message
>   drm/tinydrm: Embed drm_device in tinydrm_device
>   drm/tinydrm: Support device unplug in core
>   drm/tinydrm/mi0283qt: Let the display pipe handle power
>   drm/tinydrm: Support device unplug in drivers
> 
>  drivers/gpu/drm/drm_drv.c                   |  54 +++++++++--
>  drivers/gpu/drm/drm_fb_cma_helper.c         |  13 +--
>  drivers/gpu/drm/drm_fb_helper.c             | 108 ++++++++++++++++++++--
>  drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 135 +++++++++++++++++++---------
>  drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c |  28 +++---
>  drivers/gpu/drm/tinydrm/mi0283qt.c          |  81 +++++------------
>  drivers/gpu/drm/tinydrm/mipi-dbi.c          |  58 +++++++++---
>  drivers/gpu/drm/tinydrm/repaper.c           |  62 ++++++++-----
>  drivers/gpu/drm/tinydrm/st7586.c            |  54 ++++++-----
>  include/drm/drm_device.h                    |   9 +-
>  include/drm/drm_drv.h                       |  15 +++-
>  include/drm/drm_fb_cma_helper.h             |  11 ++-
>  include/drm/drm_fb_helper.h                 |  28 ++++++
>  include/drm/tinydrm/mipi-dbi.h              |   1 +
>  include/drm/tinydrm/tinydrm.h               |  10 ++-
>  15 files changed, 469 insertions(+), 198 deletions(-)
> 
> -- 
> 2.7.4
> 

-- 
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

Re: [PATCH v2 00/11] drm/tinydrm: Support device unplug

[Daniel Vetter]

On Fri, Sep 8, 2017 at 6:33 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> Hi Noralf,
>
> On Fri, Sep 08, 2017 at 05:07:19PM +0200, Noralf Trønnes wrote:
>> This adds device unplug support to drm_fb_helper, drm_fb_cma_helper
>> (fbdev) and tinydrm.
>>
>> There are several changes in this version:
>>
>> I've used Daniel's idea of protecting drm_device.unplugged with srcu to
>> provide race free drm_dev_unplug().
>>
>> The fbdev helper unplug patch has become very small with Daniel's help.
>> Ref is now taken and dropped in the existing helpers, so I could drop
>> drm_fb_helper_simple_init().
>>
>> I has annoyed me that fbdev is restored in drm_driver.last_close even if
>> fbdev isn't used. I've added a patch to fix that. This means I can drop
>> calling drm_atomic_helper_shutdown() in tinydrm_unregister(), since I
>> now can easily disable the pipeline from userspace by just closing the
>> users. Disabled pipeline means balanced regulator_enable/disable.
>>
>> The 'Embed drm_device in tinydrm_device' patch has gained
>> drm_driver.release functions after a discussion with Laurent. My
>> previous version relied on obscure freeing in tinydrm_release().
>> This means that I didn't retain the ack's.
>>
>> Laurent also caught an ugly devm_kmalloc() in
>> tinydrm_display_pipe_init() that I've fixed.
>
> I'm practically packing for my 2 weeks of conference travel already, so
> only very cursory read of the initial patches for core&fb-helpers. I think
> this looks really splendid now.
>
> But I won't have time for review for the next few week, would be good if
> you could drag some others into this discussions. Iirc there's recently
> been a few different people interested in udl (even some patches I think),
> they might be able to help out with testing&review.
>
> Also, would be great if you can submit this to intel-gfx on the next
> round, so that our CI can pick it up and give it a solid beating. Touching
> critical core paths like in patch 1 is always a bit scary.

While reviewing xen-front's hotunplug handling I just realized this
never landed. Can you pls resend and nag me to review it properly? I'd
really like to get the drm_dev_unplug stuff sorted out better.

Thanks, Daniel

>
> Thanks, Daniel
>
>>
>> Noralf.
>>
>> Noralf Trønnes (11):
>>   drm: Use srcu to protect drm_device.unplugged
>>   drm/fb-helper: Support device unplug
>>   drm/fb-helper: Ensure driver module is pinned in fb_open()
>>   drm/fb-helper: Don't restore if fbdev is not in use
>>   drm/fb-cma-helper: Make struct drm_fbdev_cma public
>>   drm/fb-cma-helper: Support device unplug
>>   drm/tinydrm: Drop driver registered message
>>   drm/tinydrm: Embed drm_device in tinydrm_device
>>   drm/tinydrm: Support device unplug in core
>>   drm/tinydrm/mi0283qt: Let the display pipe handle power
>>   drm/tinydrm: Support device unplug in drivers
>>
>>  drivers/gpu/drm/drm_drv.c                   |  54 +++++++++--
>>  drivers/gpu/drm/drm_fb_cma_helper.c         |  13 +--
>>  drivers/gpu/drm/drm_fb_helper.c             | 108 ++++++++++++++++++++--
>>  drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 135 +++++++++++++++++++---------
>>  drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c |  28 +++---
>>  drivers/gpu/drm/tinydrm/mi0283qt.c          |  81 +++++------------
>>  drivers/gpu/drm/tinydrm/mipi-dbi.c          |  58 +++++++++---
>>  drivers/gpu/drm/tinydrm/repaper.c           |  62 ++++++++-----
>>  drivers/gpu/drm/tinydrm/st7586.c            |  54 ++++++-----
>>  include/drm/drm_device.h                    |   9 +-
>>  include/drm/drm_drv.h                       |  15 +++-
>>  include/drm/drm_fb_cma_helper.h             |  11 ++-
>>  include/drm/drm_fb_helper.h                 |  28 ++++++
>>  include/drm/tinydrm/mipi-dbi.h              |   1 +
>>  include/drm/tinydrm/tinydrm.h               |  10 ++-
>>  15 files changed, 469 insertions(+), 198 deletions(-)
>>
>> --
>> 2.7.4
>>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch



-- 
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 v2 00/11] drm/tinydrm: Support device unplug

[Noralf Trønnes]

Den 16.03.2018 09.03, skrev Daniel Vetter:
> On Fri, Sep 8, 2017 at 6:33 PM, Daniel Vetter<daniel@ffwll.ch>  wrote:
>> Hi Noralf,
>>
>> On Fri, Sep 08, 2017 at 05:07:19PM +0200, Noralf Trønnes wrote:
>>> This adds device unplug support to drm_fb_helper, drm_fb_cma_helper
>>> (fbdev) and tinydrm.
>>>
>>> There are several changes in this version:
>>>
>>> I've used Daniel's idea of protecting drm_device.unplugged with srcu to
>>> provide race free drm_dev_unplug().
>>>
>>> The fbdev helper unplug patch has become very small with Daniel's help.
>>> Ref is now taken and dropped in the existing helpers, so I could drop
>>> drm_fb_helper_simple_init().
>>>
>>> I has annoyed me that fbdev is restored in drm_driver.last_close even if
>>> fbdev isn't used. I've added a patch to fix that. This means I can drop
>>> calling drm_atomic_helper_shutdown() in tinydrm_unregister(), since I
>>> now can easily disable the pipeline from userspace by just closing the
>>> users. Disabled pipeline means balanced regulator_enable/disable.
>>>
>>> The 'Embed drm_device in tinydrm_device' patch has gained
>>> drm_driver.release functions after a discussion with Laurent. My
>>> previous version relied on obscure freeing in tinydrm_release().
>>> This means that I didn't retain the ack's.
>>>
>>> Laurent also caught an ugly devm_kmalloc() in
>>> tinydrm_display_pipe_init() that I've fixed.
>> I'm practically packing for my 2 weeks of conference travel already, so
>> only very cursory read of the initial patches for core&fb-helpers. I think
>> this looks really splendid now.
>>
>> But I won't have time for review for the next few week, would be good if
>> you could drag some others into this discussions. Iirc there's recently
>> been a few different people interested in udl (even some patches I think),
>> they might be able to help out with testing&review.
>>
>> Also, would be great if you can submit this to intel-gfx on the next
>> round, so that our CI can pick it up and give it a solid beating. Touching
>> critical core paths like in patch 1 is always a bit scary.
> While reviewing xen-front's hotunplug handling I just realized this
> never landed. Can you pls resend and nag me to review it properly? I'd
> really like to get the drm_dev_unplug stuff sorted out better.

My plan was to pick this up after switching tinydrm over to vmalloc 
buffers,
but that work is now waiting for the generic fbdev emulation to land.

I'm currently wandering around inside drm_fb_helper looking for a way out,
I can feel the draft so there has to be an exit somewhere. I hope that in
a week or two I'm done with the next version of the RFC using the
drm_fb_helper mode setting code instead of the ioctl's.

At that point it will be a good thing to flush my "caches" of the
drm_fb_helper code, since after looking at it for a long time, I really
don't see the details anymore. So I'll pick up the unplug series then, at
least the core patches should be trivial to review and get merged if the CI
agrees.

Noralf.

> Thanks, Daniel
>
>> Thanks, Daniel
>>
>>> Noralf.
>>>
>>> Noralf Trønnes (11):
>>>    drm: Use srcu to protect drm_device.unplugged
>>>    drm/fb-helper: Support device unplug
>>>    drm/fb-helper: Ensure driver module is pinned in fb_open()
>>>    drm/fb-helper: Don't restore if fbdev is not in use
>>>    drm/fb-cma-helper: Make struct drm_fbdev_cma public
>>>    drm/fb-cma-helper: Support device unplug
>>>    drm/tinydrm: Drop driver registered message
>>>    drm/tinydrm: Embed drm_device in tinydrm_device
>>>    drm/tinydrm: Support device unplug in core
>>>    drm/tinydrm/mi0283qt: Let the display pipe handle power
>>>    drm/tinydrm: Support device unplug in drivers
>>>
>>>   drivers/gpu/drm/drm_drv.c                   |  54 +++++++++--
>>>   drivers/gpu/drm/drm_fb_cma_helper.c         |  13 +--
>>>   drivers/gpu/drm/drm_fb_helper.c             | 108 ++++++++++++++++++++--
>>>   drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 135 +++++++++++++++++++---------
>>>   drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c |  28 +++---
>>>   drivers/gpu/drm/tinydrm/mi0283qt.c          |  81 +++++------------
>>>   drivers/gpu/drm/tinydrm/mipi-dbi.c          |  58 +++++++++---
>>>   drivers/gpu/drm/tinydrm/repaper.c           |  62 ++++++++-----
>>>   drivers/gpu/drm/tinydrm/st7586.c            |  54 ++++++-----
>>>   include/drm/drm_device.h                    |   9 +-
>>>   include/drm/drm_drv.h                       |  15 +++-
>>>   include/drm/drm_fb_cma_helper.h             |  11 ++-
>>>   include/drm/drm_fb_helper.h                 |  28 ++++++
>>>   include/drm/tinydrm/mipi-dbi.h              |   1 +
>>>   include/drm/tinydrm/tinydrm.h               |  10 ++-
>>>   15 files changed, 469 insertions(+), 198 deletions(-)
>>>
>>> --
>>> 2.7.4
>>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
>
> -- 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 v2 00/11] drm/tinydrm: Support device unplug

[Daniel Vetter]

On Sat, Mar 17, 2018 at 03:40:29PM +0100, Noralf Trønnes wrote:
> 
> Den 16.03.2018 09.03, skrev Daniel Vetter:
> > On Fri, Sep 8, 2017 at 6:33 PM, Daniel Vetter<daniel@ffwll.ch>  wrote:
> > > Hi Noralf,
> > > 
> > > On Fri, Sep 08, 2017 at 05:07:19PM +0200, Noralf Trønnes wrote:
> > > > This adds device unplug support to drm_fb_helper, drm_fb_cma_helper
> > > > (fbdev) and tinydrm.
> > > > 
> > > > There are several changes in this version:
> > > > 
> > > > I've used Daniel's idea of protecting drm_device.unplugged with srcu to
> > > > provide race free drm_dev_unplug().
> > > > 
> > > > The fbdev helper unplug patch has become very small with Daniel's help.
> > > > Ref is now taken and dropped in the existing helpers, so I could drop
> > > > drm_fb_helper_simple_init().
> > > > 
> > > > I has annoyed me that fbdev is restored in drm_driver.last_close even if
> > > > fbdev isn't used. I've added a patch to fix that. This means I can drop
> > > > calling drm_atomic_helper_shutdown() in tinydrm_unregister(), since I
> > > > now can easily disable the pipeline from userspace by just closing the
> > > > users. Disabled pipeline means balanced regulator_enable/disable.
> > > > 
> > > > The 'Embed drm_device in tinydrm_device' patch has gained
> > > > drm_driver.release functions after a discussion with Laurent. My
> > > > previous version relied on obscure freeing in tinydrm_release().
> > > > This means that I didn't retain the ack's.
> > > > 
> > > > Laurent also caught an ugly devm_kmalloc() in
> > > > tinydrm_display_pipe_init() that I've fixed.
> > > I'm practically packing for my 2 weeks of conference travel already, so
> > > only very cursory read of the initial patches for core&fb-helpers. I think
> > > this looks really splendid now.
> > > 
> > > But I won't have time for review for the next few week, would be good if
> > > you could drag some others into this discussions. Iirc there's recently
> > > been a few different people interested in udl (even some patches I think),
> > > they might be able to help out with testing&review.
> > > 
> > > Also, would be great if you can submit this to intel-gfx on the next
> > > round, so that our CI can pick it up and give it a solid beating. Touching
> > > critical core paths like in patch 1 is always a bit scary.
> > While reviewing xen-front's hotunplug handling I just realized this
> > never landed. Can you pls resend and nag me to review it properly? I'd
> > really like to get the drm_dev_unplug stuff sorted out better.
> 
> My plan was to pick this up after switching tinydrm over to vmalloc buffers,
> but that work is now waiting for the generic fbdev emulation to land.
> 
> I'm currently wandering around inside drm_fb_helper looking for a way out,
> I can feel the draft so there has to be an exit somewhere. I hope that in
> a week or two I'm done with the next version of the RFC using the
> drm_fb_helper mode setting code instead of the ioctl's.
> 
> At that point it will be a good thing to flush my "caches" of the
> drm_fb_helper code, since after looking at it for a long time, I really
> don't see the details anymore. So I'll pick up the unplug series then, at
> least the core patches should be trivial to review and get merged if the CI
> agrees.

Sounds great. I chatted some more with Oleksandr on irc and explained how
he can at least prototype correct unplug code using the current upstream
stuff. He's also willing to help get your stuff landed I think. And afair
the unplug stuff pretty much looked ready for merging already, at least I
don't remember anything big pending.
-Daniel

> 
> Noralf.
> 
> > Thanks, Daniel
> > 
> > > Thanks, Daniel
> > > 
> > > > Noralf.
> > > > 
> > > > Noralf Trønnes (11):
> > > >    drm: Use srcu to protect drm_device.unplugged
> > > >    drm/fb-helper: Support device unplug
> > > >    drm/fb-helper: Ensure driver module is pinned in fb_open()
> > > >    drm/fb-helper: Don't restore if fbdev is not in use
> > > >    drm/fb-cma-helper: Make struct drm_fbdev_cma public
> > > >    drm/fb-cma-helper: Support device unplug
> > > >    drm/tinydrm: Drop driver registered message
> > > >    drm/tinydrm: Embed drm_device in tinydrm_device
> > > >    drm/tinydrm: Support device unplug in core
> > > >    drm/tinydrm/mi0283qt: Let the display pipe handle power
> > > >    drm/tinydrm: Support device unplug in drivers
> > > > 
> > > >   drivers/gpu/drm/drm_drv.c                   |  54 +++++++++--
> > > >   drivers/gpu/drm/drm_fb_cma_helper.c         |  13 +--
> > > >   drivers/gpu/drm/drm_fb_helper.c             | 108 ++++++++++++++++++++--
> > > >   drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 135 +++++++++++++++++++---------
> > > >   drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c |  28 +++---
> > > >   drivers/gpu/drm/tinydrm/mi0283qt.c          |  81 +++++------------
> > > >   drivers/gpu/drm/tinydrm/mipi-dbi.c          |  58 +++++++++---
> > > >   drivers/gpu/drm/tinydrm/repaper.c           |  62 ++++++++-----
> > > >   drivers/gpu/drm/tinydrm/st7586.c            |  54 ++++++-----
> > > >   include/drm/drm_device.h                    |   9 +-
> > > >   include/drm/drm_drv.h                       |  15 +++-
> > > >   include/drm/drm_fb_cma_helper.h             |  11 ++-
> > > >   include/drm/drm_fb_helper.h                 |  28 ++++++
> > > >   include/drm/tinydrm/mipi-dbi.h              |   1 +
> > > >   include/drm/tinydrm/tinydrm.h               |  10 ++-
> > > >   15 files changed, 469 insertions(+), 198 deletions(-)
> > > > 
> > > > --
> > > > 2.7.4
> > > > 
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
> > 
> > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57
> > 48 - http://blog.ffwll.ch
> 

-- 
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

Re: [PATCH v2 00/11] drm/tinydrm: Support device unplug

[Oleksandr Andrushchenko]

On 03/19/2018 03:45 PM, Daniel Vetter wrote:
> On Sat, Mar 17, 2018 at 03:40:29PM +0100, Noralf Trønnes wrote:
>> Den 16.03.2018 09.03, skrev Daniel Vetter:
>>> On Fri, Sep 8, 2017 at 6:33 PM, Daniel Vetter<daniel@ffwll.ch>  wrote:
>>>> Hi Noralf,
>>>>
>>>> On Fri, Sep 08, 2017 at 05:07:19PM +0200, Noralf Trønnes wrote:
>>>>> This adds device unplug support to drm_fb_helper, drm_fb_cma_helper
>>>>> (fbdev) and tinydrm.
>>>>>
>>>>> There are several changes in this version:
>>>>>
>>>>> I've used Daniel's idea of protecting drm_device.unplugged with srcu to
>>>>> provide race free drm_dev_unplug().
>>>>>
>>>>> The fbdev helper unplug patch has become very small with Daniel's help.
>>>>> Ref is now taken and dropped in the existing helpers, so I could drop
>>>>> drm_fb_helper_simple_init().
>>>>>
>>>>> I has annoyed me that fbdev is restored in drm_driver.last_close even if
>>>>> fbdev isn't used. I've added a patch to fix that. This means I can drop
>>>>> calling drm_atomic_helper_shutdown() in tinydrm_unregister(), since I
>>>>> now can easily disable the pipeline from userspace by just closing the
>>>>> users. Disabled pipeline means balanced regulator_enable/disable.
>>>>>
>>>>> The 'Embed drm_device in tinydrm_device' patch has gained
>>>>> drm_driver.release functions after a discussion with Laurent. My
>>>>> previous version relied on obscure freeing in tinydrm_release().
>>>>> This means that I didn't retain the ack's.
>>>>>
>>>>> Laurent also caught an ugly devm_kmalloc() in
>>>>> tinydrm_display_pipe_init() that I've fixed.
>>>> I'm practically packing for my 2 weeks of conference travel already, so
>>>> only very cursory read of the initial patches for core&fb-helpers. I think
>>>> this looks really splendid now.
>>>>
>>>> But I won't have time for review for the next few week, would be good if
>>>> you could drag some others into this discussions. Iirc there's recently
>>>> been a few different people interested in udl (even some patches I think),
>>>> they might be able to help out with testing&review.
>>>>
>>>> Also, would be great if you can submit this to intel-gfx on the next
>>>> round, so that our CI can pick it up and give it a solid beating. Touching
>>>> critical core paths like in patch 1 is always a bit scary.
>>> While reviewing xen-front's hotunplug handling I just realized this
>>> never landed. Can you pls resend and nag me to review it properly? I'd
>>> really like to get the drm_dev_unplug stuff sorted out better.
>> My plan was to pick this up after switching tinydrm over to vmalloc buffers,
>> but that work is now waiting for the generic fbdev emulation to land.
>>
>> I'm currently wandering around inside drm_fb_helper looking for a way out,
>> I can feel the draft so there has to be an exit somewhere. I hope that in
>> a week or two I'm done with the next version of the RFC using the
>> drm_fb_helper mode setting code instead of the ioctl's.
>>
>> At that point it will be a good thing to flush my "caches" of the
>> drm_fb_helper code, since after looking at it for a long time, I really
>> don't see the details anymore. So I'll pick up the unplug series then, at
>> least the core patches should be trivial to review and get merged if the CI
>> agrees.
> Sounds great. I chatted some more with Oleksandr on irc and explained how
> he can at least prototype correct unplug code using the current upstream
> stuff. He's also willing to help get your stuff landed I think.
Yes, I can try helping with this - please let me know
if you need something.
And at least I can plumb that into my driver and have it tested.
>   And afair
> the unplug stuff pretty much looked ready for merging already, at least I
> don't remember anything big pending.
> -Daniel
>
>> Noralf.
>>
>>> Thanks, Daniel
>>>
>>>> Thanks, Daniel
>>>>
>>>>> Noralf.
>>>>>
>>>>> Noralf Trønnes (11):
>>>>>     drm: Use srcu to protect drm_device.unplugged
>>>>>     drm/fb-helper: Support device unplug
>>>>>     drm/fb-helper: Ensure driver module is pinned in fb_open()
>>>>>     drm/fb-helper: Don't restore if fbdev is not in use
>>>>>     drm/fb-cma-helper: Make struct drm_fbdev_cma public
>>>>>     drm/fb-cma-helper: Support device unplug
>>>>>     drm/tinydrm: Drop driver registered message
>>>>>     drm/tinydrm: Embed drm_device in tinydrm_device
>>>>>     drm/tinydrm: Support device unplug in core
>>>>>     drm/tinydrm/mi0283qt: Let the display pipe handle power
>>>>>     drm/tinydrm: Support device unplug in drivers
>>>>>
>>>>>    drivers/gpu/drm/drm_drv.c                   |  54 +++++++++--
>>>>>    drivers/gpu/drm/drm_fb_cma_helper.c         |  13 +--
>>>>>    drivers/gpu/drm/drm_fb_helper.c             | 108 ++++++++++++++++++++--
>>>>>    drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 135 +++++++++++++++++++---------
>>>>>    drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c |  28 +++---
>>>>>    drivers/gpu/drm/tinydrm/mi0283qt.c          |  81 +++++------------
>>>>>    drivers/gpu/drm/tinydrm/mipi-dbi.c          |  58 +++++++++---
>>>>>    drivers/gpu/drm/tinydrm/repaper.c           |  62 ++++++++-----
>>>>>    drivers/gpu/drm/tinydrm/st7586.c            |  54 ++++++-----
>>>>>    include/drm/drm_device.h                    |   9 +-
>>>>>    include/drm/drm_drv.h                       |  15 +++-
>>>>>    include/drm/drm_fb_cma_helper.h             |  11 ++-
>>>>>    include/drm/drm_fb_helper.h                 |  28 ++++++
>>>>>    include/drm/tinydrm/mipi-dbi.h              |   1 +
>>>>>    include/drm/tinydrm/tinydrm.h               |  10 ++-
>>>>>    15 files changed, 469 insertions(+), 198 deletions(-)
>>>>>
>>>>> --
>>>>> 2.7.4
>>>>>
>>>> --
>>>> Daniel Vetter
>>>> Software Engineer, Intel Corporation
>>>> http://blog.ffwll.ch
>>> -- 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 v2 00/11] drm/tinydrm: Support device unplug

[Oleksandr Andrushchenko]

Hi, Noralf!

On 03/17/2018 04:40 PM, Noralf Trønnes wrote:
>
> Den 16.03.2018 09.03, skrev Daniel Vetter:
>> On Fri, Sep 8, 2017 at 6:33 PM, Daniel Vetter<daniel@ffwll.ch>  wrote:
>>> Hi Noralf,
>>>
>>> On Fri, Sep 08, 2017 at 05:07:19PM +0200, Noralf Trønnes wrote:
>>>> This adds device unplug support to drm_fb_helper, drm_fb_cma_helper
>>>> (fbdev) and tinydrm.
>>>>
>>>> There are several changes in this version:
>>>>
>>>> I've used Daniel's idea of protecting drm_device.unplugged with 
>>>> srcu to
>>>> provide race free drm_dev_unplug().
>>>>
>>>> The fbdev helper unplug patch has become very small with Daniel's 
>>>> help.
>>>> Ref is now taken and dropped in the existing helpers, so I could drop
>>>> drm_fb_helper_simple_init().
>>>>
>>>> I has annoyed me that fbdev is restored in drm_driver.last_close 
>>>> even if
>>>> fbdev isn't used. I've added a patch to fix that. This means I can 
>>>> drop
>>>> calling drm_atomic_helper_shutdown() in tinydrm_unregister(), since I
>>>> now can easily disable the pipeline from userspace by just closing the
>>>> users. Disabled pipeline means balanced regulator_enable/disable.
>>>>
>>>> The 'Embed drm_device in tinydrm_device' patch has gained
>>>> drm_driver.release functions after a discussion with Laurent. My
>>>> previous version relied on obscure freeing in tinydrm_release().
>>>> This means that I didn't retain the ack's.
>>>>
>>>> Laurent also caught an ugly devm_kmalloc() in
>>>> tinydrm_display_pipe_init() that I've fixed.
>>> I'm practically packing for my 2 weeks of conference travel already, so
>>> only very cursory read of the initial patches for core&fb-helpers. I 
>>> think
>>> this looks really splendid now.
>>>
>>> But I won't have time for review for the next few week, would be 
>>> good if
>>> you could drag some others into this discussions. Iirc there's recently
>>> been a few different people interested in udl (even some patches I 
>>> think),
>>> they might be able to help out with testing&review.
>>>
>>> Also, would be great if you can submit this to intel-gfx on the next
>>> round, so that our CI can pick it up and give it a solid beating. 
>>> Touching
>>> critical core paths like in patch 1 is always a bit scary.
>> While reviewing xen-front's hotunplug handling I just realized this
>> never landed. Can you pls resend and nag me to review it properly? I'd
>> really like to get the drm_dev_unplug stuff sorted out better.
>
> My plan was to pick this up after switching tinydrm over to vmalloc 
> buffers,
> but that work is now waiting for the generic fbdev emulation to land.
>
> I'm currently wandering around inside drm_fb_helper looking for a way 
> out,
> I can feel the draft so there has to be an exit somewhere. I hope that in
> a week or two I'm done with the next version of the RFC using the
> drm_fb_helper mode setting code instead of the ioctl's.
>
> At that point it will be a good thing to flush my "caches" of the
> drm_fb_helper code, since after looking at it for a long time, I really
> don't see the details anymore. So I'll pick up the unplug series then, at
> least the core patches should be trivial to review and get merged if 
> the CI
> agrees.
Could you please estimate when unplug series can be on review?
I am doing some unplug work in my PV DRM driver and would like
to understand if it is feasible to wait for you or post patches as they are
and then plumb in drm_dev_{enter|exit} later after your work is merged

Thank you,
Oleksandr
>
> Noralf.
>
>> Thanks, Daniel
>>
>>> Thanks, Daniel
>>>
>>>> Noralf.
>>>>
>>>> Noralf Trønnes (11):
>>>>    drm: Use srcu to protect drm_device.unplugged
>>>>    drm/fb-helper: Support device unplug
>>>>    drm/fb-helper: Ensure driver module is pinned in fb_open()
>>>>    drm/fb-helper: Don't restore if fbdev is not in use
>>>>    drm/fb-cma-helper: Make struct drm_fbdev_cma public
>>>>    drm/fb-cma-helper: Support device unplug
>>>>    drm/tinydrm: Drop driver registered message
>>>>    drm/tinydrm: Embed drm_device in tinydrm_device
>>>>    drm/tinydrm: Support device unplug in core
>>>>    drm/tinydrm/mi0283qt: Let the display pipe handle power
>>>>    drm/tinydrm: Support device unplug in drivers
>>>>
>>>>   drivers/gpu/drm/drm_drv.c                   |  54 +++++++++--
>>>>   drivers/gpu/drm/drm_fb_cma_helper.c         |  13 +--
>>>>   drivers/gpu/drm/drm_fb_helper.c             | 108 
>>>> ++++++++++++++++++++--
>>>>   drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 135 
>>>> +++++++++++++++++++---------
>>>>   drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c |  28 +++---
>>>>   drivers/gpu/drm/tinydrm/mi0283qt.c          |  81 +++++------------
>>>>   drivers/gpu/drm/tinydrm/mipi-dbi.c          |  58 +++++++++---
>>>>   drivers/gpu/drm/tinydrm/repaper.c           |  62 ++++++++-----
>>>>   drivers/gpu/drm/tinydrm/st7586.c            |  54 ++++++-----
>>>>   include/drm/drm_device.h                    |   9 +-
>>>>   include/drm/drm_drv.h                       |  15 +++-
>>>>   include/drm/drm_fb_cma_helper.h             |  11 ++-
>>>>   include/drm/drm_fb_helper.h                 |  28 ++++++
>>>>   include/drm/tinydrm/mipi-dbi.h              |   1 +
>>>>   include/drm/tinydrm/tinydrm.h               |  10 ++-
>>>>   15 files changed, 469 insertions(+), 198 deletions(-)
>>>>
>>>> -- 
>>>> 2.7.4
>>>>
>>> -- 
>>> Daniel Vetter
>>> Software Engineer, Intel Corporation
>>> http://blog.ffwll.ch
>>
>> -- 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

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

Re: [PATCH v2 00/11] drm/tinydrm: Support device unplug

[Noralf Trønnes]

Den 26.03.2018 15.02, skrev Oleksandr Andrushchenko:
> Hi, Noralf!
>
> On 03/17/2018 04:40 PM, Noralf Trønnes wrote:
>>
>> Den 16.03.2018 09.03, skrev Daniel Vetter:
>>> On Fri, Sep 8, 2017 at 6:33 PM, Daniel Vetter<daniel@ffwll.ch>  wrote:
>>>> Hi Noralf,
>>>>
>>>> On Fri, Sep 08, 2017 at 05:07:19PM +0200, Noralf Trønnes wrote:
>>>>> This adds device unplug support to drm_fb_helper, drm_fb_cma_helper
>>>>> (fbdev) and tinydrm.
>>>>>
>>>>> There are several changes in this version:
>>>>>
>>>>> I've used Daniel's idea of protecting drm_device.unplugged with 
>>>>> srcu to
>>>>> provide race free drm_dev_unplug().
>>>>>
>>>>> The fbdev helper unplug patch has become very small with Daniel's 
>>>>> help.
>>>>> Ref is now taken and dropped in the existing helpers, so I could drop
>>>>> drm_fb_helper_simple_init().
>>>>>
>>>>> I has annoyed me that fbdev is restored in drm_driver.last_close 
>>>>> even if
>>>>> fbdev isn't used. I've added a patch to fix that. This means I can 
>>>>> drop
>>>>> calling drm_atomic_helper_shutdown() in tinydrm_unregister(), since I
>>>>> now can easily disable the pipeline from userspace by just closing 
>>>>> the
>>>>> users. Disabled pipeline means balanced regulator_enable/disable.
>>>>>
>>>>> The 'Embed drm_device in tinydrm_device' patch has gained
>>>>> drm_driver.release functions after a discussion with Laurent. My
>>>>> previous version relied on obscure freeing in tinydrm_release().
>>>>> This means that I didn't retain the ack's.
>>>>>
>>>>> Laurent also caught an ugly devm_kmalloc() in
>>>>> tinydrm_display_pipe_init() that I've fixed.
>>>> I'm practically packing for my 2 weeks of conference travel 
>>>> already, so
>>>> only very cursory read of the initial patches for core&fb-helpers. 
>>>> I think
>>>> this looks really splendid now.
>>>>
>>>> But I won't have time for review for the next few week, would be 
>>>> good if
>>>> you could drag some others into this discussions. Iirc there's 
>>>> recently
>>>> been a few different people interested in udl (even some patches I 
>>>> think),
>>>> they might be able to help out with testing&review.
>>>>
>>>> Also, would be great if you can submit this to intel-gfx on the next
>>>> round, so that our CI can pick it up and give it a solid beating. 
>>>> Touching
>>>> critical core paths like in patch 1 is always a bit scary.
>>> While reviewing xen-front's hotunplug handling I just realized this
>>> never landed. Can you pls resend and nag me to review it properly? I'd
>>> really like to get the drm_dev_unplug stuff sorted out better.
>>
>> My plan was to pick this up after switching tinydrm over to vmalloc 
>> buffers,
>> but that work is now waiting for the generic fbdev emulation to land.
>>
>> I'm currently wandering around inside drm_fb_helper looking for a way 
>> out,
>> I can feel the draft so there has to be an exit somewhere. I hope 
>> that in
>> a week or two I'm done with the next version of the RFC using the
>> drm_fb_helper mode setting code instead of the ioctl's.
>>
>> At that point it will be a good thing to flush my "caches" of the
>> drm_fb_helper code, since after looking at it for a long time, I really
>> don't see the details anymore. So I'll pick up the unplug series 
>> then, at
>> least the core patches should be trivial to review and get merged if 
>> the CI
>> agrees.
> Could you please estimate when unplug series can be on review?
> I am doing some unplug work in my PV DRM driver and would like
> to understand if it is feasible to wait for you or post patches as 
> they are
> and then plumb in drm_dev_{enter|exit} later after your work is merged
>

Ok, so I have looked at the patches and some work I have lying around.
As things stand now I see an opportunity to move some stuff from tinydrm
into drm_simple_kms_helper as part of adding unplug support to tinydrm.
This also depends on the generic fbdev emulation I'm working on.
This all means that it won't be some trivial tweaking to the unplug
patchset before resending it. It will take some time.

My suggestion is that you just add the core unplug patch as part of your
driver submission instead of waiting for me.

drm: Use srcu to protect drm_device.unplugged
https://patchwork.freedesktop.org/patch/175779/

I believe this patch should be good to go as-is. Please CC
intel-gfx@lists.freedesktop.org if you do so to have the Intel CI verify it.

As for drm_fb_helper unplug support:

drm/fb-helper: Support device unplug
https://patchwork.freedesktop.org/patch/175785/

I'm not as confident in this one since I'm not sure that those
drm_dev_is_unplugged() checks are really necessary. The driver has to do
the check anyways. But this patch isn't necessary for you to do most of
your driver side unplug protection though. You can do testing without
fbdev enabled and let me to care of this when I get around to it.

I you pick up the patch(es) and need to change something, you don't have
to bother with retaining my authorship (but please cc me). Just claim it
for your own and make it work.
Less work for me when I get there (eventually).

Noralf.

> Thank you,
> Oleksandr
>>
>> Noralf.
>>
>>> Thanks, Daniel
>>>
>>>> Thanks, Daniel
>>>>
>>>>> Noralf.
>>>>>
>>>>> Noralf Trønnes (11):
>>>>>    drm: Use srcu to protect drm_device.unplugged
>>>>>    drm/fb-helper: Support device unplug
>>>>>    drm/fb-helper: Ensure driver module is pinned in fb_open()
>>>>>    drm/fb-helper: Don't restore if fbdev is not in use
>>>>>    drm/fb-cma-helper: Make struct drm_fbdev_cma public
>>>>>    drm/fb-cma-helper: Support device unplug
>>>>>    drm/tinydrm: Drop driver registered message
>>>>>    drm/tinydrm: Embed drm_device in tinydrm_device
>>>>>    drm/tinydrm: Support device unplug in core
>>>>>    drm/tinydrm/mi0283qt: Let the display pipe handle power
>>>>>    drm/tinydrm: Support device unplug in drivers
>>>>>
>>>>>   drivers/gpu/drm/drm_drv.c                   |  54 +++++++++--
>>>>>   drivers/gpu/drm/drm_fb_cma_helper.c         |  13 +--
>>>>>   drivers/gpu/drm/drm_fb_helper.c             | 108 
>>>>> ++++++++++++++++++++--
>>>>>   drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 135 
>>>>> +++++++++++++++++++---------
>>>>>   drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c |  28 +++---
>>>>>   drivers/gpu/drm/tinydrm/mi0283qt.c          |  81 +++++------------
>>>>>   drivers/gpu/drm/tinydrm/mipi-dbi.c          |  58 +++++++++---
>>>>>   drivers/gpu/drm/tinydrm/repaper.c           |  62 ++++++++-----
>>>>>   drivers/gpu/drm/tinydrm/st7586.c            |  54 ++++++-----
>>>>>   include/drm/drm_device.h                    |   9 +-
>>>>>   include/drm/drm_drv.h                       |  15 +++-
>>>>>   include/drm/drm_fb_cma_helper.h             |  11 ++-
>>>>>   include/drm/drm_fb_helper.h                 |  28 ++++++
>>>>>   include/drm/tinydrm/mipi-dbi.h              |   1 +
>>>>>   include/drm/tinydrm/tinydrm.h               |  10 ++-
>>>>>   15 files changed, 469 insertions(+), 198 deletions(-)
>>>>>
>>>>> -- 
>>>>> 2.7.4
>>>>>
>>>> -- 
>>>> Daniel Vetter
>>>> Software Engineer, Intel Corporation
>>>> http://blog.ffwll.ch
>>>
>>> -- 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
>

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

Re: [PATCH v2 00/11] drm/tinydrm: Support device unplug

[Oleksandr Andrushchenko]

On 03/26/2018 07:36 PM, Noralf Trønnes wrote:
>
> Den 26.03.2018 15.02, skrev Oleksandr Andrushchenko:
>> Hi, Noralf!
>>
>> On 03/17/2018 04:40 PM, Noralf Trønnes wrote:
>>>
>>> Den 16.03.2018 09.03, skrev Daniel Vetter:
>>>> On Fri, Sep 8, 2017 at 6:33 PM, Daniel Vetter<daniel@ffwll.ch>  wrote:
>>>>> Hi Noralf,
>>>>>
>>>>> On Fri, Sep 08, 2017 at 05:07:19PM +0200, Noralf Trønnes wrote:
>>>>>> This adds device unplug support to drm_fb_helper, drm_fb_cma_helper
>>>>>> (fbdev) and tinydrm.
>>>>>>
>>>>>> There are several changes in this version:
>>>>>>
>>>>>> I've used Daniel's idea of protecting drm_device.unplugged with 
>>>>>> srcu to
>>>>>> provide race free drm_dev_unplug().
>>>>>>
>>>>>> The fbdev helper unplug patch has become very small with Daniel's 
>>>>>> help.
>>>>>> Ref is now taken and dropped in the existing helpers, so I could 
>>>>>> drop
>>>>>> drm_fb_helper_simple_init().
>>>>>>
>>>>>> I has annoyed me that fbdev is restored in drm_driver.last_close 
>>>>>> even if
>>>>>> fbdev isn't used. I've added a patch to fix that. This means I 
>>>>>> can drop
>>>>>> calling drm_atomic_helper_shutdown() in tinydrm_unregister(), 
>>>>>> since I
>>>>>> now can easily disable the pipeline from userspace by just 
>>>>>> closing the
>>>>>> users. Disabled pipeline means balanced regulator_enable/disable.
>>>>>>
>>>>>> The 'Embed drm_device in tinydrm_device' patch has gained
>>>>>> drm_driver.release functions after a discussion with Laurent. My
>>>>>> previous version relied on obscure freeing in tinydrm_release().
>>>>>> This means that I didn't retain the ack's.
>>>>>>
>>>>>> Laurent also caught an ugly devm_kmalloc() in
>>>>>> tinydrm_display_pipe_init() that I've fixed.
>>>>> I'm practically packing for my 2 weeks of conference travel 
>>>>> already, so
>>>>> only very cursory read of the initial patches for core&fb-helpers. 
>>>>> I think
>>>>> this looks really splendid now.
>>>>>
>>>>> But I won't have time for review for the next few week, would be 
>>>>> good if
>>>>> you could drag some others into this discussions. Iirc there's 
>>>>> recently
>>>>> been a few different people interested in udl (even some patches I 
>>>>> think),
>>>>> they might be able to help out with testing&review.
>>>>>
>>>>> Also, would be great if you can submit this to intel-gfx on the next
>>>>> round, so that our CI can pick it up and give it a solid beating. 
>>>>> Touching
>>>>> critical core paths like in patch 1 is always a bit scary.
>>>> While reviewing xen-front's hotunplug handling I just realized this
>>>> never landed. Can you pls resend and nag me to review it properly? I'd
>>>> really like to get the drm_dev_unplug stuff sorted out better.
>>>
>>> My plan was to pick this up after switching tinydrm over to vmalloc 
>>> buffers,
>>> but that work is now waiting for the generic fbdev emulation to land.
>>>
>>> I'm currently wandering around inside drm_fb_helper looking for a 
>>> way out,
>>> I can feel the draft so there has to be an exit somewhere. I hope 
>>> that in
>>> a week or two I'm done with the next version of the RFC using the
>>> drm_fb_helper mode setting code instead of the ioctl's.
>>>
>>> At that point it will be a good thing to flush my "caches" of the
>>> drm_fb_helper code, since after looking at it for a long time, I really
>>> don't see the details anymore. So I'll pick up the unplug series 
>>> then, at
>>> least the core patches should be trivial to review and get merged if 
>>> the CI
>>> agrees.
>> Could you please estimate when unplug series can be on review?
>> I am doing some unplug work in my PV DRM driver and would like
>> to understand if it is feasible to wait for you or post patches as 
>> they are
>> and then plumb in drm_dev_{enter|exit} later after your work is merged
>>
>
> Ok, so I have looked at the patches and some work I have lying around.
> As things stand now I see an opportunity to move some stuff from tinydrm
> into drm_simple_kms_helper as part of adding unplug support to tinydrm.
> This also depends on the generic fbdev emulation I'm working on.
> This all means that it won't be some trivial tweaking to the unplug
> patchset before resending it. It will take some time.
>
> My suggestion is that you just add the core unplug patch as part of your
> driver submission instead of waiting for me.
>
> drm: Use srcu to protect drm_device.unplugged
> https://patchwork.freedesktop.org/patch/175779/
>
This is exactly the patch I need. Daniel, could you please
review this single patch?
> I believe this patch should be good to go as-is. Please CC
> intel-gfx@lists.freedesktop.org if you do so to have the Intel CI 
> verify it.
>
I will
> As for drm_fb_helper unplug support:
>
> drm/fb-helper: Support device unplug
> https://patchwork.freedesktop.org/patch/175785/
>
> I'm not as confident in this one since I'm not sure that those
> drm_dev_is_unplugged() checks are really necessary. The driver has to do
> the check anyways. But this patch isn't necessary for you to do most of
> your driver side unplug protection though. You can do testing without
> fbdev enabled and let me to care of this when I get around to it.
>
> I you pick up the patch(es) and need to change something, you don't have
> to bother with retaining my authorship 
I will retain
> (but please cc me).
Ok
> Just claim it
> for your own and make it work.
> Less work for me when I get there (eventually).
>
> Noralf.
>
Thank you,
Oleksandr
>> Thank you,
>> Oleksandr
>>>
>>> Noralf.
>>>
>>>> Thanks, Daniel
>>>>
>>>>> Thanks, Daniel
>>>>>
>>>>>> Noralf.
>>>>>>
>>>>>> Noralf Trønnes (11):
>>>>>>    drm: Use srcu to protect drm_device.unplugged
>>>>>>    drm/fb-helper: Support device unplug
>>>>>>    drm/fb-helper: Ensure driver module is pinned in fb_open()
>>>>>>    drm/fb-helper: Don't restore if fbdev is not in use
>>>>>>    drm/fb-cma-helper: Make struct drm_fbdev_cma public
>>>>>>    drm/fb-cma-helper: Support device unplug
>>>>>>    drm/tinydrm: Drop driver registered message
>>>>>>    drm/tinydrm: Embed drm_device in tinydrm_device
>>>>>>    drm/tinydrm: Support device unplug in core
>>>>>>    drm/tinydrm/mi0283qt: Let the display pipe handle power
>>>>>>    drm/tinydrm: Support device unplug in drivers
>>>>>>
>>>>>>   drivers/gpu/drm/drm_drv.c                   |  54 +++++++++--
>>>>>>   drivers/gpu/drm/drm_fb_cma_helper.c         |  13 +--
>>>>>>   drivers/gpu/drm/drm_fb_helper.c             | 108 
>>>>>> ++++++++++++++++++++--
>>>>>>   drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 135 
>>>>>> +++++++++++++++++++---------
>>>>>>   drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c |  28 +++---
>>>>>>   drivers/gpu/drm/tinydrm/mi0283qt.c          |  81 
>>>>>> +++++------------
>>>>>>   drivers/gpu/drm/tinydrm/mipi-dbi.c          |  58 +++++++++---
>>>>>>   drivers/gpu/drm/tinydrm/repaper.c           |  62 ++++++++-----
>>>>>>   drivers/gpu/drm/tinydrm/st7586.c            |  54 ++++++-----
>>>>>>   include/drm/drm_device.h                    |   9 +-
>>>>>>   include/drm/drm_drv.h                       |  15 +++-
>>>>>>   include/drm/drm_fb_cma_helper.h             |  11 ++-
>>>>>>   include/drm/drm_fb_helper.h                 |  28 ++++++
>>>>>>   include/drm/tinydrm/mipi-dbi.h              |   1 +
>>>>>>   include/drm/tinydrm/tinydrm.h               |  10 ++-
>>>>>>   15 files changed, 469 insertions(+), 198 deletions(-)
>>>>>>
>>>>>> -- 
>>>>>> 2.7.4
>>>>>>
>>>>> -- 
>>>>> Daniel Vetter
>>>>> Software Engineer, Intel Corporation
>>>>> http://blog.ffwll.ch
>>>>
>>>> -- 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
>>
>

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

Re: [PATCH v2 00/11] drm/tinydrm: Support device unplug

[Daniel Vetter]

On Mon, Mar 26, 2018 at 6:36 PM, Noralf Trønnes <noralf@tronnes.org> wrote:
> I you pick up the patch(es) and need to change something, you don't have
> to bother with retaining my authorship (but please cc me). Just claim it
> for your own and make it work.
> Less work for me when I get there (eventually).

Why can't be quite this flippant with copyright for upstream :-)

Either please put something like "Based on work by ..." into the
commit message (when it's a big rewrite) or just add your own s-o-b
line and a small note what you changed (if it's just a tiny change).

Thanks, 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