thr3ads.net - Nouveau - [Nouveau] [PATCH 1/5] drm: don't set the pci power state if the pci subsystem handles the ACPI bits [May 2019]

If this information is useful, please help other people find it:
Share via:

Karol Herbst

2019-May-04 16:32 UTC

[Nouveau] [PATCH 0/5] Potential fix for runpm issues on various laptops

While investigating the runpm issues on my GP107 I noticed that something
inside devinit makes runpm break. If Nouveau loads up to the point right
before doing devinit, runpm works without any issues, if devinit is ran,
not anymore.

Out of curiousity I even tried to "bisect" devinit by not running it
on
vbios provided signed PMU image, but on the devinit parser we have inside
Nouveau.
Allthough this one isn't as feature complete as the vbios one, I was able
to reproduce the runpm issues as well. From that point I was able to only
run a certain amount of commands until I got to some PCIe initialization
code inside devinit which trigger those runpm issues.

Devinit on my GPU was changing the PCIe link from 8.0 to 2.5, reversing
that on the fini path makes runpm work again.

There are a few other things going on, but with my limited knowledge about
PCIe in general, the change in the link speed sounded like it could cause
issues on resume if the controller and the device disagree on the actual
link.

Maybe this is just a bug within the PCI subsystem inside linux instead and
the controller has to be forced to do _something_?

Anyway, with this runpm seems to work nicely on my machine. Secure booting
the gr (even with my workaround applied I need anyway) might fail after
the GPU got runtime resumed though...

Karol Herbst (5):
  drm: don't set the pci power state if the pci subsystem handles the
    ACPI bits
  pci: enable pcie link changes for pascal
  pci: add nvkm_pcie_get_speed
  pci: save the boot pcie link speed
  pci: restore the boot pcie link speed on fini

 drm/nouveau/include/nvkm/subdev/pci.h |  6 ++++--
 drm/nouveau/nouveau_acpi.c            |  6 +++---
 drm/nouveau/nouveau_acpi.h            |  4 ++--
 drm/nouveau/nouveau_drm.c             | 15 +++++++++----
 drm/nouveau/nouveau_drv.h             |  2 ++
 drm/nouveau/nvkm/subdev/pci/base.c    |  9 ++++++--
 drm/nouveau/nvkm/subdev/pci/gk104.c   |  8 +++----
 drm/nouveau/nvkm/subdev/pci/gp100.c   | 10 +++++++++
 drm/nouveau/nvkm/subdev/pci/pcie.c    | 31 +++++++++++++++++++++++----
 drm/nouveau/nvkm/subdev/pci/priv.h    |  7 ++++++
 10 files changed, 77 insertions(+), 21 deletions(-)

-- 
2.20.1

Karol Herbst

2019-May-04 16:32 UTC

head link

[Nouveau] [PATCH 1/5] drm: don't set the pci power state if the pci subsystem handles the ACPI bits

Signed-off-by: Karol Herbst <kherbst at redhat.com>
---
 drm/nouveau/nouveau_acpi.c |  6 +++---
 drm/nouveau/nouveau_acpi.h |  4 ++--
 drm/nouveau/nouveau_drm.c  | 15 +++++++++++----
 drm/nouveau/nouveau_drv.h  |  2 ++
 4 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
index ffb19585..49b11d22 100644
--- a/drm/nouveau/nouveau_acpi.c
+++ b/drm/nouveau/nouveau_acpi.c
@@ -359,11 +359,11 @@ void nouveau_register_dsm_handler(void)
 }
 
 /* Must be called for Optimus models before the card can be turned off */
-void nouveau_switcheroo_optimus_dsm(void)
+bool nouveau_switcheroo_optimus_dsm(void)
 {
 	u32 result = 0;
 	if (!nouveau_dsm_priv.optimus_detected || nouveau_dsm_priv.optimus_skip_dsm)
-		return;
+		return false;
 
 	if (nouveau_dsm_priv.optimus_flags_detected)
 		nouveau_optimus_dsm(nouveau_dsm_priv.dhandle, NOUVEAU_DSM_OPTIMUS_FLAGS,
@@ -371,7 +371,7 @@ void nouveau_switcheroo_optimus_dsm(void)
 
 	nouveau_optimus_dsm(nouveau_dsm_priv.dhandle, NOUVEAU_DSM_OPTIMUS_CAPS,
 		NOUVEAU_DSM_OPTIMUS_SET_POWERDOWN, &result);
-
+	return true;
 }
 
 void nouveau_unregister_dsm_handler(void)
diff --git a/drm/nouveau/nouveau_acpi.h b/drm/nouveau/nouveau_acpi.h
index b86294fc..09b2a82d 100644
--- a/drm/nouveau/nouveau_acpi.h
+++ b/drm/nouveau/nouveau_acpi.h
@@ -9,7 +9,7 @@ bool nouveau_is_optimus(void);
 bool nouveau_is_v1_dsm(void);
 void nouveau_register_dsm_handler(void);
 void nouveau_unregister_dsm_handler(void);
-void nouveau_switcheroo_optimus_dsm(void);
+bool nouveau_switcheroo_optimus_dsm(void);
 int nouveau_acpi_get_bios_chunk(uint8_t *bios, int offset, int len);
 bool nouveau_acpi_rom_supported(struct device *);
 void *nouveau_acpi_edid(struct drm_device *, struct drm_connector *);
@@ -18,7 +18,7 @@ static inline bool nouveau_is_optimus(void) { return false; };
 static inline bool nouveau_is_v1_dsm(void) { return false; };
 static inline void nouveau_register_dsm_handler(void) {}
 static inline void nouveau_unregister_dsm_handler(void) {}
-static inline void nouveau_switcheroo_optimus_dsm(void) {}
+static inline bool nouveau_switcheroo_optimus_dsm(void) { return false; }
 static inline bool nouveau_acpi_rom_supported(struct device *dev) { return
false; }
 static inline int nouveau_acpi_get_bios_chunk(uint8_t *bios, int offset, int
len) { return -EINVAL; }
 static inline void *nouveau_acpi_edid(struct drm_device *dev, struct
drm_connector *connector) { return NULL; }
diff --git a/drm/nouveau/nouveau_drm.c b/drm/nouveau/nouveau_drm.c
index 5020265b..405efda2 100644
--- a/drm/nouveau/nouveau_drm.c
+++ b/drm/nouveau/nouveau_drm.c
@@ -903,6 +903,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()) {
@@ -910,12 +911,15 @@ nouveau_pmops_runtime_suspend(struct device *dev)
 		return -EBUSY;
 	}
 
-	nouveau_switcheroo_optimus_dsm();
+	drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
+	drm->runpm_dsm = nouveau_switcheroo_optimus_dsm();
 	ret = nouveau_do_suspend(drm_dev, true);
 	pci_save_state(pdev);
 	pci_disable_device(pdev);
 	pci_ignore_hotplug(pdev);
-	pci_set_power_state(pdev, PCI_D3cold);
+	if (drm->runpm_dsm)
+		pci_set_power_state(pdev, PCI_D3cold);
+
 	drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF;
 	return ret;
 }
@@ -925,7 +929,8 @@ nouveau_pmops_runtime_resume(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct drm_device *drm_dev = pci_get_drvdata(pdev);
-	struct nvif_device *device = &nouveau_drm(drm_dev)->client.device;
+	struct nouveau_drm *drm = nouveau_drm(drm_dev);
+	struct nvif_device *device = &drm->client.device;
 	int ret;
 
 	if (!nouveau_pmops_runtime()) {
@@ -933,7 +938,9 @@ nouveau_pmops_runtime_resume(struct device *dev)
 		return -EBUSY;
 	}
 
-	pci_set_power_state(pdev, PCI_D0);
+	drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
+	if (drm->runpm_dsm)
+		pci_set_power_state(pdev, PCI_D0);
 	pci_restore_state(pdev);
 	ret = pci_enable_device(pdev);
 	if (ret)
diff --git a/drm/nouveau/nouveau_drv.h b/drm/nouveau/nouveau_drv.h
index da847244..941600e9 100644
--- a/drm/nouveau/nouveau_drv.h
+++ b/drm/nouveau/nouveau_drv.h
@@ -214,6 +214,8 @@ struct nouveau_drm {
 	struct nouveau_svm *svm;
 
 	struct nouveau_dmem *dmem;
+
+	bool runpm_dsm;
 };
 
 static inline struct nouveau_drm *
-- 
2.20.1

Karol Herbst

2019-May-04 16:32 UTC

head link

[Nouveau] [PATCH 2/5] pci: enable pcie link changes for pascal

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

diff --git a/drm/nouveau/nvkm/subdev/pci/gk104.c
b/drm/nouveau/nvkm/subdev/pci/gk104.c
index e6803050..66489018 100644
--- a/drm/nouveau/nvkm/subdev/pci/gk104.c
+++ b/drm/nouveau/nvkm/subdev/pci/gk104.c
@@ -23,7 +23,7 @@
  */
 #include "priv.h"
 
-static int
+int
 gk104_pcie_version_supported(struct nvkm_pci *pci)
 {
 	return (nvkm_rd32(pci->subdev.device, 0x8c1c0) & 0x4) == 0x4 ? 2 : 1;
@@ -108,7 +108,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;
@@ -146,7 +146,7 @@ gk104_pcie_set_link_speed(struct nvkm_pci *pci, enum
nvkm_pcie_speed speed)
 	nvkm_mask(device, 0x8c040, 0x1, 0x1);
 }
 
-static int
+int
 gk104_pcie_init(struct nvkm_pci * pci)
 {
 	enum nvkm_pcie_speed lnkctl_speed, max_speed, cap_speed;
@@ -178,7 +178,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 82c5234a..eb19c7a4 100644
--- a/drm/nouveau/nvkm/subdev/pci/gp100.c
+++ b/drm/nouveau/nvkm/subdev/pci/gp100.c
@@ -35,6 +35,16 @@ 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,
 };
 
 int
diff --git a/drm/nouveau/nvkm/subdev/pci/priv.h
b/drm/nouveau/nvkm/subdev/pci/priv.h
index c17f6063..a0d4c007 100644
--- a/drm/nouveau/nvkm/subdev/pci/priv.h
+++ b/drm/nouveau/nvkm/subdev/pci/priv.h
@@ -54,6 +54,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.20.1

Karol Herbst

2019-May-04 16:32 UTC

head link

[Nouveau] [PATCH 3/5] pci: add nvkm_pcie_get_speed

Signed-off-by: Karol Herbst <kherbst 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 23803cc8..1fdf3098 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 d71e5db5..eb405dea 100644
--- a/drm/nouveau/nvkm/subdev/pci/pcie.c
+++ b/drm/nouveau/nvkm/subdev/pci/pcie.c
@@ -163,3 +163,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->pcie.cur_speed)
+		return -ENOSYS;
+	return pci->func->pcie.cur_speed(pci);
+}
-- 
2.20.1

Karol Herbst

2019-May-04 16:32 UTC

head link

[Nouveau] [PATCH 4/5] pci: save the boot pcie link speed

Signed-off-by: Karol Herbst <kherbst at redhat.com>
---
 drm/nouveau/include/nvkm/subdev/pci.h |  5 +++--
 drm/nouveau/nvkm/subdev/pci/base.c    |  7 +++++--
 drm/nouveau/nvkm/subdev/pci/pcie.c    | 15 +++++++++++----
 drm/nouveau/nvkm/subdev/pci/priv.h    |  1 +
 4 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drm/nouveau/include/nvkm/subdev/pci.h
b/drm/nouveau/include/nvkm/subdev/pci.h
index 1fdf3098..b23793a2 100644
--- a/drm/nouveau/include/nvkm/subdev/pci.h
+++ b/drm/nouveau/include/nvkm/subdev/pci.h
@@ -26,8 +26,9 @@ struct nvkm_pci {
 	} agp;
 
 	struct {
-		enum nvkm_pcie_speed speed;
-		u8 width;
+		enum nvkm_pcie_speed cur_speed;
+		enum nvkm_pcie_speed def_speed;
+		u8 cur_width;
 	} pcie;
 
 	bool msi;
diff --git a/drm/nouveau/nvkm/subdev/pci/base.c
b/drm/nouveau/nvkm/subdev/pci/base.c
index ee2431a7..b6ebee6f 100644
--- a/drm/nouveau/nvkm/subdev/pci/base.c
+++ b/drm/nouveau/nvkm/subdev/pci/base.c
@@ -100,6 +100,8 @@ nvkm_pci_preinit(struct nvkm_subdev *subdev)
 	struct nvkm_pci *pci = nvkm_pci(subdev);
 	if (pci->agp.bridge)
 		nvkm_agp_preinit(pci);
+	else if (pci_is_pcie(pci->pdev))
+		nvkm_pcie_preinit(pci);
 	return 0;
 }
 
@@ -193,8 +195,9 @@ nvkm_pci_new_(const struct nvkm_pci_func *func, struct
nvkm_device *device,
 	pci->func = func;
 	pci->pdev = device->func->pci(device)->pdev;
 	pci->irq = -1;
-	pci->pcie.speed = -1;
-	pci->pcie.width = -1;
+	pci->pcie.cur_speed = -1;
+	pci->pcie.def_speed = -1;
+	pci->pcie.cur_width = -1;
 
 	if (device->type == NVKM_DEVICE_AGP)
 		nvkm_agp_ctor(pci);
diff --git a/drm/nouveau/nvkm/subdev/pci/pcie.c
b/drm/nouveau/nvkm/subdev/pci/pcie.c
index eb405dea..727b5d6a 100644
--- a/drm/nouveau/nvkm/subdev/pci/pcie.c
+++ b/drm/nouveau/nvkm/subdev/pci/pcie.c
@@ -85,6 +85,13 @@ nvkm_pcie_oneinit(struct nvkm_pci *pci)
 	return 0;
 }
 
+int
+nvkm_pcie_preinit(struct nvkm_pci *pci)
+{
+	pci->pcie.def_speed = nvkm_pcie_get_speed(pci);
+	return 0;
+}
+
 int
 nvkm_pcie_init(struct nvkm_pci *pci)
 {
@@ -105,8 +112,8 @@ nvkm_pcie_init(struct nvkm_pci *pci)
 	if (pci->func->pcie.init)
 		pci->func->pcie.init(pci);
 
-	if (pci->pcie.speed != -1)
-		nvkm_pcie_set_link(pci, pci->pcie.speed, pci->pcie.width);
+	if (pci->pcie.cur_speed != -1)
+		nvkm_pcie_set_link(pci, pci->pcie.cur_speed, pci->pcie.cur_width);
 
 	return 0;
 }
@@ -146,8 +153,8 @@ 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;
+	pci->pcie.cur_speed = speed;
+	pci->pcie.cur_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 a0d4c007..dc3ae3cd 100644
--- a/drm/nouveau/nvkm/subdev/pci/priv.h
+++ b/drm/nouveau/nvkm/subdev/pci/priv.h
@@ -60,5 +60,6 @@ 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_preinit(struct nvkm_pci *);
 int nvkm_pcie_init(struct nvkm_pci *);
 #endif
-- 
2.20.1

Karol Herbst

2019-May-04 16:32 UTC

head link

[Nouveau] [PATCH 5/5] pci: restore the boot pcie link speed on fini

fixes runtime suspend on my gp107

Signed-off-by: Karol Herbst <kherbst at redhat.com>
---
 drm/nouveau/nvkm/subdev/pci/base.c | 2 ++
 drm/nouveau/nvkm/subdev/pci/pcie.c | 8 ++++++++
 drm/nouveau/nvkm/subdev/pci/priv.h | 1 +
 3 files changed, 11 insertions(+)

diff --git a/drm/nouveau/nvkm/subdev/pci/base.c
b/drm/nouveau/nvkm/subdev/pci/base.c
index b6ebee6f..d9fb5a83 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);
 
 	return 0;
 }
diff --git a/drm/nouveau/nvkm/subdev/pci/pcie.c
b/drm/nouveau/nvkm/subdev/pci/pcie.c
index 727b5d6a..00c07fd8 100644
--- a/drm/nouveau/nvkm/subdev/pci/pcie.c
+++ b/drm/nouveau/nvkm/subdev/pci/pcie.c
@@ -118,6 +118,14 @@ nvkm_pcie_init(struct nvkm_pci *pci)
 	return 0;
 }
 
+int
+nvkm_pcie_fini(struct nvkm_pci *pci)
+{
+	if (!IS_ERR_VALUE(pci->pcie.def_speed))
+		return nvkm_pcie_set_link(pci, pci->pcie.def_speed, 16);
+	return 0;
+}
+
 int
 nvkm_pcie_set_link(struct nvkm_pci *pci, enum nvkm_pcie_speed speed, u8 width)
 {
diff --git a/drm/nouveau/nvkm/subdev/pci/priv.h
b/drm/nouveau/nvkm/subdev/pci/priv.h
index dc3ae3cd..e7744671 100644
--- a/drm/nouveau/nvkm/subdev/pci/priv.h
+++ b/drm/nouveau/nvkm/subdev/pci/priv.h
@@ -62,4 +62,5 @@ int gk104_pcie_version_supported(struct nvkm_pci *);
 int nvkm_pcie_oneinit(struct nvkm_pci *);
 int nvkm_pcie_preinit(struct nvkm_pci *);
 int nvkm_pcie_init(struct nvkm_pci *);
+int nvkm_pcie_fini(struct nvkm_pci *);
 #endif
-- 
2.20.1

Lyude Paul

2019-May-07 19:16 UTC

head link

[Nouveau] [PATCH 1/5] drm: don't set the pci power state if the pci subsystem handles the ACPI bits

On Sat, 2019-05-04 at 18:32 +0200, Karol Herbst wrote:> Signed-off-by: Karol Herbst <kherbst at redhat.com>
> ---
>  drm/nouveau/nouveau_acpi.c |  6 +++---
>  drm/nouveau/nouveau_acpi.h |  4 ++--
>  drm/nouveau/nouveau_drm.c  | 15 +++++++++++----
>  drm/nouveau/nouveau_drv.h  |  2 ++
>  4 files changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
> index ffb19585..49b11d22 100644
> --- a/drm/nouveau/nouveau_acpi.c
> +++ b/drm/nouveau/nouveau_acpi.c
> @@ -359,11 +359,11 @@ void nouveau_register_dsm_handler(void)
>  }
>  
>  /* Must be called for Optimus models before the card can be turned off */
> -void nouveau_switcheroo_optimus_dsm(void)
> +bool nouveau_switcheroo_optimus_dsm(void)
>  {
>  	u32 result = 0;
>  	if (!nouveau_dsm_priv.optimus_detected ||
> nouveau_dsm_priv.optimus_skip_dsm)
> -		return;
> +		return false;
>  
>  	if (nouveau_dsm_priv.optimus_flags_detected)
>  		nouveau_optimus_dsm(nouveau_dsm_priv.dhandle,
> NOUVEAU_DSM_OPTIMUS_FLAGS,
> @@ -371,7 +371,7 @@ void nouveau_switcheroo_optimus_dsm(void)
>  
>  	nouveau_optimus_dsm(nouveau_dsm_priv.dhandle,
> NOUVEAU_DSM_OPTIMUS_CAPS,
>  		NOUVEAU_DSM_OPTIMUS_SET_POWERDOWN, &result);
> -
> +	return true;
>  }
>  
>  void nouveau_unregister_dsm_handler(void)
> diff --git a/drm/nouveau/nouveau_acpi.h b/drm/nouveau/nouveau_acpi.h
> index b86294fc..09b2a82d 100644
> --- a/drm/nouveau/nouveau_acpi.h
> +++ b/drm/nouveau/nouveau_acpi.h
> @@ -9,7 +9,7 @@ bool nouveau_is_optimus(void);
>  bool nouveau_is_v1_dsm(void);
>  void nouveau_register_dsm_handler(void);
>  void nouveau_unregister_dsm_handler(void);
> -void nouveau_switcheroo_optimus_dsm(void);
> +bool nouveau_switcheroo_optimus_dsm(void);
>  int nouveau_acpi_get_bios_chunk(uint8_t *bios, int offset, int len);
>  bool nouveau_acpi_rom_supported(struct device *);
>  void *nouveau_acpi_edid(struct drm_device *, struct drm_connector *);
> @@ -18,7 +18,7 @@ static inline bool nouveau_is_optimus(void) { return
> false; };
>  static inline bool nouveau_is_v1_dsm(void) { return false; };
>  static inline void nouveau_register_dsm_handler(void) {}
>  static inline void nouveau_unregister_dsm_handler(void) {}
> -static inline void nouveau_switcheroo_optimus_dsm(void) {}
> +static inline bool nouveau_switcheroo_optimus_dsm(void) { return false; }
>  static inline bool nouveau_acpi_rom_supported(struct device *dev) { return
> false; }
>  static inline int nouveau_acpi_get_bios_chunk(uint8_t *bios, int offset,
> int len) { return -EINVAL; }
>  static inline void *nouveau_acpi_edid(struct drm_device *dev, struct
> drm_connector *connector) { return NULL; }
> diff --git a/drm/nouveau/nouveau_drm.c b/drm/nouveau/nouveau_drm.c
> index 5020265b..405efda2 100644
> --- a/drm/nouveau/nouveau_drm.c
> +++ b/drm/nouveau/nouveau_drm.c
> @@ -903,6 +903,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()) {
> @@ -910,12 +911,15 @@ nouveau_pmops_runtime_suspend(struct device *dev)
>  		return -EBUSY;
>  	}
>  
> -	nouveau_switcheroo_optimus_dsm();
> +	drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
> +	drm->runpm_dsm = nouveau_switcheroo_optimus_dsm();
It seems like all nouveau_switcheroo_optimus_dsm() does here is check
nouveau_dsm_priv.optimus_detected/optimus_skip_dsm. Why not just make
add a function to check these that we call when setting up the DRM
device, then store the result of that into drm->runpm_dsm instead of
changing it every time we runtime suspend?
>  	ret = nouveau_do_suspend(drm_dev, true);
>  	pci_save_state(pdev);
>  	pci_disable_device(pdev);
>  	pci_ignore_hotplug(pdev);
> -	pci_set_power_state(pdev, PCI_D3cold);
> +	if (drm->runpm_dsm)
> +		pci_set_power_state(pdev, PCI_D3cold);
> +
>  	drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF;
>  	return ret;
>  }
> @@ -925,7 +929,8 @@ nouveau_pmops_runtime_resume(struct device *dev)
>  {
>  	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct drm_device *drm_dev = pci_get_drvdata(pdev);
> -	struct nvif_device *device = &nouveau_drm(drm_dev)->client.device;
> +	struct nouveau_drm *drm = nouveau_drm(drm_dev);
> +	struct nvif_device *device = &drm->client.device;
>  	int ret;
>  
>  	if (!nouveau_pmops_runtime()) {
> @@ -933,7 +938,9 @@ nouveau_pmops_runtime_resume(struct device *dev)
>  		return -EBUSY;
>  	}
>  
> -	pci_set_power_state(pdev, PCI_D0);
> +	drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
> +	if (drm->runpm_dsm)
> +		pci_set_power_state(pdev, PCI_D0);
>  	pci_restore_state(pdev);
>  	ret = pci_enable_device(pdev);
>  	if (ret)
> diff --git a/drm/nouveau/nouveau_drv.h b/drm/nouveau/nouveau_drv.h
> index da847244..941600e9 100644
> --- a/drm/nouveau/nouveau_drv.h
> +++ b/drm/nouveau/nouveau_drv.h
> @@ -214,6 +214,8 @@ struct nouveau_drm {
>  	struct nouveau_svm *svm;
>  
>  	struct nouveau_dmem *dmem;
> +
> +	bool runpm_dsm;
I think this is semi-recent, but upstream/checkpatch now prefers we use
u8 in structs instead of plain bools.
>  };
>  
>  static inline struct nouveau_drm *-- 
Cheers,
	Lyude Paul

Lyude Paul

2019-May-07 19:17 UTC

head link

[Nouveau] [PATCH 2/5] pci: enable pcie link changes for pascal

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

On Sat, 2019-05-04 at 18:32 +0200, Karol Herbst wrote:> Signed-off-by: Karol Herbst <kherbst at redhat.com>
> ---
>  drm/nouveau/nvkm/subdev/pci/gk104.c |  8 ++++----
>  drm/nouveau/nvkm/subdev/pci/gp100.c | 10 ++++++++++
>  drm/nouveau/nvkm/subdev/pci/priv.h  |  5 +++++
>  3 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drm/nouveau/nvkm/subdev/pci/gk104.c
> b/drm/nouveau/nvkm/subdev/pci/gk104.c
> index e6803050..66489018 100644
> --- a/drm/nouveau/nvkm/subdev/pci/gk104.c
> +++ b/drm/nouveau/nvkm/subdev/pci/gk104.c
> @@ -23,7 +23,7 @@
>   */
>  #include "priv.h"
>  
> -static int
> +int
>  gk104_pcie_version_supported(struct nvkm_pci *pci)
>  {
>  	return (nvkm_rd32(pci->subdev.device, 0x8c1c0) & 0x4) == 0x4 ? 2 :
1;
> @@ -108,7 +108,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;
> @@ -146,7 +146,7 @@ gk104_pcie_set_link_speed(struct nvkm_pci *pci, enum
> nvkm_pcie_speed speed)
>  	nvkm_mask(device, 0x8c040, 0x1, 0x1);
>  }
>  
> -static int
> +int
>  gk104_pcie_init(struct nvkm_pci * pci)
>  {
>  	enum nvkm_pcie_speed lnkctl_speed, max_speed, cap_speed;
> @@ -178,7 +178,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 82c5234a..eb19c7a4 100644
> --- a/drm/nouveau/nvkm/subdev/pci/gp100.c
> +++ b/drm/nouveau/nvkm/subdev/pci/gp100.c
> @@ -35,6 +35,16 @@ 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,
>  };
>  
>  int
> diff --git a/drm/nouveau/nvkm/subdev/pci/priv.h
> b/drm/nouveau/nvkm/subdev/pci/priv.h
> index c17f6063..a0d4c007 100644
> --- a/drm/nouveau/nvkm/subdev/pci/priv.h
> +++ b/drm/nouveau/nvkm/subdev/pci/priv.h
> @@ -54,6 +54,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

Lyude Paul

2019-May-07 19:42 UTC

head link

[Nouveau] [PATCH 3/5] pci: add nvkm_pcie_get_speed

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

On Sat, 2019-05-04 at 18:32 +0200, Karol Herbst wrote:> Signed-off-by: Karol Herbst <kherbst 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 23803cc8..1fdf3098 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 d71e5db5..eb405dea 100644
> --- a/drm/nouveau/nvkm/subdev/pci/pcie.c
> +++ b/drm/nouveau/nvkm/subdev/pci/pcie.c
> @@ -163,3 +163,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->pcie.cur_speed)
> +		return -ENOSYS;
> +	return pci->func->pcie.cur_speed(pci);
> +}-- 
Cheers,
	Lyude Paul

Lyude Paul

2019-May-07 19:43 UTC

head link

[Nouveau] [PATCH 4/5] pci: save the boot pcie link speed

We should probably have a commit message explaining the reason why we're
doing
this

With that change:

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

On Sat, 2019-05-04 at 18:32 +0200, Karol Herbst wrote:> Signed-off-by: Karol Herbst <kherbst at redhat.com>
> ---
>  drm/nouveau/include/nvkm/subdev/pci.h |  5 +++--
>  drm/nouveau/nvkm/subdev/pci/base.c    |  7 +++++--
>  drm/nouveau/nvkm/subdev/pci/pcie.c    | 15 +++++++++++----
>  drm/nouveau/nvkm/subdev/pci/priv.h    |  1 +
>  4 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/drm/nouveau/include/nvkm/subdev/pci.h
> b/drm/nouveau/include/nvkm/subdev/pci.h
> index 1fdf3098..b23793a2 100644
> --- a/drm/nouveau/include/nvkm/subdev/pci.h
> +++ b/drm/nouveau/include/nvkm/subdev/pci.h
> @@ -26,8 +26,9 @@ struct nvkm_pci {
>  	} agp;
>  
>  	struct {
> -		enum nvkm_pcie_speed speed;
> -		u8 width;
> +		enum nvkm_pcie_speed cur_speed;
> +		enum nvkm_pcie_speed def_speed;
> +		u8 cur_width;
>  	} pcie;
>  
>  	bool msi;
> diff --git a/drm/nouveau/nvkm/subdev/pci/base.c
> b/drm/nouveau/nvkm/subdev/pci/base.c
> index ee2431a7..b6ebee6f 100644
> --- a/drm/nouveau/nvkm/subdev/pci/base.c
> +++ b/drm/nouveau/nvkm/subdev/pci/base.c
> @@ -100,6 +100,8 @@ nvkm_pci_preinit(struct nvkm_subdev *subdev)
>  	struct nvkm_pci *pci = nvkm_pci(subdev);
>  	if (pci->agp.bridge)
>  		nvkm_agp_preinit(pci);
> +	else if (pci_is_pcie(pci->pdev))
> +		nvkm_pcie_preinit(pci);
>  	return 0;
>  }
>  
> @@ -193,8 +195,9 @@ nvkm_pci_new_(const struct nvkm_pci_func *func, struct
> nvkm_device *device,
>  	pci->func = func;
>  	pci->pdev = device->func->pci(device)->pdev;
>  	pci->irq = -1;
> -	pci->pcie.speed = -1;
> -	pci->pcie.width = -1;
> +	pci->pcie.cur_speed = -1;
> +	pci->pcie.def_speed = -1;
> +	pci->pcie.cur_width = -1;
>  
>  	if (device->type == NVKM_DEVICE_AGP)
>  		nvkm_agp_ctor(pci);
> diff --git a/drm/nouveau/nvkm/subdev/pci/pcie.c
> b/drm/nouveau/nvkm/subdev/pci/pcie.c
> index eb405dea..727b5d6a 100644
> --- a/drm/nouveau/nvkm/subdev/pci/pcie.c
> +++ b/drm/nouveau/nvkm/subdev/pci/pcie.c
> @@ -85,6 +85,13 @@ nvkm_pcie_oneinit(struct nvkm_pci *pci)
>  	return 0;
>  }
>  
> +int
> +nvkm_pcie_preinit(struct nvkm_pci *pci)
> +{
> +	pci->pcie.def_speed = nvkm_pcie_get_speed(pci);
> +	return 0;
> +}
> +
>  int
>  nvkm_pcie_init(struct nvkm_pci *pci)
>  {
> @@ -105,8 +112,8 @@ nvkm_pcie_init(struct nvkm_pci *pci)
>  	if (pci->func->pcie.init)
>  		pci->func->pcie.init(pci);
>  
> -	if (pci->pcie.speed != -1)
> -		nvkm_pcie_set_link(pci, pci->pcie.speed, pci->pcie.width);
> +	if (pci->pcie.cur_speed != -1)
> +		nvkm_pcie_set_link(pci, pci->pcie.cur_speed, pci-
> >pcie.cur_width);
>  
>  	return 0;
>  }
> @@ -146,8 +153,8 @@ 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;
> +	pci->pcie.cur_speed = speed;
> +	pci->pcie.cur_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 a0d4c007..dc3ae3cd 100644
> --- a/drm/nouveau/nvkm/subdev/pci/priv.h
> +++ b/drm/nouveau/nvkm/subdev/pci/priv.h
> @@ -60,5 +60,6 @@ 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_preinit(struct nvkm_pci *);
>  int nvkm_pcie_init(struct nvkm_pci *);
>  #endif-- 
Cheers,
	Lyude Paul

Lyude Paul

2019-May-07 19:44 UTC

head link

[Nouveau] [PATCH 5/5] pci: restore the boot pcie link speed on fini

I think it should be fine to just squash this and the previous patch together.
If not, longer commit message here at least

up to you, either way: Reviewed-by: Lyude Paul <lyude at redhat.com>

On Sat, 2019-05-04 at 18:32 +0200, Karol Herbst wrote:> fixes runtime suspend on my gp107
> 
> Signed-off-by: Karol Herbst <kherbst at redhat.com>
> ---
>  drm/nouveau/nvkm/subdev/pci/base.c | 2 ++
>  drm/nouveau/nvkm/subdev/pci/pcie.c | 8 ++++++++
>  drm/nouveau/nvkm/subdev/pci/priv.h | 1 +
>  3 files changed, 11 insertions(+)
> 
> diff --git a/drm/nouveau/nvkm/subdev/pci/base.c
> b/drm/nouveau/nvkm/subdev/pci/base.c
> index b6ebee6f..d9fb5a83 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);
>  
>  	return 0;
>  }
> diff --git a/drm/nouveau/nvkm/subdev/pci/pcie.c
> b/drm/nouveau/nvkm/subdev/pci/pcie.c
> index 727b5d6a..00c07fd8 100644
> --- a/drm/nouveau/nvkm/subdev/pci/pcie.c
> +++ b/drm/nouveau/nvkm/subdev/pci/pcie.c
> @@ -118,6 +118,14 @@ nvkm_pcie_init(struct nvkm_pci *pci)
>  	return 0;
>  }
>  
> +int
> +nvkm_pcie_fini(struct nvkm_pci *pci)
> +{
> +	if (!IS_ERR_VALUE(pci->pcie.def_speed))
> +		return nvkm_pcie_set_link(pci, pci->pcie.def_speed, 16);
> +	return 0;
> +}
> +
>  int
>  nvkm_pcie_set_link(struct nvkm_pci *pci, enum nvkm_pcie_speed speed, u8
> width)
>  {
> diff --git a/drm/nouveau/nvkm/subdev/pci/priv.h
> b/drm/nouveau/nvkm/subdev/pci/priv.h
> index dc3ae3cd..e7744671 100644
> --- a/drm/nouveau/nvkm/subdev/pci/priv.h
> +++ b/drm/nouveau/nvkm/subdev/pci/priv.h
> @@ -62,4 +62,5 @@ int gk104_pcie_version_supported(struct nvkm_pci *);
>  int nvkm_pcie_oneinit(struct nvkm_pci *);
>  int nvkm_pcie_preinit(struct nvkm_pci *);
>  int nvkm_pcie_init(struct nvkm_pci *);
> +int nvkm_pcie_fini(struct nvkm_pci *);
>  #endif-- 
Cheers,
	Lyude Paul

Possibly Parallel Threads

Search for more possibly parallel threads

Nouveau - May 2019 - [PATCH 1/5] drm: don't set the pci power state if the pci subsystem handles the ACPI bits

[Nouveau] [PATCH 0/5] Potential fix for runpm issues on various laptops

[Nouveau] [PATCH 1/5] drm: don't set the pci power state if the pci subsystem handles the ACPI bits

[Nouveau] [PATCH 2/5] pci: enable pcie link changes for pascal

[Nouveau] [PATCH 3/5] pci: add nvkm_pcie_get_speed

[Nouveau] [PATCH 4/5] pci: save the boot pcie link speed

[Nouveau] [PATCH 5/5] pci: restore the boot pcie link speed on fini

[Nouveau] [PATCH 1/5] drm: don't set the pci power state if the pci subsystem handles the ACPI bits

[Nouveau] [PATCH 2/5] pci: enable pcie link changes for pascal

[Nouveau] [PATCH 3/5] pci: add nvkm_pcie_get_speed

[Nouveau] [PATCH 4/5] pci: save the boot pcie link speed

[Nouveau] [PATCH 5/5] pci: restore the boot pcie link speed on fini

Possibly Parallel Threads