Nouveau - Sep 2019 - [RFC PATCH v2] clk: Remove BYPASS_PLL_CHECK from PLLs

I have looked at problem with Fermi GPUs where changing to higher clock
led to really bad perfomance (with GpuTest 20x worse perfomance) and later also
crashes of the nouveau. It seemed
to be affected by Shader Clock in Voltage Entries in the video BIOS. Disabling
BYPASS_PLL_CHECK in CLK0_CTRL seems to completely fix the issue. I have
tried to search this BYPASS_PLL_CHECK in Nvidia traces but seemed it
wasn't used nowhere for CLK settings.

Removing this works fine, but I don't know what it's really for.
Actual bit setting this BYPASS_PLL_CHECK is on 0x10:
	lookup -ac0 0x137000 0x10
	PCLOCK.CLK0_CTRL => { BYPASS_PLL_CHECK | UNK12 = 0 }
Also, disabling this bit on other CLKs doesn't seem to break anything.

v2: Add back PLL lock test

Signed-off-by: Mark Menzynski <mmenzyns at redhat.com>
---
 drm/nouveau/nvkm/subdev/clk/gf100.c | 1 -
 drm/nouveau/nvkm/subdev/clk/gk104.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drm/nouveau/nvkm/subdev/clk/gf100.c
b/drm/nouveau/nvkm/subdev/clk/gf100.c
index 7f67f9f5..fe56817b 100644
--- a/drm/nouveau/nvkm/subdev/clk/gf100.c
+++ b/drm/nouveau/nvkm/subdev/clk/gf100.c
@@ -381,7 +381,6 @@ gf100_clk_prog_2(struct gf100_clk *clk, int idx)
 				if (nvkm_rd32(device, addr + 0x00) & 0x00020000)
 					break;
 			);
-			nvkm_mask(device, addr + 0x00, 0x00000010, 0x00000010);
 
 			/* Enable sync mode */
 			nvkm_mask(device, addr + 0x00, 0x00000004, 0x00000004);
diff --git a/drm/nouveau/nvkm/subdev/clk/gk104.c
b/drm/nouveau/nvkm/subdev/clk/gk104.c
index 0b37e3da..22cac304 100644
--- a/drm/nouveau/nvkm/subdev/clk/gk104.c
+++ b/drm/nouveau/nvkm/subdev/clk/gk104.c
@@ -400,7 +400,6 @@ gk104_clk_prog_2(struct gk104_clk *clk, int idx)
 			if (nvkm_rd32(device, addr + 0x00) & 0x00020000)
 				break;
 		);
-		nvkm_mask(device, addr + 0x00, 0x00000010, 0x00000010);
 
 		/* Enable sync mode */
 		nvkm_mask(device, addr + 0x00, 0x00000004, 0x00000004);
-- 
2.21.0

Can you check to see if that bit was set prior to entering the code? I
wonder if you can just restore it if it was there, and leave it if
it's not?

On Wed, Sep 4, 2019 at 10:28 AM Mark Menzynski <mmenzyns at redhat.com>
wrote:
>
> I have looked at problem with Fermi GPUs where changing to higher clock
> led to really bad perfomance (with GpuTest 20x worse perfomance) and later
also crashes of the nouveau. It seemed
> to be affected by Shader Clock in Voltage Entries in the video BIOS.
Disabling
> BYPASS_PLL_CHECK in CLK0_CTRL seems to completely fix the issue. I have
> tried to search this BYPASS_PLL_CHECK in Nvidia traces but seemed it
> wasn't used nowhere for CLK settings.
>
> Removing this works fine, but I don't know what it's really for.
> Actual bit setting this BYPASS_PLL_CHECK is on 0x10:
>         lookup -ac0 0x137000 0x10
>         PCLOCK.CLK0_CTRL => { BYPASS_PLL_CHECK | UNK12 = 0 }
> Also, disabling this bit on other CLKs doesn't seem to break anything.
>
> v2: Add back PLL lock test
>
> Signed-off-by: Mark Menzynski <mmenzyns at redhat.com>
> ---
>  drm/nouveau/nvkm/subdev/clk/gf100.c | 1 -
>  drm/nouveau/nvkm/subdev/clk/gk104.c | 1 -
>  2 files changed, 2 deletions(-)
>
> diff --git a/drm/nouveau/nvkm/subdev/clk/gf100.c
b/drm/nouveau/nvkm/subdev/clk/gf100.c
> index 7f67f9f5..fe56817b 100644
> --- a/drm/nouveau/nvkm/subdev/clk/gf100.c
> +++ b/drm/nouveau/nvkm/subdev/clk/gf100.c
> @@ -381,7 +381,6 @@ gf100_clk_prog_2(struct gf100_clk *clk, int idx)
>                                 if (nvkm_rd32(device, addr + 0x00) &
0x00020000)
>                                         break;
>                         );
> -                       nvkm_mask(device, addr + 0x00, 0x00000010,
0x00000010);
>
>                         /* Enable sync mode */
>                         nvkm_mask(device, addr + 0x00, 0x00000004,
0x00000004);
> diff --git a/drm/nouveau/nvkm/subdev/clk/gk104.c
b/drm/nouveau/nvkm/subdev/clk/gk104.c
> index 0b37e3da..22cac304 100644
> --- a/drm/nouveau/nvkm/subdev/clk/gk104.c
> +++ b/drm/nouveau/nvkm/subdev/clk/gk104.c
> @@ -400,7 +400,6 @@ gk104_clk_prog_2(struct gk104_clk *clk, int idx)
>                         if (nvkm_rd32(device, addr + 0x00) &
0x00020000)
>                                 break;
>                 );
> -               nvkm_mask(device, addr + 0x00, 0x00000010, 0x00000010);
>
>                 /* Enable sync mode */
>                 nvkm_mask(device, addr + 0x00, 0x00000004, 0x00000004);
> --
> 2.21.0
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau