Hello Huacai,

Thanks for the patch.

On 5/21/22 15:41, Huacai Chen wrote:

Consider a configuration like this: 1, efifb (or simpledrm) is built-in; 2, a native display driver (such as radeon) is also built-in.

As mentioned in the other thread, this is not a common configuration.

Usually the native display drivers are built as a module and only the generic drivers that use a firmware-provided firmware for scanout are built-in the kernel image.

Because efifb, radeon and sysfb are all in device_initcall() level, the order in practise is like this:

efifb registered at first, but no "efi-framebuffer" device yet. radeon registered later, and /dev/fb0 created. sysfb_init() comes at last, it registers "efi-framebuffer" and then causes an error message "efifb: a framebuffer is already registered". Make sysfb_init() to be subsys_ initcall_sync() can avoid this. But Javier Martinez Canillas has a more general solution.

You are talking about "[PATCH v5 0/7] Fix some races between sysfb device registration and drivers probe", but unfortunately that approach was not as general as needed, see:

https://lore.kernel.org/all/026b1c6d-c258-fa88-ed08-d1b5784c95b0@suse.de/

I need to go back to that series but I'm not sure when will have the time.

However, this patch still makes sense because it can make the screen display as early as possible (We cannot move to subsys_initcall, since sysfb_init() should be executed after PCI enumeration).

Indeed. I agree that makes sense to move the platform device registration early. As you mentioned this only solve some of the issues since we don't know when the _driver_ is going to be registered. But at least we ensure that by the time built-in drivers are registered, the platform device is already there to match. Bringing the display output as early as possible.

Signed-off-by: Huacai Chen chenhuacai@loongson.cn

drivers/firmware/sysfb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/sysfb.c b/drivers/firmware/sysfb.c index 2bfbb05f7d89..aecf91517e54 100644 --- a/drivers/firmware/sysfb.c +++ b/drivers/firmware/sysfb.c @@ -80,4 +80,4 @@ static __init int sysfb_init(void) }

/* must execute after PCI subsystem for EFI quirks */ -device_initcall(sysfb_init); +subsys_initcall_sync(sysfb_init);

I would make the comment here more verbose. Mentioning that PCI enumeration happens in the subsys_initcall() init call level and so is safe to register this at subsys_initcall_sync(), which happens after or something like that.

Probably would be good to also mention the same in the patch description.

Reviewed-by: Javier Martinez Canillas javierm@redhat.com