[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v4 3/8] x86/EFI: retrieve EDID
On Thu, Mar 31, 2022 at 11:45:36AM +0200, Jan Beulich wrote: > When booting directly from EFI, obtaining this information from EFI is > the only possible way. And even when booting with a boot loader > interposed, it's more clean not to use legacy BIOS calls for this > purpose. (The downside being that there are no "capabilities" that we > can retrieve the EFI way.) > > To achieve this we need to propagate the handle used to obtain the > EFI_GRAPHICS_OUTPUT_PROTOCOL instance for further obtaining an > EFI_EDID_*_PROTOCOL instance, which has been part of the spec since 2.5. > > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> > --- > Setting boot_edid_caps to zero isn't desirable, but arbitrarily setting > one or both of the two low bits also doesn't seem appropriate. > > GrUB also checks an "agp-internal-edid" variable. As I haven't been able > to find any related documentation, and as GrUB being happy about the > variable being any size (rather than at least / precisely 128 bytes), > I didn't follow that route. > --- > v3: Re-base. > v2: New. > > --- a/xen/arch/arm/efi/efi-boot.h > +++ b/xen/arch/arm/efi/efi-boot.h > @@ -464,6 +464,10 @@ static void __init efi_arch_edd(void) > { > } > > +static void __init efi_arch_edid(EFI_HANDLE gop_handle) > +{ > +} > + > static void __init efi_arch_memory_setup(void) > { > } > --- a/xen/arch/x86/boot/video.S > +++ b/xen/arch/x86/boot/video.S > @@ -922,7 +922,14 @@ store_edid: > pushw %dx > pushw %di > > - cmpb $1, bootsym(opt_edid) # EDID disabled on cmdline (edid=no)? > + movb bootsym(opt_edid), %al > + cmpw $0x1313, bootsym(boot_edid_caps) # Data already retrieved? > + je .Lcheck_edid > + cmpb $2, %al # EDID forced on cmdline > (edid=force)? > + jne .Lno_edid > + > +.Lcheck_edid: > + cmpb $1, %al # EDID disabled on cmdline (edid=no)? > je .Lno_edid > > leaw vesa_glob_info, %di > --- a/xen/arch/x86/efi/efi-boot.h > +++ b/xen/arch/x86/efi/efi-boot.h > @@ -568,6 +568,49 @@ static void __init efi_arch_video_init(E > #endif > } > > +#ifdef CONFIG_VIDEO > +static bool __init copy_edid(const void *buf, unsigned int size) > +{ > + /* > + * Be conservative - for both undersized and oversized blobs it is > unclear > + * what to actually do with them. The more that unlike the VESA BIOS > + * interface we also have no associated "capabilities" value (which might > + * carry a hint as to possible interpretation). > + */ > + if ( size != ARRAY_SIZE(boot_edid_info) ) > + return false; > + > + memcpy(boot_edid_info, buf, size); > + boot_edid_caps = 0; > + > + return true; > +} > +#endif > + > +static void __init efi_arch_edid(EFI_HANDLE gop_handle) > +{ > +#ifdef CONFIG_VIDEO > + static EFI_GUID __initdata active_guid = EFI_EDID_ACTIVE_PROTOCOL_GUID; > + static EFI_GUID __initdata discovered_guid = > EFI_EDID_DISCOVERED_PROTOCOL_GUID; Is there a need to make those static? I think this function is either called from efi_start or efi_multiboot, but there aren't multiple calls to it? (also both parameters are IN only, so not to be changed by the EFI method? I have the feeling setting them to static is done because they can't be set to const? > + EFI_EDID_ACTIVE_PROTOCOL *active_edid; > + EFI_EDID_DISCOVERED_PROTOCOL *discovered_edid; > + EFI_STATUS status; > + > + status = efi_bs->OpenProtocol(gop_handle, &active_guid, > + (void **)&active_edid, efi_ih, NULL, > + EFI_OPEN_PROTOCOL_GET_PROTOCOL); > + if ( status == EFI_SUCCESS && > + copy_edid(active_edid->Edid, active_edid->SizeOfEdid) ) > + return; Isn't it enough to just call EFI_EDID_ACTIVE_PROTOCOL_GUID? >From my reading of the UEFI spec this will either return EFI_EDID_OVERRIDE_PROTOCOL_GUID or EFI_EDID_DISCOVERED_PROTOCOL_GUID. If EFI_EDID_OVERRIDE_PROTOCOL is set it must be used, and hence falling back to EFI_EDID_DISCOVERED_PROTOCOL_GUID if EFI_EDID_ACTIVE_PROTOCOL_GUID cannot be parsed would likely mean ignoring EFI_EDID_OVERRIDE_PROTOCOL? > + status = efi_bs->OpenProtocol(gop_handle, &discovered_guid, > + (void **)&discovered_edid, efi_ih, NULL, > + EFI_OPEN_PROTOCOL_GET_PROTOCOL); > + if ( status == EFI_SUCCESS ) > + copy_edid(discovered_edid->Edid, discovered_edid->SizeOfEdid); > +#endif > +} > + > static void __init efi_arch_memory_setup(void) > { > unsigned int i; > @@ -729,6 +772,7 @@ static void __init efi_arch_flush_dcache > void __init efi_multiboot2(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE > *SystemTable) > { > EFI_GRAPHICS_OUTPUT_PROTOCOL *gop; > + EFI_HANDLE gop_handle; > UINTN cols, gop_mode = ~0, rows; > > __set_bit(EFI_BOOT, &efi_flags); > @@ -742,11 +786,15 @@ void __init efi_multiboot2(EFI_HANDLE Im > &cols, &rows) == EFI_SUCCESS ) > efi_arch_console_init(cols, rows); > > - gop = efi_get_gop(); > + gop = efi_get_gop(&gop_handle); > > if ( gop ) > + { > gop_mode = efi_find_gop_mode(gop, 0, 0, 0); > > + efi_arch_edid(gop_handle); > + } > + > efi_arch_edd(); > efi_arch_cpu(); > > --- a/xen/common/efi/boot.c > +++ b/xen/common/efi/boot.c > @@ -118,7 +118,7 @@ static bool read_section(const EFI_LOADE > > static void efi_init(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable); > static void efi_console_set_mode(void); > -static EFI_GRAPHICS_OUTPUT_PROTOCOL *efi_get_gop(void); > +static EFI_GRAPHICS_OUTPUT_PROTOCOL *efi_get_gop(EFI_HANDLE *gop_handle); > static UINTN efi_find_gop_mode(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop, > UINTN cols, UINTN rows, UINTN depth); > static void efi_tables(void); > @@ -758,7 +758,7 @@ static void __init efi_console_set_mode( > StdOut->SetMode(StdOut, best); > } > > -static EFI_GRAPHICS_OUTPUT_PROTOCOL __init *efi_get_gop(void) > +static EFI_GRAPHICS_OUTPUT_PROTOCOL __init *efi_get_gop(EFI_HANDLE > *gop_handle) > { > EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *mode_info; > EFI_GRAPHICS_OUTPUT_PROTOCOL *gop = NULL; > @@ -783,7 +783,10 @@ static EFI_GRAPHICS_OUTPUT_PROTOCOL __in > continue; > status = gop->QueryMode(gop, gop->Mode->Mode, &info_size, > &mode_info); > if ( !EFI_ERROR(status) ) > + { > + *gop_handle = handles[i]; > break; > + } > } > if ( handles ) > efi_bs->FreePool(handles); > @@ -1222,6 +1225,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SY > if ( use_cfg_file ) > { > EFI_FILE_HANDLE dir_handle; > + EFI_HANDLE gop_handle; > UINTN depth, cols, rows, size; > > size = cols = rows = depth = 0; > @@ -1230,7 +1234,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SY > &cols, &rows) == EFI_SUCCESS ) > efi_arch_console_init(cols, rows); > > - gop = efi_get_gop(); > + gop = efi_get_gop(&gop_handle); > > /* Get the file system interface. */ > dir_handle = get_parent_handle(loaded_image, &file_name); > @@ -1360,7 +1364,11 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SY > dir_handle->Close(dir_handle); > > if ( gop && !base_video ) > + { > gop_mode = efi_find_gop_mode(gop, cols, rows, depth); > + > + efi_arch_edid(gop_handle); > + } > } > > /* Get the number of boot modules specified on the DT or an error (<0) */ > @@ -1387,7 +1395,6 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SY > > efi_arch_edd(); > > - /* XXX Collect EDID info. */ > efi_arch_cpu(); > > efi_tables(); > --- a/xen/include/efi/efiprot.h > +++ b/xen/include/efi/efiprot.h > @@ -724,5 +724,52 @@ struct _EFI_GRAPHICS_OUTPUT_PROTOCOL { > EFI_GRAPHICS_OUTPUT_PROTOCOL_BLT Blt; > EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE *Mode; > }; > + > +/* > + * EFI EDID Discovered Protocol > + * UEFI Specification Version 2.5 Section 11.9 > + */ > +#define EFI_EDID_DISCOVERED_PROTOCOL_GUID \ > + { 0x1C0C34F6, 0xD380, 0x41FA, { 0xA0, 0x49, 0x8a, 0xD0, 0x6C, 0x1A, > 0x66, 0xAA} } > + > +typedef struct _EFI_EDID_DISCOVERED_PROTOCOL { > + UINT32 SizeOfEdid; > + UINT8 *Edid; > +} EFI_EDID_DISCOVERED_PROTOCOL; > + > +/* > + * EFI EDID Active Protocol > + * UEFI Specification Version 2.5 Section 11.9 > + */ > +#define EFI_EDID_ACTIVE_PROTOCOL_GUID \ > + { 0xBD8C1056, 0x9F36, 0x44EC, { 0x92, 0xA8, 0xA6, 0x33, 0x7F, 0x81, > 0x79, 0x86} } > + > +typedef struct _EFI_EDID_ACTIVE_PROTOCOL { > + UINT32 SizeOfEdid; > + UINT8 *Edid; > +} EFI_EDID_ACTIVE_PROTOCOL; > + > +/* > + * EFI EDID Override Protocol > + * UEFI Specification Version 2.5 Section 11.9 > + */ > +#define EFI_EDID_OVERRIDE_PROTOCOL_GUID \ > + { 0x48ECB431, 0xFB72, 0x45C0, { 0xA9, 0x22, 0xF4, 0x58, 0xFE, 0x04, > 0x0B, 0xD5} } > + > +INTERFACE_DECL(_EFI_EDID_OVERRIDE_PROTOCOL); > + > +typedef > +EFI_STATUS > +(EFIAPI *EFI_EDID_OVERRIDE_PROTOCOL_GET_EDID) ( > + IN struct _EFI_EDID_OVERRIDE_PROTOCOL *This, > + IN EFI_HANDLE *ChildHandle, > + OUT UINT32 *Attributes, > + IN OUT UINTN *EdidSize, > + IN OUT UINT8 **Edid); > + > +typedef struct _EFI_EDID_OVERRIDE_PROTOCOL { > + EFI_EDID_OVERRIDE_PROTOCOL_GET_EDID GetEdid; > +} EFI_EDID_OVERRIDE_PROTOCOL; > + > #endif FWIW, EFI_EDID_OVERRIDE_PROTOCOL_GUID is not used by the patch, so I guess it's introduced for completeness (or because it's used by further patches). Thanks, Roger.
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |