Hi, > See my updated diff for reusing the gopi struct, please.
ok, but the diff seems to be against wrong revision. You seems to have other diffs, moving gop and gopi to global at least. Can you send it entirely? On Sat, 7 Oct 2017 02:09:29 +0200 Klemens Nanni <[email protected]> wrote: > On Fri, Oct 06, 2017 at 05:15:20AM +0000, YASUOKA Masahiko wrote: >> On Tue, 3 Oct 2017 18:15:33 -0500 >> Andrew Daugherity <[email protected]> wrote: >> > I tested this diff in combination with your "Implement machine gop command" >> > diff on a Dell PowerEdge R230 and a VirtualBox VM (EFI enabled). No >> > regressions, however 'machine gop' doesn't work quite how I expected it to >> > -- it seems the video mode set with it only applies to the bootloader, and >> > once the kernel loads, it sets a different mode (possibly a bad one). I >> > was hoping the kernel would use the mode I just set. >> >> I also expected that the kernel uses the mode to which the gop command >> set. I think it's more useful. >> >> The diff is updated. >> >> comments? > See my updated diff for reusing the gopi struct, please. > > Searching for and setting a better mode only if the user didn't pick one > manually before is definitely a good idea, thanks. > > Udated diff implementing `machine gop [n]' attached. > >> Index: sys/arch/amd64/stand/efiboot/efiboot.c >> =================================================================== >> RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efiboot.c,v >> retrieving revision 1.24 >> diff -u -p -r1.24 efiboot.c >> --- sys/arch/amd64/stand/efiboot/efiboot.c 6 Oct 2017 04:52:22 -0000 >> 1.24 >> +++ sys/arch/amd64/stand/efiboot/efiboot.c 6 Oct 2017 05:09:29 -0000 >> @@ -51,6 +51,7 @@ static EFI_GUID imgp_guid = LOADED_IMA >> static EFI_GUID blkio_guid = BLOCK_IO_PROTOCOL; >> static EFI_GUID devp_guid = DEVICE_PATH_PROTOCOL; >> u_long efi_loadaddr; >> +static int efi_gopmode = -1; >> >> static int efi_device_path_depth(EFI_DEVICE_PATH *dp, int); >> static int efi_device_path_ncmp(EFI_DEVICE_PATH *, EFI_DEVICE_PATH *, >> @@ -722,7 +723,7 @@ efi_makebootargs(void) >> EFI_GRAPHICS_OUTPUT_MODE_INFORMATION >> *gopi; >> bios_efiinfo_t ei; >> - int curmode, bestmode = -1; >> + int curmode; >> UINTN sz, gopsiz, bestsiz = 0; >> EFI_GRAPHICS_OUTPUT_MODE_INFORMATION >> *info; >> @@ -748,20 +749,23 @@ efi_makebootargs(void) >> status = EFI_CALL(BS->LocateProtocol, &gop_guid, NULL, >> (void **)&gop); >> if (!EFI_ERROR(status)) { >> - for (i = 0; i < gop->Mode->MaxMode; i++) { >> - status = EFI_CALL(gop->QueryMode, gop, i, &sz, &info); >> - if (EFI_ERROR(status)) >> - continue; >> - gopsiz = info->HorizontalResolution * >> - info->VerticalResolution; >> - if (gopsiz > bestsiz) { >> - bestmode = i; >> - bestsiz = gopsiz; >> + if (efi_gopmode < 0) { >> + for (i = 0; i < gop->Mode->MaxMode; i++) { >> + status = EFI_CALL(gop->QueryMode, gop, i, &sz, >> + &info); >> + if (EFI_ERROR(status)) >> + continue; >> + gopsiz = info->HorizontalResolution * >> + info->VerticalResolution; >> + if (gopsiz > bestsiz) { >> + efi_gopmode = i; >> + bestsiz = gopsiz; >> + } >> } >> } >> - if (bestmode >= 0) { >> + if (efi_gopmode >= 0 && efi_gopmode != gop->Mode->Mode) { >> curmode = gop->Mode->Mode; >> - if (efi_gop_setmode(gop, bestmode) != EFI_SUCCESS) >> + if (efi_gop_setmode(gop, efi_gopmode) != EFI_SUCCESS) >> (void)efi_gop_setmode(gop, curmode); >> } >> >> @@ -882,5 +886,49 @@ int >> Xpoweroff_efi(void) >> { >> EFI_CALL(RS->ResetSystem, EfiResetShutdown, EFI_SUCCESS, 0, NULL); >> + return (0); >> +} >> + >> +int >> +Xgop_efi(void) >> +{ >> + EFI_STATUS status; >> + EFI_GRAPHICS_OUTPUT *gop; >> + EFI_GRAPHICS_OUTPUT_MODE_INFORMATION >> + *info; >> + int i, mode = -1; >> + UINTN sz; >> + >> + status = EFI_CALL(BS->LocateProtocol, &gop_guid, NULL, >> + (void **)&gop); >> + if (EFI_ERROR(status)) >> + return (0); >> + >> + if (cmd.argc >= 2) { >> + mode = strtol(cmd.argv[1], NULL, 10); >> + if (0 <= mode && mode < gop->Mode->MaxMode) { >> + status = EFI_CALL(gop->QueryMode, gop, mode, >> + &sz, &info); >> + if (!EFI_ERROR(status)) { >> + if (efi_gop_setmode(gop, mode) >> + == EFI_SUCCESS) >> + efi_gopmode = mode; >> + } >> + } >> + } else { >> + for (i = 0; i < gop->Mode->MaxMode; i++) { >> + status = EFI_CALL(gop->QueryMode, gop, i, &sz, >> + &info); >> + if (EFI_ERROR(status)) >> + continue; >> + printf("Mode %d: %d x %d (stride = %d)\n", i, >> + info->HorizontalResolution, >> + info->VerticalResolution, >> + info->PixelsPerScanLine); >> + } >> + printf("\n"); >> + } >> + printf("Current Mode = %d\n", gop->Mode->Mode); >> + >> return (0); >> } >> Index: sys/arch/amd64/stand/efiboot/efiboot.h >> =================================================================== >> RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efiboot.h,v >> retrieving revision 1.2 >> diff -u -p -r1.2 efiboot.h >> --- sys/arch/amd64/stand/efiboot/efiboot.h 31 May 2017 08:40:32 -0000 >> 1.2 >> +++ sys/arch/amd64/stand/efiboot/efiboot.h 6 Oct 2017 05:09:29 -0000 >> @@ -30,6 +30,7 @@ void efi_com_init(struct consdev *); >> int efi_com_getc(dev_t); >> void efi_com_putc(dev_t, int); >> int Xvideo_efi(void); >> +int Xgop_efi(void); >> int Xexit_efi(void); >> void efi_makebootargs(void); >> >> Index: sys/arch/amd64/stand/libsa/cmd_i386.c >> =================================================================== >> RCS file: /cvs/src/sys/arch/amd64/stand/libsa/cmd_i386.c,v >> retrieving revision 1.11 >> diff -u -p -r1.11 cmd_i386.c >> --- sys/arch/amd64/stand/libsa/cmd_i386.c 31 May 2017 08:23:33 -0000 >> 1.11 >> +++ sys/arch/amd64/stand/libsa/cmd_i386.c 6 Oct 2017 05:09:29 -0000 >> @@ -62,6 +62,7 @@ const struct cmd_table cmd_machine[] = { >> { "memory", CMDT_CMD, Xmemory }, >> #ifdef EFIBOOT >> { "video", CMDT_CMD, Xvideo_efi }, >> + { "gop", CMDT_CMD, Xgop_efi }, >> { "exit", CMDT_CMD, Xexit_efi }, >> { "poweroff", CMDT_CMD, Xpoweroff_efi }, >> #endif >> >> > > > Implement `machine gop [n]' completely analogue to `machine video [n]' > in the bootloader so users can list and set available frame buffer modes. > --- > sys/arch/amd64/stand/efiboot/efiboot.c | 68 > ++++++++++++++++++++++++++++------ > sys/arch/amd64/stand/efiboot/efiboot.h | 1 + > sys/arch/amd64/stand/libsa/cmd_i386.c | 1 + > 3 files changed, 58 insertions(+), 12 deletions(-) > > diff --git a/sys/arch/amd64/stand/efiboot/efiboot.c > b/sys/arch/amd64/stand/efiboot/efiboot.c > index 283f9ab356e..cc04b07dd52 100644 > --- a/sys/arch/amd64/stand/efiboot/efiboot.c > +++ b/sys/arch/amd64/stand/efiboot/efiboot.c > @@ -708,6 +708,7 @@ static EFI_GUID smbios_guid = > SMBIOS_TABLE_GUID; > static EFI_GRAPHICS_OUTPUT *gop; > static EFI_GRAPHICS_OUTPUT_MODE_INFORMATION > *gopi; > +static int efi_gopmode = -1; > > #define efi_guidcmp(_a, _b) memcmp((_a), (_b), sizeof(EFI_GUID)) > > @@ -729,7 +730,7 @@ efi_makebootargs(void) > int i; > EFI_STATUS status; > bios_efiinfo_t ei; > - int curmode, bestmode = -1; > + int curmode; > UINTN sz, gopsiz, bestsiz = 0; > > memset(&ei, 0, sizeof(ei)); > @@ -753,20 +754,23 @@ efi_makebootargs(void) > status = EFI_CALL(BS->LocateProtocol, &gop_guid, NULL, > (void **)&gop); > if (!EFI_ERROR(status)) { > - for (i = 0; i < gop->Mode->MaxMode; i++) { > - status = EFI_CALL(gop->QueryMode, gop, i, &sz, &gopi); > - if (EFI_ERROR(status)) > - continue; > - gopsiz = gopi->HorizontalResolution * > - gopi->VerticalResolution; > - if (gopsiz > bestsiz) { > - bestmode = i; > - bestsiz = gopsiz; > + if (efi_gopmode < 0) { > + for (i = 0; i < gop->Mode->MaxMode; i++) { > + status = EFI_CALL(gop->QueryMode, gop, > + i, &sz, &gopi); > + if (EFI_ERROR(status)) > + continue; > + gopsiz = gopi->HorizontalResolution * > + gopi->VerticalResolution; > + if (gopsiz > bestsiz) { > + efi_gopmode = i; > + bestsiz = gopsiz; > + } > } > } > - if (bestmode >= 0) { > + if (efi_gopmode >= 0 && efi_gopmode != gop->Mode->Mode) { > curmode = gop->Mode->Mode; > - if (efi_gop_setmode(bestmode) != EFI_SUCCESS) > + if (efi_gop_setmode(efi_gopmode) != EFI_SUCCESS) > (void)efi_gop_setmode(curmode); > } > > @@ -892,3 +896,43 @@ Xpoweroff_efi(void) > EFI_CALL(RS->ResetSystem, EfiResetShutdown, EFI_SUCCESS, 0, NULL); > return (0); > } > + > +int > +Xgop_efi(void) > +{ > + EFI_STATUS status; > + int i, mode = -1; > + UINTN sz; > + > + status = EFI_CALL(BS->LocateProtocol, &gop_guid, NULL, > + (void **)&gop); > + if (EFI_ERROR(status)) > + return (0); > + > + if (cmd.argc >= 2) { > + mode = strtol(cmd.argv[1], NULL, 10); > + if (0 <= mode && mode < gop->Mode->MaxMode) { > + status = EFI_CALL(gop->QueryMode, gop, mode, > + &sz, &gopi); > + if (!EFI_ERROR(status)) { > + if (efi_gop_setmode(mode) == EFI_SUCCESS) > + efi_gopmode = mode; > + } > + } > + } else { > + for (i = 0; i < gop->Mode->MaxMode; i++) { > + status = EFI_CALL(gop->QueryMode, gop, i, &sz, > + &gopi); > + if (EFI_ERROR(status)) > + continue; > + printf("Mode %d: %d x %d (stride = %d)\n", i, > + gopi->HorizontalResolution, > + gopi->VerticalResolution, > + gopi->PixelsPerScanLine); > + } > + printf("\n"); > + } > + printf("Current Mode = %d\n", gop->Mode->Mode); > + > + return (0); > +} > diff --git a/sys/arch/amd64/stand/efiboot/efiboot.h > b/sys/arch/amd64/stand/efiboot/efiboot.h > index 8a07d5b46b3..3f5f5b3352c 100644 > --- a/sys/arch/amd64/stand/efiboot/efiboot.h > +++ b/sys/arch/amd64/stand/efiboot/efiboot.h > @@ -30,6 +30,7 @@ void efi_com_init(struct consdev *); > int efi_com_getc(dev_t); > void efi_com_putc(dev_t, int); > int Xvideo_efi(void); > +int Xgop_efi(void); > int Xexit_efi(void); > void efi_makebootargs(void); > > diff --git a/sys/arch/amd64/stand/libsa/cmd_i386.c > b/sys/arch/amd64/stand/libsa/cmd_i386.c > index 592cca49547..ee3b7f3cd63 100644 > --- a/sys/arch/amd64/stand/libsa/cmd_i386.c > +++ b/sys/arch/amd64/stand/libsa/cmd_i386.c > @@ -62,6 +62,7 @@ const struct cmd_table cmd_machine[] = { > { "memory", CMDT_CMD, Xmemory }, > #ifdef EFIBOOT > { "video", CMDT_CMD, Xvideo_efi }, > + { "gop", CMDT_CMD, Xgop_efi }, > { "exit", CMDT_CMD, Xexit_efi }, > { "poweroff", CMDT_CMD, Xpoweroff_efi }, > #endif > -- > 2.14.2 > >
