On Tue, Sep 17, 2019 at 10:21 AM Ben Skeggs <skeggsb@xxxxxxxxx> wrote:
>
> On Tue, 17 Sep 2019 at 18:07, Karol Herbst <kherbst@xxxxxxxxxx> wrote:
> >
> > On Tue, Sep 17, 2019 at 8:01 AM Ben Skeggs <skeggsb@xxxxxxxxx> wrote:
> > >
> > > On Fri, 13 Sep 2019 at 21:33, Karol Herbst <kherbst@xxxxxxxxxx> 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?
> >
> > modprobe nouveau
> > rmmod nouveau
> > modprobe nouveau (saved 2.5 as the "boot" speed)
> > runpm breaks.
> Given that this is more than likely a hack/workaround for another
> issue to begin with, that isn't the end of the world.
>
> >
> > Mainly I don't actually want to depend on the initial state as we
> > lower to 2.5/5.0 depending on what we queried is possible and not
> > using 2.5 is actually the fix, not use whatever the GPU booted with.
> Is 8 available everywhere we're going to be using this?
>
nouveau fallsback to a lower speed if the requested one is not
available. We have to do this detection for pstates anyway, where the
vbios requests 8.0, but the system can only do 5.0 or 2.5
> >
> > > 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?
> > >
> >
> > I think if we add code for the width, we would add a maximum width
> > detection as well as we do for the link speed.
> >
> > > >
> > > > Signed-off-by: Karol Herbst <kherbst@xxxxxxxxxx>
> > > > Reviewed-by: Lyude Paul <lyude@xxxxxxxxxx> (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@xxxxxxxxxxxxxxxxxxxxx
> > > > https://lists.freedesktop.org/mailman/listinfo/nouveau
> >
_______________________________________________
Nouveau mailing list
Nouveau@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/nouveau
