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