[PATCH v2 0/5] Thunderbolt GPU fixes

[PATCH v2 0/5] Thunderbolt GPU fixes

[Lukas Wunner]

Fix Thunderbolt-related issues in apple-gmux and vga_switcheroo, v2:

Same as v1 but includes all the acks collected and in patch [1/5]
the commit message and a code comment were edited as requested by
Bjorn Helgaas.

For details on this series please refer to the cover letter of v1:
https://lists.freedesktop.org/archives/dri-devel/2017-February/133891.html

To review the patches on GitHub, use this URL:
https://github.com/l1k/linux/commits/thunderbolt_gpu_v2

Thanks,

Lukas


Lukas Wunner (5):
  PCI: Recognize Thunderbolt devices
  drm/radeon: Don't register Thunderbolt eGPU with vga_switcheroo
  drm/amdgpu: Don't register Thunderbolt eGPU with vga_switcheroo
  drm/nouveau: Don't register Thunderbolt eGPU with vga_switcheroo
  apple-gmux: Don't switch external DP port on 2011+ MacBook Pros

 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  7 +++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    |  3 ++-
 drivers/gpu/drm/nouveau/nouveau_vga.c      | 10 +++++++++-
 drivers/gpu/drm/radeon/radeon_device.c     |  7 +++++--
 drivers/gpu/drm/radeon/radeon_kms.c        |  3 ++-
 drivers/pci/pci.h                          |  2 ++
 drivers/pci/probe.c                        | 21 ++++++++++++++++++++
 drivers/platform/x86/apple-gmux.c          | 31 +++++++++++++++++++++++++++++-
 include/linux/pci.h                        | 23 ++++++++++++++++++++++
 9 files changed, 99 insertions(+), 8 deletions(-)

-- 
2.11.0

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

[PATCH v2 1/5] PCI: Recognize Thunderbolt devices

[Lukas Wunner]

Detect on probe whether a PCI device is part of a Thunderbolt controller.
Intel uses a Vendor-Specific Extended Capability (VSEC) with ID 0x1234
on such devices.  Detect presence of this VSEC and cache it in a newly
added is_thunderbolt bit in struct pci_dev.

Also, add a helper to check whether a given PCI device is situated on a
Thunderbolt daisy chain (i.e., below a PCI device with is_thunderbolt
set).

The necessity arises from the following:

* If an external Thunderbolt GPU is connected to a dual GPU laptop,
  that GPU is currently registered with vga_switcheroo even though it
  can neither drive the laptop's panel nor be powered off by the
  platform.  To vga_switcheroo it will appear as if two discrete
  GPUs are present.  As a result, when the external GPU is runtime
  suspended, vga_switcheroo will cut power to the internal discrete GPU
  which may not be runtime suspended at all at this moment.  The
  solution is to not register external GPUs with vga_switcheroo, which
  necessitates a way to recognize if they're on a Thunderbolt daisy
  chain.

* Dual GPU MacBook Pros introduced 2011+ can no longer switch external
  DisplayPort ports between GPUs.  (They're no longer just used for DP
  but have become combined DP/Thunderbolt ports.)  The driver to switch
  the ports, drivers/platform/x86/apple-gmux.c, needs to detect presence
  of a Thunderbolt controller and, if found, keep external ports
  permanently switched to the discrete GPU.

v2: Make kerneldoc for pci_is_thunderbolt_attached() more precise,
    drop portion of commit message pertaining to separate series.
    (Bjorn Helgaas)

Cc: Andreas Noever <andreas.noever@gmail.com>
Cc: Michael Jamet <michael.jamet@intel.com>
Cc: Tomas Winkler <tomas.winkler@intel.com>
Cc: Amir Levy <amir.jer.levy@intel.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/pci/pci.h   |  2 ++
 drivers/pci/probe.c | 21 +++++++++++++++++++++
 include/linux/pci.h | 23 +++++++++++++++++++++++
 3 files changed, 46 insertions(+)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index cb17db242f30..45c2b8144911 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -3,6 +3,8 @@
 
 #define PCI_FIND_CAP_TTL	48
 
+#define PCI_VSEC_ID_INTEL_TBT	0x1234	/* Thunderbolt */
+
 extern const unsigned char pcie_link_speed[];
 
 bool pcie_cap_has_lnkctl(const struct pci_dev *dev);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 204960e70333..7963ecc6d85f 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1208,6 +1208,24 @@ void set_pcie_hotplug_bridge(struct pci_dev *pdev)
 		pdev->is_hotplug_bridge = 1;
 }
 
+static void set_pcie_thunderbolt(struct pci_dev *dev)
+{
+	int vsec = 0;
+	u32 header;
+
+	while ((vsec = pci_find_next_ext_capability(dev, vsec,
+						    PCI_EXT_CAP_ID_VNDR))) {
+		pci_read_config_dword(dev, vsec + PCI_VNDR_HEADER, &header);
+
+		/* Is the device part of a Thunderbolt controller? */
+		if (dev->vendor == PCI_VENDOR_ID_INTEL &&
+		    PCI_VNDR_HEADER_ID(header) == PCI_VSEC_ID_INTEL_TBT) {
+			dev->is_thunderbolt = 1;
+			return;
+		}
+	}
+}
+
 /**
  * pci_ext_cfg_is_aliased - is ext config space just an alias of std config?
  * @dev: PCI device
@@ -1360,6 +1378,9 @@ int pci_setup_device(struct pci_dev *dev)
 	/* need to have dev->class ready */
 	dev->cfg_size = pci_cfg_space_size(dev);
 
+	/* need to have dev->cfg_size ready */
+	set_pcie_thunderbolt(dev);
+
 	/* "Unknown power state" */
 	dev->current_state = PCI_UNKNOWN;
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index e2d1a124216a..a37ecd52b35b 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -358,6 +358,7 @@ struct pci_dev {
 	unsigned int	is_virtfn:1;
 	unsigned int	reset_fn:1;
 	unsigned int    is_hotplug_bridge:1;
+	unsigned int	is_thunderbolt:1; /* Thunderbolt controller */
 	unsigned int    __aer_firmware_first_valid:1;
 	unsigned int	__aer_firmware_first:1;
 	unsigned int	broken_intx_masking:1;
@@ -2171,6 +2172,28 @@ static inline bool pci_ari_enabled(struct pci_bus *bus)
 	return bus->self && bus->self->ari_enabled;
 }
 
+/**
+ * pci_is_thunderbolt_attached - whether device is on a Thunderbolt daisy chain
+ * @pdev: PCI device to check
+ *
+ * Walk upwards from @pdev and check for each encountered bridge if it's part
+ * of a Thunderbolt controller.  Reaching the host bridge means @pdev is not
+ * Thunderbolt-attached.  (But rather soldered to the mainboard usually.)
+ */
+static inline bool pci_is_thunderbolt_attached(struct pci_dev *pdev)
+{
+	struct pci_dev *parent = pdev;
+
+	if (pdev->is_thunderbolt)
+		return true;
+
+	while ((parent = pci_upstream_bridge(parent)))
+		if (parent->is_thunderbolt)
+			return true;
+
+	return false;
+}
+
 /* provide the legacy pci_dma_* API */
 #include <linux/pci-dma-compat.h>
 
-- 
2.11.0

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

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

[Lukas Wunner]

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.  Moreover AMD's Windows driver special-cases Thunderbolt as
well.

