[Qemu-devel] [PATCH 0/7] Resend of all outstanding ati-vga patches

[Qemu-devel] [PATCH 0/7] Resend of all outstanding ati-vga patches

[BALATON Zoltan]

Hello,

This is the latest version of all outstanding ati-vga patches
collected in one series. Some of these or previous versions of them
were sent individually before but these are now superceding all
previous patches that are not yet in master and this series is all
that should be needed.

This should fix color problems due to endianness with most drivers (if
such problems remain with some Linux drivers those should be cross
checked with real hardware), fixes running FCode ROM with appropriate
OpenBIOS patches for mac99 and includes fixes that will be needed to
get picture with MacOS 9 but that may also need VBlank interrupts
emulated so it won't boot yet.

Regards,
BALATON Zoltan

BALATON Zoltan (7):
  ati-vga: Add registers for getting apertures
  ati-vga: Add some register definitions for debugging
  ati-vga: Fix GPIO_MONID register write
  ati-vga: Fix cursor color with guest_hwcursor=true
  ati-vga: Fix hardware cursor image offset
  ati-vga: Attempt to handle CRTC offset not exact multiple of stride
  ati-vga: Add limited support for big endian frame buffer aperture

 hw/display/ati.c      | 68 +++++++++++++++++++++++++++++++++++++--------------
 hw/display/ati_dbg.c  |  9 +++++++
 hw/display/ati_int.h  |  1 +
 hw/display/ati_regs.h | 11 +++++++++
 4 files changed, 71 insertions(+), 18 deletions(-)

-- 
2.13.7

[Qemu-devel] [PATCH 3/7] ati-vga: Fix GPIO_MONID register write

[BALATON Zoltan]

Also update bitbang_i2c state when output bits are changed while
enable bits are set. This fixes EDID access by the ATI FCode ROM.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/display/ati.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/display/ati.c b/hw/display/ati.c
index c8fc62505b..699f38223b 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -564,12 +564,15 @@ static void ati_mm_write(void *opaque, hwaddr addr,
                                addr - GPIO_MONID, data, size);
             /*
              * Rage128p accesses DDC used to get EDID via these bits.
-             * Only touch i2c when write overlaps 3rd byte because some
-             * drivers access this reg via multiple partial writes and
-             * without this spurious bits would be sent.
+             * Because some drivers access this via multiple byte writes
+             * we have to be careful when we send bits to avoid spurious
+             * changes in bitbang_i2c state. So only do it when mask is set
+             * and either the enable bits are changed or output bits changed
+             * while enabled.
              */
             if ((s->regs.gpio_monid & BIT(25)) &&
-                addr <= GPIO_MONID + 2 && addr + size > GPIO_MONID + 2) {
+                ((addr <= GPIO_MONID + 2 && addr + size > GPIO_MONID + 2) ||
+                 (addr == GPIO_MONID && (s->regs.gpio_monid & 0x60000)))) {
                 s->regs.gpio_monid = ati_i2c(&s->bbi2c, s->regs.gpio_monid, 1);
             }
         }
-- 
2.13.7

[Qemu-devel] [PATCH 7/7] ati-vga: Add limited support for big endian frame buffer aperture

[BALATON Zoltan]

Set frame buffer endianness according to requested endianness for
frame buffer apertures. We set frame buffer to big endian if any of
the two apertures are set to big endian. Using different endianness
for the two apertures is not implemented. This fixes inverted colors
with MacOS and Xorg frame buffer driver but some Linux drivers may
have endianness issues even on real hardware so this may not fix all
cases. MorphOS uses aper0 in LE, Linux uses aper0 in BE and MacOS uses
aper1 in BE but not sure about others or if MacOS also may need aper0
in which case we'll need a more complex fix but MacOS has other
problems yet so for now this might work.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/display/ati.c      | 10 +++++++++-
 hw/display/ati_int.h  |  1 +
 hw/display/ati_regs.h |  2 ++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/display/ati.c b/hw/display/ati.c
index 0bfe73179d..a365e2455d 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -90,7 +90,9 @@ static void ati_vga_switch_mode(ATIVGAState *s)
             DPRINTF("Switching to %dx%d %d %d @ %x\n", h, v, stride, bpp, offs);
             vbe_ioport_write_index(&s->vga, 0, VBE_DISPI_INDEX_ENABLE);
             vbe_ioport_write_data(&s->vga, 0, VBE_DISPI_DISABLED);
