thr3ads.net - Nouveau - [Nouveau] [PATCH 1/3] pci: force disable ASPM before changing the link speed [Sep 2019]

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

Karol Herbst

2019-Sep-12 19:00 UTC

[Nouveau] [PATCH 0/3] PCIe link change improvements

everything was taken from nvgpu.

Main reason for adding is to improve stability of the PCIe link changing code
as we might want to depend on it for a workaround fixing our infamous runpm
issues on recent laptops

Karol Herbst (3):
  pci: force disable ASPM before changing the link speed
  pci/gk104: enable dl_mgr safe mode
  pci/gk104: wait for ltssm idle before changing the link

 drm/nouveau/nvkm/subdev/pci/g84.c   |  9 +++++++++
 drm/nouveau/nvkm/subdev/pci/g92.c   |  1 +
 drm/nouveau/nvkm/subdev/pci/g94.c   |  1 +
 drm/nouveau/nvkm/subdev/pci/gf100.c |  1 +
 drm/nouveau/nvkm/subdev/pci/gf106.c |  1 +
 drm/nouveau/nvkm/subdev/pci/gk104.c | 11 +++++++++++
 drm/nouveau/nvkm/subdev/pci/pcie.c  | 14 ++++++++++++++
 drm/nouveau/nvkm/subdev/pci/priv.h  |  2 ++
 8 files changed, 40 insertions(+)

-- 
2.21.0

Karol Herbst

2019-Sep-12 19:00 UTC

head link

[Nouveau] [PATCH 1/3] pci: force disable ASPM before changing the link speed

taken from nvgpu

Signed-off-by: Karol Herbst <kherbst at redhat.com>
---
 drm/nouveau/nvkm/subdev/pci/g84.c   |  9 +++++++++
 drm/nouveau/nvkm/subdev/pci/g92.c   |  1 +
 drm/nouveau/nvkm/subdev/pci/g94.c   |  1 +
 drm/nouveau/nvkm/subdev/pci/gf100.c |  1 +
 drm/nouveau/nvkm/subdev/pci/gf106.c |  1 +
 drm/nouveau/nvkm/subdev/pci/gk104.c |  1 +
 drm/nouveau/nvkm/subdev/pci/pcie.c  | 14 ++++++++++++++
 drm/nouveau/nvkm/subdev/pci/priv.h  |  2 ++
 8 files changed, 30 insertions(+)

diff --git a/drm/nouveau/nvkm/subdev/pci/g84.c
b/drm/nouveau/nvkm/subdev/pci/g84.c
index 62438d892..353d70d7e 100644
--- a/drm/nouveau/nvkm/subdev/pci/g84.c
+++ b/drm/nouveau/nvkm/subdev/pci/g84.c
@@ -122,6 +122,14 @@ g84_pci_init(struct nvkm_pci *pci)
 		nvkm_pci_mask(pci, 0x041c, 0x00000060, 0x00000000);
 }
 
+void
+g84_pcie_force_aspm_off(struct nvkm_pci *pci, bool status)
+{
+	/* L0s and L1 */
+	u32 value = status ? 0x180 : 0x0;
+	nvkm_pci_mask(pci, 0x150, 0x180, value);
+}
+
 int
 g84_pcie_init(struct nvkm_pci *pci)
 {
@@ -147,6 +155,7 @@ g84_pci_func = {
 	.pcie.set_version = g84_pcie_set_version,
 	.pcie.version = g84_pcie_version,
 	.pcie.version_supported = g84_pcie_version_supported,
+	.pcie.force_aspm_off = g84_pcie_force_aspm_off,
 };
 
 int
diff --git a/drm/nouveau/nvkm/subdev/pci/g92.c
b/drm/nouveau/nvkm/subdev/pci/g92.c
index 48874359d..8c35cc89e 100644
--- a/drm/nouveau/nvkm/subdev/pci/g92.c
+++ b/drm/nouveau/nvkm/subdev/pci/g92.c
@@ -48,6 +48,7 @@ g92_pci_func = {
 	.pcie.set_version = g84_pcie_set_version,
 	.pcie.version = g84_pcie_version,
 	.pcie.version_supported = g92_pcie_version_supported,
+	.pcie.force_aspm_off = g84_pcie_force_aspm_off,
 };
 
 int
diff --git a/drm/nouveau/nvkm/subdev/pci/g94.c
b/drm/nouveau/nvkm/subdev/pci/g94.c
index 09adb37a5..aa152d798 100644
--- a/drm/nouveau/nvkm/subdev/pci/g94.c
+++ b/drm/nouveau/nvkm/subdev/pci/g94.c
@@ -40,6 +40,7 @@ g94_pci_func = {
 	.pcie.set_version = g84_pcie_set_version,
 	.pcie.version = g84_pcie_version,
 	.pcie.version_supported = g92_pcie_version_supported,
+	.pcie.force_aspm_off = g84_pcie_force_aspm_off,
 };
 
 int
diff --git a/drm/nouveau/nvkm/subdev/pci/gf100.c
b/drm/nouveau/nvkm/subdev/pci/gf100.c
index 00a5e7d3e..8676480c3 100644
--- a/drm/nouveau/nvkm/subdev/pci/gf100.c
+++ b/drm/nouveau/nvkm/subdev/pci/gf100.c
@@ -93,6 +93,7 @@ gf100_pci_func = {
 	.pcie.set_version = gf100_pcie_set_version,
 	.pcie.version = gf100_pcie_version,
 	.pcie.version_supported = g92_pcie_version_supported,
+	.pcie.force_aspm_off = g84_pcie_force_aspm_off,
 };
 
 int
diff --git a/drm/nouveau/nvkm/subdev/pci/gf106.c
b/drm/nouveau/nvkm/subdev/pci/gf106.c
index 11bf419af..8079ebdd4 100644
--- a/drm/nouveau/nvkm/subdev/pci/gf106.c
+++ b/drm/nouveau/nvkm/subdev/pci/gf106.c
@@ -40,6 +40,7 @@ gf106_pci_func = {
 	.pcie.set_version = gf100_pcie_set_version,
 	.pcie.version = gf100_pcie_version,
 	.pcie.version_supported = g92_pcie_version_supported,
+	.pcie.force_aspm_off = g84_pcie_force_aspm_off,
 };
 
 int
diff --git a/drm/nouveau/nvkm/subdev/pci/gk104.c
b/drm/nouveau/nvkm/subdev/pci/gk104.c
index e68030507..b73922c52 100644
--- a/drm/nouveau/nvkm/subdev/pci/gk104.c
+++ b/drm/nouveau/nvkm/subdev/pci/gk104.c
@@ -219,6 +219,7 @@ gk104_pci_func = {
 	.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/pcie.c
b/drm/nouveau/nvkm/subdev/pci/pcie.c
index d71e5db50..354ac4c85 100644
--- a/drm/nouveau/nvkm/subdev/pci/pcie.c
+++ b/drm/nouveau/nvkm/subdev/pci/pcie.c
@@ -111,6 +111,14 @@ nvkm_pcie_init(struct nvkm_pci *pci)
 	return 0;
 }
 
+void
+nvkm_pcie_force_aspm_off(struct nvkm_pci *pci, bool status)
+{
+	if (!pci->func->pcie.force_aspm_off)
+		return;
+	pci->func->pcie.force_aspm_off(pci, status);
+}
+
 int
 nvkm_pcie_set_link(struct nvkm_pci *pci, enum nvkm_pcie_speed speed, u8 width)
 {
@@ -157,9 +165,15 @@ nvkm_pcie_set_link(struct nvkm_pci *pci, enum
nvkm_pcie_speed speed, u8 width)
 	nvkm_debug(subdev, "set link to %s x%i\n",
 		   nvkm_pcie_speeds[speed], width);
 
+	/* force disable ASPM */
+	nvkm_pcie_force_aspm_off(pci, true);
+
 	ret = pci->func->pcie.set_link(pci, speed, width);
 	if (ret < 0)
 		nvkm_error(subdev, "setting link failed: %i\n", ret);
 
+	/* lift force disable ASPM */
+	nvkm_pcie_force_aspm_off(pci, false);
+
 	return ret;
 }
diff --git a/drm/nouveau/nvkm/subdev/pci/priv.h
b/drm/nouveau/nvkm/subdev/pci/priv.h
index 7009aad86..c6a9ef330 100644
--- a/drm/nouveau/nvkm/subdev/pci/priv.h
+++ b/drm/nouveau/nvkm/subdev/pci/priv.h
@@ -24,6 +24,7 @@ struct nvkm_pci_func {
 		void (*set_version)(struct nvkm_pci *, u8);
 		int (*version)(struct nvkm_pci *);
 		int (*version_supported)(struct nvkm_pci *);
+		void (*force_aspm_off)(struct nvkm_pci *, bool);
 	} pcie;
 };
 
@@ -42,6 +43,7 @@ int g84_pcie_version(struct nvkm_pci *);
 void g84_pcie_set_link_speed(struct nvkm_pci *, enum nvkm_pcie_speed);
 enum nvkm_pcie_speed g84_pcie_cur_speed(struct nvkm_pci *);
 enum nvkm_pcie_speed g84_pcie_max_speed(struct nvkm_pci *);
+void g84_pcie_force_aspm_off(struct nvkm_pci *, bool status);
 int g84_pcie_init(struct nvkm_pci *);
 int g84_pcie_set_link(struct nvkm_pci *, enum nvkm_pcie_speed, u8);
 
-- 
2.21.0

Karol Herbst

2019-Sep-12 19:00 UTC

head link

[Nouveau] [PATCH 2/3] pci/gk104: enable dl_mgr safe mode

no idea why and what it does, taken from nvgpu

Signed-off-by: Karol Herbst <kherbst at redhat.com>
---
 drm/nouveau/nvkm/subdev/pci/gk104.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drm/nouveau/nvkm/subdev/pci/gk104.c
b/drm/nouveau/nvkm/subdev/pci/gk104.c
index b73922c52..6aeb07fe4 100644
--- a/drm/nouveau/nvkm/subdev/pci/gk104.c
+++ b/drm/nouveau/nvkm/subdev/pci/gk104.c
@@ -184,6 +184,7 @@ gk104_pcie_set_link(struct nvkm_pci *pci, enum
nvkm_pcie_speed speed, u8 width)
 	struct nvkm_subdev *subdev = &pci->subdev;
 	enum nvkm_pcie_speed lnk_ctl_speed = gk104_pcie_lnkctl_speed(pci);
 	enum nvkm_pcie_speed lnk_cap_speed = gk104_pcie_cap_speed(pci);
+	u32 old_dl_mgr;
 
 	if (speed > lnk_cap_speed) {
 		speed = lnk_cap_speed;
@@ -197,7 +198,9 @@ gk104_pcie_set_link(struct nvkm_pci *pci, enum
nvkm_pcie_speed speed, u8 width)
 			  " lnkctl speed\n");
 	}
 
+	old_dl_mgr = nvkm_mask(subdev->device, 0x8b8c0, 0x4, 0x4);
 	gk104_pcie_set_link_speed(pci, speed);
+	nvkm_wr32(subdev->device, 0x8b8c0, old_dl_mgr);
 	return 0;
 }
 
-- 
2.21.0

Karol Herbst

2019-Sep-12 19:00 UTC

head link

[Nouveau] [PATCH 3/3] pci/gk104: wait for ltssm idle before changing the link

taken from nvgpu

Signed-off-by: Karol Herbst <kherbst at redhat.com>
---
 drm/nouveau/nvkm/subdev/pci/gk104.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drm/nouveau/nvkm/subdev/pci/gk104.c
b/drm/nouveau/nvkm/subdev/pci/gk104.c
index 6aeb07fe4..2d1fa67eb 100644
--- a/drm/nouveau/nvkm/subdev/pci/gk104.c
+++ b/drm/nouveau/nvkm/subdev/pci/gk104.c
@@ -23,6 +23,8 @@
  */
 #include "priv.h"
 
+#include <subdev/timer.h>
+
 static int
 gk104_pcie_version_supported(struct nvkm_pci *pci)
 {
@@ -142,6 +144,11 @@ gk104_pcie_set_link_speed(struct nvkm_pci *pci, enum
nvkm_pcie_speed speed)
 		break;
 	}
 
+	/* wait for ltssm idle */
+	nvkm_msec(device, 200,
+		if ((nvkm_rd32(device, 0x8c040) & 0x1f) == 0)
+			break;
+	);
 	nvkm_mask(device, 0x8c040, 0xc0000, mask_value);
 	nvkm_mask(device, 0x8c040, 0x1, 0x1);
 }
-- 
2.21.0

Ilia Mirkin

2019-Sep-12 19:07 UTC

head link

[Nouveau] [PATCH 3/3] pci/gk104: wait for ltssm idle before changing the link

On Thu, Sep 12, 2019 at 3:00 PM Karol Herbst <kherbst at redhat.com>
wrote:>
> taken from nvgpu
>
> Signed-off-by: Karol Herbst <kherbst at redhat.com>
> ---
>  drm/nouveau/nvkm/subdev/pci/gk104.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drm/nouveau/nvkm/subdev/pci/gk104.c
b/drm/nouveau/nvkm/subdev/pci/gk104.c
> index 6aeb07fe4..2d1fa67eb 100644
> --- a/drm/nouveau/nvkm/subdev/pci/gk104.c
> +++ b/drm/nouveau/nvkm/subdev/pci/gk104.c
> @@ -23,6 +23,8 @@
>   */
>  #include "priv.h"
>
> +#include <subdev/timer.h>
> +
>  static int
>  gk104_pcie_version_supported(struct nvkm_pci *pci)
>  {
> @@ -142,6 +144,11 @@ gk104_pcie_set_link_speed(struct nvkm_pci *pci, enum
nvkm_pcie_speed speed)
>                 break;
>         }
>
> +       /* wait for ltssm idle */
> +       nvkm_msec(device, 200,
> +               if ((nvkm_rd32(device, 0x8c040) & 0x1f) == 0)
> +                       break;
> +       );
What if it doesn't idle? Should you still do the below things?
>         nvkm_mask(device, 0x8c040, 0xc0000, mask_value);
>         nvkm_mask(device, 0x8c040, 0x1, 0x1);
>  }
> --
> 2.21.0
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

Ben Skeggs

2019-Sep-17 05:56 UTC

head link

[Nouveau] [PATCH 1/3] pci: force disable ASPM before changing the link speed

