On Wed, Jul 04, 2018 at 01:28:59PM +0200, Gerd Hoffmann wrote: > This reverts the stdvga vs. cirrus selection logic. Current state is > that "cirrus" is the default and MachineClass->default_display is set to > "std" by some machine types to override that. > > The patch makes "std" the default. Setting default_display to "std" is > dropped. Machine types which likely depend on cirrus (alpha, mips, old > pc versions) will explicitly set default_display to "cirrus". > > With this patch applied all ppc machine types will use "std" as default > display, no matter whenever cirrus-vga is compiled in or not. > > Fixes: 29f9cef39e ppc: Include vga cirrus card into the compiling process > Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
So, the current state is that default_display==NULL is unpredictable and fragile. The last hunk in this patch changes the default when default_dispay==NULL, but it's still fragile. I don't think we must audit all machine-types with default_display=NULL today. But: [...] > diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c > index 80b987f7fb..668e6d099f 100644 > --- a/hw/alpha/dp264.c > +++ b/hw/alpha/dp264.c > @@ -182,6 +182,7 @@ static void clipper_machine_init(MachineClass *mc) > mc->max_cpus = 4; > mc->is_default = 1; > mc->default_cpu_type = ALPHA_CPU_TYPE_NAME("ev67"); > + mc->default_display = "cirrus"; OK. > } > > DEFINE_MACHINE("clipper", clipper_machine_init) > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index dc09466b3e..30ad22e2e3 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -424,7 +424,6 @@ static void pc_i440fx_machine_options(MachineClass *m) > m->family = "pc_piix"; > m->desc = "Standard PC (i440FX + PIIX, 1996)"; > m->default_machine_opts = "firmware=bios-256k.bin"; > - m->default_display = "std"; > machine_class_allow_dynamic_sysbus_dev(m, TYPE_RAMFB_DEVICE); I wouldn't like to do this. Long term, it would be a good idea to have zero machines with default_display==NULL, so let's keep default_machine="std" on PC? > } > > @@ -566,7 +565,7 @@ static void pc_i440fx_2_1_machine_options(MachineClass *m) > PCMachineClass *pcmc = PC_MACHINE_CLASS(m); > pc_i440fx_2_2_machine_options(m); > m->hw_version = "2.1.0"; > - m->default_display = NULL; > + m->default_display = "cirrus"; OK. > SET_MACHINE_COMPAT(m, PC_COMPAT_2_1); > pcmc->smbios_uuid_encoded = false; > pcmc->enforce_aligned_dimm = false; > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index 532241e3f8..8f2c8c8b8b 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -303,7 +303,6 @@ static void pc_q35_machine_options(MachineClass *m) > m->desc = "Standard PC (Q35 + ICH9, 2009)"; > m->units_per_default_bus = 1; > m->default_machine_opts = "firmware=bios-256k.bin"; > - m->default_display = "std"; Same comment as pc_i440fx_machine_options() above. > m->no_floppy = 1; > machine_class_allow_dynamic_sysbus_dev(m, TYPE_AMD_IOMMU_DEVICE); > machine_class_allow_dynamic_sysbus_dev(m, TYPE_INTEL_IOMMU_DEVICE); > diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c > index 3467451482..998971c53a 100644 > --- a/hw/mips/mips_malta.c > +++ b/hw/mips/mips_malta.c > @@ -1242,6 +1242,7 @@ static void mips_malta_machine_init(MachineClass *mc) > mc->block_default_type = IF_IDE; > mc->max_cpus = 16; > mc->is_default = 1; > + mc->default_display = "cirrus"; OK. > #ifdef TARGET_MIPS64 > mc->default_cpu_type = MIPS_CPU_TYPE_NAME("20Kc"); > #else > diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c > index d5725d0555..c4c7ee8aa5 100644 > --- a/hw/mips/mips_r4k.c > +++ b/hw/mips/mips_r4k.c > @@ -295,6 +295,7 @@ static void mips_machine_init(MachineClass *mc) > mc->desc = "mips r4k platform"; > mc->init = mips_r4k_init; > mc->block_default_type = IF_IDE; > + mc->default_display = "cirrus"; OK. > #ifdef TARGET_MIPS64 > mc->default_cpu_type = MIPS_CPU_TYPE_NAME("R4000"); > #else > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 3f5e1d3ec2..0ab90f3aa8 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -3962,7 +3962,6 @@ static void spapr_machine_class_init(ObjectClass *oc, > void *data) > mc->no_parallel = 1; > mc->default_boot_order = ""; > mc->default_ram_size = 512 * MiB; > - mc->default_display = "std"; Same comment as pc_i440fx_machine_options() above. > mc->kvm_type = spapr_kvm_type; > machine_class_allow_dynamic_sysbus_dev(mc, TYPE_SPAPR_PCI_HOST_BRIDGE); > mc->pci_allow_0_address = true; > diff --git a/vl.c b/vl.c > index 16b913f9d5..e60bf2d6cd 100644 > --- a/vl.c > +++ b/vl.c > @@ -4475,10 +4475,10 @@ int main(int argc, char **argv, char **envp) > if (default_vga) { > if (machine_class->default_display) { > vga_model = machine_class->default_display; > - } else if (vga_interface_available(VGA_CIRRUS)) { > - vga_model = "cirrus"; > } else if (vga_interface_available(VGA_STD)) { > vga_model = "std"; > + } else if (vga_interface_available(VGA_CIRRUS)) { > + vga_model = "cirrus"; If we don't make the machines above have default_display=NULL, we won't need this hunk, right? In either case, I suggest doing this change on a separate patch. -- Eduardo