diff mbox series

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

Message ID 20181121165642.33563-1-noralf@tronnes.org (mailing list archive)
State New, archived
Headers show
Series drm/fb-helper/generic: Only restore when in use | expand

Commit Message

Noralf Trønnes Nov. 21, 2018, 4:56 p.m. UTC
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@tronnes.org>
---
 drivers/gpu/drm/drm_fb_helper.c | 25 +++++++++++++++++++++++--
 include/drm/drm_fb_helper.h     | 14 ++++++++++++++
 2 files changed, 37 insertions(+), 2 deletions(-)

Comments

Daniel Vetter Nov. 22, 2018, 9:05 a.m. UTC | #1
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@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

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 *