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

[Noralf Trønnes]

Den 26.11.2018 21.07, skrev 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.


Ok, I haven't got the necessary steam to tackle this broader scope right 
now.
I'll deal with it if a bug report for the tinydrm/repaper driver shows up.
Not sure if anyone is actually using DRM userspace with that driver.

Btw, your tweet gave me a good laugh: "fbdev: everything is horrible"

Noralf.

> -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