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

Gerd Hoffmann posted 5 patches 2 years, 3 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
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(-)
[edk2-devel] [PATCH 0/5] OvmfPkg/QemuVideoDxe: pick up display resolution settings from the host
Posted by Gerd Hoffmann 2 years, 3 months ago
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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84995): https://edk2.groups.io/g/devel/message/84995
Mute This Topic: https://groups.io/mt/87767746/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 0/5] OvmfPkg/QemuVideoDxe: pick up display resolution settings from the host
Posted by Ard Biesheuvel 2 years, 3 months ago
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
>
>
>
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85001): https://edk2.groups.io/g/devel/message/85001
Mute This Topic: https://groups.io/mt/87767746/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 0/5] OvmfPkg/QemuVideoDxe: pick up display resolution settings from the host
Posted by Gerd Hoffmann 2 years, 3 months ago
  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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85049): https://edk2.groups.io/g/devel/message/85049
Mute This Topic: https://groups.io/mt/87767746/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-