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

BALATON Zoltan posted 3 patches 1 year, 1 month ago
There is a newer version of this series
[PATCH 1/3] ati-vga: Fix aperture sizes
Posted by BALATON Zoltan 1 year, 1 month ago
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
Re: [PATCH 1/3] ati-vga: Fix aperture sizes
Posted by Marc-André Lureau 1 year ago
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 1/3] ati-vga: Fix aperture sizes
Posted by BALATON Zoltan 1 year ago
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
>>
>>
>
>
>
Re: [PATCH 1/3] ati-vga: Fix aperture sizes
Posted by BALATON Zoltan 1 year ago
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
>>
>>
>
>
>