Re: [PATCH 04/10] drm/tegra: Set fbdev flags

[Thomas Zimmermann <tzimmermann@suse.de>]

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

Hi

Am 05.07.23 um 11:34 schrieb Javier Martinez Canillas:
> Thomas Zimmermann <tzimmermann@suse.de> writes:
> 
> [...]
>     
>>>> +	info->flags |= FBINFO_VIRTFB;
>>>
>>> I see that all fbdev drivers just do: info->flags = FBINFO_FLAG_DEFAULT | FBINFO_VIRTFB
>>>
>>> Guess you are doing in two assignments to be consistent with drm_fbdev_dma.c ?
>>> I was just curious about the rationale for setting the flags in two steps.
>>
>> The _DEFAULT flag is really just a zero. And the other flags describe
>> different aspects of the framebuffer.  I think it makes sense to set the
>> flags together with the respective state. For example, _VIRTFB is set
>> next to ->screen_buffer, because they belong together.
>>
> 
> Yes, that makes sense.
> 
>> _VIRTFB is currently only used in defio code at
>>
>> https://elixir.bootlin.com/linux/latest/source/drivers/video/fbdev/core/fb_defio.c#L232
>>
>> I think the fbdev I/O helpers should also test this flag after all
>> drivers have been annotated correctly. For example, fb_io_read() would
>> WARN_ONCE if the _VIRTFB flag has been set; and fb_sys_read() would warn
>> if it hasn't been set.  For the read helpers, it also makes sense to
>> WARN_ONCE if the _READS_FAST flag has not been set.
>>
> 
> Agreed. Maybe you could add those warn (or at least info or debug?) even
> if not all drivers have been annotated correctly. That way people can be
> aware that is missing and fix if there are remaining drivers.

Yes, we could do that. I want to go through drivers first and fix the 
low-hanging fruits. Some of the old fbdev drivers use either DMA or I/O 
memory. They would only be fix-worthy if someone complains.

Best regards
Thomas

> 
>> Best regards
>> Thomas
>>
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

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