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@xxxxxxxxxxx>
---
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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