Acked-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/gpu/drm/radeon/radeon_device.c | 7 +++++--
 drivers/gpu/drm/radeon/radeon_kms.c    | 3 ++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 4b0c388be3f5..27be17f0b227 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1471,7 +1471,9 @@ int radeon_device_init(struct radeon_device *rdev,
 
 	if (rdev->flags & RADEON_IS_PX)
 		runtime = true;
-	vga_switcheroo_register_client(rdev->pdev, &radeon_switcheroo_ops, runtime);
+	if (!pci_is_thunderbolt_attached(rdev->pdev))
+		vga_switcheroo_register_client(rdev->pdev,
+					       &radeon_switcheroo_ops, runtime);
 	if (runtime)
 		vga_switcheroo_init_domain_pm_ops(rdev->dev, &rdev->vga_pm_domain);
 
@@ -1564,7 +1566,8 @@ void radeon_device_fini(struct radeon_device *rdev)
 	/* evict vram memory */
 	radeon_bo_evict_vram(rdev);
 	radeon_fini(rdev);
-	vga_switcheroo_unregister_client(rdev->pdev);
+	if (!pci_is_thunderbolt_attached(rdev->pdev))
+		vga_switcheroo_unregister_client(rdev->pdev);
 	if (rdev->flags & RADEON_IS_PX)
 		vga_switcheroo_fini_domain_pm_ops(rdev->dev);
 	vga_client_register(rdev->pdev, NULL, NULL, NULL);
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index 56f35c06742c..e95ceec1c97a 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -115,7 +115,8 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags)
 
 	if ((radeon_runtime_pm != 0) &&
 	    radeon_has_atpx() &&
-	    ((flags & RADEON_IS_IGP) == 0))
+	    ((flags & RADEON_IS_IGP) == 0) &&
+	    !pci_is_thunderbolt_attached(rdev->pdev))
 		flags |= RADEON_IS_PX;
 
 	/* radeon_device_init should report only fatal error
-- 
2.11.0

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

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

[Lukas Wunner]

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.  Moreover AMD's Windows driver special-cases Thunderbolt as
well.

Acked-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 +++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    | 3 ++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 6abb238b25c9..a8e1465a2bac 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1763,7 +1763,9 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 		runtime = true;
 	if (amdgpu_device_is_px(ddev))
 		runtime = true;
-	vga_switcheroo_register_client(adev->pdev, &amdgpu_switcheroo_ops, runtime);
+	if (!pci_is_thunderbolt_attached(adev->pdev))
+		vga_switcheroo_register_client(adev->pdev,
+					       &amdgpu_switcheroo_ops, runtime);
 	if (runtime)
 		vga_switcheroo_init_domain_pm_ops(adev->dev, &adev->vga_pm_domain);
 
@@ -1926,7 +1928,8 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
 	amdgpu_atombios_fini(adev);
 	kfree(adev->bios);
 	adev->bios = NULL;
-	vga_switcheroo_unregister_client(adev->pdev);
+	if (!pci_is_thunderbolt_attached(adev->pdev))
+		vga_switcheroo_unregister_client(adev->pdev);
 	if (adev->flags & AMD_IS_PX)
 		vga_switcheroo_fini_domain_pm_ops(adev->dev);
 	vga_client_register(adev->pdev, NULL, NULL, NULL);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 61d94c745672..2f3b236721c9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -103,7 +103,8 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
 	    amdgpu_has_atpx() &&
 	    (amdgpu_is_atpx_hybrid() ||
 	     amdgpu_has_atpx_dgpu_power_cntl()) &&
-	    ((flags & AMD_IS_APU) == 0))
+	    ((flags & AMD_IS_APU) == 0) &&
+	    !pci_is_thunderbolt_attached(dev->pdev))
 		flags |= AMD_IS_PX;
 
 	/* amdgpu_device_init should report only fatal error
-- 
2.11.0

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

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

[Lukas Wunner]

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.

Cc: Ben Skeggs <bskeggs@redhat.com>
Acked-by: Alex Deucher <alexander.deucher@amd.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 ccb597eac538..a4aacbc0cec8 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;
+
 	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);
 }
 
 
-- 
2.11.0

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

[PATCH v2 5/5] apple-gmux: Don't switch external DP port on 2011+ MacBook Pros

[Lukas Wunner]

On MacBook Pros introduced 2011 and onward, external DP ports are
combined DP/Thunderbolt ports that are no longer fully switchable
between GPUs, they can only be driven by the discrete GPU.

More specifically, the Main Link pins (which transport the actual video
and audio streams) are soldered to the discrete GPU, whereas the AUX
Channel pins are switchable. Because the integrated GPU is missing the
Main Link, external displays appear to it as phantoms which fail to
link-train.

Force the AUX channel to the discrete GPU on these models to avoid any
confusion. Document the switching policy implemented by this commit.

Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/platform/x86/apple-gmux.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index a66be137324c..623d322447a2 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -60,6 +60,7 @@ struct apple_gmux_data {
 	/* switcheroo data */
 	acpi_handle dhandle;
 	int gpe;
