From: Sebastian Bauer <mail@sebastianbauer.info>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>,
Aleksandar Markovic <aleksandar.markovic@mips.com>,
"Michael S. Tsirkin" <mst@redhat.com>,
Richard Henderson <rth@twiddle.net>,
qemu-devel@nongnu.org, Alexander Graf <agraf@suse.de>,
qemu-ppc@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
Aurelien Jarno <aurelien@aurel32.net>,
David Gibson <david@gibson.dropbear.id.au>
Subject: Re: [Qemu-devel] [PATCH v2 2/2] vga: don't pick cirrus by default
Date: Wed, 11 Jul 2018 19:00:54 +0200 [thread overview]
Message-ID: <2b7f5bdfd86278524b4540f512eecbc7@sebastianbauer.info> (raw)
In-Reply-To: <20180711154850.GT7451@localhost.localdomain>
Am 2018-07-11 17:48, schrieb Eduardo Habkost:
> "none" looked like a false positive when I first looked, but now
> I think it's not. Shouldn't it set default_display="none"?
I think that there is some other logic burried that these machine
doesn't get a graphics display. But overall it is indeed not clearly
defined.
But see below.
>> If the patch is applied to 3.1 then I think there is enough time to
>> fix
>> issues caused by the patch. Additionally, a warning could be put in
>> the
>> ChangeLog for 3.0 that in 3.1 that the default mode will be std unless
>> machines define an own default. This is should be enough time for
>> people to
>> complain or to fix things.
> I don't think we will really make user-visible changes: we can
> simply work to keep existing behavior, but the difference is that
> this will be implemented by setting default_display explicitly on
> all machines.
Even if all machines were using explicit default settings the patch will
affect machines that are not inside the QEMU tree. If the patch is to be
applied as it is these are affected. To warn users (or devs in this
case) about this, an entry in the ChangeLog would be appropriate.
>> If the patch is to be applied to 3.0 then all non-ppc ones need to be
>> reconsidered.
>> The "important" ppc machines have been fixed already. I can do the
>> remaining
>> if this is wanted.
> This part worries me: do we have other machines that are broken
> right now?
I don't know which of them are broken or how this can be elaborated, but
they are potentially affected. For instance, the sam460ex platform
doesn't care about this setting, I can say that it is not broken. Other
platforms like the mac apparently were broken (and fixed in the
meantime). It is hard to tell which of them are really broken without
someone that knows the platform trying it and telling it. 'Make check'
did catch only one single case. It could also be that nobody cares about
other affected machines.
Overall I think the patch is an improvement over the previous state as
preferring the Cirrus doesn't make much sense if most machines don't
prefer it. The more I think over it, the more I think that the concept
needs further fine-tuning though (not necessarily in this patch).
If OTOH it would become a requirement for machines to set a default
display then this fallback logic could removed. Instead, qemu could
simply bailout on machines that define no default display (including
"none") and also bailout when the requested default display is not
available. This would be checkable by 'make check'. I still think that
the most common value can be a default (strictly bailing out when it is
not available unlike it is done now), but this is a matter of taste I
guess.
Bye
Sebastian
next prev parent reply other threads:[~2018-07-11 17:01 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-05 6:43 [Qemu-devel] [PATCH v2 0/2] vga: make stdvga the global default Gerd Hoffmann
2018-07-05 6:43 ` [Qemu-devel] [PATCH v2 1/2] vga: set default_display for all machines using cirrus Gerd Hoffmann
2018-07-05 16:35 ` Eduardo Habkost
2018-07-05 6:43 ` [Qemu-devel] [PATCH v2 2/2] vga: don't pick cirrus by default Gerd Hoffmann
2018-07-05 16:35 ` Eduardo Habkost
2018-07-06 6:53 ` Gerd Hoffmann
2018-07-09 21:08 ` Eduardo Habkost
2018-07-09 21:23 ` Eduardo Habkost
2018-07-09 22:26 ` Sebastian Bauer
2018-07-11 15:48 ` Eduardo Habkost
2018-07-11 17:00 ` Sebastian Bauer [this message]
2018-07-11 18:43 ` Eduardo Habkost
2018-07-12 4:57 ` [Qemu-devel] [Qemu-ppc] " Thomas Huth
2018-07-12 7:37 ` Sebastian Bauer
2018-07-12 17:27 ` Eduardo Habkost
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2b7f5bdfd86278524b4540f512eecbc7@sebastianbauer.info \
--to=mail@sebastianbauer.info \
--cc=agraf@suse.de \
--cc=aleksandar.markovic@mips.com \
--cc=aurelien@aurel32.net \
--cc=david@gibson.dropbear.id.au \
--cc=ehabkost@redhat.com \
--cc=kraxel@redhat.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=rth@twiddle.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.