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

From: Lukas Wunner <lukas@wunner.de>
To: Alex Deucher <alexander.deucher@amd.com>
Cc: Michael Jamet <michael.jamet@intel.com>,
	amd-gfx list <amd-gfx@lists.freedesktop.org>,
	dri-devel@lists.freedesktop.org,
	Bjorn Helgaas <bhelgaas@google.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Christian Koenig <christian.koenig@amd.com>,
	Peter Wu <peter@lekensteyn.nl>
Subject: Re: [PATCH 4/5] drm/radeon: Don't register Thunderbolt eGPU with vga_switcheroo
Date: Thu, 9 Mar 2017 11:55:04 +0100	[thread overview]
Message-ID: <20170309105504.GA7402@h08.hostsharing.net> (raw)
In-Reply-To: <CADnq5_P9yV8W+4gbgUQoL8yC_5ZBHagvcZbJUOkkRCYiL8GjfQ@mail.gmail.com>

On Wed, Mar 08, 2017 at 03:34:47PM -0500, Alex Deucher wrote:
> On Wed, Mar 8, 2017 at 12:01 AM, Lukas Wunner <lukas@wunner.de> wrote:
> > On Tue, Mar 07, 2017 at 03:30:30PM -0500, Alex Deucher wrote:
> >> On Fri, Feb 24, 2017 at 2: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.
> >>
> >> I'm not necessarily opposed to this, but I'd prefer something more
> >> generic.  E.g., what happens if someone uses another dGPU in a docking
> >> station or some other sort of PCIe bridge?
> >
> > Such a dGPU is only relevant to vga_switcheroo if it can either drive
> > the panel or be powered off by the platform.  Does such a product exist?
> >
> > I've never heard of one, and think that's because such a product doesn't
> > make sense:  A docking staton is not power-constrained, it's stationary
> > and connected to a wall outlet, so there's no need to power the dGPU off
> > when it's not in use.
> >
> > And at a docking station you're usually connected to a larger monitor,
> > so having the dGPU drive the laptop's smaller panel isn't necessary either.
> > In the rare cases where there's no larger monitor, you just use the dGPU
> > for render offloading, just as you would for contemporary ATPX laptops.
> >
> > OTOH, dGPUs in Thunderbolt enclosures *do* exist and connecting them
> > to an ATPX machine causes failure, as explained in the commit message.
> 
> Whether you introduce additional dGPUs via thunderbolt or some
> proprietary interface or a pci bridge in a docking station the result
> is still the same.  You end up with the potential scenario you
> described in this commit message that there may be confusion as to
> which GPU is controlled via ACPI power controls.
> 
> I talked to the windows team.  They special case thunderbolt as well,

Very interesting, thanks for reaching out to them.  I've already heard
that Windows 10 supports Thunderbolt eGPUs, but only with Thunderbolt 3.
I think it's desirable that we achieve feature parity with them (without
the unnecessary restriction to Thunderbolt 3).  Older Windows versions as
well as macOS apparently require all sorts of awful hacks for eGPUs.

> so the patch is probably fine.

Is that an ack or are there any remaining concerns?

> For pcie ports in a docking station, I
> suspect there may not actually be any docking stations supported by PX
> laptops where this could be an issue.

If/when such products do become available, they can probably be identified
via specific ACPI methods or if all else fails, DMI quirks.

> For non-thunderbolt detachable
> graphics there is a new ATIF function to query the bus number of the
> supported device.  I'll send a patch out for that in a bit.

Great, thanks.

> Thinking about this more, long term we should probably only register
> with vga_switcheroo if we support display muxing which is a legacy
> feature these days.  Most systems are mux-less so we just need to
> handle dgpu power control via runtime pm.

Right now registering with vga_switcheroo is still necessary even for
muxless systems primarily because DRM drivers call
vga_switcheroo_set_dynamic_switch() to pause the HDA driver and update
the power state stored internally by vga_switcheroo.

I plan to address the former by reworking vga_switcheroo audio handling
using functional device dependencies (a new PM mechanism that appeared
in v4.10, see documentation in aad800403a87), and I think the latter will
then become obsolete as well.  I've got a concept in my head and am
pumped to code it up, just a little time-constrained at the moment. :-)

Thanks,

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