+	bool external_switchable;
 	enum vga_switcheroo_client_id switch_state_display;
 	enum vga_switcheroo_client_id switch_state_ddc;
 	enum vga_switcheroo_client_id switch_state_external;
@@ -358,6 +359,19 @@ static const struct backlight_ops gmux_bl_ops = {
  * ports while the discrete GPU is asleep, but currently we do not make use
  * of this feature.
  *
+ * Our switching policy for the external port is that on those generations
+ * which are able to switch it fully, the port is switched together with the
+ * panel when IGD / DIS commands are issued to vga_switcheroo. It is thus
+ * possible to drive e.g. a beamer on battery power with the integrated GPU.
+ * The user may manually switch to the discrete GPU if more performance is
+ * needed.
+ *
+ * On all newer generations, the external port can only be driven by the
+ * discrete GPU. If a display is plugged in while the panel is switched to
+ * the integrated GPU, *both* GPUs will be in use for maximum performance.
+ * To decrease power consumption, the user may manually switch to the
+ * discrete GPU, thereby suspending the integrated GPU.
+ *
  * gmux' initial switch state on bootup is user configurable via the EFI
  * variable ``gpu-power-prefs-fa4ce28d-b62f-4c99-9cc3-6815686e30f9`` (5th byte,
  * 1 = IGD, 0 = DIS). Based on this setting, the EFI firmware tells gmux to
@@ -414,7 +428,8 @@ static int gmux_switchto(enum vga_switcheroo_client_id id)
 {
 	apple_gmux_data->switch_state_ddc = id;
 	apple_gmux_data->switch_state_display = id;
-	apple_gmux_data->switch_state_external = id;
+	if (apple_gmux_data->external_switchable)
+		apple_gmux_data->switch_state_external = id;
 
 	gmux_write_switch_state(apple_gmux_data);
 
@@ -601,6 +616,11 @@ static struct pci_dev *gmux_get_io_pdev(void)
 	return NULL;
 }
 
+static int is_thunderbolt(struct device *dev, void *data)
+{
+	return to_pci_dev(dev)->is_thunderbolt;
+}
+
 static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 {
 	struct apple_gmux_data *gmux_data;
@@ -755,6 +775,15 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 		gmux_data->gpe = -1;
 	}
 
+	/*
+	 * If Thunderbolt is present, the external DP port is not fully
+	 * switchable. Force its AUX channel to the discrete GPU.
+	 */
+	gmux_data->external_switchable =
+		!bus_for_each_dev(&pci_bus_type, NULL, NULL, is_thunderbolt);
+	if (!gmux_data->external_switchable)
+		gmux_write8(gmux_data, GMUX_PORT_SWITCH_EXTERNAL, 3);
+
 	apple_gmux_data = gmux_data;
 	init_completion(&gmux_data->powerchange_done);
 	gmux_enable_interrupts(gmux_data);
-- 
2.11.0

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

Re: [PATCH v2 1/5] PCI: Recognize Thunderbolt devices

[Bjorn Helgaas]

On Fri, Mar 10, 2017 at 2:23 PM, Lukas Wunner <lukas@wunner.de> wrote:

> +/**
> + * pci_is_thunderbolt_attached - whether device is on a Thunderbolt daisy chain
> + * @pdev: PCI device to check
> + *
> + * Walk upwards from @pdev and check for each encountered bridge if it's part
> + * of a Thunderbolt controller.  Reaching the host bridge means @pdev is not
> + * Thunderbolt-attached.  (But rather soldered to the mainboard usually.)

The "soldered to the mainboard" comment is misleading.  We'll reach
the host bridge and return "false" for any non-Thunderbolt-attached
device, including all plug-in PCI and PCIe devices.

> + */
> +static inline bool pci_is_thunderbolt_attached(struct pci_dev *pdev)
> +{
> +       struct pci_dev *parent = pdev;
> +
> +       if (pdev->is_thunderbolt)
> +               return true;
> +
> +       while ((parent = pci_upstream_bridge(parent)))
> +               if (parent->is_thunderbolt)
> +                       return true;
> +
> +       return false;
> +}
> +
>  /* provide the legacy pci_dma_* API */
>  #include <linux/pci-dma-compat.h>
>
> --
> 2.11.0
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/5] PCI: Recognize Thunderbolt devices

[Lukas Wunner]

On Fri, Mar 10, 2017 at 02:47:04PM -0600, Bjorn Helgaas wrote:
> On Fri, Mar 10, 2017 at 2:23 PM, Lukas Wunner <lukas@wunner.de> wrote:
> > +/**
> > + * pci_is_thunderbolt_attached - whether device is on a Thunderbolt daisy chain
> > + * @pdev: PCI device to check
> > + *
> > + * Walk upwards from @pdev and check for each encountered bridge if it's part
> > + * of a Thunderbolt controller.  Reaching the host bridge means @pdev is not
> > + * Thunderbolt-attached.  (But rather soldered to the mainboard usually.)
> 
> The "soldered to the mainboard" comment is misleading.  We'll reach
> the host bridge and return "false" for any non-Thunderbolt-attached
> device, including all plug-in PCI and PCIe devices.

It does say "usually". :-)  Seriously though, for someone coming from one
of the callers of pci_is_thunderbolt_attached() and trying to understand
its meaning, it may not be helpful if I had left it at "Reaching the host
bridge means @pdev is not Thunderbolt-attached."

What is the *consequence* of that?  Most Thunderbolt-equipped products
have no other PCI expansion options, so indeed if the device is not on
a Thunderbolt daisy chain it must be soldered to the mainboard.  If one
wants to be pedantic, one could add that it may alternatively be
soldered to a *daughter*board.  (Which is the case on the MacPro6,1,
the black trashcan.)  I was just trying to strike a balance between
technical correctness, didactic quality and brevity.

Best regards,

Lukas

> 
> > + */
> > +static inline bool pci_is_thunderbolt_attached(struct pci_dev *pdev)
> > +{
> > +       struct pci_dev *parent = pdev;
> > +
> > +       if (pdev->is_thunderbolt)
> > +               return true;
> > +
> > +       while ((parent = pci_upstream_bridge(parent)))
> > +               if (parent->is_thunderbolt)
> > +                       return true;
> > +
> > +       return false;
> > +}
> > +
> >  /* provide the legacy pci_dma_* API */
> >  #include <linux/pci-dma-compat.h>
> >
> > --
> > 2.11.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/5] Thunderbolt GPU fixes

[Lukas Wunner]

Hi Daniel,

On Fri, Mar 10, 2017 at 09:23:45PM +0100, Lukas Wunner wrote:
> Fix Thunderbolt-related issues in apple-gmux and vga_switcheroo, v2:
> 
> Same as v1 but includes all the acks collected and in patch [1/5]
> the commit message and a code comment were edited as requested by
> Bjorn Helgaas.
> 
> For details on this series please refer to the cover letter of v1:
> https://lists.freedesktop.org/archives/dri-devel/2017-February/133891.html
> 
> To review the patches on GitHub, use this URL:
> https://github.com/l1k/linux/commits/thunderbolt_gpu_v2

Just a gentle reminder that this series is awaiting merging.
Patches are acked and I believe I've addressed/responded to
all comments.

Have a pleasant (albeit stormy) weekend!

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

Re: [PATCH v2 0/5] Thunderbolt GPU fixes

[Lukas Wunner]

Dear drm-misc maintainers,

please consider merging this series:

On Sat, Mar 18, 2017 at 08:39:53AM +0100, Lukas Wunner wrote:
> Hi Daniel,
> 
> On Fri, Mar 10, 2017 at 09:23:45PM +0100, Lukas Wunner wrote:
> > Fix Thunderbolt-related issues in apple-gmux and vga_switcheroo, v2:
> > 
> > Same as v1 but includes all the acks collected and in patch [1/5]
> > the commit message and a code comment were edited as requested by
> > Bjorn Helgaas.
> > 
> > For details on this series please refer to the cover letter of v1:
> > https://lists.freedesktop.org/archives/dri-devel/2017-February/133891.html
> > 
> > To review the patches on GitHub, use this URL:
> > https://github.com/l1k/linux/commits/thunderbolt_gpu_v2
> 
> Just a gentle reminder that this series is awaiting merging.
> Patches are acked and I believe I've addressed/responded to
> all comments.

Thanks,

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

Re: [PATCH v2 0/5] Thunderbolt GPU fixes

[Daniel Vetter]

Hi Lukas,

On Sat, Mar 25, 2017 at 09:05:20AM +0100, Lukas Wunner wrote:
> Dear drm-misc maintainers,
> 
> please consider merging this series:

I think I pinged you in private already, but want to become drm-misc
committer so that you don't have to wait for lazy maintainers to get
around to merging patch series which are ready?

-Daniel

> 
> On Sat, Mar 18, 2017 at 08:39:53AM +0100, Lukas Wunner wrote:
> > Hi Daniel,
> > 
> > On Fri, Mar 10, 2017 at 09:23:45PM +0100, Lukas Wunner wrote:
> > > Fix Thunderbolt-related issues in apple-gmux and vga_switcheroo, v2:
> > > 
> > > Same as v1 but includes all the acks collected and in patch [1/5]
> > > the commit message and a code comment were edited as requested by
> > > Bjorn Helgaas.
> > > 
> > > For details on this series please refer to the cover letter of v1:
> > > https://lists.freedesktop.org/archives/dri-devel/2017-February/133891.html
> > > 
> > > To review the patches on GitHub, use this URL:
> > > https://github.com/l1k/linux/commits/thunderbolt_gpu_v2
> > 
> > Just a gentle reminder that this series is awaiting merging.
> > Patches are acked and I believe I've addressed/responded to
> > all comments.
> 
> Thanks,
> 
> Lukas

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/5] Thunderbolt GPU fixes

[Lukas Wunner]

On Sat, Mar 25, 2017 at 10:06:03PM +0100, Daniel Vetter wrote:
> On Sat, Mar 25, 2017 at 09:05:20AM +0100, Lukas Wunner wrote:
> > Dear drm-misc maintainers,
> > 
> > please consider merging this series:
> 
> I think I pinged you in private already, but want to become drm-misc
> committer so that you don't have to wait for lazy maintainers to get
> around to merging patch series which are ready?

Sure. \o/

I've resurrected this old request for an fdo account as that seems
to be a prerequisite:

https://bugs.freedesktop.org/show_bug.cgi?id=89906

I'm digging into the dim documentation meanwhile...

Thanks!

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

Re: [PATCH v2 0/5] Thunderbolt GPU fixes

[Lukas Wunner]

On Fri, Mar 10, 2017 at 09:23:45PM +0100, Lukas Wunner wrote:
> Fix Thunderbolt-related issues in apple-gmux and vga_switcheroo, v2:
> 
> Same as v1 but includes all the acks collected and in patch [1/5]
> the commit message and a code comment were edited as requested by
> Bjorn Helgaas.
> 
> For details on this series please refer to the cover letter of v1:
> https://lists.freedesktop.org/archives/dri-devel/2017-February/133891.html
> 
> To review the patches on GitHub, use this URL:
> https://github.com/l1k/linux/commits/thunderbolt_gpu_v2

Pushed to drm-misc-next.

Bjorn, I haven't heard back after my reply to your e-mail of March 10
(regarding the "usually soldered to the mainboard" comment). I hope
my reply was satisfactory.  If it wasn't, please feel free to amend
the comment after the 4.12 merge window closes or alternatively send
me a patch to fix it up.

Thanks!

Lukas

> Lukas Wunner (5):
>   PCI: Recognize Thunderbolt devices
>   drm/radeon: Don't register Thunderbolt eGPU with vga_switcheroo
>   drm/amdgpu: Don't register Thunderbolt eGPU with vga_switcheroo
>   drm/nouveau: Don't register Thunderbolt eGPU with vga_switcheroo
>   apple-gmux: Don't switch external DP port on 2011+ MacBook Pros
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  7 +++++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    |  3 ++-
>  drivers/gpu/drm/nouveau/nouveau_vga.c      | 10 +++++++++-
>  drivers/gpu/drm/radeon/radeon_device.c     |  7 +++++--
>  drivers/gpu/drm/radeon/radeon_kms.c        |  3 ++-
>  drivers/pci/pci.h                          |  2 ++
>  drivers/pci/probe.c                        | 21 ++++++++++++++++++++
>  drivers/platform/x86/apple-gmux.c          | 31 +++++++++++++++++++++++++++++-
>  include/linux/pci.h                        | 23 ++++++++++++++++++++++
>  9 files changed, 99 insertions(+), 8 deletions(-)
> 
> -- 
> 2.11.0
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/5] Thunderbolt GPU fixes

[Bjorn Helgaas]

On Fri, Mar 31, 2017 at 3:11 AM, Lukas Wunner <lukas@wunner.de> wrote:
> On Fri, Mar 10, 2017 at 09:23:45PM +0100, Lukas Wunner wrote:
>> Fix Thunderbolt-related issues in apple-gmux and vga_switcheroo, v2:
>>
>> Same as v1 but includes all the acks collected and in patch [1/5]
>> the commit message and a code comment were edited as requested by
>> Bjorn Helgaas.
>>
>> For details on this series please refer to the cover letter of v1:
>> https://lists.freedesktop.org/archives/dri-devel/2017-February/133891.html
>>
>> To review the patches on GitHub, use this URL:
>> https://github.com/l1k/linux/commits/thunderbolt_gpu_v2
>
> Pushed to drm-misc-next.
>
> Bjorn, I haven't heard back after my reply to your e-mail of March 10
> (regarding the "usually soldered to the mainboard" comment). I hope
> my reply was satisfactory.  If it wasn't, please feel free to amend
> the comment after the 4.12 merge window closes or alternatively send
> me a patch to fix it up.

I didn't want to needlessly prolong the discussion.  I still think
"soldered to the mainboard" is unhelpful because it contributes to a
mental model that is incorrect, namely, that Thunderbolt connectedness
is somehow related to whether the device is soldered in or removable.
I don't think it's useful to reinforce the assumption that systems
with Thunderbolt have no PCIe slots.  There's no technical reason we
can't have both, and I'm pretty sure I have an Intel box in the closet
that does.

There, I've prolonged the discussion after all :)  But it's really not
that big a deal, and you have my ack for the patch, including for the
comment, so it's OK with me to merge it as-is.

>> Lukas Wunner (5):
>>   PCI: Rhttps://cs.corp.google.com/kernel/drivers/gpu/nvidia-proprietary/nvidia/nv.c?type=cs&q=%22one+of+more+GPUs+has+fallen+off+the+bus%22&l=196ecognize Thunderbolt devices
>>   drm/radeon: Don't register Thunderbolt eGPU with vga_switcheroo
>>   drm/amdgpu: Don't register Thunderbolt eGPU with vga_switcheroo
>>   drm/nouveau: Don't register Thunderbolt eGPU with vga_switcheroo
>>   apple-gmux: Don't switch external DP port on 2011+ MacBook Pros
>>
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  7 +++++--
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    |  3 ++-
>>  drivers/gpu/drm/nouveau/nouveau_vga.c      | 10 +++++++++-
>>  drivers/gpu/drm/radeon/radeon_device.c     |  7 +++++--
>>  drivers/gpu/drm/radeon/radeon_kms.c        |  3 ++-
>>  drivers/pci/pci.h                          |  2 ++
>>  drivers/pci/probe.c                        | 21 ++++++++++++++++++++
>>  drivers/platform/x86/apple-gmux.c          | 31 +++++++++++++++++++++++++++++-
>>  include/linux/pci.h                        | 23 ++++++++++++++++++++++
>>  9 files changed, 99 insertions(+), 8 deletions(-)
>>
>> --
>> 2.11.0
>>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/5] Thunderbolt GPU fixes

[Lukas Wunner]

On Fri, Mar 31, 2017 at 06:30:48AM -0700, Bjorn Helgaas wrote:
> On Fri, Mar 31, 2017 at 3:11 AM, Lukas Wunner <lukas@wunner.de> wrote:
> > Bjorn, I haven't heard back after my reply to your e-mail of March 10
> > (regarding the "usually soldered to the mainboard" comment). I hope
> > my reply was satisfactory.  If it wasn't, please feel free to amend
> > the comment after the 4.12 merge window closes or alternatively send
> > me a patch to fix it up.
> 
> I didn't want to needlessly prolong the discussion.  I still think
> "soldered to the mainboard" is unhelpful because it contributes to a
> mental model that is incorrect, namely, that Thunderbolt connectedness
> is somehow related to whether the device is soldered in or removable.
> I don't think it's useful to reinforce the assumption that systems
> with Thunderbolt have no PCIe slots.  There's no technical reason we
> can't have both, and I'm pretty sure I have an Intel box in the closet
> that does.

You're right, systems do exist which have other PCI expansion options
besides Thunderbolt, even though they're relatively rare.  E.g. there
was one MacBook Pro (the 2011 17" model) which had a Thunderbolt 1 port
as well as a CardBus slot.

How about the patch below?  If you can think of something better please
let me know.  Thanks!

Lukas

-- >8 --
Subject: [PATCH] PCI: Clarify kernel-doc of pci_is_thunderbolt_attached()

The kernel-doc of pci_is_thunderbolt_attached() claims that a return
value of false means the device is "usually soldered to the mainboard".
Bjorn Helgaas pointed out that systems do exist which have both
Thunderbolt as well as PCIe or CardBus slots and the kernel-doc may thus
evoke incorrect assumptions.  Attempt to make it more precise.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 include/linux/pci.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 5948cfd..78f3b41 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2167,7 +2167,8 @@ static inline bool pci_ari_enabled(struct pci_bus *bus)
  *
  * Walk upwards from @pdev and check for each encountered bridge if it's part
  * of a Thunderbolt controller.  Reaching the host bridge means @pdev is not
- * Thunderbolt-attached.  (But rather soldered to the mainboard usually.)
+ * Thunderbolt-attached.  (But rather soldered to the mainboard if the system
+ * has no other PCI expansion options.)
  */
 static inline bool pci_is_thunderbolt_attached(struct pci_dev *pdev)
 {
-- 
2.11.0

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