Re: [PATCH RFC] drm/radeon: combios tables on older cards

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Jul 21, 2013 at 11:21 AM, Mark Kettenis <mark.kettenis@xxxxxxxxx> 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@xxxxxxxxxxx>
>
>
> 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux