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

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

[Thomas Zimmermann]

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

 drivers/firmware/efi/libstub/efi-stub.c |   2 +-
 drivers/firmware/efi/libstub/efistub.h  |  31 +++++-
 drivers/firmware/efi/libstub/gop.c      | 137 +++++++++++++++---------
 drivers/firmware/efi/libstub/x86-stub.c |   3 +-
 include/linux/efi.h                     |   4 +-
 5 files changed, 120 insertions(+), 57 deletions(-)

-- 
2.51.0

[PATCH 1/5] efi: Fix trailing whitespace in header file

[Thomas Zimmermann]

Resolve an issue with the coding style.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 include/linux/efi.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/efi.h b/include/linux/efi.h
index a98cc39e7aaa..544498c89ced 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -290,7 +290,7 @@ typedef efi_status_t efi_get_variable_t (efi_char16_t *name, efi_guid_t *vendor,
 					 unsigned long *data_size, void *data);
 typedef efi_status_t efi_get_next_variable_t (unsigned long *name_size, efi_char16_t *name,
 					      efi_guid_t *vendor);
-typedef efi_status_t efi_set_variable_t (efi_char16_t *name, efi_guid_t *vendor, 
+typedef efi_status_t efi_set_variable_t (efi_char16_t *name, efi_guid_t *vendor,
 					 u32 attr, unsigned long data_size,
 					 void *data);
 typedef efi_status_t efi_get_next_high_mono_count_t (u32 *count);
-- 
2.51.0

[PATCH 2/5] efi/libstub: gop: Find GOP handle instead of GOP data

[Thomas Zimmermann]

The device handle of the GOP device is required to retrieve the
correct EDID data. Find the handle instead of the GOP data. Still
return the GOP data in the function arguments, as we already looked
it up.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/firmware/efi/libstub/gop.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
index 3785fb4986b4..fd32be8dd146 100644
--- a/drivers/firmware/efi/libstub/gop.c
+++ b/drivers/firmware/efi/libstub/gop.c
@@ -402,12 +402,13 @@ setup_pixel_info(struct screen_info *si, u32 pixels_per_scan_line,
 	}
 }
 
-static efi_graphics_output_protocol_t *find_gop(unsigned long num,
-						const efi_handle_t handles[])
+static efi_handle_t find_handle_with_primary_gop(unsigned long num, const efi_handle_t handles[],
+						 efi_graphics_output_protocol_t **found_gop)
 {
 	efi_graphics_output_protocol_t *first_gop;
-	efi_handle_t h;
+	efi_handle_t h, first_gop_handle;
 
+	first_gop_handle = NULL;
 	first_gop = NULL;
 
 	for_each_efi_handle(h, handles, num) {
@@ -442,19 +443,25 @@ static efi_graphics_output_protocol_t *find_gop(unsigned long num,
 		 */
 		status = efi_bs_call(handle_protocol, h,
 				     &EFI_CONSOLE_OUT_DEVICE_GUID, &dummy);
-		if (status == EFI_SUCCESS)
-			return gop;
-
-		if (!first_gop)
+		if (status == EFI_SUCCESS) {
+			if (found_gop)
+				*found_gop = gop;
+			return h;
+		} else if (!first_gop_handle) {
+			first_gop_handle = h;
 			first_gop = gop;
+		}
 	}
 
-	return first_gop;
+	if (found_gop)
+		*found_gop = first_gop;
+	return first_gop_handle;
 }
 
 efi_status_t efi_setup_gop(struct screen_info *si)
 {
 	efi_handle_t *handles __free(efi_pool) = NULL;
+	efi_handle_t handle;
 	efi_graphics_output_protocol_mode_t *mode;
 	efi_graphics_output_mode_info_t *info;
 	efi_graphics_output_protocol_t *gop;
@@ -467,8 +474,8 @@ efi_status_t efi_setup_gop(struct screen_info *si)
 	if (status != EFI_SUCCESS)
 		return status;
 
-	gop = find_gop(num, handles);
-	if (!gop)
+	handle = find_handle_with_primary_gop(num, handles, &gop);
+	if (!handle)
 		return EFI_NOT_FOUND;
 
 	/* Change mode if requested */
-- 
2.51.0

[PATCH 3/5] efi/libstub: gop: Initialize screen_info in helper function

[Thomas Zimmermann]

Move initialization of screen_info into a single helper function.
Frees up space in the main setup helper for adding EDID support.
No functional changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/firmware/efi/libstub/gop.c | 76 +++++++++++++-----------------
 1 file changed, 33 insertions(+), 43 deletions(-)

diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
index fd32be8dd146..02459ef0f18c 100644
--- a/drivers/firmware/efi/libstub/gop.c
+++ b/drivers/firmware/efi/libstub/gop.c
@@ -367,24 +367,31 @@ static void find_bits(u32 mask, u8 *pos, u8 *size)
 	*size = __fls(mask) - *pos + 1;
 }
 
-static void
-setup_pixel_info(struct screen_info *si, u32 pixels_per_scan_line,
-		 efi_pixel_bitmask_t pixel_info, int pixel_format)
+static void setup_screen_info(struct screen_info *si, const efi_graphics_output_protocol_t *gop)
 {
-	if (pixel_format == PIXEL_BIT_MASK) {
-		find_bits(pixel_info.red_mask,
-			  &si->red_pos, &si->red_size);
-		find_bits(pixel_info.green_mask,
-			  &si->green_pos, &si->green_size);
-		find_bits(pixel_info.blue_mask,
-			  &si->blue_pos, &si->blue_size);
-		find_bits(pixel_info.reserved_mask,
-			  &si->rsvd_pos, &si->rsvd_size);
-		si->lfb_depth = si->red_size + si->green_size +
-			si->blue_size + si->rsvd_size;
-		si->lfb_linelength = (pixels_per_scan_line * si->lfb_depth) / 8;
+	const efi_graphics_output_protocol_mode_t *mode = efi_table_attr(gop, mode);
+	const efi_graphics_output_mode_info_t *info = efi_table_attr(mode, info);
+
+	si->orig_video_isVGA = VIDEO_TYPE_EFI;
+
+	si->lfb_width  = info->horizontal_resolution;
+	si->lfb_height = info->vertical_resolution;
+
+	efi_set_u64_split(efi_table_attr(mode, frame_buffer_base),
+			  &si->lfb_base, &si->ext_lfb_base);
+	if (si->ext_lfb_base)
+		si->capabilities |= VIDEO_CAPABILITY_64BIT_BASE;
+	si->pages = 1;
+
+	if (info->pixel_format == PIXEL_BIT_MASK) {
+		find_bits(info->pixel_information.red_mask, &si->red_pos, &si->red_size);
+		find_bits(info->pixel_information.green_mask, &si->green_pos, &si->green_size);
+		find_bits(info->pixel_information.blue_mask, &si->blue_pos, &si->blue_size);
+		find_bits(info->pixel_information.reserved_mask, &si->rsvd_pos, &si->rsvd_size);
+		si->lfb_depth = si->red_size + si->green_size + si->blue_size + si->rsvd_size;
+		si->lfb_linelength = (info->pixels_per_scan_line * si->lfb_depth) / 8;
 	} else {
-		if (pixel_format == PIXEL_RGB_RESERVED_8BIT_PER_COLOR) {
+		if (info->pixel_format == PIXEL_RGB_RESERVED_8BIT_PER_COLOR) {
 			si->red_pos   = 0;
 			si->blue_pos  = 16;
 		} else /* PIXEL_BGR_RESERVED_8BIT_PER_COLOR */ {
@@ -394,12 +401,16 @@ setup_pixel_info(struct screen_info *si, u32 pixels_per_scan_line,
 
 		si->green_pos = 8;
 		si->rsvd_pos  = 24;
-		si->red_size = si->green_size =
-			si->blue_size = si->rsvd_size = 8;
-
+		si->red_size = 8;
+		si->green_size = 8;
+		si->blue_size = 8;
+		si->rsvd_size = 8;
 		si->lfb_depth = 32;
-		si->lfb_linelength = pixels_per_scan_line * 4;
+		si->lfb_linelength = info->pixels_per_scan_line * 4;
 	}
+
+	si->lfb_size = si->lfb_linelength * si->lfb_height;
+	si->capabilities |= VIDEO_CAPABILITY_SKIP_QUIRKS;
 }
 
 static efi_handle_t find_handle_with_primary_gop(unsigned long num, const efi_handle_t handles[],
@@ -462,8 +473,6 @@ efi_status_t efi_setup_gop(struct screen_info *si)
 {
 	efi_handle_t *handles __free(efi_pool) = NULL;
 	efi_handle_t handle;
-	efi_graphics_output_protocol_mode_t *mode;
-	efi_graphics_output_mode_info_t *info;
 	efi_graphics_output_protocol_t *gop;
 	efi_status_t status;
 	unsigned long num;
@@ -482,27 +491,8 @@ efi_status_t efi_setup_gop(struct screen_info *si)
 	set_mode(gop);
 
 	/* EFI framebuffer */
-	mode = efi_table_attr(gop, mode);
-	info = efi_table_attr(mode, info);
-
-	si->orig_video_isVGA = VIDEO_TYPE_EFI;
-
-	si->lfb_width  = info->horizontal_resolution;
-	si->lfb_height = info->vertical_resolution;
-
-	efi_set_u64_split(efi_table_attr(mode, frame_buffer_base),
-			  &si->lfb_base, &si->ext_lfb_base);
-	if (si->ext_lfb_base)
-		si->capabilities |= VIDEO_CAPABILITY_64BIT_BASE;
-
-	si->pages = 1;
-
-	setup_pixel_info(si, info->pixels_per_scan_line,
-			     info->pixel_information, info->pixel_format);
-
-	si->lfb_size = si->lfb_linelength * si->lfb_height;
-
-	si->capabilities |= VIDEO_CAPABILITY_SKIP_QUIRKS;
+	if (si)
+		setup_screen_info(si, gop);
 
 	return EFI_SUCCESS;
 }
-- 
2.51.0

[PATCH 4/5] efi/libstub: gop: Add support for reading EDID

[Thomas Zimmermann]

Add support for EFI_EDID_DISCOVERED_PROTOCOL and EFI_EDID_ACTIVE_PROTOCOL
as defined in UEFI 2.8, sec 12.9. Define GUIDs and data structures in the
rsp header files.

In the GOP setup function, read the EDID of the primary GOP device. First
try EFI_EDID_ACTIVE_PROTOCOL, which supports user-specified EDID data. Or
else try EFI_EDID_DISCOVERED_PROTOCOL, which returns the display device's
native EDID. If no EDID could be retrieved, clear the storage.

Rename efi_setup_gop() to efi_setup_graphics() to reflect the changes
Let callers pass an optional instance of struct edid_data, if they are
interested.

While screen_info and edid_info come from the same device handle, they
should be considered indendent data. The former refers to the graphics
mode, the latter refers to the display device. GOP devices might not
provide both.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/firmware/efi/libstub/efi-stub.c |  2 +-
 drivers/firmware/efi/libstub/efistub.h  | 31 ++++++++++++++++++++-
 drivers/firmware/efi/libstub/gop.c      | 36 ++++++++++++++++++++++++-
 drivers/firmware/efi/libstub/x86-stub.c |  2 +-
 include/linux/efi.h                     |  2 ++
 5 files changed, 69 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
index 874f63b4a383..9cb814c5ba1b 100644
--- a/drivers/firmware/efi/libstub/efi-stub.c
+++ b/drivers/firmware/efi/libstub/efi-stub.c
@@ -56,7 +56,7 @@ static struct screen_info *setup_graphics(void)
 {
 	struct screen_info *si, tmp = {};
 
-	if (efi_setup_gop(&tmp) != EFI_SUCCESS)
+	if (efi_setup_graphics(&tmp, NULL) != EFI_SUCCESS)
 		return NULL;
 
 	si = alloc_screen_info();
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index f5ba032863a9..b2fb0c3fa721 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -34,6 +34,9 @@
 #define EFI_ALLOC_LIMIT		ULONG_MAX
 #endif
 
+struct edid_info;
+struct screen_info;
+
 extern bool efi_no5lvl;
 extern bool efi_nochunk;
 extern bool efi_nokaslr;
@@ -578,6 +581,32 @@ union efi_graphics_output_protocol {
 	} mixed_mode;
 };
 
+typedef union efi_edid_discovered_protocol efi_edid_discovered_protocol_t;
+
+union efi_edid_discovered_protocol {
+	struct {
+		u32 size_of_edid;
+		u8 *edid;
+	};
+	struct {
+		u32 size_of_edid;
+		u32 edid;
+	} mixed_mode;
+};
+
+typedef union efi_edid_active_protocol efi_edid_active_protocol_t;
+
+union efi_edid_active_protocol {
+	struct {
+		u32 size_of_edid;
+		u8 *edid;
+	};
+	struct {
+		u32 size_of_edid;
+		u32 edid;
+	} mixed_mode;
+};
+
 typedef union {
 	struct {
 		u32			revision;
@@ -1085,7 +1114,7 @@ efi_status_t efi_parse_options(char const *cmdline);
 
 void efi_parse_option_graphics(char *option);
 
-efi_status_t efi_setup_gop(struct screen_info *si);
+efi_status_t efi_setup_graphics(struct screen_info *si, struct edid_info *edid);
 
 efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
 				  const efi_char16_t *optstr,
diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
index 02459ef0f18c..72d74436a7a4 100644
--- a/drivers/firmware/efi/libstub/gop.c
+++ b/drivers/firmware/efi/libstub/gop.c
@@ -12,6 +12,7 @@
 #include <linux/string.h>
 #include <asm/efi.h>
 #include <asm/setup.h>
+#include <video/edid.h>
 
 #include "efistub.h"
 
@@ -413,6 +414,14 @@ static void setup_screen_info(struct screen_info *si, const efi_graphics_output_
 	si->capabilities |= VIDEO_CAPABILITY_SKIP_QUIRKS;
 }
 
+static void setup_edid_info(struct edid_info *edid, u32 gop_size_of_edid, u8 *gop_edid)
+{
+	if (!gop_edid || gop_size_of_edid < 128)
+		memset(edid->dummy, 0, sizeof(edid->dummy));
+	else
+		memcpy(edid->dummy, gop_edid, min(gop_size_of_edid, sizeof(edid->dummy)));
+}
+
 static efi_handle_t find_handle_with_primary_gop(unsigned long num, const efi_handle_t handles[],
 						 efi_graphics_output_protocol_t **found_gop)
 {
@@ -469,7 +478,7 @@ static efi_handle_t find_handle_with_primary_gop(unsigned long num, const efi_ha
 	return first_gop_handle;
 }
 
-efi_status_t efi_setup_gop(struct screen_info *si)
+efi_status_t efi_setup_graphics(struct screen_info *si, struct edid_info *edid)
 {
 	efi_handle_t *handles __free(efi_pool) = NULL;
 	efi_handle_t handle;
@@ -494,5 +503,30 @@ efi_status_t efi_setup_gop(struct screen_info *si)
 	if (si)
 		setup_screen_info(si, gop);
 
+	/* Display EDID for primary GOP */
+	if (edid) {
+		efi_edid_discovered_protocol_t *discovered_edid;
+		efi_edid_active_protocol_t *active_edid;
+		u32 gop_size_of_edid = 0;
+		u8 *gop_edid = NULL;
+
+		status = efi_bs_call(handle_protocol, handle, &EFI_EDID_ACTIVE_PROTOCOL_GUID,
+				     (void **)&active_edid);
+		if (status == EFI_SUCCESS) {
+			gop_size_of_edid = active_edid->size_of_edid;
+			gop_edid = active_edid->edid;
+		} else {
+			status = efi_bs_call(handle_protocol, handle,
+					     &EFI_EDID_DISCOVERED_PROTOCOL_GUID,
+					     (void **)&discovered_edid);
+			if (status == EFI_SUCCESS) {
+				gop_size_of_edid = discovered_edid->size_of_edid;
+				gop_edid = discovered_edid->edid;
+			}
+		}
+
+		setup_edid_info(edid, gop_size_of_edid, gop_edid);
+	}
+
 	return EFI_SUCCESS;
 }
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 761121a77f9e..b68dbfd1cb87 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -389,7 +389,7 @@ static void setup_graphics(struct boot_params *boot_params)
 {
 	struct screen_info *si = memset(&boot_params->screen_info, 0, sizeof(*si));
 
-	efi_setup_gop(si);
+	efi_setup_graphics(si, NULL);
 }
 
 static void __noreturn efi_exit(efi_handle_t handle, efi_status_t status)
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 544498c89ced..11e267492efd 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -373,6 +373,8 @@ void efi_native_runtime_setup(void);
 #define EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID	EFI_GUID(0x8b843e20, 0x8132, 0x4852,  0x90, 0xcc, 0x55, 0x1a, 0x4e, 0x4a, 0x7f, 0x1c)
 #define EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL_GUID	EFI_GUID(0x05c99a21, 0xc70f, 0x4ad2,  0x8a, 0x5f, 0x35, 0xdf, 0x33, 0x43, 0xf5, 0x1e)
 #define EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID	EFI_GUID(0x9042a9de, 0x23dc, 0x4a38,  0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a)
+#define EFI_EDID_DISCOVERED_PROTOCOL_GUID	EFI_GUID(0x1c0c34f6, 0xd380, 0x41fa,  0xa0, 0x49, 0x8a, 0xd0, 0x6c, 0x1a, 0x66, 0xaa)
+#define EFI_EDID_ACTIVE_PROTOCOL_GUID		EFI_GUID(0xbd8c1056, 0x9f36, 0x44ec,  0x92, 0xa8, 0xa6, 0x33, 0x7f, 0x81, 0x79, 0x86)
 #define EFI_PCI_IO_PROTOCOL_GUID		EFI_GUID(0x4cf5b200, 0x68b8, 0x4ca5,  0x9e, 0xec, 0xb2, 0x3e, 0x3f, 0x50, 0x02, 0x9a)
 #define EFI_FILE_INFO_ID			EFI_GUID(0x09576e92, 0x6d3f, 0x11d2,  0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
 #define EFI_SYSTEM_RESOURCE_TABLE_GUID		EFI_GUID(0xb122a263, 0x3661, 0x4f68,  0x99, 0x29, 0x78, 0xf8, 0xb0, 0xd6, 0x21, 0x80)
-- 
2.51.0

[PATCH 5/5] efi/libstub: x86: Store EDID in boot_params

[Thomas Zimmermann]

Retrieve the GOP device's EDID information in the kernel's boot
parameters. Makes the data avaialble to kernel graphics code and
drives, such as efidrm.

With efidrm, the EDID is now also available to user-space compositors
via standard DRM interfaces.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/firmware/efi/libstub/x86-stub.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index b68dbfd1cb87..8c6ff0b49912 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -388,8 +388,9 @@ static void setup_quirks(struct boot_params *boot_params)
 static void setup_graphics(struct boot_params *boot_params)
 {
 	struct screen_info *si = memset(&boot_params->screen_info, 0, sizeof(*si));
+	struct edid_info *edid = memset(&boot_params->edid_info, 0, sizeof(*edid));
 
-	efi_setup_graphics(si, NULL);
+	efi_setup_graphics(si, edid);
 }
 
 static void __noreturn efi_exit(efi_handle_t handle, efi_status_t status)
-- 
2.51.0

Re: [PATCH 1/5] efi: Fix trailing whitespace in header file

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Resolve an issue with the coding style.
>
> 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 2/5] efi/libstub: gop: Find GOP handle instead of GOP data

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> The device handle of the GOP device is required to retrieve the
> correct EDID data. Find the handle instead of the GOP data. Still
> return the GOP data in the function arguments, as we already looked
> it up.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/firmware/efi/libstub/gop.c | 27 +++++++++++++++++----------
>  1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
> index 3785fb4986b4..fd32be8dd146 100644
> --- a/drivers/firmware/efi/libstub/gop.c
> +++ b/drivers/firmware/efi/libstub/gop.c
> @@ -402,12 +402,13 @@ setup_pixel_info(struct screen_info *si, u32 pixels_per_scan_line,
>  	}
>  }
>  
> -static efi_graphics_output_protocol_t *find_gop(unsigned long num,
> -						const efi_handle_t handles[])
> +static efi_handle_t find_handle_with_primary_gop(unsigned long num, const efi_handle_t handles[],
> +						 efi_graphics_output_protocol_t **found_gop)
>  {
>  	efi_graphics_output_protocol_t *first_gop;
> -	efi_handle_t h;
> +	efi_handle_t h, first_gop_handle;
>  
> +	first_gop_handle = NULL;
>  	first_gop = NULL;
>

I think the logic of this function could be simplified if you remove some
of the variables. For example, I don't think you need a fist_gop variable
anymore now that you are passing a found_gop variable as a function param.

>  	for_each_efi_handle(h, handles, num) {
> @@ -442,19 +443,25 @@ static efi_graphics_output_protocol_t *find_gop(unsigned long num,
>  		 */
>  		status = efi_bs_call(handle_protocol, h,
>  				     &EFI_CONSOLE_OUT_DEVICE_GUID, &dummy);
> -		if (status == EFI_SUCCESS)
> -			return gop;
> -
> -		if (!first_gop)
> +		if (status == EFI_SUCCESS) {
> +			if (found_gop)
> +				*found_gop = gop;
> +			return h;
> +		} else if (!first_gop_handle) {
> +			first_gop_handle = h;
>  			first_gop = gop;

You can just assign *found_gop = gop here...

> +		}
>  	}
>  
> -	return first_gop;
> +	if (found_gop)
> +		*found_gop = first_gop;

...and then this assignment won't be needed anynmore.

> +	return first_gop_handle;

Also, given that you are calling first_gop_handle to the variable to
store the first gop handle, I would for consistency name the parameter
fist_gop and just drop the local variable with the same name.

But I agree with the general logic of the patch, so regardless of what
you prefer to do:

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 3/5] efi/libstub: gop: Initialize screen_info in helper function

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Move initialization of screen_info into a single helper function.
> Frees up space in the main setup helper for adding EDID support.
> No functional changes.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/firmware/efi/libstub/gop.c | 76 +++++++++++++-----------------
>  1 file changed, 33 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
> index fd32be8dd146..02459ef0f18c 100644
> --- a/drivers/firmware/efi/libstub/gop.c
> +++ b/drivers/firmware/efi/libstub/gop.c
> @@ -367,24 +367,31 @@ static void find_bits(u32 mask, u8 *pos, u8 *size)
>  	*size = __fls(mask) - *pos + 1;
>  }
>  
> -static void
> -setup_pixel_info(struct screen_info *si, u32 pixels_per_scan_line,
> -		 efi_pixel_bitmask_t pixel_info, int pixel_format)
> +static void setup_screen_info(struct screen_info *si, const efi_graphics_output_protocol_t *gop)
>  {
> -	if (pixel_format == PIXEL_BIT_MASK) {
> -		find_bits(pixel_info.red_mask,
> -			  &si->red_pos, &si->red_size);
> -		find_bits(pixel_info.green_mask,
> -			  &si->green_pos, &si->green_size);
> -		find_bits(pixel_info.blue_mask,
> -			  &si->blue_pos, &si->blue_size);
> -		find_bits(pixel_info.reserved_mask,
> -			  &si->rsvd_pos, &si->rsvd_size);
> -		si->lfb_depth = si->red_size + si->green_size +
> -			si->blue_size + si->rsvd_size;
> -		si->lfb_linelength = (pixels_per_scan_line * si->lfb_depth) / 8;
> +	const efi_graphics_output_protocol_mode_t *mode = efi_table_attr(gop, mode);
> +	const efi_graphics_output_mode_info_t *info = efi_table_attr(mode, info);
> +
> +	si->orig_video_isVGA = VIDEO_TYPE_EFI;
> +

Not related with your patch but I dislike so much the name of this field,
since it started as a "is VGA?" bool and ended being an enum afterwards.

But I beleve we discussed this already and decided that it would be too
much churn to change it at this point.

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 4/5] efi/libstub: gop: Add support for reading EDID

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Add support for EFI_EDID_DISCOVERED_PROTOCOL and EFI_EDID_ACTIVE_PROTOCOL
> as defined in UEFI 2.8, sec 12.9. Define GUIDs and data structures in the
> rsp header files.
>
> In the GOP setup function, read the EDID of the primary GOP device. First
> try EFI_EDID_ACTIVE_PROTOCOL, which supports user-specified EDID data. Or
> else try EFI_EDID_DISCOVERED_PROTOCOL, which returns the display device's
> native EDID. If no EDID could be retrieved, clear the storage.
>
> Rename efi_setup_gop() to efi_setup_graphics() to reflect the changes
> Let callers pass an optional instance of struct edid_data, if they are
> interested.
>
> While screen_info and edid_info come from the same device handle, they
> should be considered indendent data. The former refers to the graphics

independent

> mode, the latter refers to the display device. GOP devices might not
> provide both.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

[...]

> +static void setup_edid_info(struct edid_info *edid, u32 gop_size_of_edid, u8 *gop_edid)
> +{
> +	if (!gop_edid || gop_size_of_edid < 128)

Can you define a constant for 128 instead of having a magic number ?

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 5/5] efi/libstub: x86: Store EDID in boot_params

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Retrieve the GOP device's EDID information in the kernel's boot
> parameters. Makes the data avaialble to kernel graphics code and
> drives, such as efidrm.
>
> With efidrm, the EDID is now also available to user-space compositors
> via standard DRM interfaces.
>
> 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 2/5] efi/libstub: gop: Find GOP handle instead of GOP data

[Thomas Zimmermann]

Hi Javier,

thanks for reviewing.

Am 24.10.25 um 11:47 schrieb Javier Martinez Canillas:
> Thomas Zimmermann <tzimmermann@suse.de> writes:
>
>> The device handle of the GOP device is required to retrieve the
>> correct EDID data. Find the handle instead of the GOP data. Still
>> return the GOP data in the function arguments, as we already looked
>> it up.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/firmware/efi/libstub/gop.c | 27 +++++++++++++++++----------
>>   1 file changed, 17 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
>> index 3785fb4986b4..fd32be8dd146 100644
>> --- a/drivers/firmware/efi/libstub/gop.c
>> +++ b/drivers/firmware/efi/libstub/gop.c
>> @@ -402,12 +402,13 @@ setup_pixel_info(struct screen_info *si, u32 pixels_per_scan_line,
>>   	}
>>   }
>>   
>> -static efi_graphics_output_protocol_t *find_gop(unsigned long num,
>> -						const efi_handle_t handles[])
>> +static efi_handle_t find_handle_with_primary_gop(unsigned long num, const efi_handle_t handles[],
>> +						 efi_graphics_output_protocol_t **found_gop)
>>   {
>>   	efi_graphics_output_protocol_t *first_gop;
>> -	efi_handle_t h;
>> +	efi_handle_t h, first_gop_handle;
>>   
>> +	first_gop_handle = NULL;
>>   	first_gop = NULL;
>>
> I think the logic of this function could be simplified if you remove some
> of the variables. For example, I don't think you need a fist_gop variable
> anymore now that you are passing a found_gop variable as a function param.
>
>>   	for_each_efi_handle(h, handles, num) {
>> @@ -442,19 +443,25 @@ static efi_graphics_output_protocol_t *find_gop(unsigned long num,
>>   		 */
>>   		status = efi_bs_call(handle_protocol, h,
>>   				     &EFI_CONSOLE_OUT_DEVICE_GUID, &dummy);
>> -		if (status == EFI_SUCCESS)
>> -			return gop;
>> -
>> -		if (!first_gop)
>> +		if (status == EFI_SUCCESS) {
>> +			if (found_gop)
>> +				*found_gop = gop;
>> +			return h;
>> +		} else if (!first_gop_handle) {
>> +			first_gop_handle = h;
>>   			first_gop = gop;
> You can just assign *found_gop = gop here...
>
>> +		}
>>   	}
>>   
>> -	return first_gop;
>> +	if (found_gop)
>> +		*found_gop = first_gop;
> ...and then this assignment won't be needed anynmore.

TBH I choose that style on purpose. It's easily parseable by the eye. 
found_gop is allowed be NULL and we'd have to test within the loop. I 
found this uneasy to read. And assigning *found_gop early leaks state to 
the outside before it's ready. That's probably not a problem here, but I 
find that questionable.

>
>> +	return first_gop_handle;
> Also, given that you are calling first_gop_handle to the variable to
> store the first gop handle, I would for consistency name the parameter
> fist_gop and just drop the local variable with the same name.

The found_gop is not necessarily the first_gop. We want to return the 
primary device's handle and GOP state. The first one is only returned if 
there's no clear primary one. See [1] as for how the primary is being 
detected.

[1] 
https://elixir.bootlin.com/linux/v6.17.4/source/drivers/firmware/efi/libstub/gop.c#L433

Best regards
Thomas

>
> But I agree with the general logic of the patch, so regardless of what
> you prefer to do:
>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

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

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

More specifically, could we do the following:
- move struct edid_info edid_info into common code
- pass the detected EDID info block via a EFI config table instead of
boot_params
- make CONFIG_FIRMWARE_EDID depend on (X86 || EFI_STUB)

Re: [PATCH 3/5] efi/libstub: gop: Initialize screen_info in helper function

[Thomas Zimmermann]

Hi

Am 24.10.25 um 11:53 schrieb Javier Martinez Canillas:
> Thomas Zimmermann <tzimmermann@suse.de> writes:
>
>> Move initialization of screen_info into a single helper function.
>> Frees up space in the main setup helper for adding EDID support.
>> No functional changes.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/firmware/efi/libstub/gop.c | 76 +++++++++++++-----------------
>>   1 file changed, 33 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
>> index fd32be8dd146..02459ef0f18c 100644
>> --- a/drivers/firmware/efi/libstub/gop.c
>> +++ b/drivers/firmware/efi/libstub/gop.c
>> @@ -367,24 +367,31 @@ static void find_bits(u32 mask, u8 *pos, u8 *size)
>>   	*size = __fls(mask) - *pos + 1;
>>   }
>>   
>> -static void
>> -setup_pixel_info(struct screen_info *si, u32 pixels_per_scan_line,
>> -		 efi_pixel_bitmask_t pixel_info, int pixel_format)
>> +static void setup_screen_info(struct screen_info *si, const efi_graphics_output_protocol_t *gop)
>>   {
>> -	if (pixel_format == PIXEL_BIT_MASK) {
>> -		find_bits(pixel_info.red_mask,
>> -			  &si->red_pos, &si->red_size);
>> -		find_bits(pixel_info.green_mask,
>> -			  &si->green_pos, &si->green_size);
>> -		find_bits(pixel_info.blue_mask,
>> -			  &si->blue_pos, &si->blue_size);
>> -		find_bits(pixel_info.reserved_mask,
>> -			  &si->rsvd_pos, &si->rsvd_size);
>> -		si->lfb_depth = si->red_size + si->green_size +
>> -			si->blue_size + si->rsvd_size;
>> -		si->lfb_linelength = (pixels_per_scan_line * si->lfb_depth) / 8;
>> +	const efi_graphics_output_protocol_mode_t *mode = efi_table_attr(gop, mode);
>> +	const efi_graphics_output_mode_info_t *info = efi_table_attr(mode, info);
>> +
>> +	si->orig_video_isVGA = VIDEO_TYPE_EFI;
>> +
> Not related with your patch but I dislike so much the name of this field,
> since it started as a "is VGA?" bool and ended being an enum afterwards.
>
> But I beleve we discussed this already and decided that it would be too
> much churn to change it at this point.

That's why we need helpers for decoding, such as [1]. The kernel only 
initializes screen_info to VGAC, EFI and VLFB. We could add initializer 
functions for these cases.

[1] 
https://elixir.bootlin.com/linux/v6.17.8/source/include/linux/screen_info.h#L92

Best regards
Thomas

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

-- 
--
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 4/5] efi/libstub: gop: Add support for reading EDID

[Thomas Zimmermann]

Hi Javier

Am 24.10.25 um 11:56 schrieb Javier Martinez Canillas:
> Thomas Zimmermann <tzimmermann@suse.de> writes:
>
>> Add support for EFI_EDID_DISCOVERED_PROTOCOL and EFI_EDID_ACTIVE_PROTOCOL
>> as defined in UEFI 2.8, sec 12.9. Define GUIDs and data structures in the
>> rsp header files.
>>
>> In the GOP setup function, read the EDID of the primary GOP device. First
>> try EFI_EDID_ACTIVE_PROTOCOL, which supports user-specified EDID data. Or
>> else try EFI_EDID_DISCOVERED_PROTOCOL, which returns the display device's
>> native EDID. If no EDID could be retrieved, clear the storage.
>>
>> Rename efi_setup_gop() to efi_setup_graphics() to reflect the changes
>> Let callers pass an optional instance of struct edid_data, if they are
>> interested.
>>
>> While screen_info and edid_info come from the same device handle, they
>> should be considered indendent data. The former refers to the graphics
> independent
>
>> mode, the latter refers to the display device. GOP devices might not
>> provide both.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
> [...]
>
>> +static void setup_edid_info(struct edid_info *edid, u32 gop_size_of_edid, u8 *gop_edid)
>> +{
>> +	if (!gop_edid || gop_size_of_edid < 128)
> Can you define a constant for 128 instead of having a magic number ?

Of course. FYI the number comes from the UEFI spec 2.8, sec 12.9. It 
says that the minimum EDID size is 128 bytes.

Best regards
Thomas

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

-- 
--
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 0/5] efi: x86: Provide EDID from GOP device

[Thomas Zimmermann]

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

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

[Ard Biesheuvel]

On Mon, 17 Nov 2025 at 09:02, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> 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.
>

I understand the x86 already uses edid_info for non-EFI boot, but that
doesn't mean we have to introduce new dependencies on legacy bits like
boot_params to the EFI stub.

For generic EFI, I don't think it is necessary to clone all the config
table logic with GUIDs and stuff. Instead, given that the EFI stub is
tightly coupled with the kernel anyway, we can just decide that the
config table has both a screen_info and a edid_info struct, and the
generic EFI code consuming the config table populates both.

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

[Ard Biesheuvel]

On Tue, 18 Nov 2025 at 17:52, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Mon, 17 Nov 2025 at 09:02, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >
> > 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.
> >
>
> I understand the x86 already uses edid_info for non-EFI boot, but that
> doesn't mean we have to introduce new dependencies on legacy bits like
> boot_params to the EFI stub.
>
> For generic EFI, I don't think it is necessary to clone all the config
> table logic with GUIDs and stuff. Instead, given that the EFI stub is
> tightly coupled with the kernel anyway, we can just decide that the
> config table has both a screen_info and a edid_info struct, and the
> generic EFI code consuming the config table populates both.

I've queued this up for now so it can soak in -next for a bit, but
please let's not leave non-x86 behind here.