Re: [PATCH 3/5] drm/nouveau: Don't register Thunderbolt eGPU with vga_switcheroo

[Bjorn Helgaas <bhelgaas@google.com>]

On Fri, Feb 24, 2017 at 1:19 PM, Lukas Wunner <lukas@wunner.de> wrote:
> An external Thunderbolt GPU can neither drive the laptop's panel nor be
> powered off by the platform, so there's no point in registering it with
> vga_switcheroo.

> In fact, when the external GPU is runtime suspended,
> vga_switcheroo will cut power to the internal discrete GPU, resulting in
> a lockup.

Why does suspending the external GPU cause vga_switcheroo to cut power
to the internal GPU?  No doubt this would be obvious to a GPU person,
which I definitely am not.

> Cc: Ben Skeggs <bskeggs@redhat.com>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>  drivers/gpu/drm/nouveau/nouveau_vga.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_vga.c b/drivers/gpu/drm/nouveau/nouveau_vga.c
> index eef22c6b9665..c2a7fd606c2e 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_vga.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_vga.c
> @@ -95,6 +95,10 @@ nouveau_vga_init(struct nouveau_drm *drm)
>
>         vga_client_register(dev->pdev, dev, NULL, nouveau_vga_set_decode);
>
> +       /* don't register Thunderbolt eGPU with vga_switcheroo */
> +       if (pci_is_thunderbolt_attached(dev->pdev))
> +               return;

I guess there's no way to move this inside
vga_switcheroo_register_client() instead of putting the test in all
the drivers?

> +
>         if (nouveau_runtime_pm == 1)
>                 runtime = true;
>         if ((nouveau_runtime_pm == -1) && (nouveau_is_optimus() || nouveau_is_v1_dsm()))
> @@ -111,6 +115,11 @@ nouveau_vga_fini(struct nouveau_drm *drm)
>         struct drm_device *dev = drm->dev;
>         bool runtime = false;
>
> +       vga_client_register(dev->pdev, NULL, NULL, NULL);
> +
> +       if (pci_is_thunderbolt_attached(dev->pdev))
> +               return;
> +
>         if (nouveau_runtime_pm == 1)
>                 runtime = true;
>         if ((nouveau_runtime_pm == -1) && (nouveau_is_optimus() || nouveau_is_v1_dsm()))
> @@ -119,7 +128,6 @@ nouveau_vga_fini(struct nouveau_drm *drm)
>         vga_switcheroo_unregister_client(dev->pdev);
>         if (runtime && nouveau_is_v1_dsm() && !nouveau_is_optimus())
>                 vga_switcheroo_fini_domain_pm_ops(drm->dev->dev);
> -       vga_client_register(dev->pdev, NULL, NULL, NULL);

The amd & radeon patches look like this:

-       vga_switcheroo_unregister_client(rdev->pdev);
+       if (!pci_is_thunderbolt_attached(adev->pdev))
+               vga_switcheroo_unregister_client(adev->pdev);

This nouveau patch looks suspiciously different.  But again, I'm not a
GPU guy so all I can really say is "hmm, I wonder why it's different?"

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