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)