* [PATCH v2 0/4] Add QemuRamfbDxe driver
@ 2018-06-12 9:31 Gerd Hoffmann
2018-06-12 9:31 ` [PATCH v2 1/4] OvmfPkg: add QEMU_RAMFB_GUID Gerd Hoffmann
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2018-06-12 9:31 UTC (permalink / raw)
To: edk2-devel
Hi,
This is the efi driver for qemu ramfb, a simple boot framebuffer. Qemu
patches just have been posted to qemu-devel.
v2: tons of codestyle fixes, some control flow tweaks.
cheers,
Gerd
Gerd Hoffmann (4):
OvmfPkg: add QEMU_RAMFB_GUID
OvmfPkg: add QemuRamfbDxe
OvmfPkg: add QemuRamfb to platform console
ArmVirtPkg: add QemuRamfbDxe
OvmfPkg/Include/Guid/QemuRamfb.h | 25 ++
.../Library/PlatformBootManagerLib/PlatformData.c | 51 +++
OvmfPkg/QemuRamfbDxe/QemuRamfb.c | 396 +++++++++++++++++++++
ArmVirtPkg/ArmVirtQemu.dsc | 2 +
ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 1 +
ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 +
OvmfPkg/OvmfPkg.dec | 1 +
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 +++
14 files changed, 536 insertions(+)
create mode 100644 OvmfPkg/Include/Guid/QemuRamfb.h
create mode 100644 OvmfPkg/QemuRamfbDxe/QemuRamfb.c
create mode 100644 OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
--
2.9.3
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/4] OvmfPkg: add QEMU_RAMFB_GUID
2018-06-12 9:31 [PATCH v2 0/4] Add QemuRamfbDxe driver Gerd Hoffmann
@ 2018-06-12 9:31 ` Gerd Hoffmann
2018-06-12 15:46 ` Laszlo Ersek
2018-06-12 9:31 ` [PATCH v2 2/4] OvmfPkg: add QemuRamfbDxe Gerd Hoffmann
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Gerd Hoffmann @ 2018-06-12 9:31 UTC (permalink / raw)
To: edk2-devel
Add GUID header file for the QemuRamfbDxe driver.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
OvmfPkg/Include/Guid/QemuRamfb.h | 25 +++++++++++++++++++++++++
OvmfPkg/OvmfPkg.dec | 1 +
2 files changed, 26 insertions(+)
create mode 100644 OvmfPkg/Include/Guid/QemuRamfb.h
diff --git a/OvmfPkg/Include/Guid/QemuRamfb.h b/OvmfPkg/Include/Guid/QemuRamfb.h
new file mode 100644
index 0000000000..5c9dffb1f0
--- /dev/null
+++ b/OvmfPkg/Include/Guid/QemuRamfb.h
@@ -0,0 +1,25 @@
+/** @file
+ Recommended GUID to be used in the Vendor Hardware device path nodes that
+ identify qemu ramfb devices.
+
+ 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 that 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.
+
+**/
+
+#ifndef __QEMU_RAMFB_H__
+#define __QEMU_RAMFB_H__
+
+#define QEMU_RAMFB_GUID \
+{0x557423a1, 0x63ab, 0x406c, {0xbe, 0x7e, 0x91, 0xcd, 0xbc, 0x08, 0xc4, 0x57}}
+
+extern EFI_GUID gQemuRamfbGuid;
+
+#endif
diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index dc5597db41..7666297cf8 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -75,6 +75,7 @@
gEfiXenInfoGuid = {0xd3b46f3b, 0xd441, 0x1244, {0x9a, 0x12, 0x0, 0x12, 0x27, 0x3f, 0xc1, 0x4d}}
gOvmfPlatformConfigGuid = {0x7235c51c, 0x0c80, 0x4cab, {0x87, 0xac, 0x3b, 0x08, 0x4a, 0x63, 0x04, 0xb1}}
gVirtioMmioTransportGuid = {0x837dca9e, 0xe874, 0x4d82, {0xb2, 0x9a, 0x23, 0xfe, 0x0e, 0x23, 0xd1, 0xe2}}
+ gQemuRamfbGuid = {0x557423a1, 0x63ab, 0x406c, {0xbe, 0x7e, 0x91, 0xcd, 0xbc, 0x08, 0xc4, 0x57}}
gXenBusRootDeviceGuid = {0xa732241f, 0x383d, 0x4d9c, {0x8a, 0xe1, 0x8e, 0x09, 0x83, 0x75, 0x89, 0xd7}}
gRootBridgesConnectedEventGroupGuid = {0x24a2d66f, 0xeedd, 0x4086, {0x90, 0x42, 0xf2, 0x6e, 0x47, 0x97, 0xee, 0x69}}
--
2.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/4] OvmfPkg: add QemuRamfbDxe
2018-06-12 9:31 [PATCH v2 0/4] Add QemuRamfbDxe driver Gerd Hoffmann
2018-06-12 9:31 ` [PATCH v2 1/4] OvmfPkg: add QEMU_RAMFB_GUID Gerd Hoffmann
@ 2018-06-12 9:31 ` Gerd Hoffmann
2018-06-12 17:50 ` Laszlo Ersek
2018-06-12 9:31 ` [PATCH v2 3/4] OvmfPkg: add QemuRamfb to platform console Gerd Hoffmann
2018-06-12 9:31 ` [PATCH v2 4/4] ArmVirtPkg: add QemuRamfbDxe Gerd Hoffmann
3 siblings, 1 reply; 9+ messages in thread
From: Gerd Hoffmann @ 2018-06-12 9:31 UTC (permalink / raw)
To: edk2-devel
Add a driver for the qemu ramfb display device.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
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 0000000000..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 <Protocol/GraphicsOutput.h>
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DevicePathLib.h>
+#include <Library/FrameBufferBltLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/QemuFwCfgLib.h>
+
+#include <Guid/QemuRamfb.h>
+
+#define RAMFB_FORMAT 0x34325258 /* DRM_FORMAT_XRGB8888 */
+#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_HANDLE mRamfbHandle;
+static EFI_HANDLE mGopHandle;
+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 UINT32 ModeNumber,
+ 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 = &mQemuRamfbModeInfo[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 = &mQemuRamfbModeInfo[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,
+ &mQemuRamfbFrameBufferBltConfigureSize
+ );
+
+ if (Status == RETURN_BUFFER_TOO_SMALL) {
+ if (mQemuRamfbFrameBufferBltConfigure != NULL) {
+ FreePool (mQemuRamfbFrameBufferBltConfigure);
+ }
+ mQemuRamfbFrameBufferBltConfigure =
+ AllocatePool (mQemuRamfbFrameBufferBltConfigureSize);
+ if (mQemuRamfbFrameBufferBltConfigure == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ Status = FrameBufferBltConfigure (
+ (VOID*)(UINTN)mQemuRamfbMode.FrameBufferBase,
+ ModeInfo,
+ mQemuRamfbFrameBufferBltConfigure,
+ &mQemuRamfbFrameBufferBltConfigureSize);
+ if (RETURN_ERROR (Status)) {
+ ASSERT (Status == RETURN_UNSUPPORTED);
+ return Status;
+ }
+ }
+
+ mQemuRamfbMode.Mode = ModeNumber;
+ mQemuRamfbMode.Info = ModeInfo;
+
+ QemuFwCfgSelectItem (mRamFbFwCfgItem);
+ QemuFwCfgWriteBytes (sizeof (Config), &Config);
+
+ //
+ // clear screen
+ //
+ ZeroMem (&Black, sizeof (Black));
+ Status = FrameBufferBlt (
+ mQemuRamfbFrameBufferBltConfigure,
+ &Black,
+ EfiBltVideoFill,
+ 0, // SourceX -- ignored
+ 0, // SourceY -- ignored
+ 0, // DestinationX
+ 0, // DestinationY
+ ModeInfo->HorizontalResolution, // Width
+ ModeInfo->VerticalResolution, // Height
+ 0 // Delta -- ignored
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_WARN, "%a: clearing the screen failed: %r\n",
+ __FUNCTION__, Status));
+ }
+
+ return EFI_SUCCESS;
+}
+
+static
+EFI_STATUS
+EFIAPI
+QemuRamfbGraphicsOutputBlt (
+ IN EFI_GRAPHICS_OUTPUT_PROTOCOL *This,
+ IN EFI_GRAPHICS_OUTPUT_BLT_PIXEL *BltBuffer, OPTIONAL
+ IN EFI_GRAPHICS_OUTPUT_BLT_OPERATION BltOperation,
+ IN UINTN SourceX,
+ IN UINTN SourceY,
+ IN UINTN DestinationX,
+ IN UINTN DestinationY,
+ IN UINTN Width,
+ IN UINTN Height,
+ IN UINTN Delta
+ )
+{
+ return FrameBufferBlt (
+ mQemuRamfbFrameBufferBltConfigure,
+ BltBuffer,
+ BltOperation,
+ SourceX,
+ SourceY,
+ DestinationX,
+ DestinationY,
+ Width,
+ Height,
+ Delta);
+}
+
+EFI_GRAPHICS_OUTPUT_PROTOCOL QemuRamfbGraphicsOutput = {
+ QemuRamfbGraphicsOutputQueryMode,
+ QemuRamfbGraphicsOutputSetMode,
+ QemuRamfbGraphicsOutputBlt,
+ &mQemuRamfbMode,
+};
+
+EFI_STATUS
+EFIAPI
+InitializeQemuRamfb (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_DEVICE_PATH_PROTOCOL *RamfbDevicePath;
+ EFI_DEVICE_PATH_PROTOCOL *GopDevicePath;
+ VOID *DevicePath;
+ VENDOR_DEVICE_PATH VendorDeviceNode;
+ ACPI_ADR_DEVICE_PATH AcpiDeviceNode;
+ EFI_STATUS Status;
+ EFI_PHYSICAL_ADDRESS FbBase;
+ UINTN FbSize, MaxFbSize;
+ UINTN Size, Pages, Index;
+
+ if (!QemuFwCfgIsAvailable()) {
+ DEBUG ((DEBUG_INFO, "Ramfb: no FwCfg\n"));
+ return EFI_NOT_FOUND;
+ }
+
+ Status = QemuFwCfgFindFile("etc/ramfb", &mRamFbFwCfgItem, &Size);
+ if (EFI_ERROR (Status)) {
+ return EFI_NOT_FOUND;
+ }
+ if (Size != sizeof (RAMFB_CONFIG)) {
+ DEBUG ((DEBUG_ERROR, "Ramfb: FwCfg size mismatch (expected %d, got %d)\n",
+ sizeof (RAMFB_CONFIG), Size));
+ return EFI_NOT_FOUND;
+ }
+
+ MaxFbSize = 0;
+ for (Index = 0; Index < mQemuRamfbModeCount; Index++) {
+ mQemuRamfbModeInfo[Index].PixelsPerScanLine =
+ mQemuRamfbModeInfo[Index].HorizontalResolution;
+ mQemuRamfbModeInfo[Index].PixelFormat =
+ PixelBlueGreenRedReserved8BitPerColor;
+ FbSize = RAMFB_BPP *
+ mQemuRamfbModeInfo[Index].HorizontalResolution *
+ mQemuRamfbModeInfo[Index].VerticalResolution;
+ if (MaxFbSize < FbSize) {
+ MaxFbSize = FbSize;
+ }
+ DEBUG ((DEBUG_INFO, "Ramfb: Mode %u: %ux%u, %lu kB\n", (UINT64)Index,
+ mQemuRamfbModeInfo[Index].HorizontalResolution,
+ mQemuRamfbModeInfo[Index].VerticalResolution,
+ (UINT64)FbSize / 1024));
+ }
+
+ Pages = EFI_SIZE_TO_PAGES (MaxFbSize);
+ MaxFbSize = EFI_PAGES_TO_SIZE (Pages);
+ FbBase = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocateReservedPages (Pages);
+ if (FbBase == 0) {
+ DEBUG ((DEBUG_INFO, "Ramfb: memory allocation failed\n"));
+ return EFI_OUT_OF_RESOURCES;
+ }
+ DEBUG ((DEBUG_INFO, "Ramfb: Framebuffer at 0x%lx, %ld kB, %ld pages\n",
+ (UINT64)FbBase, (UINT64)MaxFbSize / 1024, (UINT64)Pages));
+ mQemuRamfbMode.FrameBufferSize = MaxFbSize;
+ mQemuRamfbMode.FrameBufferBase = FbBase;
+
+ //
+ // 800 x 600
+ //
+ QemuRamfbGraphicsOutputSetMode (&QemuRamfbGraphicsOutput, 1);
+
+ //
+ // ramfb vendor devpath
+ //
+ VendorDeviceNode.Header.Type = HARDWARE_DEVICE_PATH;
+ VendorDeviceNode.Header.SubType = HW_VENDOR_DP;
+ CopyGuid(&VendorDeviceNode.Guid, &gQemuRamfbGuid);
+ SetDevicePathNodeLength (&VendorDeviceNode.Header,
+ sizeof (VENDOR_DEVICE_PATH));
+
+ RamfbDevicePath = AppendDevicePathNode (NULL,
+ (EFI_DEVICE_PATH_PROTOCOL *) &VendorDeviceNode);
+ if (RamfbDevicePath == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto FreeFramebuffer;
+ }
+
+ Status = gBS->InstallMultipleProtocolInterfaces (
+ &mRamfbHandle,
+ &gEfiDevicePathProtocolGuid,
+ RamfbDevicePath,
+ NULL
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Ramfb: install Ramfb Vendor DevicePath failed: %r\n",
+ Status));
+ goto FreeRamfbDevicePath;
+ }
+
+ //
+ // gop devpath + protocol
+ //
+ ZeroMem (&AcpiDeviceNode, sizeof (ACPI_ADR_DEVICE_PATH));
+ AcpiDeviceNode.Header.Type = ACPI_DEVICE_PATH;
+ AcpiDeviceNode.Header.SubType = ACPI_ADR_DP;
+ AcpiDeviceNode.ADR = ACPI_DISPLAY_ADR (
+ 1, // DeviceIdScheme
+ 0, // HeadId
+ 0, // NonVgaOutput
+ 1, // BiosCanDetect
+ 0, // VendorInfo
+ ACPI_ADR_DISPLAY_TYPE_EXTERNAL_DIGITAL, // Type
+ 0, // Port
+ 0 // Index
+ );
+ SetDevicePathNodeLength (&AcpiDeviceNode.Header,
+ sizeof (ACPI_ADR_DEVICE_PATH));
+
+ GopDevicePath = AppendDevicePathNode (RamfbDevicePath,
+ (EFI_DEVICE_PATH_PROTOCOL *) &AcpiDeviceNode);
+ if (GopDevicePath == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto FreeRamfbHandle;
+ }
+
+ Status = gBS->InstallMultipleProtocolInterfaces (
+ &mGopHandle,
+ &gEfiDevicePathProtocolGuid,
+ GopDevicePath,
+ &gEfiGraphicsOutputProtocolGuid,
+ &QemuRamfbGraphicsOutput,
+ NULL
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Ramfb: install GOP DevicePath failed: %r\n",
+ Status));
+ goto FreeGopDevicePath;
+ }
+
+ Status = gBS->OpenProtocol (
+ mRamfbHandle,
+ &gEfiDevicePathProtocolGuid,
+ &DevicePath,
+ gImageHandle,
+ mGopHandle,
+ EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Ramfb: OpenProtocol failed: %r\n", Status));
+ goto FreeGopHandle;
+ }
+
+ return EFI_SUCCESS;
+
+FreeGopHandle:
+ gBS->UninstallMultipleProtocolInterfaces (
+ mGopHandle,
+ &gEfiDevicePathProtocolGuid,
+ GopDevicePath,
+ &gEfiGraphicsOutputProtocolGuid,
+ &QemuRamfbGraphicsOutput,
+ NULL
+ );
+FreeGopDevicePath:
+ FreePool (GopDevicePath);
+FreeRamfbHandle:
+ gBS->UninstallMultipleProtocolInterfaces (
+ mRamfbHandle,
+ &gEfiDevicePathProtocolGuid,
+ RamfbDevicePath,
+ NULL
+ );
+FreeRamfbDevicePath:
+ FreePool (RamfbDevicePath);
+FreeFramebuffer:
+ FreePool ((VOID*)(UINTN)mQemuRamfbMode.FrameBufferBase);
+ return Status;
+}
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index a2c995b910..7ddda89999 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -745,6 +745,7 @@
MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+ OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
#
diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index b199713925..52b8b1fea1 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -351,6 +351,7 @@ INF RuleOverride=CSM OvmfPkg/Csm/Csm16/Csm16.inf
!endif
INF OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+INF OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
INF OvmfPkg/PlatformDxe/Platform.inf
INF OvmfPkg/IoMmuDxe/IoMmuDxe.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index bc7db229d2..3481cdc36b 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -754,6 +754,7 @@
MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+ OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
#
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index 4ebf64b2b9..70845d6972 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -357,6 +357,7 @@ INF RuleOverride=CSM OvmfPkg/Csm/Csm16/Csm16.inf
!endif
INF OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+INF OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
INF OvmfPkg/PlatformDxe/Platform.inf
INF OvmfPkg/AmdSevDxe/AmdSevDxe.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 0767b34d18..8b0895b0ff 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -752,6 +752,7 @@
MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+ OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
#
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 9ca96f9282..1eb46ac9a2 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -357,6 +357,7 @@ INF RuleOverride=CSM OvmfPkg/Csm/Csm16/Csm16.inf
!endif
INF OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+INF OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
INF OvmfPkg/PlatformDxe/Platform.inf
INF OvmfPkg/AmdSevDxe/AmdSevDxe.inf
diff --git a/OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf b/OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
new file mode 100644
index 0000000000..5a9fd42c32
--- /dev/null
+++ b/OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
@@ -0,0 +1,52 @@
+## @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.
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = QemuRamfbDxe
+ FILE_GUID = dce1b094-7dc6-45d0-9fdd-d7fc3cc3e4ef
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+
+ ENTRY_POINT = InitializeQemuRamfb
+
+[Sources]
+ QemuRamfb.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+ BaseLib
+ BaseMemoryLib
+ DebugLib
+ DevicePathLib
+ FrameBufferBltLib
+ MemoryAllocationLib
+ UefiBootServicesTableLib
+ UefiDriverEntryPoint
+ QemuFwCfgLib
+
+[Protocols]
+ gEfiGraphicsOutputProtocolGuid # BY_START
+
+[Guids]
+ gQemuRamfbGuid
+
+[Depex]
+ TRUE
--
2.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/4] OvmfPkg: add QemuRamfb to platform console
2018-06-12 9:31 [PATCH v2 0/4] Add QemuRamfbDxe driver Gerd Hoffmann
2018-06-12 9:31 ` [PATCH v2 1/4] OvmfPkg: add QEMU_RAMFB_GUID Gerd Hoffmann
2018-06-12 9:31 ` [PATCH v2 2/4] OvmfPkg: add QemuRamfbDxe Gerd Hoffmann
@ 2018-06-12 9:31 ` Gerd Hoffmann
2018-06-12 15:57 ` Laszlo Ersek
2018-06-12 9:31 ` [PATCH v2 4/4] ArmVirtPkg: add QemuRamfbDxe Gerd Hoffmann
3 siblings, 1 reply; 9+ messages in thread
From: Gerd Hoffmann @ 2018-06-12 9:31 UTC (permalink / raw)
To: edk2-devel
Add QemuRamfbDxe device path to the list of platform console devices,
so ConSplitter will pick up the device even though it isn't a PCI GPU.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
.../Library/PlatformBootManagerLib/PlatformData.c | 51 ++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c b/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
index a50cd7bcaf..1250a6d351 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
+++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
@@ -14,6 +14,7 @@
**/
#include "BdsPlatform.h"
+#include <Guid/QemuRamfb.h>
//
// Debug Agent UART Device Path structure
@@ -37,6 +38,17 @@ typedef struct {
} USB_KEYBOARD_DEVICE_PATH;
#pragma pack ()
+//
+// QemuRamfb Device Path structure
+//
+#pragma pack (1)
+typedef struct {
+ VENDOR_DEVICE_PATH Vendor;
+ ACPI_ADR_DEVICE_PATH AcpiAdr;
+ EFI_DEVICE_PATH_PROTOCOL End;
+} VENDOR_RAMFB_DEVICE_PATH;
+#pragma pack ()
+
ACPI_HID_DEVICE_PATH gPnpPs2KeyboardDeviceNode = gPnpPs2Keyboard;
ACPI_HID_DEVICE_PATH gPnp16550ComPortDeviceNode = gPnp16550ComPort;
UART_DEVICE_PATH gUartDeviceNode = gUart;
@@ -100,6 +112,41 @@ STATIC USB_KEYBOARD_DEVICE_PATH gUsbKeyboardDevicePath = {
gEndEntire
};
+STATIC VENDOR_RAMFB_DEVICE_PATH gQemuRamfbDevicePath = {
+ {
+ {
+ HARDWARE_DEVICE_PATH,
+ HW_VENDOR_DP,
+ {
+ (UINT8) (sizeof (VENDOR_DEVICE_PATH)),
+ (UINT8) ((sizeof (VENDOR_DEVICE_PATH)) >> 8)
+ }
+ },
+ QEMU_RAMFB_GUID,
+ },
+ {
+ {
+ ACPI_DEVICE_PATH,
+ ACPI_ADR_DP,
+ {
+ (UINT8) (sizeof (ACPI_ADR_DEVICE_PATH)),
+ (UINT8) ((sizeof (ACPI_ADR_DEVICE_PATH)) >> 8)
+ }
+ },
+ ACPI_DISPLAY_ADR (
+ 1, // DeviceIdScheme
+ 0, // HeadId
+ 0, // NonVgaOutput
+ 1, // BiosCanDetect
+ 0, // VendorInfo
+ ACPI_ADR_DISPLAY_TYPE_EXTERNAL_DIGITAL, // Type
+ 0, // Port
+ 0 // Index
+ ),
+ },
+ gEndEntire
+};
+
//
// Predefined platform default console device path
//
@@ -113,6 +160,10 @@ PLATFORM_CONSOLE_CONNECT_ENTRY gPlatformConsole[] = {
CONSOLE_IN
},
{
+ (EFI_DEVICE_PATH_PROTOCOL *)&gQemuRamfbDevicePath,
+ CONSOLE_OUT
+ },
+ {
NULL,
0
}
--
2.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 4/4] ArmVirtPkg: add QemuRamfbDxe
2018-06-12 9:31 [PATCH v2 0/4] Add QemuRamfbDxe driver Gerd Hoffmann
` (2 preceding siblings ...)
2018-06-12 9:31 ` [PATCH v2 3/4] OvmfPkg: add QemuRamfb to platform console Gerd Hoffmann
@ 2018-06-12 9:31 ` Gerd Hoffmann
2018-06-12 15:59 ` Laszlo Ersek
3 siblings, 1 reply; 9+ messages in thread
From: Gerd Hoffmann @ 2018-06-12 9:31 UTC (permalink / raw)
To: edk2-devel
Add QemuRamfbDxe to dsc and fdf files for ArmVirt package.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
ArmVirtPkg/ArmVirtQemu.dsc | 2 ++
ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 1 +
ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 ++
3 files changed, 5 insertions(+)
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index d74feb709c..744d127a10 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -57,6 +57,7 @@
BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
+ FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
@@ -391,6 +392,7 @@
#
# Video support
#
+ OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
OvmfPkg/PlatformDxe/Platform.inf
diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
index 89f95b2d99..63a202c788 100644
--- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
+++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
@@ -175,6 +175,7 @@ READ_LOCK_STATUS = TRUE
#
# Video support
#
+ INF OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
INF OvmfPkg/PlatformDxe/Platform.inf
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index 1e823aeab7..e59f53b728 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -57,6 +57,7 @@
BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
+ FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
@@ -380,6 +381,7 @@
#
# Video support
#
+ OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
OvmfPkg/PlatformDxe/Platform.inf
--
2.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/4] OvmfPkg: add QEMU_RAMFB_GUID
2018-06-12 9:31 ` [PATCH v2 1/4] OvmfPkg: add QEMU_RAMFB_GUID Gerd Hoffmann
@ 2018-06-12 15:46 ` Laszlo Ersek
0 siblings, 0 replies; 9+ messages in thread
From: Laszlo Ersek @ 2018-06-12 15:46 UTC (permalink / raw)
To: Gerd Hoffmann, edk2-devel
On 06/12/18 11:31, Gerd Hoffmann wrote:
> Add GUID header file for the QemuRamfbDxe driver.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> OvmfPkg/Include/Guid/QemuRamfb.h | 25 +++++++++++++++++++++++++
> OvmfPkg/OvmfPkg.dec | 1 +
> 2 files changed, 26 insertions(+)
> create mode 100644 OvmfPkg/Include/Guid/QemuRamfb.h
>
> diff --git a/OvmfPkg/Include/Guid/QemuRamfb.h b/OvmfPkg/Include/Guid/QemuRamfb.h
> new file mode 100644
> index 0000000000..5c9dffb1f0
> --- /dev/null
> +++ b/OvmfPkg/Include/Guid/QemuRamfb.h
> @@ -0,0 +1,25 @@
> +/** @file
> + Recommended GUID to be used in the Vendor Hardware device path nodes that
> + identify qemu ramfb devices.
> +
> + 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 that 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.
> +
> +**/
> +
> +#ifndef __QEMU_RAMFB_H__
> +#define __QEMU_RAMFB_H__
> +
> +#define QEMU_RAMFB_GUID \
> +{0x557423a1, 0x63ab, 0x406c, {0xbe, 0x7e, 0x91, 0xcd, 0xbc, 0x08, 0xc4, 0x57}}
> +
> +extern EFI_GUID gQemuRamfbGuid;
> +
> +#endif
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index dc5597db41..7666297cf8 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -75,6 +75,7 @@
> gEfiXenInfoGuid = {0xd3b46f3b, 0xd441, 0x1244, {0x9a, 0x12, 0x0, 0x12, 0x27, 0x3f, 0xc1, 0x4d}}
> gOvmfPlatformConfigGuid = {0x7235c51c, 0x0c80, 0x4cab, {0x87, 0xac, 0x3b, 0x08, 0x4a, 0x63, 0x04, 0xb1}}
> gVirtioMmioTransportGuid = {0x837dca9e, 0xe874, 0x4d82, {0xb2, 0x9a, 0x23, 0xfe, 0x0e, 0x23, 0xd1, 0xe2}}
> + gQemuRamfbGuid = {0x557423a1, 0x63ab, 0x406c, {0xbe, 0x7e, 0x91, 0xcd, 0xbc, 0x08, 0xc4, 0x57}}
> gXenBusRootDeviceGuid = {0xa732241f, 0x383d, 0x4d9c, {0x8a, 0xe1, 0x8e, 0x09, 0x83, 0x75, 0x89, 0xd7}}
> gRootBridgesConnectedEventGroupGuid = {0x24a2d66f, 0xeedd, 0x4086, {0x90, 0x42, 0xf2, 0x6e, 0x47, 0x97, 0xee, 0x69}}
>
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/4] OvmfPkg: add QemuRamfb to platform console
2018-06-12 9:31 ` [PATCH v2 3/4] OvmfPkg: add QemuRamfb to platform console Gerd Hoffmann
@ 2018-06-12 15:57 ` Laszlo Ersek
0 siblings, 0 replies; 9+ messages in thread
From: Laszlo Ersek @ 2018-06-12 15:57 UTC (permalink / raw)
To: Gerd Hoffmann, edk2-devel
On 06/12/18 11:31, Gerd Hoffmann wrote:
> Add QemuRamfbDxe device path to the list of platform console devices,
> so ConSplitter will pick up the device even though it isn't a PCI GPU.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> .../Library/PlatformBootManagerLib/PlatformData.c | 51 ++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
>
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c b/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
> index a50cd7bcaf..1250a6d351 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
> @@ -14,6 +14,7 @@
> **/
>
> #include "BdsPlatform.h"
> +#include <Guid/QemuRamfb.h>
>
> //
> // Debug Agent UART Device Path structure
> @@ -37,6 +38,17 @@ typedef struct {
> } USB_KEYBOARD_DEVICE_PATH;
> #pragma pack ()
>
> +//
> +// QemuRamfb Device Path structure
> +//
> +#pragma pack (1)
> +typedef struct {
> + VENDOR_DEVICE_PATH Vendor;
> + ACPI_ADR_DEVICE_PATH AcpiAdr;
> + EFI_DEVICE_PATH_PROTOCOL End;
> +} VENDOR_RAMFB_DEVICE_PATH;
> +#pragma pack ()
> +
> ACPI_HID_DEVICE_PATH gPnpPs2KeyboardDeviceNode = gPnpPs2Keyboard;
> ACPI_HID_DEVICE_PATH gPnp16550ComPortDeviceNode = gPnp16550ComPort;
> UART_DEVICE_PATH gUartDeviceNode = gUart;
> @@ -100,6 +112,41 @@ STATIC USB_KEYBOARD_DEVICE_PATH gUsbKeyboardDevicePath = {
> gEndEntire
> };
>
> +STATIC VENDOR_RAMFB_DEVICE_PATH gQemuRamfbDevicePath = {
> + {
> + {
> + HARDWARE_DEVICE_PATH,
> + HW_VENDOR_DP,
> + {
> + (UINT8) (sizeof (VENDOR_DEVICE_PATH)),
> + (UINT8) ((sizeof (VENDOR_DEVICE_PATH)) >> 8)
> + }
> + },
> + QEMU_RAMFB_GUID,
> + },
> + {
> + {
> + ACPI_DEVICE_PATH,
> + ACPI_ADR_DP,
> + {
> + (UINT8) (sizeof (ACPI_ADR_DEVICE_PATH)),
> + (UINT8) ((sizeof (ACPI_ADR_DEVICE_PATH)) >> 8)
> + }
> + },
> + ACPI_DISPLAY_ADR (
> + 1, // DeviceIdScheme
> + 0, // HeadId
> + 0, // NonVgaOutput
> + 1, // BiosCanDetect
> + 0, // VendorInfo
> + ACPI_ADR_DISPLAY_TYPE_EXTERNAL_DIGITAL, // Type
> + 0, // Port
> + 0 // Index
> + ),
> + },
> + gEndEntire
> +};
> +
> //
> // Predefined platform default console device path
> //
> @@ -113,6 +160,10 @@ PLATFORM_CONSOLE_CONNECT_ENTRY gPlatformConsole[] = {
> CONSOLE_IN
> },
> {
> + (EFI_DEVICE_PATH_PROTOCOL *)&gQemuRamfbDevicePath,
> + CONSOLE_OUT
> + },
> + {
> NULL,
> 0
> }
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 4/4] ArmVirtPkg: add QemuRamfbDxe
2018-06-12 9:31 ` [PATCH v2 4/4] ArmVirtPkg: add QemuRamfbDxe Gerd Hoffmann
@ 2018-06-12 15:59 ` Laszlo Ersek
0 siblings, 0 replies; 9+ messages in thread
From: Laszlo Ersek @ 2018-06-12 15:59 UTC (permalink / raw)
To: Gerd Hoffmann, edk2-devel
On 06/12/18 11:31, Gerd Hoffmann wrote:
> Add QemuRamfbDxe to dsc and fdf files for ArmVirt package.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> ArmVirtPkg/ArmVirtQemu.dsc | 2 ++
> ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 1 +
> ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 ++
> 3 files changed, 5 insertions(+)
>
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index d74feb709c..744d127a10 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -57,6 +57,7 @@
> BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
> PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
> + FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
> QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
> FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
> PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
> @@ -391,6 +392,7 @@
> #
> # Video support
> #
> + OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
> OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
> OvmfPkg/PlatformDxe/Platform.inf
>
> diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> index 89f95b2d99..63a202c788 100644
> --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> @@ -175,6 +175,7 @@ READ_LOCK_STATUS = TRUE
> #
> # Video support
> #
> + INF OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
> INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
> INF OvmfPkg/PlatformDxe/Platform.inf
>
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> index 1e823aeab7..e59f53b728 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> @@ -57,6 +57,7 @@
> BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
> PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
> + FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
> QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
> FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
> PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
> @@ -380,6 +381,7 @@
> #
> # Video support
> #
> + OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
> OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
> OvmfPkg/PlatformDxe/Platform.inf
>
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/4] OvmfPkg: add QemuRamfbDxe
2018-06-12 9:31 ` [PATCH v2 2/4] OvmfPkg: add QemuRamfbDxe Gerd Hoffmann
@ 2018-06-12 17:50 ` Laszlo Ersek
0 siblings, 0 replies; 9+ messages in thread
From: Laszlo Ersek @ 2018-06-12 17:50 UTC (permalink / raw)
To: Gerd Hoffmann, edk2-devel
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 <kraxel@redhat.com>
> ---
> 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 0000000000..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 <Protocol/GraphicsOutput.h>
> +
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/FrameBufferBltLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/QemuFwCfgLib.h>
> +
> +#include <Guid/QemuRamfb.h>
> +
> +#define RAMFB_FORMAT 0x34325258 /* DRM_FORMAT_XRGB8888 */
> +#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_HANDLE mRamfbHandle;
(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_HANDLE mGopHandle;
> +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 UINT32 ModeNumber,
> + 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 = &mQemuRamfbModeInfo[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 = &mQemuRamfbModeInfo[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,
> + &mQemuRamfbFrameBufferBltConfigureSize
> + );
> +
> + if (Status == RETURN_BUFFER_TOO_SMALL) {
> + if (mQemuRamfbFrameBufferBltConfigure != NULL) {
> + FreePool (mQemuRamfbFrameBufferBltConfigure);
> + }
> + mQemuRamfbFrameBufferBltConfigure =
> + AllocatePool (mQemuRamfbFrameBufferBltConfigureSize);
> + if (mQemuRamfbFrameBufferBltConfigure == NULL) {
> + return EFI_OUT_OF_RESOURCES;
(6) Small logic issue here: at this point
"mQemuRamfbFrameBufferBltConfigure" is NULL (which is good), but
"mQemuRamfbFrameBufferBltConfigureSize" is nonzero (in fact it is "large
enough"; it's just been returned to us with the correct size).
As a result, at the next attempt to set the same mode,
FrameBufferBltConfigure() will dereference NULL.
So please, before returning, assign
"mQemuRamfbFrameBufferBltConfigureSize" zero.
> + }
> +
> + Status = FrameBufferBltConfigure (
> + (VOID*)(UINTN)mQemuRamfbMode.FrameBufferBase,
> + ModeInfo,
> + mQemuRamfbFrameBufferBltConfigure,
> + &mQemuRamfbFrameBufferBltConfigureSize);
(7) Please break off the ");" to a separate line, as discussed before.
> + if (RETURN_ERROR (Status)) {
> + ASSERT (Status == RETURN_UNSUPPORTED);
> + return Status;
> + }
(8) I think I was unclear about this in my last email, sorry about that
again. I meant for this RETURN_ERROR() check to be placed *after* (not
inside) the RETURN_BUFFER_TOO_SMALL branch.
With the idea being, after the end of the "too small" branch, regardless
of us actually having taken the "too small" branch, Status must either
be "success", or "unsupported".
Currently the code, as-is, will not catch the first
FrameBufferBltConfigure() call returning "unsupported".
> + }
> +
> + mQemuRamfbMode.Mode = ModeNumber;
> + mQemuRamfbMode.Info = ModeInfo;
> +
> + QemuFwCfgSelectItem (mRamFbFwCfgItem);
> + QemuFwCfgWriteBytes (sizeof (Config), &Config);
> +
> + //
> + // clear screen
> + //
> + ZeroMem (&Black, sizeof (Black));
> + Status = FrameBufferBlt (
> + mQemuRamfbFrameBufferBltConfigure,
> + &Black,
> + EfiBltVideoFill,
> + 0, // SourceX -- ignored
> + 0, // SourceY -- ignored
> + 0, // DestinationX
> + 0, // DestinationY
> + ModeInfo->HorizontalResolution, // Width
> + ModeInfo->VerticalResolution, // Height
> + 0 // Delta -- ignored
> + );
> + if (EFI_ERROR (Status)) {
(9) For stylistic reasons, please use "RETURN_ERROR (Status)" here. (See
the previous discussion on RETURN_STATUS vs. EFI_STATUS.)
> + DEBUG ((DEBUG_WARN, "%a: clearing the screen failed: %r\n",
> + __FUNCTION__, Status));
(10) Please fix the indentation.
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> +static
> +EFI_STATUS
> +EFIAPI
> +QemuRamfbGraphicsOutputBlt (
> + IN EFI_GRAPHICS_OUTPUT_PROTOCOL *This,
> + IN EFI_GRAPHICS_OUTPUT_BLT_PIXEL *BltBuffer, OPTIONAL
> + IN EFI_GRAPHICS_OUTPUT_BLT_OPERATION BltOperation,
> + IN UINTN SourceX,
> + IN UINTN SourceY,
> + IN UINTN DestinationX,
> + IN UINTN DestinationY,
> + IN UINTN Width,
> + IN UINTN Height,
> + IN UINTN Delta
> + )
> +{
> + return FrameBufferBlt (
> + mQemuRamfbFrameBufferBltConfigure,
> + BltBuffer,
> + BltOperation,
> + SourceX,
> + SourceY,
> + DestinationX,
> + DestinationY,
> + Width,
> + Height,
> + Delta);
(11) Please break off the ");" to a separate line.
> +}
> +
> +EFI_GRAPHICS_OUTPUT_PROTOCOL QemuRamfbGraphicsOutput = {
(12) Please make this STATIC.
(13) Please rename it to "mQemuRamfbGraphicsOutput".
> + QemuRamfbGraphicsOutputQueryMode,
> + QemuRamfbGraphicsOutputSetMode,
> + QemuRamfbGraphicsOutputBlt,
> + &mQemuRamfbMode,
> +};
> +
> +EFI_STATUS
> +EFIAPI
> +InitializeQemuRamfb (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + EFI_DEVICE_PATH_PROTOCOL *RamfbDevicePath;
> + EFI_DEVICE_PATH_PROTOCOL *GopDevicePath;
> + VOID *DevicePath;
> + VENDOR_DEVICE_PATH VendorDeviceNode;
> + ACPI_ADR_DEVICE_PATH AcpiDeviceNode;
> + EFI_STATUS Status;
> + EFI_PHYSICAL_ADDRESS FbBase;
> + UINTN FbSize, MaxFbSize;
> + UINTN Size, Pages, Index;
(14) Can you rename "Size" to "FwCfgSize" as I requested earlier please?
(15) I see that you are reluctant accepting that edk2 really prefers to
break almost all variable definitions to separate lines :) I suggest the
following compromise:
UINTN FbSize, MaxFbSize, Pages;
UINTN FwCfgSize;
UINTN Index;
Because, an argument can be made that FbSize, MaxFbSize, and Pages
belong together. However, FwCfgSize and Index should each stand alone.
> +
> + if (!QemuFwCfgIsAvailable()) {
(16) Oops, I missed this last time; please add a space between function
identifier and ().
> + DEBUG ((DEBUG_INFO, "Ramfb: no FwCfg\n"));
> + return EFI_NOT_FOUND;
> + }
> +
> + Status = QemuFwCfgFindFile("etc/ramfb", &mRamFbFwCfgItem, &Size);
(17) Ditto.
> + if (EFI_ERROR (Status)) {
> + return EFI_NOT_FOUND;
> + }
> + if (Size != sizeof (RAMFB_CONFIG)) {
> + DEBUG ((DEBUG_ERROR, "Ramfb: FwCfg size mismatch (expected %d, got %d)\n",
> + sizeof (RAMFB_CONFIG), Size));
(18) Indentation.
(19) Additionally, "sizeof" has type UINTN, and "Size" (FwCfgSize) has
type UINTN too. Please cast them both to UINT64 and print them with %lu.
> + return EFI_NOT_FOUND;
(20) I generally return EFI_PROTOCOL_ERROR whenever fw_cfg produces
garbage, but EFI_NOT_FOUND is perfectly acceptable as well. Up to you.
> + }
> +
> + MaxFbSize = 0;
> + for (Index = 0; Index < mQemuRamfbModeCount; Index++) {
> + mQemuRamfbModeInfo[Index].PixelsPerScanLine =
> + mQemuRamfbModeInfo[Index].HorizontalResolution;
> + mQemuRamfbModeInfo[Index].PixelFormat =
> + PixelBlueGreenRedReserved8BitPerColor;
> + FbSize = RAMFB_BPP *
> + mQemuRamfbModeInfo[Index].HorizontalResolution *
> + mQemuRamfbModeInfo[Index].VerticalResolution;
> + if (MaxFbSize < FbSize) {
> + MaxFbSize = FbSize;
> + }
> + DEBUG ((DEBUG_INFO, "Ramfb: Mode %u: %ux%u, %lu kB\n", (UINT64)Index,
(21) The cast is OK, but the conversion specifier should be %lu.
> + mQemuRamfbModeInfo[Index].HorizontalResolution,
> + mQemuRamfbModeInfo[Index].VerticalResolution,
> + (UINT64)FbSize / 1024));
(22) Sorry, this division is no longer portable (between 64-bit and
32-bit edk2 builds) in this form. The reason is that UINT64 values can't
*generally* be divided in 32-bit builds without compiler intrinsics; in
order to avoid those, we use named helper functions for such divisions.
However, using those helpers wasn't what I meant; I meant
(UINT64)(FbSize / 1024)
Because, FbSize is UINTN (i.e. UINT32 in 32-bit builds, UINT64 in 64-bit
builds), and so it can be divided by 1024 (of type INT32) without
helpers. It's the UINTN *quotient* that we should cast, for portable
printing.
I apologize if my previous comment on this was misleading.
> + }
> +
> + Pages = EFI_SIZE_TO_PAGES (MaxFbSize);
> + MaxFbSize = EFI_PAGES_TO_SIZE (Pages);
> + FbBase = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocateReservedPages (Pages);
> + if (FbBase == 0) {
> + DEBUG ((DEBUG_INFO, "Ramfb: memory allocation failed\n"));
(23) Sorry, missed this last time, should be DEBUG_ERROR.
> + return EFI_OUT_OF_RESOURCES;
> + }
> + DEBUG ((DEBUG_INFO, "Ramfb: Framebuffer at 0x%lx, %ld kB, %ld pages\n",
> + (UINT64)FbBase, (UINT64)MaxFbSize / 1024, (UINT64)Pages));
(24) Indentation.
(25) Regarding the MaxFbSize division, please see (22).
(26) Please use %lu for printing both the MaxFbSize quotient (see above)
and Pages. %ld is for INT64.
> + mQemuRamfbMode.FrameBufferSize = MaxFbSize;
> + mQemuRamfbMode.FrameBufferBase = FbBase;
> +
> + //
> + // 800 x 600
> + //
> + QemuRamfbGraphicsOutputSetMode (&QemuRamfbGraphicsOutput, 1);
> +
> + //
> + // ramfb vendor devpath
> + //
> + VendorDeviceNode.Header.Type = HARDWARE_DEVICE_PATH;
> + VendorDeviceNode.Header.SubType = HW_VENDOR_DP;
> + CopyGuid(&VendorDeviceNode.Guid, &gQemuRamfbGuid);
(27) Please add a space after "CopyGuid".
> + SetDevicePathNodeLength (&VendorDeviceNode.Header,
> + sizeof (VENDOR_DEVICE_PATH));
> +
> + RamfbDevicePath = AppendDevicePathNode (NULL,
> + (EFI_DEVICE_PATH_PROTOCOL *) &VendorDeviceNode);
> + if (RamfbDevicePath == NULL) {
> + Status = EFI_OUT_OF_RESOURCES;
> + goto FreeFramebuffer;
> + }
> +
> + Status = gBS->InstallMultipleProtocolInterfaces (
> + &mRamfbHandle,
> + &gEfiDevicePathProtocolGuid,
> + RamfbDevicePath,
> + NULL
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "Ramfb: install Ramfb Vendor DevicePath failed: %r\n",
> + Status));
> + goto FreeRamfbDevicePath;
> + }
> +
> + //
> + // gop devpath + protocol
> + //
> + ZeroMem (&AcpiDeviceNode, sizeof (ACPI_ADR_DEVICE_PATH));
(28) I think you can drop the ZeroMem(); all fields are set explicitly.
> + AcpiDeviceNode.Header.Type = ACPI_DEVICE_PATH;
> + AcpiDeviceNode.Header.SubType = ACPI_ADR_DP;
> + AcpiDeviceNode.ADR = ACPI_DISPLAY_ADR (
> + 1, // DeviceIdScheme
> + 0, // HeadId
> + 0, // NonVgaOutput
> + 1, // BiosCanDetect
> + 0, // VendorInfo
> + ACPI_ADR_DISPLAY_TYPE_EXTERNAL_DIGITAL, // Type
> + 0, // Port
> + 0 // Index
> + );
Sigh, this is one of those situations when different coding style
guidelines conflict. One style requirement would be to indent the
arguments two columns to the right of the beginning of
"ACPI_DISPLAY_ADR". However, in that case, we wouldn't fit in 80
characters; and in my book, that's a lot worse. So, just leave this as
it is. :/
> + SetDevicePathNodeLength (&AcpiDeviceNode.Header,
> + sizeof (ACPI_ADR_DEVICE_PATH));
> +
> + GopDevicePath = AppendDevicePathNode (RamfbDevicePath,
> + (EFI_DEVICE_PATH_PROTOCOL *) &AcpiDeviceNode);
> + if (GopDevicePath == NULL) {
> + Status = EFI_OUT_OF_RESOURCES;
> + goto FreeRamfbHandle;
> + }
> +
> + Status = gBS->InstallMultipleProtocolInterfaces (
> + &mGopHandle,
> + &gEfiDevicePathProtocolGuid,
> + GopDevicePath,
> + &gEfiGraphicsOutputProtocolGuid,
> + &QemuRamfbGraphicsOutput,
> + NULL
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "Ramfb: install GOP DevicePath failed: %r\n",
> + Status));
> + goto FreeGopDevicePath;
> + }
> +
> + Status = gBS->OpenProtocol (
> + mRamfbHandle,
> + &gEfiDevicePathProtocolGuid,
> + &DevicePath,
> + gImageHandle,
> + mGopHandle,
> + EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "Ramfb: OpenProtocol failed: %r\n", Status));
> + goto FreeGopHandle;
> + }
> +
> + return EFI_SUCCESS;
> +
> +FreeGopHandle:
> + gBS->UninstallMultipleProtocolInterfaces (
> + mGopHandle,
> + &gEfiDevicePathProtocolGuid,
> + GopDevicePath,
> + &gEfiGraphicsOutputProtocolGuid,
> + &QemuRamfbGraphicsOutput,
> + NULL
> + );
> +FreeGopDevicePath:
> + FreePool (GopDevicePath);
> +FreeRamfbHandle:
> + gBS->UninstallMultipleProtocolInterfaces (
> + mRamfbHandle,
> + &gEfiDevicePathProtocolGuid,
> + RamfbDevicePath,
> + NULL
> + );
> +FreeRamfbDevicePath:
> + FreePool (RamfbDevicePath);
> +FreeFramebuffer:
> + FreePool ((VOID*)(UINTN)mQemuRamfbMode.FrameBufferBase);
(29) Oops, completely missed this in the previous review, multiple times
-- please use FreePages() for releasing FrameBufferBase, not FreePool()!
> + return Status;
> +}
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index a2c995b910..7ddda89999 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -745,6 +745,7 @@
> MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
>
> OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
> + OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
> OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
>
> #
> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
> index b199713925..52b8b1fea1 100644
> --- a/OvmfPkg/OvmfPkgIa32.fdf
> +++ b/OvmfPkg/OvmfPkgIa32.fdf
> @@ -351,6 +351,7 @@ INF RuleOverride=CSM OvmfPkg/Csm/Csm16/Csm16.inf
> !endif
>
> INF OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
> +INF OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
> INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
> INF OvmfPkg/PlatformDxe/Platform.inf
> INF OvmfPkg/IoMmuDxe/IoMmuDxe.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index bc7db229d2..3481cdc36b 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -754,6 +754,7 @@
> MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
>
> OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
> + OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
> OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
>
> #
> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
> index 4ebf64b2b9..70845d6972 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.fdf
> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf
> @@ -357,6 +357,7 @@ INF RuleOverride=CSM OvmfPkg/Csm/Csm16/Csm16.inf
> !endif
>
> INF OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
> +INF OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
> INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
> INF OvmfPkg/PlatformDxe/Platform.inf
> INF OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 0767b34d18..8b0895b0ff 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -752,6 +752,7 @@
> MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
>
> OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
> + OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
> OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
>
> #
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index 9ca96f9282..1eb46ac9a2 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -357,6 +357,7 @@ INF RuleOverride=CSM OvmfPkg/Csm/Csm16/Csm16.inf
> !endif
>
> INF OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
> +INF OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
> INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
> INF OvmfPkg/PlatformDxe/Platform.inf
> INF OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> diff --git a/OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf b/OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
> new file mode 100644
> index 0000000000..5a9fd42c32
> --- /dev/null
> +++ b/OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
> @@ -0,0 +1,52 @@
> +## @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
(30) These two lines are too long, can you please rewrap 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.
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 0x00010005
> + BASE_NAME = QemuRamfbDxe
> + FILE_GUID = dce1b094-7dc6-45d0-9fdd-d7fc3cc3e4ef
> + MODULE_TYPE = DXE_DRIVER
> + VERSION_STRING = 1.0
> +
> + ENTRY_POINT = InitializeQemuRamfb
> +
> +[Sources]
> + QemuRamfb.c
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> + MdeModulePkg/MdeModulePkg.dec
> + OvmfPkg/OvmfPkg.dec
> +
> +[LibraryClasses]
> + BaseLib
> + BaseMemoryLib
> + DebugLib
> + DevicePathLib
> + FrameBufferBltLib
> + MemoryAllocationLib
> + UefiBootServicesTableLib
> + UefiDriverEntryPoint
> + QemuFwCfgLib
> +
> +[Protocols]
> + gEfiGraphicsOutputProtocolGuid # BY_START
(31) As requested previously, drop "# BY_START" and say ""## PRODUCES"
please.
> +
> +[Guids]
> + gQemuRamfbGuid
> +
> +[Depex]
> + TRUE
>
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-06-12 17:50 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-12 9:31 [PATCH v2 0/4] Add QemuRamfbDxe driver Gerd Hoffmann
2018-06-12 9:31 ` [PATCH v2 1/4] OvmfPkg: add QEMU_RAMFB_GUID Gerd Hoffmann
2018-06-12 15:46 ` Laszlo Ersek
2018-06-12 9:31 ` [PATCH v2 2/4] OvmfPkg: add QemuRamfbDxe Gerd Hoffmann
2018-06-12 17:50 ` Laszlo Ersek
2018-06-12 9:31 ` [PATCH v2 3/4] OvmfPkg: add QemuRamfb to platform console Gerd Hoffmann
2018-06-12 15:57 ` Laszlo Ersek
2018-06-12 9:31 ` [PATCH v2 4/4] ArmVirtPkg: add QemuRamfbDxe Gerd Hoffmann
2018-06-12 15:59 ` Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox