[PATCH 2/6] drm/fb-helper: Support device unplug

["Noralf Trønnes" <noralf@tronnes.org>]

Support device unplug by introducing a new initialization function:
drm_fb_helper_simple_init() which together with
drm_fb_helper_dev_unplug() and drm_fb_helper_fb_destroy() handles open
fbdev fd's after device unplug. There's also a
drm_fb_helper_simple_fini() for drivers who's device won't be removed.

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,
};

Not possible after unregister:
fb_open() -> fb_ops.fb_open

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) &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().

Secondly we look at the remaining struct fb_ops operations:

Called by fbcon:
- fb_ops.fb_fillrect : drm_fb_helper_{sys,cfb}_fillrect()
- fb_ops.fb_copyarea : drm_fb_helper_{sys,cfb}_copyarea()
- fb_ops.fb_imageblit : drm_fb_helper_{sys,cfb}_imageblit()

Called in fb_set_var() which is called from ioctl, sysfs and fbcon:
- fb_ops.fb_check_var : drm_fb_helper_check_var()
- fb_ops.fb_set_par : drm_fb_helper_set_par()
drm_fb_helper_set_par() is also called from drm_fb_helper_hotplug_event().

Called in fb_set_cmap() which is called from fb_set_var(), ioctl
and fbcon:
- fb_ops.fb_setcolreg
- fb_ops.fb_setcmap : drm_fb_helper_setcmap()

Called in fb_blank() which is called from ioctl and fbcon:
- fb_ops.fb_blank : drm_fb_helper_blank()

Called in fb_pan_display() which is called from fb_set_var(), ioctl,
sysfs and fbcon:
- fb_ops.fb_pan_display : drm_fb_helper_pan_display()

Called by fbcon:
- fb_ops.fb_cursor

Called in fb_read(), fb_write(), and fb_get_buffer_offset() which is
called by fbcon:
- fb_ops.fb_sync

Called by fb_set_var():
- fb_ops.fb_get_caps

Called by fbcon:
- fb_ops.fb_debug_enter : drm_fb_helper_debug_enter()
- fb_ops.fb_debug_leave : drm_fb_helper_debug_leave()

Destroy is safe
- fb_ops.fb_destroy

Finally 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, possibly after drm_fb_helper_fini().

drm_fb_helper_force_kernel_mode():
Triggered by sysrq, possibly after unplug, but before
drm_fb_helper_fini().

drm_fb_helper_hotplug_event():
Called by drm_kms_helper_hotplug_event(). I don't know if this can be
called after drm_dev_unregister(), so add a check to be safe.

Based on this analysis the following functions get a
drm_dev_is_unplugged() 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 | 204 +++++++++++++++++++++++++++++++++++++++-
 include/drm/drm_fb_helper.h     |  35 +++++++
 2 files changed, 237 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 2e33467..fc898db 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -105,6 +105,158 @@ static DEFINE_MUTEX(kernel_fb_helper_lock);
  * mmap page writes.
  */
 
+/**
+ * drm_fb_helper_simple_init - Simple fbdev emulation initialization
+ * @dev: drm device
+ * @fb_helper: driver-allocated fbdev helper structure to initialize
+ * @bpp_sel: bpp value to use for the framebuffer configuration
+ * @max_conn_count: max connector count
+ * @funcs: pointer to structure of functions associate with this helper
+ *
+ * Simple fbdev emulation initialization which calls the following functions:
+ * drm_fb_helper_prepare(), drm_fb_helper_init(),
+ * drm_fb_helper_single_add_all_connectors() and
+ * drm_fb_helper_initial_config().
+ *
+ * This function takes a ref on &drm_device and must be used together with
+ * drm_fb_helper_simple_fini() or drm_fb_helper_dev_unplug().
+ *
+ * fbdev deferred I/O users must use drm_fb_helper_defio_init().
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+int drm_fb_helper_simple_init(struct drm_device *dev,
+			      struct drm_fb_helper *fb_helper, int bpp_sel,
+			      int max_conn_count,
+			      const struct drm_fb_helper_funcs *funcs)
+{
+	int ret;
+
+	drm_fb_helper_prepare(dev, fb_helper, funcs);
+
+	ret = drm_fb_helper_init(dev, fb_helper, max_conn_count);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dev->dev, "Failed to initialize fb helper.\n");
+		return ret;
+	}
+
+	drm_dev_ref(dev);
+
+	ret = drm_fb_helper_single_add_all_connectors(fb_helper);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dev->dev, "Failed to add connectors.\n");
+		goto err_drm_fb_helper_fini;
+
+	}
+
+	ret = drm_fb_helper_initial_config(fb_helper, bpp_sel);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dev->dev, "Failed to set initial hw config.\n");
+		goto err_drm_fb_helper_fini;
+	}
+
+	return 0;
+
+err_drm_fb_helper_fini:
+	drm_fb_helper_fini(fb_helper);
+	drm_dev_unref(dev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(drm_fb_helper_simple_init);
+
+static void drm_fb_helper_simple_fini_cleanup(struct drm_fb_helper *fb_helper)
+{
+	struct fb_info *info = fb_helper->fbdev;
+	struct fb_ops *fbops = NULL;
+
+	if (info && info->fbdefio) {
+		fb_deferred_io_cleanup(info);
+		kfree(info->fbdefio);
+		info->fbdefio = NULL;
+		fbops = info->fbops;
+	}
+
+	drm_fb_helper_fini(fb_helper);
+	kfree(fbops);
+	if (fb_helper->fb)
+		drm_framebuffer_remove(fb_helper->fb);
+	drm_dev_unref(fb_helper->dev);
+}
+
+/**
+ * drm_fb_helper_simple_fini - Simple fbdev cleanup
+ * @fb_helper: fbdev emulation structure, can be NULL
+ *
+ * Simple fbdev emulation cleanup. This function unregisters fbdev, cleans up
+ * deferred I/O if necessary, finalises @fb_helper and removes the framebuffer.
+ * The driver if responsible for freeing the @fb_helper structure.
+ *
+ * Don't use this function if you use drm_fb_helper_dev_unplug().
+ */
+void drm_fb_helper_simple_fini(struct drm_fb_helper *fb_helper)
+{
+	struct fb_info *info;
+
+	if (!fb_helper)
+		return;
+
+	info = fb_helper->fbdev;
+
+	/* Has drm_fb_helper_dev_unplug() been used? */
+	if (info && info->dev)
+		drm_fb_helper_unregister_fbi(fb_helper);
+
+	if (!(info && info->fbops && info->fbops->fb_destroy))
+		drm_fb_helper_simple_fini_cleanup(fb_helper);
+}
+EXPORT_SYMBOL_GPL(drm_fb_helper_simple_fini);
+
+/**
+ * drm_fb_helper_dev_unplug - unplug an fbdev device
+ * @fb_helper: driver-allocated fbdev helper, can be NULL
+ *
+ * This unplugs the fbdev emulation for a hotpluggable DRM device, which makes
+ * fbdev inaccessible to userspace operations. This essentially unregisters
+ * fbdev and can be called while there are still open users of @fb_helper.
+ * Entry-points from fbdev into drm core/helpers are protected by the fbdev
+ * &fb_info ref count and drm_dev_is_unplugged(). This means that the driver
+ * also has to call drm_dev_unplug() to complete the unplugging.
+ *
+ * Drivers must use drm_fb_helper_fb_destroy() as their &fb_ops.fb_destroy
+ * callback and call drm_mode_config_cleanup() and free @fb_helper in their
+ * &drm_driver->release callback.
+ *
+ * @fb_helper is finalized by this function unless there are open fbdev fd's
+ * in case this is postponed to the closing of the last fd. Finalizing includes
+ * dropping the ref taken on &drm_device in drm_fb_helper_simple_init().
+ */
+void drm_fb_helper_dev_unplug(struct drm_fb_helper *fb_helper)
+{
+	drm_fb_helper_unregister_fbi(fb_helper);
+}
+EXPORT_SYMBOL(drm_fb_helper_dev_unplug);
+
+/**
+ * drm_fb_helper_fb_destroy - implementation for &fb_ops.fb_destroy
+ * @info: fbdev registered by the helper
+ *
+ * This function does the same as drm_fb_helper_simple_fini() except
+ * unregistering fbdev which is already done.
+ *
+ * &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 *helper = info->par;
+
+	DRM_DEBUG("\n");
+	drm_fb_helper_simple_fini_cleanup(helper);
+}
+EXPORT_SYMBOL(drm_fb_helper_fb_destroy);
+
 #define drm_fb_helper_for_each_connector(fbh, i__) \
 	for (({ lockdep_assert_held(&(fbh)->lock); }), \
 	     i__ = 0; i__ < (fbh)->connector_count; i__++)
@@ -498,7 +650,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 +715,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 +890,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;
@@ -949,6 +1107,48 @@ void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper)
 }
 EXPORT_SYMBOL(drm_fb_helper_unlink_fbi);
 
+/**
+ * drm_fb_helper_defio_init - fbdev deferred I/O initialization
+ * @fb_helper: driver-allocated fbdev helper
+ *
+ * This function allocates &fb_deferred_io, sets callback to
+ * drm_fb_helper_deferred_io(), delay to 50ms and calls fb_deferred_io_init().
+ *
+ * NOTE: A copy of &fb_ops is made and assigned to &info->fbops. This is done
+ * because fb_deferred_io_cleanup() clears &fbops->fb_mmap and would thereby
+ * affect other instances of that &fb_ops. This copy is freed by the helper
+ * during cleanup.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper)
+{
+	struct fb_info *info = fb_helper->fbdev;
+	struct fb_deferred_io *fbdefio;
+	struct fb_ops *fbops;
+
+	fbdefio = kzalloc(sizeof(*fbdefio), GFP_KERNEL);
+	fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
+	if (!fbdefio || !fbops) {
+		kfree(fbdefio);
+		kfree(fbops);
+		return -ENOMEM;
+	}
+
+	info->fbdefio = fbdefio;
+	fbdefio->delay = msecs_to_jiffies(50);
+	fbdefio->deferred_io = drm_fb_helper_deferred_io;
+
+	*fbops = *info->fbops;
+	info->fbops = fbops;
+
+	fb_deferred_io_init(info);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_fb_helper_defio_init);
+
 static void drm_fb_helper_dirty(struct fb_info *info, u32 x, u32 y,
 				u32 width, u32 height)
 {
@@ -2591,7 +2791,7 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
 {
 	int err = 0;
 
-	if (!drm_fbdev_emulation)
+	if (!drm_fbdev_emulation || drm_dev_is_unplugged(fb_helper->dev))
 		return 0;
 
 	mutex_lock(&fb_helper->lock);
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 33fe959..1c5a648 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -242,6 +242,14 @@ struct drm_fb_helper {
 	.fb_ioctl	= drm_fb_helper_ioctl
 
 #ifdef CONFIG_DRM_FBDEV_EMULATION
+int drm_fb_helper_simple_init(struct drm_device *dev,
+			      struct drm_fb_helper *fb_helper, int bpp_sel,
+			      int max_conn_count,
+			      const struct drm_fb_helper_funcs *funcs);
+void drm_fb_helper_simple_fini(struct drm_fb_helper *fb_helper);
+void drm_fb_helper_dev_unplug(struct drm_fb_helper *fb_helper);
+void drm_fb_helper_fb_destroy(struct fb_info *info);
+
 void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
 			   const struct drm_fb_helper_funcs *funcs);
 int drm_fb_helper_init(struct drm_device *dev,
@@ -265,6 +273,7 @@ void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch,
 
 void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper);
 
+int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper);
 void drm_fb_helper_deferred_io(struct fb_info *info,
 			       struct list_head *pagelist);
 
@@ -311,6 +320,27 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_
 int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
 				       struct drm_connector *connector);
 #else
+static inline int
+drm_fb_helper_simple_init(struct drm_device *dev,
+			  struct drm_fb_helper *fb_helper, int bpp_sel,
+			  int max_conn_count,
+			  const struct drm_fb_helper_funcs *funcs)
+{
+	return 0;
+}
+
+static inline void drm_fb_helper_simple_fini(struct drm_fb_helper *fb_helper)
+{
+}
+
+static inline void drm_fb_helper_dev_unplug(struct drm_fb_helper *fb_helper)
+{
+}
+
+static inline void drm_fb_helper_fb_destroy(struct fb_info *info)
+{
+}
+
 static inline void drm_fb_helper_prepare(struct drm_device *dev,
 					struct drm_fb_helper *helper,
 					const struct drm_fb_helper_funcs *funcs)
@@ -393,6 +423,11 @@ static inline void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper)
 {
 }
 
+static inline int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper)
+{
+	return 0;
+}
+
 static inline void drm_fb_helper_deferred_io(struct fb_info *info,
 					     struct list_head *pagelist)
 {
-- 
2.7.4

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