Re: [Qemu-trivial] [PATCH v3 37/42] hw/display/vga: extract public API from i386/pc to "hw/display/vga.h"

[Thomas Huth]

On 17.10.2017 18:44, Philippe Mathieu-Daudé wrote:
> and remove the old i386/pc dependency.
> 
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
>  include/hw/display/vga.h | 25 +++++++++++++++++++++++++
>  include/hw/i386/pc.h     | 12 ------------
>  hw/display/vga-isa-mm.c  |  3 +--
>  hw/display/vga-isa.c     |  2 +-
>  hw/display/vga.c         |  2 +-
>  hw/mips/mips_jazz.c      |  1 +
>  vl.c                     |  2 +-
>  MAINTAINERS              |  1 +
>  8 files changed, 31 insertions(+), 17 deletions(-)
>  create mode 100644 include/hw/display/vga.h
> 
> diff --git a/include/hw/display/vga.h b/include/hw/display/vga.h
> new file mode 100644
> index 0000000000..0401a3a292
> --- /dev/null
> +++ b/include/hw/display/vga.h
> @@ -0,0 +1,25 @@
> +/*
> + * QEMU VGA Emulator.
> + *
> + * Copyright (c) 2003 Fabrice Bellard
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +#ifndef QEMU_HW_DISPLAY_VGA_H
> +#define QEMU_HW_DISPLAY_VGA_H
> +
> +#include "exec/memory.h"
> +
> +enum vga_retrace_method {
> +    VGA_RETRACE_DUMB,
> +    VGA_RETRACE_PRECISE
> +};
> +
> +extern enum vga_retrace_method vga_retrace_method;
> +
> +int isa_vga_mm_init(hwaddr vram_base,
> +                    hwaddr ctrl_base, int it_shift,
> +                    MemoryRegion *address_space);
> +
> +#endif
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index b1b4c918af..25ba378bd8 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -302,18 +302,6 @@ PCIBus *find_i440fx(void);
>  extern PCIDevice *piix4_dev;
>  int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn);
>  
> -/* vga.c */
> -enum vga_retrace_method {
> -    VGA_RETRACE_DUMB,
> -    VGA_RETRACE_PRECISE
> -};
> -
> -extern enum vga_retrace_method vga_retrace_method;
> -
> -int isa_vga_mm_init(hwaddr vram_base,
> -                    hwaddr ctrl_base, int it_shift,
> -                    MemoryRegion *address_space);
> -
>  /* ne2000.c */
>  static inline bool isa_ne2000_init(ISABus *bus, int base, int irq, NICInfo 
> *nd)
>  {
> diff --git a/hw/display/vga-isa-mm.c b/hw/display/vga-isa-mm.c
> index 68c4d6e23e..e887b45651 100644
> --- a/hw/display/vga-isa-mm.c
> +++ b/hw/display/vga-isa-mm.c
> @@ -23,10 +23,9 @@
>   */
>  #include "qemu/osdep.h"
>  #include "hw/hw.h"
> -#include "hw/i386/pc.h"
> +#include "hw/display/vga.h"
>  #include "vga_int.h"
>  #include "ui/pixel_ops.h"
> -#include "qemu/timer.h"
>  
>  #define VGA_RAM_SIZE (8192 * 1024)
>  
> diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c
> index 26f69fd40a..469834add5 100644
> --- a/hw/display/vga-isa.c
> +++ b/hw/display/vga-isa.c
> @@ -25,7 +25,7 @@
>   */
>  #include "qemu/osdep.h"
>  #include "hw/hw.h"
> -#include "hw/i386/pc.h"
> +#include "hw/isa/isa.h"
>  #include "vga_int.h"
>  #include "ui/pixel_ops.h"
>  #include "qemu/timer.h"
> diff --git a/hw/display/vga.c b/hw/display/vga.c
> index 22fd069ad2..59729f7031 100644
> --- a/hw/display/vga.c
> +++ b/hw/display/vga.c
> @@ -24,7 +24,7 @@
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "hw/hw.h"
> -#include "hw/i386/pc.h"
> +#include "hw/display/vga.h"
>  #include "hw/pci/pci.h"
>  #include "vga_int.h"
>  #include "vga_regs.h"
> diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
> index b9ffb33b45..ece4a153b2 100644
> --- a/hw/mips/mips_jazz.c
> +++ b/hw/mips/mips_jazz.c
> @@ -39,6 +39,7 @@
>  #include "hw/loader.h"
>  #include "hw/timer/mc146818rtc.h"
>  #include "hw/timer/i8254.h"
> +#include "hw/display/vga.h"
>  #include "hw/audio/pcspk.h"
>  #include "sysemu/block-backend.h"
>  #include "hw/sysbus.h"
> diff --git a/vl.c b/vl.c
> index 0d1f612629..f0b7106feb 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -57,9 +57,9 @@ int main(int argc, char **argv)
>  #include "hw/boards.h"
>  #include "sysemu/accel.h"
>  #include "hw/usb.h"
> -#include "hw/i386/pc.h"
>  #include "hw/isa/isa.h"
>  #include "hw/scsi/scsi.h"
> +#include "hw/display/vga.h"
>  #include "hw/bt.h"
>  #include "sysemu/watchdog.h"
>  #include "hw/smbios/smbios.h"
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0e926a942b..81b67e4685 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -860,6 +860,7 @@ F: hw/timer/hpet*
>  F: hw/timer/i8254*
>  F: hw/timer/mc146818rtc*
>  F: hw/watchdog/wdt_ib700.c
> +F: include/hw/display/vga.h
>  F: include/hw/i2c/pm_smbus.h
>  F: include/hw/isa/i8257.h
>  F: include/hw/timer/hpet.h

Not sure whether this is the right section for vga.h ... especially
since the other hw/display/vga* files are also not listed in MAINTAINERS
yet?

Anyway, the other parts of this patch look fine to me, so with or
without the change to MAINTAINERS:

Reviewed-by: Thomas Huth <address@hidden>