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

[Daniel Vetter]

On Mon, Nov 26, 2018 at 04:38:48PM +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 use in order to know when
> to restore.
> 
> v1: Avoid special-casing and move the check to drm_fbdev_client_restore()
>     (Daniel Vetter)
> 
> Signed-off-by: Noralf Trønnes <noralf at tronnes.org>
> ---
> 
> I was tied up in my previous attempt to solve this which predated the 
> generic emulation, so I failed to see that I could solve this within
> the scope of the generic fbdev.

Still kinda annoying that between this and drm_fb_helper_is_bound we'll
have mutliple ways to figure out who owns the fbdev. And the
special-casing is still there, it's just moved around - it still only
works for drivers using the new client helpers.

The other issue with not putting this into is_bound() is that we now have
different rules between hotplug and restore (lastclose is only called when
there's not master and hence is_bound() will return true). So if you
lastclose, it won't be restored, but if you then plug something it, it
will. So I still think we should fix this consistently for all drivers
(and yeah that means rolling out fb_open/release callbacks), and also
consistently between hotplug and lastclose.
-Daniel

>  drivers/gpu/drm/drm_fb_helper.c | 24 +++++++++++++++++++++++-
>  include/drm/drm_fb_helper.h     | 10 ++++++++++
>  2 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 9a69ad7e9f3b..be93e7393704 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2942,16 +2942,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);
>  
> @@ -3143,8 +3159,14 @@ static void drm_fbdev_client_unregister(struct drm_client_dev *client)
>  static int drm_fbdev_client_restore(struct drm_client_dev *client)
>  {
>  	struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
> +	unsigned int fb_open_count;
>  
> -	drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper);
> +	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_restore_fbdev_mode_unlocked(fb_helper);
>  
>  	return 0;
>  }
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index bb9acea61369..e6ca1119d524 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -247,6 +247,16 @@ struct drm_fb_helper {
>  	 * See also: @deferred_setup
>  	 */
>  	int preferred_bpp;
> +
> +	/**
> +	 * @fb_open_count:
> +	 *
> +	 * The generic fbdev emulation uses this to track userspace/fbcon opens
> +	 * to know when to restore.
> +	 *
> +	 * 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