Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210519135723.525997-1-kai.heng.feng@canonical.com> <CADnq5_OstgUmLxa4DU2s19m5E1zScKY0Pe=644BvBTp=LAPvJg@mail.gmail.com>
In-Reply-To: <CADnq5_OstgUmLxa4DU2s19m5E1zScKY0Pe=644BvBTp=LAPvJg@mail.gmail.com>
From:   Alex Deucher <alexdeucher@gmail.com>
Date:   Thu, 20 May 2021 14:58:52 -0400
Message-ID: <CADnq5_MdCHrCKV6sjSwbknscDh1pUm-OMzxmFSwhnnDpZcTeUw@mail.gmail.com>
Subject: Re: [PATCH] vgaarb: Use ACPI HID name to find integrated GPU
To:     Kai-Heng Feng <kai.heng.feng@canonical.com>
Cc:     Dave Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Maxime Ripard <mripard@kernel.org>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        "Deucher, Alexander" <alexander.deucher@amd.com>,
        open list <linux-kernel@vger.kernel.org>,
        "open list:DRM DRIVERS" <dri-devel@lists.freedesktop.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Pushed to drm-misc-next.  Thanks!

Alex

On Wed, May 19, 2021 at 12:45 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Wed, May 19, 2021 at 9:57 AM Kai-Heng Feng
> <kai.heng.feng@canonical.com> wrote:
> >
> > Commit 3d42f1ddc47a ("vgaarb: Keep adding VGA device in queue") assumes
> > the first device is an integrated GPU. However, on AMD platforms an
> > integrated GPU can have higher PCI device number than a discrete GPU.
> >
> > Integrated GPU on ACPI platform generally has _DOD and _DOS method, so
> > use that as predicate to find integrated GPU. If the new strategy
> > doesn't work, fallback to use the first device as boot VGA.
> >
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
> Unless there are any other comments, I'll apply it tomorrow.
>
> Alex
>
> > ---
> >  drivers/gpu/vga/vgaarb.c | 31 ++++++++++++++++++++++++++-----
> >  1 file changed, 26 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
> > index 5180c5687ee5..949fde433ea2 100644
> > --- a/drivers/gpu/vga/vgaarb.c
> > +++ b/drivers/gpu/vga/vgaarb.c
> > @@ -50,6 +50,7 @@
> >  #include <linux/screen_info.h>
> >  #include <linux/vt.h>
> >  #include <linux/console.h>
> > +#include <linux/acpi.h>
> >
> >  #include <linux/uaccess.h>
> >
> > @@ -1450,9 +1451,23 @@ static struct miscdevice vga_arb_device = {
> >         MISC_DYNAMIC_MINOR, "vga_arbiter", &vga_arb_device_fops
> >  };
> >
> > +#if defined(CONFIG_ACPI)
> > +static bool vga_arb_integrated_gpu(struct device *dev)
> > +{
> > +       struct acpi_device *adev = ACPI_COMPANION(dev);
> > +
> > +       return adev && !strcmp(acpi_device_hid(adev), ACPI_VIDEO_HID);
> > +}
> > +#else
> > +static bool vga_arb_integrated_gpu(struct device *dev)
> > +{
> > +       return false;
> > +}
> > +#endif
> > +
> >  static void __init vga_arb_select_default_device(void)
> >  {
> > -       struct pci_dev *pdev;
> > +       struct pci_dev *pdev, *found = NULL;
> >         struct vga_device *vgadev;
> >
> >  #if defined(CONFIG_X86) || defined(CONFIG_IA64)
> > @@ -1505,20 +1520,26 @@ static void __init vga_arb_select_default_device(void)
> >  #endif
> >
> >         if (!vga_default_device()) {
> > -               list_for_each_entry(vgadev, &vga_list, list) {
> > +               list_for_each_entry_reverse(vgadev, &vga_list, list) {
> >                         struct device *dev = &vgadev->pdev->dev;
> >                         u16 cmd;
> >
> >                         pdev = vgadev->pdev;
> >                         pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> >                         if (cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
> > -                               vgaarb_info(dev, "setting as boot device (VGA legacy resources not available)\n");
> > -                               vga_set_default_device(pdev);
> > -                               break;
> > +                               found = pdev;
> > +                               if (vga_arb_integrated_gpu(dev))
> > +                                       break;
> >                         }
> >                 }
> >         }
> >
> > +       if (found) {
> > +               vgaarb_info(&found->dev, "setting as boot device (VGA legacy resources not available)\n");
> > +               vga_set_default_device(found);
> > +               return;
> > +       }
> > +
> >         if (!vga_default_device()) {
> >                 vgadev = list_first_entry_or_null(&vga_list,
> >                                                   struct vga_device, list);
> > --
> > 2.31.1
> >