-            s->vga.big_endian_fb = false;
+            s->vga.big_endian_fb = (s->regs.config_cntl & APER_0_ENDIAN ||
+                                    s->regs.config_cntl & APER_1_ENDIAN ?
+                                    true : false);
             /* reset VBE regs then set up mode */
             s->vga.vbe_regs[VBE_DISPI_INDEX_XRES] = h;
             s->vga.vbe_regs[VBE_DISPI_INDEX_YRES] = v;
@@ -310,6 +312,9 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, unsigned int size)
     case PALETTE_DATA:
         val = vga_ioport_read(&s->vga, VGA_PEL_D);
         break;
+    case CNFG_CNTL:
+        val = s->regs.config_cntl;
+        break;
     case CNFG_MEMSIZE:
         val = s->vga.vram_size;
         break;
@@ -604,6 +609,9 @@ static void ati_mm_write(void *opaque, hwaddr addr,
         data >>= 8;
         vga_ioport_write(&s->vga, VGA_PEL_D, data & 0xff);
         break;
+    case CNFG_CNTL:
+        s->regs.config_cntl = data;
+        break;
     case CRTC_H_TOTAL_DISP:
         s->regs.crtc_h_total_disp = data & 0x07ff07ff;
         break;
diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h
index 31a1927b3e..5b4d3be1e6 100644
--- a/hw/display/ati_int.h
+++ b/hw/display/ati_int.h
@@ -39,6 +39,7 @@ typedef struct ATIVGARegs {
     uint32_t gpio_vga_ddc;
     uint32_t gpio_dvi_ddc;
     uint32_t gpio_monid;
+    uint32_t config_cntl;
     uint32_t crtc_h_total_disp;
     uint32_t crtc_h_sync_strt_wid;
     uint32_t crtc_v_total_disp;
diff --git a/hw/display/ati_regs.h b/hw/display/ati_regs.h
index 91947ab1e7..02046e97c2 100644
--- a/hw/display/ati_regs.h
+++ b/hw/display/ati_regs.h
@@ -317,6 +317,8 @@
 #define X_MPLL_REF_DIV_MASK                     0x000000FF
 
 /* Config control values (CONFIG_CNTL) */
+#define APER_0_ENDIAN                           0x00000003
+#define APER_1_ENDIAN                           0x0000000c
 #define CFG_VGA_IO_DIS                          0x00000400
 
 /* CRTC control values (CRTC_GEN_CNTL) */
-- 
2.13.7

[Qemu-devel] [PATCH 1/7] ati-vga: Add registers for getting apertures

[BALATON Zoltan]

Some drivers (e.g. Linux radeon drm and MacOS) access these to find
apertures to access card. Try to implement these but not sure these
are correct yet.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/display/ati.c      | 15 +++++++++++++++
 hw/display/ati_dbg.c  |  5 +++++
 hw/display/ati_regs.h |  5 +++++
 3 files changed, 25 insertions(+)

diff --git a/hw/display/ati.c b/hw/display/ati.c
index a747c4cc98..c8fc62505b 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -307,6 +307,21 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, unsigned int size)
     case CNFG_MEMSIZE:
         val = s->vga.vram_size;
         break;
+    case CONFIG_APER_0_BASE:
+    case CONFIG_APER_1_BASE:
+        val = pci_default_read_config(&s->dev,
+                                      PCI_BASE_ADDRESS_0, size) & 0xfffffff0;
+        break;
+    case CONFIG_APER_SIZE:
+        val = s->vga.vram_size;
+        break;
+    case CONFIG_REG_1_BASE:
+        val = pci_default_read_config(&s->dev,
+                                      PCI_BASE_ADDRESS_2, size) & 0xfffffff0;
+        break;
+    case CONFIG_REG_APER_SIZE:
+        val = memory_region_size(&s->mm);
+        break;
     case MC_STATUS:
         val = 5;
         break;
diff --git a/hw/display/ati_dbg.c b/hw/display/ati_dbg.c
index 88b3a11315..cbc52025d0 100644
--- a/hw/display/ati_dbg.c
+++ b/hw/display/ati_dbg.c
@@ -28,6 +28,11 @@ static struct ati_regdesc ati_reg_names[] = {
     {"CNFG_CNTL", 0x00e0},
     {"GEN_RESET_CNTL", 0x00f0},
     {"CNFG_MEMSIZE", 0x00f8},
+    {"CONFIG_APER_0_BASE", 0x0100},
+    {"CONFIG_APER_1_BASE", 0x0104},
+    {"CONFIG_APER_SIZE", 0x0108},
+    {"CONFIG_REG_1_BASE", 0x010c},
+    {"CONFIG_REG_APER_SIZE", 0x0110},
     {"MEM_CNTL", 0x0140},
     {"MC_FB_LOCATION", 0x0148},
     {"MC_AGP_LOCATION", 0x014C},
diff --git a/hw/display/ati_regs.h b/hw/display/ati_regs.h
index d7155c93d5..81fb5302c0 100644
--- a/hw/display/ati_regs.h
+++ b/hw/display/ati_regs.h
@@ -46,6 +46,11 @@
 #define CNFG_CNTL                               0x00e0
 #define GEN_RESET_CNTL                          0x00f0
 #define CNFG_MEMSIZE                            0x00f8
+#define CONFIG_APER_0_BASE                      0x0100
+#define CONFIG_APER_1_BASE                      0x0104
+#define CONFIG_APER_SIZE                        0x0108
+#define CONFIG_REG_1_BASE                       0x010c
+#define CONFIG_REG_APER_SIZE                    0x0110
 #define MEM_CNTL                                0x0140
 #define MC_FB_LOCATION                          0x0148
 #define MC_AGP_LOCATION                         0x014C
-- 
2.13.7

[Qemu-devel] [PATCH 2/7] ati-vga: Add some register definitions for debugging

[BALATON Zoltan]

Add names for AMCGPIO regs to make it easier to identify these in
trace output. This is where rage128p has the DDC from the DVI port
among others but because we don't implement the flat panel controller
we don't want to connect an EDID here to make sure drivers use the VGA
output instead. But since these are often probed by drivers it helps
to see what happens by logging these registers by name.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/display/ati_dbg.c  | 4 ++++
 hw/display/ati_regs.h | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/hw/display/ati_dbg.c b/hw/display/ati_dbg.c
index cbc52025d0..7e59c41ac2 100644
--- a/hw/display/ati_dbg.c
+++ b/hw/display/ati_dbg.c
@@ -23,6 +23,10 @@ static struct ati_regdesc ati_reg_names[] = {
     {"GPIO_DVI_DDC", 0x0064},
     {"GPIO_MONID", 0x0068},
     {"I2C_CNTL_1", 0x0094},
+    {"AMCGPIO_MASK_MIR", 0x009c},
+    {"AMCGPIO_A_MIR", 0x00a0},
+    {"AMCGPIO_Y_MIR", 0x00a4},
+    {"AMCGPIO_EN_MIR", 0x00a8},
     {"PALETTE_INDEX", 0x00b0},
     {"PALETTE_DATA", 0x00b4},
     {"CNFG_CNTL", 0x00e0},
diff --git a/hw/display/ati_regs.h b/hw/display/ati_regs.h
index 81fb5302c0..91947ab1e7 100644
--- a/hw/display/ati_regs.h
+++ b/hw/display/ati_regs.h
@@ -41,6 +41,10 @@
 #define GPIO_DVI_DDC                            0x0064
 #define GPIO_MONID                              0x0068
 #define I2C_CNTL_1                              0x0094
+#define AMCGPIO_MASK_MIR                        0x009c
+#define AMCGPIO_A_MIR                           0x00a0
+#define AMCGPIO_Y_MIR                           0x00a4
+#define AMCGPIO_EN_MIR                          0x00a8
 #define PALETTE_INDEX                           0x00b0
 #define PALETTE_DATA                            0x00b4
 #define CNFG_CNTL                               0x00e0
-- 
2.13.7

[Qemu-devel] [PATCH 6/7] ati-vga: Attempt to handle CRTC offset not exact multiple of stride

[BALATON Zoltan]

MacOS uses non-0 offset so it needs this and the resulting
vbe_start_addr seems correct but picture is still broken with OpenBIOS
after FCode runs but that maybe due to firmware problems now. After
boot, picture is now correct.

It also occured to me that these CRTC regs are also present in VGA so
I wonder if they should be shared in case some drivers try to poke
them via VGA regs or these are a separate set of regs for extended
mode. Added a comment noting this but drivers I've tried so far
program the card accessing ati regs so I did not attempt to change it.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/display/ati.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/hw/display/ati.c b/hw/display/ati.c
index bbcdd6bc83..0bfe73179d 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -50,6 +50,7 @@ static void ati_vga_switch_mode(ATIVGAState *s)
         s->mode = EXT_MODE;
         if (s->regs.crtc_gen_cntl & CRTC2_EN) {
             /* CRT controller enabled, use CRTC values */
+            /* FIXME Should these be the same as VGA CRTC regs? */
             uint32_t offs = s->regs.crtc_offset & 0x07ffffff;
             int stride = (s->regs.crtc_pitch & 0x7ff) * 8;
             int bpp = 0;
@@ -101,16 +102,23 @@ static void ati_vga_switch_mode(ATIVGAState *s)
                 (s->regs.dac_cntl & DAC_8BIT_EN ? VBE_DISPI_8BIT_DAC : 0));
             /* now set offset and stride after enable as that resets these */
             if (stride) {
+                int bypp = DIV_ROUND_UP(bpp, BITS_PER_BYTE);
+
                 vbe_ioport_write_index(&s->vga, 0, VBE_DISPI_INDEX_VIRT_WIDTH);
                 vbe_ioport_write_data(&s->vga, 0, stride);
-                if (offs % stride == 0) {
-                    vbe_ioport_write_index(&s->vga, 0, VBE_DISPI_INDEX_Y_OFFSET);
-                    vbe_ioport_write_data(&s->vga, 0, offs / stride);
-                } else {
-                    /* FIXME what to do with this? */
-                    error_report("VGA offset is not multiple of pitch, "
-                                 "expect bad picture");
+                stride *= bypp;
+                if (offs % stride) {
+                    DPRINTF("CRTC offset is not multiple of pitch\n");
+                    vbe_ioport_write_index(&s->vga, 0,
+                                           VBE_DISPI_INDEX_X_OFFSET);
+                    vbe_ioport_write_data(&s->vga, 0, offs % stride / bypp);
                 }
+                vbe_ioport_write_index(&s->vga, 0, VBE_DISPI_INDEX_Y_OFFSET);
+                vbe_ioport_write_data(&s->vga, 0, offs / stride);
+                DPRINTF("VBE offset (%d,%d), vbe_start_addr=%x\n",
+                        s->vga.vbe_regs[VBE_DISPI_INDEX_X_OFFSET],
+                        s->vga.vbe_regs[VBE_DISPI_INDEX_Y_OFFSET],
+                        s->vga.vbe_start_addr);
             }
         }
     } else {
-- 
2.13.7

[Qemu-devel] [PATCH 5/7] ati-vga: Fix hardware cursor image offset

[BALATON Zoltan]

The crtc_offset is not needed, cur_offset is relative to the start of
vram not the start of displayed area. This fixes broken pointer image
with MacOS that uses non-0 crtc_offset.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/display/ati.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/display/ati.c b/hw/display/ati.c
index b849f5d510..bbcdd6bc83 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -132,9 +132,8 @@ static void ati_cursor_define(ATIVGAState *s)
         return; /* Do not update cursor if locked or rendered by guest */
     }
     /* FIXME handle cur_hv_offs correctly */
-    src = s->vga.vram_ptr + (s->regs.crtc_offset & 0x07ffffff) +
-          s->regs.cur_offset - (s->regs.cur_hv_offs >> 16) -
-          (s->regs.cur_hv_offs & 0xffff) * 16;
+    src = s->vga.vram_ptr + s->regs.cur_offset -
+          (s->regs.cur_hv_offs >> 16) - (s->regs.cur_hv_offs & 0xffff) * 16;
     for (i = 0; i < 64; i++) {
         for (j = 0; j < 8; j++, idx++) {
             data[idx] = src[i * 16 + j];
@@ -190,8 +189,7 @@ static void ati_cursor_draw_line(VGACommonState *vga, uint8_t *d, int scr_y)
         return;
     }
     /* FIXME handle cur_hv_offs correctly */
-    src = s->vga.vram_ptr + (s->regs.crtc_offset & 0x07ffffff) +
-          s->cursor_offset + (scr_y - vga->hw_cursor_y) * 16;
+    src = s->vga.vram_ptr + s->cursor_offset + (scr_y - vga->hw_cursor_y) * 16;
     dp = &dp[vga->hw_cursor_x];
     h = ((s->regs.crtc_h_total_disp >> 16) + 1) * 8;
     for (i = 0; i < 8; i++) {
-- 
2.13.7

[Qemu-devel] [PATCH 4/7] ati-vga: Fix cursor color with guest_hwcursor=true

[BALATON Zoltan]

Fixes: a38127414bd007c5b6ae64c664d9e8839393277e
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/display/ati.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/ati.c b/hw/display/ati.c
index 699f38223b..b849f5d510 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -207,7 +207,7 @@ static void ati_cursor_draw_line(VGACommonState *vga, uint8_t *d, int scr_y)
                 }
             } else {
                 color = (xbits & BIT(7) ? s->regs.cur_color1 :
-                                          s->regs.cur_color0) << 8 | 0xff;
+                                          s->regs.cur_color0) | 0xff000000;
             }
             if (vga->hw_cursor_x + i * 8 + j >= h) {
                 return; /* end of screen, don't span to next line */
-- 
2.13.7

Re: [Qemu-devel] [PATCH 4/7] ati-vga: Fix cursor color with guest_hwcursor=true

[Philippe Mathieu-Daudé]

On 8/11/19 11:14 PM, BALATON Zoltan wrote:
> Fixes: a38127414bd007c5b6ae64c664d9e8839393277e
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/display/ati.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/display/ati.c b/hw/display/ati.c
> index 699f38223b..b849f5d510 100644
> --- a/hw/display/ati.c
> +++ b/hw/display/ati.c
> @@ -207,7 +207,7 @@ static void ati_cursor_draw_line(VGACommonState *vga, uint8_t *d, int scr_y)
>                  }
>              } else {
>                  color = (xbits & BIT(7) ? s->regs.cur_color1 :
> -                                          s->regs.cur_color0) << 8 | 0xff;
> +                                          s->regs.cur_color0) | 0xff000000;
>              }
>              if (vga->hw_cursor_x + i * 8 + j >= h) {
>                  return; /* end of screen, don't span to next line */
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [Qemu-devel] [PATCH 4/7] ati-vga: Fix cursor color with guest_hwcursor=true

[BALATON Zoltan]

On Mon, 12 Aug 2019, Philippe Mathieu-Daudé wrote:
> On 8/11/19 11:14 PM, BALATON Zoltan wrote:
>> Fixes: a38127414bd007c5b6ae64c664d9e8839393277e
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>  hw/display/ati.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/display/ati.c b/hw/display/ati.c
>> index 699f38223b..b849f5d510 100644
>> --- a/hw/display/ati.c
>> +++ b/hw/display/ati.c
>> @@ -207,7 +207,7 @@ static void ati_cursor_draw_line(VGACommonState *vga, uint8_t *d, int scr_y)
>>                  }
>>              } else {
>>                  color = (xbits & BIT(7) ? s->regs.cur_color1 :
>> -                                          s->regs.cur_color0) << 8 | 0xff;
>> +                                          s->regs.cur_color0) | 0xff000000;
>>              }
>>              if (vga->hw_cursor_x + i * 8 + j >= h) {
>>                  return; /* end of screen, don't span to next line */
>>
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks. I've noticed that cursor color may still be wrong with MacOS that 
uses big endian frame buffer so maybe this will also need to take frame 
buffer endianness into account somehow but this patch corrects a 
difference between guest_hwcursor true and false values, reproducible with 
some Linux versions (as far as I remember). While the wrong cursor color 
with MacOS is now consistent after this patch with both guest_hwcursor 
true or false so that likely needs a different fix that should be applied 
both to this place and to ati_cursor_define. (MacOS does not yet boot 
fully, it needs patches to OpenBIOS to be able to run an FCode ROM and 
will probably need the VBlank interrupt implemented in ati-vga without 
which it displays a desktop but freezes there).

Regards,
BALATON Zoltan

Re: [Qemu-devel] [PATCH 4/7] ati-vga: Fix cursor color with guest_hwcursor=true

[Philippe Mathieu-Daudé]

On 8/12/19 12:28 PM, BALATON Zoltan wrote:
> On Mon, 12 Aug 2019, Philippe Mathieu-Daudé wrote:
>> On 8/11/19 11:14 PM, BALATON Zoltan wrote:
>>> Fixes: a38127414bd007c5b6ae64c664d9e8839393277e
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> ---
>>>  hw/display/ati.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/display/ati.c b/hw/display/ati.c
>>> index 699f38223b..b849f5d510 100644
>>> --- a/hw/display/ati.c
>>> +++ b/hw/display/ati.c
>>> @@ -207,7 +207,7 @@ static void ati_cursor_draw_line(VGACommonState
>>> *vga, uint8_t *d, int scr_y)
>>>                  }
>>>              } else {
>>>                  color = (xbits & BIT(7) ? s->regs.cur_color1 :
>>> -                                          s->regs.cur_color0) << 8 |
>>> 0xff;
>>> +                                          s->regs.cur_color0) |
>>> 0xff000000;
>>>              }
>>>              if (vga->hw_cursor_x + i * 8 + j >= h) {
>>>                  return; /* end of screen, don't span to next line */
>>>
>>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Thanks. I've noticed that cursor color may still be wrong with MacOS
> that uses big endian frame buffer so maybe this will also need to take
> frame buffer endianness into account somehow but this patch corrects a
> difference between guest_hwcursor true and false values, reproducible
> with some Linux versions (as far as I remember). While the wrong cursor
> color with MacOS is now consistent after this patch with both
> guest_hwcursor true or false so that likely needs a different fix that
> should be applied both to this place and to ati_cursor_define. (MacOS
> does not yet boot fully, it needs patches to OpenBIOS to be able to run
> an FCode ROM and will probably need the VBlank interrupt implemented in
> ati-vga without which it displays a desktop but freezes there).

If you remember which Linux version had this problem, or you can link to
roms that behave incorrectly, please share, so we can add display
regression tests.

Thanks,

Phil.

Re: [Qemu-devel] [PATCH 4/7] ati-vga: Fix cursor color with guest_hwcursor=true

[BALATON Zoltan]

On Mon, 12 Aug 2019, Philippe Mathieu-Daudé wrote:
> On 8/12/19 12:28 PM, BALATON Zoltan wrote:
>> On Mon, 12 Aug 2019, Philippe Mathieu-Daudé wrote:
>>> On 8/11/19 11:14 PM, BALATON Zoltan wrote:
>>>> Fixes: a38127414bd007c5b6ae64c664d9e8839393277e
>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>> ---
>>>> ?hw/display/ati.c | 2 +-
>>>> ?1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/display/ati.c b/hw/display/ati.c
>>>> index 699f38223b..b849f5d510 100644
>>>> --- a/hw/display/ati.c
>>>> +++ b/hw/display/ati.c
>>>> @@ -207,7 +207,7 @@ static void ati_cursor_draw_line(VGACommonState
>>>> *vga, uint8_t *d, int scr_y)
>>>> ???????????????? }
>>>> ???????????? } else {
>>>> ???????????????? color = (xbits & BIT(7) ? s->regs.cur_color1 :
>>>> -????????????????????????????????????????? s->regs.cur_color0) << 8 |
>>>> 0xff;
>>>> +????????????????????????????????????????? s->regs.cur_color0) |
>>>> 0xff000000;
>>>> ???????????? }
>>>> ???????????? if (vga->hw_cursor_x + i * 8 + j >= h) {
>>>> ???????????????? return; /* end of screen, don't span to next line */
>>>>
>>>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>
>> Thanks. I've noticed that cursor color may still be wrong with MacOS
>> that uses big endian frame buffer so maybe this will also need to take
>> frame buffer endianness into account somehow but this patch corrects a
>> difference between guest_hwcursor true and false values, reproducible
>> with some Linux versions (as far as I remember). While the wrong cursor
>> color with MacOS is now consistent after this patch with both
>> guest_hwcursor true or false so that likely needs a different fix that
>> should be applied both to this place and to ati_cursor_define. (MacOS
>> does not yet boot fully, it needs patches to OpenBIOS to be able to run
>> an FCode ROM and will probably need the VBlank interrupt implemented in
>> ati-vga without which it displays a desktop but freezes there).
>
> If you remember which Linux version had this problem, or you can link to
> roms that behave incorrectly, please share, so we can add display
> regression tests.

Unfortunately I don't. I think it was Andrew who found this so maybe he 
can remember.

(You may also need latest vgabios-ati.bin from Gerd's repo to get Linux 
drivers load and for rage128p that has to be in pc-bios dir because PCI 
IDs are only patched in that way.)

Regards,
BALATON Zoltan

Re: [Qemu-devel] [PATCH 4/7] ati-vga: Fix cursor color with guest_hwcursor=true

[Andrew Randrianasulu]

В сообщении от Monday 12 August 2019 13:55:45 BALATON Zoltan написал(а):
> On Mon, 12 Aug 2019, Philippe Mathieu-Daudé wrote:
> > On 8/12/19 12:28 PM, BALATON Zoltan wrote:
> >> On Mon, 12 Aug 2019, Philippe Mathieu-Daudé wrote:
> >>> On 8/11/19 11:14 PM, BALATON Zoltan wrote:
> >>>> Fixes: a38127414bd007c5b6ae64c664d9e8839393277e
> >>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> >>>> ---
> >>>> ?hw/display/ati.c | 2 +-
> >>>> ?1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/hw/display/ati.c b/hw/display/ati.c
> >>>> index 699f38223b..b849f5d510 100644
> >>>> --- a/hw/display/ati.c
> >>>> +++ b/hw/display/ati.c
> >>>> @@ -207,7 +207,7 @@ static void ati_cursor_draw_line(VGACommonState
> >>>> *vga, uint8_t *d, int scr_y)
> >>>> ???????????????? }
> >>>> ???????????? } else {
> >>>> ???????????????? color = (xbits & BIT(7) ? s->regs.cur_color1 :
> >>>> -????????????????????????????????????????? s->regs.cur_color0) << 8 |
> >>>> 0xff;
> >>>> +????????????????????????????????????????? s->regs.cur_color0) |
> >>>> 0xff000000;
> >>>> ???????????? }
> >>>> ???????????? if (vga->hw_cursor_x + i * 8 + j >= h) {
> >>>> ???????????????? return; /* end of screen, don't span to next line */
> >>>>
> >>>
> >>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >>
> >> Thanks. I've noticed that cursor color may still be wrong with MacOS
> >> that uses big endian frame buffer so maybe this will also need to take
> >> frame buffer endianness into account somehow but this patch corrects a
> >> difference between guest_hwcursor true and false values, reproducible
> >> with some Linux versions (as far as I remember). While the wrong cursor
> >> color with MacOS is now consistent after this patch with both
> >> guest_hwcursor true or false so that likely needs a different fix that
> >> should be applied both to this place and to ati_cursor_define. (MacOS
> >> does not yet boot fully, it needs patches to OpenBIOS to be able to run
> >> an FCode ROM and will probably need the VBlank interrupt implemented in
> >> ati-vga without which it displays a desktop but freezes there).
> >
> > If you remember which Linux version had this problem, or you can link to
> > roms that behave incorrectly, please share, so we can add display
> > regression tests.
> 
> Unfortunately I don't. I think it was Andrew who found this so maybe he 
> can remember.

Blue cursor was seen on specific dvd (x86) image I did for myself, 
because it was  using plain X cursor (arrow or X-shaped), not colored 
versions used by default in many distributions.

may be 'startx -- -retro" will show it briefly too?

from man Xserver (1.19.7):

-retro  starts  the  server  with  the  classic stipple and cursor visible.  The default is to start with a black root window, and to suppress display of the cursor until the
               first time an application calls XDefineCursor(). 


https://yadi.sk/d/F8cbINWzWJ-DkA

users: root and guest
passwords: toor and guest

You need to boot it to syslinux and type 'slax' there, because default will boot x86-64 kernel without aty128fb .... (my fault)
Unfortunately, i don't have fresh qemu compiled (previous tests were done from tmpfs copy of qemu source tree),
 will recompile from git and re-test. from memory, loading aty128fb and
setting config fragment in /etc/X11/xorg.conf.d for EXA AccelMethod and "Option "UseFBDev" "1" ' allowed device (ati-vga) to work.



> 
> (You may also need latest vgabios-ati.bin from Gerd's repo to get Linux 
> drivers load and for rage128p that has to be in pc-bios dir because PCI 
> IDs are only patched in that way.)
> 
> Regards,
> BALATON Zoltan

Re: [Qemu-devel] [PATCH 1/7] ati-vga: Add registers for getting apertures

[Gerd Hoffmann]

On Sun, Aug 11, 2019 at 11:14:53PM +0200, BALATON Zoltan wrote:
> Some drivers (e.g. Linux radeon drm and MacOS) access these to find
> apertures to access card. Try to implement these but not sure these
> are correct yet.

Queued up patch series.

thanks,
  Gerd