[PATCH RFC] drm/radeon: combios tables on older cards
Alex Deucher
alexdeucher at gmail.com
Sun Jul 21 13:46:46 PDT 2013
On Sun, Jul 21, 2013 at 11:21 AM, Mark Kettenis <mark.kettenis at xs4all.nl> wrote:
> Noticed that my old Radeon 7500 hung after printing
>
> drm: GPU not posted. posting now...
>
> when it wasn't selected as the primary card the BIOS. Some digging
> revealed that it was hanging in combios_parse_mmio_table() while
> parsing the ASIC INIT 3 table. Looking at the BIOS ROM for the card,
> it becomes obvious that there is no ASIC INIT 3 table in the BIOS.
> The code is just processing random garbage. No surprise it hangs!
>
> Why do I say that there is no ASIC INIT 3 table is the BIOS? This
> table is found through the MISC INFO table. The MISC INFO table can
> be found at offset 0x5e in the COMBIOS header. But the header is
> smaller than that. The COMBIOS header starts at offset 0x126. The
> standard PCI Data Structure (the bit that starts with 'PCIR') lives at
> offset 0x180. That means that the COMBIOS header can not be larger
> than 0x5a bytes and therefore cannot contain a MISC INFO table.
>
> I looked at a dozen or so BIOS images, some my own, some downloaded from:
>
> <http://www.techpowerup.com/vgabios/index.php?manufacturer=ATI&page=1>
>
> It is fairly obvious that the size of the COMBIOS header can be found
> at offset 0x6 of the header. Not sure if it is a 16-bit number or
> just an 8-bit number, but that doesn't really matter since the tables
> seems to be always smaller than 256 bytes.
>
> So I think combios_get_table_offset() should check if the requested
> table is present. This can be done by checking the offset against the
> size of the header. See the diff below. The diff is against the WIP
> OpenBSD codebase that roughly corresponds to Linux 3.8.13 at this
> point. But I don't think this bit of the code changed much since
> then.
Your assessment is correct. I've gone ahead and applied the patch.
Thanks!
Alex
>
> For what it is worth:
>
> Signed-off-by: Mark Kettenis <kettenis at openbsd.org>
>
>
> diff --git a/sys/dev/pci/drm/radeon/radeon_combios.c b/sys/dev/pci/drm/radeon/radeon_combios.c
> index c2b3535..81c3f00 100644
> --- a/sys/dev/pci/drm/radeon/radeon_combios.c
> +++ b/sys/dev/pci/drm/radeon/radeon_combios.c
> @@ -144,7 +144,7 @@ static uint16_t combios_get_table_offset(struct drm_device *dev,
> enum radeon_combios_table_offset table)
> {
> struct radeon_device *rdev = dev->dev_private;
> - int rev;
> + int rev, size;
> uint16_t offset = 0, check_offset;
>
> if (!rdev->bios)
> @@ -153,174 +153,106 @@ static uint16_t combios_get_table_offset(struct drm_device *dev,
> switch (table) {
> /* absolute offset tables */
> case COMBIOS_ASIC_INIT_1_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0xc);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0xc;
> break;
> case COMBIOS_BIOS_SUPPORT_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x14);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x14;
> break;
> case COMBIOS_DAC_PROGRAMMING_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x2a);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x2a;
> break;
> case COMBIOS_MAX_COLOR_DEPTH_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x2c);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x2c;
> break;
> case COMBIOS_CRTC_INFO_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x2e);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x2e;
> break;
> case COMBIOS_PLL_INFO_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x30);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x30;
> break;
> case COMBIOS_TV_INFO_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x32);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x32;
> break;
> case COMBIOS_DFP_INFO_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x34);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x34;
> break;
> case COMBIOS_HW_CONFIG_INFO_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x36);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x36;
> break;
> case COMBIOS_MULTIMEDIA_INFO_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x38);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x38;
> break;
> case COMBIOS_TV_STD_PATCH_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x3e);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x3e;
> break;
> case COMBIOS_LCD_INFO_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x40);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x40;
> break;
> case COMBIOS_MOBILE_INFO_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x42);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x42;
> break;
> case COMBIOS_PLL_INIT_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x46);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x46;
> break;
> case COMBIOS_MEM_CONFIG_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x48);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x48;
> break;
> case COMBIOS_SAVE_MASK_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x4a);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x4a;
> break;
> case COMBIOS_HARDCODED_EDID_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x4c);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x4c;
> break;
> case COMBIOS_ASIC_INIT_2_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x4e);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x4e;
> break;
> case COMBIOS_CONNECTOR_INFO_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x50);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x50;
> break;
> case COMBIOS_DYN_CLK_1_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x52);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x52;
> break;
> case COMBIOS_RESERVED_MEM_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x54);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x54;
> break;
> case COMBIOS_EXT_TMDS_INFO_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x58);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x58;
> break;
> case COMBIOS_MEM_CLK_INFO_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x5a);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x5a;
> break;
> case COMBIOS_EXT_DAC_INFO_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x5c);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x5c;
> break;
> case COMBIOS_MISC_INFO_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x5e);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x5e;
> break;
> case COMBIOS_CRT_INFO_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x60);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x60;
> break;
> case COMBIOS_INTEGRATED_SYSTEM_INFO_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x62);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x62;
> break;
> case COMBIOS_COMPONENT_VIDEO_INFO_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x64);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x64;
> break;
> case COMBIOS_FAN_SPEED_INFO_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x66);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x66;
> break;
> case COMBIOS_OVERDRIVE_INFO_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x68);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x68;
> break;
> case COMBIOS_OEM_INFO_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x6a);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x6a;
> break;
> case COMBIOS_DYN_CLK_2_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x6c);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x6c;
> break;
> case COMBIOS_POWER_CONNECTOR_INFO_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x6e);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x6e;
> break;
> case COMBIOS_I2C_INFO_TABLE:
> - check_offset = RBIOS16(rdev->bios_header_start + 0x70);
> - if (check_offset)
> - offset = check_offset;
> + check_offset = 0x70;
> break;
> /* relative offset tables */
> case COMBIOS_ASIC_INIT_3_TABLE: /* offset from misc info */
> @@ -439,8 +371,11 @@ static uint16_t combios_get_table_offset(struct drm_device *dev,
> break;
> }
>
> - return offset;
> + size = RBIOS8(rdev->bios_header_start + 0x6);
> + if (table < COMBIOS_ASIC_INIT_3_TABLE && check_offset < size)
> + offset = RBIOS16(rdev->bios_header_start + check_offset);
>
> + return offset;
> }
>
> bool radeon_combios_check_hardcoded_edid(struct radeon_device *rdev)
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
More information about the dri-devel
mailing list