Re: [Qemu-devel] [PATCH 2/4 v5] vmware_vga: Remove duplicated info from local state

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 2910 bytes --]

On 2012-11-03 11:58, BALATON Zoltan wrote:
> Removed info from vmsvga_state that is available from elsewhere and
> thus was duplicated here unnecessarily.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  console.h       |   20 +++++++
>  hw/vmware_vga.c |  159 +++++++++++++++++++++++--------------------------------
>  2 files changed, 85 insertions(+), 94 deletions(-)
> 
>  v5: rebased again
> 
> diff --git a/console.h b/console.h
> index 33ad69b..df33e46 100644
> --- a/console.h
> +++ b/console.h
> @@ -377,6 +377,26 @@ static inline pixman_format_code_t ds_get_format(DisplayState *ds)
>      return ds->surface->format;
>  }
>  
> +static inline int ds_get_depth(DisplayState *ds)
> +{
> +    return ds->surface->pf.depth;

This returns the current depth...

> +}
> +
> +static inline int ds_get_rmask(DisplayState *ds)
> +{
> +    return ds->surface->pf.rmask;
> +}
> +
> +static inline int ds_get_gmask(DisplayState *ds)
> +{
> +    return ds->surface->pf.gmask;
> +}
> +
> +static inline int ds_get_bmask(DisplayState *ds)
> +{
> +    return ds->surface->pf.bmask;
> +}
> +
>  #ifdef CONFIG_CURSES
>  #include <curses.h>
>  typedef chtype console_ch_t;
> diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
> index bf14e78..240443b 100644
> --- a/hw/vmware_vga.c
> +++ b/hw/vmware_vga.c

...

> @@ -730,23 +726,25 @@ static uint32_t vmsvga_value_read(void *opaque, uint32_t address)
>          return SVGA_MAX_HEIGHT;
>  
>      case SVGA_REG_DEPTH:
> -        return s->depth;
> +        return ds_get_depth(s->vga.ds);

...while we used to return a cached value here

>  
>      case SVGA_REG_BITS_PER_PIXEL:
> -        return (s->depth + 7) & ~7;
> +        return ds_get_bits_per_pixel(s->vga.ds);
>  
>      case SVGA_REG_PSEUDOCOLOR:
>          return 0x0;
>  
>      case SVGA_REG_RED_MASK:
> -        return s->wred;
> +        return ds_get_rmask(s->vga.ds);
> +
>      case SVGA_REG_GREEN_MASK:
> -        return s->wgreen;
> +        return ds_get_gmask(s->vga.ds);
> +
>      case SVGA_REG_BLUE_MASK:
> -        return s->wblue;
> +        return ds_get_bmask(s->vga.ds);
>  
>      case SVGA_REG_BYTES_PER_LINE:
> -        return ((s->depth + 7) >> 3) * s->new_width;
> +        return ds_get_bytes_per_pixel(s->vga.ds) * s->new_width;
>  
>      case SVGA_REG_FB_START: {
>          struct pci_vmsvga_state_s *pci_vmsvga

...

> @@ -1125,40 +1130,9 @@ static void vmsvga_init(struct vmsvga_state_s *s,
>      vga_common_init(&s->vga);
>      vga_init(&s->vga, address_space, io, true);
>      vmstate_register(NULL, 0, &vmstate_vga_common, &s->vga);
> -
> -    s->depth = ds_get_bits_per_pixel(s->vga.ds);

...sampled once during init.

I cannot comment on why it was done like this, just that this patch
breaks the Linux vmware X driver. Please fix or revert.

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]