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

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

[Gerd Hoffmann]

See last patch in the series for details.

v2:
 - rebase to latest master
 - more verbose commit message explaining the
   motivation for the new PCD (see patch #1).

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.34.1

[PATCH v2 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.

The underlying problem this tries to solve is that the GOP protocol has
no way to indicate the preferred video mode.  On physical hardware this
isn't much of a problem because using the highest resolution available
works just fine as that is typically the native display resolution

But 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.

So with patch drivers can use use PcdVideoHorizontalResolution and
PcdVideoVerticalResolution to indicate what the preferred display
resolution is, without overwriting the user preferences from
PlatformConfig if present.

A possible alternative approach would be to extend the GOP protocol, but
I'm not sure this is a good plan given this is mostly a problem for
virtual machines and using PCDs allows to keep this local to OvmfPkg.

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.34.1

[PATCH v2 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.34.1

[PATCH v2 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.34.1

[PATCH v2 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.34.1

[PATCH v2 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.34.1

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

[Ard Biesheuvel]

On Mon, 17 Jan 2022 at 10:58, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> See last patch in the series for details.
>
> v2:
>  - rebase to latest master
>  - more verbose commit message explaining the
>    motivation for the new PCD (see patch #1).
>
> 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
>

Acked-by: Ard Biesheuvel <ardb@kernel.org>

Pushed as #2455

Thanks,

>  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.34.1
>