Re: [Qemu-devel] [PATCH 16/18] console: stop using DisplayState in gfx hardware emulation

[Gerd Hoffmann <kraxel@redhat.com>]

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

On 03/25/13 14:56, Igor Mitsyanko wrote:
> On 03/25/2013 02:37 PM, Gerd Hoffmann wrote:
>>
>>>    Hi,
>>>
>>>  [5425.580115] displaysurface_create_from surface=0x7ff315d3df40,
>>>> 800x600, bpp 16, bswap 0 [5425.580257] displaysurface_free
>>>> surface=0x7ff3158c33b0
>>>>
>>>
>>> This is vga=0x314
>>>
>>> Looks like we have some funky interaction between vga and vmware.
>>>
>>> I'll go dig.  Meanwhile you can try vga=0x315 (800x600x24) or
>>> vga=normal (textmode), that has a high chance to workaround this.
>>>
>>> cheers,
>>>    Gerd
>>>
>>
>>
> 
>  Couldn't it be because wred, wgreen and wblue were removed? It seems like
> it was a workaround for some pre-existing problem, is it ok that you
> removed them but left depth and bypp intact?

No, it is not, and yes, this is where the inconsistency comes from.  We
read wred+wgreen+wblue directly from the surface whereas depth is cached
in the vmware vga state struct.  Patch attached.  Not fully tested yet.

I think the "pre-existing problem" is that the vmware vga has the bochs
dispi vbe interface enabled.  So the vga bios sets a video mode (via
bocks dispi interface) and the vmware svga emulation isn't notified ...

Making vmware vga emulation read stuff directly from the surface should
improve things a bit as the data returned to the guest reflects what the
actual gfx card state is, even when not set using the vmware vga
interface.  The change could still have unwanted side effects though.

I think the "real" fix is to disable bochs on vmware and hack seavgabios
to do modesetting using the vmware vga interfaces.

cheers,
  Gerd


[-- Attachment #2: 0001-vmware-remove-depth-bypp-from-state.patch --]
[-- Type: text/plain, Size: 3525 bytes --]

>From 266181a355ac8440d9143d254126244c611d1f67 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Mon, 25 Mar 2013 11:44:21 +0100
Subject: [PATCH] vmware: remove depth+bypp from state

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/vmware_vga.c |   24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index d317b4e..aa4d6f8 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -39,8 +39,7 @@ struct vmsvga_state_s {
     VGACommonState vga;
 
     int invalidated;
-    int depth;
-    int bypp;
+    int depth_vmstate_dummy;
     int enable;
     int config;
     struct {
@@ -749,11 +748,12 @@ static uint32_t vmsvga_value_read(void *opaque, uint32_t address)
         break;
 
     case SVGA_REG_DEPTH:
-        ret = s->depth;
+        ret = surface_bits_per_pixel(surface);
         break;
 
     case SVGA_REG_BITS_PER_PIXEL:
-        ret = (s->depth + 7) & ~7;
+    case SVGA_REG_HOST_BITS_PER_PIXEL:
+        ret = (surface_bits_per_pixel(surface) + 7) & ~7;
         break;
 
     case SVGA_REG_PSEUDOCOLOR:
@@ -773,7 +773,7 @@ static uint32_t vmsvga_value_read(void *opaque, uint32_t address)
         break;
 
     case SVGA_REG_BYTES_PER_LINE:
-        ret = s->bypp * s->new_width;
+        ret = surface_bytes_per_pixel(surface) * s->new_width;
         break;
 
     case SVGA_REG_FB_START: {
@@ -852,10 +852,6 @@ static uint32_t vmsvga_value_read(void *opaque, uint32_t address)
         ret = s->cursor.on;
         break;
 
-    case SVGA_REG_HOST_BITS_PER_PIXEL:
-        ret = (s->depth + 7) & ~7;
-        break;
-
     case SVGA_REG_SCRATCH_SIZE:
         ret = s->scratch_size;
         break;
@@ -885,6 +881,7 @@ static uint32_t vmsvga_value_read(void *opaque, uint32_t address)
 static void vmsvga_value_write(void *opaque, uint32_t address, uint32_t value)
 {
     struct vmsvga_state_s *s = opaque;
+    DisplaySurface *surface = qemu_console_surface(s->vga.con);
 
     trace_vmware_value_write(s->index, value);
     switch (s->index) {
@@ -924,7 +921,7 @@ static void vmsvga_value_write(void *opaque, uint32_t address, uint32_t value)
         break;
 
     case SVGA_REG_BITS_PER_PIXEL:
-        if (value != s->depth) {
+        if (value != surface_bits_per_pixel(surface)) {
             printf("%s: Bad bits per pixel: %i bits\n", __func__, value);
             s->config = 0;
         }
@@ -1125,7 +1122,7 @@ static const VMStateDescription vmstate_vmware_vga_internal = {
     .minimum_version_id_old = 0,
     .post_load = vmsvga_post_load,
     .fields      = (VMStateField[]) {
-        VMSTATE_INT32_EQUAL(depth, struct vmsvga_state_s),
+        VMSTATE_INT32_EQUAL(depth_vmstate_dummy, struct vmsvga_state_s),
         VMSTATE_INT32(enable, struct vmsvga_state_s),
         VMSTATE_INT32(config, struct vmsvga_state_s),
         VMSTATE_INT32(cursor.id, struct vmsvga_state_s),
@@ -1183,10 +1180,7 @@ 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);
-    /* Save some values here in case they are changed later.
-     * This is suspicious and needs more though why it is needed. */
-    s->depth = surface_bits_per_pixel(surface);
-    s->bypp = surface_bytes_per_pixel(surface);
+    s->depth_vmstate_dummy = surface_bits_per_pixel(surface);
 }
 
 static uint64_t vmsvga_io_read(void *opaque, hwaddr addr, unsigned size)
-- 
1.7.9.7