Re: [PATCH 3/8] firmware: sysfb: Make CONFIG_SYSFB a user-selectable option

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 02.04.26 um 18:47 schrieb Arnd Bergmann:
> On Thu, Apr 2, 2026, at 17:27, Thomas Zimmermann wrote:
>> Am 02.04.26 um 16:59 schrieb Arnd Bergmann:
>>> On Thu, Apr 2, 2026, at 16:10, Thomas Zimmermann wrote:
>>>> Am 02.04.26 um 15:08 schrieb Arnd Bergmann:
>>>>> On Thu, Apr 2, 2026, at 11:09, Thomas Zimmermann wrote:
>>>>> I don't really like this part of the series and would prefer
>>>>> to keep CONFIG_SYSFB hidden as much as possible as an x86
>>>>> (and EFI) specific implementation detail, with the hope
>>>>> of eventually seperating out the x86 bits from the EFI ones.
>>>> You mean, you want to use the EFI-provided framebuffers without the
>>>> intermediate step of going through sysfb_primary_display?
>>>>
>>>> In that case, CONFIG_SYSFB would become an x86-internal thing, right?
>>> The part that is still needed from sysfb is the arbitration
>>> between DRM_EFI and the PCI device driver for the same hardware,
>>> so I think some part of sysfb is clearly needed, in particular
>>> the sysfb_disable() function that removes the EFI framebuffer
>>> when there is a conflicting simpledrm or hardware specific
>>> driver.
>> We do most of that in the aperture-helper module. (see
>> <linux/aperture.h>). Calling sysfb_disable() from there is a workaround
>> for some corner cases. We can have an EFI-specific function that does
>> the same.
> That sounds good, yes. The same change would need to go into
> of_platform_default_populate_init() then.

The call to sysfb_disable() is a workaround for certain cases that 
aperture helpers don't handle well. In the longer term, I'd want 
aperture helpers to be more clever about aperture ownership. But as an 
intermediate step, adding other _disable() function would be an option. 
But there's no hurry.

>
>> BTW, simpledrm-on-EFI/VESA is considered obsolete and should preferably
>> be removed from that driver. Simpledrm should become a driver for
>> Devicetree nodes of type simple-framebuffer (as it originally has been
>> intended).
> Sure, I was only thinking of the case where there are both
> sysfb (from Arm/riscv UEFI) and simpledrm (from devicetree)
> objects referring to the same one, not the simpledrm
> device created by sysfb_simplefb.
>
>>> The parts that I want to keep out of that is anything
>>> related to the x86 boot protocol, non-EFI framebuffers,
>>> text console, and kexec handoff, which we don't need on
>>> non-x86 UEFI systems.
>>>
>>> I don't mind the idea of having a sysfb_primary_display
>>> in the EFI code if that helps keep EFI sane on x86,
>>> but it would be good to make that local to
>>> drivers/firmware/efi and (eventually) detached from
>>> include/uapi/linux/screen_info.h.
>> Efidrm retrieves the framebuffer settings from the contained struct
>> screen_info. Disconnecting from screen_info would require separate
>> graphics drivers for x86 and non-x86. If we split off EFI from sysfb,
>> we'd likely need a sysfbdrm driver of some sort. Just saying.
> Yes, I saw that as well and don't have an immediate idea for how
> to best do it. I saw that you already abstracted the access to
> the screen_info members in drm_sysfb_screen_info.c, which I think
> is a step in that direction.
>
> I also noticed that efidrm is mostly a subset of vesadrm, so
> in theory they could be merged back into an x86 drm driver
> along with the drm_sysfb_screen_info helpers, and have a non-x86
> driver that constructs a drm_sysfb_device directly from the
> EFI structures.

I would not want to have a unifed driver for all-things-screen_info. The 
code that can easily be shared is already in the sysfb helpers. But I 
don't mind adding a separate driver for EFI's Graphics Output Protocol. 
Then there would be current efidrm for EFI-from-screen_info and 
efigopdrm for EFI via the GOP interfaces. If EFI ever specifies another 
graphics interface, we could add another driver. The maintenance 
overhead is small on the DRM side.

What is the future of x86's boot_param and the related screen_info on 
x86-64?  Is it obsolete?  Will boot loaders run the EFI stub instead?

>
>> I think we'd also have to duplicate the framebuffer-relocation code that
>> currently works on anything using struct screen_info (see patch 5).
> You mean the code from include/linux/screen_info.h? I think
> it would make sense to have an x86 specific version of that
> to operate on the x86 screen_info, and a simpler version
> that just updates the resource for the efirdrm driver, but
> that could also be done one level higher or lower.

Makes me wonder if the relocation code could be integrated into the 
aperture helpers. It would have to track the relocation of all PCI 
graphics devices and probing DRM drivers would query the relocation 
offset for their given framebuffer.

>
>>>>> In general, I am always in favor of properly using Kconfig
>>>>> dependencies over 'select' statements, for the same reasons
>>>>> you describe, but I don't want the the x86 logic for
>>>>> the legacy VESA and VGA console handling to leak into more
>>>>> architectures than necessary.
>>>>>
>>>>> Do you think we could instead move the sysfb_init()
>>>>> function into the same two places that contain the
>>>>> sysfb_primary_display definition (arch/x86/kernel/setup.c,
>>>>> drivers/firmware/efi/efi-init.c) and simplify the efi version
>>>>> to take out the x86 bits? That would reduce the rest
>>>>> of sysfb-primary.c to the logic to unregister the device,
>>>>> and that could then be selected by both x86 and EFI.
>>>> No, I'm more than happy that sysfb finally consolidates all the
>>>> init-framebuffer setup and detection that floated around in the kernel.
>>>> I would not want it to be duplicated again.
>>>>
>>>> For now, we could certainly keep CONFIG_SYSFB hidden and autoselected.
>>>> Although I think this will require soem sort of solution at a later point.
>>> Can you clarify which problem you are trying to solve
>>> with that?
>> One thing is that some users simply what control over their kernel build.
>>
>> I also think that there might be systems that want to use
>> sysfb_primary_display (plus the relocation feature), but not create the
>> framebuffer device. Say for efi-earlycon. It needs user-control over the
>> SYSFB option to do that.
> I'm still not following, sorry. efi-earlycon doesn't require
> CONFIG_SYSFB today, and I don't see why that would need to change,
> or why it couldn't just 'select SYSFB' if it it does change.
>
>> As a side-effect, user-configurable SYSFB gives us a nice place to put
>> SYSFB_SIMPLEFB and FIRMWARE_EDID; two options that currently float
>> around in the config somewhat arbitrarily.
> You said that SYSFB_SIMPLEFB should get phased out in the future,
> right?

Yes. Better sooner than later.

>
> I'm also missing your plan for CONFIG_FIRMWARE_EDID. I only
> see three legacy drivers using the old fb_firmware_edid()
> interface, so I assume this is not what you are interested in.
>
> For the global copy that is filled by x86 and efi, and
> consumed by vesadrm and efidrm, does that even need to
> be a configuration option rather than get always enabled?

There is code in x86's old 16-bit boot/init code that reads the EDID via 
VESA. The help text on CONFIG_FIRMWARE_EDID sounds like it needs to be 
configurable because some systems can't do the VESA calls. Hence, the 
logical step seems to be to make this consistent for all systems by 
adopting the option for all EDID retrieval.

If we can remove CONFIG_FIRMWARE_EDID and make EDID support 
unconditional, I'm all for it.

Best regards
Thomas

>
>         Arnd

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)