* [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; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ 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
0 siblings, 0 replies; 6+ 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] 6+ messages in thread
end of thread, other threads:[~2025-10-13 20:05 UTC | newest]
Thread overview: 6+ 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
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).