Re: [edk2] [PATCH v2 2/4] OvmfPkg: add QemuRamfbDxe
Hi Gerd,
On 06/12/18 11:31, Gerd Hoffmann wrote:
> Add a driver for the qemu ramfb display device.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Gerd Hoffmann
> ---
> OvmfPkg/QemuRamfbDxe/QemuRamfb.c | 396
> ++
> OvmfPkg/OvmfPkgIa32.dsc | 1 +
> OvmfPkg/OvmfPkgIa32.fdf | 1 +
> OvmfPkg/OvmfPkgIa32X64.dsc| 1 +
> OvmfPkg/OvmfPkgIa32X64.fdf| 1 +
> OvmfPkg/OvmfPkgX64.dsc| 1 +
> OvmfPkg/OvmfPkgX64.fdf| 1 +
> OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf | 52 +
> 8 files changed, 454 insertions(+)
> create mode 100644 OvmfPkg/QemuRamfbDxe/QemuRamfb.c
> create mode 100644 OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
>
> diff --git a/OvmfPkg/QemuRamfbDxe/QemuRamfb.c
> b/OvmfPkg/QemuRamfbDxe/QemuRamfb.c
> new file mode 100644
> index 00..00a2875e99
> --- /dev/null
> +++ b/OvmfPkg/QemuRamfbDxe/QemuRamfb.c
> @@ -0,0 +1,396 @@
> +/** @file
> + This driver is a implementation of the Graphics Output Protocol
> + for the QEMU ramfb device.
> +
> + Copyright (c) 2018, Red Hat Inc.
> +
> + This program and the accompanying materials
> + are licensed and made available under the terms and conditions of the BSD
> License
> + which accompanies this distribution. The full text of the license may be
> found at
(1) These lines are overlong, can you please wrap them to 80 chars?
> + http://opensource.org/licenses/bsd-license.php
> +
> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
> IMPLIED.
> +
> +**/
> +
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +#define RAMFB_FORMAT 0x34325258 /* DRM_FORMAT_XRGB */
> +#define RAMFB_BPP 4
> +
> +#pragma pack (1)
> +typedef struct RAMFB_CONFIG {
> + UINT64 Address;
> + UINT32 FourCC;
> + UINT32 Flags;
> + UINT32 Width;
> + UINT32 Height;
> + UINT32 Stride;
> +} RAMFB_CONFIG;
> +#pragma pack ()
> +
> +static EFI_HANDLEmRamfbHandle;
(2) So, when I mentioned "STATIC" in my last email, I meant "STATIC",
not "static" :) I see you couldn't believe me! :) I apologize; yes,
indeed in edk2 we spell "static" "STATIC". Can you please update all
occurrences?
> +static EFI_HANDLEmGopHandle;
> +static FRAME_BUFFER_CONFIGURE*mQemuRamfbFrameBufferBltConfigure;
> +static UINTN mQemuRamfbFrameBufferBltConfigureSize;
> +static FIRMWARE_CONFIG_ITEM mRamFbFwCfgItem;
(3) My bad, I suggested inconsistent spelling for "mRamFbFwCfgItem". You
spelled it everywhere consistently "Ramfb", in identifiers.
Can you please update this to "mRamfbFwCfgItem" too?
> +
> +static EFI_GRAPHICS_OUTPUT_MODE_INFORMATION mQemuRamfbModeInfo[] = {
> + {
> +0,// Version
> +640, // HorizontalResolution
> +480, // VerticalResolution
> + },{
> +0,// Version
> +800, // HorizontalResolution
> +600, // VerticalResolution
> + },{
> +0,// Version
> +1024, // HorizontalResolution
> +768, // VerticalResolution
> + }
> +};
> +#define mQemuRamfbModeCount ARRAY_SIZE(mQemuRamfbModeInfo)
(4) Sorry I was unclear about this in my last email. What I meant is to
eliminate the "mQemuRamfbModeCount" macro entirely, and use the
ARRAY_SIZE() in its place. I count two instances in the code.
Alternatively, if you really like a dedicated macro for this, please
spell it QEMU_RAMFB_MODE_COUNT.
Either way: please insert a space character after ARRAY_SIZE and before "(".
> +
> +static EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE mQemuRamfbMode = {
> + mQemuRamfbModeCount,// MaxMode
> + 0, // Mode
> + mQemuRamfbModeInfo, // Info
> + sizeof (EFI_GRAPHICS_OUTPUT_MODE_INFORMATION), // SizeOfInfo
> +};
> +
> +static
> +EFI_STATUS
> +EFIAPI
> +QemuRamfbGraphicsOutputQueryMode (
> + IN EFI_GRAPHICS_OUTPUT_PROTOCOL *This,
> + IN UINT32ModeNumber,
> + OUT UINTN *SizeOfInfo,
> + OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION **Info
> + )
> +{
> + EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *ModeInfo;
> +
> + if (Info == NULL || SizeOfInfo == NULL || ModeNumber >=
> mQemuRamfbMode.MaxMode) {
(5) This line is too long; please use a 80 chars width.
> +return EFI_INVALID_PARAMETER;
> + }
> + ModeInfo = [ModeNumber];
> +
> + *Info = AllocateCopyPool (sizeof (EFI_GRAPHICS_OUTPUT_MODE_INFORMATION),
> +ModeInfo);
> + if (*Info == NULL) {
> +return EFI_OUT_OF_RESOURCES;
> + }
> + *SizeOfInfo = sizeof (EFI_GRAPHICS_OUTPUT_MODE_INFORMATION);
> +
> + return EFI_SUCCESS;
> +}
> +
> +static
> +EFI_STATUS
> +EFIAPI
> +QemuRamfbGraphicsOutputSetMode (
> + IN
[edk2] [PATCH v2 2/4] OvmfPkg: add QemuRamfbDxe
Add a driver for the qemu ramfb display device.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Gerd Hoffmann
---
OvmfPkg/QemuRamfbDxe/QemuRamfb.c | 396 ++
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32.fdf | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc| 1 +
OvmfPkg/OvmfPkgIa32X64.fdf| 1 +
OvmfPkg/OvmfPkgX64.dsc| 1 +
OvmfPkg/OvmfPkgX64.fdf| 1 +
OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf | 52 +
8 files changed, 454 insertions(+)
create mode 100644 OvmfPkg/QemuRamfbDxe/QemuRamfb.c
create mode 100644 OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
diff --git a/OvmfPkg/QemuRamfbDxe/QemuRamfb.c b/OvmfPkg/QemuRamfbDxe/QemuRamfb.c
new file mode 100644
index 00..00a2875e99
--- /dev/null
+++ b/OvmfPkg/QemuRamfbDxe/QemuRamfb.c
@@ -0,0 +1,396 @@
+/** @file
+ This driver is a implementation of the Graphics Output Protocol
+ for the QEMU ramfb device.
+
+ Copyright (c) 2018, Red Hat Inc.
+
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD
License
+ which accompanies this distribution. The full text of the license may be
found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#define RAMFB_FORMAT 0x34325258 /* DRM_FORMAT_XRGB */
+#define RAMFB_BPP 4
+
+#pragma pack (1)
+typedef struct RAMFB_CONFIG {
+ UINT64 Address;
+ UINT32 FourCC;
+ UINT32 Flags;
+ UINT32 Width;
+ UINT32 Height;
+ UINT32 Stride;
+} RAMFB_CONFIG;
+#pragma pack ()
+
+static EFI_HANDLEmRamfbHandle;
+static EFI_HANDLEmGopHandle;
+static FRAME_BUFFER_CONFIGURE*mQemuRamfbFrameBufferBltConfigure;
+static UINTN mQemuRamfbFrameBufferBltConfigureSize;
+static FIRMWARE_CONFIG_ITEM mRamFbFwCfgItem;
+
+static EFI_GRAPHICS_OUTPUT_MODE_INFORMATION mQemuRamfbModeInfo[] = {
+ {
+0,// Version
+640, // HorizontalResolution
+480, // VerticalResolution
+ },{
+0,// Version
+800, // HorizontalResolution
+600, // VerticalResolution
+ },{
+0,// Version
+1024, // HorizontalResolution
+768, // VerticalResolution
+ }
+};
+#define mQemuRamfbModeCount ARRAY_SIZE(mQemuRamfbModeInfo)
+
+static EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE mQemuRamfbMode = {
+ mQemuRamfbModeCount,// MaxMode
+ 0, // Mode
+ mQemuRamfbModeInfo, // Info
+ sizeof (EFI_GRAPHICS_OUTPUT_MODE_INFORMATION), // SizeOfInfo
+};
+
+static
+EFI_STATUS
+EFIAPI
+QemuRamfbGraphicsOutputQueryMode (
+ IN EFI_GRAPHICS_OUTPUT_PROTOCOL *This,
+ IN UINT32ModeNumber,
+ OUT UINTN *SizeOfInfo,
+ OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION **Info
+ )
+{
+ EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *ModeInfo;
+
+ if (Info == NULL || SizeOfInfo == NULL || ModeNumber >=
mQemuRamfbMode.MaxMode) {
+return EFI_INVALID_PARAMETER;
+ }
+ ModeInfo = [ModeNumber];
+
+ *Info = AllocateCopyPool (sizeof (EFI_GRAPHICS_OUTPUT_MODE_INFORMATION),
+ModeInfo);
+ if (*Info == NULL) {
+return EFI_OUT_OF_RESOURCES;
+ }
+ *SizeOfInfo = sizeof (EFI_GRAPHICS_OUTPUT_MODE_INFORMATION);
+
+ return EFI_SUCCESS;
+}
+
+static
+EFI_STATUS
+EFIAPI
+QemuRamfbGraphicsOutputSetMode (
+ IN EFI_GRAPHICS_OUTPUT_PROTOCOL *This,
+ IN UINT32 ModeNumber
+ )
+{
+ EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *ModeInfo;
+ RAMFB_CONFIG Config;
+ EFI_GRAPHICS_OUTPUT_BLT_PIXEL Black;
+ RETURN_STATUS Status;
+
+ if (ModeNumber >= mQemuRamfbMode.MaxMode) {
+return EFI_UNSUPPORTED;
+ }
+ ModeInfo = [ModeNumber];
+
+ DEBUG ((DEBUG_INFO, "Ramfb: SetMode %u (%ux%u)\n", ModeNumber,
+ModeInfo->HorizontalResolution, ModeInfo->VerticalResolution));
+
+ Config.Address = SwapBytes64 (mQemuRamfbMode.FrameBufferBase);
+ Config.FourCC = SwapBytes32 (RAMFB_FORMAT);
+ Config.Flags = SwapBytes32 (0);
+ Config.Width = SwapBytes32 (ModeInfo->HorizontalResolution);
+ Config.Height = SwapBytes32 (ModeInfo->VerticalResolution);
+ Config.Stride = SwapBytes32 (ModeInfo->HorizontalResolution * RAMFB_BPP);
+
+ Status = FrameBufferBltConfigure (
+ (VOID*)(UINTN)mQemuRamfbMode.FrameBufferBase,
+ ModeInfo,
+ mQemuRamfbFrameBufferBltConfigure,
+
+ );
+
+ if (Status == RETURN_BUFFER_TOO_SMALL) {
+if (mQemuRamfbFrameBufferBltConfigure != NULL) {
+ FreePool
