[v2] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes

[PATCH v2] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes

Posted by Chad Jablonski 4 weeks, 1 day ago

Rage 128 cards always request 64MB for their linear (framebuffer)
aperture. This is regardless of the amount of physical VRAM on the
board. The following are results from real hardware tests:

Card                          VRAM    PCI BAR0   CONFIG_MEMSIZE  CONFIG_APER_SIZE  AGP_APER_OFFSET
-----------------------       ----    --------   --------------  ----------------  ---------------
Rage 128 Pro Ultra TF         32MB     64MB       0x02000000      0x02000000        0x02000000
Rage 128 RF/SG AGP            16MB     64MB       0x01000000      0x02000000        0x02000000
Radeon R100 QD [Radeon 7200]  64MB    128MB       0x04000000      0x04000000        N/A

Previously the linear aperture (BAR0) would match the VRAM size.
This discrepancy caused issues with the X.org and XFree86 r128 drivers.
These drivers apply a mask of 0xfc000000 (2^26 = 64MB) to the linear
aperture address. If that address is not on a 64MB boundary the
framebuffer points to an incorrect memory location.

Testing shows that the Radeon R100 also has a BAR0 larger than VRAM
(128MB in this case) and the X.org driver for Radeon also masks to 64MB.

For Rage 128, CONFIG_APER_SIZE also differs from the previous value and
the behavior stated in the documentation. The Rage 128 register guide
states that it should contain the size of the VRAM + AGP memory. The cards
tested above show that this isn't the case. These tests also included
enabling/disabling AGP with 8MB of memory. It didn't change the
contents of CONFIG_APER_SIZE.

For Radeon R100, CONFIG_APER_SIZE behavior remains the same. There is
some worry that changing it could cause issues for other cards.
Especially given that I have only a single example here to test.

Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
---
 hw/display/ati.c     | 20 ++++++++++++++++++--
 hw/display/ati_int.h |  4 ++++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/hw/display/ati.c b/hw/display/ati.c
index f7c0006a87..7af12777c8 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -30,6 +30,7 @@
 #include "ui/console.h"
 #include "hw/display/i2c-ddc.h"
 #include "trace.h"
+#include "qemu/units.h"
 
 #define ATI_DEBUG_HW_CURSOR 0
 
