On Mon, Nov 13, 2023 at 2:53 AM Javier Martinez Canillas
<[email protected]> wrote:
>
> 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 [??? 0xffff79f30a29ee40-0x2a5000001a7
> 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.
This is the opposite of what we do for memory and memory reservations.
EFI is the source of truth for those.
This could also lead to an interesting scenario. As simple-framebuffer
can define its memory in a /reserved-memory node, but that is ignored
in EFI boot. Probably would work, but only because EFI probably
generates its memory map table from the /reserved-memory nodes.
Rob
Rob Herring <[email protected]> writes:
Hello Rob,
> On Mon, Nov 13, 2023 at 2:53 AM Javier Martinez Canillas
> <[email protected]> wrote:
>>
>> 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 [??? 0xffff79f30a29ee40-0x2a5000001a7
>> 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.
>
> This is the opposite of what we do for memory and memory reservations.
> EFI is the source of truth for those.
>
> This could also lead to an interesting scenario. As simple-framebuffer
> can define its memory in a /reserved-memory node, but that is ignored
> in EFI boot. Probably would work, but only because EFI probably
> generates its memory map table from the /reserved-memory nodes.
>
I see. So what would be the solution then? Ignoring creating a platform
device for "simple-framebuffer" if booted using EFI and have an EFI-GOP?
> Rob
>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
On Thu, Nov 16, 2023 at 3:36 AM Javier Martinez Canillas
<[email protected]> wrote:
>
> Rob Herring <[email protected]> writes:
>
> Hello Rob,
>
> > On Mon, Nov 13, 2023 at 2:53 AM Javier Martinez Canillas
> > <[email protected]> wrote:
> >>
> >> 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 [??? 0xffff79f30a29ee40-0x2a5000001a7
> >> 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.
> >
> > This is the opposite of what we do for memory and memory reservations.
> > EFI is the source of truth for those.
> >
> > This could also lead to an interesting scenario. As simple-framebuffer
> > can define its memory in a /reserved-memory node, but that is ignored
> > in EFI boot. Probably would work, but only because EFI probably
> > generates its memory map table from the /reserved-memory nodes.
> >
>
> I see. So what would be the solution then? Ignoring creating a platform
> device for "simple-framebuffer" if booted using EFI and have an EFI-GOP?
Shrug. I don't really know anything more about EFI FB, but I would
guess it can't support handling resources like clocks, power domains,
regulators, etc. that simple-fb can. So if a platform needs those, do
we say they should not setup EFI-GOP? Or is there a use case for
having both? Clients that don't muck with resources can use EFI-GOP
and those that do use simple-fb. For example, does/can grub use
EFI-GOP, but not simple-fb?
Rob
On Fri, 17 Nov 2023 at 00:09, Rob Herring <[email protected]> wrote:
>
> On Thu, Nov 16, 2023 at 3:36 AM Javier Martinez Canillas
> <[email protected]> wrote:
> >
> > Rob Herring <[email protected]> writes:
> >
> > Hello Rob,
> >
> > > On Mon, Nov 13, 2023 at 2:53 AM Javier Martinez Canillas
> > > <[email protected]> wrote:
> > >>
> > >> 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 [??? 0xffff79f30a29ee40-0x2a5000001a7
> > >> 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.
> > >
> > > This is the opposite of what we do for memory and memory reservations.
> > > EFI is the source of truth for those.
> > >
> > > This could also lead to an interesting scenario. As simple-framebuffer
> > > can define its memory in a /reserved-memory node, but that is ignored
> > > in EFI boot. Probably would work, but only because EFI probably
> > > generates its memory map table from the /reserved-memory nodes.
> > >
> >
> > I see. So what would be the solution then? Ignoring creating a platform
> > device for "simple-framebuffer" if booted using EFI and have an EFI-GOP?
>
> Shrug. I don't really know anything more about EFI FB, but I would
> guess it can't support handling resources like clocks, power domains,
> regulators, etc. that simple-fb can. So if a platform needs those, do
> we say they should not setup EFI-GOP? Or is there a use case for
> having both? Clients that don't muck with resources can use EFI-GOP
> and those that do use simple-fb. For example, does/can grub use
> EFI-GOP, but not simple-fb?
>
The EFI GOP is just a dumb framebuffer, and it is not even generally
possible to cross reference the GOP with a particular device in the
device hierarchy unless you e.g., compare the BARs of each device with
the region described by the GOP protocol.
GRUB for EFI will use the GOP and nothing else, but only at boot time
(the GOP protocol is more than a magic linear memory region, it also
implements a Blt() abstraction that permits the use of framebuffers
that are not mapped linearly into the address space at all, and GRUB
makes use of this)
The EFI stub will only expose GOPs to the kernel if they are in fact
linear framebuffers, but has zero insight into whether the hardware
needs clocks and regulators, and whether or not the framebuffer needs
IOMMU pass through (which might be the case if the scanout is using
DMA into system memory)
So calling EFI GOP 'source of truth' is rather generous, and I think
it makes sense to prioritize more accurate descriptions of the
underlying framebuffer over EFI GOP.
However, making 'simple-framebuffer' magic in this regard doesn't seem
like a great approach to me. Is there a better way we could get the
resource conflict to be decided in a way where the EFI GOP gets
superseded if its resources are claimed by another device?
Rob Herring <[email protected]> writes:
> On Thu, Nov 16, 2023 at 3:36 AM Javier Martinez Canillas
> <[email protected]> wrote:
[...]
>> >
>> > This is the opposite of what we do for memory and memory reservations.
>> > EFI is the source of truth for those.
>> >
>> > This could also lead to an interesting scenario. As simple-framebuffer
>> > can define its memory in a /reserved-memory node, but that is ignored
>> > in EFI boot. Probably would work, but only because EFI probably
>> > generates its memory map table from the /reserved-memory nodes.
>> >
>>
>> I see. So what would be the solution then? Ignoring creating a platform
>> device for "simple-framebuffer" if booted using EFI and have an EFI-GOP?
>
> Shrug. I don't really know anything more about EFI FB, but I would
> guess it can't support handling resources like clocks, power domains,
> regulators, etc. that simple-fb can. So if a platform needs those, do
That's correct, and the reason why I thought that the DTB would be the
single source of truth for the firmware provided framebuffer.
For example, in some arm platforms that u-boot does provide an EFI-GOP,
you need to boot with clk_ignore_unused or the system framebuffer just
goes away once the unused clocks are gated. Same for PD, regulators, etc.
> we say they should not setup EFI-GOP? Or is there a use case for
> having both? Clients that don't muck with resources can use EFI-GOP
> and those that do use simple-fb. For example, does/can grub use
> EFI-GOP, but not simple-fb?
>
I don't think grub can use the simple-fb, it can use the EFI-GOP if is
available though. And things work because of course grub won't try to
disable unused resources like Linux does.
> Rob
>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Ard Biesheuvel <[email protected]> writes:
Hello Ard,
> On Fri, 17 Nov 2023 at 00:09, Rob Herring <[email protected]> wrote:
[...]
>> > >
>> > > This could also lead to an interesting scenario. As simple-framebuffer
>> > > can define its memory in a /reserved-memory node, but that is ignored
>> > > in EFI boot. Probably would work, but only because EFI probably
>> > > generates its memory map table from the /reserved-memory nodes.
>> > >
>> >
>> > I see. So what would be the solution then? Ignoring creating a platform
>> > device for "simple-framebuffer" if booted using EFI and have an EFI-GOP?
>>
>> Shrug. I don't really know anything more about EFI FB, but I would
>> guess it can't support handling resources like clocks, power domains,
>> regulators, etc. that simple-fb can. So if a platform needs those, do
>> we say they should not setup EFI-GOP? Or is there a use case for
>> having both? Clients that don't muck with resources can use EFI-GOP
>> and those that do use simple-fb. For example, does/can grub use
>> EFI-GOP, but not simple-fb?
>>
>
> The EFI GOP is just a dumb framebuffer, and it is not even generally
> possible to cross reference the GOP with a particular device in the
> device hierarchy unless you e.g., compare the BARs of each device with
> the region described by the GOP protocol.
>
> GRUB for EFI will use the GOP and nothing else, but only at boot time
> (the GOP protocol is more than a magic linear memory region, it also
> implements a Blt() abstraction that permits the use of framebuffers
> that are not mapped linearly into the address space at all, and GRUB
> makes use of this)
>
> The EFI stub will only expose GOPs to the kernel if they are in fact
> linear framebuffers, but has zero insight into whether the hardware
> needs clocks and regulators, and whether or not the framebuffer needs
> IOMMU pass through (which might be the case if the scanout is using
> DMA into system memory)
>
> So calling EFI GOP 'source of truth' is rather generous, and I think
> it makes sense to prioritize more accurate descriptions of the
> underlying framebuffer over EFI GOP.
>
That was my opinion as well and the reason why I called the DTB the
single source of truth.
> However, making 'simple-framebuffer' magic in this regard doesn't seem
> like a great approach to me. Is there a better way we could get the
> resource conflict to be decided in a way where the EFI GOP gets
> superseded if its resources are claimed by another device?
>
There is an aperture [0] framework that is used by the fbdev and DRM
subsystems to allow native drivers to remove any conflicting devices
that share the same framebuffer aperture.
But it only makes sense for native drivers to use that I think, but
in this case is about two drivers that attempt to use the same frame
buffer provided by the firmware but getting it from different places.
I don't have a better idea than this patch but maybe Thomas or Sima do?
[0]: https://elixir.bootlin.com/linux/latest/source/drivers/video/aperture.c
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Hi
Am 18.11.23 um 12:10 schrieb Javier Martinez Canillas:
> Ard Biesheuvel <[email protected]> writes:
>
> Hello Ard,
>
>> On Fri, 17 Nov 2023 at 00:09, Rob Herring <[email protected]> wrote:
>
> [...]
>
>>>>>
>>>>> This could also lead to an interesting scenario. As simple-framebuffer
>>>>> can define its memory in a /reserved-memory node, but that is ignored
>>>>> in EFI boot. Probably would work, but only because EFI probably
>>>>> generates its memory map table from the /reserved-memory nodes.
>>>>>
>>>>
>>>> I see. So what would be the solution then? Ignoring creating a platform
>>>> device for "simple-framebuffer" if booted using EFI and have an EFI-GOP?
>>>
>>> Shrug. I don't really know anything more about EFI FB, but I would
>>> guess it can't support handling resources like clocks, power domains,
>>> regulators, etc. that simple-fb can. So if a platform needs those, do
>>> we say they should not setup EFI-GOP? Or is there a use case for
>>> having both? Clients that don't muck with resources can use EFI-GOP
>>> and those that do use simple-fb. For example, does/can grub use
>>> EFI-GOP, but not simple-fb?
>>>
>>
>> The EFI GOP is just a dumb framebuffer, and it is not even generally
>> possible to cross reference the GOP with a particular device in the
>> device hierarchy unless you e.g., compare the BARs of each device with
>> the region described by the GOP protocol.
>>
>> GRUB for EFI will use the GOP and nothing else, but only at boot time
>> (the GOP protocol is more than a magic linear memory region, it also
>> implements a Blt() abstraction that permits the use of framebuffers
>> that are not mapped linearly into the address space at all, and GRUB
>> makes use of this)
>>
>> The EFI stub will only expose GOPs to the kernel if they are in fact
>> linear framebuffers, but has zero insight into whether the hardware
>> needs clocks and regulators, and whether or not the framebuffer needs
>> IOMMU pass through (which might be the case if the scanout is using
>> DMA into system memory)
>>
>> So calling EFI GOP 'source of truth' is rather generous, and I think
>> it makes sense to prioritize more accurate descriptions of the
>> underlying framebuffer over EFI GOP.
>>
>
> That was my opinion as well and the reason why I called the DTB the
> single source of truth.
>
>> However, making 'simple-framebuffer' magic in this regard doesn't seem
>> like a great approach to me. Is there a better way we could get the
>> resource conflict to be decided in a way where the EFI GOP gets
>> superseded if its resources are claimed by another device?
>>
>
> There is an aperture [0] framework that is used by the fbdev and DRM
> subsystems to allow native drivers to remove any conflicting devices
> that share the same framebuffer aperture.
>
> But it only makes sense for native drivers to use that I think, but
> in this case is about two drivers that attempt to use the same frame
> buffer provided by the firmware but getting it from different places.
>
> I don't have a better idea than this patch but maybe Thomas or Sima do?
At SUSE, we've carried such a patch in our repos for some time. It works
around the double-framebuffer problem in a similar way. [1]
As Javier mentioned, we do track the framebuffer ranges for EFI/VESA/OF
framebuffers in the graphics aperture helpers. Fbdev has done this for
decades, and the current codebase extends and harmonizes this
functionality among fbdev and DRM drivers.
WRT DT vs EFI: AFAIK the EFI support on affected platforms looks at the
DT to set up the EFI framebuffer. So IMHO the DT is the authoritative
source on the framebuffer.
Best regards
Thomas
[1] https://bugzilla.suse.com/show_bug.cgi?id=1204315
>
> [0]: https://elixir.bootlin.com/linux/latest/source/drivers/video/aperture.c
>
--
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)
Thomas Zimmermann <[email protected]> writes:
> Hi
>
> Am 18.11.23 um 12:10 schrieb Javier Martinez Canillas:
>> Ard Biesheuvel <[email protected]> writes:
>>
>> Hello Ard,
>>
>>> On Fri, 17 Nov 2023 at 00:09, Rob Herring <[email protected]> wrote:
>>
>> [...]
>>
>>>>>>
>>>>>> This could also lead to an interesting scenario. As simple-framebuffer
>>>>>> can define its memory in a /reserved-memory node, but that is ignored
>>>>>> in EFI boot. Probably would work, but only because EFI probably
>>>>>> generates its memory map table from the /reserved-memory nodes.
>>>>>>
>>>>>
>>>>> I see. So what would be the solution then? Ignoring creating a platform
>>>>> device for "simple-framebuffer" if booted using EFI and have an EFI-GOP?
>>>>
>>>> Shrug. I don't really know anything more about EFI FB, but I would
>>>> guess it can't support handling resources like clocks, power domains,
>>>> regulators, etc. that simple-fb can. So if a platform needs those, do
>>>> we say they should not setup EFI-GOP? Or is there a use case for
>>>> having both? Clients that don't muck with resources can use EFI-GOP
>>>> and those that do use simple-fb. For example, does/can grub use
>>>> EFI-GOP, but not simple-fb?
>>>>
>>>
>>> The EFI GOP is just a dumb framebuffer, and it is not even generally
>>> possible to cross reference the GOP with a particular device in the
>>> device hierarchy unless you e.g., compare the BARs of each device with
>>> the region described by the GOP protocol.
>>>
>>> GRUB for EFI will use the GOP and nothing else, but only at boot time
>>> (the GOP protocol is more than a magic linear memory region, it also
>>> implements a Blt() abstraction that permits the use of framebuffers
>>> that are not mapped linearly into the address space at all, and GRUB
>>> makes use of this)
>>>
>>> The EFI stub will only expose GOPs to the kernel if they are in fact
>>> linear framebuffers, but has zero insight into whether the hardware
>>> needs clocks and regulators, and whether or not the framebuffer needs
>>> IOMMU pass through (which might be the case if the scanout is using
>>> DMA into system memory)
>>>
>>> So calling EFI GOP 'source of truth' is rather generous, and I think
>>> it makes sense to prioritize more accurate descriptions of the
>>> underlying framebuffer over EFI GOP.
>>>
>>
>> That was my opinion as well and the reason why I called the DTB the
>> single source of truth.
>>
>>> However, making 'simple-framebuffer' magic in this regard doesn't seem
>>> like a great approach to me. Is there a better way we could get the
>>> resource conflict to be decided in a way where the EFI GOP gets
>>> superseded if its resources are claimed by another device?
>>>
>>
>> There is an aperture [0] framework that is used by the fbdev and DRM
>> subsystems to allow native drivers to remove any conflicting devices
>> that share the same framebuffer aperture.
>>
>> But it only makes sense for native drivers to use that I think, but
>> in this case is about two drivers that attempt to use the same frame
>> buffer provided by the firmware but getting it from different places.
>>
>> I don't have a better idea than this patch but maybe Thomas or Sima do?
>
> At SUSE, we've carried such a patch in our repos for some time. It works
> around the double-framebuffer problem in a similar way. [1]
>
Thanks for the information. I see that your patch is basically mine but
doing it unconditionally while this one only does the sysfb_disable() if
a "simple-framebuffer" DT node has been found.
> As Javier mentioned, we do track the framebuffer ranges for EFI/VESA/OF
> framebuffers in the graphics aperture helpers. Fbdev has done this for
> decades, and the current codebase extends and harmonizes this
> functionality among fbdev and DRM drivers.
>
> WRT DT vs EFI: AFAIK the EFI support on affected platforms looks at the
> DT to set up the EFI framebuffer. So IMHO the DT is the authoritative
> source on the framebuffer.
>
Agreed. Sima Vetter also mentioned on IRC that they think this patch is
the least bad option. Rob, Ard any thoughts? Maybe we can land this as
a fix and then figure how this could be better handled in the long term?
> Best regards
> Thomas
>
> [1] https://bugzilla.suse.com/show_bug.cgi?id=1204315
>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
On Fri, Dec 1, 2023 at 4:21 AM Javier Martinez Canillas
<[email protected]> wrote:
>
> Thomas Zimmermann <[email protected]> writes:
>
> > Hi
> >
> > Am 18.11.23 um 12:10 schrieb Javier Martinez Canillas:
> >> Ard Biesheuvel <[email protected]> writes:
> >>
> >> Hello Ard,
> >>
> >>> On Fri, 17 Nov 2023 at 00:09, Rob Herring <[email protected]> wrote:
> >>
> >> [...]
> >>
> >>>>>>
> >>>>>> This could also lead to an interesting scenario. As simple-framebuffer
> >>>>>> can define its memory in a /reserved-memory node, but that is ignored
> >>>>>> in EFI boot. Probably would work, but only because EFI probably
> >>>>>> generates its memory map table from the /reserved-memory nodes.
> >>>>>>
> >>>>>
> >>>>> I see. So what would be the solution then? Ignoring creating a platform
> >>>>> device for "simple-framebuffer" if booted using EFI and have an EFI-GOP?
> >>>>
> >>>> Shrug. I don't really know anything more about EFI FB, but I would
> >>>> guess it can't support handling resources like clocks, power domains,
> >>>> regulators, etc. that simple-fb can. So if a platform needs those, do
> >>>> we say they should not setup EFI-GOP? Or is there a use case for
> >>>> having both? Clients that don't muck with resources can use EFI-GOP
> >>>> and those that do use simple-fb. For example, does/can grub use
> >>>> EFI-GOP, but not simple-fb?
> >>>>
> >>>
> >>> The EFI GOP is just a dumb framebuffer, and it is not even generally
> >>> possible to cross reference the GOP with a particular device in the
> >>> device hierarchy unless you e.g., compare the BARs of each device with
> >>> the region described by the GOP protocol.
> >>>
> >>> GRUB for EFI will use the GOP and nothing else, but only at boot time
> >>> (the GOP protocol is more than a magic linear memory region, it also
> >>> implements a Blt() abstraction that permits the use of framebuffers
> >>> that are not mapped linearly into the address space at all, and GRUB
> >>> makes use of this)
> >>>
> >>> The EFI stub will only expose GOPs to the kernel if they are in fact
> >>> linear framebuffers, but has zero insight into whether the hardware
> >>> needs clocks and regulators, and whether or not the framebuffer needs
> >>> IOMMU pass through (which might be the case if the scanout is using
> >>> DMA into system memory)
> >>>
> >>> So calling EFI GOP 'source of truth' is rather generous, and I think
> >>> it makes sense to prioritize more accurate descriptions of the
> >>> underlying framebuffer over EFI GOP.
> >>>
> >>
> >> That was my opinion as well and the reason why I called the DTB the
> >> single source of truth.
> >>
> >>> However, making 'simple-framebuffer' magic in this regard doesn't seem
> >>> like a great approach to me. Is there a better way we could get the
> >>> resource conflict to be decided in a way where the EFI GOP gets
> >>> superseded if its resources are claimed by another device?
> >>>
> >>
> >> There is an aperture [0] framework that is used by the fbdev and DRM
> >> subsystems to allow native drivers to remove any conflicting devices
> >> that share the same framebuffer aperture.
> >>
> >> But it only makes sense for native drivers to use that I think, but
> >> in this case is about two drivers that attempt to use the same frame
> >> buffer provided by the firmware but getting it from different places.
> >>
> >> I don't have a better idea than this patch but maybe Thomas or Sima do?
> >
> > At SUSE, we've carried such a patch in our repos for some time. It works
> > around the double-framebuffer problem in a similar way. [1]
> >
>
> Thanks for the information. I see that your patch is basically mine but
> doing it unconditionally while this one only does the sysfb_disable() if
> a "simple-framebuffer" DT node has been found.
>
> > As Javier mentioned, we do track the framebuffer ranges for EFI/VESA/OF
> > framebuffers in the graphics aperture helpers. Fbdev has done this for
> > decades, and the current codebase extends and harmonizes this
> > functionality among fbdev and DRM drivers.
> >
> > WRT DT vs EFI: AFAIK the EFI support on affected platforms looks at the
> > DT to set up the EFI framebuffer. So IMHO the DT is the authoritative
> > source on the framebuffer.
> >
>
> Agreed. Sima Vetter also mentioned on IRC that they think this patch is
> the least bad option. Rob, Ard any thoughts? Maybe we can land this as
> a fix and then figure how this could be better handled in the long term?
What about moving the DT setup code here to sysfb? Then we just setup
the right thing up front.
However, there might be one other issue with that and this fix. The DT
simplefb can have resources such as clocks and regulators. With
fw_devlink, the driver won't probe until those dependencies are met.
So if you want the framebuffer console up early, then you may want to
register the EFI framebuffer first and then handoff to the DT simplefb
when it probes (rather than registering the device).
But I agree, probably better to take this patch now and have those
quirks instead of flat out not working.
Rob
Rob Herring <[email protected]> writes:
Hello Rob,
> On Fri, Dec 1, 2023 at 4:21 AM Javier Martinez Canillas
[...]
>> >> I don't have a better idea than this patch but maybe Thomas or Sima do?
>> >
>> > At SUSE, we've carried such a patch in our repos for some time. It works
>> > around the double-framebuffer problem in a similar way. [1]
>> >
>>
>> Thanks for the information. I see that your patch is basically mine but
>> doing it unconditionally while this one only does the sysfb_disable() if
>> a "simple-framebuffer" DT node has been found.
>>
>> > As Javier mentioned, we do track the framebuffer ranges for EFI/VESA/OF
>> > framebuffers in the graphics aperture helpers. Fbdev has done this for
>> > decades, and the current codebase extends and harmonizes this
>> > functionality among fbdev and DRM drivers.
>> >
>> > WRT DT vs EFI: AFAIK the EFI support on affected platforms looks at the
>> > DT to set up the EFI framebuffer. So IMHO the DT is the authoritative
>> > source on the framebuffer.
>> >
>>
>> Agreed. Sima Vetter also mentioned on IRC that they think this patch is
>> the least bad option. Rob, Ard any thoughts? Maybe we can land this as
>> a fix and then figure how this could be better handled in the long term?
>
> What about moving the DT setup code here to sysfb? Then we just setup
> the right thing up front.
>
Right now sysfb is pretty platform agnostic, in the sense that just looks
at the screen_info global struct and uses it to add the platform data that
contains the firmware provided framebuffer.
How the screen_info was filled (e.g: by the Linux EFI stub using the GOP
or the x86 early boot code using VESA) is transparent to sysfb. For this
reason adding platform specific logic there, like finding OF nodes, is not
desirable.
I also suggested to Thomas to move the DT setup code to sysfb but he said
that would be the wrong approach for the reason mentioned above. Please
correct me Thomas if I'm misremembering here.
> However, there might be one other issue with that and this fix. The DT
> simplefb can have resources such as clocks and regulators. With
> fw_devlink, the driver won't probe until those dependencies are met.
> So if you want the framebuffer console up early, then you may want to
> register the EFI framebuffer first and then handoff to the DT simplefb
> when it probes (rather than registering the device).
>
> But I agree, probably better to take this patch now and have those
> quirks instead of flat out not working.
>
If we do that what's the plan? Are you thinking about merging this patch
through your OF tree or do you want to go through drm-misc with your ack?
> Rob
>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
On Mon, Dec 4, 2023 at 3:39 AM Javier Martinez Canillas
<[email protected]> wrote:
>
> Rob Herring <[email protected]> writes:
>
> Hello Rob,
>
> > On Fri, Dec 1, 2023 at 4:21 AM Javier Martinez Canillas
>
> [...]
>
> >> >> I don't have a better idea than this patch but maybe Thomas or Sima do?
> >> >
> >> > At SUSE, we've carried such a patch in our repos for some time. It works
> >> > around the double-framebuffer problem in a similar way. [1]
> >> >
> >>
> >> Thanks for the information. I see that your patch is basically mine but
> >> doing it unconditionally while this one only does the sysfb_disable() if
> >> a "simple-framebuffer" DT node has been found.
> >>
> >> > As Javier mentioned, we do track the framebuffer ranges for EFI/VESA/OF
> >> > framebuffers in the graphics aperture helpers. Fbdev has done this for
> >> > decades, and the current codebase extends and harmonizes this
> >> > functionality among fbdev and DRM drivers.
> >> >
> >> > WRT DT vs EFI: AFAIK the EFI support on affected platforms looks at the
> >> > DT to set up the EFI framebuffer. So IMHO the DT is the authoritative
> >> > source on the framebuffer.
> >> >
> >>
> >> Agreed. Sima Vetter also mentioned on IRC that they think this patch is
> >> the least bad option. Rob, Ard any thoughts? Maybe we can land this as
> >> a fix and then figure how this could be better handled in the long term?
> >
> > What about moving the DT setup code here to sysfb? Then we just setup
> > the right thing up front.
> >
>
> Right now sysfb is pretty platform agnostic, in the sense that just looks
> at the screen_info global struct and uses it to add the platform data that
> contains the firmware provided framebuffer.
>
> How the screen_info was filled (e.g: by the Linux EFI stub using the GOP
> or the x86 early boot code using VESA) is transparent to sysfb. For this
> reason adding platform specific logic there, like finding OF nodes, is not
> desirable.
>
> I also suggested to Thomas to move the DT setup code to sysfb but he said
> that would be the wrong approach for the reason mentioned above. Please
> correct me Thomas if I'm misremembering here.
>
> > However, there might be one other issue with that and this fix. The DT
> > simplefb can have resources such as clocks and regulators. With
> > fw_devlink, the driver won't probe until those dependencies are met.
> > So if you want the framebuffer console up early, then you may want to
> > register the EFI framebuffer first and then handoff to the DT simplefb
> > when it probes (rather than registering the device).
> >
> > But I agree, probably better to take this patch now and have those
> > quirks instead of flat out not working.
> >
>
> If we do that what's the plan? Are you thinking about merging this patch
> through your OF tree or do you want to go through drm-misc with your ack?
I can take it. Do we need this in 6.7 and stable?
Rob
Rob Herring <[email protected]> writes:
> On Mon, Dec 4, 2023 at 3:39 AM Javier Martinez Canillas
> <[email protected]> wrote:
>> Rob Herring <[email protected]> writes:
>> > On Fri, Dec 1, 2023 at 4:21 AM Javier Martinez Canillas
[...]
>>
>> > However, there might be one other issue with that and this fix. The DT
>> > simplefb can have resources such as clocks and regulators. With
>> > fw_devlink, the driver won't probe until those dependencies are met.
>> > So if you want the framebuffer console up early, then you may want to
>> > register the EFI framebuffer first and then handoff to the DT simplefb
>> > when it probes (rather than registering the device).
>> >
>> > But I agree, probably better to take this patch now and have those
>> > quirks instead of flat out not working.
>> >
>>
>> If we do that what's the plan? Are you thinking about merging this patch
>> through your OF tree or do you want to go through drm-misc with your ack?
>
> I can take it. Do we need this in 6.7 and stable?
>
IMO this can wait for v6.8 since is not a fix for a change introduced in
the v6.7 merge window and something that only happens on a very specific
setup (DT systems booting with u-boot EFI and providing an EFI-GOP table).
Also the -rc cycle is already in -rc5, so it seems risky to push a change
at this point. And distros can pick the patch if want to have it earlier.
> Rob
>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
On Mon, Dec 04, 2023 at 05:05:30PM +0100, Javier Martinez Canillas wrote:
> Rob Herring <[email protected]> writes:
>
> > On Mon, Dec 4, 2023 at 3:39 AM Javier Martinez Canillas
> > <[email protected]> wrote:
> >> Rob Herring <[email protected]> writes:
> >> > On Fri, Dec 1, 2023 at 4:21 AM Javier Martinez Canillas
>
> [...]
>
> >>
> >> > However, there might be one other issue with that and this fix. The DT
> >> > simplefb can have resources such as clocks and regulators. With
> >> > fw_devlink, the driver won't probe until those dependencies are met.
> >> > So if you want the framebuffer console up early, then you may want to
> >> > register the EFI framebuffer first and then handoff to the DT simplefb
> >> > when it probes (rather than registering the device).
> >> >
> >> > But I agree, probably better to take this patch now and have those
> >> > quirks instead of flat out not working.
> >> >
> >>
> >> If we do that what's the plan? Are you thinking about merging this patch
> >> through your OF tree or do you want to go through drm-misc with your ack?
> >
> > I can take it. Do we need this in 6.7 and stable?
> >
>
> IMO this can wait for v6.8 since is not a fix for a change introduced in
> the v6.7 merge window and something that only happens on a very specific
> setup (DT systems booting with u-boot EFI and providing an EFI-GOP table).
>
> Also the -rc cycle is already in -rc5, so it seems risky to push a change
> at this point. And distros can pick the patch if want to have it earlier.
Okay, I've applied it for 6.8.
Rob
Rob Herring <[email protected]> writes:
> On Mon, Dec 04, 2023 at 05:05:30PM +0100, Javier Martinez Canillas wrote:
>> Rob Herring <[email protected]> writes:
>>
>> > On Mon, Dec 4, 2023 at 3:39 AM Javier Martinez Canillas
>> > <[email protected]> wrote:
>> >> Rob Herring <[email protected]> writes:
>> >> > On Fri, Dec 1, 2023 at 4:21 AM Javier Martinez Canillas
>>
>> [...]
>>
>> >>
>> >> > However, there might be one other issue with that and this fix. The DT
>> >> > simplefb can have resources such as clocks and regulators. With
>> >> > fw_devlink, the driver won't probe until those dependencies are met.
>> >> > So if you want the framebuffer console up early, then you may want to
>> >> > register the EFI framebuffer first and then handoff to the DT simplefb
>> >> > when it probes (rather than registering the device).
>> >> >
>> >> > But I agree, probably better to take this patch now and have those
>> >> > quirks instead of flat out not working.
>> >> >
>> >>
>> >> If we do that what's the plan? Are you thinking about merging this patch
>> >> through your OF tree or do you want to go through drm-misc with your ack?
>> >
>> > I can take it. Do we need this in 6.7 and stable?
>> >
>>
>> IMO this can wait for v6.8 since is not a fix for a change introduced in
>> the v6.7 merge window and something that only happens on a very specific
>> setup (DT systems booting with u-boot EFI and providing an EFI-GOP table).
>>
>> Also the -rc cycle is already in -rc5, so it seems risky to push a change
>> at this point. And distros can pick the patch if want to have it earlier.
>
> Okay, I've applied it for 6.8.
>
Great, thanks a lot.
> Rob
>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat