[PATCH 0/8] drm, coreboot: Add DRM coreboot driver

[PATCH 0/8] drm, coreboot: Add DRM coreboot driver

[Thomas Zimmermann]

Coreboot implements framebuffer support via simplefb. Provide a
native DRM driver. Keep the simplefb code for now.

For each firmware's provided framebuffer, we want a dedicated DRM
driver tailored towards the platform's feature set. The coreboot
framebuffer device creates a simplefb device for the provided
framebuffer. When the native graphics driver unloads the simplefb
device, it leaves behind a dangling pointer in the coreboot framebuffer
device. [1] This only works because the coreboot framebuffer device
never runs this code; even after the native driver took over the
hardware. At that point the underlying coreboot framebuffer is gone,
which is inconsistent with kernel state.

Additionally, the simplefb drivers handle simple-framebuffer nodes in
the DeviceTree, but were not meant for supporting arbitrary framebuffers.
The simplefb infrastructure should be phased out for non-DT use cases.
Coreboot is one of the final users of the code (besides n64).

Patches 1 to 4 of this series prepare the kernel's coreboot support for
the DRM driver. With patch 1, the coreboot framebuffer device will only
be created if it really handles the framebuffer. Some systems emulate
UEFI instead. The other 3 patches make coreboot drivers available to
other subsystems. A DRM driver will then be able to bind directly to a
coreboot device.

Patch 5 prepares the kernel's aperture helpers for coreboot devices.
This is required to handover hardware to the native graphics driver.

Patches 6 to 8 prepare DRM and add a new driver for the coreboot
framebuffer. The corebootdrm driver follows the pattern established by
similar drivers. It also uses the same sysfb helpers. It's fairly small
therefore.

Tested on an HP Chromebook with MrChromebox 4.16. Runs with Weston and
fbcon. Xorg requires an additional patch available at [2].

[1] https://elixir.bootlin.com/linux/v6.18/source/drivers/firmware/google/framebuffer-coreboot.c#L92
[2] https://gitlab.freedesktop.org/tzimmermann/xserver/-/commit/0b326aad28549762ed2b0e2bedf8f8a42f1f6b3b

Thomas Zimmermann (8):
  firmware: google: Do sysfb test before creating coreboot framebuffer
  firmware: google: Init coreboot bus with subsys_initcall()
  firmware: google: Clean up include statements in coreboot_table.h
  firmware: google: Export coreboot driver and device interfaces
  video/aperture: Support coreboot devices
  drm/sysfb: Remove duplicate declarations
  drm/sysfb: Generalize pixel-format matching
  drm/sysfb: corebootdrm: Add DRM driver for coreboot framebuffers

 drivers/firmware/google/Kconfig               |   1 +
 drivers/firmware/google/cbmem.c               |   4 +-
 drivers/firmware/google/coreboot_table.c      |  33 +-
 .../firmware/google/framebuffer-coreboot.c    |  20 +-
 drivers/firmware/google/memconsole-coreboot.c |   3 +-
 drivers/firmware/google/vpd.c                 |   3 +-
 drivers/gpu/drm/sysfb/Kconfig                 |  16 +
 drivers/gpu/drm/sysfb/Makefile                |   1 +
 drivers/gpu/drm/sysfb/corebootdrm.c           | 402 ++++++++++++++++++
 drivers/gpu/drm/sysfb/drm_sysfb.c             |  24 ++
 drivers/gpu/drm/sysfb/drm_sysfb_helper.h      |  17 +-
 drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c |  30 --
 drivers/gpu/drm/sysfb/efidrm.c                |   8 +-
 drivers/gpu/drm/sysfb/vesadrm.c               |   8 +-
 drivers/video/aperture.c                      |  60 ++-
 include/linux/aperture.h                      |  16 +
 .../linux/coreboot.h                          |  29 +-
 17 files changed, 575 insertions(+), 100 deletions(-)
 create mode 100644 drivers/gpu/drm/sysfb/corebootdrm.c
 rename drivers/firmware/google/coreboot_table.h => include/linux/coreboot.h (86%)

-- 
2.52.0

[PATCH 1/8] firmware: google: Do sysfb test before creating coreboot framebuffer

[Thomas Zimmermann]

Test sysfb before creating the coreboot framebuffer device. Skip
device creation if the test fails, as this framebuffer does not exist.

Depending on the system setup, the initial framebuffer can be provided
by the boot loader via screen_info boot parameters and handled by the
kernel's sysfb code in drivers/firmware/sysfb.c. With the sysfb test in
the coreboot-framebuffer probing, the coreboot device is present without
the framebuffer. Even after the sysfb device has been replaced with a
native PCI device, the coreboot device persists.

Skipping device creation early avoids all these inconsistencies. It
further prepares coreboot to support graphics drivers besides the one
in framebuffer-coreboot.c.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/firmware/google/coreboot_table.c       | 17 +++++++++++++++++
 drivers/firmware/google/coreboot_table.h       |  1 +
 drivers/firmware/google/framebuffer-coreboot.c | 16 ----------------
 3 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c
index 882db32e51be..c34426e5002d 100644
--- a/drivers/firmware/google/coreboot_table.c
+++ b/drivers/firmware/google/coreboot_table.c
@@ -18,6 +18,7 @@
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/sysfb.h>
 
 #include "coreboot_table.h"
 
@@ -118,6 +119,22 @@ static int coreboot_table_populate(struct device *dev, void *ptr)
 			return -EINVAL;
 		}
 
+		switch (entry->tag) {
+		case CB_TAG_FRAMEBUFFER:
+			/*
+			 * On coreboot systems, the advertised CB_TAG_FRAMEBUFFER entry
+			 * in the coreboot table should only be used if the payload did
+			 * not pass a framebuffer information to the Linux kernel.
+			 *
+			 * If the global screen_info data has been filled, the generic
+			 * system framebuffers (sysfb) will already register a platform
+			 * device and pass that screen_info as platform_data to a driver
+			 * that can scan-out using the system-provided framebuffer.
+			 */
+			if (sysfb_handles_screen_info())
+				continue;
+		}
+
 		device = kzalloc(sizeof(device->dev) + entry->size, GFP_KERNEL);
 		if (!device)
 			return -ENOMEM;
diff --git a/drivers/firmware/google/coreboot_table.h b/drivers/firmware/google/coreboot_table.h
index bb6f0f7299b4..e3c353676940 100644
--- a/drivers/firmware/google/coreboot_table.h
+++ b/drivers/firmware/google/coreboot_table.h
@@ -40,6 +40,7 @@ struct lb_cbmem_ref {
 	u64 cbmem_addr;
 };
 
+#define CB_TAG_FRAMEBUFFER 0x12
 #define LB_TAG_CBMEM_ENTRY 0x31
 
 /* Corresponds to LB_TAG_CBMEM_ENTRY */
diff --git a/drivers/firmware/google/framebuffer-coreboot.c b/drivers/firmware/google/framebuffer-coreboot.c
index c68c9f56370f..bb53d1a47409 100644
--- a/drivers/firmware/google/framebuffer-coreboot.c
+++ b/drivers/firmware/google/framebuffer-coreboot.c
@@ -15,12 +15,9 @@
 #include <linux/module.h>
 #include <linux/platform_data/simplefb.h>
 #include <linux/platform_device.h>
-#include <linux/sysfb.h>
 
 #include "coreboot_table.h"
 
-#define CB_TAG_FRAMEBUFFER 0x12
-
 static const struct simplefb_format formats[] = SIMPLEFB_FORMATS;
 
 static int framebuffer_probe(struct coreboot_device *dev)
@@ -37,19 +34,6 @@ static int framebuffer_probe(struct coreboot_device *dev)
 		.format = NULL,
 	};
 
-	/*
-	 * On coreboot systems, the advertised LB_TAG_FRAMEBUFFER entry
-	 * in the coreboot table should only be used if the payload did
-	 * not pass a framebuffer information to the Linux kernel.
-	 *
-	 * If the global screen_info data has been filled, the Generic
-	 * System Framebuffers (sysfb) will already register a platform
-	 * device and pass that screen_info as platform_data to a driver
-	 * that can scan-out using the system provided framebuffer.
-	 */
-	if (sysfb_handles_screen_info())
-		return -ENODEV;
-
 	if (!fb->physical_address)
 		return -ENODEV;
 
-- 
2.52.0

[PATCH 2/8] firmware: google: Init coreboot bus with subsys_initcall()

[Thomas Zimmermann]

Using module_init()/device_initcall() is too late to initialize
the coreboot bus, as there might already be drivers that depend
on it.

So far this hasn't been a problem, as coreboot controls all device
creation. Initializing the coreboot bus earlier in subsys_initcall()
will allow for external coreboot drivers to register themselves
with device_initcall(). Prepares coreboot to support additional
coreboot drivers from other subsystems.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/firmware/google/coreboot_table.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c
index c34426e5002d..3bee22d2a7b3 100644
--- a/drivers/firmware/google/coreboot_table.c
+++ b/drivers/firmware/google/coreboot_table.c
@@ -268,7 +268,7 @@ static void __exit coreboot_table_driver_exit(void)
 	bus_unregister(&coreboot_bus_type);
 }
 
-module_init(coreboot_table_driver_init);
+subsys_initcall(coreboot_table_driver_init);
 module_exit(coreboot_table_driver_exit);
 
 MODULE_AUTHOR("Google, Inc.");
-- 
2.52.0

[PATCH 3/8] firmware: google: Clean up include statements in coreboot_table.h

[Thomas Zimmermann]

Include <linux/mod_devicetable.h> from source files and only forward-
declare struct coreboot_device_id in coreboot_table.h.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/firmware/google/cbmem.c                | 1 +
 drivers/firmware/google/coreboot_table.c       | 1 +
 drivers/firmware/google/coreboot_table.h       | 3 ++-
 drivers/firmware/google/framebuffer-coreboot.c | 1 +
 drivers/firmware/google/memconsole-coreboot.c  | 1 +
 drivers/firmware/google/vpd.c                  | 1 +
 6 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/google/cbmem.c b/drivers/firmware/google/cbmem.c
index 54c3b8b05e5d..3397bacdfdbe 100644
--- a/drivers/firmware/google/cbmem.c
+++ b/drivers/firmware/google/cbmem.c
@@ -12,6 +12,7 @@
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/kobject.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c
index 3bee22d2a7b3..ae20c0527032 100644
--- a/drivers/firmware/google/coreboot_table.c
+++ b/drivers/firmware/google/coreboot_table.c
@@ -14,6 +14,7 @@
 #include <linux/init.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
diff --git a/drivers/firmware/google/coreboot_table.h b/drivers/firmware/google/coreboot_table.h
index e3c353676940..4c71830c98ca 100644
--- a/drivers/firmware/google/coreboot_table.h
+++ b/drivers/firmware/google/coreboot_table.h
@@ -13,7 +13,8 @@
 #define __COREBOOT_TABLE_H
 
 #include <linux/device.h>
-#include <linux/mod_devicetable.h>
+
+struct coreboot_device_id;
 
 /* Coreboot table header structure */
 struct coreboot_table_header {
diff --git a/drivers/firmware/google/framebuffer-coreboot.c b/drivers/firmware/google/framebuffer-coreboot.c
index bb53d1a47409..7ac9dc4f5392 100644
--- a/drivers/firmware/google/framebuffer-coreboot.c
+++ b/drivers/firmware/google/framebuffer-coreboot.c
@@ -12,6 +12,7 @@
 #include <linux/device.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/platform_data/simplefb.h>
 #include <linux/platform_device.h>
diff --git a/drivers/firmware/google/memconsole-coreboot.c b/drivers/firmware/google/memconsole-coreboot.c
index c5f08617aa8d..4aa9b1cad3c3 100644
--- a/drivers/firmware/google/memconsole-coreboot.c
+++ b/drivers/firmware/google/memconsole-coreboot.c
@@ -10,6 +10,7 @@
 #include <linux/device.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
 
 #include "memconsole.h"
diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
index 339a3f74b247..8d7f123f96f4 100644
--- a/drivers/firmware/google/vpd.c
+++ b/drivers/firmware/google/vpd.c
@@ -13,6 +13,7 @@
 #include <linux/kernel.h>
 #include <linux/kobject.h>
 #include <linux/list.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
 #include <linux/platform_device.h>
-- 
2.52.0

[PATCH 4/8] firmware: google: Export coreboot driver and device interfaces

[Thomas Zimmermann]

Split coreboot_table.h: move struct coreboot_table_header into
coreboot_table.h and the rest of the header to include/linux/coreboot.h.

Prepares coreboot for allowing drivers in other subsystems.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/firmware/google/cbmem.c               |  3 +--
 drivers/firmware/google/coreboot_table.c      | 11 +++++++-
 .../firmware/google/framebuffer-coreboot.c    |  3 +--
 drivers/firmware/google/memconsole-coreboot.c |  2 +-
 drivers/firmware/google/vpd.c                 |  2 +-
 .../linux/coreboot.h                          | 27 +++++++------------
 6 files changed, 23 insertions(+), 25 deletions(-)
 rename drivers/firmware/google/coreboot_table.h => include/linux/coreboot.h (88%)

diff --git a/drivers/firmware/google/cbmem.c b/drivers/firmware/google/cbmem.c
index 3397bacdfdbe..b3e477d9ad4e 100644
--- a/drivers/firmware/google/cbmem.c
+++ b/drivers/firmware/google/cbmem.c
@@ -7,6 +7,7 @@
  * Copyright 2022 Google LLC
  */
 
+#include <linux/coreboot.h>
 #include <linux/device.h>
 #include <linux/init.h>
 #include <linux/io.h>
@@ -18,8 +19,6 @@
 #include <linux/slab.h>
 #include <linux/sysfs.h>
 
-#include "coreboot_table.h"
-
 struct cbmem_entry {
 	char *mem_file_buf;
 	u32 size;
diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c
index ae20c0527032..3b07e1d60255 100644
--- a/drivers/firmware/google/coreboot_table.c
+++ b/drivers/firmware/google/coreboot_table.c
@@ -9,6 +9,7 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/coreboot.h>
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/init.h>
@@ -21,7 +22,15 @@
 #include <linux/slab.h>
 #include <linux/sysfb.h>
 
-#include "coreboot_table.h"
+/* Coreboot table header structure */
+struct coreboot_table_header {
+	char signature[4];
+	u32 header_bytes;
+	u32 header_checksum;
+	u32 table_bytes;
+	u32 table_checksum;
+	u32 table_entries;
+};
 
 #define CB_DEV(d) container_of(d, struct coreboot_device, dev)
 #define CB_DRV(d) container_of_const(d, struct coreboot_driver, drv)
diff --git a/drivers/firmware/google/framebuffer-coreboot.c b/drivers/firmware/google/framebuffer-coreboot.c
index 7ac9dc4f5392..88815e0abb3f 100644
--- a/drivers/firmware/google/framebuffer-coreboot.c
+++ b/drivers/firmware/google/framebuffer-coreboot.c
@@ -9,6 +9,7 @@
  * Copyright 2017 Samuel Holland <samuel@sholland.org>
  */
 
+#include <linux/coreboot.h>
 #include <linux/device.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
@@ -17,8 +18,6 @@
 #include <linux/platform_data/simplefb.h>
 #include <linux/platform_device.h>
 
-#include "coreboot_table.h"
-
 static const struct simplefb_format formats[] = SIMPLEFB_FORMATS;
 
 static int framebuffer_probe(struct coreboot_device *dev)
diff --git a/drivers/firmware/google/memconsole-coreboot.c b/drivers/firmware/google/memconsole-coreboot.c
index 4aa9b1cad3c3..bc7e4bfb2d08 100644
--- a/drivers/firmware/google/memconsole-coreboot.c
+++ b/drivers/firmware/google/memconsole-coreboot.c
@@ -7,6 +7,7 @@
  * Copyright 2017 Google Inc.
  */
 
+#include <linux/coreboot.h>
 #include <linux/device.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
@@ -14,7 +15,6 @@
 #include <linux/module.h>
 
 #include "memconsole.h"
-#include "coreboot_table.h"
 
 #define CB_TAG_CBMEM_CONSOLE	0x17
 
diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
index 8d7f123f96f4..f75008b956d5 100644
--- a/drivers/firmware/google/vpd.c
+++ b/drivers/firmware/google/vpd.c
@@ -7,6 +7,7 @@
  * Copyright 2017 Google Inc.
  */
 
+#include <linux/coreboot.h>
 #include <linux/ctype.h>
 #include <linux/init.h>
 #include <linux/io.h>
@@ -20,7 +21,6 @@
 #include <linux/slab.h>
 #include <linux/sysfs.h>
 
-#include "coreboot_table.h"
 #include "vpd_decode.h"
 
 #define CB_TAG_VPD      0x2c
diff --git a/drivers/firmware/google/coreboot_table.h b/include/linux/coreboot.h
similarity index 88%
rename from drivers/firmware/google/coreboot_table.h
rename to include/linux/coreboot.h
index 4c71830c98ca..26ea5eecac52 100644
--- a/drivers/firmware/google/coreboot_table.h
+++ b/include/linux/coreboot.h
@@ -1,32 +1,26 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
- * coreboot_table.h
+ * coreboot.h
  *
- * Internal header for coreboot table access.
+ * Coreboot device and driver interfaces.
  *
  * Copyright 2014 Gerd Hoffmann <kraxel@redhat.com>
  * Copyright 2017 Google Inc.
  * Copyright 2017 Samuel Holland <samuel@sholland.org>
  */
 
-#ifndef __COREBOOT_TABLE_H
-#define __COREBOOT_TABLE_H
+#ifndef _LINUX_COREBOOT_H
+#define _LINUX_COREBOOT_H
 
 #include <linux/device.h>
 
 struct coreboot_device_id;
 
-/* Coreboot table header structure */
-struct coreboot_table_header {
-	char signature[4];
-	u32 header_bytes;
-	u32 header_checksum;
-	u32 table_bytes;
-	u32 table_checksum;
-	u32 table_entries;
-};
-
 /* List of coreboot entry structures that is used */
+
+#define CB_TAG_FRAMEBUFFER 0x12
+#define LB_TAG_CBMEM_ENTRY 0x31
+
 /* Generic */
 struct coreboot_table_entry {
 	u32 tag;
@@ -41,9 +35,6 @@ struct lb_cbmem_ref {
 	u64 cbmem_addr;
 };
 
-#define CB_TAG_FRAMEBUFFER 0x12
-#define LB_TAG_CBMEM_ENTRY 0x31
-
 /* Corresponds to LB_TAG_CBMEM_ENTRY */
 struct lb_cbmem_entry {
 	u32 tag;
@@ -118,4 +109,4 @@ void coreboot_driver_unregister(struct coreboot_driver *driver);
 	module_driver(__coreboot_driver, coreboot_driver_register, \
 			coreboot_driver_unregister)
 
-#endif /* __COREBOOT_TABLE_H */
+#endif /* _LINUX_COREBOOT_H */
-- 
2.52.0

[PATCH 5/8] video/aperture: Support coreboot devices

[Thomas Zimmermann]

Add support for coreboot devices. Until now, all devices with system
framebuffers were on the platform bus. On systems with a coreboot
firmware, the system framebuffer can be represented as coreboot device
on the coreboot bus. Handling the related graphics apertures requires
new interfaces and a custom way of removing the coreboot device. The
core aperture code is independent from the type of device.

Also move around a comment that refers to both, platform and coreboot,
devices.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/aperture.c | 60 ++++++++++++++++++++++++++++++++--------
 include/linux/aperture.h | 16 +++++++++++
 2 files changed, 64 insertions(+), 12 deletions(-)

diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
index 2b5a1e666e9b..15e664a4478b 100644
--- a/drivers/video/aperture.c
+++ b/drivers/video/aperture.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: MIT
 
 #include <linux/aperture.h>
+#include <linux/coreboot.h>
 #include <linux/device.h>
 #include <linux/list.h>
 #include <linux/mutex.h>
@@ -197,22 +198,47 @@ static int devm_aperture_acquire(struct device *dev,
 	return devm_add_action_or_reset(dev, devm_aperture_acquire_release, ap);
 }
 
+#if defined(CONFIG_GOOGLE_COREBOOT_TABLE)
+static void aperture_detach_coreboot_device(struct device *dev)
+{
+	struct coreboot_device *cbdev = dev_to_coreboot_device(dev);
+
+	device_unregister(&cbdev->dev);
+}
+
+/**
+ * devm_aperture_acquire_for_coreboot_device - Acquires ownership of an aperture
+ *                                             on behalf of a coreboot device.
+ * @cbdev:	the coreboot device to own the aperture
+ * @base:	the aperture's byte offset in physical memory
+ * @size:	the aperture size in bytes
+ *
+ * Installs the given device as the new owner of the aperture. The function
+ * expects the aperture to be provided by a coreboot device. If another
+ * driver takes over ownership of the aperture, aperture helpers will then
+ * unregister the coreboot device automatically. All acquired apertures are
+ * released automatically when the underlying device goes away.
+ *
+ * The function fails if the aperture, or parts of it, is currently
+ * owned by another device. To evict current owners, callers should use
+ * remove_conflicting_devices() et al. before calling this function.
+ *
+ * Returns:
+ * 0 on success, or a negative errno value otherwise.
+ */
+int devm_aperture_acquire_for_coreboot_device(struct coreboot_device *cbdev,
+					      resource_size_t base,
+					      resource_size_t size)
+{
+	return devm_aperture_acquire(&cbdev->dev, base, size, aperture_detach_coreboot_device);
+}
+EXPORT_SYMBOL(devm_aperture_acquire_for_coreboot_device);
+#endif
+
 static void aperture_detach_platform_device(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 
-	/*
-	 * Remove the device from the device hierarchy. This is the right thing
-	 * to do for firmware-based fb drivers, such as EFI, VESA or VGA. After
-	 * the new driver takes over the hardware, the firmware device's state
-	 * will be lost.
-	 *
-	 * For non-platform devices, a new callback would be required.
-	 *
-	 * If the aperture helpers ever need to handle native drivers, this call
-	 * would only have to unplug the DRM device, so that the hardware device
-	 * stays around after detachment.
-	 */
 	platform_device_unregister(pdev);
 }
 
@@ -264,6 +290,16 @@ static void aperture_detach_devices(resource_size_t base, resource_size_t size)
 		ap->dev = NULL; /* detach from device */
 		list_del(&ap->lh);
 
+		/*
+		 * Remove the device from the device hierarchy. This is the right thing
+		 * to do for firmware-based fb drivers, such as EFI, VESA or VGA. After
+		 * the new driver takes over the hardware, the firmware device's state
+		 * will be lost.
+		 *
+		 * If the aperture helpers ever need to handle native drivers, this call
+		 * would only have to unplug the DRM device, so that the hardware device
+		 * stays around after detachment.
+		 */
 		ap->detach(dev);
 	}
 
diff --git a/include/linux/aperture.h b/include/linux/aperture.h
index 1a9a88b11584..459823f86dc8 100644
--- a/include/linux/aperture.h
+++ b/include/linux/aperture.h
@@ -5,10 +5,17 @@
 
 #include <linux/types.h>
 
+struct coreboot_device;
 struct pci_dev;
 struct platform_device;
 
 #if defined(CONFIG_APERTURE_HELPERS)
+#if defined(CONFIG_GOOGLE_COREBOOT_TABLE)
+int devm_aperture_acquire_for_coreboot_device(struct coreboot_device *cbdev,
+					      resource_size_t base,
+					      resource_size_t size);
+#endif
+
 int devm_aperture_acquire_for_platform_device(struct platform_device *pdev,
 					      resource_size_t base,
 					      resource_size_t size);
@@ -20,6 +27,15 @@ int __aperture_remove_legacy_vga_devices(struct pci_dev *pdev);
 
 int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *name);
 #else
+#if defined(CONFIG_GOOGLE_COREBOOT_TABLE)
+static inline int devm_aperture_acquire_for_coreboot_device(struct coreboot_device *cbdev,
+							    resource_size_t base,
+							    resource_size_t size)
+{
+	return 0;
+}
+#endif
+
 static inline int devm_aperture_acquire_for_platform_device(struct platform_device *pdev,
 							    resource_size_t base,
 							    resource_size_t size)
-- 
2.52.0

[PATCH 6/8] drm/sysfb: Remove duplicate declarations

[Thomas Zimmermann]

Commit 6046b49bafff ("drm/sysfb: Share helpers for integer validation")
and commit e8c086880b2b ("drm/sysfb: Share helpers for screen_info
validation") added duplicate function declarations. Remove the latter
ones.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: e8c086880b2b ("drm/sysfb: Share helpers for screen_info validation")
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Javier Martinez Canillas <javierm@redhat.com>
Cc: dri-devel@lists.freedesktop.org
Cc: <stable@vger.kernel.org> # v6.16+
---
 drivers/gpu/drm/sysfb/drm_sysfb_helper.h | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/sysfb/drm_sysfb_helper.h b/drivers/gpu/drm/sysfb/drm_sysfb_helper.h
index da670d7eeb2e..de96bfe7562c 100644
--- a/drivers/gpu/drm/sysfb/drm_sysfb_helper.h
+++ b/drivers/gpu/drm/sysfb/drm_sysfb_helper.h
@@ -54,15 +54,6 @@ const struct drm_format_info *drm_sysfb_get_format_si(struct drm_device *dev,
 						      const struct screen_info *si);
 #endif
 
-/*
- * Input parsing
- */
-
-int drm_sysfb_get_validated_int(struct drm_device *dev, const char *name,
-				u64 value, u32 max);
-int drm_sysfb_get_validated_int0(struct drm_device *dev, const char *name,
-				 u64 value, u32 max);
-
 /*
  * Display modes
  */
-- 
2.52.0

[PATCH 7/8] drm/sysfb: Generalize pixel-format matching

[Thomas Zimmermann]

Provide drm_sysfb_get_format(), a helper that finds a specific DRM
format from a list of pixel formats. The new function builds upon
drm_sysfb_get_format_si(), which finds the DRM format from a given
instance of struct screen_info. Now get the screen_info's pixel format
in the caller. Allows for matching pixel formats in drivers without
screen_info.

Convert the callers in efidrm and vesadrm to the new interface.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/sysfb/drm_sysfb.c             | 24 +++++++++++++++
 drivers/gpu/drm/sysfb/drm_sysfb_helper.h      |  8 ++---
 drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c | 30 -------------------
 drivers/gpu/drm/sysfb/efidrm.c                |  8 ++++-
 drivers/gpu/drm/sysfb/vesadrm.c               |  8 ++++-
 5 files changed, 42 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/sysfb/drm_sysfb.c b/drivers/gpu/drm/sysfb/drm_sysfb.c
index 308f82153b15..fbfb37d0fae1 100644
--- a/drivers/gpu/drm/sysfb/drm_sysfb.c
+++ b/drivers/gpu/drm/sysfb/drm_sysfb.c
@@ -31,5 +31,29 @@ int drm_sysfb_get_validated_int0(struct drm_device *dev, const char *name,
 }
 EXPORT_SYMBOL(drm_sysfb_get_validated_int0);
 
+const struct drm_format_info *drm_sysfb_get_format(struct drm_device *dev,
+						   const struct drm_sysfb_format *formats,
+						   size_t nformats,
+						   const struct pixel_format *pixel)
+{
+	const struct drm_format_info *format = NULL;
+	size_t i;
+
+	for (i = 0; i < nformats; ++i) {
+		const struct drm_sysfb_format *f = &formats[i];
+
+		if (pixel_format_equal(pixel, &f->pixel)) {
+			format = drm_format_info(f->fourcc);
+			break;
+		}
+	}
+
+	if (!format)
+		drm_warn(dev, "No compatible color format found\n");
+
+	return format;
+}
+EXPORT_SYMBOL(drm_sysfb_get_format);
+
 MODULE_DESCRIPTION("Helpers for DRM sysfb drivers");
 MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/sysfb/drm_sysfb_helper.h b/drivers/gpu/drm/sysfb/drm_sysfb_helper.h
index de96bfe7562c..b14df5b54bc9 100644
--- a/drivers/gpu/drm/sysfb/drm_sysfb_helper.h
+++ b/drivers/gpu/drm/sysfb/drm_sysfb_helper.h
@@ -36,6 +36,10 @@ int drm_sysfb_get_validated_int(struct drm_device *dev, const char *name,
 				u64 value, u32 max);
 int drm_sysfb_get_validated_int0(struct drm_device *dev, const char *name,
 				 u64 value, u32 max);
+const struct drm_format_info *drm_sysfb_get_format(struct drm_device *dev,
+						   const struct drm_sysfb_format *formats,
+						   size_t nformats,
+						   const struct pixel_format *pixel);
 
 #if defined(CONFIG_SCREEN_INFO)
 int drm_sysfb_get_width_si(struct drm_device *dev, const struct screen_info *si);
@@ -48,10 +52,6 @@ int drm_sysfb_get_stride_si(struct drm_device *dev, const struct screen_info *si
 			    unsigned int width, unsigned int height, u64 size);
 u64 drm_sysfb_get_visible_size_si(struct drm_device *dev, const struct screen_info *si,
 				  unsigned int height, unsigned int stride, u64 size);
-const struct drm_format_info *drm_sysfb_get_format_si(struct drm_device *dev,
-						      const struct drm_sysfb_format *formats,
-						      size_t nformats,
-						      const struct screen_info *si);
 #endif
 
 /*
diff --git a/drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c b/drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c
index 885864168c54..749290196c6a 100644
--- a/drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c
+++ b/drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c
@@ -72,33 +72,3 @@ u64 drm_sysfb_get_visible_size_si(struct drm_device *dev, const struct screen_in
 	return drm_sysfb_get_validated_size0(dev, "visible size", vsize, size);
 }
 EXPORT_SYMBOL(drm_sysfb_get_visible_size_si);
-
-const struct drm_format_info *drm_sysfb_get_format_si(struct drm_device *dev,
-						      const struct drm_sysfb_format *formats,
-						      size_t nformats,
-						      const struct screen_info *si)
-{
-	const struct drm_format_info *format = NULL;
-	struct pixel_format pixel;
-	size_t i;
-	int ret;
-
-	ret = screen_info_pixel_format(si, &pixel);
-	if (ret)
-		return NULL;
-
-	for (i = 0; i < nformats; ++i) {
-		const struct drm_sysfb_format *f = &formats[i];
-
-		if (pixel_format_equal(&pixel, &f->pixel)) {
-			format = drm_format_info(f->fourcc);
-			break;
-		}
-	}
-
-	if (!format)
-		drm_warn(dev, "No compatible color format found\n");
-
-	return format;
-}
-EXPORT_SYMBOL(drm_sysfb_get_format_si);
diff --git a/drivers/gpu/drm/sysfb/efidrm.c b/drivers/gpu/drm/sysfb/efidrm.c
index 1b683d55d6ea..ee525f330441 100644
--- a/drivers/gpu/drm/sysfb/efidrm.c
+++ b/drivers/gpu/drm/sysfb/efidrm.c
@@ -45,8 +45,14 @@ static const struct drm_format_info *efidrm_get_format_si(struct drm_device *dev
 		{ PIXEL_FORMAT_XBGR8888, DRM_FORMAT_XBGR8888, },
 		{ PIXEL_FORMAT_XRGB2101010, DRM_FORMAT_XRGB2101010, },
 	};
+	struct pixel_format pixel;
+	int ret;
+
+	ret = screen_info_pixel_format(si, &pixel);
+	if (ret)
+		return NULL;
 
-	return drm_sysfb_get_format_si(dev, formats, ARRAY_SIZE(formats), si);
+	return drm_sysfb_get_format(dev, formats, ARRAY_SIZE(formats), &pixel);
 }
 
 static u64 efidrm_get_mem_flags(struct drm_device *dev, resource_size_t start,
diff --git a/drivers/gpu/drm/sysfb/vesadrm.c b/drivers/gpu/drm/sysfb/vesadrm.c
index 7b7b5ba26317..b8c16ae3faad 100644
--- a/drivers/gpu/drm/sysfb/vesadrm.c
+++ b/drivers/gpu/drm/sysfb/vesadrm.c
@@ -49,8 +49,14 @@ static const struct drm_format_info *vesadrm_get_format_si(struct drm_device *de
 		{ PIXEL_FORMAT_XBGR8888, DRM_FORMAT_XBGR8888, },
 		{ PIXEL_FORMAT_C8, DRM_FORMAT_C8, },
 	};
+	struct pixel_format pixel;
+	int ret;
+
+	ret = screen_info_pixel_format(si, &pixel);
+	if (ret)
+		return NULL;
 
-	return drm_sysfb_get_format_si(dev, formats, ARRAY_SIZE(formats), si);
+	return drm_sysfb_get_format(dev, formats, ARRAY_SIZE(formats), &pixel);
 }
 
 /*
-- 
2.52.0

[PATCH 8/8] drm/sysfb: corebootdrm: Add DRM driver for coreboot framebuffers

[Thomas Zimmermann]

Add corebotdrm, a DRM driver for coreboot framebuffers. The driver
supports a pre-initialized framebuffer with various packed RGB formats.
The driver code is fairly small and uses the same logic as the other
sysfb drivers. Most of the implementation comes from existing sysfb
helpers.

Until now, coreboot relied on simpledrm or simplefb for boot-up graphics
output. Removing either leaves a dangling pointer in framebuffer_remove()
after the native graphics driver replaced the sysfb driver. Using the
new corebootdrm driver resolves this problem.

The old coreboot framebuffer code can now only selected if corebootdrm
has been disabled. Using corebootdrm is preferred.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/firmware/google/Kconfig     |   1 +
 drivers/gpu/drm/sysfb/Kconfig       |  16 ++
 drivers/gpu/drm/sysfb/Makefile      |   1 +
 drivers/gpu/drm/sysfb/corebootdrm.c | 402 ++++++++++++++++++++++++++++
 4 files changed, 420 insertions(+)
 create mode 100644 drivers/gpu/drm/sysfb/corebootdrm.c

diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
index 41b78f5cb735..b306a0f56061 100644
--- a/drivers/firmware/google/Kconfig
+++ b/drivers/firmware/google/Kconfig
@@ -60,6 +60,7 @@ config GOOGLE_MEMCONSOLE_X86_LEGACY
 config GOOGLE_FRAMEBUFFER_COREBOOT
 	tristate "Coreboot Framebuffer"
 	depends on FB_SIMPLE || DRM_SIMPLEDRM
+	depends on DRM_COREBOOTDRM=n
 	depends on GOOGLE_COREBOOT_TABLE
 	help
 	  This option enables the kernel to search for a framebuffer in
diff --git a/drivers/gpu/drm/sysfb/Kconfig b/drivers/gpu/drm/sysfb/Kconfig
index 9c9884c7efc6..e12be823e32b 100644
--- a/drivers/gpu/drm/sysfb/Kconfig
+++ b/drivers/gpu/drm/sysfb/Kconfig
@@ -7,6 +7,22 @@ config DRM_SYSFB_HELPER
 	tristate
 	depends on DRM
 
+config DRM_COREBOOTDRM
+	tristate "Coreboot framebuffer driver"
+	depends on DRM && MMU
+	depends on GOOGLE_COREBOOT_TABLE
+	select APERTURE_HELPERS
+	select DRM_CLIENT_SELECTION
+	select DRM_GEM_SHMEM_HELPER
+	select DRM_KMS_HELPER
+	select DRM_SYSFB_HELPER
+	help
+	  DRM driver for coreboot-provided framebuffers.
+
+	  This driver assumes that the display hardware has been initialized
+	  by coreboot firmware before the kernel boots. Scanout buffer, size,
+	  and display format must be provided via coreboot framebuffer device.
+
 config DRM_EFIDRM
 	tristate "EFI framebuffer driver"
 	depends on DRM && MMU && EFI && (!SYSFB_SIMPLEFB || COMPILE_TEST)
diff --git a/drivers/gpu/drm/sysfb/Makefile b/drivers/gpu/drm/sysfb/Makefile
index a156c496413d..85c9087ab03d 100644
--- a/drivers/gpu/drm/sysfb/Makefile
+++ b/drivers/gpu/drm/sysfb/Makefile
@@ -6,6 +6,7 @@ drm_sysfb_helper-y := \
 drm_sysfb_helper-$(CONFIG_SCREEN_INFO) += drm_sysfb_screen_info.o
 obj-$(CONFIG_DRM_SYSFB_HELPER)	+= drm_sysfb_helper.o
 
+obj-$(CONFIG_DRM_COREBOOTDRM)	+= corebootdrm.o
 obj-$(CONFIG_DRM_EFIDRM)	+= efidrm.o
 obj-$(CONFIG_DRM_OFDRM)		+= ofdrm.o
 obj-$(CONFIG_DRM_SIMPLEDRM)	+= simpledrm.o
diff --git a/drivers/gpu/drm/sysfb/corebootdrm.c b/drivers/gpu/drm/sysfb/corebootdrm.c
new file mode 100644
index 000000000000..242e668c98f1
--- /dev/null
+++ b/drivers/gpu/drm/sysfb/corebootdrm.c
@@ -0,0 +1,402 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/aperture.h>
+#include <linux/coreboot.h>
+#include <linux/minmax.h>
+
+#include <drm/clients/drm_client_setup.h>
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_state_helper.h>
+#include <drm/drm_connector.h>
+#include <drm/drm_damage_helper.h>
+#include <drm/drm_device.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_fbdev_shmem.h>
+#include <drm/drm_framebuffer.h>
+#include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_gem_shmem_helper.h>
+#include <drm/drm_managed.h>
+#include <drm/drm_modeset_helper_vtables.h>
+#include <drm/drm_print.h>
+#include <drm/drm_probe_helper.h>
+
+#include "drm_sysfb_helper.h"
+
+#define DRIVER_NAME	"corebootdrm"
+#define DRIVER_DESC	"DRM driver for Coreboot framebuffers"
+#define DRIVER_MAJOR	1
+#define DRIVER_MINOR	0
+
+static const struct drm_format_info *
+corebootdrm_get_format_fb(struct drm_device *dev, const struct lb_framebuffer *fb)
+{
+	static const struct drm_sysfb_format formats[] = {
+		{ PIXEL_FORMAT_XRGB1555, DRM_FORMAT_XRGB1555, },
+		{ PIXEL_FORMAT_RGB565, DRM_FORMAT_RGB565, },
+		{ PIXEL_FORMAT_RGB888, DRM_FORMAT_RGB888, },
+		{ PIXEL_FORMAT_XRGB8888, DRM_FORMAT_XRGB8888, },
+		{ PIXEL_FORMAT_XBGR8888, DRM_FORMAT_XBGR8888, },
+		{ PIXEL_FORMAT_XRGB2101010, DRM_FORMAT_XRGB2101010, },
+	};
+	const struct pixel_format pixel = {
+		.bits_per_pixel = fb->bits_per_pixel,
+		.indexed  = false,
+		.alpha = {
+			.offset = 0,
+			.length = 0,
+		},
+		.red = {
+			.offset = fb->red_mask_pos,
+			.length = fb->red_mask_size,
+		},
+		.green = {
+			.offset = fb->green_mask_pos,
+			.length = fb->green_mask_size,
+		},
+		.blue = {
+			.offset = fb->blue_mask_pos,
+			.length = fb->blue_mask_size,
+		},
+	};
+
+	return drm_sysfb_get_format(dev, formats, ARRAY_SIZE(formats), &pixel);
+}
+
+static int corebootdrm_get_width_fb(struct drm_device *dev, const struct lb_framebuffer *fb)
+{
+	return drm_sysfb_get_validated_int0(dev, "width", fb->x_resolution, INT_MAX);
+}
+
+static int corebootdrm_get_height_fb(struct drm_device *dev, const struct lb_framebuffer *fb)
+{
+	return drm_sysfb_get_validated_int0(dev, "height", fb->y_resolution, INT_MAX);
+}
+
+static int corebootdrm_get_pitch_fb(struct drm_device *dev, const struct drm_format_info *format,
+				    unsigned int width, const struct lb_framebuffer *fb)
+{
+	u64 bytes_per_line = fb->bytes_per_line;
+
+	if (!bytes_per_line)
+		bytes_per_line = drm_format_info_min_pitch(format, 0, width);
+
+	return drm_sysfb_get_validated_int0(dev, "pitch", bytes_per_line, INT_MAX);
+}
+
+static u64 corebootdrm_get_address_fb(struct drm_device *dev, unsigned int height,
+				      unsigned int pitch, const struct lb_framebuffer *fb)
+{
+	u64 visible_size, size;
+
+	if (!fb->physical_address)
+		return 0;
+
+	if (check_mul_overflow(height, pitch, &visible_size))
+		return 0;
+	size = PAGE_ALIGN(visible_size);
+
+	if (fb->physical_address > (U64_MAX - size))
+		return 0;
+
+	return fb->physical_address;
+}
+
+/*
+ * Simple Framebuffer device
+ */
+
+struct corebootdrm_device {
+	struct drm_sysfb_device sysfb;
+
+	/* modesetting */
+	u32 formats[DRM_SYSFB_PLANE_NFORMATS(1)];
+	struct drm_plane primary_plane;
+	struct drm_crtc crtc;
+	struct drm_encoder encoder;
+	struct drm_connector connector;
+};
+
+/*
+ * Modesetting
+ */
+
+static const u64 corebootdrm_primary_plane_format_modifiers[] = {
+	DRM_SYSFB_PLANE_FORMAT_MODIFIERS,
+};
+
+static const struct drm_plane_helper_funcs corebootdrm_primary_plane_helper_funcs = {
+	DRM_SYSFB_PLANE_HELPER_FUNCS,
+};
+
+static const struct drm_plane_funcs corebootdrm_primary_plane_funcs = {
+	DRM_SYSFB_PLANE_FUNCS,
+	.destroy = drm_plane_cleanup,
+};
+
+static const struct drm_crtc_helper_funcs corebootdrm_crtc_helper_funcs = {
+	DRM_SYSFB_CRTC_HELPER_FUNCS,
+};
+
+static const struct drm_crtc_funcs corebootdrm_crtc_funcs = {
+	DRM_SYSFB_CRTC_FUNCS,
+	.destroy = drm_crtc_cleanup,
+};
+
+static const struct drm_encoder_funcs corebootdrm_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
+static const struct drm_connector_helper_funcs corebootdrm_connector_helper_funcs = {
+	DRM_SYSFB_CONNECTOR_HELPER_FUNCS,
+};
+
+static const struct drm_connector_funcs corebootdrm_connector_funcs = {
+	DRM_SYSFB_CONNECTOR_FUNCS,
+	.destroy = drm_connector_cleanup,
+};
+
+static const struct drm_mode_config_funcs corebootdrm_mode_config_funcs = {
+	DRM_SYSFB_MODE_CONFIG_FUNCS,
+};
+
+/*
+ * Init / Cleanup
+ */
+
+static int corebootdrm_device_init(struct corebootdrm_device *cdev, struct coreboot_device *cbdev)
+{
+	const struct lb_framebuffer *fb = &cbdev->framebuffer;
+	struct drm_sysfb_device *sysfb = &cdev->sysfb;
+	struct drm_device *dev = &sysfb->dev;
+	const struct drm_format_info *format;
+	int width, height, pitch;
+	u64 address;
+	int width_mm = 0, height_mm = 0;
+	resource_size_t fb_size, fb_base, fb_pgbase, fb_pgsize;
+	struct resource *res, *mem = NULL;
+	void __iomem *screen_base;
+	struct drm_plane *primary_plane;
+	struct drm_crtc *crtc;
+	struct drm_encoder *encoder;
+	struct drm_connector *connector;
+	unsigned long max_width, max_height;
+	size_t nformats;
+	int ret;
+
+	/*
+	 * Hardware settings
+	 */
+
+	format = corebootdrm_get_format_fb(dev, fb);
+	if (!format)
+		return -EINVAL;
+	width = corebootdrm_get_width_fb(dev, fb);
+	if (width < 0)
+		return width;
+	height = corebootdrm_get_height_fb(dev, fb);
+	if (height < 0)
+		return height;
+	pitch = corebootdrm_get_pitch_fb(dev, format, width, fb);
+	if (pitch < 0)
+		return pitch;
+	address = corebootdrm_get_address_fb(dev, height, pitch, fb);
+	if (!address)
+		return -EINVAL;
+
+	sysfb->fb_mode = drm_sysfb_mode(width, height, width_mm, height_mm);
+	sysfb->fb_format = format;
+	sysfb->fb_pitch = pitch;
+
+	drm_dbg(dev, "display mode={" DRM_MODE_FMT "}\n", DRM_MODE_ARG(&sysfb->fb_mode));
+	drm_dbg(dev, "framebuffer format=%p4cc, size=%dx%d, pitch=%d byte\n",
+		&format->format, width, height, pitch);
+
+	/*
+	 * Memory management
+	 */
+
+	fb_base = address;
+	fb_size = pitch * height;
+
+	fb_pgbase = round_down(fb_base, PAGE_SIZE);
+	fb_pgsize = fb_base - fb_pgbase + round_up(fb_size, PAGE_SIZE);
+
+	ret = devm_aperture_acquire_for_coreboot_device(cbdev, fb_pgbase, fb_pgsize);
+	if (ret) {
+		drm_err(dev, "could not acquire memory range %pr: %d\n", res, ret);
+		return ret;
+	}
+
+	drm_dbg(dev, "using I/O memory framebuffer at %pr\n", res);
+
+	mem = devm_request_mem_region(&cbdev->dev, fb_pgbase, fb_pgsize, dev_name(&cbdev->dev));
+	if (!mem) {
+		/*
+		 * We cannot make this fatal. Sometimes this comes from magic
+		 * spaces our resource handlers simply don't know about. Use
+		 * the I/O-memory resource as-is and try to map that instead.
+		 */
+		drm_warn(dev, "could not acquire memory region %pr\n", res);
+		mem = res;
+	}
+
+	screen_base = devm_ioremap_wc(&cbdev->dev, fb_pgbase, fb_pgsize);
+	if (!screen_base)
+		return -ENOMEM;
+
+	iosys_map_set_vaddr_iomem(&sysfb->fb_addr, screen_base);
+
+	/*
+	 * Modesetting
+	 */
+
+	ret = drmm_mode_config_init(dev);
+	if (ret)
+		return ret;
+
+	max_width = max_t(unsigned long, width, DRM_SHADOW_PLANE_MAX_WIDTH);
+	max_height = max_t(unsigned long, height, DRM_SHADOW_PLANE_MAX_HEIGHT);
+
+	dev->mode_config.min_width = width;
+	dev->mode_config.max_width = max_width;
+	dev->mode_config.min_height = height;
+	dev->mode_config.max_height = max_height;
+	dev->mode_config.preferred_depth = format->depth;
+	dev->mode_config.funcs = &corebootdrm_mode_config_funcs;
+
+	/* Primary plane */
+
+	nformats = drm_sysfb_build_fourcc_list(dev, &format->format, 1,
+					       cdev->formats, ARRAY_SIZE(cdev->formats));
+
+	primary_plane = &cdev->primary_plane;
+	ret = drm_universal_plane_init(dev, primary_plane, 0, &corebootdrm_primary_plane_funcs,
+				       cdev->formats, nformats,
+				       corebootdrm_primary_plane_format_modifiers,
+				       DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (ret)
+		return ret;
+	drm_plane_helper_add(primary_plane, &corebootdrm_primary_plane_helper_funcs);
+	drm_plane_enable_fb_damage_clips(primary_plane);
+
+	/* CRTC */
+
+	crtc = &cdev->crtc;
+	ret = drm_crtc_init_with_planes(dev, crtc, primary_plane, NULL,
+					&corebootdrm_crtc_funcs, NULL);
+	if (ret)
+		return ret;
+	drm_crtc_helper_add(crtc, &corebootdrm_crtc_helper_funcs);
+
+	/* Encoder */
+
+	encoder = &cdev->encoder;
+	ret = drm_encoder_init(dev, encoder, &corebootdrm_encoder_funcs,
+			       DRM_MODE_ENCODER_NONE, NULL);
+	if (ret)
+		return ret;
+	encoder->possible_crtcs = drm_crtc_mask(crtc);
+
+	/* Connector */
+
+	connector = &cdev->connector;
+	ret = drm_connector_init(dev, connector, &corebootdrm_connector_funcs,
+				 DRM_MODE_CONNECTOR_Unknown);
+	if (ret)
+		return ret;
+	drm_connector_helper_add(connector, &corebootdrm_connector_helper_funcs);
+	drm_connector_set_panel_orientation_with_quirk(connector,
+						       DRM_MODE_PANEL_ORIENTATION_UNKNOWN,
+						       width, height);
+
+	ret = drm_connector_attach_encoder(connector, encoder);
+	if (ret)
+		return ret;
+
+	drm_mode_config_reset(dev);
+
+	return 0;
+}
+
+/*
+ * DRM driver
+ */
+
+DEFINE_DRM_GEM_FOPS(corebootdrm_fops);
+
+static struct drm_driver corebootdrm_drm_driver = {
+	DRM_GEM_SHMEM_DRIVER_OPS,
+	DRM_FBDEV_SHMEM_DRIVER_OPS,
+	.name			= DRIVER_NAME,
+	.desc			= DRIVER_DESC,
+	.major			= DRIVER_MAJOR,
+	.minor			= DRIVER_MINOR,
+	.driver_features	= DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET,
+	.fops			= &corebootdrm_fops,
+};
+
+/*
+ * Coreboot driver
+ */
+
+static int corebootdrm_probe(struct coreboot_device *cbdev)
+{
+	const struct lb_framebuffer *fb = &cbdev->framebuffer;
+	struct corebootdrm_device *cdev;
+	struct drm_sysfb_device *sysfb;
+	struct drm_device *dev;
+	int ret;
+
+	if (fb->size < sizeof(*fb)) {
+		drm_err(dev, "coreboot framebuffer entry too small\n");
+		return -EINVAL;
+	}
+
+	cdev = devm_drm_dev_alloc(&cbdev->dev, &corebootdrm_drm_driver,
+				  struct corebootdrm_device, sysfb.dev);
+	if (IS_ERR(cdev))
+		return PTR_ERR(cdev);
+	dev_set_drvdata(&cbdev->dev, cdev);
+
+	ret = corebootdrm_device_init(cdev, cbdev);
+	if (ret)
+		return ret;
+	sysfb = &cdev->sysfb;
+	dev = &sysfb->dev;
+
+	ret = drm_dev_register(dev, 0);
+	if (ret)
+		return ret;
+
+	drm_client_setup(dev, sysfb->fb_format);
+
+	return 0;
+}
+
+static void corebootdrm_remove(struct coreboot_device *cbdev)
+{
+	struct corebootdrm_device *cdev = dev_get_drvdata(&cbdev->dev);
+	struct drm_device *dev = &cdev->sysfb.dev;
+
+	drm_dev_unplug(dev);
+}
+
+static const struct coreboot_device_id corebootdrm_id_table[] = {
+	{ .tag = CB_TAG_FRAMEBUFFER },
+	{ },
+};
+MODULE_DEVICE_TABLE(coreboot, corebootdrm_id_table);
+
+static struct coreboot_driver corebootdrm_driver = {
+	.probe = corebootdrm_probe,
+	.remove = corebootdrm_remove,
+	.drv = {
+		.name = "corebootdrm",
+	},
+	.id_table = corebootdrm_id_table,
+};
+module_coreboot_driver(corebootdrm_driver);
+
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL");
-- 
2.52.0

Re: [PATCH 1/8] firmware: google: Do sysfb test before creating coreboot framebuffer

[Julius Werner]

This seems less consistent to me, although tbh I don't understand the
Linux device and driver framework that well. I think the original idea
here was that the coreboot bus represents the coreboot table, and each
entry is represented as one device (regardless of whether any driver
actually wants to do anything with that entry). That's why we're
creating a device for all the tags even though most of them aren't
really interesting for kernel drivers. This also helps with inspecting
things in sysfs.

So the device with TAG_FRAMEBUFFER doesn't mean that we have a
framebuffer, it just means that we have an entry in the table
describing a framebuffer. Whether it should actually be used to set up
a framebuffer should be up to the binding driver. That's why I think
keeping this decision in the driver probe makes more sense, and
excluding it from the devices on the bus is weird (because you're just
randomly excluding one of the entries in the table from being
represented like the others, just because of details about how its
drivers would want to use it).

If you want these devices to be bound by drivers outside this
directory, then I think either that other driver needs to have the
logic to decide when a coreboot framebuffer should actually be used,
or maybe you should have a small stub driver in this directory that
binds to the list entry device, makes the decision whether to actually
use it, and if so creates a sub-device or something (if that's
possible?) which the actual outside driver can find and bind to.

On Thu, Jan 8, 2026 at 3:51 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Test sysfb before creating the coreboot framebuffer device. Skip
> device creation if the test fails, as this framebuffer does not exist.
>
> Depending on the system setup, the initial framebuffer can be provided
> by the boot loader via screen_info boot parameters and handled by the
> kernel's sysfb code in drivers/firmware/sysfb.c. With the sysfb test in
> the coreboot-framebuffer probing, the coreboot device is present without
> the framebuffer. Even after the sysfb device has been replaced with a
> native PCI device, the coreboot device persists.
>
> Skipping device creation early avoids all these inconsistencies. It
> further prepares coreboot to support graphics drivers besides the one
> in framebuffer-coreboot.c.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/firmware/google/coreboot_table.c       | 17 +++++++++++++++++
>  drivers/firmware/google/coreboot_table.h       |  1 +
>  drivers/firmware/google/framebuffer-coreboot.c | 16 ----------------
>  3 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c
> index 882db32e51be..c34426e5002d 100644
> --- a/drivers/firmware/google/coreboot_table.c
> +++ b/drivers/firmware/google/coreboot_table.c
> @@ -18,6 +18,7 @@
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
> +#include <linux/sysfb.h>
>
>  #include "coreboot_table.h"
>
> @@ -118,6 +119,22 @@ static int coreboot_table_populate(struct device *dev, void *ptr)
>                         return -EINVAL;
>                 }
>
> +               switch (entry->tag) {
> +               case CB_TAG_FRAMEBUFFER:
> +                       /*
> +                        * On coreboot systems, the advertised CB_TAG_FRAMEBUFFER entry
> +                        * in the coreboot table should only be used if the payload did
> +                        * not pass a framebuffer information to the Linux kernel.
> +                        *
> +                        * If the global screen_info data has been filled, the generic
> +                        * system framebuffers (sysfb) will already register a platform
> +                        * device and pass that screen_info as platform_data to a driver
> +                        * that can scan-out using the system-provided framebuffer.
> +                        */
> +                       if (sysfb_handles_screen_info())
> +                               continue;
> +               }
> +
>                 device = kzalloc(sizeof(device->dev) + entry->size, GFP_KERNEL);
>                 if (!device)
>                         return -ENOMEM;
> diff --git a/drivers/firmware/google/coreboot_table.h b/drivers/firmware/google/coreboot_table.h
> index bb6f0f7299b4..e3c353676940 100644
> --- a/drivers/firmware/google/coreboot_table.h
> +++ b/drivers/firmware/google/coreboot_table.h
> @@ -40,6 +40,7 @@ struct lb_cbmem_ref {
>         u64 cbmem_addr;
>  };
>
> +#define CB_TAG_FRAMEBUFFER 0x12
>  #define LB_TAG_CBMEM_ENTRY 0x31
>
>  /* Corresponds to LB_TAG_CBMEM_ENTRY */
> diff --git a/drivers/firmware/google/framebuffer-coreboot.c b/drivers/firmware/google/framebuffer-coreboot.c
> index c68c9f56370f..bb53d1a47409 100644
> --- a/drivers/firmware/google/framebuffer-coreboot.c
> +++ b/drivers/firmware/google/framebuffer-coreboot.c
> @@ -15,12 +15,9 @@
>  #include <linux/module.h>
>  #include <linux/platform_data/simplefb.h>
>  #include <linux/platform_device.h>
> -#include <linux/sysfb.h>
>
>  #include "coreboot_table.h"
>
> -#define CB_TAG_FRAMEBUFFER 0x12
> -
>  static const struct simplefb_format formats[] = SIMPLEFB_FORMATS;
>
>  static int framebuffer_probe(struct coreboot_device *dev)
> @@ -37,19 +34,6 @@ static int framebuffer_probe(struct coreboot_device *dev)
>                 .format = NULL,
>         };
>
> -       /*
> -        * On coreboot systems, the advertised LB_TAG_FRAMEBUFFER entry
> -        * in the coreboot table should only be used if the payload did
> -        * not pass a framebuffer information to the Linux kernel.
> -        *
> -        * If the global screen_info data has been filled, the Generic
> -        * System Framebuffers (sysfb) will already register a platform
> -        * device and pass that screen_info as platform_data to a driver
> -        * that can scan-out using the system provided framebuffer.
> -        */
> -       if (sysfb_handles_screen_info())
> -               return -ENODEV;
> -
>         if (!fb->physical_address)
>                 return -ENODEV;
>
> --
> 2.52.0
>

Re: [PATCH 0/8] drm, coreboot: Add DRM coreboot driver

[Brian Norris]

On Thu, Jan 08, 2026 at 03:19:40PM +0100, Thomas Zimmermann wrote:
> Coreboot implements framebuffer support via simplefb. Provide a
> native DRM driver. Keep the simplefb code for now.

I'm not much of a DRM-er, but what's blocking us from just replacing the
simplefb driver with DRM completely? Just being conservative and
allowing flexibility? Or are there technical reasons this wouldn't be a
proper replacement? It sounds like supporting 2 drivers provides at
least some small complications, like in patch 1, where you're trying to
avoid repeating similar logic in 2 framebuffer-handling drivers.

Brian

Re: [PATCH 0/8] drm, coreboot: Add DRM coreboot driver

[Thomas Zimmermann]

Hi

Am 08.01.26 um 19:10 schrieb Brian Norris:
> On Thu, Jan 08, 2026 at 03:19:40PM +0100, Thomas Zimmermann wrote:
>> Coreboot implements framebuffer support via simplefb. Provide a
>> native DRM driver. Keep the simplefb code for now.
> I'm not much of a DRM-er, but what's blocking us from just replacing the
> simplefb driver with DRM completely? Just being conservative and
> allowing flexibility? Or are there technical reasons this wouldn't be a
> proper replacement? It sounds like supporting 2 drivers provides at
> least some small complications, like in patch 1, where you're trying to
> avoid repeating similar logic in 2 framebuffer-handling drivers.

Yes, that's true.

There are currently two drivers that bind to the created 
simple-framebuffer: fbdev's simplefb and DRM's simpledrm. The new 
corebootdrm intents to replace simpledrm. And simplefb is deprecated: if 
no one uses that any longer, I'm all for removing coreboot's current 
framebuffer handling.

Best regards
Thomas

>
> Brian

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Re: [PATCH 1/8] firmware: google: Do sysfb test before creating coreboot framebuffer

[Thomas Zimmermann]

Hi

Am 08.01.26 um 17:55 schrieb Julius Werner:
> This seems less consistent to me, although tbh I don't understand the
> Linux device and driver framework that well. I think the original idea
> here was that the coreboot bus represents the coreboot table, and each
> entry is represented as one device (regardless of whether any driver
> actually wants to do anything with that entry). That's why we're
> creating a device for all the tags even though most of them aren't
> really interesting for kernel drivers. This also helps with inspecting
> things in sysfs.

We need to distinguish between coreboot and sysfb. Only one of these 
subsystems can handle the framebuffer. Having a framebuffer device on 
the coreboot bus, if the underlying framebuffer is managed by a 
different device seems even more incorrect.

>
> So the device with TAG_FRAMEBUFFER doesn't mean that we have a
> framebuffer, it just means that we have an entry in the table
> describing a framebuffer. Whether it should actually be used to set up
> a framebuffer should be up to the binding driver. That's why I think
> keeping this decision in the driver probe makes more sense, and
> excluding it from the devices on the bus is weird (because you're just
> randomly excluding one of the entries in the table from being
> represented like the others, just because of details about how its
> drivers would want to use it).
>
> If you want these devices to be bound by drivers outside this
> directory, then I think either that other driver needs to have the
> logic to decide when a coreboot framebuffer should actually be used,
> or maybe you should have a small stub driver in this directory that
> binds to the list entry device, makes the decision whether to actually
> use it, and if so creates a sub-device or something (if that's
> possible?) which the actual outside driver can find and bind to.

This is what the current code in framebuffer-coreboot.c does. It binds 
to the coreboot-framebuffer device and create another device for 
external DRM/fbdev drivers to handle. This is problematic, as

1) the additional device will be gone after the native hardware drivers 
takes over the display, so the pdev pointer at [1] is dangling. 
Apparently no one ever unloads the coreboot-framebuffer device to 
trigger this problem.

2) The corboot-framebuffer device sets itself as the external device's 
parent. This is incorrect. The framebuffer exists on top of a PCI 
graphics device, so that device should be the parent. Otherwise the user 
can hot-unplug the PCI hardware and the coreboot framebuffer operates on 
a bogus graphics aperture. Modeling the parent-child relationships 
correctly, avoids this problem.  We've seen these problems with 
UEFI/VESA framebuffers and fixed it accordingly. [2] Something similar 
should be done for coreboot. Coreboot devices can still be located on 
the coreboot bus.

[1] 
https://elixir.bootlin.com/linux/v6.18.4/source/drivers/firmware/google/framebuffer-coreboot.c#L92
[2] 
https://elixir.bootlin.com/linux/v6.18.4/source/drivers/firmware/sysfb.c#L160

Another question I have is why there's a separate device for the 
coreboot-table? Couldn't the kernel just create the coreboot bus and 
then populate it with the table entries? DT does that. If the 
coreboot-table device is really just the parent for the other deivce, 
that is incorrect, as I describe above.

The coreboot bus is only for convenience, I guess? Other subsystems 
(sysfb, DT) create platform devices directly.

Best regards
Thomas



>
> On Thu, Jan 8, 2026 at 3:51 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Test sysfb before creating the coreboot framebuffer device. Skip
>> device creation if the test fails, as this framebuffer does not exist.
>>
>> Depending on the system setup, the initial framebuffer can be provided
>> by the boot loader via screen_info boot parameters and handled by the
>> kernel's sysfb code in drivers/firmware/sysfb.c. With the sysfb test in
>> the coreboot-framebuffer probing, the coreboot device is present without
>> the framebuffer. Even after the sysfb device has been replaced with a
>> native PCI device, the coreboot device persists.
>>
>> Skipping device creation early avoids all these inconsistencies. It
>> further prepares coreboot to support graphics drivers besides the one
>> in framebuffer-coreboot.c.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/firmware/google/coreboot_table.c       | 17 +++++++++++++++++
>>   drivers/firmware/google/coreboot_table.h       |  1 +
>>   drivers/firmware/google/framebuffer-coreboot.c | 16 ----------------
>>   3 files changed, 18 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c
>> index 882db32e51be..c34426e5002d 100644
>> --- a/drivers/firmware/google/coreboot_table.c
>> +++ b/drivers/firmware/google/coreboot_table.c
>> @@ -18,6 +18,7 @@
>>   #include <linux/of.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/slab.h>
>> +#include <linux/sysfb.h>
>>
>>   #include "coreboot_table.h"
>>
>> @@ -118,6 +119,22 @@ static int coreboot_table_populate(struct device *dev, void *ptr)
>>                          return -EINVAL;
>>                  }
>>
>> +               switch (entry->tag) {
>> +               case CB_TAG_FRAMEBUFFER:
>> +                       /*
>> +                        * On coreboot systems, the advertised CB_TAG_FRAMEBUFFER entry
>> +                        * in the coreboot table should only be used if the payload did
>> +                        * not pass a framebuffer information to the Linux kernel.
>> +                        *
>> +                        * If the global screen_info data has been filled, the generic
>> +                        * system framebuffers (sysfb) will already register a platform
>> +                        * device and pass that screen_info as platform_data to a driver
>> +                        * that can scan-out using the system-provided framebuffer.
>> +                        */
>> +                       if (sysfb_handles_screen_info())
>> +                               continue;
>> +               }
>> +
>>                  device = kzalloc(sizeof(device->dev) + entry->size, GFP_KERNEL);
>>                  if (!device)
>>                          return -ENOMEM;
>> diff --git a/drivers/firmware/google/coreboot_table.h b/drivers/firmware/google/coreboot_table.h
>> index bb6f0f7299b4..e3c353676940 100644
>> --- a/drivers/firmware/google/coreboot_table.h
>> +++ b/drivers/firmware/google/coreboot_table.h
>> @@ -40,6 +40,7 @@ struct lb_cbmem_ref {
>>          u64 cbmem_addr;
>>   };
>>
>> +#define CB_TAG_FRAMEBUFFER 0x12
>>   #define LB_TAG_CBMEM_ENTRY 0x31
>>
>>   /* Corresponds to LB_TAG_CBMEM_ENTRY */
>> diff --git a/drivers/firmware/google/framebuffer-coreboot.c b/drivers/firmware/google/framebuffer-coreboot.c
>> index c68c9f56370f..bb53d1a47409 100644
>> --- a/drivers/firmware/google/framebuffer-coreboot.c
>> +++ b/drivers/firmware/google/framebuffer-coreboot.c
>> @@ -15,12 +15,9 @@
>>   #include <linux/module.h>
>>   #include <linux/platform_data/simplefb.h>
>>   #include <linux/platform_device.h>
>> -#include <linux/sysfb.h>
>>
>>   #include "coreboot_table.h"
>>
>> -#define CB_TAG_FRAMEBUFFER 0x12
>> -
>>   static const struct simplefb_format formats[] = SIMPLEFB_FORMATS;
>>
>>   static int framebuffer_probe(struct coreboot_device *dev)
>> @@ -37,19 +34,6 @@ static int framebuffer_probe(struct coreboot_device *dev)
>>                  .format = NULL,
>>          };
>>
>> -       /*
>> -        * On coreboot systems, the advertised LB_TAG_FRAMEBUFFER entry
>> -        * in the coreboot table should only be used if the payload did
>> -        * not pass a framebuffer information to the Linux kernel.
>> -        *
>> -        * If the global screen_info data has been filled, the Generic
>> -        * System Framebuffers (sysfb) will already register a platform
>> -        * device and pass that screen_info as platform_data to a driver
>> -        * that can scan-out using the system provided framebuffer.
>> -        */
>> -       if (sysfb_handles_screen_info())
>> -               return -ENODEV;
>> -
>>          if (!fb->physical_address)
>>                  return -ENODEV;
>>
>> --
>> 2.52.0
>>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Re: [PATCH 1/8] firmware: google: Do sysfb test before creating coreboot framebuffer

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

Hello Thomas and Julius,

> Hi
>
> Am 08.01.26 um 17:55 schrieb Julius Werner:
>> This seems less consistent to me, although tbh I don't understand the
>> Linux device and driver framework that well. I think the original idea
>> here was that the coreboot bus represents the coreboot table, and each
>> entry is represented as one device (regardless of whether any driver
>> actually wants to do anything with that entry). That's why we're
>> creating a device for all the tags even though most of them aren't
>> really interesting for kernel drivers. This also helps with inspecting
>> things in sysfs.
>
> We need to distinguish between coreboot and sysfb. Only one of these 
> subsystems can handle the framebuffer. Having a framebuffer device on 
> the coreboot bus, if the underlying framebuffer is managed by a 
> different device seems even more incorrect.
>

I'm not familiar with coreboot so please forgive me if something that I
say makes no sense but IMO what should be done is the following:

* coreboot_table_populate() should populate the bus and devices for all
  devices. That way the complete device hierarchy will be exposed in the
  sysfs entries as Julius mentioned.

* the check on whether the CB_TAG_FRAMEBUFFER should be used or not by the
  kernel should also be there (what Thomas did in patch 1/8) but instead
  of omiting registering the coreboot device in the bus, the device could
  be marked as "unused" or "disabled" (by adding a field to coreboot_device).

* the coreboot_bus_match() function then can take this new field into
  account and only match if (device->entry.tag == id->tag && !device->unused)
  or something like that.

That way no device will be removed from the bus and the corebootdrm driver
will only be probed when the device has to actually be used by the kernel.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 2/8] firmware: google: Init coreboot bus with subsys_initcall()

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

Hello Thomas,

> Using module_init()/device_initcall() is too late to initialize
> the coreboot bus, as there might already be drivers that depend
> on it.
>
> So far this hasn't been a problem, as coreboot controls all device
> creation. Initializing the coreboot bus earlier in subsys_initcall()
> will allow for external coreboot drivers to register themselves
> with device_initcall(). Prepares coreboot to support additional
> coreboot drivers from other subsystems.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/firmware/google/coreboot_table.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c
> index c34426e5002d..3bee22d2a7b3 100644
> --- a/drivers/firmware/google/coreboot_table.c
> +++ b/drivers/firmware/google/coreboot_table.c
> @@ -268,7 +268,7 @@ static void __exit coreboot_table_driver_exit(void)
>  	bus_unregister(&coreboot_bus_type);
>  }
>  
> -module_init(coreboot_table_driver_init);
> +subsys_initcall(coreboot_table_driver_init);
>  module_exit(coreboot_table_driver_exit);
>  
>  MODULE_AUTHOR("Google, Inc.");
> -- 
> 2.52.0
>

I agree and is also consistent to the init call level used by EFI, PSCI,
SCMI, DMI, etc.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 3/8] firmware: google: Clean up include statements in coreboot_table.h

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Include <linux/mod_devicetable.h> from source files and only forward-
> declare struct coreboot_device_id in coreboot_table.h.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 4/8] firmware: google: Export coreboot driver and device interfaces

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Split coreboot_table.h: move struct coreboot_table_header into
> coreboot_table.h and the rest of the header to include/linux/coreboot.h.
>
> Prepares coreboot for allowing drivers in other subsystems.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

I think that patches 2, 3 and 4 have merits on their own and could be
picked by the coreboot folks without the need to wait for discussions
about the other patches to settle.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 5/8] video/aperture: Support coreboot devices

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Add support for coreboot devices. Until now, all devices with system
> framebuffers were on the platform bus. On systems with a coreboot
> firmware, the system framebuffer can be represented as coreboot device
> on the coreboot bus. Handling the related graphics apertures requires
> new interfaces and a custom way of removing the coreboot device. The
> core aperture code is independent from the type of device.
>
> Also move around a comment that refers to both, platform and coreboot,
> devices.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

This patch also has merits on its own IMO. But it depends on patch 4, so I
don't know what the merge strategy could be here.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 6/8] drm/sysfb: Remove duplicate declarations

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Commit 6046b49bafff ("drm/sysfb: Share helpers for integer validation")
> and commit e8c086880b2b ("drm/sysfb: Share helpers for screen_info
> validation") added duplicate function declarations. Remove the latter
> ones.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: e8c086880b2b ("drm/sysfb: Share helpers for screen_info validation")
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Javier Martinez Canillas <javierm@redhat.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: <stable@vger.kernel.org> # v6.16+
> ---
>  drivers/gpu/drm/sysfb/drm_sysfb_helper.h | 9 ---------
>  1 file changed, 9 deletions(-)
>

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 7/8] drm/sysfb: Generalize pixel-format matching

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Provide drm_sysfb_get_format(), a helper that finds a specific DRM
> format from a list of pixel formats. The new function builds upon
> drm_sysfb_get_format_si(), which finds the DRM format from a given
> instance of struct screen_info. Now get the screen_info's pixel format
> in the caller. Allows for matching pixel formats in drivers without
> screen_info.
>
> Convert the callers in efidrm and vesadrm to the new interface.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

Again another patch that could just land and doesn't need to wait for the rest.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 0/8] drm, coreboot: Add DRM coreboot driver

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Hi
>
> Am 08.01.26 um 19:10 schrieb Brian Norris:
>> On Thu, Jan 08, 2026 at 03:19:40PM +0100, Thomas Zimmermann wrote:
>>> Coreboot implements framebuffer support via simplefb. Provide a
>>> native DRM driver. Keep the simplefb code for now.
>> I'm not much of a DRM-er, but what's blocking us from just replacing the
>> simplefb driver with DRM completely? Just being conservative and
>> allowing flexibility? Or are there technical reasons this wouldn't be a
>> proper replacement? It sounds like supporting 2 drivers provides at
>> least some small complications, like in patch 1, where you're trying to
>> avoid repeating similar logic in 2 framebuffer-handling drivers.

>
> Yes, that's true.
>
> There are currently two drivers that bind to the created 
> simple-framebuffer: fbdev's simplefb and DRM's simpledrm. The new 
> corebootdrm intents to replace simpledrm. And simplefb is deprecated: if 
> no one uses that any longer, I'm all for removing coreboot's current 
> framebuffer handling.
>
> Best regards
> Thomas
>

As Thomas said, simplefb has already been replaced by simpledrm. DRM
provides a lot of advantages over simplefb, for instance is a KMS driver
so one could start KMS apps (e.g., a wayland compositor) which is not
possible with simplefb.

And as Thomas said, fbdev is deprecated and likely going away at some
point in the future.

The real question is about simpledrm vs corebootdrm but Thomas also has
explained why that change is an improvement. Then simpledrm could only
focus on DT based systems and have dedicated drivers for EFI, VESA and
Coreboot system framebuffers.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 1/8] firmware: google: Do sysfb test before creating coreboot framebuffer

[Julius Werner]

> Another question I have is why there's a separate device for the
> coreboot-table? Couldn't the kernel just create the coreboot bus and
> then populate it with the table entries? DT does that. If the
> coreboot-table device is really just the parent for the other deivce,
> that is incorrect, as I describe above.

To be honest I didn't write this code and probably don't understand it
well enough to answer that question (we should probably CC +Samuel,
who did). I assume that it was done to get the automatic matching of
the ACPI/DT entry that tells the kernel that a coreboot table exists.
If there's a better way to achieve that, feel free to change it.

> * the check on whether the CB_TAG_FRAMEBUFFER should be used or not by the
>   kernel should also be there (what Thomas did in patch 1/8) but instead
>   of omiting registering the coreboot device in the bus, the device could
>   be marked as "unused" or "disabled" (by adding a field to coreboot_device).

This sounds like a reasonable compromise. It still feels a bit odd to
me that the check whether other framebuffers exist happens during
parsing of the table rather than at the point where the table entry is
actually used by something, but if that's the easiest way to make it
work, I guess there's no harm in doing it there. (I'm still curious,
though, why can't you just do the check in corebootdrm_probe(), and
return -ENODEV if the other framebuffer is used? Wouldn't that be
equivalent to what the old driver did?)


> * the coreboot_bus_match() function then can take this new field into
>   account and only match if (device->entry.tag == id->tag && !device->unused)
>   or something like that.
>
> That way no device will be removed from the bus and the corebootdrm driver
> will only be probed when the device has to actually be used by the kernel.
>
> --
> Best regards,
>
> Javier Martinez Canillas
> Core Platforms
> Red Hat
>

Re: [PATCH 1/8] firmware: google: Do sysfb test before creating coreboot framebuffer

[Thomas Zimmermann]

Hi

Am 13.01.26 um 23:32 schrieb Julius Werner:
>> Another question I have is why there's a separate device for the
>> coreboot-table? Couldn't the kernel just create the coreboot bus and
>> then populate it with the table entries? DT does that. If the
>> coreboot-table device is really just the parent for the other deivce,
>> that is incorrect, as I describe above.
> To be honest I didn't write this code and probably don't understand it
> well enough to answer that question (we should probably CC +Samuel,
> who did). I assume that it was done to get the automatic matching of
> the ACPI/DT entry that tells the kernel that a coreboot table exists.
> If there's a better way to achieve that, feel free to change it.

IDK. My take would have been to provide the coreboot table under 
/sys/firmware; similar to DT. And then walk over it and create devices 
for the known entries.

>
>> * the check on whether the CB_TAG_FRAMEBUFFER should be used or not by the
>>    kernel should also be there (what Thomas did in patch 1/8) but instead
>>    of omiting registering the coreboot device in the bus, the device could
>>    be marked as "unused" or "disabled" (by adding a field to coreboot_device).
> This sounds like a reasonable compromise. It still feels a bit odd to
> me that the check whether other framebuffers exist happens during
> parsing of the table rather than at the point where the table entry is
> actually used by something, but if that's the easiest way to make it
> work, I guess there's no harm in doing it there. (I'm still curious,
> though, why can't you just do the check in corebootdrm_probe(), and
> return -ENODEV if the other framebuffer is used? Wouldn't that be
> equivalent to what the old driver did?)

I have a new iteration of the series that creates a coreboot-framebuffer 
platform device for at the same place that currently creates the 
simple-framebuffer device (in framebuffer_probe()). So we can leave most 
of the code it as. There are also a number of bugs that this is going to 
fix. I'll post it soon.

Best regards
Thomas

>
>
>> * the coreboot_bus_match() function then can take this new field into
>>    account and only match if (device->entry.tag == id->tag && !device->unused)
>>    or something like that.
>>
>> That way no device will be removed from the bus and the corebootdrm driver
>> will only be probed when the device has to actually be used by the kernel.
>>
>> --
>> Best regards,
>>
>> Javier Martinez Canillas
>> Core Platforms
>> Red Hat
>>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Re: [PATCH 6/8] drm/sysfb: Remove duplicate declarations

[Thomas Zimmermann]

Am 09.01.26 um 11:31 schrieb Javier Martinez Canillas:
> Thomas Zimmermann <tzimmermann@suse.de> writes:
>
>> Commit 6046b49bafff ("drm/sysfb: Share helpers for integer validation")
>> and commit e8c086880b2b ("drm/sysfb: Share helpers for screen_info
>> validation") added duplicate function declarations. Remove the latter
>> ones.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Fixes: e8c086880b2b ("drm/sysfb: Share helpers for screen_info validation")
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: Javier Martinez Canillas <javierm@redhat.com>
>> Cc: dri-devel@lists.freedesktop.org
>> Cc: <stable@vger.kernel.org> # v6.16+
>> ---
>>   drivers/gpu/drm/sysfb/drm_sysfb_helper.h | 9 ---------
>>   1 file changed, 9 deletions(-)
>>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

Thanks. I'm going to put this into drm-misc-fixes in any case.

>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Re: [PATCH 1/8] firmware: google: Do sysfb test before creating coreboot framebuffer

[Julius Werner]

> IDK. My take would have been to provide the coreboot table under
> /sys/firmware; similar to DT. And then walk over it and create devices
> for the known entries.

We have established userspace infrastructure relying on the current
sysfs layout (at least the `cbmem-XXXXXXXX` nodes), so while you may
be right that there may have been better approaches to do this to
begin with, I would much prefer if we can stick with what's there now.

> I have a new iteration of the series that creates a coreboot-framebuffer
> platform device for at the same place that currently creates the
> simple-framebuffer device (in framebuffer_probe()). So we can leave most
> of the code it as. There are also a number of bugs that this is going to
> fix. I'll post it soon.

Sounds good, thanks!

Re: [PATCH 8/8] drm/sysfb: corebootdrm: Add DRM driver for coreboot framebuffers

[kernel test robot]

Hi Thomas,

kernel test robot noticed the following build errors:

[auto build test ERROR on chrome-platform/for-next]
[also build test ERROR on drm-misc/drm-misc-next daeinki-drm-exynos/exynos-drm-next drm/drm-next drm-i915/for-linux-next drm-i915/for-linux-next-fixes linus/master chrome-platform/for-firmware-next v6.19-rc5 next-20260114]
[cannot apply to drm-tip/drm-tip]
[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/Thomas-Zimmermann/firmware-google-Do-sysfb-test-before-creating-coreboot-framebuffer/20260108-225542
base:   https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git for-next
patch link:    https://lore.kernel.org/r/20260108145058.56943-9-tzimmermann%40suse.de
patch subject: [PATCH 8/8] drm/sysfb: corebootdrm: Add DRM driver for coreboot framebuffers
config: parisc-allmodconfig (https://download.01.org/0day-ci/archive/20260115/202601150553.5KsVSXQr-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260115/202601150553.5KsVSXQr-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601150553.5KsVSXQr-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/gpu/drm/sysfb/corebootdrm.c: In function 'corebootdrm_device_init':
>> drivers/gpu/drm/sysfb/corebootdrm.c:225:15: error: implicit declaration of function 'devm_aperture_acquire_for_coreboot_device'; did you mean 'devm_aperture_acquire_for_platform_device'? [-Wimplicit-function-declaration]
     225 |         ret = devm_aperture_acquire_for_coreboot_device(cbdev, fb_pgbase, fb_pgsize);
         |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |               devm_aperture_acquire_for_platform_device


vim +225 drivers/gpu/drm/sysfb/corebootdrm.c

   162	
   163	/*
   164	 * Init / Cleanup
   165	 */
   166	
   167	static int corebootdrm_device_init(struct corebootdrm_device *cdev, struct coreboot_device *cbdev)
   168	{
   169		const struct lb_framebuffer *fb = &cbdev->framebuffer;
   170		struct drm_sysfb_device *sysfb = &cdev->sysfb;
   171		struct drm_device *dev = &sysfb->dev;
   172		const struct drm_format_info *format;
   173		int width, height, pitch;
   174		u64 address;
   175		int width_mm = 0, height_mm = 0;
   176		resource_size_t fb_size, fb_base, fb_pgbase, fb_pgsize;
   177		struct resource *res, *mem = NULL;
   178		void __iomem *screen_base;
   179		struct drm_plane *primary_plane;
   180		struct drm_crtc *crtc;
   181		struct drm_encoder *encoder;
   182		struct drm_connector *connector;
   183		unsigned long max_width, max_height;
   184		size_t nformats;
   185		int ret;
   186	
   187		/*
   188		 * Hardware settings
   189		 */
   190	
   191		format = corebootdrm_get_format_fb(dev, fb);
   192		if (!format)
   193			return -EINVAL;
   194		width = corebootdrm_get_width_fb(dev, fb);
   195		if (width < 0)
   196			return width;
   197		height = corebootdrm_get_height_fb(dev, fb);
   198		if (height < 0)
   199			return height;
   200		pitch = corebootdrm_get_pitch_fb(dev, format, width, fb);
   201		if (pitch < 0)
   202			return pitch;
   203		address = corebootdrm_get_address_fb(dev, height, pitch, fb);
   204		if (!address)
   205			return -EINVAL;
   206	
   207		sysfb->fb_mode = drm_sysfb_mode(width, height, width_mm, height_mm);
   208		sysfb->fb_format = format;
   209		sysfb->fb_pitch = pitch;
   210	
   211		drm_dbg(dev, "display mode={" DRM_MODE_FMT "}\n", DRM_MODE_ARG(&sysfb->fb_mode));
   212		drm_dbg(dev, "framebuffer format=%p4cc, size=%dx%d, pitch=%d byte\n",
   213			&format->format, width, height, pitch);
   214	
   215		/*
   216		 * Memory management
   217		 */
   218	
   219		fb_base = address;
   220		fb_size = pitch * height;
   221	
   222		fb_pgbase = round_down(fb_base, PAGE_SIZE);
   223		fb_pgsize = fb_base - fb_pgbase + round_up(fb_size, PAGE_SIZE);
   224	
 > 225		ret = devm_aperture_acquire_for_coreboot_device(cbdev, fb_pgbase, fb_pgsize);
   226		if (ret) {
   227			drm_err(dev, "could not acquire memory range %pr: %d\n", res, ret);
   228			return ret;
   229		}
   230	
   231		drm_dbg(dev, "using I/O memory framebuffer at %pr\n", res);
   232	
   233		mem = devm_request_mem_region(&cbdev->dev, fb_pgbase, fb_pgsize, dev_name(&cbdev->dev));
   234		if (!mem) {
   235			/*
   236			 * We cannot make this fatal. Sometimes this comes from magic
   237			 * spaces our resource handlers simply don't know about. Use
   238			 * the I/O-memory resource as-is and try to map that instead.
   239			 */
   240			drm_warn(dev, "could not acquire memory region %pr\n", res);
   241			mem = res;
   242		}
   243	
   244		screen_base = devm_ioremap_wc(&cbdev->dev, fb_pgbase, fb_pgsize);
   245		if (!screen_base)
   246			return -ENOMEM;
   247	
   248		iosys_map_set_vaddr_iomem(&sysfb->fb_addr, screen_base);
   249	
   250		/*
   251		 * Modesetting
   252		 */
   253	
   254		ret = drmm_mode_config_init(dev);
   255		if (ret)
   256			return ret;
   257	
   258		max_width = max_t(unsigned long, width, DRM_SHADOW_PLANE_MAX_WIDTH);
   259		max_height = max_t(unsigned long, height, DRM_SHADOW_PLANE_MAX_HEIGHT);
   260	
   261		dev->mode_config.min_width = width;
   262		dev->mode_config.max_width = max_width;
   263		dev->mode_config.min_height = height;
   264		dev->mode_config.max_height = max_height;
   265		dev->mode_config.preferred_depth = format->depth;
   266		dev->mode_config.funcs = &corebootdrm_mode_config_funcs;
   267	
   268		/* Primary plane */
   269	
   270		nformats = drm_sysfb_build_fourcc_list(dev, &format->format, 1,
   271						       cdev->formats, ARRAY_SIZE(cdev->formats));
   272	
   273		primary_plane = &cdev->primary_plane;
   274		ret = drm_universal_plane_init(dev, primary_plane, 0, &corebootdrm_primary_plane_funcs,
   275					       cdev->formats, nformats,
   276					       corebootdrm_primary_plane_format_modifiers,
   277					       DRM_PLANE_TYPE_PRIMARY, NULL);
   278		if (ret)
   279			return ret;
   280		drm_plane_helper_add(primary_plane, &corebootdrm_primary_plane_helper_funcs);
   281		drm_plane_enable_fb_damage_clips(primary_plane);
   282	
   283		/* CRTC */
   284	
   285		crtc = &cdev->crtc;
   286		ret = drm_crtc_init_with_planes(dev, crtc, primary_plane, NULL,
   287						&corebootdrm_crtc_funcs, NULL);
   288		if (ret)
   289			return ret;
   290		drm_crtc_helper_add(crtc, &corebootdrm_crtc_helper_funcs);
   291	
   292		/* Encoder */
   293	
   294		encoder = &cdev->encoder;
   295		ret = drm_encoder_init(dev, encoder, &corebootdrm_encoder_funcs,
   296				       DRM_MODE_ENCODER_NONE, NULL);
   297		if (ret)
   298			return ret;
   299		encoder->possible_crtcs = drm_crtc_mask(crtc);
   300	
   301		/* Connector */
   302	
   303		connector = &cdev->connector;
   304		ret = drm_connector_init(dev, connector, &corebootdrm_connector_funcs,
   305					 DRM_MODE_CONNECTOR_Unknown);
   306		if (ret)
   307			return ret;
   308		drm_connector_helper_add(connector, &corebootdrm_connector_helper_funcs);
   309		drm_connector_set_panel_orientation_with_quirk(connector,
   310							       DRM_MODE_PANEL_ORIENTATION_UNKNOWN,
   311							       width, height);
   312	
   313		ret = drm_connector_attach_encoder(connector, encoder);
   314		if (ret)
   315			return ret;
   316	
   317		drm_mode_config_reset(dev);
   318	
   319		return 0;
   320	}
   321	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 8/8] drm/sysfb: corebootdrm: Add DRM driver for coreboot framebuffers

[kernel test robot]

Hi Thomas,

kernel test robot noticed the following build errors:

[auto build test ERROR on chrome-platform/for-next]
[also build test ERROR on drm-misc/drm-misc-next daeinki-drm-exynos/exynos-drm-next drm/drm-next drm-i915/for-linux-next drm-i915/for-linux-next-fixes linus/master chrome-platform/for-firmware-next v6.19-rc5 next-20260114]
[cannot apply to drm-tip/drm-tip]
[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/Thomas-Zimmermann/firmware-google-Do-sysfb-test-before-creating-coreboot-framebuffer/20260108-225542
base:   https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git for-next
patch link:    https://lore.kernel.org/r/20260108145058.56943-9-tzimmermann%40suse.de
patch subject: [PATCH 8/8] drm/sysfb: corebootdrm: Add DRM driver for coreboot framebuffers
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20260115/202601150622.i0pTAj9w-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260115/202601150622.i0pTAj9w-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601150622.i0pTAj9w-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/sysfb/corebootdrm.c:225:8: error: call to undeclared function 'devm_aperture_acquire_for_coreboot_device'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     225 |         ret = devm_aperture_acquire_for_coreboot_device(cbdev, fb_pgbase, fb_pgsize);
         |               ^
   drivers/gpu/drm/sysfb/corebootdrm.c:225:8: note: did you mean 'devm_aperture_acquire_for_platform_device'?
   include/linux/aperture.h:19:5: note: 'devm_aperture_acquire_for_platform_device' declared here
      19 | int devm_aperture_acquire_for_platform_device(struct platform_device *pdev,
         |     ^
   1 error generated.


vim +/devm_aperture_acquire_for_coreboot_device +225 drivers/gpu/drm/sysfb/corebootdrm.c

   162	
   163	/*
   164	 * Init / Cleanup
   165	 */
   166	
   167	static int corebootdrm_device_init(struct corebootdrm_device *cdev, struct coreboot_device *cbdev)
   168	{
   169		const struct lb_framebuffer *fb = &cbdev->framebuffer;
   170		struct drm_sysfb_device *sysfb = &cdev->sysfb;
   171		struct drm_device *dev = &sysfb->dev;
   172		const struct drm_format_info *format;
   173		int width, height, pitch;
   174		u64 address;
   175		int width_mm = 0, height_mm = 0;
   176		resource_size_t fb_size, fb_base, fb_pgbase, fb_pgsize;
   177		struct resource *res, *mem = NULL;
   178		void __iomem *screen_base;
   179		struct drm_plane *primary_plane;
   180		struct drm_crtc *crtc;
   181		struct drm_encoder *encoder;
   182		struct drm_connector *connector;
   183		unsigned long max_width, max_height;
   184		size_t nformats;
   185		int ret;
   186	
   187		/*
   188		 * Hardware settings
   189		 */
   190	
   191		format = corebootdrm_get_format_fb(dev, fb);
   192		if (!format)
   193			return -EINVAL;
   194		width = corebootdrm_get_width_fb(dev, fb);
   195		if (width < 0)
   196			return width;
   197		height = corebootdrm_get_height_fb(dev, fb);
   198		if (height < 0)
   199			return height;
   200		pitch = corebootdrm_get_pitch_fb(dev, format, width, fb);
   201		if (pitch < 0)
   202			return pitch;
   203		address = corebootdrm_get_address_fb(dev, height, pitch, fb);
   204		if (!address)
   205			return -EINVAL;
   206	
   207		sysfb->fb_mode = drm_sysfb_mode(width, height, width_mm, height_mm);
   208		sysfb->fb_format = format;
   209		sysfb->fb_pitch = pitch;
   210	
   211		drm_dbg(dev, "display mode={" DRM_MODE_FMT "}\n", DRM_MODE_ARG(&sysfb->fb_mode));
   212		drm_dbg(dev, "framebuffer format=%p4cc, size=%dx%d, pitch=%d byte\n",
   213			&format->format, width, height, pitch);
   214	
   215		/*
   216		 * Memory management
   217		 */
   218	
   219		fb_base = address;
   220		fb_size = pitch * height;
   221	
   222		fb_pgbase = round_down(fb_base, PAGE_SIZE);
   223		fb_pgsize = fb_base - fb_pgbase + round_up(fb_size, PAGE_SIZE);
   224	
 > 225		ret = devm_aperture_acquire_for_coreboot_device(cbdev, fb_pgbase, fb_pgsize);
   226		if (ret) {
   227			drm_err(dev, "could not acquire memory range %pr: %d\n", res, ret);
   228			return ret;
   229		}
   230	
   231		drm_dbg(dev, "using I/O memory framebuffer at %pr\n", res);
   232	
   233		mem = devm_request_mem_region(&cbdev->dev, fb_pgbase, fb_pgsize, dev_name(&cbdev->dev));
   234		if (!mem) {
   235			/*
   236			 * We cannot make this fatal. Sometimes this comes from magic
   237			 * spaces our resource handlers simply don't know about. Use
   238			 * the I/O-memory resource as-is and try to map that instead.
   239			 */
   240			drm_warn(dev, "could not acquire memory region %pr\n", res);
   241			mem = res;
   242		}
   243	
   244		screen_base = devm_ioremap_wc(&cbdev->dev, fb_pgbase, fb_pgsize);
   245		if (!screen_base)
   246			return -ENOMEM;
   247	
   248		iosys_map_set_vaddr_iomem(&sysfb->fb_addr, screen_base);
   249	
   250		/*
   251		 * Modesetting
   252		 */
   253	
   254		ret = drmm_mode_config_init(dev);
   255		if (ret)
   256			return ret;
   257	
   258		max_width = max_t(unsigned long, width, DRM_SHADOW_PLANE_MAX_WIDTH);
   259		max_height = max_t(unsigned long, height, DRM_SHADOW_PLANE_MAX_HEIGHT);
   260	
   261		dev->mode_config.min_width = width;
   262		dev->mode_config.max_width = max_width;
   263		dev->mode_config.min_height = height;
   264		dev->mode_config.max_height = max_height;
   265		dev->mode_config.preferred_depth = format->depth;
   266		dev->mode_config.funcs = &corebootdrm_mode_config_funcs;
   267	
   268		/* Primary plane */
   269	
   270		nformats = drm_sysfb_build_fourcc_list(dev, &format->format, 1,
   271						       cdev->formats, ARRAY_SIZE(cdev->formats));
   272	
   273		primary_plane = &cdev->primary_plane;
   274		ret = drm_universal_plane_init(dev, primary_plane, 0, &corebootdrm_primary_plane_funcs,
   275					       cdev->formats, nformats,
   276					       corebootdrm_primary_plane_format_modifiers,
   277					       DRM_PLANE_TYPE_PRIMARY, NULL);
   278		if (ret)
   279			return ret;
   280		drm_plane_helper_add(primary_plane, &corebootdrm_primary_plane_helper_funcs);
   281		drm_plane_enable_fb_damage_clips(primary_plane);
   282	
   283		/* CRTC */
   284	
   285		crtc = &cdev->crtc;
   286		ret = drm_crtc_init_with_planes(dev, crtc, primary_plane, NULL,
   287						&corebootdrm_crtc_funcs, NULL);
   288		if (ret)
   289			return ret;
   290		drm_crtc_helper_add(crtc, &corebootdrm_crtc_helper_funcs);
   291	
   292		/* Encoder */
   293	
   294		encoder = &cdev->encoder;
   295		ret = drm_encoder_init(dev, encoder, &corebootdrm_encoder_funcs,
   296				       DRM_MODE_ENCODER_NONE, NULL);
   297		if (ret)
   298			return ret;
   299		encoder->possible_crtcs = drm_crtc_mask(crtc);
   300	
   301		/* Connector */
   302	
   303		connector = &cdev->connector;
   304		ret = drm_connector_init(dev, connector, &corebootdrm_connector_funcs,
   305					 DRM_MODE_CONNECTOR_Unknown);
   306		if (ret)
   307			return ret;
   308		drm_connector_helper_add(connector, &corebootdrm_connector_helper_funcs);
   309		drm_connector_set_panel_orientation_with_quirk(connector,
   310							       DRM_MODE_PANEL_ORIENTATION_UNKNOWN,
   311							       width, height);
   312	
   313		ret = drm_connector_attach_encoder(connector, encoder);
   314		if (ret)
   315			return ret;
   316	
   317		drm_mode_config_reset(dev);
   318	
   319		return 0;
   320	}
   321	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki