qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes
@ 2025-10-01  3:46 Chad Jablonski
  2025-10-07 20:52 ` BALATON Zoltan
  0 siblings, 1 reply; 10+ messages in thread
From: Chad Jablonski @ 2025-10-01  3:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Chad Jablonski

Real Rage 128 cards always request 64MB for their linear (framebuffer)
aperture. This is regardless of the amount of physical VRAM on the
board. This is required for 64MB alignment which is important given the
26-bit addressing in src and dst registers.

This discrepancy caused X to segfault or display garbage depending on
the version tested. X expects this 64MB alignment.

This was confirmed by testing against the behavior of real 16MB and 32MB
Rage 128 cards.

Real Radeon R100 cards request 128MB for linear aperture. This was
tested against a Radeon 7200 with 64MB of VRAM.

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

diff --git a/hw/display/ati.c b/hw/display/ati.c
index f7c0006a87..db189e0767 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -30,9 +30,13 @@
 #include "ui/console.h"
 #include "hw/display/i2c-ddc.h"
 #include "trace.h"
+#include "qemu/units.h"
 
 #define ATI_DEBUG_HW_CURSOR 0
 
+#define ATI_RAGE128_LINEAR_APERTURE_SIZE (64 * MiB)
+#define ATI_RADEON_LINEAR_APERTURE_SIZE (128 * MiB)
+
 #ifdef CONFIG_PIXMAN
 #define DEFAULT_X_PIXMAN 3
 #else
@@ -361,7 +365,7 @@ 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 = memory_region_size(&s->linear_aper);
         break;
     case CONFIG_REG_1_BASE:
         val = pci_default_read_config(&s->dev,
@@ -1011,7 +1015,25 @@ 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);
+    uint64_t aperture_size;
+    if (s->dev_id == PCI_DEVICE_ID_ATI_RADEON_QY) {
+        aperture_size = ATI_RADEON_LINEAR_APERTURE_SIZE;
+    } else {
+        aperture_size = ATI_RAGE128_LINEAR_APERTURE_SIZE;
+    }
+    memory_region_init(&s->linear_aper, OBJECT(dev), "ati-linear-aperture0",
+                       aperture_size);
+
+    /*
+     * Rage 128: Framebuffer inhabits the bottom 32MB of the linear aperture.
+     *           The top 32MB is reserved for AGP (not implemented).
+     *
+     * Radeon: The entire linear aperture is used for VRAM.
+     *         AGP is mapped separately.
+     */
+    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..ff2a69a0a5 100644
--- a/hw/display/ati_int.h
+++ b/hw/display/ati_int.h
@@ -97,6 +97,7 @@ struct ATIVGAState {
     QEMUCursor *cursor;
     QEMUTimer vblank_timer;
     bitbang_i2c_interface bbi2c;
+    MemoryRegion linear_aper;
     MemoryRegion io;
     MemoryRegion mm;
     ATIVGARegs regs;
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes
  2025-10-01  3:46 [PATCH] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes Chad Jablonski
@ 2025-10-07 20:52 ` BALATON Zoltan
  2025-10-07 20:57   ` BALATON Zoltan
  2025-10-13 14:52   ` Chad Jablonski
  0 siblings, 2 replies; 10+ messages in thread
From: BALATON Zoltan @ 2025-10-07 20:52 UTC (permalink / raw)
  To: Chad Jablonski; +Cc: qemu-devel, Gerd Hoffmann, marcandre.lureau

Hello,

Thanks for the contribution.

On Tue, 30 Sep 2025, Chad Jablonski wrote:
> Real Rage 128 cards always request 64MB for their linear (framebuffer)
> aperture. This is regardless of the amount of physical VRAM on the
> board. This is required for 64MB alignment which is important given the
> 26-bit addressing in src and dst registers.
>
> This discrepancy caused X to segfault or display garbage depending on
> the version tested. X expects this 64MB alignment.

The documentation does not mention 64MB alignment. It says apertures must 
be on a 32MB boundary and src and dst offsets are 128 bit aligned but 
maybe I don't have the right documentation for these chips or don't get 
what it means.

> This was confirmed by testing against the behavior of real 16MB and 32MB
> Rage 128 cards.
>
> Real Radeon R100 cards request 128MB for linear aperture. This was
> tested against a Radeon 7200 with 64MB of VRAM.

Can you check what the CONFIG_APER_SIZE register contains on these cards? 
Do all Rage 128 (and Pro) cards have 64MB and Radeon 7xxx/M6 have 128MB? 
The documentation is again not clear on this because it lists default 
value of 0x2000000 for CONFIG_APER_SIZE on Rage 128 Pro and nothing for 
Radeon but in a figure it shows this should contain both VRAM and AGP 
areas that suggests 64MB but it's possible that the documentation is 
wrong.

> Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
> ---
> hw/display/ati.c     | 26 ++++++++++++++++++++++++--
> hw/display/ati_int.h |  1 +
> 2 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/hw/display/ati.c b/hw/display/ati.c
> index f7c0006a87..db189e0767 100644
> --- a/hw/display/ati.c
> +++ b/hw/display/ati.c
> @@ -30,9 +30,13 @@
> #include "ui/console.h"
> #include "hw/display/i2c-ddc.h"
> #include "trace.h"
> +#include "qemu/units.h"
>
> #define ATI_DEBUG_HW_CURSOR 0
>
> +#define ATI_RAGE128_LINEAR_APERTURE_SIZE (64 * MiB)
> +#define ATI_RADEON_LINEAR_APERTURE_SIZE (128 * MiB)

Maybe these should go in ati_int.h and may try to make it shorter like 
ATI_*_LINEAR_APER_SIZE.

> +
> #ifdef CONFIG_PIXMAN
> #define DEFAULT_X_PIXMAN 3
> #else
> @@ -361,7 +365,7 @@ 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 = memory_region_size(&s->linear_aper);

This was changed in commit f7ecde051dd73 to be half the memory size 
because at least a Radeon FCocde ROM seemed to detect VRAM size that way. 
I should test that again but it needed OpenBIOS patches that were not 
merged so I had to find those again as I don't have that setup any more. 
Checking on real card may be the best source of what this should be but I 
think this might break that FCode ROM which is from a PowerMac card. This 
suggests it should be half the aper size.

>         break;
>     case CONFIG_REG_1_BASE:
>         val = pci_default_read_config(&s->dev,
> @@ -1011,7 +1015,25 @@ 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);
> +    uint64_t aperture_size;

Coding style says variables should be defined at the beginning of blocks 
so in this case at the beginning of the funcion.

> +    if (s->dev_id == PCI_DEVICE_ID_ATI_RADEON_QY) {

I usually check for Rage128Pro against everything else (in case more 
Radeon models will be added in the future, but I broke that in a few 
places). So you could write this as

aper_size = s->dev_id == PCI_DEVICE_ID_ATI_RADEON_PF ?
             ATI_RAGE128_LINEAR_APER_SIZE : ATI_RADEON_LINEAR_APER_SIZE;

> +        aperture_size = ATI_RADEON_LINEAR_APERTURE_SIZE;
> +    } else {
> +        aperture_size = ATI_RAGE128_LINEAR_APERTURE_SIZE;
> +    }
> +    memory_region_init(&s->linear_aper, OBJECT(dev), "ati-linear-aperture0",
> +                       aperture_size);
> +

No new line needed here, it's still setting up BAR0 so should be in one 
block.

Regards,
BALATON Zoltan

> +    /*
> +     * Rage 128: Framebuffer inhabits the bottom 32MB of the linear aperture.
> +     *           The top 32MB is reserved for AGP (not implemented).
> +     *
> +     * Radeon: The entire linear aperture is used for VRAM.
> +     *         AGP is mapped separately.
> +     */
> +    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..ff2a69a0a5 100644
> --- a/hw/display/ati_int.h
> +++ b/hw/display/ati_int.h
> @@ -97,6 +97,7 @@ struct ATIVGAState {
>     QEMUCursor *cursor;
>     QEMUTimer vblank_timer;
>     bitbang_i2c_interface bbi2c;
> +    MemoryRegion linear_aper;
>     MemoryRegion io;
>     MemoryRegion mm;
>     ATIVGARegs regs;
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes
  2025-10-07 20:52 ` BALATON Zoltan
@ 2025-10-07 20:57   ` BALATON Zoltan
  2025-10-13 14:52   ` Chad Jablonski
  1 sibling, 0 replies; 10+ messages in thread
From: BALATON Zoltan @ 2025-10-07 20:57 UTC (permalink / raw)
  To: Chad Jablonski; +Cc: qemu-devel, Gerd Hoffmann, marcandre.lureau

On Tue, 7 Oct 2025, BALATON Zoltan wrote:
> Hello,
>
> Thanks for the contribution.
>
> On Tue, 30 Sep 2025, Chad Jablonski wrote:
>> Real Rage 128 cards always request 64MB for their linear (framebuffer)
>> aperture. This is regardless of the amount of physical VRAM on the
>> board. This is required for 64MB alignment which is important given the
>> 26-bit addressing in src and dst registers.
>> 
>> This discrepancy caused X to segfault or display garbage depending on
>> the version tested. X expects this 64MB alignment.
>
> The documentation does not mention 64MB alignment. It says apertures must be 
> on a 32MB boundary and src and dst offsets are 128 bit aligned but maybe I 
> don't have the right documentation for these chips or don't get what it 
> means.
>
>> This was confirmed by testing against the behavior of real 16MB and 32MB
>> Rage 128 cards.
>> 
>> Real Radeon R100 cards request 128MB for linear aperture. This was
>> tested against a Radeon 7200 with 64MB of VRAM.
>
> Can you check what the CONFIG_APER_SIZE register contains on these cards? Do 
> all Rage 128 (and Pro) cards have 64MB and Radeon 7xxx/M6 have 128MB? The 
> documentation is again not clear on this because it lists default value of 
> 0x2000000 for CONFIG_APER_SIZE on Rage 128 Pro and nothing for Radeon but in 
> a figure it shows this should contain both VRAM and AGP areas that suggests 
> 64MB but it's possible that the documentation is wrong.
>
>> Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
>> ---
>> hw/display/ati.c     | 26 ++++++++++++++++++++++++--
>> hw/display/ati_int.h |  1 +
>> 2 files changed, 25 insertions(+), 2 deletions(-)
>> 
>> diff --git a/hw/display/ati.c b/hw/display/ati.c
>> index f7c0006a87..db189e0767 100644
>> --- a/hw/display/ati.c
>> +++ b/hw/display/ati.c
>> @@ -30,9 +30,13 @@
>> #include "ui/console.h"
>> #include "hw/display/i2c-ddc.h"
>> #include "trace.h"
>> +#include "qemu/units.h"
>> 
>> #define ATI_DEBUG_HW_CURSOR 0
>> 
>> +#define ATI_RAGE128_LINEAR_APERTURE_SIZE (64 * MiB)
>> +#define ATI_RADEON_LINEAR_APERTURE_SIZE (128 * MiB)

Also maybe call it ATI_R100_APER_SIZE instead of RADEON as later Radeons 
probably increased this.

Regards,
BALATON Zoltan


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes
  2025-10-07 20:52 ` BALATON Zoltan
  2025-10-07 20:57   ` BALATON Zoltan
@ 2025-10-13 14:52   ` Chad Jablonski
  2025-10-13 16:24     ` BALATON Zoltan
  1 sibling, 1 reply; 10+ messages in thread
From: Chad Jablonski @ 2025-10-13 14:52 UTC (permalink / raw)
  To: BALATON Zoltan, Chad Jablonski
  Cc: qemu-devel, Gerd Hoffmann, marcandre.lureau

Hi Balaton,

Thanks for taking a look! I'll address all of these and send a v2.

> The documentation does not mention 64MB alignment. It says apertures must 
> be on a 32MB boundary and src and dst offsets are 128 bit aligned but 
> maybe I don't have the right documentation for these chips or don't get 
> what it means.
>

Agreed, yeah the register docs do say there is a max of 32MB of framebuffer
memory and a max of 32MB of AGP memory (register reference pg. 2-6)
which implies 32MB alignment. And the software guide does explicitly say
that apertures must be located on a 32MB boundary (software guide 2-19).
It then goes on to describe how the aperture is split into 32MB for frame
buffer space and 32MB for AGP/PCI space. Which implies that 64MB is needed
for the full aperture. And from what I understand requesting 64MB will
naturally lead to a 64MB alignment from PCI.

Outside of what I observed on the real hardware another thing that lead me
to believe that the 64MB alignment is correct was this line in the XOrg
r128 driver: https://gitlab.freedesktop.org/xorg/driver/xf86-video-r128/-/blob/XORG-RELEASE-1/src/r128_driver.c#L855
The driver is assuming a 64MB alignment by masking addresses with
0xfc000000 (2^26 = 64MB). When qemu requests 16MB or 32MB it breaks that assumption
and the driver looks in the wrong memory area for the framebuffer.

>
> Can you check what the CONFIG_APER_SIZE register contains on these cards? 
> Do all Rage 128 (and Pro) cards have 64MB and Radeon 7xxx/M6 have 128MB? 
> The documentation is again not clear on this because it lists default 
> value of 0x2000000 for CONFIG_APER_SIZE on Rage 128 Pro and nothing for 
> Radeon but in a figure it shows this should contain both VRAM and AGP 
> areas that suggests 64MB but it's possible that the documentation is 
> wrong.
>

I will take a look at this!

>
> This was changed in commit f7ecde051dd73 to be half the memory size 
> because at least a Radeon FCocde ROM seemed to detect VRAM size that way. 
> I should test that again but it needed OpenBIOS patches that were not 
> merged so I had to find those again as I don't have that setup any more. 
> Checking on real card may be the best source of what this should be but I 
> think this might break that FCode ROM which is from a PowerMac card. This 
> suggests it should be half the aper size.
>

Ah, great, this is important context. I'll look at this more
closely and check the real hardware.

>
> Coding style says variables should be defined at the beginning of blocks 
> so in this case at the beginning of the funcion.
>

I'll also address all of the style comments.

Thanks again,
Chad


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes
  2025-10-13 14:52   ` Chad Jablonski
@ 2025-10-13 16:24     ` BALATON Zoltan
  2025-10-13 20:04       ` Chad Jablonski
  0 siblings, 1 reply; 10+ messages in thread
From: BALATON Zoltan @ 2025-10-13 16:24 UTC (permalink / raw)
  To: Chad Jablonski; +Cc: qemu-devel, Gerd Hoffmann, marcandre.lureau

On Mon, 13 Oct 2025, Chad Jablonski wrote:
> Hi Balaton,
>
> Thanks for taking a look! I'll address all of these and send a v2.
>
>> The documentation does not mention 64MB alignment. It says apertures must
>> be on a 32MB boundary and src and dst offsets are 128 bit aligned but
>> maybe I don't have the right documentation for these chips or don't get
>> what it means.
>>
>
> Agreed, yeah the register docs do say there is a max of 32MB of framebuffer
> memory and a max of 32MB of AGP memory (register reference pg. 2-6)
> which implies 32MB alignment. And the software guide does explicitly say
> that apertures must be located on a 32MB boundary (software guide 2-19).
> It then goes on to describe how the aperture is split into 32MB for frame
> buffer space and 32MB for AGP/PCI space. Which implies that 64MB is needed
> for the full aperture. And from what I understand requesting 64MB will
> naturally lead to a 64MB alignment from PCI.
>
> Outside of what I observed on the real hardware another thing that lead me
> to believe that the 64MB alignment is correct was this line in the XOrg
> r128 driver: https://gitlab.freedesktop.org/xorg/driver/xf86-video-r128/-/blob/XORG-RELEASE-1/src/r128_driver.c#L855
> The driver is assuming a 64MB alignment by masking addresses with
> 0xfc000000 (2^26 = 64MB). When qemu requests 16MB or 32MB it breaks that assumption
> and the driver looks in the wrong memory area for the framebuffer.

I'm not saying the 64MB alignment is not correct (I don't know what is 
correct and assuming the Xorg driver was tested with real cards it's 
possible this assumption holds) but maybe it comes from having a 64MB VRAM 
BAR that contains twice the size of actual VRAM including the AGP window 
which also satisfies the 32MB alignment for VRAM, so the commit message 
may nees to be adjusted to say that instead of something not supported by 
documentation. Do you have any PCI cards? There were PCI versions of 
these. I wonder if they also have the same VRAM BAR. If not then no 
problem, we can go with what works for known drivers.

