Re: [Intel-gfx] [PATCH] drm/i915/fbdev: Implement fb_dirty for intel custom fb helper

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1: Type: text/plain, Size: 2834 bytes --]

Hi

Am 21.12.22 um 15:51 schrieb Ville Syrjälä:
> On Wed, Dec 21, 2022 at 11:49:59AM +0100, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 29.11.22 um 13:43 schrieb Jouni Högander:
>>> After splitting generic drm_fb_helper into it's own file it's left to
>>> helper implementation to have fb_dirty function. Currently intel
>>> fb doesn't have it. This is causing problems when PSR is enabled.
>>>
>>> Implement simple fb_dirty callback to deliver notifications to psr
>>> about updates in fb console.
>>
>> I'm a bit confused about i915's use of fb_dirty here. How is this
>> supposed to interact with mmap?  i915 doesn't use deferred I/O so fbdev
>> mmap will never call fb_dirty if userspace writes to mmap'ed pages. Is
>> this only required for the kernel's graphics console?
> 
> It's required for everything. mmap is presumably borked for
> the cases where we can't use any hw based damage tracking.

In this case, it would make sense to implement the update with fb_dirty 
(instead of the fb_ops I mentioned).

For mmap you can use fbdev's deferred I/O. There's 
drm_fb_helper_deferrer_io() that tracks mmaped pages and regularly calls 
fb_dirty to let the driver do an update.

Best regards
Thomas

> 
>>
>> Best regards
>> Thomas
>>
>>>
>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/display/intel_fbdev.c | 9 +++++++++
>>>    1 file changed, 9 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
>>> index 5575d7abdc09..7c7fba3fe69e 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
>>> @@ -328,8 +328,17 @@ static int intelfb_create(struct drm_fb_helper *helper,
>>>    	return ret;
>>>    }
>>>    
>>> +static int intelfb_dirty(struct drm_fb_helper *helper, struct drm_clip_rect *clip)
>>> +{
>>> +	if (helper->fb->funcs->dirty)
>>> +		return helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, clip, 1);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>    static const struct drm_fb_helper_funcs intel_fb_helper_funcs = {
>>>    	.fb_probe = intelfb_create,
>>> +	.fb_dirty = intelfb_dirty,
>>>    };
>>>    
>>>    static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
>>
>> -- 
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Ivo Totev
> 
> 
> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]