[PATCH] drm/Makefile: Move tiny drivers before native drivers

[Huacai Chen]

Hi, Thomas,

On Wed, Nov 8, 2023 at 4:14 PM Thomas Zimmermann <tzimmermann at suse.de> 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.
Yes, it is a long-known problem but only exposed after commit
60aebc9559492cea ("drivers/firmware: Move sysfb_init() from
device_initcall to subsys_initcall_sync"), because the platform device
for simpledrm was not created as early as possible in old kernels.

>
> 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.
>
> 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.
Yes, this looks a little strange, which I haven't investigated before.
Jaak, could you please try to see whether sysfb_disable() is called in
bad kernels?

>
> [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.
Yes, native drivers are usually built as modules, but Jaak tries to
built-in, and then reports this bug. :)

Huacai

>
> 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 at ristioja.ee>
> > Signed-off-by: Huacai Chen <chenhuacai at loongson.cn>
> > ---
> >   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/
>
> --
> 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)