[Nouveau] "enable ctxprog xfer only when we need it to save power" introduces big performance regression

[Marcin Slusarz]

On Thu, Nov 10, 2011 at 08:10:44AM +0100, Martin Peres wrote:
> Le 09/11/2011 23:10, Marcin Slusarz a écrit :
> > For anyone who don't read IRC logs - it turns out it regressed only on 
> > my box, because I have page flipping disabled (due to page flipping 
> > being very buggy here, see 
> > https://bugs.freedesktop.org/show_bug.cgi?id=42398), which forces gpu 
> > context switch on every frame - even with (OpenGL) full screen apps. 
> > And this patch slows down context switches. So, can we disable xfers 
> > in ctxprog only when page flipping is enabled? Or is there any other 
> > option? Marcin 
> Hmm, it isn't logical yet. To test for performance regression, I 
> launched the well known context switch test that is glxgears and only 
> found a performance decrease of 1%.
> 
> I'll test with pageflip disabled though and see for myself.
> 
> Anyway, we'll do something about it. I just hope that most cards don't 
> have this problem. This is clearly an hw regression.

Heh, with page flipping enabled, regression is still there, only smaller
(61->54, instead of 49 FPS).

I want my Nouveau performance back ;)

---
From: Marcin Slusarz <marcin.slusarz at gmail.com>
Subject: [PATCH] drm/nv50/gr: make "xfers only in ctxprog" optional

Commit fbba036a56fe0e5c5e8c91daf3fa211f88d94a03
"drm/nv50/gr: enable ctxprog xfer only when we need it to save power"
introduced performance regression.

So revert to previous behaviour and add module option (nv50_xfer_ctxprog=0/1)
to restore it back.

Signed-off-by: Marcin Slusarz <marcin.slusarz at gmail.com>
---
 drivers/gpu/drm/nouveau/nouveau_drv.c |    4 ++++
 drivers/gpu/drm/nouveau/nouveau_drv.h |    1 +
 drivers/gpu/drm/nouveau/nv50_graph.c  |    5 ++++-
 drivers/gpu/drm/nouveau/nv50_grctx.c  |    6 ++++--
 4 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.c b/drivers/gpu/drm/nouveau/nouveau_drv.c
index 9791d13..6ce2347 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.c
@@ -120,6 +120,10 @@ MODULE_PARM_DESC(msi, "Enable MSI (default: off)\n");
 int nouveau_msi;
 module_param_named(msi, nouveau_msi, int, 0400);
 
+MODULE_PARM_DESC(nv50_xfer_ctxprog, "Enable xfers only in ctxprog (decreases performance, lowers power usage) (default: 0)");
+int nouveau_nv50_xfer_ctxprog = 0;
+module_param_named(nv50_xfer_ctxprog, nouveau_nv50_xfer_ctxprog, int, 0400);
+
 MODULE_PARM_DESC(ctxfw, "Use external HUB/GPC ucode (fermi)\n");
 int nouveau_ctxfw;
 module_param_named(ctxfw, nouveau_ctxfw, int, 0400);
diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h
index 4c0be3a..cdf5497 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -849,6 +849,7 @@ extern int nouveau_override_conntype;
 extern char *nouveau_perflvl;
 extern int nouveau_perflvl_wr;
 extern int nouveau_msi;
+extern int nouveau_nv50_xfer_ctxprog;
 extern int nouveau_ctxfw;
 
 extern int nouveau_pci_suspend(struct pci_dev *pdev, pm_message_t pm_state);
diff --git a/drivers/gpu/drm/nouveau/nv50_graph.c b/drivers/gpu/drm/nouveau/nv50_graph.c
index 01e4f812..d980bc0 100644
--- a/drivers/gpu/drm/nouveau/nv50_graph.c
+++ b/drivers/gpu/drm/nouveau/nv50_graph.c
@@ -167,7 +167,10 @@ nv50_graph_init(struct drm_device *dev, int engine)
 	nv_wr32(dev, 0x400324, 0x00000000);
 	for (i = 0; i < pgraph->ctxprog_size; i++)
 		nv_wr32(dev, 0x400328, pgraph->ctxprog[i]);
-	nv_wr32(dev, 0x400824, 0x00000000);
+	if (nouveau_nv50_xfer_ctxprog)
+		nv_wr32(dev, 0x400824, 0x00000000);
+	else
+		nv_wr32(dev, 0x400824, 0x00004000);
 	nv_wr32(dev, 0x400828, 0x00000000);
 	nv_wr32(dev, 0x40082c, 0x00000000);
 	nv_wr32(dev, 0x400830, 0x00000000);
diff --git a/drivers/gpu/drm/nouveau/nv50_grctx.c b/drivers/gpu/drm/nouveau/nv50_grctx.c
index 4b46d69..0bf2b0c 100644
--- a/drivers/gpu/drm/nouveau/nv50_grctx.c
+++ b/drivers/gpu/drm/nouveau/nv50_grctx.c
@@ -202,7 +202,8 @@ nv50_grctx_init(struct nouveau_grctx *ctx)
 	}
 
 	cp_set (ctx, STATE, RUNNING);
-	cp_set (ctx, XFER_SWITCH, ENABLE);
+	if (nouveau_nv50_xfer_ctxprog)
+		cp_set (ctx, XFER_SWITCH, ENABLE);
 	/* decide whether we're loading/unloading the context */
 	cp_bra (ctx, AUTO_SAVE, PENDING, cp_setup_save);
 	cp_bra (ctx, USER_SAVE, PENDING, cp_setup_save);
@@ -269,7 +270,8 @@ nv50_grctx_init(struct nouveau_grctx *ctx)
 	cp_name(ctx, cp_exit);
 	cp_set (ctx, USER_SAVE, NOT_PENDING);
 	cp_set (ctx, USER_LOAD, NOT_PENDING);
-	cp_set (ctx, XFER_SWITCH, DISABLE);
+	if (nouveau_nv50_xfer_ctxprog)
+		cp_set (ctx, XFER_SWITCH, DISABLE);
 	cp_set (ctx, STATE, STOPPED);
 	cp_out (ctx, CP_END);
 	ctx->ctxvals_pos += 0x400; /* padding... no idea why you need it */
-- 
1.7.8.rc3