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
[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
[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
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
[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
[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
[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
[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
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
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
[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
- [PATCH v2 2/4] pci: enable pcie link changes for pascal
- [PATCH v4 1/4] pci: enable pcie link changes for pascal
- [PATCH] pci/gp100: Enable changing pcie link speeds
- [PATCH 1/4] pci: enable pcie link changes for pascal
- [PATCH v4 0/4] add PCIe workaround to fix runpm on laptops