Re: [RFC PATCH] of/platform: Disable sysfb if a simple-framebuffer node is found
On Mon, 13 Nov 2023 at 23:57, Javier Martinez Canillas wrote: > > Andrew Worsley writes: > > Hello Andrew, > > > On Mon, 13 Nov 2023 at 20:18, Thomas Zimmermann wrote: > >> Am 13.11.23 um 09:51 schrieb Javier Martinez Canillas: > >> > Some DT platforms use EFI to boot and in this case the EFI Boot Services > >> > may register a EFI_GRAPHICS_OUTPUT_PROTOCOL handle, that will later be > >> > queried by the Linux EFI stub to fill the global struct screen_info data. > >> > > > [...] > > > > > I applied the patch and just the simpledrm driver is probed (the efifb is > > not): > > >[...] > > Great, thanks for testing. The patch works then as expected. Can I get > your Tested-by then ? Sure absolutely. > > > [...] > We were talking with Thomas that the sysfb design seems to be reaching its > limits and need some rework but currently you either need some driver that > matches the "simple-framebuffer" device that is registered by OF or won't > get an early framebuffer in the system. > > That could be either simpledrm or simplefb. But if a DT has a device node > for "simple-framebuffer", how can the OF core know if there is a driver to > match that device? And same for any other device defined in the DTB. > > It's similar on platforms that use sysfb to register the device (e.g: x86) > since either "simple-framebuffer" is registered (if CONFIG_SYSFB_SIMPLEFB > is enabled) or "efi-framebuffer" (if CONFIG_SYSFB_SIMPLEFB is disabled). > > That means CONFIG_SYSFB_SIMPLEFB=y and CONFIG_DRM_SIMPLEDRM disabled won't > work either, even if CONFIG_FB_EFI=y which is the case you are mentioning. > > What I think that doesn't make sense is to remove conflicting framebuffers > from drivers that can only handle firmware provided framebuffers. As said > in the other thread, drm_aperture_remove_framebuffers() is only meant for > native DRM drivers. Ok - I'm taking it that conflicts between EFI and DT didn't happen in the past but when they do DT wins. I guess there may be more such conflicts in the future so would be resolved in a similar way as more drivers are updated to support DT settings. Perhaps one day all drivers would have DT settings and this could be standardised in some way. > -- > Best regards, > > Javier Martinez Canillas > Core Platforms > Red Hat > Thanks Andrew
Re: [RFC PATCH] of/platform: Disable sysfb if a simple-framebuffer node is found
On Mon, 13 Nov 2023 at 20:18, Thomas Zimmermann wrote:
>
>
>
> Am 13.11.23 um 09:51 schrieb Javier Martinez Canillas:
> > Some DT platforms use EFI to boot and in this case the EFI Boot Services
> > may register a EFI_GRAPHICS_OUTPUT_PROTOCOL handle, that will later be
> > queried by the Linux EFI stub to fill the global struct screen_info data.
> >
> > The data is used by the Generic System Framebuffers (sysfb) framework to
> > add a platform device with platform data about the system framebuffer.
> >
> > But if there is a "simple-framebuffer" node in the DT, the OF core will
> > also do the same and add another device for the system framebuffer.
> >
> > This could lead for example, to two platform devices ("simple-framebuffer"
> > and "efi-framebuffer") to be added and matched with their corresponding
> > drivers. So both efifb and simpledrm will be probed, leading to following:
> >
> > [0.055752] efifb: framebuffer at 0xbd58dc000, using 16000k, total 16000k
> > [0.055755] efifb: mode is 2560x1600x32, linelength=10240, pages=1
> > [0.055758] efifb: scrolling: redraw
> > [0.055759] efifb: Truecolor: size=2:10:10:10, shift=30:20:10:0
> > ...
> > [3.295896] simple-framebuffer bd58dc000.framebuffer: [drm] *ERROR*
> > could not acquire memory range [??? 0x79f30a29ee40-0x2a501a7
> > flags 0x0]: -16
> > [3.298018] simple-framebuffer: probe of bd58dc000.framebuffer
> > failed with error -16
> >
> > To prevent the issue, make the OF core to disable sysfb if there is a node
> > with a "simple-framebuffer" compatible. That way only this device will be
> > registered and sysfb would not attempt to register another one using the
> > screen_info data even if this has been filled.
> >
> > This seems the correct thing to do in this case because:
> >
> > a) On a DT platform, the DTB is the single source of truth since is what
> > describes the hardware topology. Even if EFI Boot Services are used to
> > boot the machine.
> >
> > b) The of_platform_default_populate_init() function is called in the
> > arch_initcall_sync() initcall level while the sysfb_init() function
> > is called later in the subsys_initcall() initcall level.
> >
> > Reported-by: Andrew Worsley
> > Closes:
> > https://lore.kernel.org/all/2023042926.52990-2-amwors...@gmail.com
> > Signed-off-by: Javier Martinez Canillas
>
> Reviewed-by: Thomas Zimmermann
>
> > ---
> >
> > drivers/of/platform.c | 18 --
> > 1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > index f235ab55b91e..0a692fdfef59 100644
> > --- a/drivers/of/platform.c
> > +++ b/drivers/of/platform.c
> > @@ -20,6 +20,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #include "of_private.h"
> >
> > @@ -621,8 +622,21 @@ static int __init
> > of_platform_default_populate_init(void)
> > }
> >
> > node = of_get_compatible_child(of_chosen,
> > "simple-framebuffer");
> > - of_platform_device_create(node, NULL, NULL);
> > - of_node_put(node);
> > + if (node) {
> > + /*
> > + * Since a "simple-framebuffer" device is already
> > added
> > + * here, disable the Generic System Framebuffers
> > (sysfb)
> > + * to prevent it from registering another device for
> > the
> > + * system framebuffer later (e.g: using the
> > screen_info
> > + * data that may had been filled as well).
> > + *
> > + * This can happen for example on DT systems that do
> > EFI
> > + * booting and may provide a GOP handle to the EFI
> > stub.
> > + */
> > + sysfb_disable();
> > + of_platform_device_create(node, NULL, NULL);
> > + of_node_put(node);
> > + }
> >
> > /* Populate everything else. */
> > of_platform_default_populate(NULL, NULL, NULL);
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, B
Re: [PATCH] Fix failure of simpledrm probe when trying to grab FB from the EFI-based Framebuffer
On Sat, 11 Nov 2023 at 20:10, Javier Martinez Canillas wrote: > > > On Sat, 11 Nov 2023 at 15:30, Andrew Worsley wrote: > >> > >>The simpledrm.c does not call aperture_remove_conflicting_devices() in > >> it's probe > >>function as the drivers/video/aperture.c documentation says it should. > >> Consequently > >>it's request for the FB memory fails. > >> > > The current behaviour is correct since aperture_remove_conflicting_devices() > is for native drivers to remove simple framebuffer devices such as simpledrm, > simplefb, efifb, etc. The efifb is the driver that has "grabbed" the FB earlier Here's a debug output with a dump_stack() call in the devm_aperture_acquire() % grep --color -A14 -B4 devm_aperture_acquire ~/dmesg2.txt [0.055752] efifb: framebuffer at 0xbd58dc000, using 16000k, total 16000k [0.055755] efifb: mode is 2560x1600x32, linelength=10240, pages=1 [0.055758] efifb: scrolling: redraw [0.055759] efifb: Truecolor: size=2:10:10:10, shift=30:20:10:0 [0.055771] devm_aperture_acquire: dump stack for debugging [0.055775] CPU: 2 PID: 1 Comm: swapper/0 Tainted: G S 6.5.0-asahi-005 52-gb486fd3ed8fc-dirty #26 [0.055779] Hardware name: Apple MacBook Air (M1, 2020) (DT) [0.055782] Call trace: [0.055784] dump_backtrace+0xfc/0x150 [0.055790] show_stack+0x24/0x40 [0.055793] dump_stack_lvl+0x50/0x68 [0.055797] dump_stack+0x18/0x28 [0.055800] devm_aperture_acquire_for_platform_device+0x4c/0x190 [0.055806] efifb_probe+0x794/0x850 [0.055809] platform_probe+0xb4/0xe8 [0.055815] really_probe+0x178/0x410 [0.055819] __driver_probe_device+0xb0/0x180 [0.055823] driver_probe_device+0x50/0x258 [0.055826] __driver_attach+0x10c/0x270 [0.055830] bus_for_each_dev+0x90/0xf0 [0.055832] driver_attach+0x30/0x48 [0.055835] bus_add_driver+0x100/0x220 [0.055838] driver_register+0x74/0x118 [0.055842] __platform_driver_register+0x30/0x48 [0.055846] efifb_driver_init+0x28/0x40 [0.055850] do_one_initcall+0xe0/0x2f0 [0.055853] do_initcall_level+0xa4/0x128 -- [2.724249] systemd-journald[277]: varlink-22: Changing state pending-disconnect \xe2\ x86\x92 processing-disconnect [2.725413] systemd-journald[277]: varlink-22: Changing state processing-disconnect \x e2\x86\x92 disconnected [2.758337] systemd-journald[277]: Successfully sent stream file descriptor to service manager. [2.765929] systemd-journald[277]: Successfully sent stream file descriptor to service manager. [3.022207] devm_aperture_acquire: dump stack for debugging [3.024280] CPU: 3 PID: 56 Comm: kworker/u16:1 Tainted: G S 6.5.0-asah i-00552-gb486fd3ed8fc-dirty #26 [3.026385] Hardware name: Apple MacBook Air (M1, 2020) (DT) [3.028483] Workqueue: events_unbound deferred_probe_work_func [3.030554] Call trace: [3.032564] dump_backtrace+0xfc/0x150 [3.034580] show_stack+0x24/0x40 [3.036557] dump_stack_lvl+0x50/0x68 [3.038500] dump_stack+0x18/0x28 [3.040408] devm_aperture_acquire_for_platform_device+0x4c/0x190 [3.042322] devm_aperture_acquire_from_firmware+0x40/0x90 [3.044226] simpledrm_probe+0x800/0xe18 [3.046109] platform_probe+0xb4/0xe8 [3.047992] really_probe+0x178/0x410 [3.049840] __driver_probe_device+0xb0/0x180 [3.051684] driver_probe_device+0x50/0x258 [3.053453] __device_attach_driver+0x148/0x1f8 [3.055162] bus_for_each_drv+0x98/0xf8 [3.056814] __device_attach+0x108/0x1d0 [3.058436] device_initial_probe+0x20/0x38 [3.060024] bus_probe_device+0x4c/0xb8 [3.061603] deferred_probe_work_func+0xb8/0x120 [3.063175] process_one_work+0x1f0/0x458 [3.064715] worker_thread+0x2b8/0x4d0 [3.066233] kthread+0xe4/0x180 > > >> ... > >> [3.085302] simple-framebuffer bd58dc000.framebuffer: [drm] *ERROR* > >> could not acquire memory range [??? 0x6e1d8629d580-0x2a501a7 flags > >> 0x0]: -16 > >> [3.086433] simple-framebuffer: probe of bd58dc000.framebuffer failed > >> with error -16 > >> ... > >> > > This is -EBUSY. What is your kernel configuration? Can you share it please. Attached - hope that's acceptable... > > >>In my case no driver provided /dev/dri/card0 device is available on > >> boot up and X > >>fails to start as per this from X start up log. > >> > >> ... > >> [ 5.616] (WW) Falling back to old probe method for modesetting > >> [ 5.616] (EE) open /dev/dri/card0: No such file or directory > >> ... > >> > >>Fault confirmed and fixed on Asahi 6.5.0 kernel with both CONFIG_FB_EFI > >> and > >>CONFIG_DRM_SIMPLEDRM config options set. > >> > >>
Re: [PATCH] Fix failure of simpledrm probe when trying to grab FB from the EFI-based Framebuffer
It's inline - part of the email - not an attachment?
I can see it in the copy that went to me...
Andrew
On Sat, 11 Nov 2023 at 15:30, Andrew Worsley wrote:
>
>The simpledrm.c does not call aperture_remove_conflicting_devices() in
> it's probe
>function as the drivers/video/aperture.c documentation says it should.
> Consequently
>it's request for the FB memory fails.
>
> ...
> [3.085302] simple-framebuffer bd58dc000.framebuffer: [drm] *ERROR* could
> not acquire memory range [??? 0x6e1d8629d580-0x2a501a7 flags 0x0]: -16
> [3.086433] simple-framebuffer: probe of bd58dc000.framebuffer failed with
> error -16
> ...
>
>In my case no driver provided /dev/dri/card0 device is available on boot
> up and X
>fails to start as per this from X start up log.
>
> ...
> [ 5.616] (WW) Falling back to old probe method for modesetting
> [ 5.616] (EE) open /dev/dri/card0: No such file or directory
> ...
>
>Fault confirmed and fixed on Asahi 6.5.0 kernel with both CONFIG_FB_EFI and
> CONFIG_DRM_SIMPLEDRM config options set.
>
> Signed-off-by: Andrew Worsley
> ---
> drivers/gpu/drm/tiny/simpledrm.c | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/tiny/simpledrm.c
> b/drivers/gpu/drm/tiny/simpledrm.c
> index 5fefc895bca2..e55a536b04cf 100644
> --- a/drivers/gpu/drm/tiny/simpledrm.c
> +++ b/drivers/gpu/drm/tiny/simpledrm.c
> @@ -8,6 +8,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -828,6 +829,13 @@ static struct simpledrm_device
> *simpledrm_device_create(struct drm_driver *drv,
> if (mem) {
> void *screen_base;
>
> + ret = aperture_remove_conflicting_devices(mem->start,
> resource_size(mem),
> + DRIVER_NAME);
> + if (ret) {
> + drm_err(dev, "aperture_remove_conflicting_devices:
> failed:%d\n",
> + __func__, ret);
> + return ERR_PTR(ret);
> + }
> ret = devm_aperture_acquire_from_firmware(dev, mem->start,
> resource_size(mem));
> if (ret) {
> drm_err(dev, "could not acquire memory range %pr:
> %d\n", mem, ret);
> @@ -848,6 +856,13 @@ static struct simpledrm_device
> *simpledrm_device_create(struct drm_driver *drv,
> if (!res)
> return ERR_PTR(-EINVAL);
>
> + ret = aperture_remove_conflicting_devices(res->start,
> resource_size(res),
> + DRIVER_NAME);
> + if (ret) {
> + drm_err(dev, "aperture_remove_conflicting_devices:
> failed:%d\n",
> + __func__, ret);
> + return ERR_PTR(ret);
> + }
> ret = devm_aperture_acquire_from_firmware(dev, res->start,
> resource_size(res));
> if (ret) {
> drm_err(dev, "could not acquire memory range %pr:
> %d\n", res, ret);
> --
> 2.42.0
>
[no subject]
This patch fix's the failure of the frame buffer driver on my Asahi kernel which prevented X11 from starting on my Apple M1 laptop. It seems like a straight forward failure to follow the procedure described in drivers/video/aperture.c to remove the ealier driver. This patch is very simple and minimal. Very likely there may be better ways to fix this and very like there may be other drivers which have the same problem but I submit this so at least there is an interim fix for my problem. Thanks Andrew Worsley
[PATCH] Fix failure of simpledrm probe when trying to grab FB from the EFI-based Framebuffer
The simpledrm.c does not call aperture_remove_conflicting_devices() in it's
probe
function as the drivers/video/aperture.c documentation says it should.
Consequently
it's request for the FB memory fails.
...
[3.085302] simple-framebuffer bd58dc000.framebuffer: [drm] *ERROR* could
not acquire memory range [??? 0x6e1d8629d580-0x2a501a7 flags 0x0]: -16
[3.086433] simple-framebuffer: probe of bd58dc000.framebuffer failed with
error -16
...
In my case no driver provided /dev/dri/card0 device is available on boot up
and X
fails to start as per this from X start up log.
...
[ 5.616] (WW) Falling back to old probe method for modesetting
[ 5.616] (EE) open /dev/dri/card0: No such file or directory
...
Fault confirmed and fixed on Asahi 6.5.0 kernel with both CONFIG_FB_EFI and
CONFIG_DRM_SIMPLEDRM config options set.
Signed-off-by: Andrew Worsley
---
drivers/gpu/drm/tiny/simpledrm.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 5fefc895bca2..e55a536b04cf 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include
#include
@@ -828,6 +829,13 @@ static struct simpledrm_device
*simpledrm_device_create(struct drm_driver *drv,
if (mem) {
void *screen_base;
+ ret = aperture_remove_conflicting_devices(mem->start,
resource_size(mem),
+ DRIVER_NAME);
+ if (ret) {
+ drm_err(dev, "aperture_remove_conflicting_devices:
failed:%d\n",
+ __func__, ret);
+ return ERR_PTR(ret);
+ }
ret = devm_aperture_acquire_from_firmware(dev, mem->start,
resource_size(mem));
if (ret) {
drm_err(dev, "could not acquire memory range %pr:
%d\n", mem, ret);
@@ -848,6 +856,13 @@ static struct simpledrm_device
*simpledrm_device_create(struct drm_driver *drv,
if (!res)
return ERR_PTR(-EINVAL);
+ ret = aperture_remove_conflicting_devices(res->start,
resource_size(res),
+ DRIVER_NAME);
+ if (ret) {
+ drm_err(dev, "aperture_remove_conflicting_devices:
failed:%d\n",
+ __func__, ret);
+ return ERR_PTR(ret);
+ }
ret = devm_aperture_acquire_from_firmware(dev, res->start,
resource_size(res));
if (ret) {
drm_err(dev, "could not acquire memory range %pr:
%d\n", res, ret);
--
2.42.0
