2024-01-23 12:11:45

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH] Revert "drivers/firmware: Move sysfb_init() from device_initcall to subsys_initcall_sync"

This reverts commit 60aebc9559492cea6a9625f514a8041717e3a2e4.

Commit 60aebc9559492cea ("drivers/firmware: Move sysfb_init() from
device_initcall to subsys_initcall_sync") messes up initialization order
of the graphics drivers and leads to blank displays on some systems. So
revert the commit.

To make the display drivers fully independent from initialization
order requires to track framebuffer memory by device and independently
from the loaded drivers. The kernel currently lacks the infrastructure
to do so.

Reported-by: Jaak Ristioja <[email protected]>
Closes: https://lore.kernel.org/dri-devel/[email protected]/T/#t
Reported-by: Huacai Chen <[email protected]>
Closes: https://lore.kernel.org/dri-devel/[email protected]/
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10133
Signed-off-by: Thomas Zimmermann <[email protected]>
Cc: Javier Martinez Canillas <[email protected]>
Cc: Thorsten Leemhuis <[email protected]>
Cc: Jani Nikula <[email protected]>
---
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 82fcfd29bc4d2..3c197db42c9d9 100644
--- a/drivers/firmware/sysfb.c
+++ b/drivers/firmware/sysfb.c
@@ -128,4 +128,4 @@ static __init int sysfb_init(void)
}

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



2024-01-23 13:25:25

by Huacai Chen

[permalink] [raw]
Subject: Re: [PATCH] Revert "drivers/firmware: Move sysfb_init() from device_initcall to subsys_initcall_sync"

I'm very sorry to hear that, If Jaak can respond, I think I can find
the root cause and fix that...

Huacai

On Tue, Jan 23, 2024 at 8:09 PM Thomas Zimmermann <tzimmermann@susede> wrote:
>
> This reverts commit 60aebc9559492cea6a9625f514a8041717e3a2e4.
>
> Commit 60aebc9559492cea ("drivers/firmware: Move sysfb_init() from
> device_initcall to subsys_initcall_sync") messes up initialization order
> of the graphics drivers and leads to blank displays on some systems. So
> revert the commit.
>
> To make the display drivers fully independent from initialization
> order requires to track framebuffer memory by device and independently
> from the loaded drivers. The kernel currently lacks the infrastructure
> to do so.
>
> Reported-by: Jaak Ristioja <[email protected]>
> Closes: https://lore.kernel.org/dri-devel/[email protected]/T/#t
> Reported-by: Huacai Chen <[email protected]>
> Closes: https://lore.kernel.org/dri-devel/[email protected]/
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10133
> Signed-off-by: Thomas Zimmermann <[email protected]>
> Cc: Javier Martinez Canillas <[email protected]>
> Cc: Thorsten Leemhuis <[email protected]>
> Cc: Jani Nikula <[email protected]>
> ---
> 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 82fcfd29bc4d2..3c197db42c9d9 100644
> --- a/drivers/firmware/sysfb.c
> +++ b/drivers/firmware/sysfb.c
> @@ -128,4 +128,4 @@ static __init int sysfb_init(void)
> }
>
> /* must execute after PCI subsystem for EFI quirks */
> -subsys_initcall_sync(sysfb_init);
> +device_initcall(sysfb_init);
> --
> 2.43.0
>

2024-01-23 13:35:20

by Thomas Zimmermann

[permalink] [raw]
Subject: Re: [PATCH] Revert "drivers/firmware: Move sysfb_init() from device_initcall to subsys_initcall_sync"

Hi

Am 23.01.24 um 14:12 schrieb Huacai Chen:
> I'm very sorry to hear that, If Jaak can respond, I think I can find
> the root cause and fix that...

No problem, don't worry. We wanted to revert that patch anyway. And the
*real* fix here is to track the framebuffer ownership more closely.

Best regards
Thomas

>
> Huacai
>
> On Tue, Jan 23, 2024 at 8:09 PM Thomas Zimmermann <[email protected]> wrote:
>>
>> This reverts commit 60aebc9559492cea6a9625f514a8041717e3a2e4.
>>
>> Commit 60aebc9559492cea ("drivers/firmware: Move sysfb_init() from
>> device_initcall to subsys_initcall_sync") messes up initialization order
>> of the graphics drivers and leads to blank displays on some systems. So
>> revert the commit.
>>
>> To make the display drivers fully independent from initialization
>> order requires to track framebuffer memory by device and independently
>> from the loaded drivers. The kernel currently lacks the infrastructure
>> to do so.
>>
>> Reported-by: Jaak Ristioja <[email protected]>
>> Closes: https://lore.kernel.org/dri-devel/[email protected]/T/#t
>> Reported-by: Huacai Chen <[email protected]>
>> Closes: https://lore.kernel.org/dri-devel/[email protected]/
>> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10133
>> Signed-off-by: Thomas Zimmermann <[email protected]>
>> Cc: Javier Martinez Canillas <[email protected]>
>> Cc: Thorsten Leemhuis <[email protected]>
>> Cc: Jani Nikula <[email protected]>
>> ---
>> 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 82fcfd29bc4d2..3c197db42c9d9 100644
>> --- a/drivers/firmware/sysfb.c
>> +++ b/drivers/firmware/sysfb.c
>> @@ -128,4 +128,4 @@ static __init int sysfb_init(void)
>> }
>>
>> /* must execute after PCI subsystem for EFI quirks */
>> -subsys_initcall_sync(sysfb_init);
>> +device_initcall(sysfb_init);
>> --
>> 2.43.0
>>

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


Attachments:
OpenPGP_signature.asc (855.00 B)
OpenPGP digital signature

2024-01-23 13:46:41

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH] Revert "drivers/firmware: Move sysfb_init() from device_initcall to subsys_initcall_sync"

Thomas Zimmermann <[email protected]> writes:

> This reverts commit 60aebc9559492cea6a9625f514a8041717e3a2e4.
>
> Commit 60aebc9559492cea ("drivers/firmware: Move sysfb_init() from
> device_initcall to subsys_initcall_sync") messes up initialization order
> of the graphics drivers and leads to blank displays on some systems. So
> revert the commit.
>
> To make the display drivers fully independent from initialization
> order requires to track framebuffer memory by device and independently
> from the loaded drivers. The kernel currently lacks the infrastructure
> to do so.
>
> Reported-by: Jaak Ristioja <[email protected]>
> Closes: https://lore.kernel.org/dri-devel/[email protected]/T/#t
> Reported-by: Huacai Chen <[email protected]>
> Closes: https://lore.kernel.org/dri-devel/[email protected]/
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10133
> Signed-off-by: Thomas Zimmermann <[email protected]>
> Cc: Javier Martinez Canillas <[email protected]>
> Cc: Thorsten Leemhuis <[email protected]>
> Cc: Jani Nikula <[email protected]>
> ---

Reviewed-by: Javier Martinez Canillas <[email protected]>

--
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


2024-01-23 14:39:26

by Jani Nikula

[permalink] [raw]
Subject: Re: [PATCH] Revert "drivers/firmware: Move sysfb_init() from device_initcall to subsys_initcall_sync"

On Tue, 23 Jan 2024, Thomas Zimmermann <[email protected]> wrote:
> This reverts commit 60aebc9559492cea6a9625f514a8041717e3a2e4.
>
> Commit 60aebc9559492cea ("drivers/firmware: Move sysfb_init() from
> device_initcall to subsys_initcall_sync") messes up initialization order
> of the graphics drivers and leads to blank displays on some systems. So
> revert the commit.
>
> To make the display drivers fully independent from initialization
> order requires to track framebuffer memory by device and independently
> from the loaded drivers. The kernel currently lacks the infrastructure
> to do so.
>
> Reported-by: Jaak Ristioja <[email protected]>
> Closes: https://lore.kernel.org/dri-devel/[email protected]/T/#t
> Reported-by: Huacai Chen <[email protected]>
> Closes: https://lore.kernel.org/dri-devel/[email protected]/
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10133

FWIW,

Acked-by: Jani Nikula <[email protected]>

but would be great to get a Tested-by from Jaak or from the gitlab issue
reporter.

Thanks.


> Signed-off-by: Thomas Zimmermann <[email protected]>
> Cc: Javier Martinez Canillas <[email protected]>
> Cc: Thorsten Leemhuis <[email protected]>
> Cc: Jani Nikula <[email protected]>
> ---
> 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 82fcfd29bc4d2..3c197db42c9d9 100644
> --- a/drivers/firmware/sysfb.c
> +++ b/drivers/firmware/sysfb.c
> @@ -128,4 +128,4 @@ static __init int sysfb_init(void)
> }
>
> /* must execute after PCI subsystem for EFI quirks */
> -subsys_initcall_sync(sysfb_init);
> +device_initcall(sysfb_init);

--
Jani Nikula, Intel

2024-01-23 19:17:28

by Jani Nikula

[permalink] [raw]
Subject: Re: [PATCH] Revert "drivers/firmware: Move sysfb_init() from device_initcall to subsys_initcall_sync"

On Tue, 23 Jan 2024, Jani Nikula <[email protected]> wrote:
> On Tue, 23 Jan 2024, Thomas Zimmermann <[email protected]> wrote:
>> This reverts commit 60aebc9559492cea6a9625f514a8041717e3a2e4.
>>
>> Commit 60aebc9559492cea ("drivers/firmware: Move sysfb_init() from
>> device_initcall to subsys_initcall_sync") messes up initialization order
>> of the graphics drivers and leads to blank displays on some systems. So
>> revert the commit.
>>
>> To make the display drivers fully independent from initialization
>> order requires to track framebuffer memory by device and independently
>> from the loaded drivers. The kernel currently lacks the infrastructure
>> to do so.
>>
>> Reported-by: Jaak Ristioja <[email protected]>
>> Closes: https://lore.kernel.org/dri-devel/[email protected]/T/#t
>> Reported-by: Huacai Chen <[email protected]>
>> Closes: https://lore.kernel.org/dri-devel/[email protected]/
>> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10133
>
> FWIW,
>
> Acked-by: Jani Nikula <[email protected]>
>
> but would be great to get a Tested-by from Jaak or from the gitlab issue
> reporter.

The revert works for the gitlab issue reporter.

BR,
Jani.



--
Jani Nikula, Intel