On 23.01.24 09:53, Jani Nikula wrote:
> On Wed, 08 Nov 2023, Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:
>>
>> thanks for the patch.
>>
>> Am 08.11.23 um 03:46 schrieb Huacai Chen:
>>> After commit 60aebc9559492cea ("drivers/firmware: Move sysfb_init() from
>>> device_initcall to subsys_initcall_sync") some Lenovo laptops get a blank
>>> screen until the display manager starts.
>>>
>>> This regression occurs with such a Kconfig combination:
>>> CONFIG_SYSFB=y
>>> CONFIG_SYSFB_SIMPLEFB=y
>>> CONFIG_DRM_SIMPLEDRM=y
>>> CONFIG_DRM_I915=y # Or other native drivers such as radeon, amdgpu
>>>
>>> If replace CONFIG_DRM_SIMPLEDRM with CONFIG_FB_SIMPLE (they use the same
>>> device), there is no blank screen. The root cause is the initialization
>>> order, and this order depends on the Makefile.
>>>
>>> FB_SIMPLE is before native DRM drivers (e.g. i915, radeon, amdgpu, and
>>> so on), but DRM_SIMPLEDRM is after them. Thus, if we use FB_SIMPLE, I915
>>> will takeover FB_SIMPLE, then no problem; and if we use DRM_SIMPLEDRM,
>>> DRM_SIMPLEDRM will try to takeover I915, but fails to work.
>>
>> But what exactly is the problem? From the lengthy discussion threat, it
>> looks like you've stumbled across a long-known problem, where the
>> firmware driver probes a device that has already been taken by a native
>> driver. But that should not be possible.
>>
>> As you know, there's a platform device that represents the firmware
>> framebuffer. The firmware drivers, such as simpledrm, bind to it. In
>> i915 and the other native drivers we remove that platform device, so
>> that simpledrm does not run.
>
> The problem is still not resolved. Another bug report at [1].
>
> The commit message here points at 60aebc955949 ("drivers/firmware: Move
> sysfb_init() from device_initcall to subsys_initcall_sync") as
> regressing, and Jaak also bisected it (see Closes:).
>
> I agree the patch here is just papering over the issue, but lacking a
> proper fix, for months, a revert would be in order, no?
>
> [1] https://gitlab.freedesktop.org/drm/intel/-/issues/10133
Interesting.
JFYI for those that don't follow this closely: Huacai Chen proposed a
fix and asked a earlier reporter to test it, but afaics heard nothing back:
https://lore.kernel.org/all/CAAhV-H5eXM7FNzuRCMthAziG_jg75XwQV3grpw=sdyJ-9GXgvA@xxxxxxxxxxxxxx/
That's afaics why this got stuck (and why I didn't request on a escalate
this weeks ago).
Ciao, Thorsten
>> We call the DRM aperture helpers at [1]. It's implemented at [2]. The
>> function contains a call to sysfb_disable(), [3] which should be invoked
>> for the i915 device and remove the platform device.
>>
>> [1]
>> https://elixir.bootlin.com/linux/v6.5/source/drivers/gpu/drm/i915/i915_driver.c#L489
>> [2]
>> https://elixir.bootlin.com/linux/v6.5/source/drivers/video/aperture.c#L347
>> [3]
>> https://elixir.bootlin.com/linux/v6.5/source/drivers/firmware/sysfb.c#L63
>>
>> Can you investigate why this does not work? Is sysfb_disable() not being
>> called? Does it remove the platform device?
>>
>>>
>>> So we can move the "tiny" directory before native DRM drivers to solve
>>> this problem.
>>
>> Relying on linking order is just as unreliable. The usual workaround is
>> to build native drivers as modules. But first, please investigate where
>> the current code fails.
>>
>> Best regards
>> Thomas
>>
>>>
>>> Fixes: 60aebc9559492cea ("drivers/firmware: Move sysfb_init() from device_initcall to subsys_initcall_sync")
>>> Closes: https://lore.kernel.org/dri-devel/ZUnNi3q3yB3zZfTl@P70.localdomain/T/#t
>>> Reported-by: Jaak Ristioja <jaak@xxxxxxxxxxx>
>>> Signed-off-by: Huacai Chen <chenhuacai@xxxxxxxxxxx>
>>> ---
>>> drivers/gpu/drm/Makefile | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>>> index 8e1bde059170..db0f3d3aff43 100644
>>> --- a/drivers/gpu/drm/Makefile
>>> +++ b/drivers/gpu/drm/Makefile
>>> @@ -141,6 +141,7 @@ obj-y += arm/
>>> obj-y += display/
>>> obj-$(CONFIG_DRM_TTM) += ttm/
>>> obj-$(CONFIG_DRM_SCHED) += scheduler/
>>> +obj-y += tiny/
>>> obj-$(CONFIG_DRM_RADEON)+= radeon/
>>> obj-$(CONFIG_DRM_AMDGPU)+= amd/amdgpu/
>>> obj-$(CONFIG_DRM_AMDGPU)+= amd/amdxcp/
>>> @@ -182,7 +183,6 @@ obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
>>> obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
>>> obj-y += hisilicon/
>>> obj-y += mxsfb/
>>> -obj-y += tiny/
>>> obj-$(CONFIG_DRM_PL111) += pl111/
>>> obj-$(CONFIG_DRM_TVE200) += tve200/
>>> obj-$(CONFIG_DRM_XEN) += xen/
>
