From: Jan Kiszka <jan.kiszka@web.de>
To: Blue Swirl <blauwirbel@gmail.com>
Cc: qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [PATCH 1/4] vga: improve VGA logic
Date: Fri, 13 Jan 2012 21:08:46 +0100 [thread overview]
Message-ID: <4F108F4E.3050101@web.de> (raw)
In-Reply-To: <CAAu8pHvGdMeC4Kw-Eqex7FjjaPR=FaGJEEmSuoL7KLd4-vr75g@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 12528 bytes --]
On 2012-01-08 22:09, Blue Swirl wrote:
> Improve VGA selection logic, push check for device availabilty to vl.c.
> Create the devices at board level unconditionally.
>
> Remove now unused pci_try_create*() functions.
>
> Make PCI VGA devices optional.
>
> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
> ---
> hw/alpha_pci.c | 6 +---
> hw/boards.h | 1 -
> hw/mips_malta.c | 6 +----
> hw/pc.c | 5 ----
> hw/pci.c | 20 -----------------
> hw/pci.h | 4 ---
> hw/qdev.c | 4 +++
> hw/qdev.h | 1 +
> hw/s390-virtio.c | 1 -
> hw/spapr.c | 1 -
> hw/vmware_vga.h | 8 +-----
> vl.c | 62 +++++++++++++++++++++++++++++++++++++++--------------
> 12 files changed, 55 insertions(+), 64 deletions(-)
>
> diff --git a/hw/alpha_pci.c b/hw/alpha_pci.c
> index e975702..6735577 100644
> --- a/hw/alpha_pci.c
> +++ b/hw/alpha_pci.c
> @@ -121,10 +121,8 @@ void alpha_pci_vga_setup(PCIBus *pci_bus)
> pci_cirrus_vga_init(pci_bus);
> return;
> case VGA_VMWARE:
> - if (pci_vmsvga_init(pci_bus)) {
> - return;
> - }
> - break;
> + pci_vmsvga_init(pci_bus);
> + return;
> }
> /* If VGA is enabled at all, and one of the above didn't work, then
> fallback to Standard VGA. */
> diff --git a/hw/boards.h b/hw/boards.h
> index 716fd7b..f6d3784 100644
> --- a/hw/boards.h
> +++ b/hw/boards.h
> @@ -22,7 +22,6 @@ typedef struct QEMUMachine {
> unsigned int no_serial:1,
> no_parallel:1,
> use_virtcon:1,
> - no_vga:1,
> no_floppy:1,
> no_cdrom:1,
> no_sdcard:1;
> diff --git a/hw/mips_malta.c b/hw/mips_malta.c
> index e625ec3..4a4f76d 100644
> --- a/hw/mips_malta.c
> +++ b/hw/mips_malta.c
> @@ -996,11 +996,7 @@ void mips_malta_init (ram_addr_t ram_size,
> if (cirrus_vga_enabled) {
> pci_cirrus_vga_init(pci_bus);
> } else if (vmsvga_enabled) {
> - if (!pci_vmsvga_init(pci_bus)) {
> - fprintf(stderr, "Warning: vmware_vga not available,"
> - " using standard VGA instead\n");
> - pci_vga_init(pci_bus);
> - }
> + pci_vmsvga_init(pci_bus);
> } else if (std_vga_enabled) {
> pci_vga_init(pci_bus);
> }
> diff --git a/hw/pc.c b/hw/pc.c
> index 85304cf..8cb78d9 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -1085,11 +1085,6 @@ DeviceState *pc_vga_init(ISABus *isa_bus,
> PCIBus *pci_bus)
> } else if (vmsvga_enabled) {
> if (pci_bus) {
> dev = pci_vmsvga_init(pci_bus);
> - if (!dev) {
> - fprintf(stderr, "Warning: vmware_vga not available,"
> - " using standard VGA instead\n");
> - dev = pci_vga_init(pci_bus);
> - }
> } else {
> fprintf(stderr, "%s: vmware_vga: no PCI bus\n", __FUNCTION__);
> }
> diff --git a/hw/pci.c b/hw/pci.c
> index c3082bc..54400ac 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1572,21 +1572,6 @@ PCIDevice *pci_create_multifunction(PCIBus
> *bus, int devfn, bool multifunction,
> return DO_UPCAST(PCIDevice, qdev, dev);
> }
>
> -PCIDevice *pci_try_create_multifunction(PCIBus *bus, int devfn,
> - bool multifunction,
> - const char *name)
> -{
> - DeviceState *dev;
> -
> - dev = qdev_try_create(&bus->qbus, name);
> - if (!dev) {
> - return NULL;
> - }
> - qdev_prop_set_uint32(dev, "addr", devfn);
> - qdev_prop_set_bit(dev, "multifunction", multifunction);
> - return DO_UPCAST(PCIDevice, qdev, dev);
> -}
> -
> PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
> bool multifunction,
> const char *name)
> @@ -1606,11 +1591,6 @@ PCIDevice *pci_create_simple(PCIBus *bus, int
> devfn, const char *name)
> return pci_create_simple_multifunction(bus, devfn, false, name);
> }
>
> -PCIDevice *pci_try_create(PCIBus *bus, int devfn, const char *name)
> -{
> - return pci_try_create_multifunction(bus, devfn, false, name);
> -}
> -
> static int pci_find_space(PCIDevice *pdev, uint8_t size)
> {
> int config_size = pci_config_size(pdev);
> diff --git a/hw/pci.h b/hw/pci.h
> index 625e717..160ba93 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -469,12 +469,8 @@ PCIDevice *pci_create_multifunction(PCIBus *bus,
> int devfn, bool multifunction,
> PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
> bool multifunction,
> const char *name);
> -PCIDevice *pci_try_create_multifunction(PCIBus *bus, int devfn,
> - bool multifunction,
> - const char *name);
> PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name);
> PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name);
> -PCIDevice *pci_try_create(PCIBus *bus, int devfn, const char *name);
>
> static inline int pci_is_express(const PCIDevice *d)
> {
> diff --git a/hw/qdev.c b/hw/qdev.c
> index d0cf66d..c424b46 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -80,6 +80,10 @@ static DeviceInfo *qdev_find_info(BusInfo
> *bus_info, const char *name)
> return NULL;
> }
>
> +bool qdev_exists(const char *name)
> +{
> + return !!qdev_find_info(NULL, name);
> +}
> static void qdev_property_add_legacy(DeviceState *dev, Property *prop,
> Error **errp);
>
> diff --git a/hw/qdev.h b/hw/qdev.h
> index 2abb767..6b58dd8 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -179,6 +179,7 @@ typedef struct GlobalProperty {
>
> DeviceState *qdev_create(BusState *bus, const char *name);
> DeviceState *qdev_try_create(BusState *bus, const char *name);
> +bool qdev_exists(const char *name);
> int qdev_device_help(QemuOpts *opts);
> DeviceState *qdev_device_add(QemuOpts *opts);
> int qdev_init(DeviceState *dev) QEMU_WARN_UNUSED_RESULT;
> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
> index 2210b8a..51123a7 100644
> --- a/hw/s390-virtio.c
> +++ b/hw/s390-virtio.c
> @@ -317,7 +317,6 @@ static QEMUMachine s390_machine = {
> .no_serial = 1,
> .no_parallel = 1,
> .use_virtcon = 1,
> - .no_vga = 1,
> .max_cpus = 255,
> .is_default = 1,
> };
> diff --git a/hw/spapr.c b/hw/spapr.c
> index 0e1f80d..906d2e9 100644
> --- a/hw/spapr.c
> +++ b/hw/spapr.c
> @@ -693,7 +693,6 @@ static QEMUMachine spapr_machine = {
> .desc = "pSeries Logical Partition (PAPR compliant)",
> .init = ppc_spapr_init,
> .max_cpus = MAX_CPUS,
> - .no_vga = 1,
> .no_parallel = 1,
> .use_scsi = 1,
> };
> diff --git a/hw/vmware_vga.h b/hw/vmware_vga.h
> index db11cbf..000fbdd 100644
> --- a/hw/vmware_vga.h
> +++ b/hw/vmware_vga.h
> @@ -8,12 +8,8 @@ static inline DeviceState *pci_vmsvga_init(PCIBus *bus)
> {
> PCIDevice *dev;
>
> - dev = pci_try_create(bus, -1, "vmware-svga");
> - if (!dev || qdev_init(&dev->qdev) < 0) {
> - return NULL;
> - } else {
> - return &dev->qdev;
> - }
> + dev = pci_create_simple(bus, -1, "vmware-svga");
> + return &dev->qdev;
> }
>
> #endif
> diff --git a/vl.c b/vl.c
> index ba55b35..d5868b1 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -271,7 +271,6 @@ static int default_serial = 1;
> static int default_parallel = 1;
> static int default_virtcon = 1;
> static int default_monitor = 1;
> -static int default_vga = 1;
> static int default_floppy = 1;
> static int default_cdrom = 1;
> static int default_sdcard = 1;
> @@ -290,11 +289,6 @@ static struct {
> { .driver = "virtio-serial-pci", .flag = &default_virtcon },
> { .driver = "virtio-serial-s390", .flag = &default_virtcon },
> { .driver = "virtio-serial", .flag = &default_virtcon },
> - { .driver = "VGA", .flag = &default_vga },
> - { .driver = "cirrus-vga", .flag = &default_vga },
> - { .driver = "vmware-svga", .flag = &default_vga },
> - { .driver = "isa-vga", .flag = &default_vga },
> - { .driver = "qxl-vga", .flag = &default_vga },
> };
>
> static void res_free(void)
> @@ -1525,18 +1519,48 @@ static const QEMUOption qemu_options[] = {
> #include "qemu-options-wrapper.h"
> { NULL },
> };
> +
> +static bool vga_available(void)
> +{
> + return qdev_exists("VGA") || qdev_exists("isa-vga");
> +}
> +
> +static bool cirrus_vga_available(void)
> +{
> + return qdev_exists("cirrus-vga") || qdev_exists("isa-cirrus-vga");
> +}
> +
> +static bool vmware_vga_available(void)
> +{
> + return qdev_exists("vmware-svga");
> +}
> +
> static void select_vgahw (const char *p)
> {
> const char *opts;
>
> - default_vga = 0;
> vga_interface_type = VGA_NONE;
> if (strstart(p, "std", &opts)) {
> - vga_interface_type = VGA_STD;
> + if (vga_available()) {
> + vga_interface_type = VGA_STD;
> + } else {
> + fprintf(stderr, "Error: standard VGA not available\n");
> + exit(0);
> + }
> } else if (strstart(p, "cirrus", &opts)) {
> - vga_interface_type = VGA_CIRRUS;
> + if (cirrus_vga_available()) {
> + vga_interface_type = VGA_CIRRUS;
> + } else {
> + fprintf(stderr, "Error: Cirrus VGA not available\n");
> + exit(0);
> + }
> } else if (strstart(p, "vmware", &opts)) {
> - vga_interface_type = VGA_VMWARE;
> + if (vmware_vga_available()) {
> + vga_interface_type = VGA_VMWARE;
> + } else {
> + fprintf(stderr, "Error: VMWare SVGA not available\n");
> + exit(0);
> + }
> } else if (strstart(p, "xenfb", &opts)) {
> vga_interface_type = VGA_XENFB;
> } else if (strstart(p, "qxl", &opts)) {
> @@ -2155,6 +2179,7 @@ int main(int argc, char **argv, char **envp)
> const char *loadvm = NULL;
> QEMUMachine *machine;
> const char *cpu_model;
> + const char *vga_model = NULL;
> const char *pid_file = NULL;
> const char *incoming = NULL;
> #ifdef CONFIG_VNC
> @@ -2581,7 +2606,7 @@ int main(int argc, char **argv, char **envp)
> rtc_utc = 0;
> break;
> case QEMU_OPTION_vga:
> - select_vgahw (optarg);
> + vga_model = optarg;
> break;
> case QEMU_OPTION_g:
> {
> @@ -2978,7 +3003,6 @@ int main(int argc, char **argv, char **envp)
> default_parallel = 0;
> default_virtcon = 0;
> default_monitor = 0;
> - default_vga = 0;
> default_net = 0;
> default_floppy = 0;
> default_cdrom = 0;
> @@ -3140,9 +3164,6 @@ int main(int argc, char **argv, char **envp)
> if (!machine->use_virtcon) {
> default_virtcon = 0;
> }
> - if (machine->no_vga) {
> - default_vga = 0;
> - }
> if (machine->no_floppy) {
> default_floppy = 0;
> }
> @@ -3178,8 +3199,6 @@ int main(int argc, char **argv, char **envp)
> if (default_virtcon)
> add_device_config(DEV_VIRTCON, "vc:80Cx24C");
> }
> - if (default_vga)
> - vga_interface_type = VGA_CIRRUS;
>
> socket_init();
>
> @@ -3330,6 +3349,15 @@ int main(int argc, char **argv, char **envp)
>
> module_call_init(MODULE_INIT_DEVICE);
>
> + /* must be after qdev registration but before machine init */
> + if (vga_model) {
> + select_vgahw(vga_model);
> + } else if (cirrus_vga_available()) {
> + select_vgahw("cirrus");
> + } else {
> + select_vgahw("none");
> + }
> +
> if (qemu_opts_foreach(qemu_find_opts("device"), device_help_func,
> NULL, 0) != 0)
> exit(0);
>
Looks good to me.
Reviewed-by: Jan Kiszka <jan.kiszka@siemens.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
next prev parent reply other threads:[~2012-01-13 20:08 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-01-08 21:09 [Qemu-devel] [PATCH 1/4] vga: improve VGA logic Blue Swirl
2012-01-13 20:08 ` Jan Kiszka [this message]
2012-01-24 15:57 ` Markus Armbruster
2012-01-24 17:29 ` Blue Swirl
2012-01-24 18:29 ` Markus Armbruster
2012-01-25 18:01 ` Blue Swirl
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=4F108F4E.3050101@web.de \
--to=jan.kiszka@web.de \
--cc=blauwirbel@gmail.com \
--cc=qemu-devel@nongnu.org \
/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.