Re: [PATCH v4 08/34] efi: Add video support to the app

Heinrich Schuchardt
2021-11-07T08:11:10Z

On 11/4/21 04:09, Simon Glass wrote:
> The current EFI video driver only works when running in the stub. In that
> case the stub calls boot services (before jumping to U-Boot proper) and
> copies the graphics info over to the efi table. This is necessary because
> the stub exits boot services before jumping to U-Boot.
>
> The app maintains access to boot services throughout its life, so does not
> need to do this. Update the driver to support calling boot services
> directly.
>
> Enable video output for the app. Note that this uses the
> EFI_GRAPHICS_OUTPUT_PROTOCOL protocol, even though it mentions vesa.
>
> A sample qemu command-line for this case is:
>
> qemu-system-x86_64 -bios /usr/share/edk2.git/ovmf-ia32/OVMF-pure-efi.fd
> -drive id=disk,file=try.img,if=none,format=raw -nic none
> -device ahci,id=ahci -device ide-hd,drive=disk,bus=ahci.0
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Add a note that EFI_GRAPHICS_OUTPUT_PROTOCOL is used
> - Update documentation
>
> arch/x86/dts/efi-x86_app.dts | 4 +++
> board/efi/efi-x86_app/Kconfig | 4 +++
> doc/develop/uefi/u-boot_on_efi.rst | 2 +-
> drivers/video/Kconfig | 2 +-
> drivers/video/efi.c | 45 ++++++++++++++++++++++++------
> include/configs/efi-x86_app.h | 6 ++--
> 6 files changed, 50 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/dts/efi-x86_app.dts b/arch/x86/dts/efi-x86_app.dts
> index 04e044a07a8..a5316e2a1a7 100644
> --- a/arch/x86/dts/efi-x86_app.dts
> +++ b/arch/x86/dts/efi-x86_app.dts
> @@ -25,4 +25,8 @@
> compatible = "efi,reset";
> u-boot,dm-pre-reloc;
> };
> + efi-fb {
> + compatible = "efi-fb";
> + };
> +
> };
> diff --git a/board/efi/efi-x86_app/Kconfig b/board/efi/efi-x86_app/Kconfig
> index e412702eed7..ecd08d73146 100644
> --- a/board/efi/efi-x86_app/Kconfig
> +++ b/board/efi/efi-x86_app/Kconfig
> @@ -12,4 +12,8 @@ config SYS_SOC
> config SYS_CONFIG_NAME
> default "efi-x86_app"
>
> +config BOARD_SPECIFIC_OPTIONS # dummy
> + def_bool y
> + imply VIDEO_EFI
> +
> endif
> diff --git a/doc/develop/uefi/u-boot_on_efi.rst b/doc/develop/uefi/u-boot_on_efi.rst
> index f275a524ce7..5f2f850f071 100644
> --- a/doc/develop/uefi/u-boot_on_efi.rst
> +++ b/doc/develop/uefi/u-boot_on_efi.rst
> @@ -265,7 +265,7 @@ This work could be extended in a number of ways:
>
> - Figure out how to solve the interrupt problem
>
> -- Add more drivers to the application side (e.g. video, block devices, USB,
> +- Add more drivers to the application side (e.g. block devices, USB,
> environment access). This would mostly be an academic exercise as a strong
> use case is not readily apparent, but it might be fun.
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 2f4650f8309..a58f87f479b 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -250,7 +250,7 @@ config VIDEO_COREBOOT
>
> config VIDEO_EFI
> bool "Enable EFI framebuffer driver support"
> - depends on EFI_STUB
> + depends on EFI_STUB || EFI_APP
> help
> Turn on this option to enable a framebuffeer driver when U-Boot is
> loaded as a payload (see README.u-boot_on_efi) by an EFI BIOS where
> diff --git a/drivers/video/efi.c b/drivers/video/efi.c
> index c248bd352a7..4e13a3efcb1 100644
> --- a/drivers/video/efi.c
> +++ b/drivers/video/efi.c
> @@ -50,6 +50,28 @@ static void efi_find_pixel_bits(u32 mask, u8 *pos, u8 *size)
> *size = len;
> }
>
> +static int get_mode_info(struct vesa_mode_info *vesa)
> +{
> + efi_guid_t efi_gop_guid = EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID;
> + struct efi_boot_services *boot = efi_get_boot();
> + struct efi_gop_mode *mode;
> + struct efi_gop *gop;
> + int ret;
> +
> + if (!boot)
> + return log_msg_ret("sys", -ENOSYS);
> + ret = boot->locate_protocol(&efi_gop_guid, NULL, (void **)&gop);
> + if (ret)
> + return log_msg_ret("prot", -ENOTSUPP);
> +
> + mode = gop->mode;
> + vesa->phys_base_ptr = mode->fb_base;
> + vesa->x_resolution = mode->info->width;
> + vesa->y_resolution = mode->info->height;
> +
> + return 0;
> +}
> +
> static int save_vesa_mode(struct vesa_mode_info *vesa)
> {
> struct efi_entry_gopmode *mode;
> @@ -57,16 +79,23 @@ static int save_vesa_mode(struct vesa_mode_info *vesa)
> int size;
> int ret;
>
> - ret = efi_info_get(EFIET_GOP_MODE, (void **)&mode, &size);
> - if (ret == -ENOENT) {
> - debug("efi graphics output protocol mode not found\n");
> - return -ENXIO;
> + if (IS_ENABLED(CONFIG_EFI_APP)) {
> + ret = get_mode_info(vesa);
> + if (ret) {
> + printf("efi graphics output protocol not found\n");
> + return -ENXIO;
> + }
> + } else {
> + ret = efi_info_get(EFIET_GOP_MODE, (void **)&mode, &size);
> + if (ret == -ENOENT) {
> + printf("efi graphics output protocol mode not found\n");

%s/efi/EFI/

I will correct this when merging.

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

> + return -ENXIO;
> + }
> + vesa->phys_base_ptr = mode->fb_base;
> + vesa->x_resolution = mode->info->width;
> + vesa->y_resolution = mode->info->height;
> }
>
> - vesa->phys_base_ptr = mode->fb_base;
> - vesa->x_resolution = mode->info->width;
> - vesa->y_resolution = mode->info->height;
> -
> if (mode->info->pixel_format < EFI_GOT_BITMASK) {
> fbinfo = &efi_framebuffer_format_map[mode->info->pixel_format];
> vesa->red_mask_size = fbinfo->red.size;
> diff --git a/include/configs/efi-x86_app.h b/include/configs/efi-x86_app.h
> index 33418cfbec4..6061a6db0a4 100644
> --- a/include/configs/efi-x86_app.h
> +++ b/include/configs/efi-x86_app.h
> @@ -10,8 +10,8 @@
>
> #undef CONFIG_TPM_TIS_BASE_ADDRESS
>
> -#define CONFIG_STD_DEVICES_SETTINGS "stdin=usbkbd,vga,serial\0" \
> - "stdout=vga,serial\0" \
> - "stderr=vga,serial\0"
> +#define CONFIG_STD_DEVICES_SETTINGS "stdin=serial\0" \
> + "stdout=vidconsole\0" \
> + "stderr=vidconsole\0"
>
> #endif
>