Re: [PATCH 00/12] arch,fbdev: Move screen_info into arch/

[Thomas Zimmermann]

Hi

Am 29.06.23 um 15:31 schrieb Arnd Bergmann:
> On Thu, Jun 29, 2023, at 13:45, Thomas Zimmermann wrote:
> > The variables screen_info and edid_info provide information about
> > the system's screen, and possibly EDID data of the connected display.
> > Both are defined and set by architecture code. But both variables are
> > declared in non-arch header files. Dependencies are at bease loosely
> > tracked. To resolve this, move the global state screen_info and its
> > companion edid_info into arch/. Only declare them on architectures
> > that define them. List dependencies on the variables in the Kconfig
> > files. Also clean up the callers.
> >
> > Patch 1 to 4 resolve a number of unnecessary include statements of
> > <linux/screen_info.h>. The header should only be included in source
> > files that access struct screen_info.
> >
> > Patches 5 to 7 move the declaration of screen_info and edid_info to
> > <asm-generic/screen_info.h>. Architectures that provide either set
> > a Kconfig token to enable them.
> >
> > Patches 8 to 9 make users of screen_info depend on the architecture's
> > feature.
> >
> > Finally, patches 10 to 12 rework fbdev's handling of firmware EDID
> > data to make use of existing helpers and the refactored edid_info.
> >
> > Tested on x86-64. Built for a variety of platforms.
>
> This all looks like a nice cleanup!

I guess that patches 1 to 4 are uncontroversial and could be landed 
quickly. Patches 10 to 12 are probably as well.

> > Future directions: with the patchset in place, it will become possible
> > to provide screen_info and edid_info only if there are users. Some
> > architectures do this by testing for CONFIG_VT, CONFIG_DUMMY_CONSOLE,
> > etc. A more uniform approach would be nice. We should also attempt
> > to minimize access to the global screen_info as much as possible. To
> > do so, some drivers, such as efifb and vesafb, would require an update.
> > The firmware's EDID data could possibly made available outside of fbdev.
> > For example, the simpledrm and ofdrm drivers could provide such data
> > to userspace compositors.
>
> I suspect that most architectures that provide a screen_info only
> have this in order to compile the framebuffer drivers, and provide
> hardcoded data that does not even reflect any real hardware.

That's quite possible. Only x86's bootparam and EFI code sets 
screen_info from external data. The rest is hardcoded. A number of 
architectures protect screen_info with CONFIG_VT, CONFIG_DUMMY_CONSOLE, 
etc. In a later patchset, I wanted to change this such that these users 
of screen_info would enable the feature via select in their Kconfig.

Do you know the reason for this branch in dummycon:

https://elixir.bootlin.com/linux/v6.4/source/drivers/video/console/dummycon.c#L21

What is special about arm that dummycon uses the screeninfo?

> We can probably reduce the number of architectures that do this
> a lot, especially if we get EFI out of the picture.

Can you elaborate?

Best regards
Thomas

>        Arnd

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