[PATCH 0/5] OvmfPkg/QemuVideoDxe: pick up display resolution settings from the host

[PATCH 0/5] OvmfPkg/QemuVideoDxe: pick up display resolution settings from the host

[Gerd Hoffmann]

See last patch in the series for details.

Gerd Hoffmann (5):
  OvmfPkg: add PcdVideoResolutionSource
  OvmfPkg/QemuVideoDxe: simplify InitializeBochsGraphicsMode
  OvmfPkg/QemuVideoDxe: drop QEMU_VIDEO_BOCHS_MODES->ColorDepth
  OvmfPkg/QemuVideoDxe: factor out QemuVideoBochsAddMode
  OvmfPkg/QemuVideoDxe: parse edid blob, detect display resolution

 OvmfPkg/OvmfPkg.dec                   |   7 +
 OvmfPkg/AmdSev/AmdSevX64.dsc          |   1 +
 OvmfPkg/Microvm/MicrovmX64.dsc        |   1 +
 OvmfPkg/OvmfPkgIa32.dsc               |   1 +
 OvmfPkg/OvmfPkgIa32X64.dsc            |   1 +
 OvmfPkg/OvmfPkgX64.dsc                |   1 +
 OvmfPkg/OvmfXen.dsc                   |   1 +
 OvmfPkg/PlatformDxe/Platform.inf      |   1 +
 OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf |   3 +
 OvmfPkg/QemuVideoDxe/Qemu.h           |   6 +-
 OvmfPkg/PlatformDxe/Platform.c        |   3 +
 OvmfPkg/QemuVideoDxe/Driver.c         |  14 +-
 OvmfPkg/QemuVideoDxe/Gop.c            |   2 +-
 OvmfPkg/QemuVideoDxe/Initialize.c     | 251 +++++++++++++++++++-------
 14 files changed, 213 insertions(+), 80 deletions(-)

-- 
2.33.1

[PATCH 1/5] OvmfPkg: add PcdVideoResolutionSource

[Gerd Hoffmann]

It's a UINT8 (enum) PCD telling where the PcdVideoHorizontalResolution
and PcdVideoVerticalResolution values are coming from.  It can be:

 0 (unset aka default from dsc file), or
 1 (from PlatformConfig), or
 2 (set by Video Driver).

It will be used by video drivers to avoid overriding PlatformConfig
values, or override each others values in case multiple display devices
are present.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/OvmfPkg.dec              | 7 +++++++
 OvmfPkg/AmdSev/AmdSevX64.dsc     | 1 +
 OvmfPkg/Microvm/MicrovmX64.dsc   | 1 +
 OvmfPkg/OvmfPkgIa32.dsc          | 1 +
 OvmfPkg/OvmfPkgIa32X64.dsc       | 1 +
 OvmfPkg/OvmfPkgX64.dsc           | 1 +
 OvmfPkg/OvmfXen.dsc              | 1 +
 OvmfPkg/PlatformDxe/Platform.inf | 1 +
 OvmfPkg/PlatformDxe/Platform.c   | 3 +++
 9 files changed, 17 insertions(+)

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 769bef0ffa12..7aa94ca02863 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -408,6 +408,13 @@ [PcdsDynamic, PcdsDynamicEx]
   #  instance in PiSmmCpuDxeSmm, and CpuHotplugSmm.
   gUefiOvmfPkgTokenSpaceGuid.PcdCpuHotEjectDataAddress|0|UINT64|0x46
 
+  ## This PCD tracks where PcdVideo{Horizontal,Vertical}Resolution
+  #  values are coming from.
+  #    0 - unset (defaults from platform dsc)
+  #    1 - set from PlatformConfig
+  #    2 - set by GOP Driver.
+  gUefiOvmfPkgTokenSpaceGuid.PcdVideoResolutionSource|0|UINT8|0x64
+
 [PcdsFeatureFlag]
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuBootOrderPciTranslation|TRUE|BOOLEAN|0x1c
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuBootOrderMmioTranslation|FALSE|BOOLEAN|0x1d
diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index 04ae61cf69d8..343ac643f021 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -527,6 +527,7 @@ [PcdsDynamicDefault]
   gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|800
   gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|600
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable|FALSE
+  gUefiOvmfPkgTokenSpaceGuid.PcdVideoResolutionSource|0
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId|0
   gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase|0x0
   gUefiOvmfPkgTokenSpaceGuid.PcdPciIoSize|0x0
diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
index 1c2e600febee..cb28bd8bba2c 100644
--- a/OvmfPkg/Microvm/MicrovmX64.dsc
+++ b/OvmfPkg/Microvm/MicrovmX64.dsc
@@ -560,6 +560,7 @@ [PcdsDynamicDefault]
   gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|800
   gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|600
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable|FALSE
+  gUefiOvmfPkgTokenSpaceGuid.PcdVideoResolutionSource|0
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId|0
   gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase|0x0
   gUefiOvmfPkgTokenSpaceGuid.PcdPciIoSize|0x0
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 085cc7ece15d..2f05d93ed4af 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -590,6 +590,7 @@ [PcdsDynamicDefault]
   gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|800
   gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|600
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable|FALSE
+  gUefiOvmfPkgTokenSpaceGuid.PcdVideoResolutionSource|0
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId|0
   gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase|0x0
   gUefiOvmfPkgTokenSpaceGuid.PcdPciIoSize|0x0
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 0ce122ddb50c..bcab54aaa02a 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -596,6 +596,7 @@ [PcdsDynamicDefault]
   gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|800
   gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|600
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable|FALSE
+  gUefiOvmfPkgTokenSpaceGuid.PcdVideoResolutionSource|0
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId|0
   gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase|0x0
   gUefiOvmfPkgTokenSpaceGuid.PcdPciIoSize|0x0
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 4589adff388d..ee0fdf492fd4 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -595,6 +595,7 @@ [PcdsDynamicDefault]
   gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|800
   gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|600
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable|FALSE
+  gUefiOvmfPkgTokenSpaceGuid.PcdVideoResolutionSource|0
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId|0
   gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase|0x0
   gUefiOvmfPkgTokenSpaceGuid.PcdPciIoSize|0x0
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index de0d30bfb807..1f6352195b75 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -468,6 +468,7 @@ [PcdsDynamicDefault]
   gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|800
   gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|600
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable|FALSE
+  gUefiOvmfPkgTokenSpaceGuid.PcdVideoResolutionSource|0
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId|0
   gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase|0x0
   gUefiOvmfPkgTokenSpaceGuid.PcdPciIoSize|0x0
diff --git a/OvmfPkg/PlatformDxe/Platform.inf b/OvmfPkg/PlatformDxe/Platform.inf
index 14727c1220e8..d2f75d78b601 100644
--- a/OvmfPkg/PlatformDxe/Platform.inf
+++ b/OvmfPkg/PlatformDxe/Platform.inf
@@ -47,6 +47,7 @@ [LibraryClasses]
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution
   gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution
+  gUefiOvmfPkgTokenSpaceGuid.PcdVideoResolutionSource
 
 [Protocols]
   gEfiDevicePathProtocolGuid      ## PRODUCES
diff --git a/OvmfPkg/PlatformDxe/Platform.c b/OvmfPkg/PlatformDxe/Platform.c
index 69a7ecb83d03..4bf22712c78f 100644
--- a/OvmfPkg/PlatformDxe/Platform.c
+++ b/OvmfPkg/PlatformDxe/Platform.c
@@ -742,6 +742,9 @@ ExecutePlatformConfig (
                   PlatformConfig.VerticalResolution
                   );
     ASSERT_RETURN_ERROR (PcdStatus);
+
+    PcdStatus = PcdSet8S (PcdVideoResolutionSource, 1);
+    ASSERT_RETURN_ERROR (PcdStatus);
   }
 
   return EFI_SUCCESS;
-- 
2.33.1

[PATCH 2/5] OvmfPkg/QemuVideoDxe: simplify InitializeBochsGraphicsMode

[Gerd Hoffmann]

struct QEMU_VIDEO_MODE_DATA has all the data needed to set the video
mode, there is no need to take the extra indirection and use
struct QEMU_VIDEO_BOCHS_MODES.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/QemuVideoDxe/Qemu.h       |  3 +--
 OvmfPkg/QemuVideoDxe/Driver.c     | 14 +++++++-------
 OvmfPkg/QemuVideoDxe/Gop.c        |  2 +-
 OvmfPkg/QemuVideoDxe/Initialize.c |  2 +-
 4 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/OvmfPkg/QemuVideoDxe/Qemu.h b/OvmfPkg/QemuVideoDxe/Qemu.h
index 8f05898f862c..fef648c967b2 100644
--- a/OvmfPkg/QemuVideoDxe/Qemu.h
+++ b/OvmfPkg/QemuVideoDxe/Qemu.h
@@ -150,7 +150,6 @@ extern UINT16                        Seq_800_600_256_60[];
 extern UINT8                         Crtc_1024_768_256_60[];
 extern UINT16                        Seq_1024_768_256_60[];
 extern QEMU_VIDEO_CIRRUS_MODES       QemuVideoCirrusModes[];
-extern QEMU_VIDEO_BOCHS_MODES        QemuVideoBochsModes[];
 extern EFI_DRIVER_BINDING_PROTOCOL   gQemuVideoDriverBinding;
 extern EFI_COMPONENT_NAME_PROTOCOL   gQemuVideoComponentName;
 extern EFI_COMPONENT_NAME2_PROTOCOL  gQemuVideoComponentName2;
@@ -414,7 +413,7 @@ InitializeCirrusGraphicsMode (
 VOID
 InitializeBochsGraphicsMode (
   QEMU_VIDEO_PRIVATE_DATA  *Private,
-  QEMU_VIDEO_BOCHS_MODES   *ModeData
+  QEMU_VIDEO_MODE_DATA     *ModeData
   );
 
 VOID
diff --git a/OvmfPkg/QemuVideoDxe/Driver.c b/OvmfPkg/QemuVideoDxe/Driver.c
index d9f0a2464aa3..b91909a14e59 100644
--- a/OvmfPkg/QemuVideoDxe/Driver.c
+++ b/OvmfPkg/QemuVideoDxe/Driver.c
@@ -987,14 +987,14 @@ VgaOutb (
 VOID
 InitializeBochsGraphicsMode (
   QEMU_VIDEO_PRIVATE_DATA  *Private,
-  QEMU_VIDEO_BOCHS_MODES   *ModeData
+  QEMU_VIDEO_MODE_DATA     *ModeData
   )
 {
   DEBUG ((
     DEBUG_INFO,
     "InitializeBochsGraphicsMode: %dx%d @ %d\n",
-    ModeData->Width,
-    ModeData->Height,
+    ModeData->HorizontalResolution,
+    ModeData->VerticalResolution,
     ModeData->ColorDepth
     ));
 
@@ -1007,10 +1007,10 @@ InitializeBochsGraphicsMode (
   BochsWrite (Private, VBE_DISPI_INDEX_Y_OFFSET, 0);
 
   BochsWrite (Private, VBE_DISPI_INDEX_BPP, (UINT16)ModeData->ColorDepth);
-  BochsWrite (Private, VBE_DISPI_INDEX_XRES, (UINT16)ModeData->Width);
-  BochsWrite (Private, VBE_DISPI_INDEX_VIRT_WIDTH, (UINT16)ModeData->Width);
-  BochsWrite (Private, VBE_DISPI_INDEX_YRES, (UINT16)ModeData->Height);
-  BochsWrite (Private, VBE_DISPI_INDEX_VIRT_HEIGHT, (UINT16)ModeData->Height);
+  BochsWrite (Private, VBE_DISPI_INDEX_XRES, (UINT16)ModeData->HorizontalResolution);
+  BochsWrite (Private, VBE_DISPI_INDEX_VIRT_WIDTH, (UINT16)ModeData->HorizontalResolution);
+  BochsWrite (Private, VBE_DISPI_INDEX_YRES, (UINT16)ModeData->VerticalResolution);
+  BochsWrite (Private, VBE_DISPI_INDEX_VIRT_HEIGHT, (UINT16)ModeData->VerticalResolution);
 
   BochsWrite (
     Private,
diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c
index 5ad0afe88378..0c4dea7fb6f2 100644
--- a/OvmfPkg/QemuVideoDxe/Gop.c
+++ b/OvmfPkg/QemuVideoDxe/Gop.c
@@ -177,7 +177,7 @@ Routine Description:
       break;
     case QEMU_VIDEO_BOCHS_MMIO:
     case QEMU_VIDEO_BOCHS:
-      InitializeBochsGraphicsMode (Private, &QemuVideoBochsModes[ModeData->InternalModeIndex]);
+      InitializeBochsGraphicsMode (Private, ModeData);
       break;
     default:
       ASSERT (FALSE);
diff --git a/OvmfPkg/QemuVideoDxe/Initialize.c b/OvmfPkg/QemuVideoDxe/Initialize.c
index 533ec661d64f..8a70cf848483 100644
--- a/OvmfPkg/QemuVideoDxe/Initialize.c
+++ b/OvmfPkg/QemuVideoDxe/Initialize.c
@@ -202,7 +202,7 @@ QemuVideoCirrusModeSetup (
 ///
 /// Table of supported video modes
 ///
-QEMU_VIDEO_BOCHS_MODES  QemuVideoBochsModes[] = {
+STATIC QEMU_VIDEO_BOCHS_MODES  QemuVideoBochsModes[] = {
   { 640,  480,  32 },
   { 800,  480,  32 },
   { 800,  600,  32 },
-- 
2.33.1

[PATCH 3/5] OvmfPkg/QemuVideoDxe: drop QEMU_VIDEO_BOCHS_MODES->ColorDepth

[Gerd Hoffmann]

All video modes in the list are 32-bit,
so drop the useless ColorDepth field.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/QemuVideoDxe/Qemu.h       |  1 -
 OvmfPkg/QemuVideoDxe/Initialize.c | 80 +++++++++++++++----------------
 2 files changed, 39 insertions(+), 42 deletions(-)

diff --git a/OvmfPkg/QemuVideoDxe/Qemu.h b/OvmfPkg/QemuVideoDxe/Qemu.h
index fef648c967b2..1e6507f44caa 100644
--- a/OvmfPkg/QemuVideoDxe/Qemu.h
+++ b/OvmfPkg/QemuVideoDxe/Qemu.h
@@ -132,7 +132,6 @@ typedef struct {
 typedef struct {
   UINT32    Width;
   UINT32    Height;
-  UINT32    ColorDepth;
 } QEMU_VIDEO_BOCHS_MODES;
 
 #define QEMU_VIDEO_PRIVATE_DATA_FROM_GRAPHICS_OUTPUT_THIS(a) \
diff --git a/OvmfPkg/QemuVideoDxe/Initialize.c b/OvmfPkg/QemuVideoDxe/Initialize.c
index 8a70cf848483..2b174d13faf2 100644
--- a/OvmfPkg/QemuVideoDxe/Initialize.c
+++ b/OvmfPkg/QemuVideoDxe/Initialize.c
@@ -203,43 +203,43 @@ QemuVideoCirrusModeSetup (
 /// Table of supported video modes
 ///
 STATIC QEMU_VIDEO_BOCHS_MODES  QemuVideoBochsModes[] = {
-  { 640,  480,  32 },
-  { 800,  480,  32 },
-  { 800,  600,  32 },
-  { 832,  624,  32 },
-  { 960,  640,  32 },
-  { 1024, 600,  32 },
-  { 1024, 768,  32 },
-  { 1152, 864,  32 },
-  { 1152, 870,  32 },
-  { 1280, 720,  32 },
-  { 1280, 760,  32 },
-  { 1280, 768,  32 },
-  { 1280, 800,  32 },
-  { 1280, 960,  32 },
-  { 1280, 1024, 32 },
-  { 1360, 768,  32 },
-  { 1366, 768,  32 },
-  { 1400, 1050, 32 },
-  { 1440, 900,  32 },
-  { 1600, 900,  32 },
-  { 1600, 1200, 32 },
-  { 1680, 1050, 32 },
-  { 1920, 1080, 32 },
-  { 1920, 1200, 32 },
-  { 1920, 1440, 32 },
-  { 2000, 2000, 32 },
-  { 2048, 1536, 32 },
-  { 2048, 2048, 32 },
-  { 2560, 1440, 32 },
-  { 2560, 1600, 32 },
-  { 2560, 2048, 32 },
-  { 2800, 2100, 32 },
-  { 3200, 2400, 32 },
-  { 3840, 2160, 32 },
-  { 4096, 2160, 32 },
-  { 7680, 4320, 32 },
-  { 8192, 4320, 32 }
+  { 640,  480  },
+  { 800,  480  },
+  { 800,  600  },
+  { 832,  624  },
+  { 960,  640  },
+  { 1024, 600  },
+  { 1024, 768  },
+  { 1152, 864  },
+  { 1152, 870  },
+  { 1280, 720  },
+  { 1280, 760  },
+  { 1280, 768  },
+  { 1280, 800  },
+  { 1280, 960  },
+  { 1280, 1024 },
+  { 1360, 768  },
+  { 1366, 768  },
+  { 1400, 1050 },
+  { 1440, 900  },
+  { 1600, 900  },
+  { 1600, 1200 },
+  { 1680, 1050 },
+  { 1920, 1080 },
+  { 1920, 1200 },
+  { 1920, 1440 },
+  { 2000, 2000 },
+  { 2048, 1536 },
+  { 2048, 2048 },
+  { 2560, 1440 },
+  { 2560, 1600 },
+  { 2560, 2048 },
+  { 2800, 2100 },
+  { 3200, 2400 },
+  { 3840, 2160 },
+  { 4096, 2160 },
+  { 7680, 4320 },
+  { 8192, 4320 }
 };
 
 #define QEMU_VIDEO_BOCHS_MODE_COUNT \
@@ -348,14 +348,12 @@ QemuVideoBochsModeSetup (
   for (Index = 0; Index < QEMU_VIDEO_BOCHS_MODE_COUNT; Index++) {
     UINTN  RequiredFbSize;
 
-    ASSERT (VideoMode->ColorDepth % 8 == 0);
-    RequiredFbSize = (UINTN)VideoMode->Width * VideoMode->Height *
-                     (VideoMode->ColorDepth / 8);
+    RequiredFbSize = (UINTN)VideoMode->Width * VideoMode->Height * 4;
     if (RequiredFbSize <= AvailableFbSize) {
       ModeData->InternalModeIndex    = Index;
       ModeData->HorizontalResolution = VideoMode->Width;
       ModeData->VerticalResolution   = VideoMode->Height;
-      ModeData->ColorDepth           = VideoMode->ColorDepth;
+      ModeData->ColorDepth           = 32;
       DEBUG ((
         DEBUG_INFO,
         "Adding Mode %d as Bochs Internal Mode %d: %dx%d, %d-bit\n",
-- 
2.33.1

[PATCH 4/5] OvmfPkg/QemuVideoDxe: factor out QemuVideoBochsAddMode

[Gerd Hoffmann]

Add helper function to add a video mode to the list of modes.
Move code.  Minor debug logging tweaks, no other functional
change.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/QemuVideoDxe/Initialize.c | 77 +++++++++++++++++++------------
 1 file changed, 47 insertions(+), 30 deletions(-)

diff --git a/OvmfPkg/QemuVideoDxe/Initialize.c b/OvmfPkg/QemuVideoDxe/Initialize.c
index 2b174d13faf2..8c5c9176ad21 100644
--- a/OvmfPkg/QemuVideoDxe/Initialize.c
+++ b/OvmfPkg/QemuVideoDxe/Initialize.c
@@ -245,16 +245,53 @@ STATIC QEMU_VIDEO_BOCHS_MODES  QemuVideoBochsModes[] = {
 #define QEMU_VIDEO_BOCHS_MODE_COUNT \
   (ARRAY_SIZE (QemuVideoBochsModes))
 
+STATIC
+VOID
+QemuVideoBochsAddMode (
+  QEMU_VIDEO_PRIVATE_DATA  *Private,
+  UINT32                   AvailableFbSize,
+  UINT32                   Width,
+  UINT32                   Height
+  )
+{
+  QEMU_VIDEO_MODE_DATA  *ModeData = Private->ModeData + Private->MaxMode;
+  UINTN                 RequiredFbSize;
+
+  RequiredFbSize = (UINTN)Width * Height * 4;
+  if (RequiredFbSize > AvailableFbSize) {
+    DEBUG ((
+      DEBUG_INFO,
+      "Skipping Bochs Mode %dx%d, 32-bit (not enough vram)\n",
+      Width,
+      Height
+      ));
+    return;
+  }
+
+  ModeData->InternalModeIndex    = (UINT32)Private->MaxMode;
+  ModeData->HorizontalResolution = Width;
+  ModeData->VerticalResolution   = Height;
+  ModeData->ColorDepth           = 32;
+  DEBUG ((
+    DEBUG_INFO,
+    "Adding Bochs Internal Mode %d: %dx%d, %d-bit\n",
+    ModeData->InternalModeIndex,
+    ModeData->HorizontalResolution,
+    ModeData->VerticalResolution,
+    ModeData->ColorDepth
+    ));
+
+  Private->MaxMode++;
+}
+
 EFI_STATUS
 QemuVideoBochsModeSetup (
   QEMU_VIDEO_PRIVATE_DATA  *Private,
   BOOLEAN                  IsQxl
   )
 {
-  UINT32                  AvailableFbSize;
-  UINT32                  Index;
-  QEMU_VIDEO_MODE_DATA    *ModeData;
-  QEMU_VIDEO_BOCHS_MODES  *VideoMode;
+  UINT32  AvailableFbSize;
+  UINT32  Index;
 
   //
   // Fetch the available framebuffer size.
@@ -343,34 +380,14 @@ QemuVideoBochsModeSetup (
     return EFI_OUT_OF_RESOURCES;
   }
 
-  ModeData  = Private->ModeData;
-  VideoMode = &QemuVideoBochsModes[0];
   for (Index = 0; Index < QEMU_VIDEO_BOCHS_MODE_COUNT; Index++) {
-    UINTN  RequiredFbSize;
-
-    RequiredFbSize = (UINTN)VideoMode->Width * VideoMode->Height * 4;
-    if (RequiredFbSize <= AvailableFbSize) {
-      ModeData->InternalModeIndex    = Index;
-      ModeData->HorizontalResolution = VideoMode->Width;
-      ModeData->VerticalResolution   = VideoMode->Height;
-      ModeData->ColorDepth           = 32;
-      DEBUG ((
-        DEBUG_INFO,
-        "Adding Mode %d as Bochs Internal Mode %d: %dx%d, %d-bit\n",
-        (INT32)(ModeData - Private->ModeData),
-        ModeData->InternalModeIndex,
-        ModeData->HorizontalResolution,
-        ModeData->VerticalResolution,
-        ModeData->ColorDepth
-        ));
-
-      ModeData++;
-    }
-
-    VideoMode++;
+    QemuVideoBochsAddMode (
+      Private,
+      AvailableFbSize,
+      QemuVideoBochsModes[Index].Width,
+      QemuVideoBochsModes[Index].Height
+      );
   }
 
-  Private->MaxMode = ModeData - Private->ModeData;
-
   return EFI_SUCCESS;
 }
-- 
2.33.1

[PATCH 5/5] OvmfPkg/QemuVideoDxe: parse edid blob, detect display resolution

[Gerd Hoffmann]

Check whenever an EDID blob is present.  In case it is get the display
resolution from it.  Unless PcdVideoResolutionSource indicates the
display resolution has been set already update
PcdVideoHorizontalResolution and PcdVideoVerticalResolution accordingly.
Also add the resolution to the mode list.

This will make OVMF boot up with the display resolution configured by
QEMU, which is 1024x768 by default.  The resolution can be set using the
xres and yres properties.  Here is an example for FullHD:

qemu-system-x86_64 -device VGA,xres=1920,yres=1080

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3778
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1749250
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf |   3 +
 OvmfPkg/QemuVideoDxe/Qemu.h           |   2 +
 OvmfPkg/QemuVideoDxe/Initialize.c     | 102 +++++++++++++++++++++++++-
 3 files changed, 105 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
index fe8befd51d3c..43a6e07faa88 100644
--- a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+++ b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
@@ -63,4 +63,7 @@ [Protocols]
 
 [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
+  gUefiOvmfPkgTokenSpaceGuid.PcdVideoResolutionSource
   gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution
diff --git a/OvmfPkg/QemuVideoDxe/Qemu.h b/OvmfPkg/QemuVideoDxe/Qemu.h
index 1e6507f44caa..57341a0bbfc4 100644
--- a/OvmfPkg/QemuVideoDxe/Qemu.h
+++ b/OvmfPkg/QemuVideoDxe/Qemu.h
@@ -115,6 +115,8 @@ typedef struct {
   FRAME_BUFFER_CONFIGURE          *FrameBufferBltConfigure;
   UINTN                           FrameBufferBltConfigureSize;
   UINT8                           FrameBufferVramBarIndex;
+
+  UINT8                           Edid[128];
 } QEMU_VIDEO_PRIVATE_DATA;
 
 ///
diff --git a/OvmfPkg/QemuVideoDxe/Initialize.c b/OvmfPkg/QemuVideoDxe/Initialize.c
index 8c5c9176ad21..2a3cbc65c32a 100644
--- a/OvmfPkg/QemuVideoDxe/Initialize.c
+++ b/OvmfPkg/QemuVideoDxe/Initialize.c
@@ -284,6 +284,88 @@ QemuVideoBochsAddMode (
   Private->MaxMode++;
 }
 
+STATIC
+VOID
+QemuVideoBochsEdid (
+  QEMU_VIDEO_PRIVATE_DATA  *Private,
+  UINT32                   *XRes,
+  UINT32                   *YRes
+  )
+{
+  EFI_STATUS  Status;
+
+  if (Private->Variant != QEMU_VIDEO_BOCHS_MMIO) {
+    return;
+  }
+
+  Status = Private->PciIo->Mem.Read (
+                                 Private->PciIo,
+                                 EfiPciIoWidthUint8,
+                                 PCI_BAR_IDX2,
+                                 0,
+                                 sizeof (Private->Edid),
+                                 Private->Edid
+                                 );
+  if (Status != EFI_SUCCESS) {
+    DEBUG ((
+      DEBUG_INFO,
+      "%a: mmio read failed\n",
+      __FUNCTION__
+      ));
+    return;
+  }
+
+  if ((Private->Edid[0] != 0x00) ||
+      (Private->Edid[1] != 0xff))
+  {
+    DEBUG ((
+      DEBUG_INFO,
+      "%a: magic check failed\n",
+      __FUNCTION__
+      ));
+    return;
+  }
+
+  DEBUG ((
+    DEBUG_INFO,
+    "%a: blob found (extensions: %d)\n",
+    __FUNCTION__,
+    Private->Edid[126]
+    ));
+
+  if ((Private->Edid[54] == 0x00) &&
+      (Private->Edid[55] == 0x00))
+  {
+    DEBUG ((
+      DEBUG_INFO,
+      "%a: no detailed timing descriptor\n",
+      __FUNCTION__
+      ));
+    return;
+  }
+
+  *XRes = Private->Edid[56] | ((Private->Edid[58] & 0xf0) << 4);
+  *YRes = Private->Edid[59] | ((Private->Edid[61] & 0xf0) << 4);
+  DEBUG ((
+    DEBUG_INFO,
+    "%a: default resolution: %dx%d\n",
+    __FUNCTION__,
+    *XRes,
+    *YRes
+    ));
+
+  if (PcdGet8 (PcdVideoResolutionSource) == 0) {
+    Status = PcdSet32S (PcdVideoHorizontalResolution, *XRes);
+    ASSERT_RETURN_ERROR (Status);
+    Status = PcdSet32S (PcdVideoVerticalResolution, *YRes);
+    ASSERT_RETURN_ERROR (Status);
+    Status = PcdSet8S (PcdVideoResolutionSource, 2);
+    ASSERT_RETURN_ERROR (Status);
+  }
+
+  // TODO: register edid as gEfiEdidDiscoveredProtocolGuid ?
+}
+
 EFI_STATUS
 QemuVideoBochsModeSetup (
   QEMU_VIDEO_PRIVATE_DATA  *Private,
@@ -291,7 +373,7 @@ QemuVideoBochsModeSetup (
   )
 {
   UINT32  AvailableFbSize;
-  UINT32  Index;
+  UINT32  Index, XRes = 0, YRes = 0;
 
   //
   // Fetch the available framebuffer size.
@@ -374,13 +456,29 @@ QemuVideoBochsModeSetup (
   // Setup Video Modes
   //
   Private->ModeData = AllocatePool (
-                        sizeof (Private->ModeData[0]) * QEMU_VIDEO_BOCHS_MODE_COUNT
+                        sizeof (Private->ModeData[0]) * (QEMU_VIDEO_BOCHS_MODE_COUNT+1)
                         );
   if (Private->ModeData == NULL) {
     return EFI_OUT_OF_RESOURCES;
   }
 
+  QemuVideoBochsEdid (Private, &XRes, &YRes);
+  if (XRes && YRes) {
+    QemuVideoBochsAddMode (
+      Private,
+      AvailableFbSize,
+      XRes,
+      YRes
+      );
+  }
+
   for (Index = 0; Index < QEMU_VIDEO_BOCHS_MODE_COUNT; Index++) {
+    if ((QemuVideoBochsModes[Index].Width == XRes) &&
+        (QemuVideoBochsModes[Index].Height == YRes))
+    {
+      continue; // duplicate with edid resolution
+    }
+
     QemuVideoBochsAddMode (
       Private,
       AvailableFbSize,
-- 
2.33.1

Re: [edk2-devel] [PATCH 0/5] OvmfPkg/QemuVideoDxe: pick up display resolution settings from the host

[Ard Biesheuvel]

On Thu, 16 Dec 2021 at 15:30, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> See last patch in the series for details.
>
> Gerd Hoffmann (5):
>   OvmfPkg: add PcdVideoResolutionSource
>   OvmfPkg/QemuVideoDxe: simplify InitializeBochsGraphicsMode
>   OvmfPkg/QemuVideoDxe: drop QEMU_VIDEO_BOCHS_MODES->ColorDepth
>   OvmfPkg/QemuVideoDxe: factor out QemuVideoBochsAddMode
>   OvmfPkg/QemuVideoDxe: parse edid blob, detect display resolution
>

This looks fine to me in principle, the only question I have about is
the use of a PCD PcdVideoResolutionSource to keep track of what the
height/width PCDs mean.

I don't think this is very idiomatic for EDK2, and I wonder if it
would be better to use [fixed] PCDs for build time configuration, and
perhaps expose the other width/height data (and the associated policy
of what takes precedence over what) via a protocol instead.

In general, I am not too keen on overusing PCDs for any of this, as it
does not have the implied dependency ordering that PPIs or protocols
have.

Jiewen, any thoughts?


>  OvmfPkg/OvmfPkg.dec                   |   7 +
>  OvmfPkg/AmdSev/AmdSevX64.dsc          |   1 +
>  OvmfPkg/Microvm/MicrovmX64.dsc        |   1 +
>  OvmfPkg/OvmfPkgIa32.dsc               |   1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc            |   1 +
>  OvmfPkg/OvmfPkgX64.dsc                |   1 +
>  OvmfPkg/OvmfXen.dsc                   |   1 +
>  OvmfPkg/PlatformDxe/Platform.inf      |   1 +
>  OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf |   3 +
>  OvmfPkg/QemuVideoDxe/Qemu.h           |   6 +-
>  OvmfPkg/PlatformDxe/Platform.c        |   3 +
>  OvmfPkg/QemuVideoDxe/Driver.c         |  14 +-
>  OvmfPkg/QemuVideoDxe/Gop.c            |   2 +-
>  OvmfPkg/QemuVideoDxe/Initialize.c     | 251 +++++++++++++++++++-------
>  14 files changed, 213 insertions(+), 80 deletions(-)
>
> --
> 2.33.1
>
>
>
> 
>
>

Re: [edk2-devel] [PATCH 0/5] OvmfPkg/QemuVideoDxe: pick up display resolution settings from the host

[Gerd Hoffmann]

  Hi,

> I don't think this is very idiomatic for EDK2, and I wonder if it
> would be better to use [fixed] PCDs for build time configuration, and
> perhaps expose the other width/height data (and the associated policy
> of what takes precedence over what) via a protocol instead.

Well, yes.  Ideally we would just tag the resolution in the GOP protocol
mode list, simliar to the linux kernel which has a
DRM_MODE_TYPE_PREFERRED flag for that purpose.  Problem is the GOP
protocol doesn't support that ...

It's also kind-of specific to virtual machines.  On physical hardware
just using the highest resolution available works just fine as that is
typically the native display resolution.  In a virtual machine you don't
want come up with a huge 4k window by default just because the virtual
vga is able to handle that.  Cutting down the video mode list isn't a
great solution either as that would also remove the modes from the
platform configuration so the user wouldn't be able to pick a resolution
higher than the default any more.

> In general, I am not too keen on overusing PCDs for any of this, as it
> does not have the implied dependency ordering that PPIs or protocols
> have.

Laszlo mentioned OvmfPkg has a less strict policy than the rest of edk2
for this kind of pcd usage.

take care,
  Gerd