Re: selecting proper GOP when there are multiple GOPs
On Thu, 16 Jun 2022 19:37:57 +0200 (CEST)
Mark Kettenis wrote:
>> Date: Thu, 16 Jun 2022 23:49:05 +0900 (JST)
>> From: YASUOKA Masahiko
(snip)
>> @@ -444,6 +445,30 @@ efi_video_init(void)
>> int i, mode80x25, mode100x31;
>> UINTNcols, rows;
>> EFI_STATUS status;
>> +EFI_HANDLE *handles;
>> +UINTNnhandles;
>> +EFI_GRAPHICS_OUTPUT *first_gop = NULL;
>> +EFI_DEVICE_PATH *devp_test = NULL;
>> +
>> +status = BS->LocateHandleBuffer(ByProtocol, _guid, NULL, ,
>> +);
>> +if (status != EFI_SUCCESS)
>> +panic("BS->LocateHandleBuffer() returns %d", status);
>
> What about headless machines? I suspect that most x86 machines
> without a GPU of some sorts will still provide a framebuffer of some
> sorts in their UEFI implementations. But maybe some machines don't?
I have not seen a machine which doesn't yet.
> If there are no GOP protocol handles, LocateHandleBuffer() seems to
> return EFI_NOT_FOUND, which would result in a panic, which wouldn't be
> very helpful.
Yes, previous version seems to care about that. We don't need to
change that behavior.
The diff is updated.
ok?
Index: sys/arch/amd64/stand//efiboot/conf.c
===
RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/conf.c,v
retrieving revision 1.36
diff -u -p -r1.36 conf.c
--- sys/arch/amd64/stand//efiboot/conf.c8 Jun 2021 02:45:49 -
1.36
+++ sys/arch/amd64/stand//efiboot/conf.c17 Jun 2022 00:23:40 -
@@ -40,7 +40,7 @@
#include "efidev.h"
#include "efipxe.h"
-const char version[] = "3.59";
+const char version[] = "3.60";
#ifdef EFI_DEBUG
intdebug = 0;
Index: sys/arch/amd64/stand//efiboot/efiboot.c
===
RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efiboot.c,v
retrieving revision 1.38
diff -u -p -r1.38 efiboot.c
--- sys/arch/amd64/stand//efiboot/efiboot.c 7 Jun 2021 00:04:20 -
1.38
+++ sys/arch/amd64/stand//efiboot/efiboot.c 17 Jun 2022 00:23:40 -
@@ -424,8 +424,9 @@ efi_memprobe_internal(void)
/***
* Console
***/
-static SIMPLE_TEXT_OUTPUT_INTERFACE *conout = NULL;
-static SIMPLE_INPUT_INTERFACE *conin;
+static SIMPLE_TEXT_OUTPUT_INTERFACE*conout = NULL;
+static SIMPLE_INPUT_INTERFACE *conin;
+static EFI_GRAPHICS_OUTPUT *gop = NULL;
static EFI_GUID con_guid
= EFI_CONSOLE_CONTROL_PROTOCOL_GUID;
static EFI_GUID gop_guid
@@ -444,6 +445,28 @@ efi_video_init(void)
int i, mode80x25, mode100x31;
UINTNcols, rows;
EFI_STATUS status;
+ EFI_HANDLE *handles;
+ UINTNnhandles;
+ EFI_GRAPHICS_OUTPUT *first_gop = NULL;
+ EFI_DEVICE_PATH *devp_test = NULL;
+
+ status = BS->LocateHandleBuffer(ByProtocol, _guid, NULL, ,
+ );
+ if (!EFI_ERROR(status)) {
+ for (i = 0; i < nhandles; i++) {
+ status = BS->HandleProtocol(handles[i], _guid,
+ (void **));
+ if (first_gop == NULL)
+ first_gop = gop;
+ status = BS->HandleProtocol(handles[i], _guid,
+ (void **)_test);
+ if (status == EFI_SUCCESS)
+ break;
+ }
+ if (status != EFI_SUCCESS)
+ gop = first_gop;
+ BS->FreePool(handles);
+ }
conout = ST->ConOut;
status = BS->LocateProtocol(_guid, NULL, (void **));
@@ -808,7 +831,6 @@ efi_com_putc(dev_t dev, int c)
*/
static EFI_GUID acpi_guid = ACPI_20_TABLE_GUID;
static EFI_GUID smbios_guid = SMBIOS_TABLE_GUID;
-static EFI_GRAPHICS_OUTPUT *gop;
static int gopmode = -1;
#defineefi_guidcmp(_a, _b) memcmp((_a), (_b), sizeof(EFI_GUID))
@@ -853,8 +875,7 @@ efi_makebootargs(void)
/*
* Frame buffer
*/
- status = BS->LocateProtocol(_guid, NULL, (void **));
- if (!EFI_ERROR(status)) {
+ if (gop != NULL) {
if (gopmode < 0) {
for (i = 0; i < gop->Mode->MaxMode; i++) {
status = gop->QueryMode(gop, i, , );
@@ -1030,10 +1051,10 @@ Xgop_efi(void)
Re: selecting proper GOP when there are multiple GOPs
> Date: Thu, 16 Jun 2022 23:49:05 +0900 (JST)
> From: YASUOKA Masahiko
>
> On Thu, 16 Jun 2022 15:52:41 +0300
> Nick Henderson wrote:
> > Any updates on this patch? Would love to see it included in the next
> > release.
>
> Yes.
>
> I'll commit this this weekend even if I don't get no ok.
>
> ok?
Sorry. I forgot about this diff.
> Index: sys/arch/amd64/stand/efiboot/efiboot.c
> ===
> RCS file: /disk/cvs/openbsd/src/sys/arch/amd64/stand/efiboot/efiboot.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 efiboot.c
> --- sys/arch/amd64/stand/efiboot/efiboot.c7 Jun 2021 00:04:20 -
> 1.38
> +++ sys/arch/amd64/stand/efiboot/efiboot.c2 May 2022 07:53:38 -
> @@ -424,8 +424,9 @@ efi_memprobe_internal(void)
> /***
> * Console
> ***/
> -static SIMPLE_TEXT_OUTPUT_INTERFACE *conout = NULL;
> -static SIMPLE_INPUT_INTERFACE *conin;
> +static SIMPLE_TEXT_OUTPUT_INTERFACE *conout = NULL;
> +static SIMPLE_INPUT_INTERFACE*conin;
> +static EFI_GRAPHICS_OUTPUT *gop = NULL;
> static EFI_GUID con_guid
> = EFI_CONSOLE_CONTROL_PROTOCOL_GUID;
> static EFI_GUID gop_guid
> @@ -444,6 +445,30 @@ efi_video_init(void)
> int i, mode80x25, mode100x31;
> UINTNcols, rows;
> EFI_STATUS status;
> + EFI_HANDLE *handles;
> + UINTNnhandles;
> + EFI_GRAPHICS_OUTPUT *first_gop = NULL;
> + EFI_DEVICE_PATH *devp_test = NULL;
> +
> + status = BS->LocateHandleBuffer(ByProtocol, _guid, NULL, ,
> + );
> + if (status != EFI_SUCCESS)
> + panic("BS->LocateHandleBuffer() returns %d", status);
What about headless machines? I suspect that most x86 machines
without a GPU of some sorts will still provide a framebuffer of some
sorts in their UEFI implementations. But maybe some machines don't?
If there are no GOP protocol handles, LocateHandleBuffer() seems to
return EFI_NOT_FOUND, which would result in a panic, which wouldn't be
very helpful.
> + for (i = 0; i < nhandles; i++) {
> + status = BS->HandleProtocol(handles[i], _guid,
> + (void **));
> + if (first_gop == NULL)
> + first_gop = gop;
> + status = BS->HandleProtocol(handles[i], _guid,
> + (void **)_test);
> + if (status == EFI_SUCCESS)
> + break;
> + }
> + if (status != EFI_SUCCESS)
> + gop = first_gop;
> + if (gop == NULL)
> + panic("no gop found");
> + BS->FreePool(handles);
>
> conout = ST->ConOut;
> status = BS->LocateProtocol(_guid, NULL, (void **));
> @@ -808,7 +833,6 @@ efi_com_putc(dev_t dev, int c)
> */
> static EFI_GUID acpi_guid = ACPI_20_TABLE_GUID;
> static EFI_GUID smbios_guid = SMBIOS_TABLE_GUID;
> -static EFI_GRAPHICS_OUTPUT *gop;
> static intgopmode = -1;
>
> #define efi_guidcmp(_a, _b) memcmp((_a), (_b), sizeof(EFI_GUID))
> @@ -853,57 +877,54 @@ efi_makebootargs(void)
> /*
>* Frame buffer
>*/
> - status = BS->LocateProtocol(_guid, NULL, (void **));
> - if (!EFI_ERROR(status)) {
> - if (gopmode < 0) {
> - for (i = 0; i < gop->Mode->MaxMode; i++) {
> - status = gop->QueryMode(gop, i, , );
> - if (EFI_ERROR(status))
> - continue;
> - gopsiz = gopi->HorizontalResolution *
> - gopi->VerticalResolution;
> - if (gopsiz > bestsiz) {
> - gopmode = i;
> - bestsiz = gopsiz;
> - }
> + if (gopmode < 0) {
> + for (i = 0; i < gop->Mode->MaxMode; i++) {
> + status = gop->QueryMode(gop, i, , );
> + if (EFI_ERROR(status))
> + continue;
> + gopsiz = gopi->HorizontalResolution *
> + gopi->VerticalResolution;
> + if (gopsiz > bestsiz) {
> + gopmode = i;
> + bestsiz = gopsiz;
> }
> }
> - if (gopmode >= 0 && gopmode != gop->Mode->Mode) {
> - curmode = gop->Mode->Mode;
> - if (efi_gop_setmode(gopmode) != EFI_SUCCESS)
> -
Re: selecting proper GOP when there are multiple GOPs
On Thu, 16 Jun 2022 15:52:41 +0300
Nick Henderson wrote:
> Any updates on this patch? Would love to see it included in the next release.
Yes.
I'll commit this this weekend even if I don't get no ok.
ok?
Index: sys/arch/amd64/stand/efiboot/efiboot.c
===
RCS file: /disk/cvs/openbsd/src/sys/arch/amd64/stand/efiboot/efiboot.c,v
retrieving revision 1.38
diff -u -p -r1.38 efiboot.c
--- sys/arch/amd64/stand/efiboot/efiboot.c 7 Jun 2021 00:04:20 -
1.38
+++ sys/arch/amd64/stand/efiboot/efiboot.c 2 May 2022 07:53:38 -
@@ -424,8 +424,9 @@ efi_memprobe_internal(void)
/***
* Console
***/
-static SIMPLE_TEXT_OUTPUT_INTERFACE *conout = NULL;
-static SIMPLE_INPUT_INTERFACE *conin;
+static SIMPLE_TEXT_OUTPUT_INTERFACE*conout = NULL;
+static SIMPLE_INPUT_INTERFACE *conin;
+static EFI_GRAPHICS_OUTPUT *gop = NULL;
static EFI_GUID con_guid
= EFI_CONSOLE_CONTROL_PROTOCOL_GUID;
static EFI_GUID gop_guid
@@ -444,6 +445,30 @@ efi_video_init(void)
int i, mode80x25, mode100x31;
UINTNcols, rows;
EFI_STATUS status;
+ EFI_HANDLE *handles;
+ UINTNnhandles;
+ EFI_GRAPHICS_OUTPUT *first_gop = NULL;
+ EFI_DEVICE_PATH *devp_test = NULL;
+
+ status = BS->LocateHandleBuffer(ByProtocol, _guid, NULL, ,
+ );
+ if (status != EFI_SUCCESS)
+ panic("BS->LocateHandleBuffer() returns %d", status);
+ for (i = 0; i < nhandles; i++) {
+ status = BS->HandleProtocol(handles[i], _guid,
+ (void **));
+ if (first_gop == NULL)
+ first_gop = gop;
+ status = BS->HandleProtocol(handles[i], _guid,
+ (void **)_test);
+ if (status == EFI_SUCCESS)
+ break;
+ }
+ if (status != EFI_SUCCESS)
+ gop = first_gop;
+ if (gop == NULL)
+ panic("no gop found");
+ BS->FreePool(handles);
conout = ST->ConOut;
status = BS->LocateProtocol(_guid, NULL, (void **));
@@ -808,7 +833,6 @@ efi_com_putc(dev_t dev, int c)
*/
static EFI_GUID acpi_guid = ACPI_20_TABLE_GUID;
static EFI_GUID smbios_guid = SMBIOS_TABLE_GUID;
-static EFI_GRAPHICS_OUTPUT *gop;
static int gopmode = -1;
#defineefi_guidcmp(_a, _b) memcmp((_a), (_b), sizeof(EFI_GUID))
@@ -853,57 +877,54 @@ efi_makebootargs(void)
/*
* Frame buffer
*/
- status = BS->LocateProtocol(_guid, NULL, (void **));
- if (!EFI_ERROR(status)) {
- if (gopmode < 0) {
- for (i = 0; i < gop->Mode->MaxMode; i++) {
- status = gop->QueryMode(gop, i, , );
- if (EFI_ERROR(status))
- continue;
- gopsiz = gopi->HorizontalResolution *
- gopi->VerticalResolution;
- if (gopsiz > bestsiz) {
- gopmode = i;
- bestsiz = gopsiz;
- }
+ if (gopmode < 0) {
+ for (i = 0; i < gop->Mode->MaxMode; i++) {
+ status = gop->QueryMode(gop, i, , );
+ if (EFI_ERROR(status))
+ continue;
+ gopsiz = gopi->HorizontalResolution *
+ gopi->VerticalResolution;
+ if (gopsiz > bestsiz) {
+ gopmode = i;
+ bestsiz = gopsiz;
}
}
- if (gopmode >= 0 && gopmode != gop->Mode->Mode) {
- curmode = gop->Mode->Mode;
- if (efi_gop_setmode(gopmode) != EFI_SUCCESS)
- (void)efi_gop_setmode(curmode);
- }
-
- gopi = gop->Mode->Info;
- switch (gopi->PixelFormat) {
- case PixelBlueGreenRedReserved8BitPerColor:
- ei->fb_red_mask = 0x00ff;
- ei->fb_green_mask= 0xff00;
- ei->fb_blue_mask = 0x00ff;
- ei->fb_reserved_mask = 0xff00;
- break;
- case PixelRedGreenBlueReserved8BitPerColor:
