Re: [Qemu-devel] [PATCH] spapr: Add support for -vga option

[Anthony Liguori <anthony@codemonkey.ws>]

On 12/14/2011 03:22 AM, Benjamin Herrenschmidt wrote:
> The -vga option is a handy "high level" option for instanciating things
> automatically. This adds support for it to the spapr (aka pseries)
> machine.
>
> Also instanciate the USB keyboard and mouse when that option is used
> (you can still use -device to create individual devices without all
> the defaults)
>
> The default remains no VGA and no USB, and of course you can use -device
> to manually instanciate individual devices.
>
> Signed-off-by: Benjamin Herrenschmidt<benh@kernel.crashing.org>
> ---
>   Makefile.target |    2 +-
>   hw/spapr.c      |   47 ++++++++++++++++++++++++++++++++++++++++++++++-
>   2 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile.target b/Makefile.target
> index 03e3c55..c78fc25 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -243,7 +243,7 @@ obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
>
>   # shared objects
>   obj-ppc-y = ppc.o ppc_booke.o
> -obj-ppc-y += vga.o
> +obj-ppc-y += vga.o cirrus_vga.o
>   # PREP target
>   obj-ppc-y += mc146818rtc.o
>   obj-ppc-y += ppc_prep.o
> diff --git a/hw/spapr.c b/hw/spapr.c
> index c5353fb..3b9e576 100644
> --- a/hw/spapr.c
> +++ b/hw/spapr.c
> @@ -49,6 +49,7 @@
>   #include "pci.h"
>   #include "usb.h"
>   #include "usb-ohci.h"
> +#include "pc.h"
>
>   #include "exec-memory.h"
>
> @@ -76,6 +77,7 @@
>   #define PHANDLE_XICP            0x00001111
>
>   sPAPREnvironment *spapr;
> +static int spapr_has_graphics;
>
>   qemu_irq spapr_allocate_irq(uint32_t hint, uint32_t *irq_num)
>   {
> @@ -318,6 +320,9 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
>       _FDT((fdt_property(fdt, "linux,initrd-end",
>                          &end_prop, sizeof(end_prop))));
>       _FDT((fdt_property_string(fdt, "qemu,boot-device", boot_device)));
> +    _FDT((fdt_property_cell(fdt, "qemu,graphic-width", graphic_width)));
> +    _FDT((fdt_property_cell(fdt, "qemu,graphic-height", graphic_height)));
> +    _FDT((fdt_property_cell(fdt, "qemu,graphic-depth", graphic_depth)));
>
>       _FDT((fdt_end_node(fdt)));
>
> @@ -563,7 +568,9 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr,
>           }
>       }
>
> -    spapr_populate_chosen_stdout(fdt, spapr->vio_bus);
> +    if (!spapr_has_graphics) {
> +	spapr_populate_chosen_stdout(fdt, spapr->vio_bus);
> +    }
>
>       _FDT((fdt_pack(fdt)));
>
> @@ -603,6 +610,26 @@ static void spapr_reset(void *opaque)
>
>   }
>
> +static int spapr_vga_init(PCIBus *pci_bus)
> +{
> +    /* Default is nothing */
> +    if (cirrus_vga_enabled) {
> +        pci_cirrus_vga_init(pci_bus);
> +    } else if (vmsvga_enabled) {
> +	fprintf(stderr, "Warning: vmware_vga not available,"
> +		" using standard VGA instead\n");
> +	pci_vga_init(pci_bus);
> +#ifdef CONFIG_SPICE
> +    } else if (qxl_enabled) {
> +        pci_create_simple(pci_bus, -1, "qxl-vga");
> +#endif
> +    } else if (std_vga_enabled) {
> +        pci_vga_init(pci_bus);
> +    } else
> +       return 0;
> +    return 1;
> +}
> +
>   /* pSeries LPAR / sPAPR hardware init */
>   static void ppc_spapr_init(ram_addr_t ram_size,
>                              const char *boot_device,
> @@ -746,9 +773,27 @@ static void ppc_spapr_init(ram_addr_t ram_size,
>           spapr_vscsi_create(spapr->vio_bus, 0x2000 + i);
>       }
>
> +    /* Graphics */
> +    if (spapr_vga_init(QLIST_FIRST(&spapr->phbs)->host_state.bus)) {
> +	spapr_has_graphics = 1;
> +	usb_enabled = 1;
> +    }
> +
> +    /* Default depth is 15 which is bad, SLOF only supports 8 so it should
> +     * be the default (while we should still allow to specify 16 or 32)
> +     * so I'm cheating and converting 15 to 8 here. Ideally we should have
> +     * a way to set the defaults per machine.
> +     */
> +    if (graphic_depth == 15)
> +        graphic_depth = 8;
> +
>       /* USB */
>       if (usb_enabled) {
>           usb_ohci_init_pci(QLIST_FIRST(&spapr->phbs)->host_state.bus, -1);
> +	if (spapr_has_graphics) {
> +	    usbdevice_create("keyboard");
> +	    usbdevice_create("mouse");

It is better to use usb_create() and pass it the usb bus directly as this will 
make it easier to express this as composition down the road.

Regards,

Anthony Liguori

> +	}
>       }
>
>       if (kernel_filename) {
>
>
>