Re: [PATCH 7/8] hw/ppc: Set graphic display dimensions generically

Sun, 15 Feb 2026 01:52:45 -0800 

On Sun, 15 Feb 2026, Philippe Mathieu-Daudé wrote:

On 14/2/26 12:46, BALATON Zoltan wrote:

On Sat, 14 Feb 2026, Philippe Mathieu-Daudé wrote:

If a dimension is not set, have the machine init code set
the default value by calling the ppc_graphic_dimensions()
helper, common to all PowerPC machines. Declare local
variables to avoid using the global ones.

Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
---
include/hw/ppc/ppc.h    |  2 ++
hw/ppc/mac_newworld.c   | 10 ++++++----
hw/ppc/mac_oldworld.c   | 10 ++++++----
hw/ppc/ppc.c            |  8 ++++++++
hw/ppc/prep.c           |  4 ++++
hw/ppc/spapr.c          |  4 ++++
system/globals-target.c |  6 ------
7 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/include/hw/ppc/ppc.h b/include/hw/ppc/ppc.h
index cb51d704c6d..14cc09ab22b 100644
--- a/include/hw/ppc/ppc.h
+++ b/include/hw/ppc/ppc.h
@@ -102,6 +102,8 @@ enum {
    ARCH_MAC99_U3,
};

+void ppc_graphic_dimensions(int *width, int *height, int *depth);
+
#define FW_CFG_PPC_WIDTH        (FW_CFG_ARCH_LOCAL + 0x00)
#define FW_CFG_PPC_HEIGHT       (FW_CFG_ARCH_LOCAL + 0x01)
#define FW_CFG_PPC_DEPTH        (FW_CFG_ARCH_LOCAL + 0x02)
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 7275563a155..daf0029c01a 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -156,6 +156,7 @@ static void ppc_core99_init(MachineState *machine)
    DeviceState *uninorth_internal_dev = NULL, *uninorth_agp_dev = NULL;
    hwaddr nvram_addr = 0xFFF04000;
    uint64_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TBFREQ;
+    int graphic_width, graphic_height, graphic_depth;

    /* init CPUs */
    for (i = 0; i < machine->smp.cpus; i++) {
@@ -432,10 +433,6 @@ static void ppc_core99_init(MachineState *machine)

    pci_vga_init(pci_bus);
-    if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth ! = 8) { 
-        graphic_depth = 15;
-    }
-
    pci_init_nic_devices(pci_bus, mc->default_nic);

    /* The NewWorld NVRAM is not located in the MacIO device */
@@ -480,6 +477,11 @@ static void ppc_core99_init(MachineState *machine)
    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, ppc_boot_device);
+    ppc_graphic_dimensions(&graphic_width, &graphic_height, &graphic_depth); +    if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth ! = 8) { 
+        graphic_depth = 15;
+    }
+
    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_WIDTH, graphic_width);
    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_HEIGHT, graphic_height);
    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_DEPTH, graphic_depth);
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index e679d338985..ea1f778877c 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -108,6 +108,7 @@ static void ppc_heathrow_init(MachineState *machine)
    DriveInfo *dinfo, *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
    void *fw_cfg;
    uint64_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TBFREQ;
+    int graphic_width, graphic_height, graphic_depth;

    /* init CPUs */
    for (i = 0; i < machine->smp.cpus; i++) {
@@ -288,10 +289,6 @@ static void ppc_heathrow_init(MachineState *machine)
        pci_create_simple(pci_bus, -1, "pci-ohci");
    }
-    if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth ! = 8) { 
-        graphic_depth = 15;
-    }
-
    /* No PCI init: the BIOS will do it */

    dev = qdev_new(TYPE_FW_CFG_MEM);
@@ -321,6 +318,11 @@ static void ppc_heathrow_init(MachineState *machine)
    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, ppc_boot_device);
+    ppc_graphic_dimensions(&graphic_width, &graphic_height, &graphic_depth); +    if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth ! = 8) { 
+        graphic_depth = 15;
+    }
+
    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_WIDTH, graphic_width);
    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_HEIGHT, graphic_height);
    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_DEPTH, graphic_depth);
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index a512d4fa647..d7b4466d701 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -29,6 +29,7 @@
#include "qemu/timer.h"
#include "exec/cpu-interrupt.h"
#include "system/cpus.h"
+#include "system/system.h"
#include "qemu/log.h"
#include "qemu/main-loop.h"
#include "qemu/error-report.h"
@@ -1557,3 +1558,10 @@ void ppc_irq_reset(PowerPCCPU *cpu)
        kvmppc_set_interrupt(cpu, PPC_INTERRUPT_EXT, 0);
    }
}
+
+void ppc_graphic_dimensions(int *width, int *height, int *depth)
+{
+    *width = graphic_width ?: 800;
+    *height = graphic_height ?: 600;
+    *depth = graphic_depth ?: 32;
+}
I think there's no reason to have this common to all ppc machines. If we change this we could also make it per machine like for other targets. There's nothing common in these machines that dictate these defaults. It seems it used to be an arbitrary default in QEMU that some machines later deviated from. So might as well just keep it as global for now after you removed the SPARC and M68K special cases. If the ppc machines are the last users of these global defaults you can just move them to hw/ppc/ppc.{h,c} for now and let ppc people clean this up eventually. 

What you are asking is to use 800x600x32 as the default for all
non-SPARC / non-M68K targets, so PPC don't have to change? But then
This is what the original code did. But now I get that you need a way to tell if these were set so changed the default to 0 so the sparc and m68k can set their own defaults if the user did not provide values. 


SPARC and M68k won't be able to use the '-g' CLI (which Paolo asked
me not to remove, thus this series). What is worrying you exactly?
Too much churn and introducing interdependency between ppc machines that should be removed later. The ppc_graphic_dimensions does not make sense as these are different machines with potentially different default resolutions (and most of these should be able to use EDID that we since have in QEMU). 


Can you provide a patch showing how you'd prefer it to be done?
Maybe turning these into properties could work. I could look at adding such properties but don't know how to change the -g option. But considering these are all machines that Mark maintains maybe he should do something about it. The easiest were if he agreed to drop -g option but he never agrees removing anything so then he should provide patches to clean this up the way he prefers. I could try to make patches but I'm afraid those would become wasted effort so may I only do it when there's an approved way that will be accepted. 

Regards,
BALATON Zoltan

Reply via email to