@@ -361,7 +362,8 @@ 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 / 2;
+        val = s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF ?
+            ATI_RAGE128_LINEAR_APER_SIZE / 2 : s->vga.vram_size / 2;
         break;
     case CONFIG_REG_1_BASE:
         val = pci_default_read_config(&s->dev,
@@ -952,6 +954,7 @@ static void ati_vga_realize(PCIDevice *dev, Error **errp)
 {
     ATIVGAState *s = ATI_VGA(dev);
     VGACommonState *vga = &s->vga;
+    uint64_t aper_size;
 
 #ifndef CONFIG_PIXMAN
     if (s->use_pixman != 0) {
@@ -1011,7 +1014,20 @@ static void ati_vga_realize(PCIDevice *dev, Error **errp)
     /* io space is alias to beginning of mmregs */
     memory_region_init_alias(&s->io, OBJECT(s), "ati.io", &s->mm, 0, 0x100);
 
-    pci_register_bar(dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH, &vga->vram);
+    /*
+     * Rage128: Framebuffer inhabits the bottom 32MB of the linear aperture.
+     *          The top 32MB is reserved for AGP (not implemented).
+     *
+     * R100: Linear aperture layout differs from Rage 128. No
+     *       AGP_APER_OFFSET register exists.
+     */
+    aper_size = s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF ?
+        ATI_RAGE128_LINEAR_APER_SIZE : ATI_R100_LINEAR_APER_SIZE;
+    memory_region_init(&s->linear_aper, OBJECT(dev), "ati-linear-aperture0",
+                       aper_size);
+    memory_region_add_subregion(&s->linear_aper, 0, &vga->vram);
+
+    pci_register_bar(dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH, &s->linear_aper);
     pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->io);
     pci_register_bar(dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->mm);
 
diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h
index f5a47b82b0..4f66d0df3f 100644
--- a/hw/display/ati_int.h
+++ b/hw/display/ati_int.h
@@ -29,6 +29,9 @@
 /* Radeon RV100 (VE) */
 #define PCI_DEVICE_ID_ATI_RADEON_QY 0x5159
 
+#define ATI_RAGE128_LINEAR_APER_SIZE (64 * MiB)
+#define ATI_R100_LINEAR_APER_SIZE (128 * MiB)
+
 #define TYPE_ATI_VGA "ati-vga"
 OBJECT_DECLARE_SIMPLE_TYPE(ATIVGAState, ATI_VGA)
 
@@ -97,6 +100,7 @@ struct ATIVGAState {
     QEMUCursor *cursor;
     QEMUTimer vblank_timer;
     bitbang_i2c_interface bbi2c;
+    MemoryRegion linear_aper;
     MemoryRegion io;
     MemoryRegion mm;
     ATIVGARegs regs;
-- 
2.51.0

Re: [PATCH v2] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes

Posted by BALATON Zoltan 4 weeks, 1 day ago

On Wed, 15 Oct 2025, Chad Jablonski wrote:
> Rage 128 cards always request 64MB for their linear (framebuffer)
> aperture. This is regardless of the amount of physical VRAM on the
> board. The following are results from real hardware tests:
>
> Card                          VRAM    PCI BAR0   CONFIG_MEMSIZE  CONFIG_APER_SIZE  AGP_APER_OFFSET
> -----------------------       ----    --------   --------------  ----------------  ---------------
> Rage 128 Pro Ultra TF         32MB     64MB       0x02000000      0x02000000        0x02000000
> Rage 128 RF/SG AGP            16MB     64MB       0x01000000      0x02000000        0x02000000
> Radeon R100 QD [Radeon 7200]  64MB    128MB       0x04000000      0x04000000        N/A
>
> Previously the linear aperture (BAR0) would match the VRAM size.
> This discrepancy caused issues with the X.org and XFree86 r128 drivers.
> These drivers apply a mask of 0xfc000000 (2^26 = 64MB) to the linear
> aperture address. If that address is not on a 64MB boundary the
> framebuffer points to an incorrect memory location.
>
> Testing shows that the Radeon R100 also has a BAR0 larger than VRAM
> (128MB in this case) and the X.org driver for Radeon also masks to 64MB.
>
> For Rage 128, CONFIG_APER_SIZE also differs from the previous value and
> the behavior stated in the documentation. The Rage 128 register guide
> states that it should contain the size of the VRAM + AGP memory. The cards
> tested above show that this isn't the case. These tests also included
> enabling/disabling AGP with 8MB of memory. It didn't change the
> contents of CONFIG_APER_SIZE.
>
> For Radeon R100, CONFIG_APER_SIZE behavior remains the same. There is
> some worry that changing it could cause issues for other cards.
> Especially given that I have only a single example here to test.
>
> Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
> ---
> hw/display/ati.c     | 20 ++++++++++++++++++--
> hw/display/ati_int.h |  4 ++++
> 2 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/hw/display/ati.c b/hw/display/ati.c
> index f7c0006a87..7af12777c8 100644
> --- a/hw/display/ati.c
> +++ b/hw/display/ati.c
> @@ -30,6 +30,7 @@
> #include "ui/console.h"
> #include "hw/display/i2c-ddc.h"
> #include "trace.h"
> +#include "qemu/units.h"

Just some nits. This include now also belongs to ati_int.h where it's 
used.

> #define ATI_DEBUG_HW_CURSOR 0
>
> @@ -361,7 +362,8 @@ 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 / 2;
> +        val = s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF ?
> +            ATI_RAGE128_LINEAR_APER_SIZE / 2 : s->vga.vram_size / 2;

Should indent below s-> like at other places.

>         break;
>     case CONFIG_REG_1_BASE:
>         val = pci_default_read_config(&s->dev,
> @@ -952,6 +954,7 @@ static void ati_vga_realize(PCIDevice *dev, Error **errp)
> {
>     ATIVGAState *s = ATI_VGA(dev);
>     VGACommonState *vga = &s->vga;
> +    uint64_t aper_size;
>
> #ifndef CONFIG_PIXMAN
>     if (s->use_pixman != 0) {
> @@ -1011,7 +1014,20 @@ static void ati_vga_realize(PCIDevice *dev, Error **errp)
>     /* io space is alias to beginning of mmregs */
>     memory_region_init_alias(&s->io, OBJECT(s), "ati.io", &s->mm, 0, 0x100);
>
> -    pci_register_bar(dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH, &vga->vram);
> +    /*
> +     * Rage128: Framebuffer inhabits the bottom 32MB of the linear aperture.
> +     *          The top 32MB is reserved for AGP (not implemented).
> +     *
> +     * R100: Linear aperture layout differs from Rage 128. No
> +     *       AGP_APER_OFFSET register exists.
> +     */

We don't use AGP_APER_OFFSET here so this comment isn't that clean. The 
layout does not differ in that VRAM is still at the beginning but only in 
that Radeon has no AGP window.

> +    aper_size = s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF ?
> +        ATI_RAGE128_LINEAR_APER_SIZE : ATI_R100_LINEAR_APER_SIZE;

Indent.

Regards,
BALATON Zoltan

> +    memory_region_init(&s->linear_aper, OBJECT(dev), "ati-linear-aperture0",
> +                       aper_size);
> +    memory_region_add_subregion(&s->linear_aper, 0, &vga->vram);
> +
> +    pci_register_bar(dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH, &s->linear_aper);
>     pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->io);
>     pci_register_bar(dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->mm);
>
> diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h
> index f5a47b82b0..4f66d0df3f 100644
> --- a/hw/display/ati_int.h
> +++ b/hw/display/ati_int.h
> @@ -29,6 +29,9 @@
> /* Radeon RV100 (VE) */
> #define PCI_DEVICE_ID_ATI_RADEON_QY 0x5159
>
> +#define ATI_RAGE128_LINEAR_APER_SIZE (64 * MiB)
> +#define ATI_R100_LINEAR_APER_SIZE (128 * MiB)
> +
> #define TYPE_ATI_VGA "ati-vga"
> OBJECT_DECLARE_SIMPLE_TYPE(ATIVGAState, ATI_VGA)
>
> @@ -97,6 +100,7 @@ struct ATIVGAState {
>     QEMUCursor *cursor;
>     QEMUTimer vblank_timer;
>     bitbang_i2c_interface bbi2c;
> +    MemoryRegion linear_aper;
>     MemoryRegion io;
>     MemoryRegion mm;
>     ATIVGARegs regs;
>

Re: [PATCH v2] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes

Posted by BALATON Zoltan 4 weeks, 1 day ago

On Thu, 16 Oct 2025, BALATON Zoltan wrote:
> On Wed, 15 Oct 2025, Chad Jablonski wrote:
>> Rage 128 cards always request 64MB for their linear (framebuffer)
>> aperture. This is regardless of the amount of physical VRAM on the
>> board. The following are results from real hardware tests:
>> 
>> Card                          VRAM    PCI BAR0   CONFIG_MEMSIZE 
>> CONFIG_APER_SIZE  AGP_APER_OFFSET
>> -----------------------       ----    --------   -------------- 
>> ----------------  ---------------
>> Rage 128 Pro Ultra TF         32MB     64MB       0x02000000 
>> 0x02000000        0x02000000
>> Rage 128 RF/SG AGP            16MB     64MB       0x01000000 
>> 0x02000000        0x02000000
>> Radeon R100 QD [Radeon 7200]  64MB    128MB       0x04000000 
>> 0x04000000        N/A
>> 
>> Previously the linear aperture (BAR0) would match the VRAM size.
>> This discrepancy caused issues with the X.org and XFree86 r128 drivers.
>> These drivers apply a mask of 0xfc000000 (2^26 = 64MB) to the linear
>> aperture address. If that address is not on a 64MB boundary the
>> framebuffer points to an incorrect memory location.
>> 
>> Testing shows that the Radeon R100 also has a BAR0 larger than VRAM
>> (128MB in this case) and the X.org driver for Radeon also masks to 64MB.
>> 
>> For Rage 128, CONFIG_APER_SIZE also differs from the previous value and
>> the behavior stated in the documentation. The Rage 128 register guide
>> states that it should contain the size of the VRAM + AGP memory. The cards
>> tested above show that this isn't the case. These tests also included
>> enabling/disabling AGP with 8MB of memory. It didn't change the
>> contents of CONFIG_APER_SIZE.
>> 
>> For Radeon R100, CONFIG_APER_SIZE behavior remains the same. There is
>> some worry that changing it could cause issues for other cards.
>> Especially given that I have only a single example here to test.
>> 
>> Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
>> ---
>> hw/display/ati.c     | 20 ++++++++++++++++++--
>> hw/display/ati_int.h |  4 ++++
>> 2 files changed, 22 insertions(+), 2 deletions(-)
>> 
>> diff --git a/hw/display/ati.c b/hw/display/ati.c
>> index f7c0006a87..7af12777c8 100644
>> --- a/hw/display/ati.c
>> +++ b/hw/display/ati.c
>> @@ -30,6 +30,7 @@
>> #include "ui/console.h"
>> #include "hw/display/i2c-ddc.h"
>> #include "trace.h"
>> +#include "qemu/units.h"
>
> Just some nits. This include now also belongs to ati_int.h where it's used.
>
>> #define ATI_DEBUG_HW_CURSOR 0
>> 
>> @@ -361,7 +362,8 @@ 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 / 2;
>> +        val = s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF ?
>> +            ATI_RAGE128_LINEAR_APER_SIZE / 2 : s->vga.vram_size / 2;
>
> Should indent below s-> like at other places.
>
>>         break;
>>     case CONFIG_REG_1_BASE:
>>         val = pci_default_read_config(&s->dev,
>> @@ -952,6 +954,7 @@ static void ati_vga_realize(PCIDevice *dev, Error 
>> **errp)
>> {
>>     ATIVGAState *s = ATI_VGA(dev);
>>     VGACommonState *vga = &s->vga;
>> +    uint64_t aper_size;
>> 
>> #ifndef CONFIG_PIXMAN
>>     if (s->use_pixman != 0) {
>> @@ -1011,7 +1014,20 @@ static void ati_vga_realize(PCIDevice *dev, Error 
>> **errp)
>>     /* io space is alias to beginning of mmregs */
>>     memory_region_init_alias(&s->io, OBJECT(s), "ati.io", &s->mm, 0, 
>> 0x100);
>> 
>> -    pci_register_bar(dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH, &vga->vram);
>> +    /*
>> +     * Rage128: Framebuffer inhabits the bottom 32MB of the linear 
>> aperture.
>> +     *          The top 32MB is reserved for AGP (not implemented).
>> +     *
>> +     * R100: Linear aperture layout differs from Rage 128. No
>> +     *       AGP_APER_OFFSET register exists.
>> +     */
>
> We don't use AGP_APER_OFFSET here so this comment isn't that clean. The

I mean comment isn't that clear. Maybe it's enough to say that framebuffer 
is at the beginning of the aperture which has some additional space or AGP 
window for Rage128 which we don't emulate.

> layout does not differ in that VRAM is still at the beginning but only in 
> that Radeon has no AGP window.
>
>> +    aper_size = s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF ?
>> +        ATI_RAGE128_LINEAR_APER_SIZE : ATI_R100_LINEAR_APER_SIZE;
>
> Indent.
>
> Regards,
> BALATON Zoltan
>
>> +    memory_region_init(&s->linear_aper, OBJECT(dev), 
>> "ati-linear-aperture0",
>> +                       aper_size);
>> +    memory_region_add_subregion(&s->linear_aper, 0, &vga->vram);
>> +
>> +    pci_register_bar(dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH, 
>> &s->linear_aper);
>>     pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->io);
>>     pci_register_bar(dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->mm);
>> 
>> diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h
>> index f5a47b82b0..4f66d0df3f 100644
>> --- a/hw/display/ati_int.h
>> +++ b/hw/display/ati_int.h
>> @@ -29,6 +29,9 @@
>> /* Radeon RV100 (VE) */
>> #define PCI_DEVICE_ID_ATI_RADEON_QY 0x5159
>> 
>> +#define ATI_RAGE128_LINEAR_APER_SIZE (64 * MiB)
>> +#define ATI_R100_LINEAR_APER_SIZE (128 * MiB)
>> +
>> #define TYPE_ATI_VGA "ati-vga"
>> OBJECT_DECLARE_SIMPLE_TYPE(ATIVGAState, ATI_VGA)
>> 
>> @@ -97,6 +100,7 @@ struct ATIVGAState {
>>     QEMUCursor *cursor;
>>     QEMUTimer vblank_timer;
>>     bitbang_i2c_interface bbi2c;
>> +    MemoryRegion linear_aper;
>>     MemoryRegion io;
>>     MemoryRegion mm;
>>     ATIVGARegs regs;
>> 
>
>

Re: [PATCH v2] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes

Posted by Chad Jablonski 4 weeks ago

Fixing all of these and sending v3 shortly.

- Chad