From: Sebastian Bauer <mail@sebastianbauer.info>

Drivers for this card exists on PPC-based AmigaOS guests so it is useful to
allow users to emulate the graphics card for PPC machines.

As cirrus vga is currently preferred over std(vga) in absence of any user
choice, this change also sets the default display of spapr machines to
std as otherwise qemu refuses to start these machines. Not specifying an
explicit graphics mode is for instance done by 'make check'.

Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 default-configs/ppc-softmmu.mak | 1 +
 hw/ppc/spapr.c                  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index 7e1a3d8135..6f12bf84f0 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -29,6 +29,7 @@ CONFIG_IDE_SII3112=y
 CONFIG_I2C=y
 CONFIG_BITBANG_I2C=y
 CONFIG_M41T80=y
+CONFIG_VGA_CIRRUS=y
 
 # For Macs
 CONFIG_MAC=y
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 062d9dc346..6e8723d3ba 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3962,6 +3962,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
     mc->no_parallel = 1;
     mc->default_boot_order = "";
     mc->default_ram_size = 512 * M_BYTE;
+    mc->default_display = "std";
     mc->kvm_type = spapr_kvm_type;
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_SPAPR_PCI_HOST_BRIDGE);
     mc->pci_allow_0_address = true;
-- 
2.17.1

On 03/07/18 06:58, David Gibson wrote:

> From: Sebastian Bauer <mail@sebastianbauer.info>
> 
> Drivers for this card exists on PPC-based AmigaOS guests so it is useful to
> allow users to emulate the graphics card for PPC machines.
> 
> As cirrus vga is currently preferred over std(vga) in absence of any user
> choice, this change also sets the default display of spapr machines to
> std as otherwise qemu refuses to start these machines.

... which has quite nicely broken the VGA display for both the PPC Mac 
machines. Patch to follow shortly.


ATB,

Mark.

Am 2018-07-03 21:00, schrieb Mark Cave-Ayland:
> On 03/07/18 06:58, David Gibson wrote:
> 
>> From: Sebastian Bauer <mail@sebastianbauer.info>
>> 
>> Drivers for this card exists on PPC-based AmigaOS guests so it is 
>> useful to
>> allow users to emulate the graphics card for PPC machines.
>> 
>> As cirrus vga is currently preferred over std(vga) in absence of any 
>> user
>> choice, this change also sets the default display of spapr machines to
>> std as otherwise qemu refuses to start these machines.
> 
> ... which has quite nicely broken the VGA display for both the PPC Mac
> machines. Patch to follow shortly.

Please see my note in the comment section of the patch where I propose 
two possible solutions:

Either to give up the vga cirrus preference or to apply the same as was 
done with the spapr machine. I would prefer the first variant but it 
would also cause some differences on other machines.

Bye
Sebastian

On 03/07/18 20:24, Sebastian Bauer wrote:

> Am 2018-07-03 21:00, schrieb Mark Cave-Ayland:
>> On 03/07/18 06:58, David Gibson wrote:
>>
>>> From: Sebastian Bauer <mail@sebastianbauer.info>
>>>
>>> Drivers for this card exists on PPC-based AmigaOS guests so it is 
>>> useful to
>>> allow users to emulate the graphics card for PPC machines.
>>>
>>> As cirrus vga is currently preferred over std(vga) in absence of any 
>>> user
>>> choice, this change also sets the default display of spapr machines to
>>> std as otherwise qemu refuses to start these machines.
>>
>> ... which has quite nicely broken the VGA display for both the PPC Mac
>> machines. Patch to follow shortly.
> 
> Please see my note in the comment section of the patch where I propose 
> two possible solutions:
> 
> Either to give up the vga cirrus preference or to apply the same as was 
> done with the spapr machine. I would prefer the first variant but it 
> would also cause some differences on other machines.

I understand that the decision was taken to add VGA cirrus to the PPC 
build and therefore accept the default VGA device change, however your 
the commit message only mentions spapr and not all the other PPC 
machines whose defaults are also potentially affected:

$ ./qemu-system-ppc -M ?
Supported machines are:
40p                  IBM RS/6000 7020 (40p)
bamboo               bamboo
g3beige              Heathrow based PowerMAC (default)
mac99                Mac99 based PowerMAC
mpc8544ds            mpc8544ds
none                 empty machine
ppce500              generic paravirt e500 platform
prep                 PowerPC PREP platform
ref405ep             ref405ep
sam460ex             aCube Sam460ex
taihu                taihu
virtex-ml507         Xilinx Virtex ML507 reference design

I'm not able to personally validate all of these, but a quick check 
shows that at least both PReP machines (-M prep and -M 40p) are also 
broken by this patch which tells me it hasn't had much testing.

David, do you know what the defaults are for the other PPC machines in 
order for Sebastian to test and send a follow-up patch?


ATB,

Mark.

Am 2018-07-04 06:50, schrieb Mark Cave-Ayland:
>> Either to give up the vga cirrus preference or to apply the same as 
>> was done with the spapr machine. I would prefer the first variant but 
>> it would also cause some differences on other machines.
> 
> I understand that the decision was taken to add VGA cirrus to the PPC
> build and therefore accept the default VGA device change, however your
> the commit message only mentions spapr and not all the other PPC
> machines whose defaults are also potentially affected:
> 
> $ ./qemu-system-ppc -M ?
> Supported machines are:
> 40p                  IBM RS/6000 7020 (40p)
> bamboo               bamboo
> g3beige              Heathrow based PowerMAC (default)
> mac99                Mac99 based PowerMAC
> mpc8544ds            mpc8544ds
> none                 empty machine
> ppce500              generic paravirt e500 platform
> prep                 PowerPC PREP platform
> ref405ep             ref405ep
> sam460ex             aCube Sam460ex
> taihu                taihu
> virtex-ml507         Xilinx Virtex ML507 reference design
> 
> I'm not able to personally validate all of these, but a quick check
> shows that at least both PReP machines (-M prep and -M 40p) are also
> broken by this patch which tells me it hasn't had much testing.
> 
> David, do you know what the defaults are for the other PPC machines in
> order for Sebastian to test and send a follow-up patch?

I don't know much how to validate these machines (I did a 'make check' 
but it does not show up any problems) and what impact these change has. 
But I suggested in the note section (which is not the commit message) 
that some of the machines properly behave different now (OTOH CirrusGD 
is a vga card nonetheless, I would have expected that at least the Mac 
can deal with that card). This can either be fixed by following the 
'spapr' approach (as it explicitly forbids this card as the primary 
display and was therefore visible immediately) or by generally 
rethinking whether CirrusGD really should be the preferred card over std 
vga (after all it is considered as obsolete so why should it be 
preferred?). I think it would be nice if a consensus can be found about 
the "correct approach" way to fix it. I personally think that adding 
more exceptions is not the right way :)

Last but not least, all of the targets should still work as before if 
you use -vga std option.

Bye
Sebastian

On 04/07/18 06:33, Sebastian Bauer wrote:

> Am 2018-07-04 06:50, schrieb Mark Cave-Ayland:
>>> Either to give up the vga cirrus preference or to apply the same as 
>>> was done with the spapr machine. I would prefer the first variant but 
>>> it would also cause some differences on other machines.
>>
>> I understand that the decision was taken to add VGA cirrus to the PPC
>> build and therefore accept the default VGA device change, however your
>> the commit message only mentions spapr and not all the other PPC
>> machines whose defaults are also potentially affected:
>>
>> $ ./qemu-system-ppc -M ?
>> Supported machines are:
>> 40p                  IBM RS/6000 7020 (40p)
>> bamboo               bamboo
>> g3beige              Heathrow based PowerMAC (default)
>> mac99                Mac99 based PowerMAC
>> mpc8544ds            mpc8544ds
>> none                 empty machine
>> ppce500              generic paravirt e500 platform
>> prep                 PowerPC PREP platform
>> ref405ep             ref405ep
>> sam460ex             aCube Sam460ex
>> taihu                taihu
>> virtex-ml507         Xilinx Virtex ML507 reference design
>>
>> I'm not able to personally validate all of these, but a quick check
>> shows that at least both PReP machines (-M prep and -M 40p) are also
>> broken by this patch which tells me it hasn't had much testing.
>>
>> David, do you know what the defaults are for the other PPC machines in
>> order for Sebastian to test and send a follow-up patch?
> 
> I don't know much how to validate these machines (I did a 'make check' 
> but it does not show up any problems) and what impact these change has. 
> But I suggested in the note section (which is not the commit message) 
> that some of the machines properly behave different now (OTOH CirrusGD 
> is a vga card nonetheless, I would have expected that at least the Mac 
> can deal with that card). This can either be fixed by following the 
> 'spapr' approach (as it explicitly forbids this card as the primary 
> display and was therefore visible immediately) or by generally 
> rethinking whether CirrusGD really should be the preferred card over std 
> vga (after all it is considered as obsolete so why should it be 
> preferred?). I think it would be nice if a consensus can be found about 
> the "correct approach" way to fix it. I personally think that adding 
> more exceptions is not the right way :)

Right, but as the patch submitter it's your responsibility to ensure 
that your patch doesn't break other people's machines and/or follow up 
with the appropriate patches. If you're not willing to do that then we 
should revert the patch in its current form until a better way forward 
has been found.

> Last but not least, all of the targets should still work as before if 
> you use -vga std option.

Except that -vga std has been the default for these machines for a long 
time, and it's going to be me that will get a large majority of the 
complaints if this behaviour changes.


ATB,

Mark.

Hi,

Am 2018-07-04 07:56, schrieb Mark Cave-Ayland:
> Right, but as the patch submitter it's your responsibility to ensure
> that your patch doesn't break other people's machines and/or follow up
> with the appropriate patches. If you're not willing to do that then we
> should revert the patch in its current form until a better way forward
> has been found.

Of course, I will come up with a follow up patch that will hopefully fix 
the problem (hopefully, as I simply have no clue how to test if some 
platforms break if it is not covered by 'make check', though I 
appreciate any hints on doing this as well). That's not the question. My 
question is what the scope of the patch actually should be. I mentioned 
two possible ways of proceeding.

>> Last but not least, all of the targets should still work as before if 
>> you use -vga std option.
> Except that -vga std has been the default for these machines for a
> long time, and it's going to be me that will get a large majority of
> the complaints if this behaviour changes.

I fully agree that -vga std makes the most sense, but why does QEMU 
prefer the Cirrus one over vga then when there seems to be some 
agreement that Cirrus is obsolete and many machines don't work with 
Cirrus?

My patch did not deliberately set a different -vga default, it is 
actually coded that way in QEMU and a side-effect of the inclusion: For 
any machine prefer the Cirrus card if it is available as default -vga. 
Should my patch address this (i.e., to lower the pri of the cirrus)?

Or should my patch add further exceptions to the respective machines?

It is also possible to do the latter now (as release is imminent) and 
schedule the former the next dev cycle.

Bye
Sebastian

On Wed, 4 Jul 2018, Sebastian Bauer wrote:
> Am 2018-07-04 06:50, schrieb Mark Cave-Ayland:
>>> Either to give up the vga cirrus preference or to apply the same as was 
>>> done with the spapr machine. I would prefer the first variant but it would 
>>> also cause some differences on other machines.
>> 
>> I understand that the decision was taken to add VGA cirrus to the PPC
>> build and therefore accept the default VGA device change, however your
>> the commit message only mentions spapr and not all the other PPC
>> machines whose defaults are also potentially affected:
>> 
>> $ ./qemu-system-ppc -M ?
>> Supported machines are:
>> 40p                  IBM RS/6000 7020 (40p)
>> bamboo               bamboo
>> g3beige              Heathrow based PowerMAC (default)
>> mac99                Mac99 based PowerMAC
>> mpc8544ds            mpc8544ds
>> none                 empty machine
>> ppce500              generic paravirt e500 platform
>> prep                 PowerPC PREP platform
>> ref405ep             ref405ep
>> sam460ex             aCube Sam460ex
>> taihu                taihu
>> virtex-ml507         Xilinx Virtex ML507 reference design
>> 
>> I'm not able to personally validate all of these, but a quick check
>> shows that at least both PReP machines (-M prep and -M 40p) are also
>> broken by this patch which tells me it hasn't had much testing.
>> 
>> David, do you know what the defaults are for the other PPC machines in
>> order for Sebastian to test and send a follow-up patch?
>
> I don't know much how to validate these machines (I did a 'make check' but it 
> does not show up any problems) and what impact these change has. But I 
> suggested in the note section (which is not the commit message) that some of 
> the machines properly behave different now (OTOH CirrusGD is a vga card 
> nonetheless, I would have expected that at least the Mac can deal with that 
> card). This can either be fixed by following the 'spapr' approach (as it 
> explicitly forbids this card as the primary display and was therefore visible 
> immediately) or by generally rethinking whether CirrusGD really should be the 
> preferred card over std vga (after all it is considered as obsolete so why 
> should it be preferred?). I think it would be nice if a consensus can be 
> found about the "correct approach" way to fix it. I personally think that 
> adding more exceptions is not the right way :)

Maybe including Gerd Hoffmann in this discussion would help finding a 
preferred approach.

> Last but not least, all of the targets should still work as before if you use 
> -vga std option.

That's not an acceptable solution because it wasn't needed before and now 
people who depended on that would need to change their ways. I think 
changing the default to std at least for ppc machines if that's possible 
before adding cirrus would be a good solution but I don't know the 
details. Although I think this patch is not critical so if it's not easy 
to fix now it could also be reverted and then we can address it in next 
devel cycle in a better way.

Regards,
BALATON Zoltan