Regards,
BALATON Zoltan

>> Can you check what the CONFIG_APER_SIZE register contains on these cards?
>> Do all Rage 128 (and Pro) cards have 64MB and Radeon 7xxx/M6 have 128MB?
>> The documentation is again not clear on this because it lists default
>> value of 0x2000000 for CONFIG_APER_SIZE on Rage 128 Pro and nothing for
>> Radeon but in a figure it shows this should contain both VRAM and AGP
>> areas that suggests 64MB but it's possible that the documentation is
>> wrong.
>>
>
> I will take a look at this!
>
>>
>> This was changed in commit f7ecde051dd73 to be half the memory size
>> because at least a Radeon FCocde ROM seemed to detect VRAM size that way.
>> I should test that again but it needed OpenBIOS patches that were not
>> merged so I had to find those again as I don't have that setup any more.
>> Checking on real card may be the best source of what this should be but I
>> think this might break that FCode ROM which is from a PowerMac card. This
>> suggests it should be half the aper size.
>>
>
> Ah, great, this is important context. I'll look at this more
> closely and check the real hardware.
>
>>
>> Coding style says variables should be defined at the beginning of blocks
>> so in this case at the beginning of the funcion.
>>
>
> I'll also address all of the style comments.
>
> Thanks again,
> Chad
>
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes
  2025-10-13 16:24     ` BALATON Zoltan
@ 2025-10-13 20:04       ` Chad Jablonski
  2025-10-14 23:26         ` BALATON Zoltan
  0 siblings, 1 reply; 10+ messages in thread
From: Chad Jablonski @ 2025-10-13 20:04 UTC (permalink / raw)
  To: BALATON Zoltan, Chad Jablonski
  Cc: qemu-devel, Gerd Hoffmann, marcandre.lureau

>
> I'm not saying the 64MB alignment is not correct (I don't know what is 
> correct and assuming the Xorg driver was tested with real cards it's 
> possible this assumption holds) but maybe it comes from having a 64MB VRAM 
> BAR that contains twice the size of actual VRAM including the AGP window 
> which also satisfies the 32MB alignment for VRAM, so the commit message 
> may nees to be adjusted to say that instead of something not supported by 
> documentation. Do you have any PCI cards? There were PCI versions of 
> these. I wonder if they also have the same VRAM BAR. If not then no 
> problem, we can go with what works for known drivers.
>

Unfortunately I don't have a PCI Rage 128. They actually seem to be somewhat
rare compared to the AGP versions!

Testing results from two Rage 128 AGP cards:

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


* PCI BAR0 (Region 0) is fixed at 64MB for both cards, regardless of actual
  VRAM size.

* CONFIG_MEMSIZE correctly reports actual VRAM size (32MB and 16MB).

* CONFIG_APER_SIZE is fixed at 32MB (0x02000000) on both cards, regardless
  of:
   - Actual VRAM size
   - AGP enabled/disabled status
   - AGP_APER_SIZE configuration (tested with 8MB AGP enabled)

   This directly contradicts the Rage 128 Pro Register Reference Guide
   (pg. 3-202) which states: "Size of linear apertures (both 0 and 1).
   This includes both the frame buffer image and the AGP system memory
   image area."

* AGP_APER_OFFSET is also fixed at 32MB (0x02000000) on both cards. On the
  16MB card, this creates a 16MB gap between the end of framebuffer and the
  start of the AGP window.

So for Rage 128, CONFIG_APER_SIZE should be set to a fixed 32MB value, not
dynamically calculated from CONFIG_MEMSIZE + AGP space as the documentation
suggests and as was previously implemented by me 😬.

I'm less sure about the Radeon and your additional context around the Radeon
on PowerPC makes me very nervous to change it right now. Especially given I
have a single example of that card and it's for a PC. I think it makes more
sense to leave the behavior of CONFIG_APER_SIZE for that card unchanged.

I will send a v2 patch that:
  * Sets Rage 128 CONFIG_APER_SIZE to 32MB
  * Updates the commit message to reflect that
  * Leaves Radeon unchanged (half of the vram_size)
  * Addresses the style issues



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes
  2025-10-13 20:04       ` Chad Jablonski
