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@xxxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
