[PULL 10/20] hw/ppc: Set graphic display dimensions generically

Philippe Mathieu-Daudé posted 20 patches 1 month, 2 weeks ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Laurent Vivier <laurent@vivier.eu>, Jiri Pirko <jiri@resnulli.us>, Jason Wang <jasowang@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Nicholas Piggin <npiggin@gmail.com>, Chinmay Rath <rathc@linux.ibm.com>, "Hervé Poussineau" <hpoussin@reactos.org>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Artyom Tarasenko <atar4qemu@gmail.com>, Stefano Garzarella <sgarzare@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>, Zhao Liu <zhao1.liu@intel.com>, Dmitry Fleytman <dmitry.fleytman@gmail.com>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
There is a newer version of this series
[PULL 10/20] hw/ppc: Set graphic display dimensions generically
Posted by Philippe Mathieu-Daudé 1 month, 2 weeks ago
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é <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20260216213121.47122-8-philmd@linaro.org>
---
 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;
+}
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index c4efd1d3908..7077b047b25 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -43,6 +43,7 @@
 #include "exec/target_page.h"
 #include "system/kvm.h"
 #include "system/reset.h"
+#include "system/system.h"
 #include "trace.h"
 #include "elf.h"
 #include "qemu/units.h"
@@ -250,6 +251,7 @@ static void ibm_40p_init(MachineState *machine)
     uint32_t kernel_base = 0, initrd_base = 0;
     long kernel_size = 0, initrd_size = 0;
     char boot_device;
+    int graphic_width, graphic_height, graphic_depth;
 
     if (kvm_enabled()) {
         error_report("machine %s does not support the KVM accelerator",
@@ -412,6 +414,8 @@ static void ibm_40p_init(MachineState *machine)
     fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)machine->ram_size);
     fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, ARCH_PREP);
 
+    ppc_graphic_dimensions(&graphic_width, &graphic_height, &graphic_depth);
+
     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/spapr.c b/hw/ppc/spapr.c
index 274f38785f2..c50c7da34d5 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1143,6 +1143,7 @@ static void spapr_dt_chosen(SpaprMachineState *spapr, void *fdt, bool reset)
 {
     MachineState *machine = MACHINE(spapr);
     int chosen;
+    int graphic_width, graphic_height, graphic_depth;
 
     _FDT(chosen = fdt_add_subnode(fdt, 0, "chosen"));
 
@@ -1177,6 +1178,9 @@ static void spapr_dt_chosen(SpaprMachineState *spapr, void *fdt, bool reset)
         if (machine->boot_config.has_menu && machine->boot_config.menu) {
             _FDT((fdt_setprop_cell(fdt, chosen, "qemu,boot-menu", true)));
         }
+
+        ppc_graphic_dimensions(&graphic_width, &graphic_height, &graphic_depth);
+
         _FDT(fdt_setprop_cell(fdt, chosen, "qemu,graphic-width", graphic_width));
         _FDT(fdt_setprop_cell(fdt, chosen, "qemu,graphic-height", graphic_height));
         _FDT(fdt_setprop_cell(fdt, chosen, "qemu,graphic-depth", graphic_depth));
diff --git a/system/globals-target.c b/system/globals-target.c
index 17a27a06218..ffa6c308b59 100644
--- a/system/globals-target.c
+++ b/system/globals-target.c
@@ -9,12 +9,6 @@
 #include "qemu/osdep.h"
 #include "system/system.h"
 
-#if defined(TARGET_SPARC) || defined(TARGET_M68K)
 int graphic_width;
 int graphic_height;
 int graphic_depth;
-#else
-int graphic_width = 800;
-int graphic_height = 600;
-int graphic_depth = 32;
-#endif
-- 
2.52.0


Re: [PULL 10/20] hw/ppc: Set graphic display dimensions generically
Posted by BALATON Zoltan 1 month, 2 weeks ago
On Mon, 23 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é <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Message-Id: <20260216213121.47122-8-philmd@linaro.org>
> ---
> 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;
> +}

Sorry but didn't we say that there should be no common 
ppc_graphic_dimensions function but each board should set its own 
defaults?

Regards,
BALATON Zoltan
Re: [PULL 10/20] hw/ppc: Set graphic display dimensions generically
Posted by BALATON Zoltan 1 month, 2 weeks ago
On Mon, 23 Feb 2026, BALATON Zoltan wrote:
> On Mon, 23 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é <philmd@linaro.org>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Message-Id: <20260216213121.47122-8-philmd@linaro.org>
>> ---
>> 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;
>> +}
>
> Sorry but didn't we say that there should be no common ppc_graphic_dimensions 
> function but each board should set its own defaults?

OK I see there's a v2 with the correct patches.

Regards,
BALATON Zoltan
Re: [PULL 10/20] hw/ppc: Set graphic display dimensions generically
Posted by Philippe Mathieu-Daudé 1 month, 2 weeks ago
On 23/2/26 00:50, BALATON Zoltan wrote:
> On Mon, 23 Feb 2026, BALATON Zoltan wrote:
>> On Mon, 23 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é <philmd@linaro.org>
>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>> Message-Id: <20260216213121.47122-8-philmd@linaro.org>
>>> ---
>>> 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/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;
>>> +}
>>
>> Sorry but didn't we say that there should be no common 
>> ppc_graphic_dimensions function but each board should set its own 
>> defaults?
> 
> OK I see there's a v2 with the correct patches.

Yeah sorry I swiched back in the older branch to check something
and forgot to go to the newer again before posting the PR!