@ 2025-10-14 23:26         ` BALATON Zoltan
  2025-10-15 14:34           ` Chad Jablonski
  0 siblings, 1 reply; 10+ messages in thread
From: BALATON Zoltan @ 2025-10-14 23:26 UTC (permalink / raw)
  To: Chad Jablonski; +Cc: qemu-devel, Gerd Hoffmann, marcandre.lureau

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

On Mon, 13 Oct 2025, Chad Jablonski wrote:
>> I'm not saying the 64MB alignment is not correct (I don't know what is
>> correct and assuming the Xorg driver was tested with real cards it's
>> possible this assumption holds) but maybe it comes from having a 64MB VRAM
>> BAR that contains twice the size of actual VRAM including the AGP window
>> which also satisfies the 32MB alignment for VRAM, so the commit message
>> may nees to be adjusted to say that instead of something not supported by
>> documentation. Do you have any PCI cards? There were PCI versions of
>> these. I wonder if they also have the same VRAM BAR. If not then no
>> problem, we can go with what works for known drivers.
>>
>
> Unfortunately I don't have a PCI Rage 128. They actually seem to be somewhat
> rare compared to the AGP versions!
>
> Testing results from two Rage 128 AGP cards:
>
> 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
>
>
> * PCI BAR0 (Region 0) is fixed at 64MB for both cards, regardless of actual
>  VRAM size.
>
> * CONFIG_MEMSIZE correctly reports actual VRAM size (32MB and 16MB).
>
> * CONFIG_APER_SIZE is fixed at 32MB (0x02000000) on both cards, regardless
>  of:
>   - Actual VRAM size
>   - AGP enabled/disabled status
>   - AGP_APER_SIZE configuration (tested with 8MB AGP enabled)
>
>   This directly contradicts the Rage 128 Pro Register Reference Guide
>   (pg. 3-202) which states: "Size of linear apertures (both 0 and 1).
>   This includes both the frame buffer image and the AGP system memory
>   image area."
>
> * AGP_APER_OFFSET is also fixed at 32MB (0x02000000) on both cards. On the
>  16MB card, this creates a 16MB gap between the end of framebuffer and the
>  start of the AGP window.

This matches the docs in which a figure shows area after CONFIG_MEMSIZE as 
VRAM extension area so 32MB is reserved for VRAM even if there's less on 
the card and the AGP window starts after that.

> So for Rage 128, CONFIG_APER_SIZE should be set to a fixed 32MB value, not
> dynamically calculated from CONFIG_MEMSIZE + AGP space as the documentation
> suggests and as was previously implemented by me 😬.
>
> I'm less sure about the Radeon and your additional context around the Radeon
> on PowerPC makes me very nervous to change it right now. Especially given I
> have a single example of that card and it's for a PC. I think it makes more
> sense to leave the behavior of CONFIG_APER_SIZE for that card unchanged.

Question is if Radeon has BAR0 matching VRAM size because in that case if 
Rage128 has fixed 64MB and Radeon has size of VRAM for BAR0 then 
CONFIG_APER_SIZE can be half of BAR0 which would work for both. But if 
Radeon also has a fixed size BAR0 larger than actual VRAM (mathching max 
supported VRAM instead then current calculation is needed using VRAM size 
for radeon according to the FCode ROM I've tested. Changing 
CONFIG_APER_SIZE only for Rage128 should not break anything as I did not 
see anything using that so that would also work if we can't find out what 
Radeon has.

Regards,
BALATON Zoltan

> I will send a v2 patch that:
>  * Sets Rage 128 CONFIG_APER_SIZE to 32MB
>  * Updates the commit message to reflect that
>  * Leaves Radeon unchanged (half of the vram_size)
>  * Addresses the style issues
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes
  2025-10-14 23:26         ` BALATON Zoltan
@ 2025-10-15 14:34           ` Chad Jablonski
  2025-10-15 23:51             ` BALATON Zoltan
  0 siblings, 1 reply; 10+ messages in thread
From: Chad Jablonski @ 2025-10-15 14:34 UTC (permalink / raw)
  To: BALATON Zoltan, Chad Jablonski
  Cc: qemu-devel, Gerd Hoffmann, marcandre.lureau

>
> Question is if Radeon has BAR0 matching VRAM size because in that case if 
> Rage128 has fixed 64MB and Radeon has size of VRAM for BAR0 then 
> CONFIG_APER_SIZE can be half of BAR0 which would work for both. But if 
> Radeon also has a fixed size BAR0 larger than actual VRAM (mathching max 
> supported VRAM instead then current calculation is needed using VRAM size 
> for radeon according to the FCode ROM I've tested. Changing 
> CONFIG_APER_SIZE only for Rage128 should not break anything as I did not 
> see anything using that so that would also work if we can't find out what 
> Radeon has.
>

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

Looking at the linux source it appears the R100 doesn't have an
AGP_APER_OFFSET register. The register at that offset according to the
linux driver is something else entirely. This suggests that
the R100 doesn't map AGP memory space into BAR0 in the way that the
Rage 128 does. It makes sense that the R128's BAR0 would be twice the
max memory for the architecture given that it also has to do this AGP
mapping. But if the R100 _doesn't_ then it wouldn't need to make room
for AGP and it may just be that the BAR0 is the size of the max memory.
I don't have documentation for the R100 though so it's tough to know for sure.

But to answer your question about the BAR0 matching VRAM on the R100.
At least for this card it does not. The VRAM is half of the BAR0. I have a
32MB Radeon arriving soon so I'll be able to test that to see if it also
follows that pattern or if BAR0 is still 128MB. From what I'm seeing there
may have been some 128MB R100's, it's not entirely clear to me. So it's
possible that the 128MB is the max VRAM.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes
  2025-10-15 14:34           ` Chad Jablonski
@ 2025-10-15 23:51             ` BALATON Zoltan
  2025-10-17 13:56               ` Chad Jablonski
  0 siblings, 1 reply; 10+ messages in thread
From: BALATON Zoltan @ 2025-10-15 23:51 UTC (permalink / raw)
  To: Chad Jablonski; +Cc: qemu-devel, Gerd Hoffmann, marcandre.lureau

On Wed, 15 Oct 2025, Chad Jablonski wrote:
>> Question is if Radeon has BAR0 matching VRAM size because in that case if
>> Rage128 has fixed 64MB and Radeon has size of VRAM for BAR0 then
>> CONFIG_APER_SIZE can be half of BAR0 which would work for both. But if
>> Radeon also has a fixed size BAR0 larger than actual VRAM (mathching max
>> supported VRAM instead then current calculation is needed using VRAM size
>> for radeon according to the FCode ROM I've tested. Changing
>> CONFIG_APER_SIZE only for Rage128 should not break anything as I did not
>> see anything using that so that would also work if we can't find out what
>> Radeon has.
>>
>
> 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

For this R100 card APER_SIZE matches MEMSIZE but the BAR length is twice 
that. Maybe the other card you get will shed some light on what's going on 
with Radeon. I've looked up what the FCode ROM I've tested was doing. This 
is from a Card#109-85500-00 Rom#113-85501-226 according to the IDs it sets 
in the device tree, probably from a PowerMac but I don't have the card 
just found this ROM. The detokenized part of this Radeon 7000/RV100 ROM 
that accesses CONFIG_APER_SIZE looks like this:

   7321: const_REG_CONFIG_APER_SIZE
   7323: ati-reg-l@
   7325: dup
   7326: b(to) var_aper_size
   7329: 2*
   7330: b(to) var_ram_size
   7333: const_REG_CONFIG_REG_APER_SIZE
   7335: ati-reg-l@
   7337: 2*
   7338: b(to) var_mmio_size

The variable names were invented by me so it's possible that it actually 
means vram and mmio BAR size but it does 2* for both the VRAM and REG 
APERS and this is the size it then uses for map-in and map-out calls so 
this suggests these registers are half of the BAR size at least for this 
card.

> Looking at the linux source it appears the R100 doesn't have an
> AGP_APER_OFFSET register. The register at that offset according to the
> linux driver is something else entirely. This suggests that
> the R100 doesn't map AGP memory space into BAR0 in the way that the
> Rage 128 does. It makes sense that the R128's BAR0 would be twice the
> max memory for the architecture given that it also has to do this AGP
> mapping. But if the R100 _doesn't_ then it wouldn't need to make room
> for AGP and it may just be that the BAR0 is the size of the max memory.
> I don't have documentation for the R100 though so it's tough to know for sure.
>
> But to answer your question about the BAR0 matching VRAM on the R100.
> At least for this card it does not. The VRAM is half of the BAR0. I have a
> 32MB Radeon arriving soon so I'll be able to test that to see if it also
> follows that pattern or if BAR0 is still 128MB. From what I'm seeing there
> may have been some 128MB R100's, it's not entirely clear to me. So it's
> possible that the 128MB is the max VRAM.

I only have a Rage Mobility M6 Design Guide (M6 is another name for RV100) 
that talks about memory size and it says that it's minimum 8MB maximum 
64MB but could be R100 supported more. It's still possible that the BAR 
size is twice the VRAM size for some reason but we don't have enough 
evidence for that.

Regards,
BALATON Zoltan


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes
  2025-10-15 23:51             ` BALATON Zoltan
@ 2025-10-17 13:56               ` Chad Jablonski
  0 siblings, 0 replies; 10+ messages in thread
From: Chad Jablonski @ 2025-10-17 13:56 UTC (permalink / raw)
  To: BALATON Zoltan, Chad Jablonski
  Cc: qemu-devel, Gerd Hoffmann, marcandre.lureau

>
> For this R100 card APER_SIZE matches MEMSIZE but the BAR length is twice 
> that. Maybe the other card you get will shed some light on what's going on 
> with Radeon. I've looked up what the FCode ROM I've tested was doing. This 
> is from a Card#109-85500-00 Rom#113-85501-226 according to the IDs it sets 
> in the device tree, probably from a PowerMac but I don't have the card 
> just found this ROM. The detokenized part of this Radeon 7000/RV100 ROM 
> that accesses CONFIG_APER_SIZE looks like this:
>
>    7321: const_REG_CONFIG_APER_SIZE
>    7323: ati-reg-l@
>    7325: dup
>    7326: b(to) var_aper_size
>    7329: 2*
>    7330: b(to) var_ram_size
>    7333: const_REG_CONFIG_REG_APER_SIZE
>    7335: ati-reg-l@
>    7337: 2*
>    7338: b(to) var_mmio_size
>
> The variable names were invented by me so it's possible that it actually 
> means vram and mmio BAR size but it does 2* for both the VRAM and REG 
> APERS and this is the size it then uses for map-in and map-out calls so 
> this suggests these registers are half of the BAR size at least for this 
> card.
>

I got the card! It looks like it was exactly as you suspected and I
think this matches with what you're seeing in the FCode. The
CONFIG_APER_SIZE is half of BAR0 for both Rage 128 and R100:

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
Radeon RV100 QY [Radeon 7000/VE]  32MB    128MB       0x02000000      0x04000000        N/A

>
> I only have a Rage Mobility M6 Design Guide (M6 is another name for RV100) 
> that talks about memory size and it says that it's minimum 8MB maximum 
> 64MB but could be R100 supported more. It's still possible that the BAR 
> size is twice the VRAM size for some reason but we don't have enough 
> evidence for that.
>

Oh! I didn't realize that! That's good to know, I'll dig that up.

I'll include what I learned from the new card and send along a v3 shortly.

- Chad


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-10-17 13:57 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-01  3:46 [PATCH] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes Chad Jablonski
2025-10-07 20:52 ` BALATON Zoltan
2025-10-07 20:57   ` BALATON Zoltan
2025-10-13 14:52   ` Chad Jablonski
2025-10-13 16:24     ` BALATON Zoltan
2025-10-13 20:04       ` Chad Jablonski
2025-10-14 23:26         ` BALATON Zoltan
2025-10-15 14:34           ` Chad Jablonski
2025-10-15 23:51             ` BALATON Zoltan
2025-10-17 13:56               ` Chad Jablonski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).