Nouveau - Sep 2019 - [PATCH v4 0/4] add PCIe workaround to fix runpm on laptops

not much changed since the last time I sent those patches out, but there
are a couple of annoying bug fixes, which users would probably never hit
unless they do rmmod/modprobe nouveau cycles.

Biggest change is that I force the link to a 8.0 speed rather than the
speed the GPU came up with.

Also this series depends on the PCIe improvement patches I sent out
recently.

Karol Herbst (4):
  pci: enable pcie link changes for pascal
  pci: add nvkm_pcie_get_speed
  pci: set the pcie link speed to 8.0 when suspending
  drm: abort runtime suspend if we hit an error

 drm/nouveau/include/nvkm/core/device.h |  2 ++
 drm/nouveau/include/nvkm/subdev/pci.h  |  4 ++-
 drm/nouveau/nouveau_drm.c              |  6 +++++
 drm/nouveau/nvkm/subdev/clk/base.c     |  2 +-
 drm/nouveau/nvkm/subdev/pci/base.c     |  2 ++
 drm/nouveau/nvkm/subdev/pci/gk104.c    |  8 +++---
 drm/nouveau/nvkm/subdev/pci/gp100.c    | 11 ++++++++
 drm/nouveau/nvkm/subdev/pci/pcie.c     | 35 +++++++++++++++++++++++---
 drm/nouveau/nvkm/subdev/pci/priv.h     |  6 +++++
 9 files changed, 66 insertions(+), 10 deletions(-)

-- 
2.21.0

v2: add force disable ASPM func pointer

Signed-off-by: Karol Herbst <kherbst at redhat.com>
Reviewed-by: Lyude Paul <lyude at redhat.com> (v1)
---
 drm/nouveau/nvkm/subdev/pci/gk104.c |  8 ++++----
 drm/nouveau/nvkm/subdev/pci/gp100.c | 11 +++++++++++
 drm/nouveau/nvkm/subdev/pci/priv.h  |  5 +++++
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drm/nouveau/nvkm/subdev/pci/gk104.c
b/drm/nouveau/nvkm/subdev/pci/gk104.c
index 804ef017f..61c4e76e8 100644
--- a/drm/nouveau/nvkm/subdev/pci/gk104.c
+++ b/drm/nouveau/nvkm/subdev/pci/gk104.c
@@ -25,7 +25,7 @@
 
 #include <subdev/timer.h>
 
-static int
+int
 gk104_pcie_version_supported(struct nvkm_pci *pci)
 {
 	return (nvkm_rd32(pci->subdev.device, 0x8c1c0) & 0x4) == 0x4 ? 2 : 1;
@@ -110,7 +110,7 @@ gk104_pcie_lnkctl_speed(struct nvkm_pci *pci)
 	return -1;
 }
 
-static enum nvkm_pcie_speed
+enum nvkm_pcie_speed
 gk104_pcie_max_speed(struct nvkm_pci *pci)
 {
 	u32 max_speed = nvkm_rd32(pci->subdev.device, 0x8c1c0) & 0x300000;
@@ -156,7 +156,7 @@ gk104_pcie_set_link_speed(struct nvkm_pci *pci, enum
nvkm_pcie_speed speed)
 	return 0;
 }
 
-static int
+int
 gk104_pcie_init(struct nvkm_pci * pci)
 {
 	enum nvkm_pcie_speed lnkctl_speed, max_speed, cap_speed;
@@ -188,7 +188,7 @@ gk104_pcie_init(struct nvkm_pci * pci)
 	return 0;
 }
 
-static int
+int
 gk104_pcie_set_link(struct nvkm_pci *pci, enum nvkm_pcie_speed speed, u8 width)
 {
 	struct nvkm_subdev *subdev = &pci->subdev;
diff --git a/drm/nouveau/nvkm/subdev/pci/gp100.c
b/drm/nouveau/nvkm/subdev/pci/gp100.c
index 82c5234a0..163233f16 100644
--- a/drm/nouveau/nvkm/subdev/pci/gp100.c
+++ b/drm/nouveau/nvkm/subdev/pci/gp100.c
@@ -35,6 +35,17 @@ gp100_pci_func = {
 	.wr08 = nv40_pci_wr08,
 	.wr32 = nv40_pci_wr32,
 	.msi_rearm = gp100_pci_msi_rearm,
+
+	.pcie.init = gk104_pcie_init,
+	.pcie.set_link = gk104_pcie_set_link,
+
+	.pcie.max_speed = gk104_pcie_max_speed,
+	.pcie.cur_speed = g84_pcie_cur_speed,
+
+	.pcie.set_version = gf100_pcie_set_version,
+	.pcie.version = gf100_pcie_version,
+	.pcie.version_supported = gk104_pcie_version_supported,
+	.pcie.force_aspm_off = g84_pcie_force_aspm_off,
 };
 
 int
diff --git a/drm/nouveau/nvkm/subdev/pci/priv.h
b/drm/nouveau/nvkm/subdev/pci/priv.h
index c6a9ef330..82c78befa 100644
--- a/drm/nouveau/nvkm/subdev/pci/priv.h
+++ b/drm/nouveau/nvkm/subdev/pci/priv.h
@@ -56,6 +56,11 @@ int gf100_pcie_cap_speed(struct nvkm_pci *);
 int gf100_pcie_init(struct nvkm_pci *);
 int gf100_pcie_set_link(struct nvkm_pci *, enum nvkm_pcie_speed, u8);
 
+int gk104_pcie_init(struct nvkm_pci *);
+int gk104_pcie_set_link(struct nvkm_pci *, enum nvkm_pcie_speed, u8 width);
+enum nvkm_pcie_speed gk104_pcie_max_speed(struct nvkm_pci *);
+int gk104_pcie_version_supported(struct nvkm_pci *);
+
 int nvkm_pcie_oneinit(struct nvkm_pci *);
 int nvkm_pcie_init(struct nvkm_pci *);
 #endif
-- 
2.21.0

v2: fixed compilation error

Signed-off-by: Karol Herbst <kherbst at redhat.com>
Reviewed-by: Lyude Paul <lyude at redhat.com>
---
 drm/nouveau/include/nvkm/subdev/pci.h | 1 +
 drm/nouveau/nvkm/subdev/pci/pcie.c    | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/drm/nouveau/include/nvkm/subdev/pci.h
b/drm/nouveau/include/nvkm/subdev/pci.h
index 4803a4fad..b29101e48 100644
--- a/drm/nouveau/include/nvkm/subdev/pci.h
+++ b/drm/nouveau/include/nvkm/subdev/pci.h
@@ -53,4 +53,5 @@ int gp100_pci_new(struct nvkm_device *, int, struct nvkm_pci
**);
 
 /* pcie functions */
 int nvkm_pcie_set_link(struct nvkm_pci *, enum nvkm_pcie_speed, u8 width);
+enum nvkm_pcie_speed nvkm_pcie_get_speed(struct nvkm_pci *);
 #endif
diff --git a/drm/nouveau/nvkm/subdev/pci/pcie.c
b/drm/nouveau/nvkm/subdev/pci/pcie.c
index 354ac4c85..b4203ff1a 100644
--- a/drm/nouveau/nvkm/subdev/pci/pcie.c
+++ b/drm/nouveau/nvkm/subdev/pci/pcie.c
@@ -177,3 +177,11 @@ nvkm_pcie_set_link(struct nvkm_pci *pci, enum
nvkm_pcie_speed speed, u8 width)
 
 	return ret;
 }
+
+enum nvkm_pcie_speed
+nvkm_pcie_get_speed(struct nvkm_pci *pci)
+{
+	if (!pci || !pci_is_pcie(pci->pdev) || !pci->func->pcie.cur_speed)
+		return -ENODEV;
+	return pci->func->pcie.cur_speed(pci);
+}
-- 
2.21.0

Apperantly things go south if we suspend the device with a PCIe link speed
set to 2.5. Fixes runtime suspend on my gp107.

This all looks like some bug inside the pci subsystem and I would prefer a
fix there instead of nouveau, but maybe there is no real nice way of doing
that outside of drivers?

v2: squashed together patch 4 and 5
v3: only restore pcie speed on machines with runpm
    add NvRunpmWorkaround config option to disable the workaround
v4: only run the code on suspend
    always put the card into 8.0 mode, not what nouveau detected on load

Signed-off-by: Karol Herbst <kherbst at redhat.com>
Reviewed-by: Lyude Paul <lyude at redhat.com> (v2)
---
 drm/nouveau/include/nvkm/core/device.h |  2 ++
 drm/nouveau/include/nvkm/subdev/pci.h  |  3 ++-
 drm/nouveau/nouveau_drm.c              |  1 +
 drm/nouveau/nvkm/subdev/clk/base.c     |  2 +-
 drm/nouveau/nvkm/subdev/pci/base.c     |  2 ++
 drm/nouveau/nvkm/subdev/pci/pcie.c     | 27 ++++++++++++++++++++++----
 drm/nouveau/nvkm/subdev/pci/priv.h     |  1 +
 7 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/drm/nouveau/include/nvkm/core/device.h
