Re: [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci)

[Laszlo Ersek <lersek@redhat.com>]

On 09/05/16 14:26, Alexander Graf wrote:
> On 09/02/2016 12:02 AM, Laszlo Ersek wrote:
>> On 08/19/16 14:49, Laszlo Ersek wrote:
>>
>>> - Also tested loading and launching Linux from GRUB. Here the results
>>>    differ of course: x86_64 Fedora 24 drives virtio-gpu-pci with its
>>>    native driver without problems, whereas AARCH64 Fedora 23 doesn't
>>> even
>>>    look for virtio-gpu-pci, apparently.
>> Update:
>>
>> Once booted from GRUB -- on top of OVMF / ArmVirtQemu respectively --,
>> both x86_64 and aarch64 Fedora *24* guests work well with
>> virtio-gpu-pci, as far as the character console (tty1, "virtiodrmfb") is
>> concerned.
> 
> Hooray :).
> 
> Unfortunately grub2 has no idea what PixelBltOnly is, it only knows the
> traditional ones.

For me grub2 seems to work without any issues -- it only uses the Simple
Text Out protocol for displaying text (not graphics). The text gets
turned into graphics by edk2's GraphicsConsoleDxe, and that one knows
how to handle PixelBltOnly.

> So to make it work with graphics, I had to explicitly
> set it to the correct mode - see the patch below.

That is not the correct mode at all;
PixelBlueGreenRedReserved8BitPerColor means that the framebuffer is
accessible directly, and the pixels are represented in the framebuffer
as BGRX8 UINT32 elements.

Whereas, virtio-gpu-pci has no framebuffer at all. PixelBltOnly means
that the client can only display graphics on the device by explicitly
calling the EFI_GRAPHICS_OUTPUT_PROTOCOL.Blt() method.

> 
> Is PixelBltOnly properly defined?

Yes, it is.

> If so, someone should probably write
> up a patch for grub2 to teach it about it ;).

Actually, I'm surprised the patch you pasted below works at all. If it
does (i.e., you get pictures displayed by grub2) after applying the
patch, that means grub2 already uses Blt() only, and no direct
framebuffer access -- which is great (*). In that case, it should only
be modified not to croak on PixelBltOnly.

(*) Hmmm, from a 5-second grep, I found "grub-core/video/efi_gop.c", and
it does call Blt() in the grub_video_gop_swap_buffers() function. I have
no idea what the "grub_video_adapter.swap_buffers" member function is
for -- double buffering perhaps?

If grub2 draws only into its own memory buffers, and then sends them to
the display with Blt(), that should be okay. As long as the array format
matches the Blt() specification, that is.

Thanks
Laszlo

> 
> 
> Alex
> 
> diff --git a/OvmfPkg/VirtioGpuDxe/Gop.c b/OvmfPkg/VirtioGpuDxe/Gop.c
> index c6ff9ed..57bfd32 100644
> --- a/OvmfPkg/VirtioGpuDxe/Gop.c
> +++ b/OvmfPkg/VirtioGpuDxe/Gop.c
> @@ -215,7 +215,7 @@ GopQueryMode (
> 
>    GopModeInfo->HorizontalResolution = mGopResolutions[ModeNumber].Width;
>    GopModeInfo->VerticalResolution   = mGopResolutions[ModeNumber].Height;
> -  GopModeInfo->PixelFormat          = PixelBltOnly;
> +  GopModeInfo->PixelFormat          =
> PixelBlueGreenRedReserved8BitPerColor;
>    GopModeInfo->PixelsPerScanLine    = mGopResolutions[ModeNumber].Width;
> 
>    *SizeOfInfo = sizeof *GopModeInfo;
> @@ -263,7 +263,7 @@ GopSetMode (
>      VgpuGop->GopMode.Info            = &VgpuGop->GopModeInfo;
>      VgpuGop->GopMode.SizeOfInfo      = sizeof VgpuGop->GopModeInfo;
> 
> -    VgpuGop->GopModeInfo.PixelFormat = PixelBltOnly;
> +    VgpuGop->GopModeInfo.PixelFormat =
> PixelBlueGreenRedReserved8BitPerColor;
> 
>      //
>      // This is the first time we create a host side resource.
>