[PATCH 0/2] Add generic framebuffer support to EFI earlycon driver

[PATCH 0/2] Add generic framebuffer support to EFI earlycon driver

[Markuss Broks]

Make the EFI earlycon driver be suitable for any linear framebuffers.
This should be helpful for early porting of boards with no other means of
output, like smartphones/tablets. There seems to be an issue with early_ioremap
function on ARM32, but I am unable to find the exact cause. It appears the mappings
returned by it are somehow incorrect, thus the driver is disabled on ARM. EFI early
console was disabled on IA64 previously, so I kept it in EFI earlycon Kconfig.

This patch also changes behavior on EFI systems, by selecting the mapping type
based on if the framebuffer region intersects with system RAM. If it does, it's
common sense that it should be in RAM as a whole, and so the system RAM mapping is
used. It was tested to be working on my PC (Intel Z490 platform).

Markuss Broks (2):
  drivers: serial: earlycon: Pass device-tree node
  efi: earlycon: Add support for generic framebuffers and move to fbdev
    subsystem

 .../admin-guide/kernel-parameters.txt         |  12 +-
 MAINTAINERS                                   |   5 +
 drivers/firmware/efi/Kconfig                  |   6 +-
 drivers/firmware/efi/Makefile                 |   1 -
 drivers/firmware/efi/earlycon.c               | 246 --------------
 drivers/tty/serial/earlycon.c                 |   3 +
 drivers/video/fbdev/Kconfig                   |  11 +
 drivers/video/fbdev/Makefile                  |   1 +
 drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
 include/linux/serial_core.h                   |   1 +
 10 files changed, 331 insertions(+), 256 deletions(-)
 delete mode 100644 drivers/firmware/efi/earlycon.c
 create mode 100644 drivers/video/fbdev/earlycon.c

-- 
2.37.0

[PATCH 0/2] Add generic framebuffer support to EFI earlycon driver

[Markuss Broks]

Make the EFI earlycon driver be suitable for any linear framebuffers.
This should be helpful for early porting of boards with no other means of
output, like smartphones/tablets. There seems to be an issue with early_ioremap
function on ARM32, but I am unable to find the exact cause. It appears the mappings
returned by it are somehow incorrect, thus the driver is disabled on ARM. EFI early
console was disabled on IA64 previously, so I kept it in EFI earlycon Kconfig.

This patch also changes behavior on EFI systems, by selecting the mapping type
based on if the framebuffer region intersects with system RAM. If it does, it's
common sense that it should be in RAM as a whole, and so the system RAM mapping is
used. It was tested to be working on my PC (Intel Z490 platform).

Markuss Broks (2):
  drivers: serial: earlycon: Pass device-tree node
  efi: earlycon: Add support for generic framebuffers and move to fbdev
    subsystem

 .../admin-guide/kernel-parameters.txt         |  12 +-
 MAINTAINERS                                   |   5 +
 drivers/firmware/efi/Kconfig                  |   6 +-
 drivers/firmware/efi/Makefile                 |   1 -
 drivers/firmware/efi/earlycon.c               | 246 --------------
 drivers/tty/serial/earlycon.c                 |   3 +
 drivers/video/fbdev/Kconfig                   |  11 +
 drivers/video/fbdev/Makefile                  |   1 +
 drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
 include/linux/serial_core.h                   |   1 +
 10 files changed, 331 insertions(+), 256 deletions(-)
 delete mode 100644 drivers/firmware/efi/earlycon.c
 create mode 100644 drivers/video/fbdev/earlycon.c

-- 
2.37.0

[PATCH 1/2] drivers: serial: earlycon: Pass device-tree node

[Markuss Broks]

Pass a pointer to device-tree node in case the driver probed from
OF. This makes early console drivers able to fetch options from
device-tree node properties.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 drivers/tty/serial/earlycon.c | 3 +++
 include/linux/serial_core.h   | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index 57c70851f22a0e78805f34d1a7700708104b6f6a..14e8a7fe54486a1c377a6659c37a73858de5bf0b 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -304,6 +304,9 @@ int __init of_setup_earlycon(const struct earlycon_id *match,
 		strlcpy(early_console_dev.options, options,
 			sizeof(early_console_dev.options));
 	}
+
+	early_console_dev.node = node;
+
 	earlycon_init(&early_console_dev, match->name);
 	err = match->setup(&early_console_dev, options);
 	earlycon_print_info(&early_console_dev);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index cbd5070bc87f42aa450c4ca7af8a9b59fbe88574..3295721f33e482124fae8370b5889d5d6c012303 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -349,6 +349,7 @@ struct earlycon_device {
 	struct uart_port port;
 	char options[16];		/* e.g., 115200n8 */
 	unsigned int baud;
+	unsigned long node;
 };
 
 struct earlycon_id {
-- 
2.37.0

[PATCH 1/2] drivers: serial: earlycon: Pass device-tree node

[Markuss Broks]

Pass a pointer to device-tree node in case the driver probed from
OF. This makes early console drivers able to fetch options from
device-tree node properties.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 drivers/tty/serial/earlycon.c | 3 +++
 include/linux/serial_core.h   | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index 57c70851f22a0e78805f34d1a7700708104b6f6a..14e8a7fe54486a1c377a6659c37a73858de5bf0b 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -304,6 +304,9 @@ int __init of_setup_earlycon(const struct earlycon_id *match,
 		strlcpy(early_console_dev.options, options,
 			sizeof(early_console_dev.options));
 	}
+
+	early_console_dev.node = node;
+
 	earlycon_init(&early_console_dev, match->name);
 	err = match->setup(&early_console_dev, options);
 	earlycon_print_info(&early_console_dev);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index cbd5070bc87f42aa450c4ca7af8a9b59fbe88574..3295721f33e482124fae8370b5889d5d6c012303 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -349,6 +349,7 @@ struct earlycon_device {
 	struct uart_port port;
 	char options[16];		/* e.g., 115200n8 */
 	unsigned int baud;
+	unsigned long node;
 };
 
 struct earlycon_id {
-- 
2.37.0

[PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Markuss Broks]

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.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 .../admin-guide/kernel-parameters.txt         |  12 +-
 MAINTAINERS                                   |   5 +
 drivers/firmware/efi/Kconfig                  |   6 +-
 drivers/firmware/efi/Makefile                 |   1 -
 drivers/firmware/efi/earlycon.c               | 246 --------------
 drivers/video/fbdev/Kconfig                   |  11 +
 drivers/video/fbdev/Makefile                  |   1 +
 drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
 8 files changed, 327 insertions(+), 256 deletions(-)
 delete mode 100644 drivers/firmware/efi/earlycon.c
 create mode 100644 drivers/video/fbdev/earlycon.c

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 8090130b544b0701237a7b657a29c83c000a60f4..22a8625bb342f95e731fcc3a24390fca2c0f194c 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1281,12 +1281,9 @@
 			specified address. The serial port must already be
 			setup and configured. Options are not yet supported.
 
-		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).
 
 		linflex,<addr>
 			Use early console provided by Freescale LINFlexD UART
@@ -1294,6 +1291,11 @@
 			address must be provided, and the serial port must
 			already be setup and configured.
 
+		simplefb,<addr>,<width>,<height>,<bpp>
+			Use early console with simple framebuffer that is
+			pre-initialized by firmware. A valid base address,
+			width, height and pixel size must be provided.
+
 	earlyprintk=	[X86,SH,ARM,M68k,S390]
 			earlyprintk=vga
 			earlyprintk=sclp
diff --git a/MAINTAINERS b/MAINTAINERS
index 1fc9ead83d2aa3e60ccc4cfa8ee16df09ef579bf..140dee7f6920197a895d310afdde67a0de474522 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7033,6 +7033,11 @@ Q:	http://patchwork.linuxtv.org/project/linux-media/list/
 T:	git git://linuxtv.org/anttip/media_tree.git
 F:	drivers/media/tuners/e4000*
 
+EARLY CONSOLE FRAMEBUFFER DRIVER
+M:	Markuss Broks <markuss.broks@gmail.com>
+S:	Maintained
+F:	drivers/video/fbdev/earlycon.c
+
 EARTH_PT1 MEDIA DRIVER
 M:	Akihiro Tsukada <tskd08@gmail.com>
 L:	linux-media@vger.kernel.org
diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index 7aa4717cdcac46f91dd202f868c463388eb02735..56241b2e19383d70387f07114a27bb4f33ac7857 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -259,10 +259,8 @@ config EFI_DISABLE_PCI_DMA
 	  may be used to override this option.
 
 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
 
 config EFI_CUSTOM_SSDT_OVERLAYS
 	bool "Load custom ACPI SSDT overlay from an EFI variable"
diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index c02ff25dd47707090a2ab86ee4f330e467f878f5..64eea61fbb43d76ec2d5416d467dfbb9aa21bda0 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -44,6 +44,5 @@ obj-$(CONFIG_ARM64)			+= $(arm-obj-y)
 riscv-obj-$(CONFIG_EFI)			:= efi-init.o riscv-runtime.o
 obj-$(CONFIG_RISCV)			+= $(riscv-obj-y)
 obj-$(CONFIG_EFI_CAPSULE_LOADER)	+= capsule-loader.o
-obj-$(CONFIG_EFI_EARLYCON)		+= earlycon.o
 obj-$(CONFIG_UEFI_CPER_ARM)		+= cper-arm.o
 obj-$(CONFIG_UEFI_CPER_X86)		+= cper-x86.o
diff --git a/drivers/firmware/efi/earlycon.c b/drivers/firmware/efi/earlycon.c
deleted file mode 100644
index a52236e11e5f73ddea5bb1f42ca2ca7c42425dab..0000000000000000000000000000000000000000
--- a/drivers/firmware/efi/earlycon.c
+++ /dev/null
@@ -1,246 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Copyright (C) 2013 Intel Corporation; author Matt Fleming
- */
-
-#include <linux/console.h>
-#include <linux/efi.h>
-#include <linux/font.h>
-#include <linux/io.h>
-#include <linux/kernel.h>
-#include <linux/serial_core.h>
-#include <linux/screen_info.h>
-
-#include <asm/early_ioremap.h>
-
-static const struct console *earlycon_console __initdata;
-static const struct font_desc *font;
-static u32 efi_x, efi_y;
-static u64 fb_base;
-static bool fb_wb;
-static void *efi_fb;
-
-/*
- * EFI earlycon needs to use early_memremap() to map the framebuffer.
- * But early_memremap() is not usable for 'earlycon=efifb keep_bootcon',
- * memremap() should be used instead. memremap() will be available after
- * paging_init() which is earlier than initcall callbacks. Thus adding this
- * early initcall function early_efi_map_fb() to map the whole EFI framebuffer.
- */
-static int __init efi_earlycon_remap_fb(void)
-{
-	/* bail if there is no bootconsole or it has been disabled already */
-	if (!earlycon_console || !(earlycon_console->flags & CON_ENABLED))
-		return 0;
-
-	efi_fb = memremap(fb_base, screen_info.lfb_size,
-			  fb_wb ? MEMREMAP_WB : MEMREMAP_WC);
-
-	return efi_fb ? 0 : -ENOMEM;
-}
-early_initcall(efi_earlycon_remap_fb);
-
-static int __init efi_earlycon_unmap_fb(void)
-{
-	/* unmap the bootconsole fb unless keep_bootcon has left it enabled */
-	if (efi_fb && !(earlycon_console->flags & CON_ENABLED))
-		memunmap(efi_fb);
-	return 0;
-}
-late_initcall(efi_earlycon_unmap_fb);
-
-static __ref void *efi_earlycon_map(unsigned long start, unsigned long len)
-{
-	pgprot_t fb_prot;
-
-	if (efi_fb)
-		return efi_fb + start;
-
-	fb_prot = fb_wb ? PAGE_KERNEL : pgprot_writecombine(PAGE_KERNEL);
-	return early_memremap_prot(fb_base + start, len, pgprot_val(fb_prot));
-}
-
-static __ref void efi_earlycon_unmap(void *addr, unsigned long len)
-{
-	if (efi_fb)
-		return;
-
-	early_memunmap(addr, len);
-}
-
-static void efi_earlycon_clear_scanline(unsigned int y)
-{
-	unsigned long *dst;
-	u16 len;
-
-	len = screen_info.lfb_linelength;
-	dst = efi_earlycon_map(y*len, len);
-	if (!dst)
-		return;
-
-	memset(dst, 0, len);
-	efi_earlycon_unmap(dst, len);
-}
-
-static void efi_earlycon_scroll_up(void)
-{
-	unsigned long *dst, *src;
-	u16 len;
-	u32 i, height;
-
-	len = screen_info.lfb_linelength;
-	height = screen_info.lfb_height;
-
-	for (i = 0; i < height - font->height; i++) {
-		dst = efi_earlycon_map(i*len, len);
-		if (!dst)
-			return;
-
-		src = efi_earlycon_map((i + font->height) * len, len);
-		if (!src) {
-			efi_earlycon_unmap(dst, len);
-			return;
-		}
-
-		memmove(dst, src, len);
-
-		efi_earlycon_unmap(src, len);
-		efi_earlycon_unmap(dst, len);
-	}
-}
-
-static void efi_earlycon_write_char(u32 *dst, unsigned char c, unsigned int h)
-{
-	const u32 color_black = 0x00000000;
-	const u32 color_white = 0x00ffffff;
-	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)
-			*dst = color_white;
-		else
-			*dst = color_black;
-		dst++;
-	}
-}
-
-static void
-efi_earlycon_write(struct console *con, const char *str, unsigned int num)
-{
-	struct screen_info *si;
-	unsigned int len;
-	const char *s;
-	void *dst;
-
-	si = &screen_info;
-	len = si->lfb_linelength;
-
-	while (num) {
-		unsigned int linemax;
-		unsigned int h, count = 0;
-
-		for (s = str; *s && *s != '\n'; s++) {
-			if (count == num)
-				break;
-			count++;
-		}
-
-		linemax = (si->lfb_width - efi_x) / font->width;
-		if (count > linemax)
-			count = linemax;
-
-		for (h = 0; h < font->height; h++) {
-			unsigned int n, x;
-
-			dst = efi_earlycon_map((efi_y + h) * len, len);
-			if (!dst)
-				return;
-
-			s = str;
-			n = count;
-			x = efi_x;
-
-			while (n-- > 0) {
-				efi_earlycon_write_char(dst + x*4, *s, h);
-				x += font->width;
-				s++;
-			}
-
-			efi_earlycon_unmap(dst, len);
-		}
-
-		num -= count;
-		efi_x += count * font->width;
-		str += count;
-
-		if (num > 0 && *s == '\n') {
-			efi_x = 0;
-			efi_y += font->height;
-			str++;
-			num--;
-		}
-
-		if (efi_x + font->width > si->lfb_width) {
-			efi_x = 0;
-			efi_y += font->height;
-		}
-
-		if (efi_y + font->height > si->lfb_height) {
-			u32 i;
-
-			efi_y -= font->height;
-			efi_earlycon_scroll_up();
-
-			for (i = 0; i < font->height; i++)
-				efi_earlycon_clear_scanline(efi_y + i);
-		}
-	}
-}
-
-static int __init efi_earlycon_setup(struct earlycon_device *device,
-				     const char *opt)
-{
-	struct screen_info *si;
-	u16 xres, yres;
-	u32 i;
-
-	if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI)
-		return -ENODEV;
-
-	fb_base = screen_info.lfb_base;
-	if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
-		fb_base |= (u64)screen_info.ext_lfb_base << 32;
-
-	fb_wb = opt && !strcmp(opt, "ram");
-
-	si = &screen_info;
-	xres = si->lfb_width;
-	yres = si->lfb_height;
-
-	/*
-	 * efi_earlycon_write_char() implicitly assumes a framebuffer with
-	 * 32 bits per pixel.
-	 */
-	if (si->lfb_depth != 32)
-		return -ENODEV;
-
-	font = get_default_font(xres, yres, -1, -1);
-	if (!font)
-		return -ENODEV;
-
-	efi_y = rounddown(yres, font->height) - font->height;
-	for (i = 0; i < (yres - efi_y) / font->height; i++)
-		efi_earlycon_scroll_up();
-
-	device->con->write = efi_earlycon_write;
-	earlycon_console = device->con;
-	return 0;
-}
-EARLYCON_DECLARE(efifb, efi_earlycon_setup);
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index f2a6b81e45c41334db71cca35f4fa5a827f728fe..2bb3f7a9e6e0f8ad2e9042d8c602b1cdec30755c 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -627,6 +627,17 @@ config FB_VESA
 	  You will get a boot time penguin logo at no additional cost. Please
 	  read <file:Documentation/fb/vesafb.rst>. If unsure, say Y.
 
