On Wed, 08 Nov 2023, Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:
> Hi,
>
> 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?
BR,
Jani.
[1] https://gitlab.freedesktop.org/drm/intel/-/issues/10133
>
> 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/
--
Jani Nikula, Intel
