Re: [PATCH] drm/fb-helper: Mark screen buffers in system memory with FB_VIRTFB

[Thomas Zimmermann <tzimmermann@suse.de>]

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

Hi

Am 27.01.22 um 12:42 schrieb Daniel Vetter:
> On Thu, Jan 27, 2022 at 11:26:21AM +0100, Thomas Zimmermann wrote:
>> Mark screen buffers in system memory with FB_VIRTFB. Otherwise, the
>> buffers are mmap'ed as I/O memory (i.e., VM_IO). For shadow buffers,
>> also set the FB_READS_FAST hint.
> 
> Maybe clarify that this only holds for the defio case, and since we have
> our own shadow copy for that anyway it shouldn't matter. I'm also not sure
> how much the memcpy gains us compared to just redrawing ...
> 
> What's the motivation here, or just something you spotted?

Correctness mostly. fbdev's fbdefio tests for (the absence of) this flag 
and sets VM_IO accordingly.

It's actually for userspace. Maybe userspace tests these flags as well 
and can optimize memcpy pattern for different types of caching. But I 
wouldn't expect it TBH.

Best regards
Thomas

> -Daniel
> 
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/gpu/drm/drm_fb_helper.c | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>> index ed43b987d306..f15127a32f7a 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -2346,6 +2346,7 @@ static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
>>   	fbi->fbops = &drm_fbdev_fb_ops;
>>   	fbi->screen_size = sizes->surface_height * fb->pitches[0];
>>   	fbi->fix.smem_len = fbi->screen_size;
>> +	fbi->flags = FBINFO_DEFAULT;
>>   
>>   	drm_fb_helper_fill_info(fbi, fb_helper, sizes);
>>   
>> @@ -2353,19 +2354,21 @@ static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
>>   		fbi->screen_buffer = vzalloc(fbi->screen_size);
>>   		if (!fbi->screen_buffer)
>>   			return -ENOMEM;
>> +		fbi->flags |= FBINFO_VIRTFB | FBINFO_READS_FAST;
>>   
>>   		fbi->fbdefio = &drm_fbdev_defio;
>> -
>>   		fb_deferred_io_init(fbi);
>>   	} else {
>>   		/* buffer is mapped for HW framebuffer */
>>   		ret = drm_client_buffer_vmap(fb_helper->buffer, &map);
>>   		if (ret)
>>   			return ret;
>> -		if (map.is_iomem)
>> +		if (map.is_iomem) {
>>   			fbi->screen_base = map.vaddr_iomem;
>> -		else
>> +		} else {
>>   			fbi->screen_buffer = map.vaddr;
>> +			fbi->flags |= FBINFO_VIRTFB;
>> +		}
>>   
>>   		/*
>>   		 * Shamelessly leak the physical address to user-space. As
>> -- 
>> 2.34.1
>>
> 

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