[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/4] vga: improve VGA logic
From: |
Jan Kiszka |
Subject: |
Re: [Qemu-devel] [PATCH 1/4] vga: improve VGA logic |
Date: |
Fri, 13 Jan 2012 21:08:46 +0100 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
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 <address@hidden>
> ---
> 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 <address@hidden>
signature.asc
Description: OpenPGP digital signature