On Fri, 13 Sep 2019 at 05:00, Karol Herbst <kherbst at redhat.com>
wrote:>
> taken from nvgpu
>
> Signed-off-by: Karol Herbst <kherbst at redhat.com>
> ---
>  drm/nouveau/nvkm/subdev/pci/g84.c   |  9 +++++++++
>  drm/nouveau/nvkm/subdev/pci/g92.c   |  1 +
>  drm/nouveau/nvkm/subdev/pci/g94.c   |  1 +
>  drm/nouveau/nvkm/subdev/pci/gf100.c |  1 +
>  drm/nouveau/nvkm/subdev/pci/gf106.c |  1 +
>  drm/nouveau/nvkm/subdev/pci/gk104.c |  1 +
>  drm/nouveau/nvkm/subdev/pci/pcie.c  | 14 ++++++++++++++
>  drm/nouveau/nvkm/subdev/pci/priv.h  |  2 ++
>  8 files changed, 30 insertions(+)
>
> diff --git a/drm/nouveau/nvkm/subdev/pci/g84.c
b/drm/nouveau/nvkm/subdev/pci/g84.c
> index 62438d892..353d70d7e 100644
> --- a/drm/nouveau/nvkm/subdev/pci/g84.c
> +++ b/drm/nouveau/nvkm/subdev/pci/g84.c
> @@ -122,6 +122,14 @@ g84_pci_init(struct nvkm_pci *pci)
>                 nvkm_pci_mask(pci, 0x041c, 0x00000060, 0x00000000);
>  }
>
> +void
> +g84_pcie_force_aspm_off(struct nvkm_pci *pci, bool status)
> +{
> +       /* L0s and L1 */
> +       u32 value = status ? 0x180 : 0x0;
> +       nvkm_pci_mask(pci, 0x150, 0x180, value);
> +}
> +
>  int
>  g84_pcie_init(struct nvkm_pci *pci)
>  {
> @@ -147,6 +155,7 @@ g84_pci_func = {
>         .pcie.set_version = g84_pcie_set_version,
>         .pcie.version = g84_pcie_version,
>         .pcie.version_supported = g84_pcie_version_supported,
> +       .pcie.force_aspm_off = g84_pcie_force_aspm_off,
>  };
>
>  int
> diff --git a/drm/nouveau/nvkm/subdev/pci/g92.c
b/drm/nouveau/nvkm/subdev/pci/g92.c
> index 48874359d..8c35cc89e 100644
> --- a/drm/nouveau/nvkm/subdev/pci/g92.c
> +++ b/drm/nouveau/nvkm/subdev/pci/g92.c
> @@ -48,6 +48,7 @@ g92_pci_func = {
>         .pcie.set_version = g84_pcie_set_version,
>         .pcie.version = g84_pcie_version,
>         .pcie.version_supported = g92_pcie_version_supported,
> +       .pcie.force_aspm_off = g84_pcie_force_aspm_off,
>  };
>
>  int
> diff --git a/drm/nouveau/nvkm/subdev/pci/g94.c
b/drm/nouveau/nvkm/subdev/pci/g94.c
> index 09adb37a5..aa152d798 100644
> --- a/drm/nouveau/nvkm/subdev/pci/g94.c
> +++ b/drm/nouveau/nvkm/subdev/pci/g94.c
> @@ -40,6 +40,7 @@ g94_pci_func = {
>         .pcie.set_version = g84_pcie_set_version,
>         .pcie.version = g84_pcie_version,
>         .pcie.version_supported = g92_pcie_version_supported,
> +       .pcie.force_aspm_off = g84_pcie_force_aspm_off,
>  };
>
>  int
> diff --git a/drm/nouveau/nvkm/subdev/pci/gf100.c
b/drm/nouveau/nvkm/subdev/pci/gf100.c
> index 00a5e7d3e..8676480c3 100644
> --- a/drm/nouveau/nvkm/subdev/pci/gf100.c
> +++ b/drm/nouveau/nvkm/subdev/pci/gf100.c
> @@ -93,6 +93,7 @@ gf100_pci_func = {
>         .pcie.set_version = gf100_pcie_set_version,
>         .pcie.version = gf100_pcie_version,
>         .pcie.version_supported = g92_pcie_version_supported,
> +       .pcie.force_aspm_off = g84_pcie_force_aspm_off,
>  };
>
>  int
> diff --git a/drm/nouveau/nvkm/subdev/pci/gf106.c
b/drm/nouveau/nvkm/subdev/pci/gf106.c
> index 11bf419af..8079ebdd4 100644
> --- a/drm/nouveau/nvkm/subdev/pci/gf106.c
> +++ b/drm/nouveau/nvkm/subdev/pci/gf106.c
> @@ -40,6 +40,7 @@ gf106_pci_func = {
>         .pcie.set_version = gf100_pcie_set_version,
>         .pcie.version = gf100_pcie_version,
>         .pcie.version_supported = g92_pcie_version_supported,
> +       .pcie.force_aspm_off = g84_pcie_force_aspm_off,
>  };
>
>  int
> diff --git a/drm/nouveau/nvkm/subdev/pci/gk104.c
b/drm/nouveau/nvkm/subdev/pci/gk104.c
> index e68030507..b73922c52 100644
> --- a/drm/nouveau/nvkm/subdev/pci/gk104.c
> +++ b/drm/nouveau/nvkm/subdev/pci/gk104.c
> @@ -219,6 +219,7 @@ gk104_pci_func = {
>         .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/pcie.c
b/drm/nouveau/nvkm/subdev/pci/pcie.c
> index d71e5db50..354ac4c85 100644
> --- a/drm/nouveau/nvkm/subdev/pci/pcie.c
> +++ b/drm/nouveau/nvkm/subdev/pci/pcie.c
> @@ -111,6 +111,14 @@ nvkm_pcie_init(struct nvkm_pci *pci)
>         return 0;
>  }
>
> +void
> +nvkm_pcie_force_aspm_off(struct nvkm_pci *pci, bool status)
> +{
> +       if (!pci->func->pcie.force_aspm_off)
> +               return;
> +       pci->func->pcie.force_aspm_off(pci, status);
> +}
> +
>  int
>  nvkm_pcie_set_link(struct nvkm_pci *pci, enum nvkm_pcie_speed speed, u8
width)
>  {
> @@ -157,9 +165,15 @@ nvkm_pcie_set_link(struct nvkm_pci *pci, enum
nvkm_pcie_speed speed, u8 width)
>         nvkm_debug(subdev, "set link to %s x%i\n",
>                    nvkm_pcie_speeds[speed], width);
>
> +       /* force disable ASPM */
> +       nvkm_pcie_force_aspm_off(pci, true);
> +
>         ret = pci->func->pcie.set_link(pci, speed, width);
>         if (ret < 0)
>                 nvkm_error(subdev, "setting link failed: %i\n",
ret);
>
> +       /* lift force disable ASPM */
> +       nvkm_pcie_force_aspm_off(pci, false);This doesn't do what is says on the box.  It's forcibly disabling
ASPM, then forcibly enabling it.  You should instead restore the
settings from before forcing it off, which is what NVGPU actually does
too.

Ben.
> +
>         return ret;
>  }
> diff --git a/drm/nouveau/nvkm/subdev/pci/priv.h
b/drm/nouveau/nvkm/subdev/pci/priv.h
> index 7009aad86..c6a9ef330 100644
> --- a/drm/nouveau/nvkm/subdev/pci/priv.h
> +++ b/drm/nouveau/nvkm/subdev/pci/priv.h
> @@ -24,6 +24,7 @@ struct nvkm_pci_func {
>                 void (*set_version)(struct nvkm_pci *, u8);
>                 int (*version)(struct nvkm_pci *);
>                 int (*version_supported)(struct nvkm_pci *);
> +               void (*force_aspm_off)(struct nvkm_pci *, bool);
>         } pcie;
>  };
>
> @@ -42,6 +43,7 @@ int g84_pcie_version(struct nvkm_pci *);
>  void g84_pcie_set_link_speed(struct nvkm_pci *, enum nvkm_pcie_speed);
>  enum nvkm_pcie_speed g84_pcie_cur_speed(struct nvkm_pci *);
>  enum nvkm_pcie_speed g84_pcie_max_speed(struct nvkm_pci *);
> +void g84_pcie_force_aspm_off(struct nvkm_pci *, bool status);
>  int g84_pcie_init(struct nvkm_pci *);
>  int g84_pcie_set_link(struct nvkm_pci *, enum nvkm_pcie_speed, u8);
>
> --
> 2.21.0
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

Possibly Parallel Threads

Search for more apparently analagous threads

Nouveau - Sep 2019 - [PATCH 1/3] pci: force disable ASPM before changing the link speed

[Nouveau] [PATCH 0/3] PCIe link change improvements

[Nouveau] [PATCH 1/3] pci: force disable ASPM before changing the link speed

[Nouveau] [PATCH 2/3] pci/gk104: enable dl_mgr safe mode

[Nouveau] [PATCH 3/3] pci/gk104: wait for ltssm idle before changing the link

[Nouveau] [PATCH 3/3] pci/gk104: wait for ltssm idle before changing the link

[Nouveau] [PATCH 1/3] pci: force disable ASPM before changing the link speed

Possibly Parallel Threads