Hi Thomas,

On Thu, 11 Jun 2020 at 09:28, Thomas Zimmermann <tzimmerm...@suse.de> wrote:

> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -59,7 +59,6 @@ static struct drm_driver driver;
>  static const struct pci_device_id pciidlist[] = {
>         AST_VGA_DEVICE(PCI_CHIP_AST2000, NULL),
>         AST_VGA_DEVICE(PCI_CHIP_AST2100, NULL),
> -       /*      AST_VGA_DEVICE(PCI_CHIP_AST1180, NULL), - don't bind to 1180 
> for now */

Since we don't bind to this pciid, the (removed) code is never
used/dead. For the series:
Reviewed-by: Emil Velikov <emil.l.veli...@gmail.com>

Small idea below: feel free to ignore or if you agree - follow-up at a
random point in the future.


> +       if (dev->pdev->revision >= 0x40) {
> +               ast->chip = AST2500;
> +               DRM_INFO("AST 2500 detected\n");
> +       } else if (dev->pdev->revision >= 0x30) {
> +               ast->chip = AST2400;
> +               DRM_INFO("AST 2400 detected\n");
> +       } else if (dev->pdev->revision >= 0x30) {
> +               ast->chip = AST2400;
> +               DRM_INFO("AST 2400 detected\n");
> +       } else if (dev->pdev->revision >= 0x20) {
> +               ast->chip = AST2300;
> +               DRM_INFO("AST 2300 detected\n");
> +       } else if (dev->pdev->revision >= 0x10) {
> +               switch (scu_rev & 0x0300) {
> +               case 0x0200:
> +                       ast->chip = AST1100;
> +                       DRM_INFO("AST 1100 detected\n");
> +                       break;
> +               case 0x0100:
> +                       ast->chip = AST2200;
> +                       DRM_INFO("AST 2200 detected\n");
> +                       break;
> +               case 0x0000:
> +                       ast->chip = AST2150;
> +                       DRM_INFO("AST 2150 detected\n");
> +                       break;
> +               default:
> +                       ast->chip = AST2100;
> +                       DRM_INFO("AST 2100 detected\n");
> +                       break;
>                 }
> +               ast->vga2_clone = false;
> +       } else {
> +               ast->chip = AST2000;
> +               DRM_INFO("AST 2000 detected\n");
>         }
>
Instead of the above if/else spaghetti, one can use something alike

static const struct foo {
    u8 rev_maj; // revision & 0xf0 >> 4
    u8 rev_scu; // scu_rev & 0x0300 >> 8, ignored if table has 0xf
    enum ast_chip;
    const char *name;
} bar {
   { 0x3, 0xf, AST2400, "2400" },
   { 0x2, 0xf, AST2300, "2300" },
   { 0x1, 0x3, AST2100, "2100" },
   { 0x1, 0x2, AST1100, "1100" },
   { 0x1, 0x1, AST2200, "2200" },
   { 0x1, 0x0, AST2150, "2150" },
   { 0x0, 0xf, AST2000, "2000" },
};

+ trivial loop.

-Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to