Re: [RFC PATCH] cirrus-vga: Add property to set frame buffer endianness and make it default to little endian

[Fabiano Rosas <farosas@suse.de>]

BALATON Zoltan <balaton@eik.bme.hu> writes:

> I've tried this but it does not seem to work and leaves default to
> auto. Could somebody tell how this should work?
>

This patch sets the hw_compat to "off" so when new code migrates to/from
an old machine type, it will set the endianness to LE due to the
conditional you added.

  ./qemu-system-x86_64 -display none -nodefaults -vga cirrus -M pc-q35-10.2
  s->big_endian_fb: 2 --> s->vga.big_endian_fb: 0

overriding works:

  ./qemu-system-x86_64 -display none -nodefaults -vga cirrus -M pc-q35-10.2 -global cirrus-vga.x-big-endian-fb=on
  s->big_endian_fb: 1 --> s->vga.big_endian_fb: 1
  
  ./qemu-system-x86_64 -display none -nodefaults -vga cirrus -M pc-q35-10.2 -global cirrus-vga.x-big-endian-fb=off
  s->big_endian_fb: 2 --> s->vga.big_endian_fb: 0

The default value is a separate thing, this patch sets it to "auto" so
unless the hw_compat or the user overrides it, that's what you get.

  ./qemu-system-x86_64 -display none -nodefaults -vga cirrus
  s->big_endian_fb: 0
  (s->vga.big_endian_fb: 0, unchanged)
  
  ./qemu-system-x86_64 -display none -nodefaults -vga cirrus -global cirrus-vga.x-big-endian-fb=on
  s->big_endian_fb: 1 --> s->vga.big_endian_fb: 1
  
  ./qemu-system-x86_64 -display none -nodefaults -vga cirrus -global cirrus-vga.x-big-endian-fb=off
  s->big_endian_fb: 2 --> s->vga.big_endian_fb: 0

Maybe what you want is to say the old behavior is auto? Then set the
default to off.

> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/core/machine.c                | 2 ++
>  hw/display/cirrus_vga.c          | 6 ++++++
>  hw/display/cirrus_vga_internal.h | 2 ++
>  hw/display/cirrus_vga_isa.c      | 2 ++
>  4 files changed, 12 insertions(+)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 0aa77a57e9..e573ef46bc 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -41,6 +41,8 @@
>  GlobalProperty hw_compat_10_2[] = {
>      { "scsi-block", "migrate-pr", "off" },
>      { "isa-cirrus-vga", "global-vmstate", "true" },
> +    { "isa-cirrus-vga", "x-big-endian-fb", "off" },
> +    { "cirrus-vga", "x-big-endian-fb", "off" },
>  };
>  const size_t hw_compat_10_2_len = G_N_ELEMENTS(hw_compat_10_2);
>  
> diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
> index 629b34fc68..e379f125ae 100644
> --- a/hw/display/cirrus_vga.c
> +++ b/hw/display/cirrus_vga.c
> @@ -2930,6 +2930,10 @@ void cirrus_init_common(CirrusVGAState *s, Object *owner,
>      s->vga.cursor_invalidate = cirrus_cursor_invalidate;
>      s->vga.cursor_draw_line = cirrus_cursor_draw_line;
>  
> +    if (s->big_endian_fb != ON_OFF_AUTO_AUTO) {
> +        s->vga.big_endian_fb = (s->big_endian_fb == ON_OFF_AUTO_ON);
> +    }
> +
>      qemu_register_reset(cirrus_reset, s);
>  }
>  
> @@ -2987,6 +2991,8 @@ static const Property pci_vga_cirrus_properties[] = {
>                         cirrus_vga.vga.vram_size_mb, 4),
>      DEFINE_PROP_BOOL("blitter", struct PCICirrusVGAState,
>                       cirrus_vga.enable_blitter, true),
> +    DEFINE_PROP_ON_OFF_AUTO("x-big-endian-fb", struct PCICirrusVGAState,
> +                            cirrus_vga.big_endian_fb, ON_OFF_AUTO_AUTO),
>  };
>  
>  static void cirrus_vga_class_init(ObjectClass *klass, const void *data)
> diff --git a/hw/display/cirrus_vga_internal.h b/hw/display/cirrus_vga_internal.h
> index a78ebbd920..7c07201f18 100644
> --- a/hw/display/cirrus_vga_internal.h
> +++ b/hw/display/cirrus_vga_internal.h
> @@ -26,6 +26,7 @@
>  #ifndef CIRRUS_VGA_INTERNAL_H
>  #define CIRRUS_VGA_INTERNAL_H
>  
> +#include "hw/core/qdev-properties.h"
>  #include "vga_int.h"
>  
>  /* IDs */
> @@ -94,6 +95,7 @@ typedef struct CirrusVGAState {
>      int real_vram_size; /* XXX: suppress that */
>      int device_id;
>      int bustype;
> +    OnOffAuto big_endian_fb;
>  } CirrusVGAState;
>  
>  void cirrus_init_common(CirrusVGAState *s, Object *owner,
> diff --git a/hw/display/cirrus_vga_isa.c b/hw/display/cirrus_vga_isa.c
> index 76034a8860..c2b0b61476 100644
> --- a/hw/display/cirrus_vga_isa.c
> +++ b/hw/display/cirrus_vga_isa.c
> @@ -75,6 +75,8 @@ static const Property isa_cirrus_vga_properties[] = {
>                       cirrus_vga.enable_blitter, true),
>      DEFINE_PROP_BOOL("global-vmstate", struct ISACirrusVGAState,
>                       cirrus_vga.vga.global_vmstate, false),
> +    DEFINE_PROP_ON_OFF_AUTO("x-big-endian-fb", struct PCICirrusVGAState,

s/PCICirrusVGAState/ISACirrusVGAState/

> +                            cirrus_vga.big_endian_fb, ON_OFF_AUTO_AUTO),
>  };
>  
>  static void isa_cirrus_vga_class_init(ObjectClass *klass, const void
>  *data)