1. Patchew
  2. Seabios
  3. View series

[SeaBIOS] Re: [PATCH] ati-vga: Fix bios tables for radeon KMS driver

Kevin O'Connor via SeaBIOS posted 1 patch 2 days, 13 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/seabios tags/patchew/amEAuJhnWM7v5cMh@morn
[SeaBIOS] Re: [PATCH] ati-vga: Fix bios tables for radeon KMS driver
Posted by Kevin O'Connor via SeaBIOS 2 days, 13 hours ago 
On Tue, Jul 21, 2026 at 10:46:58PM -0400, Chad Jablonski wrote:
> This fixes two issues when running ati-vga with model=rv100 under QEMU
> using a KMS driver.
> 
> First, the radeon KMS driver does an additional check when looking up COMBIOS
> table entries. It checks a header size field to validate that the entry
> falls in the header region. Prior to this patch that size field is zero
> which causes the driver to fallback to another method (CLOCK_CNTL
> registers) also resulting in bad values and a division by zero.
> 
> 0x60 matches what I've observed in the bios of a Radeon QY (RV100)
> (PCI 1002:5159). But I don't think the value is actually important
> other than needing to be large enough to encompass the table entries
> used here.
> 
> Second, the connector info section needs a non-zero value in the DDC
> type field. Zero causes the radeon driver to fail to initialize the DDC
> bus and then no connectors are found. DDC_DVI (0x2) is used here so that
> the GPIO_DVI_DDC register is used for the radeon. From what I have seen
> r128 drivers ignore this field entirely.
> 
> Tested on Debian Squeeze (Linux 2.6.32) and Debian Trixie (Linux 6.12.94).
> 
> Signed-off-by: Chad Jablonski <chad@jablonski.xyz>

Thanks.  It seems the ati-tables.S code was introduced by Gerd, so
hopefully he'll be able to review.

That said, is there any reason why this code needs to be in assembler?
It should be simpler to maintain the code if we can use C code
instead.  Perhaps something like the below (untested).  I'm just
guessing on table sizes, alignments, and field names so the example
could likely be improved.

-Kevin


commit eb5ccb59b40dceedec8b381df4a6b6b7cb4193f5
Author: Kevin O'Connor <kevin@koconnor.net>
Date:   Wed Jul 22 13:12:58 2026 -0400

    atiext: Implement ATI tables in atiext.c
    
    Move the definitions in ati-tables.S to atiext.c .
    
    Signed-off-by: Kevin O'Connor <kevin@koconnor.net>

diff --git a/vgasrc/ati-tables.S b/vgasrc/ati-tables.S
deleted file mode 100644
index cdbde2f..0000000
--- a/vgasrc/ati-tables.S
+++ /dev/null
@@ -1,43 +0,0 @@
-//
-// Fake ati bios tables.
-//
-// aty128fb and radeonfb try to gather informations from these tables,
-// so add some stuff here to make the drivers happy.  Specifically
-// radeonfb needs the pll information, otherwise it'll crash with a
-// division by zero ...
-//
-	.org 0x48
-	.word _ati_main
-
-	// main info
-	.org 0x50
-_ati_main:
-	.org 0x50 + 0x30
-	.word _ati_pll
-	.org 0x50 + 0x50
-	.word _ati_connector
-
-	// pll info
-	.org 0x100
-_ati_pll:
-	.word	0		// ??? (not used by radeonfb)
-	.word	0
-	.word	0
-	.word	0
-	.word	23000		// sclk
-	.word	23000		// mclk
-	.word	0
-	.word	2700		// ref_clk
-	.word	4		// ref_div
-	.long	12000		// ppll_min
-	.long	35000		// ppll_max
-
-	// connector info
-	.org 0x140
-_ati_connector:
-	.byte 0x10		// one chip
-	.byte 0x01		// one connector
-	.word 0x3000		// type DVI-I
-	.word 0			// end of list
-
-	.org 0x200
diff --git a/vgasrc/atiext.c b/vgasrc/atiext.c
index 69dfd46..f003eea 100644
--- a/vgasrc/atiext.c
+++ b/vgasrc/atiext.c
@@ -338,6 +338,65 @@ static void ati_i2c_edid_rage128(void)
     dprintf(1, "ati: ... %s\n", valid ? "good" : "invalid");
 }
 
