| Message ID | 5150B3F0.3080003@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, 25 Mar 2013, Gerd Hoffmann wrote: > 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. Tried that before and it didn't work that time (but maybe things have improved since). I could not find the "Problem" but you might look at http://lists.nongnu.org/archive/html/qemu-devel/2012-11/msg00512.html and commit 1f202568e0553b416483e5993f1bde219c22cf72 for the symptoms and why these were added back after being removed shortly. If these are not relevant any more just ignore this message (or use it to get rid of the dummy depth variable in your patch). Regards, BALATON Zoltan
On 2013-03-25 21:30, Gerd Hoffmann wrote: > 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. Unfortunately, this doesn't change the picture (except for the expected "vmsvga_value_read: Bad register 1c"). The 0x315 workaround does indeed work. Jan
Hi, >> 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. > > Unfortunately, this doesn't change the picture (except for the > expected "vmsvga_value_read: Bad register 1c"). The 0x315 > workaround does indeed work. Hmm, the patch fixes it for me (boot vesafb with 800x600 or 1024x768 @ 16bpp, Xorg starts successfully) ... Can I get a full X server log? cheers, Gerd
On 2013-04-03 13:50, Gerd Hoffmann wrote: > Hi, > >>> 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. >> >> Unfortunately, this doesn't change the picture (except for the >> expected "vmsvga_value_read: Bad register 1c"). The 0x315 >> workaround does indeed work. > > Hmm, the patch fixes it for me (boot vesafb with 800x600 or 1024x768 @ > 16bpp, Xorg starts successfully) ... > > Can I get a full X server log? Here is the one without the patch (it no longer applies). If you want me to rerun with that patch, please provide an update. Thanks, Jan
On 04/10/13 10:31, Jan Kiszka wrote: > On 2013-04-03 13:50, Gerd Hoffmann wrote: >> Hi, >> >>>> 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. >>> >>> Unfortunately, this doesn't change the picture (except for the >>> expected "vmsvga_value_read: Bad register 1c"). The 0x315 >>> workaround does indeed work. >> >> Hmm, the patch fixes it for me (boot vesafb with 800x600 or 1024x768 @ >> 16bpp, Xorg starts successfully) ... >> >> Can I get a full X server log? > > Here is the one without the patch (it no longer applies). If you want me > to rerun with that patch, please provide an update. Pushed latest pixman bits (including vmware fixes) to git://git.kraxel.org/qemu rebase/pixman Should be working fine again. cheers, Gerd
On 2013-04-16 09:42, Gerd Hoffmann wrote: > On 04/10/13 10:31, Jan Kiszka wrote: >> On 2013-04-03 13:50, Gerd Hoffmann wrote: >>> Hi, >>> >>>>> 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. >>>> >>>> Unfortunately, this doesn't change the picture (except for the >>>> expected "vmsvga_value_read: Bad register 1c"). The 0x315 >>>> workaround does indeed work. >>> >>> Hmm, the patch fixes it for me (boot vesafb with 800x600 or 1024x768 @ >>> 16bpp, Xorg starts successfully) ... >>> >>> Can I get a full X server log? >> >> Here is the one without the patch (it no longer applies). If you want me >> to rerun with that patch, please provide an update. > > Pushed latest pixman bits (including vmware fixes) to > git://git.kraxel.org/qemu rebase/pixman > > Should be working fine again. Yep, works again. Thanks, Jan
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)