[PATCH 0/3] Misc ati-vga patches

[PATCH 0/3] Misc ati-vga patches

[BALATON Zoltan]

Some misc patches I had laying around that could be upstreamed just to
clean up my tree a bit.

BALATON Zoltan (3):
  ati-vga: Fix aperture sizes
  ati-vga: Support unaligned access to GPIO DDC registers
  ati-vga: Add 30 bit palette access register

 hw/display/ati.c      | 50 ++++++++++++++++++++++++++++---------------
 hw/display/ati_dbg.c  |  1 +
 hw/display/ati_int.h  |  1 +
 hw/display/ati_regs.h |  1 +
 4 files changed, 36 insertions(+), 17 deletions(-)

-- 
2.30.9

[PATCH 1/3] ati-vga: Fix aperture sizes

[BALATON Zoltan]

Apparently these should be half the memory region sizes confirmed at
least by Radeon drivers while Rage 128 Pro drivers don't seem to use
these.

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

diff --git a/hw/display/ati.c b/hw/display/ati.c
index c36282c343..f0bf1d7493 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -349,14 +349,14 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, unsigned int size)
                                       PCI_BASE_ADDRESS_0, size) & 0xfffffff0;
         break;
     case CONFIG_APER_SIZE:
-        val = s->vga.vram_size;
+        val = s->vga.vram_size / 2;
         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);
+        val = memory_region_size(&s->mm) / 2;
         break;
     case MC_STATUS:
         val = 5;
-- 
2.30.9

[PATCH 2/3] ati-vga: Support unaligned access to GPIO DDC registers

[BALATON Zoltan]

The GPIO_VGA_DDC and GPIO_DVI_DDC registers are used on Radeon for DDC
access. Some drivers like the PPC Mac FCode ROM uses unaligned writes
to these registers so implement this the same way as already done for
GPIO_MONID which is used the same way for the Rage 128 Pro.

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

diff --git a/hw/display/ati.c b/hw/display/ati.c
index f0bf1d7493..ce63935ead 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -319,11 +319,13 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, unsigned int size)
     case DAC_CNTL:
         val = s->regs.dac_cntl;
         break;
-    case GPIO_VGA_DDC:
-        val = s->regs.gpio_vga_ddc;
+    case GPIO_VGA_DDC ... GPIO_VGA_DDC + 3:
+        val = ati_reg_read_offs(s->regs.gpio_vga_ddc,
+                                addr - GPIO_VGA_DDC, size);
         break;
