[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH RFC 2/3] xen/arm: efi: Introduce and fill the vga_console_info


  • To: Julien Grall <julien@xxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Mon, 7 Feb 2022 09:53:48 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=bbajDJNRxGbgxuYUH8E1+csdYq9ygwMnbguqJxXnRUc=; b=glZqj8e8vkqJFkxQtIwrdiNRBWCBy63oRM09mx8tUUnNxpqlqThvtrZQ+7YN54E0MvAjvlEW8mQ8+o9FJzmswiEE/sWaEgP2913TitZojHA1gwc+Ooq8GSV6UW03tBRbWOQdqqEtYCymvVO6Qluu3utOMe+mh33oWNK9AYCr1cI+wEdi1WZmI8qCaMMf/+AgATgUNyqPz2CGP8OnxQkfnnLHm66j98h5El5x8LhlDtbUEqekPUoXYfFpu1Y0Z79KKcbGpPJfveVwkKTtJBdDc8WRUhdtewHj6ERxQRUEcgLmh391ylsYkWe/FpjKhaqDnxs0XI2VAnYbagi/UR6mUQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=iU5iVbImd7RTHkKaWFP+I3kZqMkA00d4JmSYtFY7aQVc7TgwsspzFgXdNu9E7TFUhC4s6Jg9P8azDozkpE/jqnGQaxihfV67ZSGMmINcd61n56Ax4szQg47JT6gW+3zYkKJ4DYWEV4vud4MilkZl2fY1QiLnaOOm1xT0gskiS6MV6vCpSovvXTNLpoamB9NOPNLrVmmMTDxDZISFPDzV5UroTIpRsBEPvsAx/bMUudG5Q+SX8OYBS/THO3O0x0x4/A8aGbU1StBBaH0QZeaLLhQFCfzlIwBc42xFpypy1MCEP15mZ9ojr9FqcK63DT+nKsYqxsXkkqKiqt3LafsYbw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: ehem+xen@xxxxxxx, Julien Grall <jgrall@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Mon, 07 Feb 2022 08:53:52 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 06.02.2022 20:28, Julien Grall wrote:
> From: Julien Grall <jgrall@xxxxxxxxxx>
> 
> In a follow-up patch will we want to add support for EFI framebuffer
> in dom0. Yet, Xen may not use the framebuffer, so it would be ideal
> to not have to enable CONFIG_VIDEO/CONFIG_VGA.
> 
> Introduce vga_console_info in a hacky way and move the code
> to fill it up from x86 to common.
> 
> Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
> 
> ----
> 
> This is a bit of a hack. Sent early to gather opinion on whether
> we should enable allow Dom0 to use the EFI Framebuffer even
> if Xen is built with CONFIG_VIDEO=n on Arm.

I have no input here; this will need to be settled among you Arm folks.
I have no objection to the code movement, just one nit:

> @@ -1025,6 +1026,77 @@ static void __init efi_get_apple_properties(void)
>      }
>  }
>  
> +static void __init efi_video_init(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop,
> +                                  UINTN info_size,
> +                                  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION 
> *mode_info)
> +{
> +#if defined(CONFIG_VIDEO) || defined(CONFIG_ARM)
> +    int bpp = 0;
> +
> +    switch ( mode_info->PixelFormat )
> +    {
> +    case PixelRedGreenBlueReserved8BitPerColor:
> +        vga_console_info.u.vesa_lfb.red_pos = 0;
> +        vga_console_info.u.vesa_lfb.red_size = 8;
> +        vga_console_info.u.vesa_lfb.green_pos = 8;
> +        vga_console_info.u.vesa_lfb.green_size = 8;
> +        vga_console_info.u.vesa_lfb.blue_pos = 16;
> +        vga_console_info.u.vesa_lfb.blue_size = 8;
> +        vga_console_info.u.vesa_lfb.rsvd_pos = 24;
> +        vga_console_info.u.vesa_lfb.rsvd_size = 8;
> +        bpp = 32;
> +        break;
> +    case PixelBlueGreenRedReserved8BitPerColor:
> +        vga_console_info.u.vesa_lfb.red_pos = 16;
> +        vga_console_info.u.vesa_lfb.red_size = 8;
> +        vga_console_info.u.vesa_lfb.green_pos = 8;
> +        vga_console_info.u.vesa_lfb.green_size = 8;
> +        vga_console_info.u.vesa_lfb.blue_pos = 0;
> +        vga_console_info.u.vesa_lfb.blue_size = 8;
> +        vga_console_info.u.vesa_lfb.rsvd_pos = 24;
> +        vga_console_info.u.vesa_lfb.rsvd_size = 8;
> +        bpp = 32;
> +        break;
> +    case PixelBitMask:
> +        bpp = set_color(mode_info->PixelInformation.RedMask, bpp,
> +                        &vga_console_info.u.vesa_lfb.red_pos,
> +                        &vga_console_info.u.vesa_lfb.red_size);
> +        bpp = set_color(mode_info->PixelInformation.GreenMask, bpp,
> +                        &vga_console_info.u.vesa_lfb.green_pos,
> +                        &vga_console_info.u.vesa_lfb.green_size);
> +        bpp = set_color(mode_info->PixelInformation.BlueMask, bpp,
> +                        &vga_console_info.u.vesa_lfb.blue_pos,
> +                        &vga_console_info.u.vesa_lfb.blue_size);
> +        if ( mode_info->PixelInformation.ReservedMask )
> +            bpp = set_color(mode_info->PixelInformation.ReservedMask, bpp,
> +                            &vga_console_info.u.vesa_lfb.rsvd_pos,
> +                            &vga_console_info.u.vesa_lfb.rsvd_size);
> +        if ( bpp > 0 )
> +            break;
> +        /* fall through */
> +    default:
> +        PrintErr(L"Current graphics mode is unsupported!\r\n");
> +        bpp  = 0;
> +        break;
> +    }
> +    if ( bpp > 0 )
> +    {
> +        vga_console_info.video_type = XEN_VGATYPE_EFI_LFB;
> +        vga_console_info.u.vesa_lfb.gbl_caps = 2; /* possibly non-VGA */
> +        vga_console_info.u.vesa_lfb.width =
> +            mode_info->HorizontalResolution;
> +        vga_console_info.u.vesa_lfb.height = mode_info->VerticalResolution;
> +        vga_console_info.u.vesa_lfb.bits_per_pixel = bpp;
> +        vga_console_info.u.vesa_lfb.bytes_per_line =
> +            (mode_info->PixelsPerScanLine * bpp + 7) >> 3;
> +        vga_console_info.u.vesa_lfb.lfb_base = gop->Mode->FrameBufferBase;
> +        vga_console_info.u.vesa_lfb.ext_lfb_base = 
> gop->Mode->FrameBufferBase >> 32;
> +        vga_console_info.u.vesa_lfb.lfb_size =
> +            (gop->Mode->FrameBufferSize + 0xffff) >> 16;
> +    }
> +#endif
> +}

While you move this code, could you please insert blank lines between
non-fall-through case blocks, and perhaps another one between the switch()
and the if() blocks? And it looks like
- the "gop" parameter could also do with becoming pointer-to-const,
- the expanded #ifdef could do with a comment briefly explaining why Arm
  needs-special casing.

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.