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

Noralf Trønnes noralf at tronnes.org
Fri Sep 8 15:07:21 UTC 2017


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



More information about the dri-devel mailing list