-    case GPIO_DVI_DDC:
-        val = s->regs.gpio_dvi_ddc;
+    case GPIO_DVI_DDC ... GPIO_DVI_DDC + 3:
+        val = ati_reg_read_offs(s->regs.gpio_dvi_ddc,
+                                addr - GPIO_DVI_DDC, size);
         break;
     case GPIO_MONID ... GPIO_MONID + 3:
         val = ati_reg_read_offs(s->regs.gpio_monid,
@@ -626,29 +628,34 @@ static void ati_mm_write(void *opaque, hwaddr addr,
         s->regs.dac_cntl = data & 0xffffe3ff;
         s->vga.dac_8bit = !!(data & DAC_8BIT_EN);
         break;
-    case GPIO_VGA_DDC:
+    /*
+     * GPIO regs for DDC access. Because some drivers access these via
+     * multiple byte writes we have to be careful when we send bits to
+     * avoid spurious changes in bitbang_i2c state. Only do it when either
+     * the enable bits are changed or output bits changed while enabled.
+     */
+    case GPIO_VGA_DDC ... GPIO_VGA_DDC + 3:
         if (s->dev_id != PCI_DEVICE_ID_ATI_RAGE128_PF) {
             /* FIXME: Maybe add a property to select VGA or DVI port? */
         }
         break;
-    case GPIO_DVI_DDC:
+    case GPIO_DVI_DDC ... GPIO_DVI_DDC + 3:
         if (s->dev_id != PCI_DEVICE_ID_ATI_RAGE128_PF) {
-            s->regs.gpio_dvi_ddc = ati_i2c(&s->bbi2c, data, 0);
+            ati_reg_write_offs(&s->regs.gpio_dvi_ddc,
+                               addr - GPIO_DVI_DDC, data, size);
+            if ((addr <= GPIO_DVI_DDC + 2 && addr + size > GPIO_DVI_DDC + 2) ||
+                (addr == GPIO_DVI_DDC && (s->regs.gpio_dvi_ddc & 0x30000))) {
+                s->regs.gpio_dvi_ddc = ati_i2c(&s->bbi2c,
+                                               s->regs.gpio_dvi_ddc, 0);
+            }
         }
         break;
     case GPIO_MONID ... GPIO_MONID + 3:
         /* FIXME What does Radeon have here? */
         if (s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF) {
+            /* Rage128p accesses DDC via MONID(1-2) with additional mask bit */
             ati_reg_write_offs(&s->regs.gpio_monid,
                                addr - GPIO_MONID, data, size);
-            /*
-             * Rage128p accesses DDC used to get EDID via these bits.
-             * 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 && (s->regs.gpio_monid & 0x60000)))) {
-- 
2.30.9

[PATCH 3/3] ati-vga: Add 30 bit palette access register

[BALATON Zoltan]

Radeon cards have a 30 bit DAC and corresponding palette register to
access it. We only use 8 bits but let the guests use 10 bit color
values for those that access it through this register.

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

diff --git a/hw/display/ati.c b/hw/display/ati.c
index ce63935ead..684fdbf4e2 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -339,6 +339,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 PALETTE_30_DATA:
+        val = s->regs.palette[vga_ioport_read(&s->vga, VGA_PEL_IR)];
+        break;
     case CNFG_CNTL:
         val = s->regs.config_cntl;
         break;
@@ -684,6 +687,12 @@ static void ati_mm_write(void *opaque, hwaddr addr,
         data >>= 8;
         vga_ioport_write(&s->vga, VGA_PEL_D, data & 0xff);
         break;
+    case PALETTE_30_DATA:
+        s->regs.palette[vga_ioport_read(&s->vga, VGA_PEL_IW)] = data;
+        vga_ioport_write(&s->vga, VGA_PEL_D, (data >> 22) & 0xff);
+        vga_ioport_write(&s->vga, VGA_PEL_D, (data >> 12) & 0xff);
+        vga_ioport_write(&s->vga, VGA_PEL_D, (data >> 2) & 0xff);
+        break;
     case CNFG_CNTL:
         s->regs.config_cntl = data;
         break;
diff --git a/hw/display/ati_dbg.c b/hw/display/ati_dbg.c
index bd0ecd48c7..84f48a83ea 100644
--- a/hw/display/ati_dbg.c
+++ b/hw/display/ati_dbg.c
@@ -30,6 +30,7 @@ static struct ati_regdesc ati_reg_names[] = {
     {"AMCGPIO_EN_MIR", 0x00a8},
     {"PALETTE_INDEX", 0x00b0},
     {"PALETTE_DATA", 0x00b4},
+    {"PALETTE_30_DATA", 0x00b8},
     {"CNFG_CNTL", 0x00e0},
     {"GEN_RESET_CNTL", 0x00f0},
     {"CNFG_MEMSIZE", 0x00f8},
diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h
index e8d3c7af75..8abb873f01 100644
--- a/hw/display/ati_int.h
+++ b/hw/display/ati_int.h
@@ -44,6 +44,7 @@ typedef struct ATIVGARegs {
     uint32_t gpio_dvi_ddc;
     uint32_t gpio_monid;
     uint32_t config_cntl;
+    uint32_t palette[256];
     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 d6282b2ef2..881469e6d6 100644
--- a/hw/display/ati_regs.h
+++ b/hw/display/ati_regs.h
@@ -48,6 +48,7 @@
 #define AMCGPIO_EN_MIR                          0x00a8
 #define PALETTE_INDEX                           0x00b0
 #define PALETTE_DATA                            0x00b4
+#define PALETTE_30_DATA                         0x00b8
 #define CNFG_CNTL                               0x00e0
 #define GEN_RESET_CNTL                          0x00f0
 #define CNFG_MEMSIZE                            0x00f8
-- 
2.30.9

Re: [PATCH 0/3] Misc ati-vga patches

[BALATON Zoltan]

On Tue, 10 Oct 2023, BALATON Zoltan wrote:
> Some misc patches I had laying around that could be upstreamed just to
> clean up my tree a bit.

Ping? Is Gerd still the maintainer of gfx or who else should be cc'd on 
such patches?

Regards,
BALATON Zoltan

> BALATON Zoltan (3):
>  ati-vga: Fix aperture sizes
>  ati-vga: Support unaligned access to GPIO DDC registers
>  ati-vga: Add 30 bit palette access register
>
> hw/display/ati.c      | 50 ++++++++++++++++++++++++++++---------------
> hw/display/ati_dbg.c  |  1 +
> hw/display/ati_int.h  |  1 +
> hw/display/ati_regs.h |  1 +
> 4 files changed, 36 insertions(+), 17 deletions(-)
>
>

Re: [PATCH 0/3] Misc ati-vga patches

[BALATON Zoltan]

On Mon, 23 Oct 2023, BALATON Zoltan wrote:
> On Tue, 10 Oct 2023, BALATON Zoltan wrote:
>> Some misc patches I had laying around that could be upstreamed just to
>> clean up my tree a bit.
>
> Ping? Is Gerd still the maintainer of gfx or who else should be cc'd on such 
> patches?

Ping^2

Regards,
BALATON Zoltan
>
>> BALATON Zoltan (3):
>>  ati-vga: Fix aperture sizes
>>  ati-vga: Support unaligned access to GPIO DDC registers
>>  ati-vga: Add 30 bit palette access register
>> 
>> hw/display/ati.c      | 50 ++++++++++++++++++++++++++++---------------
>> hw/display/ati_dbg.c  |  1 +
>> hw/display/ati_int.h  |  1 +
>> hw/display/ati_regs.h |  1 +
>> 4 files changed, 36 insertions(+), 17 deletions(-)
>> 
>> 
>
>

Re: [PATCH 1/3] ati-vga: Fix aperture sizes

[Marc-André Lureau]

Hi

On Tue, Oct 10, 2023 at 5:03 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> Apparently these should be half the memory region sizes confirmed at
> least by Radeon drivers while Rage 128 Pro drivers don't seem to use
> these.

There doesn't seem to be adjustments for the kernel PPC driver
https://github.com/torvalds/linux/blob/master/drivers/video/fbdev/aty/radeon_base.c#L2037

Do you have any other pointers?

thanks

>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/display/ati.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/display/ati.c b/hw/display/ati.c
> index c36282c343..f0bf1d7493 100644
> --- a/hw/display/ati.c
> +++ b/hw/display/ati.c
> @@ -349,14 +349,14 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, unsigned int size)
>                                        PCI_BASE_ADDRESS_0, size) & 0xfffffff0;
>          break;
>      case CONFIG_APER_SIZE:
> -        val = s->vga.vram_size;
> +        val = s->vga.vram_size / 2;
>          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);
> +        val = memory_region_size(&s->mm) / 2;
>          break;
>      case MC_STATUS:
>          val = 5;
> --
> 2.30.9
>
>


-- 
Marc-André Lureau

Re: [PATCH 2/3] ati-vga: Support unaligned access to GPIO DDC registers

[Marc-André Lureau]

On Tue, Oct 10, 2023 at 5:03 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> The GPIO_VGA_DDC and GPIO_DVI_DDC registers are used on Radeon for DDC
> access. Some drivers like the PPC Mac FCode ROM uses unaligned writes
> to these registers so implement this the same way as already done for
> GPIO_MONID which is used the same way for the Rage 128 Pro.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>

Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  hw/display/ati.c | 37 ++++++++++++++++++++++---------------
>  1 file changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/hw/display/ati.c b/hw/display/ati.c
> index f0bf1d7493..ce63935ead 100644
> --- a/hw/display/ati.c
> +++ b/hw/display/ati.c
> @@ -319,11 +319,13 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, unsigned int size)
>      case DAC_CNTL:
>          val = s->regs.dac_cntl;
>          break;
> -    case GPIO_VGA_DDC:
> -        val = s->regs.gpio_vga_ddc;
> +    case GPIO_VGA_DDC ... GPIO_VGA_DDC + 3:
> +        val = ati_reg_read_offs(s->regs.gpio_vga_ddc,
> +                                addr - GPIO_VGA_DDC, size);
>          break;
> -    case GPIO_DVI_DDC:
> -        val = s->regs.gpio_dvi_ddc;
> +    case GPIO_DVI_DDC ... GPIO_DVI_DDC + 3:
> +        val = ati_reg_read_offs(s->regs.gpio_dvi_ddc,
> +                                addr - GPIO_DVI_DDC, size);
>          break;
>      case GPIO_MONID ... GPIO_MONID + 3:
>          val = ati_reg_read_offs(s->regs.gpio_monid,
> @@ -626,29 +628,34 @@ static void ati_mm_write(void *opaque, hwaddr addr,
>          s->regs.dac_cntl = data & 0xffffe3ff;
>          s->vga.dac_8bit = !!(data & DAC_8BIT_EN);
>          break;
> -    case GPIO_VGA_DDC:
> +    /*
> +     * GPIO regs for DDC access. Because some drivers access these via
> +     * multiple byte writes we have to be careful when we send bits to
> +     * avoid spurious changes in bitbang_i2c state. Only do it when either
> +     * the enable bits are changed or output bits changed while enabled.
> +     */
> +    case GPIO_VGA_DDC ... GPIO_VGA_DDC + 3:
>          if (s->dev_id != PCI_DEVICE_ID_ATI_RAGE128_PF) {
>              /* FIXME: Maybe add a property to select VGA or DVI port? */
>          }
>          break;
> -    case GPIO_DVI_DDC:
> +    case GPIO_DVI_DDC ... GPIO_DVI_DDC + 3:
>          if (s->dev_id != PCI_DEVICE_ID_ATI_RAGE128_PF) {
> -            s->regs.gpio_dvi_ddc = ati_i2c(&s->bbi2c, data, 0);
> +            ati_reg_write_offs(&s->regs.gpio_dvi_ddc,
> +                               addr - GPIO_DVI_DDC, data, size);
> +            if ((addr <= GPIO_DVI_DDC + 2 && addr + size > GPIO_DVI_DDC + 2) ||
> +                (addr == GPIO_DVI_DDC && (s->regs.gpio_dvi_ddc & 0x30000))) {
> +                s->regs.gpio_dvi_ddc = ati_i2c(&s->bbi2c,
> +                                               s->regs.gpio_dvi_ddc, 0);
> +            }
>          }
>          break;
>      case GPIO_MONID ... GPIO_MONID + 3:
>          /* FIXME What does Radeon have here? */
>          if (s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF) {
> +            /* Rage128p accesses DDC via MONID(1-2) with additional mask bit */
>              ati_reg_write_offs(&s->regs.gpio_monid,
>                                 addr - GPIO_MONID, data, size);
> -            /*
> -             * Rage128p accesses DDC used to get EDID via these bits.
> -             * 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 && (s->regs.gpio_monid & 0x60000)))) {
> --
> 2.30.9
>
>


-- 
Marc-André Lureau

Re: [PATCH 1/3] ati-vga: Fix aperture sizes

[BALATON Zoltan]

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

On Mon, 30 Oct 2023, Marc-André Lureau wrote:
> Hi
>
> On Tue, Oct 10, 2023 at 5:03 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>>
>> Apparently these should be half the memory region sizes confirmed at
>> least by Radeon drivers while Rage 128 Pro drivers don't seem to use
>> these.
>
> There doesn't seem to be adjustments for the kernel PPC driver
> https://github.com/torvalds/linux/blob/master/drivers/video/fbdev/aty/radeon_base.c#L2037
>
> Do you have any other pointers?

There was some discussion back whan this was added:

https://patchew.org/QEMU/99bb800cba3596e47d2681642116756330dc6f63.1562320946.git.balaton@eik.bme.hu/

and this was also in a patch Gerd sent once around that time but I don't 
find that patch now. I've found this while trying to get some RV100 ROMs 
from real card running which did multiply this by 2 while the Rage128Pro 
ROMs and drivers did not access it and used the BAR sizes I think. 
According to the discussion above maybe this also depends on some other 
bit but I don't have detailed enough docs to know.

Regards,
BALATON Zoltan

> thanks
>
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>  hw/display/ati.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/display/ati.c b/hw/display/ati.c
>> index c36282c343..f0bf1d7493 100644
>> --- a/hw/display/ati.c
>> +++ b/hw/display/ati.c
>> @@ -349,14 +349,14 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, unsigned int size)
>>                                        PCI_BASE_ADDRESS_0, size) & 0xfffffff0;
>>          break;
>>      case CONFIG_APER_SIZE:
>> -        val = s->vga.vram_size;
>> +        val = s->vga.vram_size / 2;
>>          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);
>> +        val = memory_region_size(&s->mm) / 2;
>>          break;
>>      case MC_STATUS:
>>          val = 5;
>> --
>> 2.30.9
>>
>>
>
>
>

Re: [PATCH 0/3] Misc ati-vga patches

[Mark Cave-Ayland]

On 30/10/2023 10:37, BALATON Zoltan wrote:

> On Mon, 23 Oct 2023, BALATON Zoltan wrote:
>> On Tue, 10 Oct 2023, BALATON Zoltan wrote:
>>> Some misc patches I had laying around that could be upstreamed just to
>>> clean up my tree a bit.
>>
>> Ping? Is Gerd still the maintainer of gfx or who else should be cc'd on such patches?
> 
> Ping^2
> 
> Regards,
> BALATON Zoltan
>>
>>> BALATON Zoltan (3):
>>>  ati-vga: Fix aperture sizes
>>>  ati-vga: Support unaligned access to GPIO DDC registers
>>>  ati-vga: Add 30 bit palette access register
>>>
>>> hw/display/ati.c      | 50 ++++++++++++++++++++++++++++---------------
>>> hw/display/ati_dbg.c  |  1 +
>>> hw/display/ati_int.h  |  1 +
>>> hw/display/ati_regs.h |  1 +
>>> 4 files changed, 36 insertions(+), 17 deletions(-)

Slightly off-topic, but are there any known issues with the ati-vga device 
(hardware?) cursor handling? At least here with testing MorphOS with -display gtk, 
the guest unconditionally grabs the mouse and it's impossible to release it. Within 
the windows itself the mouse "skids" which means I can't get to the edge of the 
window to close it, and so I end up having to switch to a text console and kill QEMU 
to be able to access the desktop again.


ATB,

Mark.

Re: [PATCH 0/3] Misc ati-vga patches

[BALATON Zoltan]

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

On Mon, 30 Oct 2023, Mark Cave-Ayland wrote:
> On 30/10/2023 10:37, BALATON Zoltan wrote:
>> On Mon, 23 Oct 2023, BALATON Zoltan wrote:
>>> On Tue, 10 Oct 2023, BALATON Zoltan wrote:
>>>> Some misc patches I had laying around that could be upstreamed just to
>>>> clean up my tree a bit.
>>> 
>>> Ping? Is Gerd still the maintainer of gfx or who else should be cc'd on 
>>> such patches?
>> 
>> Ping^2
>> 
>> Regards,
>> BALATON Zoltan
>>> 
>>>> BALATON Zoltan (3):
>>>>  ati-vga: Fix aperture sizes
>>>>  ati-vga: Support unaligned access to GPIO DDC registers
>>>>  ati-vga: Add 30 bit palette access register
>>>> 
>>>> hw/display/ati.c      | 50 ++++++++++++++++++++++++++++---------------
>>>> hw/display/ati_dbg.c  |  1 +
>>>> hw/display/ati_int.h  |  1 +
>>>> hw/display/ati_regs.h |  1 +
>>>> 4 files changed, 36 insertions(+), 17 deletions(-)
>
> Slightly off-topic, but are there any known issues with the ati-vga device 
> (hardware?) cursor handling? At least here with testing MorphOS with -display 
> gtk, the guest unconditionally grabs the mouse and it's impossible to release 
> it. Within the windows itself the mouse "skids" which means I can't get to 
> the edge of the window to close it, and so I end up having to switch to a 
> text console and kill QEMU to be able to access the desktop again.

Try -device ati-vga,guest_hwcursor=true that may work better. There are 
two ways in QEMU to handle HW cursor and the default is to let the host 
move the pointer but the problem with this in my understanding is that the 
host and guest may have different mouse pointer acceleration settings so 
the host pointer and guest pointer may get out of sync which causes it to 
jump ot lag. There's another API which does it differently and let the 
guest dictate where the pointer is which does not get out of sync but may 
freeze or jump if the guest is busy. Or something like that. I've 
implemented both in ati-vga as first it used the host pointer but others 
also reported problems so added the guest_hwcursor option as well. Maybe 
also using -display sdl can help as I did not get that problem with sdl.

Regards,
BALATON Zoltan

Re: [PATCH 1/3] ati-vga: Fix aperture sizes

[BALATON Zoltan]

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

On Mon, 30 Oct 2023, Marc-André Lureau wrote:
> Hi
>
> On Tue, Oct 10, 2023 at 5:03 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>>
>> Apparently these should be half the memory region sizes confirmed at
>> least by Radeon drivers while Rage 128 Pro drivers don't seem to use
>> these.
>
> There doesn't seem to be adjustments for the kernel PPC driver
> https://github.com/torvalds/linux/blob/master/drivers/video/fbdev/aty/radeon_base.c#L2037
>
> Do you have any other pointers?

The FCode ROM from a Radeon 7000 card has something like this (obtained 
by detokenising FCode so names are added for conprehensibility):

b(;)
map-in-io-bar
   const_REG_CONFIG_APER_SIZE \ 0x108
   ati-reg-l@ \ fetch 32bits
   dup
   b(to) var_aper_size
   2*
   b(to) var_ram_size
   const_REG_CONFIG_REG_APER_SIZE \ 0x110
   ati-reg-l@
   2*
   b(to) var_mmio_size

Similar Rage128Pro ROMs do not access these and it does not write or check 
the bit Gerd referred to in HOST_PATH_CNTL so I think having half of 
memory sizes in these regs is correct based on the ATI card ROM that 
should get it right.

Linux seems to do it differently in different drivers. The fbdev you 
referred to does not double it but the DRM driver in 
linux/drivers/gpu/drm/radeon/r100.c::r100_get_accessible_vram() does in 
some cases which seems to depend on a bit in HOST_PATH_CNTL so maybe I 
shuold also add that bit but not sure what would set it for the FCode ROM 
which does not seem to set itself and still assumes the APER_SIZE regs 
return half the size. I don't know what's correct here but adding the bit 
in HOST_PATH_CNTL won't break the ROM and might work better with Linux DRM 
driver so maybe I should add that as well.

Regards,
BALATON Zoltan

> thanks
>
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>  hw/display/ati.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/display/ati.c b/hw/display/ati.c
>> index c36282c343..f0bf1d7493 100644
>> --- a/hw/display/ati.c
>> +++ b/hw/display/ati.c
>> @@ -349,14 +349,14 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, unsigned int size)
>>                                        PCI_BASE_ADDRESS_0, size) & 0xfffffff0;
>>          break;
>>      case CONFIG_APER_SIZE:
>> -        val = s->vga.vram_size;
>> +        val = s->vga.vram_size / 2;
>>          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);
>> +        val = memory_region_size(&s->mm) / 2;
>>          break;
>>      case MC_STATUS:
>>          val = 5;
>> --
>> 2.30.9
>>
>>
>
>
>