Hi Andy,
On 7/29/22 00:19, Andy Shevchenko wrote:
On Thu, Jul 28, 2022 at 4:32 PM Markuss Broks <markuss.broks@xxxxxxxxx> wrote:
Add early console support for generic linear framebuffer devices.
This driver supports probing from cmdline early parameters
or from the device-tree using information in simple-framebuffer node.
The EFI functionality should be retained in whole.
The driver was disabled on ARM because of a bug in early_ioremap
implementation on ARM.
...
- efifb,[options]
+ efifb
Start an early, unaccelerated console on the EFI
- memory mapped framebuffer (if available). On cache
- coherent non-x86 systems that use system memory for
- the framebuffer, pass the 'ram' option so that it is
- mapped with the correct attributes.
+ memory mapped framebuffer (if available).
If somebody passes those (legacy) options, what will happen?
Those would be ignored. Instead it would be automatically determined if
framebuffer is located in RAM or in the I/O space.
...
config EFI_EARLYCON
- def_bool y
- depends on SERIAL_EARLYCON && !ARM && !IA64
- select FONT_SUPPORT
- select ARCH_USE_MEMREMAP_PROT
+ bool "EFI early console support"
+ depends on FB_EARLYCON && !IA64
This doesn't sound right. Previously on my configuration it was
selected automatically, now I need to select it explicitly? I mean
that for me EFI_EARLYCON should be selected by default as it was
before.
The problem I had with EFI_EARLYCON selected by default was that it
would also carry fbdev with itself. Luckily, that's solved if it's moved
to console subsystem.
...
+static int __init simplefb_earlycon_remap_fb(void)
+{
+ int is_ram;
+ blank line.
+ /* bail if there is no bootconsole or it has been disabled already */
+ if (!earlycon_console || !(earlycon_console->flags & CON_ENABLED))
+ return 0;
+
+ is_ram = region_intersects(info.phys_base, info.size,
+ IORESOURCE_SYSTEM_RAM, IORES_DESC_NONE);
+ is_ram = is_ram == REGION_INTERSECTS;
Was it in the original code? Otherwise, I would go with plain conditional:
if (region_intersects())
base = ...
else
base = ...
It wasn't in original code. Original code assumed MEMREMAP_WC always
unless "ram" was specified as an option to efifb (e.g.
earlycon=efifb,ram). This code instead checks if the framebuffer is in
RAM, and sets the mapping accordingly.
Another issue is that region_intersects() returns REGION_INTERSECTS
(defined as 0) when true. I suppose we could use something like:
if (region_intersects() == REGION_INTERSECTS)
...
+ info.virt_base = memremap(info.phys_base, info.size,
+ is_ram ? MEMREMAP_WB : MEMREMAP_WC);
+
+ return info.virt_base ? 0 : -ENOMEM;
+}
...
+static void simplefb_earlycon_write_char(u8 *dst, unsigned char c, unsigned int h)
+{
+ const u8 *src;
+ int m, n, bytes;
+ u8 x;
+
+ bytes = BITS_TO_BYTES(font->width);
+ src = font->data + c * font->height * bytes + h * bytes;
+
+ for (m = 0; m < font->width; m++) {
+ n = m % 8;
+ x = *(src + m / 8);
+ if ((x >> (7 - n)) & 1)
+ memset(dst, 0xff, (info.depth / 8));
+ else
+ memset(dst, 0, (info.depth / 8));
+ dst += (info.depth / 8);
+ }
+}
Wondering if we already have something like this in DRM/fbdev and can
split into a generic helper.
...
+ ret = sscanf(device->options, "%u,%u,%u", &info.x, &info.y, &info.depth);
+ if (ret != 3)
+ return -ENODEV;
Don't we have some standard template of this, something like XxYxD,
where X, Y, and D are respective decimal numbers?
I'm not aware of this.
-Markuss