+config FB_EARLYCON
+	bool "Generic framebuffer early console"
+	depends on FB && SERIAL_EARLYCON && !ARM
+	select FONT_SUPPORT
+	select ARCH_USE_MEMREMAP_PROT
+	help
+	  Say Y here if you want early console support for firmware established
+	  linear framebuffer. Unless you are using EFI framebuffer, you need to
+	  specify framebuffer geometry and address in device-tree or in kernel
+	  command line.
+
 config FB_EFI
 	bool "EFI-based Framebuffer Support"
 	depends on (FB = y) && !IA64 && EFI
diff --git a/drivers/video/fbdev/Makefile b/drivers/video/fbdev/Makefile
index 7795c4126706fd7bb832d94da5d14b0060849d1f..07406ab45a3f306a07d008a9f0a401347aeaa695 100644
--- a/drivers/video/fbdev/Makefile
+++ b/drivers/video/fbdev/Makefile
@@ -129,6 +129,7 @@ obj-$(CONFIG_FB_MX3)		  += mx3fb.o
 obj-$(CONFIG_FB_DA8XX)		  += da8xx-fb.o
 obj-$(CONFIG_FB_SSD1307)	  += ssd1307fb.o
 obj-$(CONFIG_FB_SIMPLE)           += simplefb.o
+obj-$(CONFIG_FB_EARLYCON)         += earlycon.o
 
 # the test framebuffer is last
 obj-$(CONFIG_FB_VIRTUAL)          += vfb.o
diff --git a/drivers/video/fbdev/earlycon.c b/drivers/video/fbdev/earlycon.c
new file mode 100644
index 0000000000000000000000000000000000000000..5f6f4b228a50f23c1c77cf7ba5d50ff833efdd46
--- /dev/null
+++ b/drivers/video/fbdev/earlycon.c
@@ -0,0 +1,301 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2013 Intel Corporation; author Matt Fleming
+ * Copyright (C) 2022 Markuss Broks <markuss.broks@gmail.com>
+ */
+
+#include <asm/early_ioremap.h>
+#include <linux/console.h>
+#include <linux/efi.h>
+#include <linux/font.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+#include <linux/serial_core.h>
+#include <linux/screen_info.h>
+
+struct fb_earlycon {
+	u32 x, y, curr_x, curr_y, depth, stride;
+	size_t size;
+	phys_addr_t phys_base;
+	void __iomem *virt_base;
+};
+
+static const struct console *earlycon_console __initconst;
+static struct fb_earlycon info;
+static const struct font_desc *font;
+
+static int __init simplefb_earlycon_remap_fb(void)
+{
+	int is_ram;
+	/* 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;
+
+	info.virt_base = memremap(info.phys_base, info.size,
+				  is_ram ? MEMREMAP_WB : MEMREMAP_WC);
+
+	return info.virt_base ? 0 : -ENOMEM;
+}
+early_initcall(simplefb_earlycon_remap_fb);
+
+static int __init simplefb_earlycon_unmap_fb(void)
+{
+	/* unmap the bootconsole fb unless keep_bootcon has left it enabled */
+	if (info.virt_base && !(earlycon_console->flags & CON_ENABLED))
+		memunmap(info.virt_base);
+	return 0;
+}
+late_initcall(simplefb_earlycon_unmap_fb);
+
+static __ref void *simplefb_earlycon_map(unsigned long start, unsigned long len)
+{
+	pgprot_t fb_prot;
+
+	if (info.virt_base)
+		return info.virt_base + start;
+
+	fb_prot = PAGE_KERNEL;
+	return early_memremap_prot(info.phys_base + start, len, pgprot_val(fb_prot));
+}
+
+static __ref void simplefb_earlycon_unmap(void *addr, unsigned long len)
+{
+	if (info.virt_base)
+		return;
+
+	early_memunmap(addr, len);
+}
+
+static void simplefb_earlycon_clear_scanline(unsigned int y)
+{
+	unsigned long *dst;
+	u16 len;
+
+	len = info.stride;
+	dst = simplefb_earlycon_map(y * len, len);
+	if (!dst)
+		return;
+
+	memset(dst, 0, len);
+	simplefb_earlycon_unmap(dst, len);
+}
+
+static void simplefb_earlycon_scroll_up(void)
+{
+	unsigned long *dst, *src;
+	u16 len;
+	u32 i, height;
+
+	len = info.stride;
+	height = info.y;
+
+	for (i = 0; i < height - font->height; i++) {
+		dst = simplefb_earlycon_map(i * len, len);
+		if (!dst)
+			return;
+
+		src = simplefb_earlycon_map((i + font->height) * len, len);
+		if (!src) {
+			simplefb_earlycon_unmap(dst, len);
+			return;
+		}
+
+		memmove(dst, src, len);
+
+		simplefb_earlycon_unmap(src, len);
+		simplefb_earlycon_unmap(dst, len);
+	}
+}
+
+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);
+	}
+}
+
+static void
+simplefb_earlycon_write(struct console *con, const char *str, unsigned int num)
+{
+	unsigned int len;
+	const char *s;
+	void *dst;
+
+	len = info.stride;
+
+	while (num) {
+		unsigned int linemax;
+		unsigned int h, count = 0;
+
+		for (s = str; *s && *s != '\n'; s++) {
+			if (count == num)
+				break;
+			count++;
+		}
+
+		linemax = (info.x - info.curr_x) / font->width;
+		if (count > linemax)
+			count = linemax;
+
+		for (h = 0; h < font->height; h++) {
+			unsigned int n, x;
+
+			dst = simplefb_earlycon_map((info.curr_y + h) * len, len);
+			if (!dst)
+				return;
+
+			s = str;
+			n = count;
+			x = info.curr_x;
+
+			while (n-- > 0) {
+				simplefb_earlycon_write_char(dst + (x * 4), *s, h);
+				x += font->width;
+				s++;
+			}
+
+			simplefb_earlycon_unmap(dst, len);
+		}
+
+		num -= count;
+		info.curr_x += count * font->width;
+		str += count;
+
+		if (num > 0 && *s == '\n') {
+			info.curr_x = 0;
+			info.curr_y += font->height;
+			str++;
+			num--;
+		}
+
+		if (info.curr_x + font->width > info.x) {
+			info.curr_x = 0;
+			info.curr_y += font->height;
+		}
+
+		if (info.curr_y + font->height > info.y) {
+			u32 i;
+
+			info.curr_y -= font->height;
+			simplefb_earlycon_scroll_up();
+
+			for (i = 0; i < font->height; i++)
+				simplefb_earlycon_clear_scanline(info.curr_y + i);
+		}
+	}
+}
+
+static int __init simplefb_earlycon_setup_common(struct earlycon_device *device,
+						 const char *opt)
+{
+	int i;
+
+	info.stride = info.x * 4;
+	info.size = info.x * info.y * (info.depth / 8);
+
+	font = get_default_font(info.x, info.y, -1, -1);
+	if (!font)
+		return -ENODEV;
+
+	info.curr_y = rounddown(info.y, font->height) - font->height;
+	for (i = 0; i < (info.y - info.curr_y) / font->height; i++)
+		simplefb_earlycon_scroll_up();
+
+	device->con->write = simplefb_earlycon_write;
+	earlycon_console = device->con;
+	return 0;
+}
+
+static int __init simplefb_earlycon_setup(struct earlycon_device *device,
+					  const char *opt)
+{
+	struct uart_port *port = &device->port;
+	int ret;
+
+	if (!port->mapbase)
+		return -ENODEV;
+
+	info.phys_base = port->mapbase;
+
+	ret = sscanf(device->options, "%u,%u,%u", &info.x, &info.y, &info.depth);
+	if (ret != 3)
+		return -ENODEV;
+
+	return simplefb_earlycon_setup_common(device, opt);
+}
+
+EARLYCON_DECLARE(simplefb, simplefb_earlycon_setup);
+
+#ifdef CONFIG_EFI_EARLYCON
+static int __init simplefb_earlycon_setup_efi(struct earlycon_device *device,
+					      const char *opt)
+{
+	if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI)
+		return -ENODEV;
+
+	info.phys_base = screen_info.lfb_base;
+	if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
+		info.phys_base |= (u64)screen_info.ext_lfb_base << 32;
+
+	info.x = screen_info.lfb_width;
+	info.y = screen_info.lfb_height;
+	info.depth = screen_info.lfb_depth;
+
+	return simplefb_earlycon_setup_common(device, opt);
+}
+
+EARLYCON_DECLARE(efifb, simplefb_earlycon_setup_efi);
+#endif
+
+#ifdef CONFIG_OF_EARLY_FLATTREE
+static int __init simplefb_earlycon_setup_of(struct earlycon_device *device,
+					     const char *opt)
+{
+	struct uart_port *port = &device->port;
+	const __be32 *val;
+
+	if (!port->mapbase)
+		return -ENODEV;
+
+	info.phys_base = port->mapbase;
+
+	val = of_get_flat_dt_prop(device->node, "width", NULL);
+	if (!val)
+		return -ENODEV;
+	info.x = be32_to_cpu(*val);
+
+	val = of_get_flat_dt_prop(device->node, "height", NULL);
+	if (!val)
+		return -ENODEV;
+	info.y = be32_to_cpu(*val);
+
+	val = of_get_flat_dt_prop(device->node, "stride", NULL);
+	if (!val)
+		return -ENODEV;
+	info.depth = (be32_to_cpu(*val) / info.x) * 8;
+
+	return simplefb_earlycon_setup_common(device, opt);
+}
+
+OF_EARLYCON_DECLARE(simplefb, "simple-framebuffer", simplefb_earlycon_setup_of);
+#endif
-- 
2.37.0

[PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Markuss Broks]

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.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 .../admin-guide/kernel-parameters.txt         |  12 +-
 MAINTAINERS                                   |   5 +
 drivers/firmware/efi/Kconfig                  |   6 +-
 drivers/firmware/efi/Makefile                 |   1 -
 drivers/firmware/efi/earlycon.c               | 246 --------------
 drivers/video/fbdev/Kconfig                   |  11 +
 drivers/video/fbdev/Makefile                  |   1 +
 drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
 8 files changed, 327 insertions(+), 256 deletions(-)
 delete mode 100644 drivers/firmware/efi/earlycon.c
 create mode 100644 drivers/video/fbdev/earlycon.c

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 8090130b544b0701237a7b657a29c83c000a60f4..22a8625bb342f95e731fcc3a24390fca2c0f194c 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1281,12 +1281,9 @@
 			specified address. The serial port must already be
 			setup and configured. Options are not yet supported.
 
-		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).
 
 		linflex,<addr>
 			Use early console provided by Freescale LINFlexD UART
@@ -1294,6 +1291,11 @@
 			address must be provided, and the serial port must
 			already be setup and configured.
 
+		simplefb,<addr>,<width>,<height>,<bpp>
+			Use early console with simple framebuffer that is
+			pre-initialized by firmware. A valid base address,
+			width, height and pixel size must be provided.
+
 	earlyprintk=	[X86,SH,ARM,M68k,S390]
 			earlyprintk=vga
 			earlyprintk=sclp
diff --git a/MAINTAINERS b/MAINTAINERS
index 1fc9ead83d2aa3e60ccc4cfa8ee16df09ef579bf..140dee7f6920197a895d310afdde67a0de474522 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7033,6 +7033,11 @@ Q:	http://patchwork.linuxtv.org/project/linux-media/list/
 T:	git git://linuxtv.org/anttip/media_tree.git
 F:	drivers/media/tuners/e4000*
 
+EARLY CONSOLE FRAMEBUFFER DRIVER
+M:	Markuss Broks <markuss.broks@gmail.com>
+S:	Maintained
+F:	drivers/video/fbdev/earlycon.c
+
 EARTH_PT1 MEDIA DRIVER
 M:	Akihiro Tsukada <tskd08@gmail.com>
 L:	linux-media@vger.kernel.org
diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index 7aa4717cdcac46f91dd202f868c463388eb02735..56241b2e19383d70387f07114a27bb4f33ac7857 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -259,10 +259,8 @@ config EFI_DISABLE_PCI_DMA
 	  may be used to override this option.
 
 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
 
 config EFI_CUSTOM_SSDT_OVERLAYS
 	bool "Load custom ACPI SSDT overlay from an EFI variable"
diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index c02ff25dd47707090a2ab86ee4f330e467f878f5..64eea61fbb43d76ec2d5416d467dfbb9aa21bda0 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -44,6 +44,5 @@ obj-$(CONFIG_ARM64)			+= $(arm-obj-y)
 riscv-obj-$(CONFIG_EFI)			:= efi-init.o riscv-runtime.o
 obj-$(CONFIG_RISCV)			+= $(riscv-obj-y)
 obj-$(CONFIG_EFI_CAPSULE_LOADER)	+= capsule-loader.o
-obj-$(CONFIG_EFI_EARLYCON)		+= earlycon.o
 obj-$(CONFIG_UEFI_CPER_ARM)		+= cper-arm.o
 obj-$(CONFIG_UEFI_CPER_X86)		+= cper-x86.o
diff --git a/drivers/firmware/efi/earlycon.c b/drivers/firmware/efi/earlycon.c
deleted file mode 100644
index a52236e11e5f73ddea5bb1f42ca2ca7c42425dab..0000000000000000000000000000000000000000
--- a/drivers/firmware/efi/earlycon.c
+++ /dev/null
@@ -1,246 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Copyright (C) 2013 Intel Corporation; author Matt Fleming
- */
-
-#include <linux/console.h>
-#include <linux/efi.h>
-#include <linux/font.h>
-#include <linux/io.h>
-#include <linux/kernel.h>
-#include <linux/serial_core.h>
-#include <linux/screen_info.h>
-
-#include <asm/early_ioremap.h>
-
-static const struct console *earlycon_console __initdata;
-static const struct font_desc *font;
-static u32 efi_x, efi_y;
-static u64 fb_base;
-static bool fb_wb;
-static void *efi_fb;
-
-/*
- * EFI earlycon needs to use early_memremap() to map the framebuffer.
- * But early_memremap() is not usable for 'earlycon=efifb keep_bootcon',
- * memremap() should be used instead. memremap() will be available after
- * paging_init() which is earlier than initcall callbacks. Thus adding this
- * early initcall function early_efi_map_fb() to map the whole EFI framebuffer.
- */
-static int __init efi_earlycon_remap_fb(void)
-{
-	/* bail if there is no bootconsole or it has been disabled already */
-	if (!earlycon_console || !(earlycon_console->flags & CON_ENABLED))
-		return 0;
-
-	efi_fb = memremap(fb_base, screen_info.lfb_size,
-			  fb_wb ? MEMREMAP_WB : MEMREMAP_WC);
-
-	return efi_fb ? 0 : -ENOMEM;
-}
-early_initcall(efi_earlycon_remap_fb);
-
-static int __init efi_earlycon_unmap_fb(void)
-{
-	/* unmap the bootconsole fb unless keep_bootcon has left it enabled */
-	if (efi_fb && !(earlycon_console->flags & CON_ENABLED))
-		memunmap(efi_fb);
-	return 0;
-}
-late_initcall(efi_earlycon_unmap_fb);
-
-static __ref void *efi_earlycon_map(unsigned long start, unsigned long len)
-{
-	pgprot_t fb_prot;
-
-	if (efi_fb)
-		return efi_fb + start;
-
-	fb_prot = fb_wb ? PAGE_KERNEL : pgprot_writecombine(PAGE_KERNEL);
-	return early_memremap_prot(fb_base + start, len, pgprot_val(fb_prot));
-}
-
-static __ref void efi_earlycon_unmap(void *addr, unsigned long len)
-{
-	if (efi_fb)
-		return;
-
-	early_memunmap(addr, len);
-}
-
-static void efi_earlycon_clear_scanline(unsigned int y)
-{
-	unsigned long *dst;
-	u16 len;
-
-	len = screen_info.lfb_linelength;
-	dst = efi_earlycon_map(y*len, len);
-	if (!dst)
-		return;
-
-	memset(dst, 0, len);
-	efi_earlycon_unmap(dst, len);
-}
-
-static void efi_earlycon_scroll_up(void)
-{
-	unsigned long *dst, *src;
-	u16 len;
-	u32 i, height;
-
-	len = screen_info.lfb_linelength;
-	height = screen_info.lfb_height;
-
-	for (i = 0; i < height - font->height; i++) {
-		dst = efi_earlycon_map(i*len, len);
-		if (!dst)
-			return;
-
-		src = efi_earlycon_map((i + font->height) * len, len);
-		if (!src) {
-			efi_earlycon_unmap(dst, len);
-			return;
-		}
-
-		memmove(dst, src, len);
-
-		efi_earlycon_unmap(src, len);
-		efi_earlycon_unmap(dst, len);
-	}
-}
-
-static void efi_earlycon_write_char(u32 *dst, unsigned char c, unsigned int h)
-{
-	const u32 color_black = 0x00000000;
-	const u32 color_white = 0x00ffffff;
-	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)
-			*dst = color_white;
-		else
-			*dst = color_black;
-		dst++;
-	}
-}
-
-static void
-efi_earlycon_write(struct console *con, const char *str, unsigned int num)
-{
-	struct screen_info *si;
-	unsigned int len;
-	const char *s;
-	void *dst;
-
-	si = &screen_info;
-	len = si->lfb_linelength;
-
-	while (num) {
-		unsigned int linemax;
-		unsigned int h, count = 0;
-
-		for (s = str; *s && *s != '\n'; s++) {
-			if (count == num)
-				break;
-			count++;
-		}
-
-		linemax = (si->lfb_width - efi_x) / font->width;
-		if (count > linemax)
-			count = linemax;
-
-		for (h = 0; h < font->height; h++) {
-			unsigned int n, x;
-
-			dst = efi_earlycon_map((efi_y + h) * len, len);
-			if (!dst)
-				return;
-
-			s = str;
-			n = count;
-			x = efi_x;
-
-			while (n-- > 0) {
-				efi_earlycon_write_char(dst + x*4, *s, h);
-				x += font->width;
-				s++;
-			}
-
-			efi_earlycon_unmap(dst, len);
-		}
-
-		num -= count;
-		efi_x += count * font->width;
-		str += count;
-
-		if (num > 0 && *s == '\n') {
-			efi_x = 0;
-			efi_y += font->height;
-			str++;
-			num--;
-		}
-
-		if (efi_x + font->width > si->lfb_width) {
-			efi_x = 0;
-			efi_y += font->height;
-		}
-
-		if (efi_y + font->height > si->lfb_height) {
-			u32 i;
-
-			efi_y -= font->height;
-			efi_earlycon_scroll_up();
-
-			for (i = 0; i < font->height; i++)
-				efi_earlycon_clear_scanline(efi_y + i);
-		}
-	}
-}
-
-static int __init efi_earlycon_setup(struct earlycon_device *device,
-				     const char *opt)
-{
-	struct screen_info *si;
-	u16 xres, yres;
-	u32 i;
-
-	if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI)
-		return -ENODEV;
-
-	fb_base = screen_info.lfb_base;
-	if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
-		fb_base |= (u64)screen_info.ext_lfb_base << 32;
-
-	fb_wb = opt && !strcmp(opt, "ram");
-
-	si = &screen_info;
-	xres = si->lfb_width;
-	yres = si->lfb_height;
-
-	/*
-	 * efi_earlycon_write_char() implicitly assumes a framebuffer with
-	 * 32 bits per pixel.
-	 */
-	if (si->lfb_depth != 32)
-		return -ENODEV;
-
-	font = get_default_font(xres, yres, -1, -1);
-	if (!font)
-		return -ENODEV;
-
-	efi_y = rounddown(yres, font->height) - font->height;
-	for (i = 0; i < (yres - efi_y) / font->height; i++)
-		efi_earlycon_scroll_up();
-
-	device->con->write = efi_earlycon_write;
-	earlycon_console = device->con;
-	return 0;
-}
-EARLYCON_DECLARE(efifb, efi_earlycon_setup);
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index f2a6b81e45c41334db71cca35f4fa5a827f728fe..2bb3f7a9e6e0f8ad2e9042d8c602b1cdec30755c 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -627,6 +627,17 @@ config FB_VESA
 	  You will get a boot time penguin logo at no additional cost. Please
 	  read <file:Documentation/fb/vesafb.rst>. If unsure, say Y.
 
+config FB_EARLYCON
+	bool "Generic framebuffer early console"
+	depends on FB && SERIAL_EARLYCON && !ARM
+	select FONT_SUPPORT
+	select ARCH_USE_MEMREMAP_PROT
+	help
+	  Say Y here if you want early console support for firmware established
+	  linear framebuffer. Unless you are using EFI framebuffer, you need to
+	  specify framebuffer geometry and address in device-tree or in kernel
+	  command line.
+
 config FB_EFI
 	bool "EFI-based Framebuffer Support"
 	depends on (FB = y) && !IA64 && EFI
diff --git a/drivers/video/fbdev/Makefile b/drivers/video/fbdev/Makefile
index 7795c4126706fd7bb832d94da5d14b0060849d1f..07406ab45a3f306a07d008a9f0a401347aeaa695 100644
--- a/drivers/video/fbdev/Makefile
+++ b/drivers/video/fbdev/Makefile
@@ -129,6 +129,7 @@ obj-$(CONFIG_FB_MX3)		  += mx3fb.o
 obj-$(CONFIG_FB_DA8XX)		  += da8xx-fb.o
 obj-$(CONFIG_FB_SSD1307)	  += ssd1307fb.o
 obj-$(CONFIG_FB_SIMPLE)           += simplefb.o
+obj-$(CONFIG_FB_EARLYCON)         += earlycon.o
 
 # the test framebuffer is last
 obj-$(CONFIG_FB_VIRTUAL)          += vfb.o
diff --git a/drivers/video/fbdev/earlycon.c b/drivers/video/fbdev/earlycon.c
new file mode 100644
index 0000000000000000000000000000000000000000..5f6f4b228a50f23c1c77cf7ba5d50ff833efdd46
--- /dev/null
+++ b/drivers/video/fbdev/earlycon.c
@@ -0,0 +1,301 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2013 Intel Corporation; author Matt Fleming
+ * Copyright (C) 2022 Markuss Broks <markuss.broks@gmail.com>
+ */
+
+#include <asm/early_ioremap.h>
+#include <linux/console.h>
+#include <linux/efi.h>
+#include <linux/font.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+#include <linux/serial_core.h>
+#include <linux/screen_info.h>
+
+struct fb_earlycon {
+	u32 x, y, curr_x, curr_y, depth, stride;
+	size_t size;
+	phys_addr_t phys_base;
+	void __iomem *virt_base;
+};
+
+static const struct console *earlycon_console __initconst;
+static struct fb_earlycon info;
+static const struct font_desc *font;
+
+static int __init simplefb_earlycon_remap_fb(void)
+{
+	int is_ram;
+	/* 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;
+
+	info.virt_base = memremap(info.phys_base, info.size,
+				  is_ram ? MEMREMAP_WB : MEMREMAP_WC);
+
+	return info.virt_base ? 0 : -ENOMEM;
+}
+early_initcall(simplefb_earlycon_remap_fb);
+
+static int __init simplefb_earlycon_unmap_fb(void)
+{
+	/* unmap the bootconsole fb unless keep_bootcon has left it enabled */
+	if (info.virt_base && !(earlycon_console->flags & CON_ENABLED))
+		memunmap(info.virt_base);
+	return 0;
+}
+late_initcall(simplefb_earlycon_unmap_fb);
+
+static __ref void *simplefb_earlycon_map(unsigned long start, unsigned long len)
+{
+	pgprot_t fb_prot;
+
+	if (info.virt_base)
+		return info.virt_base + start;
+
+	fb_prot = PAGE_KERNEL;
+	return early_memremap_prot(info.phys_base + start, len, pgprot_val(fb_prot));
+}
+
+static __ref void simplefb_earlycon_unmap(void *addr, unsigned long len)
+{
+	if (info.virt_base)
+		return;
+
+	early_memunmap(addr, len);
+}
+
+static void simplefb_earlycon_clear_scanline(unsigned int y)
+{
+	unsigned long *dst;
+	u16 len;
+
+	len = info.stride;
+	dst = simplefb_earlycon_map(y * len, len);
+	if (!dst)
+		return;
+
+	memset(dst, 0, len);
+	simplefb_earlycon_unmap(dst, len);
+}
+
+static void simplefb_earlycon_scroll_up(void)
+{
+	unsigned long *dst, *src;
+	u16 len;
+	u32 i, height;
+
+	len = info.stride;
+	height = info.y;
+
+	for (i = 0; i < height - font->height; i++) {
+		dst = simplefb_earlycon_map(i * len, len);
+		if (!dst)
+			return;
+
+		src = simplefb_earlycon_map((i + font->height) * len, len);
+		if (!src) {
+			simplefb_earlycon_unmap(dst, len);
+			return;
+		}
+
+		memmove(dst, src, len);
+
+		simplefb_earlycon_unmap(src, len);
+		simplefb_earlycon_unmap(dst, len);
+	}
+}
+
+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);
+	}
+}
+
+static void
+simplefb_earlycon_write(struct console *con, const char *str, unsigned int num)
+{
+	unsigned int len;
+	const char *s;
+	void *dst;
+
+	len = info.stride;
+
+	while (num) {
+		unsigned int linemax;
+		unsigned int h, count = 0;
+
+		for (s = str; *s && *s != '\n'; s++) {
+			if (count == num)
+				break;
+			count++;
+		}
+
+		linemax = (info.x - info.curr_x) / font->width;
+		if (count > linemax)
+			count = linemax;
+
+		for (h = 0; h < font->height; h++) {
+			unsigned int n, x;
+
+			dst = simplefb_earlycon_map((info.curr_y + h) * len, len);
+			if (!dst)
+				return;
+
+			s = str;
+			n = count;
+			x = info.curr_x;
+
+			while (n-- > 0) {
+				simplefb_earlycon_write_char(dst + (x * 4), *s, h);
+				x += font->width;
+				s++;
+			}
+
+			simplefb_earlycon_unmap(dst, len);
+		}
+
+		num -= count;
+		info.curr_x += count * font->width;
+		str += count;
+
+		if (num > 0 && *s == '\n') {
+			info.curr_x = 0;
+			info.curr_y += font->height;
+			str++;
+			num--;
+		}
+
+		if (info.curr_x + font->width > info.x) {
+			info.curr_x = 0;
+			info.curr_y += font->height;
+		}
+
+		if (info.curr_y + font->height > info.y) {
+			u32 i;
+
+			info.curr_y -= font->height;
+			simplefb_earlycon_scroll_up();
+
+			for (i = 0; i < font->height; i++)
+				simplefb_earlycon_clear_scanline(info.curr_y + i);
+		}
+	}
+}
+
+static int __init simplefb_earlycon_setup_common(struct earlycon_device *device,
+						 const char *opt)
+{
+	int i;
+
+	info.stride = info.x * 4;
+	info.size = info.x * info.y * (info.depth / 8);
+
+	font = get_default_font(info.x, info.y, -1, -1);
+	if (!font)
+		return -ENODEV;
+
+	info.curr_y = rounddown(info.y, font->height) - font->height;
+	for (i = 0; i < (info.y - info.curr_y) / font->height; i++)
+		simplefb_earlycon_scroll_up();
+
+	device->con->write = simplefb_earlycon_write;
+	earlycon_console = device->con;
+	return 0;
+}
+
+static int __init simplefb_earlycon_setup(struct earlycon_device *device,
+					  const char *opt)
+{
+	struct uart_port *port = &device->port;
+	int ret;
+
+	if (!port->mapbase)
+		return -ENODEV;
+
+	info.phys_base = port->mapbase;
+
+	ret = sscanf(device->options, "%u,%u,%u", &info.x, &info.y, &info.depth);
+	if (ret != 3)
+		return -ENODEV;
+
+	return simplefb_earlycon_setup_common(device, opt);
+}
+
+EARLYCON_DECLARE(simplefb, simplefb_earlycon_setup);
+
+#ifdef CONFIG_EFI_EARLYCON
+static int __init simplefb_earlycon_setup_efi(struct earlycon_device *device,
+					      const char *opt)
+{
+	if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI)
+		return -ENODEV;
+
+	info.phys_base = screen_info.lfb_base;
+	if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
+		info.phys_base |= (u64)screen_info.ext_lfb_base << 32;
+
+	info.x = screen_info.lfb_width;
+	info.y = screen_info.lfb_height;
+	info.depth = screen_info.lfb_depth;
+
+	return simplefb_earlycon_setup_common(device, opt);
+}
+
+EARLYCON_DECLARE(efifb, simplefb_earlycon_setup_efi);
+#endif
+
+#ifdef CONFIG_OF_EARLY_FLATTREE
+static int __init simplefb_earlycon_setup_of(struct earlycon_device *device,
+					     const char *opt)
+{
+	struct uart_port *port = &device->port;
+	const __be32 *val;
+
+	if (!port->mapbase)
+		return -ENODEV;
+
+	info.phys_base = port->mapbase;
+
+	val = of_get_flat_dt_prop(device->node, "width", NULL);
+	if (!val)
+		return -ENODEV;
+	info.x = be32_to_cpu(*val);
+
+	val = of_get_flat_dt_prop(device->node, "height", NULL);
+	if (!val)
+		return -ENODEV;
+	info.y = be32_to_cpu(*val);
+
+	val = of_get_flat_dt_prop(device->node, "stride", NULL);
+	if (!val)
+		return -ENODEV;
+	info.depth = (be32_to_cpu(*val) / info.x) * 8;
+
+	return simplefb_earlycon_setup_common(device, opt);
+}
+
+OF_EARLYCON_DECLARE(simplefb, "simple-framebuffer", simplefb_earlycon_setup_of);
+#endif
-- 
2.37.0

Re: [PATCH 1/2] drivers: serial: earlycon: Pass device-tree node

[Greg Kroah-Hartman]

On Thu, Jul 28, 2022 at 05:28:18PM +0300, Markuss Broks wrote:
> Pass a pointer to device-tree node in case the driver probed from
> OF. This makes early console drivers able to fetch options from
> device-tree node properties.
> 
> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
> ---
>  drivers/tty/serial/earlycon.c | 3 +++
>  include/linux/serial_core.h   | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index 57c70851f22a0e78805f34d1a7700708104b6f6a..14e8a7fe54486a1c377a6659c37a73858de5bf0b 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -304,6 +304,9 @@ int __init of_setup_earlycon(const struct earlycon_id *match,
>  		strlcpy(early_console_dev.options, options,
>  			sizeof(early_console_dev.options));
>  	}
> +
> +	early_console_dev.node = node;
> +
>  	earlycon_init(&early_console_dev, match->name);
>  	err = match->setup(&early_console_dev, options);
>  	earlycon_print_info(&early_console_dev);
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index cbd5070bc87f42aa450c4ca7af8a9b59fbe88574..3295721f33e482124fae8370b5889d5d6c012303 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -349,6 +349,7 @@ struct earlycon_device {
>  	struct uart_port port;
>  	char options[16];		/* e.g., 115200n8 */
>  	unsigned int baud;
> +	unsigned long node;

That should not be an unsigned long, but rather an 'int'.  Something got
messed up, of_setup_earlycon() should be changed to reflect this before
propagating the error to other places in the kernel.

And it's not really a "node" but an "offset", right?

thanks,

greg k-h

Re: [PATCH 1/2] drivers: serial: earlycon: Pass device-tree node

[Greg Kroah-Hartman]

On Thu, Jul 28, 2022 at 05:28:18PM +0300, Markuss Broks wrote:
> Pass a pointer to device-tree node in case the driver probed from
> OF. This makes early console drivers able to fetch options from
> device-tree node properties.
> 
> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
> ---
>  drivers/tty/serial/earlycon.c | 3 +++
>  include/linux/serial_core.h   | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index 57c70851f22a0e78805f34d1a7700708104b6f6a..14e8a7fe54486a1c377a6659c37a73858de5bf0b 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -304,6 +304,9 @@ int __init of_setup_earlycon(const struct earlycon_id *match,
>  		strlcpy(early_console_dev.options, options,
>  			sizeof(early_console_dev.options));
>  	}
> +
> +	early_console_dev.node = node;
> +
>  	earlycon_init(&early_console_dev, match->name);
>  	err = match->setup(&early_console_dev, options);
>  	earlycon_print_info(&early_console_dev);
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index cbd5070bc87f42aa450c4ca7af8a9b59fbe88574..3295721f33e482124fae8370b5889d5d6c012303 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -349,6 +349,7 @@ struct earlycon_device {
>  	struct uart_port port;
>  	char options[16];		/* e.g., 115200n8 */
>  	unsigned int baud;
> +	unsigned long node;

That should not be an unsigned long, but rather an 'int'.  Something got
messed up, of_setup_earlycon() should be changed to reflect this before
propagating the error to other places in the kernel.

And it's not really a "node" but an "offset", right?

thanks,

greg k-h

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Greg Kroah-Hartman]

On Thu, Jul 28, 2022 at 05:28:19PM +0300, Markuss Broks 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.
> 
> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
> ---
>  .../admin-guide/kernel-parameters.txt         |  12 +-
>  MAINTAINERS                                   |   5 +
>  drivers/firmware/efi/Kconfig                  |   6 +-
>  drivers/firmware/efi/Makefile                 |   1 -
>  drivers/firmware/efi/earlycon.c               | 246 --------------
>  drivers/video/fbdev/Kconfig                   |  11 +
>  drivers/video/fbdev/Makefile                  |   1 +
>  drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
>  8 files changed, 327 insertions(+), 256 deletions(-)
>  delete mode 100644 drivers/firmware/efi/earlycon.c
>  create mode 100644 drivers/video/fbdev/earlycon.c

That should be a rename, not a delete/create, right?

thanks,

greg k-h

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Greg Kroah-Hartman]

On Thu, Jul 28, 2022 at 05:28:19PM +0300, Markuss Broks 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.
> 
> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
> ---
>  .../admin-guide/kernel-parameters.txt         |  12 +-
>  MAINTAINERS                                   |   5 +
>  drivers/firmware/efi/Kconfig                  |   6 +-
>  drivers/firmware/efi/Makefile                 |   1 -
>  drivers/firmware/efi/earlycon.c               | 246 --------------
>  drivers/video/fbdev/Kconfig                   |  11 +
>  drivers/video/fbdev/Makefile                  |   1 +
>  drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
>  8 files changed, 327 insertions(+), 256 deletions(-)
>  delete mode 100644 drivers/firmware/efi/earlycon.c
>  create mode 100644 drivers/video/fbdev/earlycon.c

That should be a rename, not a delete/create, right?

thanks,

greg k-h

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Arnd Bergmann]

On Thu, Jul 28, 2022 at 4:28 PM Markuss Broks <markuss.broks@gmail.com> 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.
>
> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
> ---
>  .../admin-guide/kernel-parameters.txt         |  12 +-
>  MAINTAINERS                                   |   5 +
>  drivers/firmware/efi/Kconfig                  |   6 +-
>  drivers/firmware/efi/Makefile                 |   1 -
>  drivers/firmware/efi/earlycon.c               | 246 --------------
>  drivers/video/fbdev/Kconfig                   |  11 +
>  drivers/video/fbdev/Makefile                  |   1 +
>  drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++

It looks like this is not actually related to fbdev, and since drivers are
moving from fbdev/simplefb towards drm/simpledrm, maybe it would be
better to put this into either drivers/gpu/drm/tiny/ or possibly
drivers/video/console to let this be used without enabling fbdev?

        Arnd

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Arnd Bergmann]

On Thu, Jul 28, 2022 at 4:28 PM Markuss Broks <markuss.broks@gmail.com> 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.
>
> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
> ---
>  .../admin-guide/kernel-parameters.txt         |  12 +-
>  MAINTAINERS                                   |   5 +
>  drivers/firmware/efi/Kconfig                  |   6 +-
>  drivers/firmware/efi/Makefile                 |   1 -
>  drivers/firmware/efi/earlycon.c               | 246 --------------
>  drivers/video/fbdev/Kconfig                   |  11 +
>  drivers/video/fbdev/Makefile                  |   1 +
>  drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++

It looks like this is not actually related to fbdev, and since drivers are
moving from fbdev/simplefb towards drm/simpledrm, maybe it would be
better to put this into either drivers/gpu/drm/tiny/ or possibly
drivers/video/console to let this be used without enabling fbdev?

        Arnd

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Markuss Broks]

Hi Greg,

On 7/28/22 17:39, Greg Kroah-Hartman wrote:
> On Thu, Jul 28, 2022 at 05:28:19PM +0300, Markuss Broks 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.
>>
>> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
>> ---
>>   .../admin-guide/kernel-parameters.txt         |  12 +-
>>   MAINTAINERS                                   |   5 +
>>   drivers/firmware/efi/Kconfig                  |   6 +-
>>   drivers/firmware/efi/Makefile                 |   1 -
>>   drivers/firmware/efi/earlycon.c               | 246 --------------
>>   drivers/video/fbdev/Kconfig                   |  11 +
>>   drivers/video/fbdev/Makefile                  |   1 +
>>   drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
>>   8 files changed, 327 insertions(+), 256 deletions(-)
>>   delete mode 100644 drivers/firmware/efi/earlycon.c
>>   create mode 100644 drivers/video/fbdev/earlycon.c
> 
> That should be a rename, not a delete/create, right?

Should this change be split into two separate commits,
one for moving the file and the second for making changes?

> 
> thanks,
> 
> greg k-h

- Markuss

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Markuss Broks]

Hi Greg,

On 7/28/22 17:39, Greg Kroah-Hartman wrote:
> On Thu, Jul 28, 2022 at 05:28:19PM +0300, Markuss Broks 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.
>>
>> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
>> ---
>>   .../admin-guide/kernel-parameters.txt         |  12 +-
>>   MAINTAINERS                                   |   5 +
>>   drivers/firmware/efi/Kconfig                  |   6 +-
>>   drivers/firmware/efi/Makefile                 |   1 -
>>   drivers/firmware/efi/earlycon.c               | 246 --------------
>>   drivers/video/fbdev/Kconfig                   |  11 +
>>   drivers/video/fbdev/Makefile                  |   1 +
>>   drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
>>   8 files changed, 327 insertions(+), 256 deletions(-)
>>   delete mode 100644 drivers/firmware/efi/earlycon.c
>>   create mode 100644 drivers/video/fbdev/earlycon.c
> 
> That should be a rename, not a delete/create, right?

Should this change be split into two separate commits,
one for moving the file and the second for making changes?

> 
> thanks,
> 
> greg k-h

- Markuss

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Markuss Broks]

Hi Arnd,

On 7/28/22 17:48, Arnd Bergmann wrote:
> On Thu, Jul 28, 2022 at 4:28 PM Markuss Broks <markuss.broks@gmail.com> 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.
>>
>> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
>> ---
>>   .../admin-guide/kernel-parameters.txt         |  12 +-
>>   MAINTAINERS                                   |   5 +
>>   drivers/firmware/efi/Kconfig                  |   6 +-
>>   drivers/firmware/efi/Makefile                 |   1 -
>>   drivers/firmware/efi/earlycon.c               | 246 --------------
>>   drivers/video/fbdev/Kconfig                   |  11 +
>>   drivers/video/fbdev/Makefile                  |   1 +
>>   drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
> 
> It looks like this is not actually related to fbdev, and since drivers are
> moving from fbdev/simplefb towards drm/simpledrm, maybe it would be
> better to put this into either drivers/gpu/drm/tiny/ or possibly
> drivers/video/console to let this be used without enabling fbdev?

Ideally this shouldn't depend on anything, because it isn't utilizing 
any of fbdev code and won't be utilizing any of drm/console code. I 
agree that either of those would be a better place for it, but which one 
do you think would suit more for this driver?

> 
>          Arnd

- Markuss

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Markuss Broks]

Hi Arnd,

On 7/28/22 17:48, Arnd Bergmann wrote:
> On Thu, Jul 28, 2022 at 4:28 PM Markuss Broks <markuss.broks@gmail.com> 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.
>>
>> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
>> ---
>>   .../admin-guide/kernel-parameters.txt         |  12 +-
>>   MAINTAINERS                                   |   5 +
>>   drivers/firmware/efi/Kconfig                  |   6 +-
>>   drivers/firmware/efi/Makefile                 |   1 -
>>   drivers/firmware/efi/earlycon.c               | 246 --------------
>>   drivers/video/fbdev/Kconfig                   |  11 +
>>   drivers/video/fbdev/Makefile                  |   1 +
>>   drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
> 
> It looks like this is not actually related to fbdev, and since drivers are
> moving from fbdev/simplefb towards drm/simpledrm, maybe it would be
> better to put this into either drivers/gpu/drm/tiny/ or possibly
> drivers/video/console to let this be used without enabling fbdev?

Ideally this shouldn't depend on anything, because it isn't utilizing 
any of fbdev code and won't be utilizing any of drm/console code. I 
agree that either of those would be a better place for it, but which one 
do you think would suit more for this driver?

> 
>          Arnd

- Markuss

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Greg Kroah-Hartman]

On Thu, Jul 28, 2022 at 05:52:04PM +0300, Markuss Broks wrote:
> Hi Greg,
> 
> On 7/28/22 17:39, Greg Kroah-Hartman wrote:
> > On Thu, Jul 28, 2022 at 05:28:19PM +0300, Markuss Broks 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.
> > > 
> > > Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
> > > ---
> > >   .../admin-guide/kernel-parameters.txt         |  12 +-
> > >   MAINTAINERS                                   |   5 +
> > >   drivers/firmware/efi/Kconfig                  |   6 +-
> > >   drivers/firmware/efi/Makefile                 |   1 -
> > >   drivers/firmware/efi/earlycon.c               | 246 --------------
> > >   drivers/video/fbdev/Kconfig                   |  11 +
> > >   drivers/video/fbdev/Makefile                  |   1 +
> > >   drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
> > >   8 files changed, 327 insertions(+), 256 deletions(-)
> > >   delete mode 100644 drivers/firmware/efi/earlycon.c
> > >   create mode 100644 drivers/video/fbdev/earlycon.c
> > 
> > That should be a rename, not a delete/create, right?
> 
> Should this change be split into two separate commits,
> one for moving the file and the second for making changes?

Git will show a rename and modification properly, if you use -M to git
format-patch, so it should be fine.

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Greg Kroah-Hartman]

On Thu, Jul 28, 2022 at 05:52:04PM +0300, Markuss Broks wrote:
> Hi Greg,
> 
> On 7/28/22 17:39, Greg Kroah-Hartman wrote:
> > On Thu, Jul 28, 2022 at 05:28:19PM +0300, Markuss Broks 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.
> > > 
> > > Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
> > > ---
> > >   .../admin-guide/kernel-parameters.txt         |  12 +-
> > >   MAINTAINERS                                   |   5 +
> > >   drivers/firmware/efi/Kconfig                  |   6 +-
> > >   drivers/firmware/efi/Makefile                 |   1 -
> > >   drivers/firmware/efi/earlycon.c               | 246 --------------
> > >   drivers/video/fbdev/Kconfig                   |  11 +
> > >   drivers/video/fbdev/Makefile                  |   1 +
> > >   drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
> > >   8 files changed, 327 insertions(+), 256 deletions(-)
> > >   delete mode 100644 drivers/firmware/efi/earlycon.c
> > >   create mode 100644 drivers/video/fbdev/earlycon.c
> > 
> > That should be a rename, not a delete/create, right?
> 
> Should this change be split into two separate commits,
> one for moving the file and the second for making changes?

Git will show a rename and modification properly, if you use -M to git
format-patch, so it should be fine.

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Arnd Bergmann]

On Thu, Jul 28, 2022 at 4:57 PM Markuss Broks <markuss.broks@gmail.com> wrote:
> On 7/28/22 17:48, Arnd Bergmann wrote:
> > On Thu, Jul 28, 2022 at 4:28 PM Markuss Broks <markuss.broks@gmail.com> 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.
> >>
> >> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
> >> ---
> >>   .../admin-guide/kernel-parameters.txt         |  12 +-
> >>   MAINTAINERS                                   |   5 +
> >>   drivers/firmware/efi/Kconfig                  |   6 +-
> >>   drivers/firmware/efi/Makefile                 |   1 -
> >>   drivers/firmware/efi/earlycon.c               | 246 --------------
> >>   drivers/video/fbdev/Kconfig                   |  11 +
> >>   drivers/video/fbdev/Makefile                  |   1 +
> >>   drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
> >
> > It looks like this is not actually related to fbdev, and since drivers are
> > moving from fbdev/simplefb towards drm/simpledrm, maybe it would be
> > better to put this into either drivers/gpu/drm/tiny/ or possibly
> > drivers/video/console to let this be used without enabling fbdev?
>
> Ideally this shouldn't depend on anything, because it isn't utilizing
> any of fbdev code and won't be utilizing any of drm/console code. I
> agree that either of those would be a better place for it, but which one
> do you think would suit more for this driver?

I think ideally this would be integrated with simpledrm in the long run,
but I have no idea what that means in terms of future code changes.

Maybe Thomas Zimmermann has an idea here.

        Arnd

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Arnd Bergmann]

On Thu, Jul 28, 2022 at 4:57 PM Markuss Broks <markuss.broks@gmail.com> wrote:
> On 7/28/22 17:48, Arnd Bergmann wrote:
> > On Thu, Jul 28, 2022 at 4:28 PM Markuss Broks <markuss.broks@gmail.com> 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.
> >>
> >> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
> >> ---
> >>   .../admin-guide/kernel-parameters.txt         |  12 +-
> >>   MAINTAINERS                                   |   5 +
> >>   drivers/firmware/efi/Kconfig                  |   6 +-
> >>   drivers/firmware/efi/Makefile                 |   1 -
> >>   drivers/firmware/efi/earlycon.c               | 246 --------------
> >>   drivers/video/fbdev/Kconfig                   |  11 +
> >>   drivers/video/fbdev/Makefile                  |   1 +
> >>   drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
> >
> > It looks like this is not actually related to fbdev, and since drivers are
> > moving from fbdev/simplefb towards drm/simpledrm, maybe it would be
> > better to put this into either drivers/gpu/drm/tiny/ or possibly
> > drivers/video/console to let this be used without enabling fbdev?
>
> Ideally this shouldn't depend on anything, because it isn't utilizing
> any of fbdev code and won't be utilizing any of drm/console code. I
> agree that either of those would be a better place for it, but which one
> do you think would suit more for this driver?

I think ideally this would be integrated with simpledrm in the long run,
but I have no idea what that means in terms of future code changes.

Maybe Thomas Zimmermann has an idea here.

        Arnd

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 2976 bytes --]

Hi

Am 28.07.22 um 17:16 schrieb Arnd Bergmann:
> On Thu, Jul 28, 2022 at 4:57 PM Markuss Broks <markuss.broks@gmail.com> wrote:
>> On 7/28/22 17:48, Arnd Bergmann wrote:
>>> On Thu, Jul 28, 2022 at 4:28 PM Markuss Broks <markuss.broks@gmail.com> 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.
>>>>
>>>> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
>>>> ---
>>>>    .../admin-guide/kernel-parameters.txt         |  12 +-
>>>>    MAINTAINERS                                   |   5 +
>>>>    drivers/firmware/efi/Kconfig                  |   6 +-
>>>>    drivers/firmware/efi/Makefile                 |   1 -
>>>>    drivers/firmware/efi/earlycon.c               | 246 --------------
>>>>    drivers/video/fbdev/Kconfig                   |  11 +
>>>>    drivers/video/fbdev/Makefile                  |   1 +
>>>>    drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
>>>
>>> It looks like this is not actually related to fbdev, and since drivers are
>>> moving from fbdev/simplefb towards drm/simpledrm, maybe it would be
>>> better to put this into either drivers/gpu/drm/tiny/ or possibly
>>> drivers/video/console to let this be used without enabling fbdev?
>>
>> Ideally this shouldn't depend on anything, because it isn't utilizing
>> any of fbdev code and won't be utilizing any of drm/console code. I
>> agree that either of those would be a better place for it, but which one
>> do you think would suit more for this driver?
> 
> I think ideally this would be integrated with simpledrm in the long run,
> but I have no idea what that means in terms of future code changes.
> 
> Maybe Thomas Zimmermann has an idea here.

It's not a graphics driver, so it doesn't belong to fbdev or DRM. I'd 
put the code under drivers/video/console.

Direct integration with simpledrm (or any other firmware graphics 
driver) is probably not an option. Those drivers operate on platform 
devices, which aren't available when earlycon runs.

There's no management of framebuffer ownership AFAICT? For fbdev and 
DRM, we manage the ownership of the framebuffer memory. When a driver 
takes over the framebuffer, it first has to remove any driver previously 
owning that memory.  That apparently hasn't been a need for early 
consoles so far (?) Maybe we should integrate them into the ownership 
management (see drivers/video/aperture.c).

Best regards
Thomas

> 
>          Arnd

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 2976 bytes --]

Hi

Am 28.07.22 um 17:16 schrieb Arnd Bergmann:
> On Thu, Jul 28, 2022 at 4:57 PM Markuss Broks <markuss.broks@gmail.com> wrote:
>> On 7/28/22 17:48, Arnd Bergmann wrote:
>>> On Thu, Jul 28, 2022 at 4:28 PM Markuss Broks <markuss.broks@gmail.com> 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.
>>>>
>>>> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
>>>> ---
>>>>    .../admin-guide/kernel-parameters.txt         |  12 +-
>>>>    MAINTAINERS                                   |   5 +
>>>>    drivers/firmware/efi/Kconfig                  |   6 +-
>>>>    drivers/firmware/efi/Makefile                 |   1 -
>>>>    drivers/firmware/efi/earlycon.c               | 246 --------------
>>>>    drivers/video/fbdev/Kconfig                   |  11 +
>>>>    drivers/video/fbdev/Makefile                  |   1 +
>>>>    drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
>>>
>>> It looks like this is not actually related to fbdev, and since drivers are
>>> moving from fbdev/simplefb towards drm/simpledrm, maybe it would be
>>> better to put this into either drivers/gpu/drm/tiny/ or possibly
>>> drivers/video/console to let this be used without enabling fbdev?
>>
>> Ideally this shouldn't depend on anything, because it isn't utilizing
>> any of fbdev code and won't be utilizing any of drm/console code. I
>> agree that either of those would be a better place for it, but which one
>> do you think would suit more for this driver?
> 
> I think ideally this would be integrated with simpledrm in the long run,
> but I have no idea what that means in terms of future code changes.
> 
> Maybe Thomas Zimmermann has an idea here.

It's not a graphics driver, so it doesn't belong to fbdev or DRM. I'd 
put the code under drivers/video/console.

Direct integration with simpledrm (or any other firmware graphics 
driver) is probably not an option. Those drivers operate on platform 
devices, which aren't available when earlycon runs.

There's no management of framebuffer ownership AFAICT? For fbdev and 
DRM, we manage the ownership of the framebuffer memory. When a driver 
takes over the framebuffer, it first has to remove any driver previously 
owning that memory.  That apparently hasn't been a need for early 
consoles so far (?) Maybe we should integrate them into the ownership 
management (see drivers/video/aperture.c).

Best regards
Thomas

> 
>          Arnd

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 1/2] drivers: serial: earlycon: Pass device-tree node

[Andy Shevchenko]

On Thu, Jul 28, 2022 at 4:41 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, Jul 28, 2022 at 05:28:18PM +0300, Markuss Broks wrote:
> > Pass a pointer to device-tree node in case the driver probed from
> > OF. This makes early console drivers able to fetch options from
> > device-tree node properties.

...

> > +     unsigned long node;
>
> That should not be an unsigned long, but rather an 'int'.  Something got
> messed up, of_setup_earlycon() should be changed to reflect this before
> propagating the error to other places in the kernel.

It's a pointer, but what puzzles me, why it can't be declared as a such:

 struct device_node *node;

?

> And it's not really a "node" but an "offset", right?

Seems no.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] drivers: serial: earlycon: Pass device-tree node

[Andy Shevchenko]

On Thu, Jul 28, 2022 at 4:41 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, Jul 28, 2022 at 05:28:18PM +0300, Markuss Broks wrote:
> > Pass a pointer to device-tree node in case the driver probed from
> > OF. This makes early console drivers able to fetch options from
> > device-tree node properties.

...

> > +     unsigned long node;
>
> That should not be an unsigned long, but rather an 'int'.  Something got
> messed up, of_setup_earlycon() should be changed to reflect this before
> propagating the error to other places in the kernel.

It's a pointer, but what puzzles me, why it can't be declared as a such:

 struct device_node *node;

?

> And it's not really a "node" but an "offset", right?

Seems no.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Andy Shevchenko]

On Thu, Jul 28, 2022 at 4:58 PM Markuss Broks <markuss.broks@gmail.com> wrote:
> On 7/28/22 17:39, Greg Kroah-Hartman wrote:
> > On Thu, Jul 28, 2022 at 05:28:19PM +0300, Markuss Broks wrote:

> >>   delete mode 100644 drivers/firmware/efi/earlycon.c
> >>   create mode 100644 drivers/video/fbdev/earlycon.c
> >
> > That should be a rename, not a delete/create, right?
>
> Should this change be split into two separate commits,
> one for moving the file and the second for making changes?

I think it's a pointer to use `git format-patch -M -C ...` when
preparing patches.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Andy Shevchenko]

On Thu, Jul 28, 2022 at 4:58 PM Markuss Broks <markuss.broks@gmail.com> wrote:
> On 7/28/22 17:39, Greg Kroah-Hartman wrote:
> > On Thu, Jul 28, 2022 at 05:28:19PM +0300, Markuss Broks wrote:

> >>   delete mode 100644 drivers/firmware/efi/earlycon.c
> >>   create mode 100644 drivers/video/fbdev/earlycon.c
> >
> > That should be a rename, not a delete/create, right?
>
> Should this change be split into two separate commits,
> one for moving the file and the second for making changes?

I think it's a pointer to use `git format-patch -M -C ...` when
preparing patches.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Andy Shevchenko]

On Thu, Jul 28, 2022 at 4:32 PM Markuss Broks <markuss.broks@gmail.com> 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?

...

>  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.

...

> +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 = ...

> +       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?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Andy Shevchenko]

On Thu, Jul 28, 2022 at 4:32 PM Markuss Broks <markuss.broks@gmail.com> 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?

...

>  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.

...

> +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 = ...

> +       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?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[kernel test robot]

Hi Markuss,

I love your patch! Yet something to improve:

[auto build test ERROR on tty/tty-testing]
[also build test ERROR on efi/next staging/staging-testing usb/usb-testing linus/master v5.19-rc8 next-20220728]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Markuss-Broks/Add-generic-framebuffer-support-to-EFI-earlycon-driver/20220728-223117
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
config: ia64-allmodconfig (https://download.01.org/0day-ci/archive/20220729/202207290523.Br8Zr7V3-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/b6a59e731326deaa78f7dcbd97520e2eed2bc707
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Markuss-Broks/Add-generic-framebuffer-support-to-EFI-earlycon-driver/20220728-223117
        git checkout b6a59e731326deaa78f7dcbd97520e2eed2bc707
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/video/fbdev/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   drivers/video/fbdev/earlycon.c: In function 'simplefb_earlycon_map':
>> drivers/video/fbdev/earlycon.c:65:16: error: implicit declaration of function 'early_memremap_prot'; did you mean 'early_memremap'? [-Werror=implicit-function-declaration]
      65 |         return early_memremap_prot(info.phys_base + start, len, pgprot_val(fb_prot));
         |                ^~~~~~~~~~~~~~~~~~~
         |                early_memremap
>> drivers/video/fbdev/earlycon.c:65:16: warning: returning 'int' from a function with return type 'void *' makes pointer from integer without a cast [-Wint-conversion]
      65 |         return early_memremap_prot(info.phys_base + start, len, pgprot_val(fb_prot));
         |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors
   {standard input}: Assembler messages:
   {standard input}:210: Error: Register number out of range 0..0
   {standard input}:211: Error: Register number out of range 0..0
   {standard input}:211: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:211: Warning: Only the first path encountering the conflict is reported
   {standard input}:210: Warning: This is the location of the conflicting usage
   {standard input}:212: Error: Register number out of range 0..0
   {standard input}:212: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:212: Warning: Only the first path encountering the conflict is reported
   {standard input}:210: Warning: This is the location of the conflicting usage
   {standard input}:212: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:212: Warning: Only the first path encountering the conflict is reported
   {standard input}:211: Warning: This is the location of the conflicting usage
   {standard input}:214: Error: Register number out of range 0..0
   {standard input}:214: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:214: Warning: Only the first path encountering the conflict is reported
   {standard input}:210: Warning: This is the location of the conflicting usage
   {standard input}:214: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:214: Warning: Only the first path encountering the conflict is reported
   {standard input}:211: Warning: This is the location of the conflicting usage
   {standard input}:214: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:214: Warning: Only the first path encountering the conflict is reported
   {standard input}:212: Warning: This is the location of the conflicting usage
   {standard input}:215: Error: Register number out of range 0..0
   {standard input}:215: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:215: Warning: Only the first path encountering the conflict is reported
   {standard input}:210: Warning: This is the location of the conflicting usage
   {standard input}:215: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:215: Warning: Only the first path encountering the conflict is reported
   {standard input}:211: Warning: This is the location of the conflicting usage
   {standard input}:215: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:215: Warning: Only the first path encountering the conflict is reported
   {standard input}:212: Warning: This is the location of the conflicting usage
   {standard input}:215: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:215: Warning: Only the first path encountering the conflict is reported
   {standard input}:214: Warning: This is the location of the conflicting usage
   {standard input}:216: Error: Register number out of range 0..0
   {standard input}:216: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:216: Warning: Only the first path encountering the conflict is reported
   {standard input}:210: Warning: This is the location of the conflicting usage
   {standard input}:216: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:216: Warning: Only the first path encountering the conflict is reported
   {standard input}:211: Warning: This is the location of the conflicting usage
   {standard input}:216: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:216: Warning: Only the first path encountering the conflict is reported
   {standard input}:212: Warning: This is the location of the conflicting usage
   {standard input}:216: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:216: Warning: Only the first path encountering the conflict is reported
   {standard input}:214: Warning: This is the location of the conflicting usage
   {standard input}:216: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:216: Warning: Only the first path encountering the conflict is reported
   {standard input}:215: Warning: This is the location of the conflicting usage
   {standard input}:220: Error: Register number out of range 0..0
   {standard input}:376: Error: Register number out of range 0..1
   {standard input}:376: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:376: Warning: Only the first path encountering the conflict is reported
   {standard input}:374: Warning: This is the location of the conflicting usage
   {standard input}:378: Error: Register number out of range 0..1
   {standard input}:378: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:378: Warning: Only the first path encountering the conflict is reported
   {standard input}:374: Warning: This is the location of the conflicting usage
   {standard input}:378: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:378: Warning: Only the first path encountering the conflict is reported
   {standard input}:376: Warning: This is the location of the conflicting usage
   {standard input}:379: Error: Register number out of range 0..1
   {standard input}:379: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:379: Warning: Only the first path encountering the conflict is reported
   {standard input}:374: Warning: This is the location of the conflicting usage
   {standard input}:379: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:379: Warning: Only the first path encountering the conflict is reported
   {standard input}:376: Warning: This is the location of the conflicting usage
   {standard input}:379: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:379: Warning: Only the first path encountering the conflict is reported
   {standard input}:378: Warning: This is the location of the conflicting usage
   {standard input}:380: Error: Register number out of range 0..1
   {standard input}:380: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:380: Warning: Only the first path encountering the conflict is reported
   {standard input}:374: Warning: This is the location of the conflicting usage
   {standard input}:380: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:380: Warning: Only the first path encountering the conflict is reported
   {standard input}:376: Warning: This is the location of the conflicting usage
   {standard input}:380: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:380: Warning: Only the first path encountering the conflict is reported
   {standard input}:378: Warning: This is the location of the conflicting usage
   {standard input}:380: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:380: Warning: Only the first path encountering the conflict is reported
   {standard input}:379: Warning: This is the location of the conflicting usage
   {standard input}:383: Error: Register number out of range 0..1
   {standard input}:384: Error: Register number out of range 0..1
   {standard input}:384: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:384: Warning: Only the first path encountering the conflict is reported
   {standard input}:383: Warning: This is the location of the conflicting usage


vim +65 drivers/video/fbdev/earlycon.c

    56	
    57	static __ref void *simplefb_earlycon_map(unsigned long start, unsigned long len)
    58	{
    59		pgprot_t fb_prot;
    60	
    61		if (info.virt_base)
    62			return info.virt_base + start;
    63	
    64		fb_prot = PAGE_KERNEL;
  > 65		return early_memremap_prot(info.phys_base + start, len, pgprot_val(fb_prot));
    66	}
    67	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[kernel test robot]

Hi Markuss,

I love your patch! Yet something to improve:

[auto build test ERROR on tty/tty-testing]
[also build test ERROR on efi/next staging/staging-testing usb/usb-testing linus/master v5.19-rc8 next-20220728]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Markuss-Broks/Add-generic-framebuffer-support-to-EFI-earlycon-driver/20220728-223117
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
config: ia64-allmodconfig (https://download.01.org/0day-ci/archive/20220729/202207290523.Br8Zr7V3-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/b6a59e731326deaa78f7dcbd97520e2eed2bc707
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Markuss-Broks/Add-generic-framebuffer-support-to-EFI-earlycon-driver/20220728-223117
        git checkout b6a59e731326deaa78f7dcbd97520e2eed2bc707
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/video/fbdev/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   drivers/video/fbdev/earlycon.c: In function 'simplefb_earlycon_map':
>> drivers/video/fbdev/earlycon.c:65:16: error: implicit declaration of function 'early_memremap_prot'; did you mean 'early_memremap'? [-Werror=implicit-function-declaration]
      65 |         return early_memremap_prot(info.phys_base + start, len, pgprot_val(fb_prot));
         |                ^~~~~~~~~~~~~~~~~~~
         |                early_memremap
>> drivers/video/fbdev/earlycon.c:65:16: warning: returning 'int' from a function with return type 'void *' makes pointer from integer without a cast [-Wint-conversion]
      65 |         return early_memremap_prot(info.phys_base + start, len, pgprot_val(fb_prot));
         |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors
   {standard input}: Assembler messages:
   {standard input}:210: Error: Register number out of range 0..0
   {standard input}:211: Error: Register number out of range 0..0
   {standard input}:211: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:211: Warning: Only the first path encountering the conflict is reported
   {standard input}:210: Warning: This is the location of the conflicting usage
   {standard input}:212: Error: Register number out of range 0..0
   {standard input}:212: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:212: Warning: Only the first path encountering the conflict is reported
   {standard input}:210: Warning: This is the location of the conflicting usage
   {standard input}:212: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:212: Warning: Only the first path encountering the conflict is reported
   {standard input}:211: Warning: This is the location of the conflicting usage
   {standard input}:214: Error: Register number out of range 0..0
   {standard input}:214: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:214: Warning: Only the first path encountering the conflict is reported
   {standard input}:210: Warning: This is the location of the conflicting usage
   {standard input}:214: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:214: Warning: Only the first path encountering the conflict is reported
   {standard input}:211: Warning: This is the location of the conflicting usage
   {standard input}:214: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:214: Warning: Only the first path encountering the conflict is reported
   {standard input}:212: Warning: This is the location of the conflicting usage
   {standard input}:215: Error: Register number out of range 0..0
   {standard input}:215: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:215: Warning: Only the first path encountering the conflict is reported
   {standard input}:210: Warning: This is the location of the conflicting usage
   {standard input}:215: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:215: Warning: Only the first path encountering the conflict is reported
   {standard input}:211: Warning: This is the location of the conflicting usage
   {standard input}:215: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:215: Warning: Only the first path encountering the conflict is reported
   {standard input}:212: Warning: This is the location of the conflicting usage
   {standard input}:215: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:215: Warning: Only the first path encountering the conflict is reported
   {standard input}:214: Warning: This is the location of the conflicting usage
   {standard input}:216: Error: Register number out of range 0..0
   {standard input}:216: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:216: Warning: Only the first path encountering the conflict is reported
   {standard input}:210: Warning: This is the location of the conflicting usage
   {standard input}:216: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:216: Warning: Only the first path encountering the conflict is reported
   {standard input}:211: Warning: This is the location of the conflicting usage
   {standard input}:216: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:216: Warning: Only the first path encountering the conflict is reported
   {standard input}:212: Warning: This is the location of the conflicting usage
   {standard input}:216: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:216: Warning: Only the first path encountering the conflict is reported
   {standard input}:214: Warning: This is the location of the conflicting usage
   {standard input}:216: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 35
   {standard input}:216: Warning: Only the first path encountering the conflict is reported
   {standard input}:215: Warning: This is the location of the conflicting usage
   {standard input}:220: Error: Register number out of range 0..0
   {standard input}:376: Error: Register number out of range 0..1
   {standard input}:376: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:376: Warning: Only the first path encountering the conflict is reported
   {standard input}:374: Warning: This is the location of the conflicting usage
   {standard input}:378: Error: Register number out of range 0..1
   {standard input}:378: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:378: Warning: Only the first path encountering the conflict is reported
   {standard input}:374: Warning: This is the location of the conflicting usage
   {standard input}:378: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:378: Warning: Only the first path encountering the conflict is reported
   {standard input}:376: Warning: This is the location of the conflicting usage
   {standard input}:379: Error: Register number out of range 0..1
   {standard input}:379: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:379: Warning: Only the first path encountering the conflict is reported
   {standard input}:374: Warning: This is the location of the conflicting usage
   {standard input}:379: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:379: Warning: Only the first path encountering the conflict is reported
   {standard input}:376: Warning: This is the location of the conflicting usage
   {standard input}:379: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:379: Warning: Only the first path encountering the conflict is reported
   {standard input}:378: Warning: This is the location of the conflicting usage
   {standard input}:380: Error: Register number out of range 0..1
   {standard input}:380: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:380: Warning: Only the first path encountering the conflict is reported
   {standard input}:374: Warning: This is the location of the conflicting usage
   {standard input}:380: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:380: Warning: Only the first path encountering the conflict is reported
   {standard input}:376: Warning: This is the location of the conflicting usage
   {standard input}:380: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:380: Warning: Only the first path encountering the conflict is reported
   {standard input}:378: Warning: This is the location of the conflicting usage
   {standard input}:380: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:380: Warning: Only the first path encountering the conflict is reported
   {standard input}:379: Warning: This is the location of the conflicting usage
   {standard input}:383: Error: Register number out of range 0..1
   {standard input}:384: Error: Register number out of range 0..1
   {standard input}:384: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
   {standard input}:384: Warning: Only the first path encountering the conflict is reported
   {standard input}:383: Warning: This is the location of the conflicting usage


vim +65 drivers/video/fbdev/earlycon.c

    56	
    57	static __ref void *simplefb_earlycon_map(unsigned long start, unsigned long len)
    58	{
    59		pgprot_t fb_prot;
    60	
    61		if (info.virt_base)
    62			return info.virt_base + start;
    63	
    64		fb_prot = PAGE_KERNEL;
  > 65		return early_memremap_prot(info.phys_base + start, len, pgprot_val(fb_prot));
    66	}
    67	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH 1/2] drivers: serial: earlycon: Pass device-tree node

[Greg Kroah-Hartman]

On Thu, Jul 28, 2022 at 11:04:24PM +0200, Andy Shevchenko wrote:
> On Thu, Jul 28, 2022 at 4:41 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Thu, Jul 28, 2022 at 05:28:18PM +0300, Markuss Broks wrote:
> > > Pass a pointer to device-tree node in case the driver probed from
> > > OF. This makes early console drivers able to fetch options from
> > > device-tree node properties.
> 
> ...
> 
> > > +     unsigned long node;
> >
> > That should not be an unsigned long, but rather an 'int'.  Something got
> > messed up, of_setup_earlycon() should be changed to reflect this before
> > propagating the error to other places in the kernel.
> 
> It's a pointer, but what puzzles me, why it can't be declared as a such:
> 
>  struct device_node *node;
> 
> ?

It should not be a pointer, trace things backwards, it comes from a call
to of_setup_earlycon() from early_init_dt_scan_chosen_stdout() which has
offset declared as an int, and then does:
	if (of_setup_earlycon(match, offset, options) == 0)

So why would it be a node?

> > And it's not really a "node" but an "offset", right?
> 
> Seems no.

Really?  What am I missing here?

confused,

greg k-h

Re: [PATCH 1/2] drivers: serial: earlycon: Pass device-tree node

[Greg Kroah-Hartman]

On Thu, Jul 28, 2022 at 11:04:24PM +0200, Andy Shevchenko wrote:
> On Thu, Jul 28, 2022 at 4:41 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Thu, Jul 28, 2022 at 05:28:18PM +0300, Markuss Broks wrote:
> > > Pass a pointer to device-tree node in case the driver probed from
> > > OF. This makes early console drivers able to fetch options from
> > > device-tree node properties.
> 
> ...
> 
> > > +     unsigned long node;
> >
> > That should not be an unsigned long, but rather an 'int'.  Something got
> > messed up, of_setup_earlycon() should be changed to reflect this before
> > propagating the error to other places in the kernel.
> 
> It's a pointer, but what puzzles me, why it can't be declared as a such:
> 
>  struct device_node *node;
> 
> ?

It should not be a pointer, trace things backwards, it comes from a call
to of_setup_earlycon() from early_init_dt_scan_chosen_stdout() which has
offset declared as an int, and then does:
	if (of_setup_earlycon(match, offset, options) == 0)

So why would it be a node?

> > And it's not really a "node" but an "offset", right?
> 
> Seems no.

Really?  What am I missing here?

confused,

greg k-h

Re: [PATCH 1/2] drivers: serial: earlycon: Pass device-tree node

[Andy Shevchenko]

On Fri, Jul 29, 2022 at 9:57 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, Jul 28, 2022 at 11:04:24PM +0200, Andy Shevchenko wrote:
> > On Thu, Jul 28, 2022 at 4:41 PM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > > On Thu, Jul 28, 2022 at 05:28:18PM +0300, Markuss Broks wrote:

...

> > > > +     unsigned long node;
> > >
> > > That should not be an unsigned long, but rather an 'int'.  Something got
> > > messed up, of_setup_earlycon() should be changed to reflect this before
> > > propagating the error to other places in the kernel.
> >
> > It's a pointer, but what puzzles me, why it can't be declared as a such:
> >
> >  struct device_node *node;
> >
> > ?
>
> It should not be a pointer, trace things backwards, it comes from a call
> to of_setup_earlycon() from early_init_dt_scan_chosen_stdout() which has
> offset declared as an int, and then does:
>         if (of_setup_earlycon(match, offset, options) == 0)
>
> So why would it be a node?

This is a very good question.

> > > And it's not really a "node" but an "offset", right?
> >
> > Seems no.
>
> Really?  What am I missing here?

It's me who is missing something here, thanks for your elaboration!
After it it becomes clear that your first question should be
addressed.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] drivers: serial: earlycon: Pass device-tree node

[Andy Shevchenko]

On Fri, Jul 29, 2022 at 9:57 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, Jul 28, 2022 at 11:04:24PM +0200, Andy Shevchenko wrote:
> > On Thu, Jul 28, 2022 at 4:41 PM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > > On Thu, Jul 28, 2022 at 05:28:18PM +0300, Markuss Broks wrote:

...

> > > > +     unsigned long node;
> > >
> > > That should not be an unsigned long, but rather an 'int'.  Something got
> > > messed up, of_setup_earlycon() should be changed to reflect this before
> > > propagating the error to other places in the kernel.
> >
> > It's a pointer, but what puzzles me, why it can't be declared as a such:
> >
> >  struct device_node *node;
> >
> > ?
>
> It should not be a pointer, trace things backwards, it comes from a call
> to of_setup_earlycon() from early_init_dt_scan_chosen_stdout() which has
> offset declared as an int, and then does:
>         if (of_setup_earlycon(match, offset, options) == 0)
>
> So why would it be a node?

This is a very good question.

> > > And it's not really a "node" but an "offset", right?
> >
> > Seems no.
>
> Really?  What am I missing here?

It's me who is missing something here, thanks for your elaboration!
After it it becomes clear that your first question should be
addressed.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Andy Shevchenko]

On Sat, Jul 30, 2022 at 10:55 AM Markuss Broks <markuss.broks@gmail.com> wrote:
> On 7/29/22 00:19, Andy Shevchenko wrote:
> > On Thu, Jul 28, 2022 at 4:32 PM Markuss Broks <markuss.broks@gmail.com> wrote:

...

> I suppose we could use something like:
>
> if (region_intersects() == REGION_INTERSECTS)

Yes.

...

> >> +       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.

I believe we won't introduce more chaos in already existing formats of
one or another thing, so I prefer to be stuck with in practice use
(e.g. "1024x768x16" without quotes for x=1024, y=768, depth=16).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Andy Shevchenko]

On Sat, Jul 30, 2022 at 10:55 AM Markuss Broks <markuss.broks@gmail.com> wrote:
> On 7/29/22 00:19, Andy Shevchenko wrote:
> > On Thu, Jul 28, 2022 at 4:32 PM Markuss Broks <markuss.broks@gmail.com> wrote:

...

> I suppose we could use something like:
>
> if (region_intersects() == REGION_INTERSECTS)

Yes.

...

> >> +       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.

I believe we won't introduce more chaos in already existing formats of
one or another thing, so I prefer to be stuck with in practice use
(e.g. "1024x768x16" without quotes for x=1024, y=768, depth=16).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Markuss Broks]

Hi Andy,

On 7/29/22 00:19, Andy Shevchenko wrote:
> On Thu, Jul 28, 2022 at 4:32 PM Markuss Broks <markuss.broks@gmail.com> 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

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Markuss Broks]

Hi Andy,

On 7/29/22 00:19, Andy Shevchenko wrote:
> On Thu, Jul 28, 2022 at 4:32 PM Markuss Broks <markuss.broks@gmail.com> 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

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Markuss Broks]

Hi Greg,

On 7/28/22 18:01, Greg Kroah-Hartman wrote:
> On Thu, Jul 28, 2022 at 05:52:04PM +0300, Markuss Broks wrote:
>> Hi Greg,
>>
>> On 7/28/22 17:39, Greg Kroah-Hartman wrote:
>>> On Thu, Jul 28, 2022 at 05:28:19PM +0300, Markuss Broks 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.
>>>>
>>>> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
>>>> ---
>>>>    .../admin-guide/kernel-parameters.txt         |  12 +-
>>>>    MAINTAINERS                                   |   5 +
>>>>    drivers/firmware/efi/Kconfig                  |   6 +-
>>>>    drivers/firmware/efi/Makefile                 |   1 -
>>>>    drivers/firmware/efi/earlycon.c               | 246 --------------
>>>>    drivers/video/fbdev/Kconfig                   |  11 +
>>>>    drivers/video/fbdev/Makefile                  |   1 +
>>>>    drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
>>>>    8 files changed, 327 insertions(+), 256 deletions(-)
>>>>    delete mode 100644 drivers/firmware/efi/earlycon.c
>>>>    create mode 100644 drivers/video/fbdev/earlycon.c
>>>
>>> That should be a rename, not a delete/create, right?
>>
>> Should this change be split into two separate commits,
>> one for moving the file and the second for making changes?
> 
> Git will show a rename and modification properly, if you use -M to git
> format-patch, so it should be fine.

It appears that there are so many changes Git would refuse to make it a 
"move" no matter what I do. What should be done here: should it be two 
separate commits for move/change or should it just be kept as delete/create?

- Markuss

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Markuss Broks]

Hi Greg,

On 7/28/22 18:01, Greg Kroah-Hartman wrote:
> On Thu, Jul 28, 2022 at 05:52:04PM +0300, Markuss Broks wrote:
>> Hi Greg,
>>
>> On 7/28/22 17:39, Greg Kroah-Hartman wrote:
>>> On Thu, Jul 28, 2022 at 05:28:19PM +0300, Markuss Broks 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.
>>>>
>>>> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
>>>> ---
>>>>    .../admin-guide/kernel-parameters.txt         |  12 +-
>>>>    MAINTAINERS                                   |   5 +
>>>>    drivers/firmware/efi/Kconfig                  |   6 +-
>>>>    drivers/firmware/efi/Makefile                 |   1 -
>>>>    drivers/firmware/efi/earlycon.c               | 246 --------------
>>>>    drivers/video/fbdev/Kconfig                   |  11 +
>>>>    drivers/video/fbdev/Makefile                  |   1 +
>>>>    drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
>>>>    8 files changed, 327 insertions(+), 256 deletions(-)
>>>>    delete mode 100644 drivers/firmware/efi/earlycon.c
>>>>    create mode 100644 drivers/video/fbdev/earlycon.c
>>>
>>> That should be a rename, not a delete/create, right?
>>
>> Should this change be split into two separate commits,
>> one for moving the file and the second for making changes?
> 
> Git will show a rename and modification properly, if you use -M to git
> format-patch, so it should be fine.

It appears that there are so many changes Git would refuse to make it a 
"move" no matter what I do. What should be done here: should it be two 
separate commits for move/change or should it just be kept as delete/create?

- Markuss

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Greg Kroah-Hartman]

On Sat, Aug 06, 2022 at 07:26:07PM +0300, Markuss Broks wrote:
> Hi Greg,
> 
> On 7/28/22 18:01, Greg Kroah-Hartman wrote:
> > On Thu, Jul 28, 2022 at 05:52:04PM +0300, Markuss Broks wrote:
> > > Hi Greg,
> > > 
> > > On 7/28/22 17:39, Greg Kroah-Hartman wrote:
> > > > On Thu, Jul 28, 2022 at 05:28:19PM +0300, Markuss Broks 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.
> > > > > 
> > > > > Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
> > > > > ---
> > > > >    .../admin-guide/kernel-parameters.txt         |  12 +-
> > > > >    MAINTAINERS                                   |   5 +
> > > > >    drivers/firmware/efi/Kconfig                  |   6 +-
> > > > >    drivers/firmware/efi/Makefile                 |   1 -
> > > > >    drivers/firmware/efi/earlycon.c               | 246 --------------
> > > > >    drivers/video/fbdev/Kconfig                   |  11 +
> > > > >    drivers/video/fbdev/Makefile                  |   1 +
> > > > >    drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
> > > > >    8 files changed, 327 insertions(+), 256 deletions(-)
> > > > >    delete mode 100644 drivers/firmware/efi/earlycon.c
> > > > >    create mode 100644 drivers/video/fbdev/earlycon.c
> > > > 
> > > > That should be a rename, not a delete/create, right?
> > > 
> > > Should this change be split into two separate commits,
> > > one for moving the file and the second for making changes?
> > 
> > Git will show a rename and modification properly, if you use -M to git
> > format-patch, so it should be fine.
> 
> It appears that there are so many changes Git would refuse to make it a
> "move" no matter what I do. What should be done here: should it be two
> separate commits for move/change or should it just be kept as delete/create?

One commit to move the file, and then add your changes on top of it
might be the easiest to review, right?

thanks,

greg k-h

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Greg Kroah-Hartman]

On Sat, Aug 06, 2022 at 07:26:07PM +0300, Markuss Broks wrote:
> Hi Greg,
> 
> On 7/28/22 18:01, Greg Kroah-Hartman wrote:
> > On Thu, Jul 28, 2022 at 05:52:04PM +0300, Markuss Broks wrote:
> > > Hi Greg,
> > > 
> > > On 7/28/22 17:39, Greg Kroah-Hartman wrote:
> > > > On Thu, Jul 28, 2022 at 05:28:19PM +0300, Markuss Broks 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.
> > > > > 
> > > > > Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
> > > > > ---
> > > > >    .../admin-guide/kernel-parameters.txt         |  12 +-
> > > > >    MAINTAINERS                                   |   5 +
> > > > >    drivers/firmware/efi/Kconfig                  |   6 +-
> > > > >    drivers/firmware/efi/Makefile                 |   1 -
> > > > >    drivers/firmware/efi/earlycon.c               | 246 --------------
> > > > >    drivers/video/fbdev/Kconfig                   |  11 +
> > > > >    drivers/video/fbdev/Makefile                  |   1 +
> > > > >    drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
> > > > >    8 files changed, 327 insertions(+), 256 deletions(-)
> > > > >    delete mode 100644 drivers/firmware/efi/earlycon.c
> > > > >    create mode 100644 drivers/video/fbdev/earlycon.c
> > > > 
> > > > That should be a rename, not a delete/create, right?
> > > 
> > > Should this change be split into two separate commits,
> > > one for moving the file and the second for making changes?
> > 
> > Git will show a rename and modification properly, if you use -M to git
> > format-patch, so it should be fine.
> 
> It appears that there are so many changes Git would refuse to make it a
> "move" no matter what I do. What should be done here: should it be two
> separate commits for move/change or should it just be kept as delete/create?

One commit to move the file, and then add your changes on top of it
might be the easiest to review, right?

thanks,

greg k-h

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Daniel Vetter]

On Sun, Aug 07, 2022 at 08:53:14AM +0200, Greg Kroah-Hartman wrote:
> On Sat, Aug 06, 2022 at 07:26:07PM +0300, Markuss Broks wrote:
> > Hi Greg,
> > 
> > On 7/28/22 18:01, Greg Kroah-Hartman wrote:
> > > On Thu, Jul 28, 2022 at 05:52:04PM +0300, Markuss Broks wrote:
> > > > Hi Greg,
> > > > 
> > > > On 7/28/22 17:39, Greg Kroah-Hartman wrote:
> > > > > On Thu, Jul 28, 2022 at 05:28:19PM +0300, Markuss Broks 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.
> > > > > > 
> > > > > > Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
> > > > > > ---
> > > > > >    .../admin-guide/kernel-parameters.txt         |  12 +-
> > > > > >    MAINTAINERS                                   |   5 +
> > > > > >    drivers/firmware/efi/Kconfig                  |   6 +-
> > > > > >    drivers/firmware/efi/Makefile                 |   1 -
> > > > > >    drivers/firmware/efi/earlycon.c               | 246 --------------
> > > > > >    drivers/video/fbdev/Kconfig                   |  11 +
> > > > > >    drivers/video/fbdev/Makefile                  |   1 +
> > > > > >    drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
> > > > > >    8 files changed, 327 insertions(+), 256 deletions(-)
> > > > > >    delete mode 100644 drivers/firmware/efi/earlycon.c
> > > > > >    create mode 100644 drivers/video/fbdev/earlycon.c
> > > > > 
> > > > > That should be a rename, not a delete/create, right?
> > > > 
> > > > Should this change be split into two separate commits,
> > > > one for moving the file and the second for making changes?
> > > 
> > > Git will show a rename and modification properly, if you use -M to git
> > > format-patch, so it should be fine.
> > 
> > It appears that there are so many changes Git would refuse to make it a
> > "move" no matter what I do. What should be done here: should it be two
> > separate commits for move/change or should it just be kept as delete/create?
> 
> One commit to move the file, and then add your changes on top of it
> might be the easiest to review, right?

+1

I think this should be a least
- commit to move the file, as unchanged as possible
- commit to auto-select the right mapping mode (or maybe that's only in
  v2)
- actual change to add the simplefb support with a clearly readable diff

But also video/console is for Greg to maintain, I'm trying hard to not go
even more stupid :-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 2/2] efi: earlycon: Add support for generic framebuffers and move to fbdev subsystem

[Daniel Vetter]

On Sun, Aug 07, 2022 at 08:53:14AM +0200, Greg Kroah-Hartman wrote:
> On Sat, Aug 06, 2022 at 07:26:07PM +0300, Markuss Broks wrote:
> > Hi Greg,
> > 
> > On 7/28/22 18:01, Greg Kroah-Hartman wrote:
> > > On Thu, Jul 28, 2022 at 05:52:04PM +0300, Markuss Broks wrote:
> > > > Hi Greg,
> > > > 
> > > > On 7/28/22 17:39, Greg Kroah-Hartman wrote:
> > > > > On Thu, Jul 28, 2022 at 05:28:19PM +0300, Markuss Broks 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.
> > > > > > 
> > > > > > Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
> > > > > > ---
> > > > > >    .../admin-guide/kernel-parameters.txt         |  12 +-
> > > > > >    MAINTAINERS                                   |   5 +
> > > > > >    drivers/firmware/efi/Kconfig                  |   6 +-
> > > > > >    drivers/firmware/efi/Makefile                 |   1 -
> > > > > >    drivers/firmware/efi/earlycon.c               | 246 --------------
> > > > > >    drivers/video/fbdev/Kconfig                   |  11 +
> > > > > >    drivers/video/fbdev/Makefile                  |   1 +
> > > > > >    drivers/video/fbdev/earlycon.c                | 301 ++++++++++++++++++
> > > > > >    8 files changed, 327 insertions(+), 256 deletions(-)
> > > > > >    delete mode 100644 drivers/firmware/efi/earlycon.c
> > > > > >    create mode 100644 drivers/video/fbdev/earlycon.c
> > > > > 
> > > > > That should be a rename, not a delete/create, right?
> > > > 
> > > > Should this change be split into two separate commits,
> > > > one for moving the file and the second for making changes?
> > > 
> > > Git will show a rename and modification properly, if you use -M to git
> > > format-patch, so it should be fine.
> > 
> > It appears that there are so many changes Git would refuse to make it a
> > "move" no matter what I do. What should be done here: should it be two
> > separate commits for move/change or should it just be kept as delete/create?
> 
> One commit to move the file, and then add your changes on top of it
> might be the easiest to review, right?

+1

I think this should be a least
- commit to move the file, as unchanged as possible
- commit to auto-select the right mapping mode (or maybe that's only in
  v2)
- actual change to add the simplefb support with a clearly readable diff

But also video/console is for Greg to maintain, I'm trying hard to not go
even more stupid :-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch