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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

Am 05.07.23 um 11:34 schrieb Javier Martinez Canillas:
Thomas Zimmermann <tzimmermann@xxxxxxx> 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: OpenPGP_signature
Description: OpenPGP digital signature


[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux