* [PATCH 1/5] OvmfPkg: add PcdVideoResolutionSource
2021-12-16 14:29 [PATCH 0/5] OvmfPkg/QemuVideoDxe: pick up display resolution settings from the host Gerd Hoffmann
@ 2021-12-16 14:29 ` Gerd Hoffmann
2021-12-16 14:29 ` [PATCH 2/5] OvmfPkg/QemuVideoDxe: simplify InitializeBochsGraphicsMode Gerd Hoffmann
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2021-12-16 14:29 UTC (permalink / raw)
To: devel
Cc: Erdem Aktas, Julien Grall, Brijesh Singh, Pawel Polawski, Min Xu,
Jiewen Yao, Tom Lendacky, Ard Biesheuvel, Jordan Justen,
Philippe Mathieu-Daudé, James Bottomley, Anthony Perard,
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/5] OvmfPkg/QemuVideoDxe: simplify InitializeBochsGraphicsMode
2021-12-16 14:29 [PATCH 0/5] OvmfPkg/QemuVideoDxe: pick up display resolution settings from the host Gerd Hoffmann
2021-12-16 14:29 ` [PATCH 1/5] OvmfPkg: add PcdVideoResolutionSource Gerd Hoffmann
@ 2021-12-16 14:29 ` Gerd Hoffmann
2021-12-16 14:29 ` [PATCH 3/5] OvmfPkg/QemuVideoDxe: drop QEMU_VIDEO_BOCHS_MODES->ColorDepth Gerd Hoffmann
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2021-12-16 14:29 UTC (permalink / raw)
To: devel
Cc: Erdem Aktas, Julien Grall, Brijesh Singh, Pawel Polawski, Min Xu,
Jiewen Yao, Tom Lendacky, Ard Biesheuvel, Jordan Justen,
Philippe Mathieu-Daudé, James Bottomley, Anthony Perard,
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 3/5] OvmfPkg/QemuVideoDxe: drop QEMU_VIDEO_BOCHS_MODES->ColorDepth
2021-12-16 14:29 [PATCH 0/5] OvmfPkg/QemuVideoDxe: pick up display resolution settings from the host Gerd Hoffmann
2021-12-16 14:29 ` [PATCH 1/5] OvmfPkg: add PcdVideoResolutionSource Gerd Hoffmann
2021-12-16 14:29 ` [PATCH 2/5] OvmfPkg/QemuVideoDxe: simplify InitializeBochsGraphicsMode Gerd Hoffmann
@ 2021-12-16 14:29 ` Gerd Hoffmann
2021-12-16 14:29 ` [PATCH 4/5] OvmfPkg/QemuVideoDxe: factor out QemuVideoBochsAddMode Gerd Hoffmann
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2021-12-16 14:29 UTC (permalink / raw)
To: devel
Cc: Erdem Aktas, Julien Grall, Brijesh Singh, Pawel Polawski, Min Xu,
Jiewen Yao, Tom Lendacky, Ard Biesheuvel, Jordan Justen,
Philippe Mathieu-Daudé, James Bottomley, Anthony Perard,
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 4/5] OvmfPkg/QemuVideoDxe: factor out QemuVideoBochsAddMode
2021-12-16 14:29 [PATCH 0/5] OvmfPkg/QemuVideoDxe: pick up display resolution settings from the host Gerd Hoffmann
` (2 preceding siblings ...)
2021-12-16 14:29 ` [PATCH 3/5] OvmfPkg/QemuVideoDxe: drop QEMU_VIDEO_BOCHS_MODES->ColorDepth Gerd Hoffmann
@ 2021-12-16 14:29 ` Gerd Hoffmann
2021-12-16 14:29 ` [PATCH 5/5] OvmfPkg/QemuVideoDxe: parse edid blob, detect display resolution Gerd Hoffmann
2021-12-16 14:52 ` [edk2-devel] [PATCH 0/5] OvmfPkg/QemuVideoDxe: pick up display resolution settings from the host Ard Biesheuvel
5 siblings, 0 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2021-12-16 14:29 UTC (permalink / raw)
To: devel
Cc: Erdem Aktas, Julien Grall, Brijesh Singh, Pawel Polawski, Min Xu,
Jiewen Yao, Tom Lendacky, Ard Biesheuvel, Jordan Justen,
Philippe Mathieu-Daudé, James Bottomley, Anthony Perard,
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 5/5] OvmfPkg/QemuVideoDxe: parse edid blob, detect display resolution
2021-12-16 14:29 [PATCH 0/5] OvmfPkg/QemuVideoDxe: pick up display resolution settings from the host Gerd Hoffmann
` (3 preceding siblings ...)
2021-12-16 14:29 ` [PATCH 4/5] OvmfPkg/QemuVideoDxe: factor out QemuVideoBochsAddMode Gerd Hoffmann
@ 2021-12-16 14:29 ` Gerd Hoffmann
2021-12-16 14:52 ` [edk2-devel] [PATCH 0/5] OvmfPkg/QemuVideoDxe: pick up display resolution settings from the host Ard Biesheuvel
5 siblings, 0 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2021-12-16 14:29 UTC (permalink / raw)
To: devel
Cc: Erdem Aktas, Julien Grall, Brijesh Singh, Pawel Polawski, Min Xu,
Jiewen Yao, Tom Lendacky, Ard Biesheuvel, Jordan Justen,
Philippe Mathieu-Daudé, James Bottomley, Anthony Perard,
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [edk2-devel] [PATCH 0/5] OvmfPkg/QemuVideoDxe: pick up display resolution settings from the host
2021-12-16 14:29 [PATCH 0/5] OvmfPkg/QemuVideoDxe: pick up display resolution settings from the host Gerd Hoffmann
` (4 preceding siblings ...)
2021-12-16 14:29 ` [PATCH 5/5] OvmfPkg/QemuVideoDxe: parse edid blob, detect display resolution Gerd Hoffmann
@ 2021-12-16 14:52 ` Ard Biesheuvel
2021-12-17 6:28 ` Gerd Hoffmann
5 siblings, 1 reply; 8+ messages in thread
From: Ard Biesheuvel @ 2021-12-16 14:52 UTC (permalink / raw)
To: edk2-devel-groups-io, Gerd Hoffmann
Cc: Erdem Aktas, Julien Grall, Brijesh Singh, Pawel Polawski, Min Xu,
Jiewen Yao, Tom Lendacky, Ard Biesheuvel, Jordan Justen,
Philippe Mathieu-Daudé, James Bottomley, Anthony Perard
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
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [edk2-devel] [PATCH 0/5] OvmfPkg/QemuVideoDxe: pick up display resolution settings from the host
2021-12-16 14:52 ` [edk2-devel] [PATCH 0/5] OvmfPkg/QemuVideoDxe: pick up display resolution settings from the host Ard Biesheuvel
@ 2021-12-17 6:28 ` Gerd Hoffmann
0 siblings, 0 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2021-12-17 6:28 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel-groups-io, Erdem Aktas, Julien Grall, Brijesh Singh,
Pawel Polawski, Min Xu, Jiewen Yao, Tom Lendacky, Ard Biesheuvel,
Jordan Justen, Philippe Mathieu-Daudé, James Bottomley,
Anthony Perard
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
^ permalink raw reply [flat|nested] 8+ messages in thread