[PATCH] drm/fb-helper/generic: Only restore when in use

[Daniel Vetter]

On Wed, Nov 21, 2018 at 05:56:42PM +0100, Noralf Trønnes wrote:
> On drm_driver->last_close the generic fbdev emulation will restore fbdev
> regardless of it being used or not. This is a problem for e-ink displays
> which don't want to be overwritten with zeroes when DRM userspace closes.
> 
> Amend this by adding an open counter to track fbdev use to know when to
> restore. Make use of it in the generic fbdev emulation.
> 
> It wasn't possible to add this funtionality to say all the drivers that
> use the DRM_FB_HELPER_DEFAULT_OPS macro, because some of its users set
> .fb_open and .fb_release.
> 
> If some driver wants this functionality it can export drm_fbdev_fb_open()
> and drm_fbdev_fb_release() and use them.
> 
> Signed-off-by: Noralf Trønnes <noralf at tronnes.org>

Hm, the special-casing is a bit annoying. I think it'd be much nicer to
roll out calls to the helper open/releas implementations to all the
drivers which have an fb_open/release already. And then make this work for
everyone. Quick grep says that there's only radeon/amdgpu, nouveau and udl
doing that.
-Daniel

> ---
>  drivers/gpu/drm/drm_fb_helper.c | 25 +++++++++++++++++++++++--
>  include/drm/drm_fb_helper.h     | 14 ++++++++++++++
>  2 files changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 9a69ad7e9f3b..24ffaee11f5a 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -530,7 +530,7 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
>  int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
>  {
>  	bool do_delayed;
> -	int ret;
> +	int ret = 0;
>  
>  	if (!drm_fbdev_emulation || !fb_helper)
>  		return -ENODEV;
> @@ -539,7 +539,8 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
>  		return 0;
>  
>  	mutex_lock(&fb_helper->lock);
> -	ret = restore_fbdev_mode(fb_helper);
> +	if (fb_helper->fb_open_count)
> +		ret = restore_fbdev_mode(fb_helper);
>  
>  	do_delayed = fb_helper->delayed_hotplug;
>  	if (do_delayed)
> @@ -888,6 +889,8 @@ int drm_fb_helper_init(struct drm_device *dev,
>  		i++;
>  	}
>  
> +	fb_helper->fb_open_count = 1;
> +
>  	dev->fb_helper = fb_helper;
>  
>  	return 0;
> @@ -2942,16 +2945,32 @@ EXPORT_SYMBOL(drm_fb_helper_output_poll_changed);
>  static int drm_fbdev_fb_open(struct fb_info *info, int user)
>  {
>  	struct drm_fb_helper *fb_helper = info->par;
> +	unsigned int fb_open_count;
>  
>  	if (!try_module_get(fb_helper->dev->driver->fops->owner))
>  		return -ENODEV;
>  
> +	mutex_lock(&fb_helper->lock);
> +	fb_open_count = fb_helper->fb_open_count++;
> +	mutex_unlock(&fb_helper->lock);
> +
> +	if (!fb_open_count)
> +		drm_fb_helper_blank(FB_BLANK_UNBLANK, info);
> +
>  	return 0;
>  }
>  
>  static int drm_fbdev_fb_release(struct fb_info *info, int user)
>  {
>  	struct drm_fb_helper *fb_helper = info->par;
> +	unsigned int fb_open_count;
> +
> +	mutex_lock(&fb_helper->lock);
> +	fb_open_count = --fb_helper->fb_open_count;
> +	mutex_unlock(&fb_helper->lock);
> +
> +	if (!fb_open_count)
> +		drm_fb_helper_blank(FB_BLANK_POWERDOWN, info);
>  
>  	module_put(fb_helper->dev->driver->fops->owner);
>  
> @@ -3050,6 +3069,8 @@ int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
>  		      sizes->surface_width, sizes->surface_height,
>  		      sizes->surface_bpp);
>  
> +	fb_helper->fb_open_count = 0;
> +
>  	format = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth);
>  	buffer = drm_client_framebuffer_create(client, sizes->surface_width,
>  					       sizes->surface_height, format);
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index bb9acea61369..846ca8509427 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -247,6 +247,20 @@ struct drm_fb_helper {
>  	 * See also: @deferred_setup
>  	 */
>  	int preferred_bpp;
> +
> +	/**
> +	 * @fb_open_count:
> +	 *
> +	 * Used to track userspace/fbcon opens to know when to restore fbdev in
> +	 * drm_fb_helper_restore_fbdev_mode_unlocked(). The value is set to 1 by
> +	 * default which means it will always restore. Drivers that want to
> +	 * track this can set this to zero in their
> +	 * &drm_fb_helper_funcs.fb_probe function. Additionally the value must
> +	 * be increased in &fb_ops.fb_open and decreased in &fb_ops.fb_release.
> +	 *
> +	 * The value is protected by @lock.
> +	 */
> +	unsigned int fb_open_count;
>  };
>  
>  static inline struct drm_fb_helper *
> -- 
> 2.15.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch