Re: [PATCH 0/5] efi: x86: Provide EDID from GOP device

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2817 bytes --]

Hi

Am 14.11.25 um 09:31 schrieb Ard Biesheuvel:
> On Wed, 15 Oct 2025 at 18:08, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Add support for EFI_EDID_ACTIVE_PROTOCOL and EFI_EDID_DISCOVERED_PROTOCOL
>> on x86. Refactor the GOP helpers for EDID support, then retrieve the EDID
>> into x86 boot_params.
>>
>> Later boot code copies the EDID from the boot parameters into the global
>> variable edid_info. Graphics drivers, such as efidrm, can pick up the
>> information from there. In the case of efidrm, it provides the EDID to
>> user-space compositors, which use it for improved QoS on the display
>> output. Similar functionality is already available on old VESA systems
>> with vesadrm.
>>
>> Tested on x86 EFI systems.
>>
>> Another patch is required to provide EDID on non-x86 systems via the
>> generic EFI stub. The implementation can directly build upon this
>> series.
>>
>> Thomas Zimmermann (5):
>>    efi: Fix trailing whitespace in header file
>>    efi/libstub: gop: Find GOP handle instead of GOP data
>>    efi/libstub: gop: Initialize screen_info in helper function
>>    efi/libstub: gop: Add support for reading EDID
>>    efi/libstub: x86: Store EDID in boot_params
>>
> Hi,
>
> Apologies for the delay. This series looks fine to me, although I
> would prefer it if we could make things a bit more generic?
>
> Everything you are adding here is arch-agnostic, except for the bit
> where we use x86-specific plumbing to pass the EDID info between the
> EFI stub and the core kernel.

Attached is an RFC patch that I already have. This would be the next 
step for EDID support. I've not yet sent the generic-EFI patch, as I did 
not have opportunity to test it. The patch addresses most of what you 
ask for, I think.

>
> More specifically, could we do the following:
> - move struct edid_info edid_info into common code

edid_info is related to screen_info, so it follows the same conventions. 
Arnd Bergmann made x86-specific changes for screen_info in commit 
b8466fe82b79 ("efi: move screen_info into efi init code"). x86 has it's 
own thing, sort of. See the attached patch for my non-x86 solution.

> - pass the detected EDID info block via a EFI config table instead of
> boot_params

The x86 code uses boot params for screen_info already and also transfers 
edid_info on VESA systems via boot params (or if grub set up boot_params 
for us). [1] It's all there and working already. If we transfer 
edid_info via config table, we'd need extra code on x86.

> - make CONFIG_FIRMWARE_EDID depend on (X86 || EFI_STUB)

See the attached patch.

Best regards
Thomas


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


[-- Attachment #2: 0001-efi-transfer-EDID-from-stub-to-kernel.patch --]
[-- Type: text/x-patch, Size: 11641 bytes --]

From 10be917b0cf90f22f2fd900620cb9acf5838ff55 Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann <tzimmermann@suse.de>
Date: Wed, 15 Oct 2025 13:36:06 +0200
Subject: [PATCH] efi: transfer EDID from stub to kernel

- Add UUID for storing the EDID data in the UEFI configuration table.
- The UUID has been created by uuidgen.
- Enable edid_info on EFI systems
---
 drivers/firmware/efi/efi-init.c               | 33 +++++++++++++-
 drivers/firmware/efi/efi.c                    |  2 +
 drivers/firmware/efi/libstub/Makefile         |  2 +-
 drivers/firmware/efi/libstub/edid_info.c      | 40 +++++++++++++++++
 drivers/firmware/efi/libstub/efi-stub-entry.c | 17 +++++++
 drivers/firmware/efi/libstub/efi-stub.c       | 44 +++++++++++++------
 drivers/firmware/efi/libstub/efistub.h        |  4 ++
 drivers/video/Kconfig                         |  8 ++--
 include/linux/efi.h                           |  8 ++--
 9 files changed, 136 insertions(+), 22 deletions(-)
 create mode 100644 drivers/firmware/efi/libstub/edid_info.c

diff --git a/drivers/firmware/efi/efi-init.c b/drivers/firmware/efi/efi-init.c
index a65c2d5b9e7b..2719311f2d43 100644
--- a/drivers/firmware/efi/efi-init.c
+++ b/drivers/firmware/efi/efi-init.c
@@ -23,7 +23,10 @@
 
 #include <asm/efi.h>
 
+#include <video/edid.h>
+
 unsigned long __initdata screen_info_table = EFI_INVALID_TABLE_ADDR;
+unsigned long __initdata edid_info_table = EFI_INVALID_TABLE_ADDR;
 
 static int __init is_memory(efi_memory_desc_t *md)
 {
@@ -57,14 +60,19 @@ static phys_addr_t __init efi_to_phys(unsigned long addr)
 extern __weak const efi_config_table_type_t efi_arch_tables[];
 
 /*
- * x86 defines its own screen_info and uses it even without EFI,
- * everything else can get it from here.
+ * x86 defines its own screen_info and edid_info and uses them even
+ * without EFI, everything else can get them from here.
  */
 #if !defined(CONFIG_X86) && (defined(CONFIG_SYSFB) || defined(CONFIG_EFI_EARLYCON))
 struct screen_info screen_info __section(".data");
 EXPORT_SYMBOL_GPL(screen_info);
 #endif
 
+#if !defined(CONFIG_X86) && defined(CONFIG_FIRMWARE_EDID)
+struct edid_info edid_info __section(".data");
+EXPORT_SYMBOL_GPL(edid_info);
+#endif
+
 static void __init init_screen_info(void)
 {
 	struct screen_info *si;
@@ -88,6 +96,23 @@ static void __init init_screen_info(void)
 	}
 }
 
+static void __init init_edid_info(void)
+{
+#if defined(CONFIG_FIRMWARE_EDID)
+	struct edid_info *edid;
+
+	if (edid_info_table == EFI_INVALID_TABLE_ADDR)
+		return;
+
+	edid = early_memremap(edid_info_table, sizeof(*edid));
+	if (!edid)
+		return; /* not an error; EDID is optional */
+	edid_info = *edid;
+	memset(edid, 0, sizeof(*edid));
+	early_memunmap(edid, sizeof(*edid));
+#endif
+}
+
 static int __init uefi_init(u64 efi_system_table)
 {
 	efi_config_table_t *config_tables;
@@ -275,4 +300,8 @@ void __init efi_init(void)
 	    IS_ENABLED(CONFIG_SYSFB) ||
 	    IS_ENABLED(CONFIG_EFI_EARLYCON))
 		init_screen_info();
+
+	if (IS_ENABLED(CONFIG_X86) ||
+	    IS_ENABLED(CONFIG_FIRMWARE_EDID))
+		init_edid_info();
 }
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 1ce428e2ac8a..7a3047a238ae 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -64,6 +64,7 @@ static unsigned long __initdata rt_prop = EFI_INVALID_TABLE_ADDR;
 static unsigned long __initdata initrd = EFI_INVALID_TABLE_ADDR;
 
 extern unsigned long screen_info_table;
+extern unsigned long edid_info_table;
 
 struct mm_struct efi_mm = {
 	.mm_mt			= MTREE_INIT_EXT(mm_mt, MM_MT_FLAGS, efi_mm.mmap_lock),
@@ -639,6 +640,7 @@ static const efi_config_table_type_t common_tables[] __initconst = {
 #endif
 #ifdef CONFIG_EFI_GENERIC_STUB
 	{LINUX_EFI_SCREEN_INFO_TABLE_GUID,	&screen_info_table			},
+	{LINUX_EFI_EDID_INFO_TABLE_GUID,	&edid_info_table			},
 #endif
 	{},
 };
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 94b05e4451dd..12ae5c6be028 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -80,7 +80,7 @@ $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
 	$(call if_changed_rule,cc_o_c)
 
 lib-$(CONFIG_EFI_GENERIC_STUB)	+= efi-stub.o string.o intrinsics.o systable.o \
-				   screen_info.o efi-stub-entry.o
+				   screen_info.o edid_info.o efi-stub-entry.o
 
 lib-$(CONFIG_ARM)		+= arm32-stub.o
 lib-$(CONFIG_ARM64)		+= kaslr.o arm64.o arm64-stub.o smbios.o
diff --git a/drivers/firmware/efi/libstub/edid_info.c b/drivers/firmware/efi/libstub/edid_info.c
new file mode 100644
index 000000000000..882c85faf395
--- /dev/null
+++ b/drivers/firmware/efi/libstub/edid_info.c
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/efi.h>
+#include <video/edid.h>
+#include <asm/efi.h>
+
+#include "efistub.h"
+
+static efi_guid_t edid_info_guid = LINUX_EFI_EDID_INFO_TABLE_GUID;
+
+struct edid_info *__alloc_edid_info(void)
+{
+	struct edid_info *edid;
+	efi_status_t status;
+
+	status = efi_bs_call(allocate_pool, EFI_ACPI_RECLAIM_MEMORY,
+			     sizeof(*edid), (void **)&edid);
+
+	if (status != EFI_SUCCESS)
+		return NULL;
+
+	memset(edid, 0, sizeof(*edid));
+
+	status = efi_bs_call(install_configuration_table,
+			     &edid_info_guid, edid);
+	if (status == EFI_SUCCESS)
+		return edid;
+
+	efi_bs_call(free_pool, edid);
+	return NULL;
+}
+
+void free_edid_info(struct edid_info *edid)
+{
+	if (!edid)
+		return;
+
+	efi_bs_call(install_configuration_table, &edid_info_guid, NULL);
+	efi_bs_call(free_pool, edid);
+}
diff --git a/drivers/firmware/efi/libstub/efi-stub-entry.c b/drivers/firmware/efi/libstub/efi-stub-entry.c
index a6c049835190..7ba8a23bbc55 100644
--- a/drivers/firmware/efi/libstub/efi-stub-entry.c
+++ b/drivers/firmware/efi/libstub/efi-stub-entry.c
@@ -5,9 +5,12 @@
 
 #include <asm/efi.h>
 
+#include <video/edid.h>
+
 #include "efistub.h"
 
 static unsigned long screen_info_offset;
+static unsigned long edid_info_offset;
 
 struct screen_info *alloc_screen_info(void)
 {
@@ -22,6 +25,19 @@ struct screen_info *alloc_screen_info(void)
 	return NULL;
 }
 
+struct edid_info *alloc_edid_info(void)
+{
+#if defined(CONFIG_FIRMWARE_EDID)
+	if (IS_ENABLED(CONFIG_ARM))
+		return __alloc_edid_info();
+
+	if (IS_ENABLED(CONFIG_X86) &&
+	    IS_ENABLED(CONFIG_FIRMWARE_EDID))
+		return (void *)&edid_info + edid_info_offset;
+#endif
+	return NULL;
+}
+
 /*
  * EFI entry point for the generic EFI stub used by ARM, arm64, RISC-V and
  * LoongArch. This is the entrypoint that is described in the PE/COFF header
@@ -74,6 +90,7 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 	}
 
 	screen_info_offset = image_addr - (unsigned long)image->image_base;
+	edid_info_offset = image_addr - (unsigned long)image->image_base;
 
 	status = efi_stub_common(handle, image, image_addr, cmdline_ptr);
 
diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
index 9cb814c5ba1b..9cb2a5c154b4 100644
--- a/drivers/firmware/efi/libstub/efi-stub.c
+++ b/drivers/firmware/efi/libstub/efi-stub.c
@@ -12,6 +12,7 @@
 #include <linux/efi.h>
 #include <linux/screen_info.h>
 #include <asm/efi.h>
+#include <video/edid.h>
 
 #include "efistub.h"
 
@@ -49,22 +50,37 @@ static u64 virtmap_base = EFI_RT_VIRTUAL_BASE;
 static bool flat_va_mapping = (EFI_RT_VIRTUAL_OFFSET != 0);
 
 void __weak free_screen_info(struct screen_info *si)
-{
-}
+{ }
+
+void __weak free_edid_info(struct edid_info *edid)
+{ }
 
-static struct screen_info *setup_graphics(void)
+static void setup_graphics(struct screen_info **si, struct edid_info **edid)
 {
-	struct screen_info *si, tmp = {};
+	efi_status_t status;
+
+	*si = alloc_screen_info();
+	*edid = alloc_edid_info();
 
-	if (efi_setup_graphics(&tmp, NULL) != EFI_SUCCESS)
-		return NULL;
+	if (!*si && !*edid)
+		return;
 
-	si = alloc_screen_info();
-	if (!si)
-		return NULL;
+	status = efi_setup_graphics(*si, *edid);
+	if (status) {
+		/* Clear on error */
+		if (*edid)
+			memset(*edid, 0, sizeof(**edid));
+		if (*si)
+			memset(*si, 0, sizeof(**si));
+	}
+}
 
-	*si = tmp;
-	return si;
+static void release_graphics(struct screen_info *si, struct edid_info *edid)
+{
+	if (edid)
+		free_edid_info(edid);
+	if (si)
+		free_screen_info(si);
 }
 
 static void install_memreserve_table(void)
@@ -146,13 +162,14 @@ efi_status_t efi_stub_common(efi_handle_t handle,
 			     char *cmdline_ptr)
 {
 	struct screen_info *si;
+	struct edid_info *edid;
 	efi_status_t status;
 
 	status = check_platform_features();
 	if (status != EFI_SUCCESS)
 		return status;
 
-	si = setup_graphics();
+	setup_graphics(&si, &edid);
 
 	efi_retrieve_eventlog();
 
@@ -172,7 +189,8 @@ efi_status_t efi_stub_common(efi_handle_t handle,
 
 	status = efi_boot_kernel(handle, image, image_addr, cmdline_ptr);
 
-	free_screen_info(si);
+	release_graphics(si, edid);
+
 	return status;
 }
 
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index b2fb0c3fa721..120c9a499362 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -1179,6 +1179,10 @@ struct screen_info *alloc_screen_info(void);
 struct screen_info *__alloc_screen_info(void);
 void free_screen_info(struct screen_info *si);
 
+struct edid_info *alloc_edid_info(void);
+struct edid_info *__alloc_edid_info(void);
+void free_edid_info(struct edid_info *edid);
+
 void efi_cache_sync_image(unsigned long image_base,
 			  unsigned long alloc_size);
 
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index d51777df12d1..f452fac90a9f 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -63,11 +63,13 @@ endif # HAS_IOMEM
 
 config FIRMWARE_EDID
 	bool "Enable firmware EDID"
-	depends on X86
+	depends on EFI || X86
 	help
 	  This enables access to the EDID transferred from the firmware.
-	  On x86, this is from the VESA BIOS. DRM display drivers will
-	  be able to export the information to userspace.
+	  On EFI systems, the EDID comes from the same device as the
+	  primary GOP. On x86 with BIOS, it comes from the VESA BIOS.
+	  DRM display drivers will be able to export the information
+	  to userspace.
 
 	  Also enable this if DDC/I2C transfers do not work for your driver
 	  and if you are using nvidiafb, i810fb or savagefb.
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 11e267492efd..eeff8393c9c0 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -406,11 +406,13 @@ void efi_native_runtime_setup(void);
 #define EFI_CC_FINAL_EVENTS_TABLE_GUID		EFI_GUID(0xdd4a4648, 0x2de7, 0x4665, 0x96, 0x4d, 0x21, 0xd9, 0xef, 0x5f, 0xb4, 0x46)
 
 /*
- * This GUID is used to pass to the kernel proper the struct screen_info
- * structure that was populated by the stub based on the GOP protocol instance
- * associated with ConOut
+ * These GUIDs are used to pass to the kernel proper the struct screen_info
+ * and struct edid_info structures that were populated by the stub based on
+ * the GOP protocol instance associated with ConOut.
  */
 #define LINUX_EFI_SCREEN_INFO_TABLE_GUID	EFI_GUID(0xe03fc20a, 0x85dc, 0x406e,  0xb9, 0x0e, 0x4a, 0xb5, 0x02, 0x37, 0x1d, 0x95)
+#define LINUX_EFI_EDID_INFO_TABLE_GUID		EFI_GUID(0x4e6c17ea, 0x6ec0, 0x4838,  0x8c, 0xae, 0x2c, 0xf5, 0x4d, 0x5f, 0x58, 0x64)
+
 #define LINUX_EFI_ARM_CPU_STATE_TABLE_GUID	EFI_GUID(0xef79e4aa, 0x3c3d, 0x4989,  0xb9, 0x02, 0x07, 0xa9, 0x43, 0xe5, 0x50, 0xd2)
 #define LINUX_EFI_LOADER_ENTRY_GUID		EFI_GUID(0x4a67b082, 0x0a4c, 0x41cf,  0xb6, 0xc7, 0x44, 0x0b, 0x29, 0xbb, 0x8c, 0x4f)
 #define LINUX_EFI_RANDOM_SEED_TABLE_GUID	EFI_GUID(0x1ce1e5bc, 0x7ceb, 0x42f2,  0x81, 0xe5, 0x8a, 0xad, 0xf1, 0x80, 0xf5, 0x7b)
-- 
2.51.1