+/****************************************************************
+ * Fake ati bios tables
+ ****************************************************************/
+
+// aty128fb and radeonfb try to gather informations from these tables,
+// so add some stuff here to make the drivers happy.  Specifically
+// radeonfb needs the pll information, otherwise it'll crash with a
+// division by zero ...
+struct ati_main_table_s {
+    u8 pad_0[48];
+    u16 pll_table;
+    u8 pad_50[30];
+    u16 connector_table;
+    u8 pad_82[94];
+} PACKED;
+
+struct ati_pll_table_s {
+    u8 pad_0[8];
+    u16 sclk;
+    u16 mclk;
+    u8 pad_12[2];
+    u16 ref_clk;
+    u16 ref_div;
+    u32 ppll_min;
+    u32 ppll_max;
+    u8 pad_26[38];
+} PACKED;
+
+struct ati_connector_table_s {
+    u8 num_chip;
+    u8 num_connector;
+    u16 flags;
+    u16 next;
+} PACKED;
+
+extern u16 _rom_header_ati_table_anchor;
+static struct ati_main_table_s ati_main_table VAR16 __aligned(16);
+static struct ati_pll_table_s ati_pll_table VAR16 __aligned(16) = {
+    .sclk = 23000,
+    .mclk = 23000,
+    .ref_clk = 2700,
+    .ref_div = 4,
+    .ppll_min = 12000,
+    .ppll_max = 35000,
+};
+static struct ati_connector_table_s ati_connector_table VAR16 __aligned(16) = {
+    .num_chip = 0x10,
+    .num_connector = 1,
+    .flags = 0x3000,
+};
+
+static void
+ati_table_setup(void)
+{
+    SET_VGA(ati_main_table.pll_table, (u32)&ati_pll_table);
+    SET_VGA(ati_main_table.connector_table, (u32)&ati_connector_table);
+    SET_VGA(_rom_header_ati_table_anchor, (u32)&ati_main_table);
+}
+
 /****************************************************************
  * init
  ****************************************************************/
@@ -379,6 +438,8 @@ ati_setup(void)
     SET_VGA(VBE_total_memory, totalmem);
     SET_VGA(ati_io_addr, io_addr);
 
+    ati_table_setup();
+
     // Validate modes
     struct generic_svga_mode *m = svga_modes;
     unsigned int mcount = GET_GLOBAL(svga_mcount);
diff --git a/vgasrc/vgaentry.S b/vgasrc/vgaentry.S
index f9624fc..14e557a 100644
--- a/vgasrc/vgaentry.S
+++ b/vgasrc/vgaentry.S
@@ -41,7 +41,11 @@ _rom_header_signature:
         .asciz "IBM"
 
 #if CONFIG_VGA_ATI
-#include "ati-tables.S"
+	.org 0x48
+        .global _rom_header_ati_table_anchor
+_rom_header_ati_table_anchor:
+	.word 0
+        .align 16
 #endif
 
 /****************************************************************

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH] ati-vga: Fix bios tables for radeon KMS driver
Posted by Gerd Hoffmann via SeaBIOS 2 days, 2 hours ago 
  Hi,

> Thanks.  It seems the ati-tables.S code was introduced by Gerd, so
> hopefully he'll be able to review.

Changes look sensible to me.

> That said, is there any reason why this code needs to be in assembler?

I don't remember.  Maybe to have the linker handle the table references,
so they are set at compile time ...

> +static void
> +ati_table_setup(void)
> +{
> +    SET_VGA(ati_main_table.pll_table, (u32)&ati_pll_table);
> +    SET_VGA(ati_main_table.connector_table, (u32)&ati_connector_table);
> +    SET_VGA(_rom_header_ati_table_anchor, (u32)&ati_main_table);
> +}

... instead of runtime.

I have no objections to move this to C though.

take care,
  Gerd

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH] ati-vga: Fix bios tables for radeon KMS driver
Posted by Chad Jablonski 2 days, 4 hours ago 
>
> That said, is there any reason why this code needs to be in assembler?
> It should be simpler to maintain the code if we can use C code
> instead.  Perhaps something like the below (untested).  I'm just
> guessing on table sizes, alignments, and field names so the example
> could likely be improved.
>

Let me see if I can get this in C. Thanks for taking a look and thank you
for the massive head start.
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.