On Tue, Sep 05, 2017 at 12:47:15AM +0000, Klemens Nanni wrote:
> This diff fixes missing video output when booting in UEFI mode on the
> ThinkPad X121e (and possibly other boards with crappy implemenatations)
> as reported earlier:
> 
>       https://marc.info/?l=openbsd-bugs&m=150407033628497&w=2
> 
> When looking for the best mode in terms of resolution, we might end up
> with modes providing higher resolutions than what's possible on the
> actual display. Some buggy UEFI implemenations may also fail for other
> non-obvious reasons.
> 
> Either ways, current behaviour is to just proceed after a failed attempt
> of setting some better mode. With this diff the currently working GOP
> mode is saved and set again in case SetMode() was unsuccessful.
> 
> That way my X121e boots fine using mode 7 (640x480x32bpp) instead of
> blanking out, efifb(4) comes up until radeon(4) takes over with native
> resolution.
> 
> Also tested successfully/without any regressions on a Dell Latitude
> E5550 as well as a Fujitsu Lifebook A Series boards both running
> latest stock UEFI firmware.
> 
> 
> Not sure yet whether this is a mistake or I'm not getting it, but
> either status indicating an error or ending up with an incorrect mode
> should each be enough to call it a failure.
> In fact, EFI_ERROR(status) may be non-zero although gop->Mode->Mode was
> indeed set and equal to bestmode as this was the case on my X121e.
> 
> 
> Feedback? Comments?
> 
> I also have a working and tested diff for `machine fb [n]' analog to
> `machine video [n]' that enables listing/and setting the GOP mode just
> like FreeBSD does it with `gop list | get | set <mode>'. I'd also like to
> get this in together with some other minor clean ups in case this is
> the way to go.
> 
> Testers, especially with broken implemenations like the X121e are
> greatly appreciated.
Updated diff introducing efi_gop_setmode() as helper to reduce duplicate
code and enhance readability.

This code continues to work on three different UEFI implementations
without any regression.

Has anyone tested this on their machine by any chance?


diff --git a/sys/arch/amd64/stand/efiboot/efiboot.c 
b/sys/arch/amd64/stand/efiboot/efiboot.c
index 3d245aeb458..f8196b2cbcb 100644
--- a/sys/arch/amd64/stand/efiboot/efiboot.c
+++ b/sys/arch/amd64/stand/efiboot/efiboot.c
@@ -59,6 +59,8 @@ static void    efi_heap_init(void);
 static void     efi_memprobe_internal(void);
 static void     efi_video_init(void);
 static void     efi_video_reset(void);
+static EFI_STATUS
+                efi_gop_setmode(EFI_GRAPHICS_OUTPUT *gop, int mode);
 EFI_STATUS      efi_main(EFI_HANDLE, EFI_SYSTEM_TABLE *);
 
 void (*run_i386)(u_long, u_long, int, int, int, int, int, int, int, int)
@@ -699,6 +701,18 @@ static EFI_GUID smbios_guid = SMBIOS_TABLE_GUID;
 
 #define        efi_guidcmp(_a, _b)     memcmp((_a), (_b), sizeof(EFI_GUID))
 
+static EFI_STATUS
+efi_gop_setmode(EFI_GRAPHICS_OUTPUT *gop, int mode)
+{
+       EFI_STATUS      status;
+
+       status = EFI_CALL(gop->SetMode, gop, mode);
+       if (EFI_ERROR(status) || gop->Mode->Mode != mode)
+               printf("GOP SetMode() failed (%d)\n", status);
+
+       return (status);
+}
+
 void
 efi_makebootargs(void)
 {
@@ -708,7 +722,7 @@ efi_makebootargs(void)
        EFI_GRAPHICS_OUTPUT_MODE_INFORMATION
                                *gopi;
        bios_efiinfo_t           ei;
-       int                      bestmode = -1;
+       int                      curmode, bestmode = -1;
        UINTN                    sz, gopsiz, bestsiz = 0;
        EFI_GRAPHICS_OUTPUT_MODE_INFORMATION
                                *info;
@@ -746,9 +760,10 @@ efi_makebootargs(void)
                        }
                }
                if (bestmode >= 0) {
-                       status = EFI_CALL(gop->SetMode, gop, bestmode);
-                       if (EFI_ERROR(status) && gop->Mode->Mode != bestmode)
-                               printf("GOP setmode failed(%d)\n", status);
+                       curmode = gop->Mode->Mode;
+                       if (efi_gop_setmode(gop, bestmode) !=
+                           EFI_SUCCESS)
+                               (void)efi_gop_setmode(gop, curmode);
                }
 
                gopi = gop->Mode->Info;

Reply via email to