[3/5] efi: try to use the currently set GOP mode

Commit Message

Roger Pau Monne Nov. 23, 2022, 3:45 p.m. UTC

Modify efi_find_gop_mode() so that passing cols or rows as 0 is
interpreted as a request to attempt to keep the currently set mode,
and do so if the mode query for information is successful and the depth
is supported.

Signed-off-by: Roger Pau Monné <[e-mail]>
---
 xen/common/efi/boot.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Jan Beulich Dec. 5, 2022, 2:32 p.m. UTC | #1

On 23.11.2022 16:45, Roger Pau Monne wrote:
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -864,6 +864,26 @@ static UINTN __init efi_find_gop_mode(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop,
>      UINTN gop_mode = ~0, info_size, size;
>      unsigned int i;
>  
> +    if ( (!cols || !rows) && gop->Mode->Mode < gop->Mode->MaxMode )
> +    {
> +        /* If no (valid) resolution suggested, try to use the current mode. */
> +        status = gop->QueryMode(gop, gop->Mode->Mode, &info_size, &mode_info);
> +        if ( EFI_ERROR(status) )
> +            PrintErr(L"Invalid current graphics mode\r\n");
> +        else if ( mode_info->PixelFormat < PixelBltOnly )
> +            return gop->Mode->Mode;
> +        else
> +        {
> +            /*
> +             * Try to find a mode with the same resolution and a valid pixel
> +             * format.
> +             */
> +            cols = mode_info->HorizontalResolution;
> +            rows = mode_info->VerticalResolution;

For these I think you want to replace cols and rows individually, i.e.

            if ( !cols )
                cols = mode_info->HorizontalResolution;
            if ( !rows )
                rows = mode_info->VerticalResolution;

whereas ...

> +            depth = 0;

... this case looks more complicated, as 0 is also already legitimate
as a value to come in. By zapping a non-zero incoming value you may end
up switching modes _just_ to alter depth, and then you may not fulfill
what was requested. For now I think depth simply wants leaving alone
here (and perhaps using the current mode's value if the incoming value
was zero, thus eliminating the need for a mode change in certain cases).

In a separate change we might then enhance this, e.g. by finding a
supported mode matching geometry but only coming close for "depth".

Jan

Patch

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 7e8a8b7857..b91a7179a9 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -864,6 +864,26 @@  static UINTN __init efi_find_gop_mode(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop,
     UINTN gop_mode = ~0, info_size, size;
     unsigned int i;
 
+    if ( (!cols || !rows) && gop->Mode->Mode < gop->Mode->MaxMode )
+    {
+        /* If no (valid) resolution suggested, try to use the current mode. */
+        status = gop->QueryMode(gop, gop->Mode->Mode, &info_size, &mode_info);
+        if ( EFI_ERROR(status) )
+            PrintErr(L"Invalid current graphics mode\r\n");
+        else if ( mode_info->PixelFormat < PixelBltOnly )
+            return gop->Mode->Mode;
+        else
+        {
+            /*
+             * Try to find a mode with the same resolution and a valid pixel
+             * format.
+             */
+            cols = mode_info->HorizontalResolution;
+            rows = mode_info->VerticalResolution;
+            depth = 0;
+        }
+    }
+
     for ( i = size = 0; i < gop->Mode->MaxMode; ++i )
     {
         unsigned int bpp = 0;