Thread (30 messages) 30 messages, 4 authors, 2023-07-06

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

From: Javier Martinez Canillas <javierm@redhat.com>
Date: 2023-07-05 09:35:01
Also in: dri-devel, linux-arm-kernel, linux-samsung-soc, linux-tegra

Thomas Zimmermann [off-list ref] writes:

[...]
   
quoted
quoted
+	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.
Best regards
Thomas
-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help