b/drm/nouveau/include/nvkm/core/device.h
index 6d55cd047..4fb3f972f 100644
--- a/drm/nouveau/include/nvkm/core/device.h
+++ b/drm/nouveau/include/nvkm/core/device.h
@@ -125,6 +125,8 @@ struct nvkm_device {
 	u8  chiprev;
 	u32 crystal;
 
+	bool has_runpm;
+
 	struct {
 		struct notifier_block nb;
 	} acpi;
diff --git a/drm/nouveau/include/nvkm/subdev/pci.h
b/drm/nouveau/include/nvkm/subdev/pci.h
index b29101e48..7245513d9 100644
--- a/drm/nouveau/include/nvkm/subdev/pci.h
+++ b/drm/nouveau/include/nvkm/subdev/pci.h
@@ -52,6 +52,7 @@ int gk104_pci_new(struct nvkm_device *, int, struct nvkm_pci
**);
 int gp100_pci_new(struct nvkm_device *, int, struct nvkm_pci **);
 
 /* pcie functions */
-int nvkm_pcie_set_link(struct nvkm_pci *, enum nvkm_pcie_speed, u8 width);
+int nvkm_pcie_set_link(struct nvkm_pci *, enum nvkm_pcie_speed, u8 width,
+		       bool save);
 enum nvkm_pcie_speed nvkm_pcie_get_speed(struct nvkm_pci *);
 #endif
diff --git a/drm/nouveau/nouveau_drm.c b/drm/nouveau/nouveau_drm.c
index 3d32afe8a..78d55c525 100644
--- a/drm/nouveau/nouveau_drm.c
+++ b/drm/nouveau/nouveau_drm.c
@@ -676,6 +676,7 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
 
 	if (nouveau_atomic)
 		driver_pci.driver_features |= DRIVER_ATOMIC;
+	device->has_runpm = nouveau_pmops_runtime();
 
 	drm_dev = drm_dev_alloc(&driver_pci, &pdev->dev);
 	if (IS_ERR(drm_dev)) {
diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
index ba6a868d4..e30e77453 100644
--- a/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drm/nouveau/nvkm/subdev/clk/base.c
@@ -277,7 +277,7 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
 	nvkm_debug(subdev, "setting performance state %d\n", pstatei);
 	clk->pstate = pstatei;
 
-	nvkm_pcie_set_link(pci, pstate->pcie_speed, pstate->pcie_width);
+	nvkm_pcie_set_link(pci, pstate->pcie_speed, pstate->pcie_width, true);
 
 	if (fb && fb->ram && fb->ram->func->calc) {
 		struct nvkm_ram *ram = fb->ram;
diff --git a/drm/nouveau/nvkm/subdev/pci/base.c
b/drm/nouveau/nvkm/subdev/pci/base.c
index ee2431a78..c9b60ef76 100644
--- a/drm/nouveau/nvkm/subdev/pci/base.c
+++ b/drm/nouveau/nvkm/subdev/pci/base.c
@@ -90,6 +90,8 @@ nvkm_pci_fini(struct nvkm_subdev *subdev, bool suspend)
 
 	if (pci->agp.bridge)
 		nvkm_agp_fini(pci);
+	else if (pci_is_pcie(pci->pdev))
+		nvkm_pcie_fini(pci, suspend);
 
 	return 0;
 }
diff --git a/drm/nouveau/nvkm/subdev/pci/pcie.c
b/drm/nouveau/nvkm/subdev/pci/pcie.c
index b4203ff1a..5cab4a240 100644
--- a/drm/nouveau/nvkm/subdev/pci/pcie.c
+++ b/drm/nouveau/nvkm/subdev/pci/pcie.c
@@ -23,6 +23,8 @@
  */
 #include "priv.h"
 
+#include <core/option.h>
+
 static char *nvkm_pcie_speeds[] = {
 	"2.5GT/s",
 	"5.0GT/s",
@@ -106,11 +108,25 @@ nvkm_pcie_init(struct nvkm_pci *pci)
 		pci->func->pcie.init(pci);
 
 	if (pci->pcie.speed != -1)
-		nvkm_pcie_set_link(pci, pci->pcie.speed, pci->pcie.width);
+		nvkm_pcie_set_link(pci, pci->pcie.speed,
+				   pci->pcie.width, false);
 
 	return 0;
 }
 
+int
+nvkm_pcie_fini(struct nvkm_pci *pci, bool suspend)
+{
+	struct nvkm_device *device = pci->subdev.device;
+	if (!device->has_runpm || !suspend)
+		return 0;
+
+	if (!nvkm_boolopt(device->cfgopt, "NvRunpmWorkaround", true))
+		return 0;
+
+	return nvkm_pcie_set_link(pci, NVKM_PCIE_SPEED_8_0, 16, false);
+}
+
 void
 nvkm_pcie_force_aspm_off(struct nvkm_pci *pci, bool status)
 {
@@ -120,7 +136,8 @@ nvkm_pcie_force_aspm_off(struct nvkm_pci *pci, bool status)
 }
 
 int
-nvkm_pcie_set_link(struct nvkm_pci *pci, enum nvkm_pcie_speed speed, u8 width)
+nvkm_pcie_set_link(struct nvkm_pci *pci, enum nvkm_pcie_speed speed, u8 width,
+                   bool save)
 {
 	struct nvkm_subdev *subdev = &pci->subdev;
 	enum nvkm_pcie_speed cur_speed, max_speed;
@@ -154,8 +171,10 @@ nvkm_pcie_set_link(struct nvkm_pci *pci, enum
nvkm_pcie_speed speed, u8 width)
 		speed = max_speed;
 	}
 
-	pci->pcie.speed = speed;
-	pci->pcie.width = width;
+	if (save) {
+		pci->pcie.speed = speed;
+		pci->pcie.width = width;
+	}
 
 	if (speed == cur_speed) {
 		nvkm_debug(subdev, "requested matches current speed\n");
diff --git a/drm/nouveau/nvkm/subdev/pci/priv.h
b/drm/nouveau/nvkm/subdev/pci/priv.h
index 82c78befa..6bea37c15 100644
--- a/drm/nouveau/nvkm/subdev/pci/priv.h
+++ b/drm/nouveau/nvkm/subdev/pci/priv.h
@@ -63,4 +63,5 @@ int gk104_pcie_version_supported(struct nvkm_pci *);
 
 int nvkm_pcie_oneinit(struct nvkm_pci *);
 int nvkm_pcie_init(struct nvkm_pci *);
+int nvkm_pcie_fini(struct nvkm_pci *, bool suspend);
 #endif
-- 
2.21.0

Signed-off-by: Karol Herbst <kherbst at redhat.com>
---
 drm/nouveau/nouveau_drm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drm/nouveau/nouveau_drm.c b/drm/nouveau/nouveau_drm.c
index 78d55c525..6a6819d96 100644
--- a/drm/nouveau/nouveau_drm.c
+++ b/drm/nouveau/nouveau_drm.c
@@ -911,6 +911,7 @@ nouveau_pmops_runtime_suspend(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct drm_device *drm_dev = pci_get_drvdata(pdev);
+	struct nouveau_drm *drm = nouveau_drm(drm_dev);
 	int ret;
 
 	if (!nouveau_pmops_runtime()) {
@@ -920,6 +921,10 @@ nouveau_pmops_runtime_suspend(struct device *dev)
 
 	nouveau_switcheroo_optimus_dsm();
 	ret = nouveau_do_suspend(drm_dev, true);
+	if (ret) {
+		NV_ERROR(drm, "suspend failed with: %d\n", ret);
+		return ret;
+	}
 	pci_save_state(pdev);
 	pci_disable_device(pdev);
 	pci_ignore_hotplug(pdev);
-- 
2.21.0

On Fri, 13 Sep 2019 at 21:33, Karol Herbst <kherbst at redhat.com>
wrote:
>
> v2: fixed compilation error
Is there any need for this patch at all now, if you're forcing 8_0
rather than the pre-DEVINIT speed?
>
> Signed-off-by: Karol Herbst <kherbst at redhat.com>
> Reviewed-by: Lyude Paul <lyude at redhat.com>
> ---
>  drm/nouveau/include/nvkm/subdev/pci.h | 1 +
>  drm/nouveau/nvkm/subdev/pci/pcie.c    | 8 ++++++++
>  2 files changed, 9 insertions(+)
>
> diff --git a/drm/nouveau/include/nvkm/subdev/pci.h
b/drm/nouveau/include/nvkm/subdev/pci.h
> index 4803a4fad..b29101e48 100644
> --- a/drm/nouveau/include/nvkm/subdev/pci.h
> +++ b/drm/nouveau/include/nvkm/subdev/pci.h
> @@ -53,4 +53,5 @@ int gp100_pci_new(struct nvkm_device *, int, struct
nvkm_pci **);
>
>  /* pcie functions */
>  int nvkm_pcie_set_link(struct nvkm_pci *, enum nvkm_pcie_speed, u8 width);
> +enum nvkm_pcie_speed nvkm_pcie_get_speed(struct nvkm_pci *);
>  #endif
> diff --git a/drm/nouveau/nvkm/subdev/pci/pcie.c
b/drm/nouveau/nvkm/subdev/pci/pcie.c
> index 354ac4c85..b4203ff1a 100644
> --- a/drm/nouveau/nvkm/subdev/pci/pcie.c
> +++ b/drm/nouveau/nvkm/subdev/pci/pcie.c
> @@ -177,3 +177,11 @@ nvkm_pcie_set_link(struct nvkm_pci *pci, enum
nvkm_pcie_speed speed, u8 width)
>
>         return ret;
>  }
> +
> +enum nvkm_pcie_speed
> +nvkm_pcie_get_speed(struct nvkm_pci *pci)
> +{
> +       if (!pci || !pci_is_pcie(pci->pdev) ||
!pci->func->pcie.cur_speed)
> +               return -ENODEV;
> +       return pci->func->pcie.cur_speed(pci);
> +}
> --
> 2.21.0
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

On Fri, 13 Sep 2019 at 21:33, Karol Herbst <kherbst at redhat.com>
wrote:
>
> Apperantly things go south if we suspend the device with a PCIe link speed
> set to 2.5. Fixes runtime suspend on my gp107.
>
> This all looks like some bug inside the pci subsystem and I would prefer a
> fix there instead of nouveau, but maybe there is no real nice way of doing
> that outside of drivers?
>
> v2: squashed together patch 4 and 5
> v3: only restore pcie speed on machines with runpm
>     add NvRunpmWorkaround config option to disable the workaround
> v4: only run the code on suspend
>     always put the card into 8.0 mode, not what nouveau detected on load
Why this change?  Also, I know we don't currently touch it, but what
if x16 isn't available on some systems and we break stuff when we do
start playing with link width?
>
> Signed-off-by: Karol Herbst <kherbst at redhat.com>
> Reviewed-by: Lyude Paul <lyude at redhat.com> (v2)
> ---
>  drm/nouveau/include/nvkm/core/device.h |  2 ++
>  drm/nouveau/include/nvkm/subdev/pci.h  |  3 ++-
>  drm/nouveau/nouveau_drm.c              |  1 +
>  drm/nouveau/nvkm/subdev/clk/base.c     |  2 +-
>  drm/nouveau/nvkm/subdev/pci/base.c     |  2 ++
>  drm/nouveau/nvkm/subdev/pci/pcie.c     | 27 ++++++++++++++++++++++----
>  drm/nouveau/nvkm/subdev/pci/priv.h     |  1 +
>  7 files changed, 32 insertions(+), 6 deletions(-)
>
> diff --git a/drm/nouveau/include/nvkm/core/device.h
b/drm/nouveau/include/nvkm/core/device.h
> index 6d55cd047..4fb3f972f 100644
> --- a/drm/nouveau/include/nvkm/core/device.h
> +++ b/drm/nouveau/include/nvkm/core/device.h
> @@ -125,6 +125,8 @@ struct nvkm_device {
>         u8  chiprev;
>         u32 crystal;
>
> +       bool has_runpm;
> +
>         struct {
>                 struct notifier_block nb;
>         } acpi;
> diff --git a/drm/nouveau/include/nvkm/subdev/pci.h
b/drm/nouveau/include/nvkm/subdev/pci.h
> index b29101e48..7245513d9 100644
> --- a/drm/nouveau/include/nvkm/subdev/pci.h
> +++ b/drm/nouveau/include/nvkm/subdev/pci.h
> @@ -52,6 +52,7 @@ int gk104_pci_new(struct nvkm_device *, int, struct
nvkm_pci **);
>  int gp100_pci_new(struct nvkm_device *, int, struct nvkm_pci **);
>
>  /* pcie functions */
> -int nvkm_pcie_set_link(struct nvkm_pci *, enum nvkm_pcie_speed, u8 width);
> +int nvkm_pcie_set_link(struct nvkm_pci *, enum nvkm_pcie_speed, u8 width,
> +                      bool save);
>  enum nvkm_pcie_speed nvkm_pcie_get_speed(struct nvkm_pci *);
>  #endif
> diff --git a/drm/nouveau/nouveau_drm.c b/drm/nouveau/nouveau_drm.c
> index 3d32afe8a..78d55c525 100644
> --- a/drm/nouveau/nouveau_drm.c
> +++ b/drm/nouveau/nouveau_drm.c
> @@ -676,6 +676,7 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
>
>         if (nouveau_atomic)
>                 driver_pci.driver_features |= DRIVER_ATOMIC;
> +       device->has_runpm = nouveau_pmops_runtime();
>
>         drm_dev = drm_dev_alloc(&driver_pci, &pdev->dev);
>         if (IS_ERR(drm_dev)) {
> diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
b/drm/nouveau/nvkm/subdev/clk/base.c
> index ba6a868d4..e30e77453 100644
> --- a/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -277,7 +277,7 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
>         nvkm_debug(subdev, "setting performance state %d\n",
pstatei);
>         clk->pstate = pstatei;
>
> -       nvkm_pcie_set_link(pci, pstate->pcie_speed,
pstate->pcie_width);
> +       nvkm_pcie_set_link(pci, pstate->pcie_speed,
pstate->pcie_width, true);
>
>         if (fb && fb->ram &&
fb->ram->func->calc) {
>                 struct nvkm_ram *ram = fb->ram;
> diff --git a/drm/nouveau/nvkm/subdev/pci/base.c
b/drm/nouveau/nvkm/subdev/pci/base.c
> index ee2431a78..c9b60ef76 100644
> --- a/drm/nouveau/nvkm/subdev/pci/base.c
> +++ b/drm/nouveau/nvkm/subdev/pci/base.c
> @@ -90,6 +90,8 @@ nvkm_pci_fini(struct nvkm_subdev *subdev, bool suspend)
>
>         if (pci->agp.bridge)
>                 nvkm_agp_fini(pci);
> +       else if (pci_is_pcie(pci->pdev))
> +               nvkm_pcie_fini(pci, suspend);
>
>         return 0;
>  }
> diff --git a/drm/nouveau/nvkm/subdev/pci/pcie.c
b/drm/nouveau/nvkm/subdev/pci/pcie.c
> index b4203ff1a..5cab4a240 100644
> --- a/drm/nouveau/nvkm/subdev/pci/pcie.c
> +++ b/drm/nouveau/nvkm/subdev/pci/pcie.c
> @@ -23,6 +23,8 @@
>   */
>  #include "priv.h"
>
> +#include <core/option.h>
> +
>  static char *nvkm_pcie_speeds[] = {
>         "2.5GT/s",
>         "5.0GT/s",
> @@ -106,11 +108,25 @@ nvkm_pcie_init(struct nvkm_pci *pci)
>                 pci->func->pcie.init(pci);
>
>         if (pci->pcie.speed != -1)
> -               nvkm_pcie_set_link(pci, pci->pcie.speed,
pci->pcie.width);
> +               nvkm_pcie_set_link(pci, pci->pcie.speed,
> +                                  pci->pcie.width, false);
>
>         return 0;
>  }
>
> +int
> +nvkm_pcie_fini(struct nvkm_pci *pci, bool suspend)
> +{
> +       struct nvkm_device *device = pci->subdev.device;
> +       if (!device->has_runpm || !suspend)
> +               return 0;
> +
> +       if (!nvkm_boolopt(device->cfgopt, "NvRunpmWorkaround",
true))
> +               return 0;
> +
> +       return nvkm_pcie_set_link(pci, NVKM_PCIE_SPEED_8_0, 16, false);
> +}
> +
>  void
>  nvkm_pcie_force_aspm_off(struct nvkm_pci *pci, bool status)
>  {
> @@ -120,7 +136,8 @@ nvkm_pcie_force_aspm_off(struct nvkm_pci *pci, bool
status)
>  }
>
>  int
> -nvkm_pcie_set_link(struct nvkm_pci *pci, enum nvkm_pcie_speed speed, u8
width)
> +nvkm_pcie_set_link(struct nvkm_pci *pci, enum nvkm_pcie_speed speed, u8
width,
> +                   bool save)
>  {
>         struct nvkm_subdev *subdev = &pci->subdev;
>         enum nvkm_pcie_speed cur_speed, max_speed;
> @@ -154,8 +171,10 @@ nvkm_pcie_set_link(struct nvkm_pci *pci, enum
nvkm_pcie_speed speed, u8 width)
>                 speed = max_speed;
>         }
>
> -       pci->pcie.speed = speed;
> -       pci->pcie.width = width;
> +       if (save) {
> +               pci->pcie.speed = speed;
> +               pci->pcie.width = width;
> +       }
>
>         if (speed == cur_speed) {
>                 nvkm_debug(subdev, "requested matches current
speed\n");
> diff --git a/drm/nouveau/nvkm/subdev/pci/priv.h
b/drm/nouveau/nvkm/subdev/pci/priv.h
> index 82c78befa..6bea37c15 100644
> --- a/drm/nouveau/nvkm/subdev/pci/priv.h
> +++ b/drm/nouveau/nvkm/subdev/pci/priv.h
> @@ -63,4 +63,5 @@ int gk104_pcie_version_supported(struct nvkm_pci *);
>
>  int nvkm_pcie_oneinit(struct nvkm_pci *);
>  int nvkm_pcie_init(struct nvkm_pci *);
> +int nvkm_pcie_fini(struct nvkm_pci *, bool suspend);
>  #endif
> --
> 2.21.0
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

Reviewed-by: Lyude Paul <lyude at redhat.com> for v2

On Fri, 2019-09-13 at 13:33 +0200, Karol Herbst wrote:
> v2: add force disable ASPM func pointer
> 
> Signed-off-by: Karol Herbst <kherbst at redhat.com>
> Reviewed-by: Lyude Paul <lyude at redhat.com> (v1)
> ---
>  drm/nouveau/nvkm/subdev/pci/gk104.c |  8 ++++----
>  drm/nouveau/nvkm/subdev/pci/gp100.c | 11 +++++++++++
>  drm/nouveau/nvkm/subdev/pci/priv.h  |  5 +++++
>  3 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/drm/nouveau/nvkm/subdev/pci/gk104.c
> b/drm/nouveau/nvkm/subdev/pci/gk104.c
> index 804ef017f..61c4e76e8 100644
> --- a/drm/nouveau/nvkm/subdev/pci/gk104.c
> +++ b/drm/nouveau/nvkm/subdev/pci/gk104.c
> @@ -25,7 +25,7 @@
>  
>  #include <subdev/timer.h>
>  
> -static int
> +int
>  gk104_pcie_version_supported(struct nvkm_pci *pci)
>  {
>  	return (nvkm_rd32(pci->subdev.device, 0x8c1c0) & 0x4) == 0x4 ? 2 :
1;
> @@ -110,7 +110,7 @@ gk104_pcie_lnkctl_speed(struct nvkm_pci *pci)
>  	return -1;
>  }
>  
> -static enum nvkm_pcie_speed
> +enum nvkm_pcie_speed
>  gk104_pcie_max_speed(struct nvkm_pci *pci)
>  {
>  	u32 max_speed = nvkm_rd32(pci->subdev.device, 0x8c1c0) & 0x300000;
> @@ -156,7 +156,7 @@ gk104_pcie_set_link_speed(struct nvkm_pci *pci, enum
> nvkm_pcie_speed speed)
>  	return 0;
>  }
>  
> -static int
> +int
>  gk104_pcie_init(struct nvkm_pci * pci)
>  {
>  	enum nvkm_pcie_speed lnkctl_speed, max_speed, cap_speed;
> @@ -188,7 +188,7 @@ gk104_pcie_init(struct nvkm_pci * pci)
>  	return 0;
>  }
>  
> -static int
> +int
>  gk104_pcie_set_link(struct nvkm_pci *pci, enum nvkm_pcie_speed speed, u8
> width)
>  {
>  	struct nvkm_subdev *subdev = &pci->subdev;
> diff --git a/drm/nouveau/nvkm/subdev/pci/gp100.c
> b/drm/nouveau/nvkm/subdev/pci/gp100.c
> index 82c5234a0..163233f16 100644
> --- a/drm/nouveau/nvkm/subdev/pci/gp100.c
> +++ b/drm/nouveau/nvkm/subdev/pci/gp100.c
> @@ -35,6 +35,17 @@ gp100_pci_func = {
>  	.wr08 = nv40_pci_wr08,
>  	.wr32 = nv40_pci_wr32,
>  	.msi_rearm = gp100_pci_msi_rearm,
> +
> +	.pcie.init = gk104_pcie_init,
> +	.pcie.set_link = gk104_pcie_set_link,
> +
> +	.pcie.max_speed = gk104_pcie_max_speed,
> +	.pcie.cur_speed = g84_pcie_cur_speed,
> +
> +	.pcie.set_version = gf100_pcie_set_version,
> +	.pcie.version = gf100_pcie_version,
> +	.pcie.version_supported = gk104_pcie_version_supported,
> +	.pcie.force_aspm_off = g84_pcie_force_aspm_off,
>  };
>  
>  int
> diff --git a/drm/nouveau/nvkm/subdev/pci/priv.h
> b/drm/nouveau/nvkm/subdev/pci/priv.h
> index c6a9ef330..82c78befa 100644
> --- a/drm/nouveau/nvkm/subdev/pci/priv.h
> +++ b/drm/nouveau/nvkm/subdev/pci/priv.h
> @@ -56,6 +56,11 @@ int gf100_pcie_cap_speed(struct nvkm_pci *);
>  int gf100_pcie_init(struct nvkm_pci *);
>  int gf100_pcie_set_link(struct nvkm_pci *, enum nvkm_pcie_speed, u8);
>  
> +int gk104_pcie_init(struct nvkm_pci *);
> +int gk104_pcie_set_link(struct nvkm_pci *, enum nvkm_pcie_speed, u8
width);
> +enum nvkm_pcie_speed gk104_pcie_max_speed(struct nvkm_pci *);
> +int gk104_pcie_version_supported(struct nvkm_pci *);
> +
>  int nvkm_pcie_oneinit(struct nvkm_pci *);
>  int nvkm_pcie_init(struct nvkm_pci *);
>  #endif
-- 
Cheers,
	Lyude Paul

On Fri, 2019-09-13 at 13:33 +0200, Karol Herbst wrote:
> Apperantly things go south if we suspend the device with a PCIe link speed
> set to 2.5. Fixes runtime suspend on my gp107.
> 
> This all looks like some bug inside the pci subsystem and I would prefer a
> fix there instead of nouveau, but maybe there is no real nice way of doing
> that outside of drivers?
> 
> v2: squashed together patch 4 and 5
> v3: only restore pcie speed on machines with runpm
>     add NvRunpmWorkaround config option to disable the workaround
> v4: only run the code on suspend
>     always put the card into 8.0 mode, not what nouveau detected on load
> 
> Signed-off-by: Karol Herbst <kherbst at redhat.com>
> Reviewed-by: Lyude Paul <lyude at redhat.com> (v2)
> ---
>  drm/nouveau/include/nvkm/core/device.h |  2 ++
>  drm/nouveau/include/nvkm/subdev/pci.h  |  3 ++-
>  drm/nouveau/nouveau_drm.c              |  1 +
>  drm/nouveau/nvkm/subdev/clk/base.c     |  2 +-
>  drm/nouveau/nvkm/subdev/pci/base.c     |  2 ++
>  drm/nouveau/nvkm/subdev/pci/pcie.c     | 27 ++++++++++++++++++++++----
>  drm/nouveau/nvkm/subdev/pci/priv.h     |  1 +
>  7 files changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/drm/nouveau/include/nvkm/core/device.h
> b/drm/nouveau/include/nvkm/core/device.h
> index 6d55cd047..4fb3f972f 100644
> --- a/drm/nouveau/include/nvkm/core/device.h
> +++ b/drm/nouveau/include/nvkm/core/device.h
> @@ -125,6 +125,8 @@ struct nvkm_device {
>  	u8  chiprev;
>  	u32 crystal;
>  
> +	bool has_runpm;
> +
This could be a bitfield, up to you. Other then that:

Reviewed-by: Lyude Paul <lyude at redhat.com>
>  	struct {
>  		struct notifier_block nb;
>  	} acpi;
> diff --git a/drm/nouveau/include/nvkm/subdev/pci.h
> b/drm/nouveau/include/nvkm/subdev/pci.h
> index b29101e48..7245513d9 100644
> --- a/drm/nouveau/include/nvkm/subdev/pci.h
> +++ b/drm/nouveau/include/nvkm/subdev/pci.h
> @@ -52,6 +52,7 @@ int gk104_pci_new(struct nvkm_device *, int, struct
> nvkm_pci **);
>  int gp100_pci_new(struct nvkm_device *, int, struct nvkm_pci **);
>  
>  /* pcie functions */
> -int nvkm_pcie_set_link(struct nvkm_pci *, enum nvkm_pcie_speed, u8 width);
> +int nvkm_pcie_set_link(struct nvkm_pci *, enum nvkm_pcie_speed, u8 width,
> +		       bool save);
>  enum nvkm_pcie_speed nvkm_pcie_get_speed(struct nvkm_pci *);
>  #endif
> diff --git a/drm/nouveau/nouveau_drm.c b/drm/nouveau/nouveau_drm.c
> index 3d32afe8a..78d55c525 100644
> --- a/drm/nouveau/nouveau_drm.c
> +++ b/drm/nouveau/nouveau_drm.c
> @@ -676,6 +676,7 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
>  
>  	if (nouveau_atomic)
>  		driver_pci.driver_features |= DRIVER_ATOMIC;
> +	device->has_runpm = nouveau_pmops_runtime();
>  
>  	drm_dev = drm_dev_alloc(&driver_pci, &pdev->dev);
>  	if (IS_ERR(drm_dev)) {
> diff --git a/drm/nouveau/nvkm/subdev/clk/base.c
> b/drm/nouveau/nvkm/subdev/clk/base.c
> index ba6a868d4..e30e77453 100644
> --- a/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -277,7 +277,7 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
>  	nvkm_debug(subdev, "setting performance state %d\n", pstatei);
>  	clk->pstate = pstatei;
>  
> -	nvkm_pcie_set_link(pci, pstate->pcie_speed, pstate->pcie_width);
> +	nvkm_pcie_set_link(pci, pstate->pcie_speed, pstate->pcie_width,
true);
>  
>  	if (fb && fb->ram && fb->ram->func->calc) {
>  		struct nvkm_ram *ram = fb->ram;
> diff --git a/drm/nouveau/nvkm/subdev/pci/base.c
> b/drm/nouveau/nvkm/subdev/pci/base.c
> index ee2431a78..c9b60ef76 100644
> --- a/drm/nouveau/nvkm/subdev/pci/base.c
> +++ b/drm/nouveau/nvkm/subdev/pci/base.c
> @@ -90,6 +90,8 @@ nvkm_pci_fini(struct nvkm_subdev *subdev, bool suspend)
>  
>  	if (pci->agp.bridge)
>  		nvkm_agp_fini(pci);
> +	else if (pci_is_pcie(pci->pdev))
> +		nvkm_pcie_fini(pci, suspend);
>  
>  	return 0;
>  }
> diff --git a/drm/nouveau/nvkm/subdev/pci/pcie.c
> b/drm/nouveau/nvkm/subdev/pci/pcie.c
> index b4203ff1a..5cab4a240 100644
> --- a/drm/nouveau/nvkm/subdev/pci/pcie.c
> +++ b/drm/nouveau/nvkm/subdev/pci/pcie.c
> @@ -23,6 +23,8 @@
>   */
>  #include "priv.h"
>  
> +#include <core/option.h>
> +
>  static char *nvkm_pcie_speeds[] = {
>  	"2.5GT/s",
>  	"5.0GT/s",
> @@ -106,11 +108,25 @@ nvkm_pcie_init(struct nvkm_pci *pci)
>  		pci->func->pcie.init(pci);
>  
>  	if (pci->pcie.speed != -1)
> -		nvkm_pcie_set_link(pci, pci->pcie.speed, pci->pcie.width);
> +		nvkm_pcie_set_link(pci, pci->pcie.speed,
> +				   pci->pcie.width, false);
>  
>  	return 0;
>  }
>  
> +int
> +nvkm_pcie_fini(struct nvkm_pci *pci, bool suspend)
> +{
> +	struct nvkm_device *device = pci->subdev.device;
> +	if (!device->has_runpm || !suspend)
> +		return 0;
> +
> +	if (!nvkm_boolopt(device->cfgopt, "NvRunpmWorkaround",
true))
> +		return 0;
> +
> +	return nvkm_pcie_set_link(pci, NVKM_PCIE_SPEED_8_0, 16, false);
> +}
> +
>  void
>  nvkm_pcie_force_aspm_off(struct nvkm_pci *pci, bool status)
>  {
> @@ -120,7 +136,8 @@ nvkm_pcie_force_aspm_off(struct nvkm_pci *pci, bool
> status)
>  }
>  
>  int
> -nvkm_pcie_set_link(struct nvkm_pci *pci, enum nvkm_pcie_speed speed, u8
> width)
> +nvkm_pcie_set_link(struct nvkm_pci *pci, enum nvkm_pcie_speed speed, u8
> width,
> +                   bool save)
>  {
>  	struct nvkm_subdev *subdev = &pci->subdev;
>  	enum nvkm_pcie_speed cur_speed, max_speed;
> @@ -154,8 +171,10 @@ nvkm_pcie_set_link(struct nvkm_pci *pci, enum
> nvkm_pcie_speed speed, u8 width)
>  		speed = max_speed;
>  	}
>  
> -	pci->pcie.speed = speed;
> -	pci->pcie.width = width;
> +	if (save) {
> +		pci->pcie.speed = speed;
> +		pci->pcie.width = width;
> +	}
>  
>  	if (speed == cur_speed) {
>  		nvkm_debug(subdev, "requested matches current speed\n");
> diff --git a/drm/nouveau/nvkm/subdev/pci/priv.h
> b/drm/nouveau/nvkm/subdev/pci/priv.h
> index 82c78befa..6bea37c15 100644
> --- a/drm/nouveau/nvkm/subdev/pci/priv.h
> +++ b/drm/nouveau/nvkm/subdev/pci/priv.h
> @@ -63,4 +63,5 @@ int gk104_pcie_version_supported(struct nvkm_pci *);
>  
>  int nvkm_pcie_oneinit(struct nvkm_pci *);
>  int nvkm_pcie_init(struct nvkm_pci *);
> +int nvkm_pcie_fini(struct nvkm_pci *, bool suspend);
>  #endif
-- 
Cheers,
	Lyude Paul