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

[Thomas Zimmermann]

Hi Javier

Am 05.07.23 um 10:34 schrieb Javier Martinez Canillas:
> Thomas Zimmermann <tzimmermann at suse.de> writes:
> 
>> Set fbdev default flags FBNFO_DEFAULT and mark the framebuffer with
>> FBINFO_VIRTFB. The framebuffer range is in DMA-able memory and should
>> be accessed with the CPU's regular memory ops.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>> Cc: Thierry Reding <thierry.reding at gmail.com>
>> Cc: Mikko Perttunen <mperttunen at nvidia.com>
>> ---
>>   drivers/gpu/drm/tegra/fbdev.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/tegra/fbdev.c b/drivers/gpu/drm/tegra/fbdev.c
>> index 82577b7c88da..8074430c52f1 100644
>> --- a/drivers/gpu/drm/tegra/fbdev.c
>> +++ b/drivers/gpu/drm/tegra/fbdev.c
>> @@ -103,6 +103,8 @@ static int tegra_fbdev_probe(struct drm_fb_helper *helper,
>>   		return PTR_ERR(info);
>>   	}
>>   
>> +	info->flags = FBINFO_DEFAULT;
>> +
>>   	fb = tegra_fb_alloc(drm, &cmd, &bo, 1);
>>   	if (IS_ERR(fb)) {
>>   		err = PTR_ERR(fb);
>> @@ -132,6 +134,7 @@ static int tegra_fbdev_probe(struct drm_fb_helper *helper,
>>   		}
>>   	}
>>   
>> +	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.

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

Best regards
Thomas

> 
> Reviewed-by: Javier Martinez Canillas <javierm at redhat.com>
> 

-- 
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)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20230705/c9423668/attachment-0001.sig>