All of lore.kernel.org
 help / color / mirror / Atom feed
* "enable ctxprog xfer only when we need it to save power" introduces big performance regression
@ 2011-10-29 17:08 Marcin Slusarz
       [not found] ` <20111029170801.GA3219-OI9uyE9O0yo@public.gmane.org>
  0 siblings, 1 reply; 16+ messages in thread
From: Marcin Slusarz @ 2011-10-29 17:08 UTC (permalink / raw)
  To: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Martin Peres; +Cc: Ben Skeggs

Hi

I've bisected pretty big performance regression (nv92):

$ git bisect good
b2737681d5442f05ab6419e05468c3d2511a5ced is the first bad commit
commit b2737681d5442f05ab6419e05468c3d2511a5ced
Author: Martin Peres <martin.peres-Iz16wY1oaNPLSKGbIzaifA@public.gmane.org>
Date:   Sat Jul 30 23:08:45 2011 +0200

    drm/nv50/gr: enable ctxprog xfer only when we need it to save power

    This patch adds instructions to ctxprog and by doing, impacts context
    switching performance.  My testcase showed a 1% performance cost using
    glxgears that is a context-switch bound application.

    Please test and report bugs/performance/power/other.

    Many thanks to Maxim Levitsky for his dedicated work on lowering power
    consumption with nouveau.

    More patches are coming thanks to his work:

    https://bugs.freedesktop.org/show_bug.cgi?id=37922

    Signed-off-by: Martin Peres <martin.peres-Iz16wY1oaNPLSKGbIzaifA@public.gmane.org>
    Signed-off-by: Ben Skeggs <bskeggs-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

:040000 040000 3fcd1388cf04c71cb7a11f29f7be6033c948f119 418a527ee5970eabaa0eb6f8cb3dc96b4da523ea M      drivers

The numbers are (FPS):
nexuiz:            61 ->   49 (-19%)
xonotic:           59 ->   37 (-37%)
openarena:        163 ->  127 (-22%)
world of padman:  242 ->  181 (-25%)
warsow:            91 ->   61 (-32%)
glxgears:        1198 -> 1107 (- 7%)

Now, what can we do about it?

Marcin

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: "enable ctxprog xfer only when we need it to save power" introduces big performance regression
       [not found] ` <20111029170801.GA3219-OI9uyE9O0yo@public.gmane.org>
@ 2011-10-29 17:29   ` Marcin Slusarz
       [not found]     ` <20111029172923.GA3110-OI9uyE9O0yo@public.gmane.org>
  0 siblings, 1 reply; 16+ messages in thread
From: Marcin Slusarz @ 2011-10-29 17:29 UTC (permalink / raw)
  To: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Martin Peres; +Cc: Ben Skeggs

On Sat, Oct 29, 2011 at 07:08:01PM +0200, Marcin Slusarz wrote:
> Hi
> 
> I've bisected pretty big performance regression (nv92):
> 
> $ git bisect good
> b2737681d5442f05ab6419e05468c3d2511a5ced is the first bad commit
> commit b2737681d5442f05ab6419e05468c3d2511a5ced
> Author: Martin Peres <martin.peres-Iz16wY1oaNPLSKGbIzaifA@public.gmane.org>
> Date:   Sat Jul 30 23:08:45 2011 +0200
> 
>     drm/nv50/gr: enable ctxprog xfer only when we need it to save power

Weird, reverting it on top of current git does not restore it - in
glxgears it's even worse (-~50%).

Marcin

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: "enable ctxprog xfer only when we need it to save power" introduces big performance regression
       [not found]     ` <20111029172923.GA3110-OI9uyE9O0yo@public.gmane.org>
@ 2011-10-29 18:00       ` Maarten Maathuis
       [not found]         ` <CAGZ4FETt6QzMkxrMQwJsszjuQq0wdt85ujRZbxkdurkduaHh2A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2011-10-29 19:09       ` Marcin Slusarz
  1 sibling, 1 reply; 16+ messages in thread
From: Maarten Maathuis @ 2011-10-29 18:00 UTC (permalink / raw)
  To: Marcin Slusarz
  Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ben Skeggs,
	Martin Peres

On Sat, Oct 29, 2011 at 7:29 PM, Marcin Slusarz
<marcin.slusarz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Sat, Oct 29, 2011 at 07:08:01PM +0200, Marcin Slusarz wrote:
>> Hi
>>
>> I've bisected pretty big performance regression (nv92):
>>
>> $ git bisect good
>> b2737681d5442f05ab6419e05468c3d2511a5ced is the first bad commit
>> commit b2737681d5442f05ab6419e05468c3d2511a5ced
>> Author: Martin Peres <martin.peres-Iz16wY1oaNPLSKGbIzaifA@public.gmane.org>
>> Date:   Sat Jul 30 23:08:45 2011 +0200
>>
>>     drm/nv50/gr: enable ctxprog xfer only when we need it to save power
>
> Weird, reverting it on top of current git does not restore it - in
> glxgears it's even worse (-~50%).
>
> Marcin
> _______________________________________________
> Nouveau mailing list
> Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau
>

Before this patch, what ensured that context switching was enabled?

-- 
Far away from the primal instinct, the song seems to fade away, the
river get wider between your thoughts and the things we do and say.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: "enable ctxprog xfer only when we need it to save power" introduces big performance regression
       [not found]     ` <20111029172923.GA3110-OI9uyE9O0yo@public.gmane.org>
  2011-10-29 18:00       ` Maarten Maathuis
@ 2011-10-29 19:09       ` Marcin Slusarz
       [not found]         ` <20111029190938.GA3105-OI9uyE9O0yo@public.gmane.org>
  1 sibling, 1 reply; 16+ messages in thread
From: Marcin Slusarz @ 2011-10-29 19:09 UTC (permalink / raw)
  To: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Martin Peres; +Cc: Ben Skeggs

On Sat, Oct 29, 2011 at 07:29:23PM +0200, Marcin Slusarz wrote:
> On Sat, Oct 29, 2011 at 07:08:01PM +0200, Marcin Slusarz wrote:
> > Hi
> > 
> > I've bisected pretty big performance regression (nv92):
> > 
> > $ git bisect good
> > b2737681d5442f05ab6419e05468c3d2511a5ced is the first bad commit
> > commit b2737681d5442f05ab6419e05468c3d2511a5ced
> > Author: Martin Peres <martin.peres-Iz16wY1oaNPLSKGbIzaifA@public.gmane.org>
> > Date:   Sat Jul 30 23:08:45 2011 +0200
> > 
> >     drm/nv50/gr: enable ctxprog xfer only when we need it to save power
> 
> Weird, reverting it on top of current git does not restore it - in
> glxgears it's even worse (-~50%).

It seems "drm/nv50/gr: refactor initialisation" relies on the above.

So, reverting these commits:

drm/nv50/gr: typo fix, how about we not reset fifo during graph init?
drm/nv50/gr: refactor initialisation
drm/nv50/gr: enable ctxprog xfer only when we need it to save power

on top of current git restores performance to the previous state.

(First commit fixes bug in 2nd, so it needs to be reverted first)

Marcin

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: "enable ctxprog xfer only when we need it to save power" introduces big performance regression
       [not found]         ` <20111029190938.GA3105-OI9uyE9O0yo@public.gmane.org>
@ 2011-10-30  0:25           ` Maxim Levitsky
  2011-10-30 10:33             ` Marcin Slusarz
  2011-11-09 22:10           ` Marcin Slusarz
  1 sibling, 1 reply; 16+ messages in thread
From: Maxim Levitsky @ 2011-10-30  0:25 UTC (permalink / raw)
  To: Marcin Slusarz
  Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ben Skeggs,
	Martin Peres

On Sat, 2011-10-29 at 21:09 +0200, Marcin Slusarz wrote:
> On Sat, Oct 29, 2011 at 07:29:23PM +0200, Marcin Slusarz wrote:
> > On Sat, Oct 29, 2011 at 07:08:01PM +0200, Marcin Slusarz wrote:
> > > Hi
> > > 
> > > I've bisected pretty big performance regression (nv92):
> > > 
> > > $ git bisect good
> > > b2737681d5442f05ab6419e05468c3d2511a5ced is the first bad commit
> > > commit b2737681d5442f05ab6419e05468c3d2511a5ced
> > > Author: Martin Peres <martin.peres-Iz16wY1oaNPLSKGbIzaifA@public.gmane.org>
> > > Date:   Sat Jul 30 23:08:45 2011 +0200
> > > 
> > >     drm/nv50/gr: enable ctxprog xfer only when we need it to save power
> > 
> > Weird, reverting it on top of current git does not restore it - in
> > glxgears it's even worse (-~50%).
> 
> It seems "drm/nv50/gr: refactor initialisation" relies on the above.
> 
> So, reverting these commits:
> 
> drm/nv50/gr: typo fix, how about we not reset fifo during graph init?
> drm/nv50/gr: refactor initialisation
> drm/nv50/gr: enable ctxprog xfer only when we need it to save power
> 
> on top of current git restores performance to the previous state.
> 
> (First commit fixes bug in 2nd, so it needs to be reverted first)
> 
> Marcin


could you extract the ctxprog blob uses on your system and post it?

Best regards,
        Maxim Levitsky

PS: this is same mail I send you without CC'ing the list, sorry hit
wrong button,

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: "enable ctxprog xfer only when we need it to save power" introduces big performance regression
       [not found]         ` <CAGZ4FETt6QzMkxrMQwJsszjuQq0wdt85ujRZbxkdurkduaHh2A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2011-10-30  1:15           ` Ben Skeggs
  0 siblings, 0 replies; 16+ messages in thread
From: Ben Skeggs @ 2011-10-30  1:15 UTC (permalink / raw)
  To: Maarten Maathuis; +Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Martin Peres

On Sat, 2011-10-29 at 20:00 +0200, Maarten Maathuis wrote:
> On Sat, Oct 29, 2011 at 7:29 PM, Marcin Slusarz
> <marcin.slusarz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Sat, Oct 29, 2011 at 07:08:01PM +0200, Marcin Slusarz wrote:
> >> Hi
> >>
> >> I've bisected pretty big performance regression (nv92):
> >>
> >> $ git bisect good
> >> b2737681d5442f05ab6419e05468c3d2511a5ced is the first bad commit
> >> commit b2737681d5442f05ab6419e05468c3d2511a5ced
> >> Author: Martin Peres <martin.peres-Iz16wY1oaNPLSKGbIzaifA@public.gmane.org>
> >> Date:   Sat Jul 30 23:08:45 2011 +0200
> >>
> >>     drm/nv50/gr: enable ctxprog xfer only when we need it to save power
> >
> > Weird, reverting it on top of current git does not restore it - in
> > glxgears it's even worse (-~50%).
> >
> > Marcin
> > _______________________________________________
> > Nouveau mailing list
> > Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> > http://lists.freedesktop.org/mailman/listinfo/nouveau
> >
> 
> Before this patch, what ensured that context switching was enabled?
It was enabled by default during graph init, this was removed with the
"drm/nv50/gr: refactor initialisation" commit.

Ben.
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: "enable ctxprog xfer only when we need it to save power" introduces big performance regression
  2011-10-30  0:25           ` Maxim Levitsky
@ 2011-10-30 10:33             ` Marcin Slusarz
       [not found]               ` <20111030103349.GA3157-OI9uyE9O0yo@public.gmane.org>
  0 siblings, 1 reply; 16+ messages in thread
From: Marcin Slusarz @ 2011-10-30 10:33 UTC (permalink / raw)
  To: Maxim Levitsky
  Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ben Skeggs,
	Martin Peres

On Sun, Oct 30, 2011 at 02:25:08AM +0200, Maxim Levitsky wrote:
> On Sat, 2011-10-29 at 21:09 +0200, Marcin Slusarz wrote:
> > On Sat, Oct 29, 2011 at 07:29:23PM +0200, Marcin Slusarz wrote:
> > > On Sat, Oct 29, 2011 at 07:08:01PM +0200, Marcin Slusarz wrote:
> > > > Hi
> > > > 
> > > > I've bisected pretty big performance regression (nv92):
> > > > 
> > > > $ git bisect good
> > > > b2737681d5442f05ab6419e05468c3d2511a5ced is the first bad commit
> > > > commit b2737681d5442f05ab6419e05468c3d2511a5ced
> > > > Author: Martin Peres <martin.peres-Iz16wY1oaNPLSKGbIzaifA@public.gmane.org>
> > > > Date:   Sat Jul 30 23:08:45 2011 +0200
> > > > 
> > > >     drm/nv50/gr: enable ctxprog xfer only when we need it to save power
> > > 
> > > Weird, reverting it on top of current git does not restore it - in
> > > glxgears it's even worse (-~50%).
> > 
> > It seems "drm/nv50/gr: refactor initialisation" relies on the above.
> > 
> > So, reverting these commits:
> > 
> > drm/nv50/gr: typo fix, how about we not reset fifo during graph init?
> > drm/nv50/gr: refactor initialisation
> > drm/nv50/gr: enable ctxprog xfer only when we need it to save power
> > 
> > on top of current git restores performance to the previous state.
> > 
> > (First commit fixes bug in 2nd, so it needs to be reverted first)
> > 
> > Marcin
> 
> 
> could you extract the ctxprog blob uses on your system and post it?
> 

How can I extract it?

Marcin

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: "enable ctxprog xfer only when we need it to save power" introduces big performance regression
       [not found]               ` <20111030103349.GA3157-OI9uyE9O0yo@public.gmane.org>
@ 2011-10-30 10:37                 ` Marcin Kościelnicki
  0 siblings, 0 replies; 16+ messages in thread
From: Marcin Kościelnicki @ 2011-10-30 10:37 UTC (permalink / raw)
  To: Marcin Slusarz
  Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Peres,
	Martin-CC+yJ3UmIYqDUpFQwHEjaQ, Ben Skeggs

On 30.10.2011 12:33, Marcin Slusarz wrote:
> On Sun, Oct 30, 2011 at 02:25:08AM +0200, Maxim Levitsky wrote:
>> On Sat, 2011-10-29 at 21:09 +0200, Marcin Slusarz wrote:
>> > On Sat, Oct 29, 2011 at 07:29:23PM +0200, Marcin Slusarz wrote:
>> > > On Sat, Oct 29, 2011 at 07:08:01PM +0200, Marcin Slusarz wrote:
>> > > > Hi
>> > > >
>> > > > I've bisected pretty big performance regression (nv92):
>> > > >
>> > > > $ git bisect good
>> > > > b2737681d5442f05ab6419e05468c3d2511a5ced is the first bad 
>> commit
>> > > > commit b2737681d5442f05ab6419e05468c3d2511a5ced
>> > > > Author: Martin Peres <martin.peres@ensi-bourges.fr>
>> > > > Date:   Sat Jul 30 23:08:45 2011 +0200
>> > > >
>> > > >     drm/nv50/gr: enable ctxprog xfer only when we need it to 
>> save power
>> > >
>> > > Weird, reverting it on top of current git does not restore it - 
>> in
>> > > glxgears it's even worse (-~50%).
>> >
>> > It seems "drm/nv50/gr: refactor initialisation" relies on the 
>> above.
>> >
>> > So, reverting these commits:
>> >
>> > drm/nv50/gr: typo fix, how about we not reset fifo during graph 
>> init?
>> > drm/nv50/gr: refactor initialisation
>> > drm/nv50/gr: enable ctxprog xfer only when we need it to save 
>> power
>> >
>> > on top of current git restores performance to the previous state.
>> >
>> > (First commit fixes bug in 2nd, so it needs to be reverted first)
>> >
>> > Marcin
>>
>>
>> could you extract the ctxprog blob uses on your system and post it?
>>
>
> How can I extract it?
>
> Marcin

Do a mmiotrace, look for a string of writes to 400328 preceded by a 
write of 0 to 400324.

Marcin Kościelnicki
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: "enable ctxprog xfer only when we need it to save power" introduces big performance regression
       [not found]         ` <20111029190938.GA3105-OI9uyE9O0yo@public.gmane.org>
  2011-10-30  0:25           ` Maxim Levitsky
@ 2011-11-09 22:10           ` Marcin Slusarz
       [not found]             ` <20111109221009.GD3402-OI9uyE9O0yo@public.gmane.org>
  1 sibling, 1 reply; 16+ messages in thread
From: Marcin Slusarz @ 2011-11-09 22:10 UTC (permalink / raw)
  To: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Martin Peres; +Cc: Ben Skeggs

On Sat, Oct 29, 2011 at 09:09:38PM +0200, Marcin Slusarz wrote:
> On Sat, Oct 29, 2011 at 07:29:23PM +0200, Marcin Slusarz wrote:
> > On Sat, Oct 29, 2011 at 07:08:01PM +0200, Marcin Slusarz wrote:
> > > Hi
> > > 
> > > I've bisected pretty big performance regression (nv92):
> > > 
> > > $ git bisect good
> > > b2737681d5442f05ab6419e05468c3d2511a5ced is the first bad commit
> > > commit b2737681d5442f05ab6419e05468c3d2511a5ced
> > > Author: Martin Peres <martin.peres-Iz16wY1oaNPLSKGbIzaifA@public.gmane.org>
> > > Date:   Sat Jul 30 23:08:45 2011 +0200
> > > 
> > >     drm/nv50/gr: enable ctxprog xfer only when we need it to save power
> > 
> > Weird, reverting it on top of current git does not restore it - in
> > glxgears it's even worse (-~50%).
> 
> It seems "drm/nv50/gr: refactor initialisation" relies on the above.
> 
> So, reverting these commits:
> 
> drm/nv50/gr: typo fix, how about we not reset fifo during graph init?
> drm/nv50/gr: refactor initialisation
> drm/nv50/gr: enable ctxprog xfer only when we need it to save power
> 
> on top of current git restores performance to the previous state.
> 
> (First commit fixes bug in 2nd, so it needs to be reverted first)

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: "enable ctxprog xfer only when we need it to save power" introduces big performance regression
       [not found]             ` <20111109221009.GD3402-OI9uyE9O0yo@public.gmane.org>
@ 2011-11-10  7:10               ` Martin Peres
       [not found]                 ` <4EBB78F4.2020902-Iz16wY1oaNPLSKGbIzaifA@public.gmane.org>
  0 siblings, 1 reply; 16+ messages in thread
From: Martin Peres @ 2011-11-10  7:10 UTC (permalink / raw)
  To: Marcin Slusarz; +Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ben Skeggs

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.

Martin

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: "enable ctxprog xfer only when we need it to save power" introduces big performance regression
       [not found]                 ` <4EBB78F4.2020902-Iz16wY1oaNPLSKGbIzaifA@public.gmane.org>
@ 2011-12-28 21:39                   ` Marcin Slusarz
       [not found]                     ` <20111228213902.GA4275-OI9uyE9O0yo@public.gmane.org>
  0 siblings, 1 reply; 16+ messages in thread
From: Marcin Slusarz @ 2011-12-28 21:39 UTC (permalink / raw)
  To: Martin Peres; +Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ben Skeggs

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@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@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

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: "enable ctxprog xfer only when we need it to save power" introduces big performance regression
       [not found]                     ` <20111228213902.GA4275-OI9uyE9O0yo@public.gmane.org>
@ 2011-12-28 23:58                       ` Martin Peres
       [not found]                         ` <4EFBAD2B.8080207-Iz16wY1oaNPLSKGbIzaifA@public.gmane.org>
  0 siblings, 1 reply; 16+ messages in thread
From: Martin Peres @ 2011-12-28 23:58 UTC (permalink / raw)
  To: Marcin Slusarz; +Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ben Skeggs

On 28/12/2011 22:39, Marcin Slusarz wrote:
> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Hi,

I'm really sorry about not following up to your many emails you sent me 
in the past.

I now have time to work on this issue and I would like a little more 
information if you don't mind. Could you do a mmiotrace and look at what 
nvidia does in their ctxprog for your specific card ?

If nVidia does the same thing as we do, well, you'll have to wait for 
something I've been thinking for some times now.

What we want is to define power consumption profiles. A simple example 
is, when on battery, you don't want to save as much power as possible 
but when you are on sector, you may want to get the full performance out 
of your card (You may also don't mind the performance loss, but one 
thing at a time).

Ben (darktama) has recently talked about introducing a kind of automatic 
reclocking kind of like what radeon does. That is to say, having 
performance profiles that are switched according to the energy source 
(battery / sector). This automatic behaviour could then be overriden by 
the user, just like radeon.

We would then turn some nobs according to the current performance 
profile. One of these nobs could be xfer on some specific cards (yours).

Would that solution suit you? Clearly, introducing a new kernel 
parameter isn't an acceptable solution to the problem.

I hope we can find a way that would suit both performance and 
battery-life while being kernel-friendly.

Martin (mupuf)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: "enable ctxprog xfer only when we need it to save power" introduces big performance regression
       [not found]                         ` <4EFBAD2B.8080207-Iz16wY1oaNPLSKGbIzaifA@public.gmane.org>
@ 2011-12-29  1:38                           ` Marcin Slusarz
       [not found]                             ` <20111229013850.GA4076-OI9uyE9O0yo@public.gmane.org>
  0 siblings, 1 reply; 16+ messages in thread
From: Marcin Slusarz @ 2011-12-29  1:38 UTC (permalink / raw)
  To: Martin Peres; +Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ben Skeggs

On Thu, Dec 29, 2011 at 12:58:35AM +0100, Martin Peres wrote:
> On 28/12/2011 22:39, Marcin Slusarz wrote:
> > 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> Hi,
> 
> I'm really sorry about not following up to your many emails you sent me 
> in the past.
> 
> I now have time to work on this issue and I would like a little more 
> information if you don't mind. Could you do a mmiotrace and look at what 
> nvidia does in their ctxprog for your specific card ?

Nvidia does the same, see http://paste.pocoo.org/show/500376/ (we talked about
it on IRC on 30 Oct). If you want me to get something else out of the mmiotrace,
just tell me.

> If nVidia does the same thing as we do, well, you'll have to wait for 
> something I've been thinking for some times now.
> 
> What we want is to define power consumption profiles. A simple example 
> is, when on battery, you don't want to save as much power as possible 
> but when you are on sector, you may want to get the full performance out 
> of your card (You may also don't mind the performance loss, but one 
> thing at a time).
>
> Ben (darktama) has recently talked about introducing a kind of automatic 
> reclocking kind of like what radeon does. That is to say, having 
> performance profiles that are switched according to the energy source 
> (battery / sector). This automatic behaviour could then be overriden by 
> the user, just like radeon.
> 
> We would then turn some nobs according to the current performance 
> profile. One of these nobs could be xfer on some specific cards (yours).
> 
> Would that solution suit you?

Sounds good. I initially thought about connecting it to reclocking, but your
idea is even better - xfers enablement could depend only on power source.

I tried to implement xfers runtime switching, but couldn't figure out how to
change ctxprog behaviour without rebuilding and reuploading whole thing.
Reading host-writable memory from ctxprog would be enough.

> Clearly, introducing a new kernel 
> parameter isn't an acceptable solution to the problem.

Yeah, I just wanted to start the discussion.

Marcin

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: "enable ctxprog xfer only when we need it to save power" introduces big performance regression
       [not found]                             ` <20111229013850.GA4076-OI9uyE9O0yo@public.gmane.org>
@ 2012-01-13 21:26                               ` Martin Peres
       [not found]                                 ` <4F10A18E.1030908-GANU6spQydw@public.gmane.org>
  0 siblings, 1 reply; 16+ messages in thread
From: Martin Peres @ 2012-01-13 21:26 UTC (permalink / raw)
  To: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

[-- Attachment #1: Type: text/plain, Size: 801 bytes --]

Hi Marcin,

As promised, I started working on your performance problem.

Le 29/12/2011 02:38, Marcin Slusarz a écrit :
> I tried to implement xfers runtime switching, but couldn't figure out 
> how to change ctxprog behaviour without rebuilding and reuploading 
> whole thing. Reading host-writable memory from ctxprog would be enough. 
Well, I did that for you (see the patch enclosed). The patch uses the 
4th ctxprog flag register's bit 0 to store the wanted behaviour.
Mwk suggested me to do so and it worked perfectly.

You have all the instructions in the commit message, please report on 
the actual results!

By the mean time, I'll plug it to a PM brain so as it would switch back 
and forth between the two modes according to the load or the perflvl.

Take care,

Martin

[-- Attachment #2: 0001-drm-nv50-gr-make-ctxprog-decide-at-run-time-to-disab.patch --]
[-- Type: text/x-patch, Size: 2676 bytes --]

From 8e6667c87074b1b519fef0946083d46d01dfe8a0 Mon Sep 17 00:00:00 2001
From: Martin Peres <martin.peres-BktLLJ5BOkI@public.gmane.org>
Date: Fri, 13 Jan 2012 22:05:28 +0100
Subject: [PATCH] drm/nv50/gr: make ctxprog decide at run time to disable or
 not xfer

This commit is a follow-up on "drm/nv50/gr: enable ctxprog xfer only when we need it to save power".

The current situation is that some cards (nv92) have an hw regression that lower performance that
has been reported to about 15%.

This commit is the initial work to get the best of both world, that is to say:
- Performance when it is actually needed
- Power consumption when current performance is enough for the current needs

How-to:
- To go for performance      : nvapoke 400830 1
- To go for power consumption: nvapoke 400830 0

Then, force the card to do a context switch (switch to a TTY for instance).
Make sure it worked by peeking 400824. Bit 0x4000 is the XFER_ENABLE.

Reported-by: Marcin Slusarz <marcin.slusarz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Martin Peres <martin.peres-BktLLJ5BOkI@public.gmane.org>
---
 drivers/gpu/drm/nouveau/nv50_grctx.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nv50_grctx.c b/drivers/gpu/drm/nouveau/nv50_grctx.c
index 4b46d69..b2fdb7d 100644
--- a/drivers/gpu/drm/nouveau/nv50_grctx.c
+++ b/drivers/gpu/drm/nouveau/nv50_grctx.c
@@ -74,6 +74,11 @@
 #define CP_FLAG_INTR_NOT_PENDING      0
 #define CP_FLAG_INTR_PENDING          1
 
+/* CTXCTL_FLAGS_3: should only be written by host ! */
+#define CP_FLAG_PM_XFER               ((3 * 32) + 0)
+#define CP_FLAG_PM_XFER_ENABLE        0
+#define CP_FLAG_PM_XFER_DISABLE       1
+
 #define CP_CTX                   0x00100000
 #define CP_CTX_COUNT             0x000f0000
 #define CP_CTX_COUNT_SHIFT               16
@@ -163,6 +168,7 @@ enum cp_label {
 	cp_setup_save,
 	cp_swap_state,
 	cp_prepare_exit,
+	cp_xfer,
 	cp_exit,
 };
 
@@ -263,13 +269,17 @@ nv50_grctx_init(struct nouveau_grctx *ctx)
 	cp_set (ctx, UNK03, CLEAR);
 	cp_set (ctx, UNK1D, CLEAR);
 
-	cp_bra (ctx, USER_SAVE, PENDING, cp_exit);
+	cp_bra (ctx, USER_SAVE, PENDING, cp_xfer);
 	cp_out (ctx, CP_NEXT_TO_CURRENT);
 
+	/* disable xfer unless we are told otherwise by the host */
+	cp_name(ctx, cp_xfer);
+	cp_bra (ctx, PM_XFER, DISABLE, cp_exit);
+	cp_set (ctx, XFER_SWITCH, DISABLE);
+
 	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);
 	cp_set (ctx, STATE, STOPPED);
 	cp_out (ctx, CP_END);
 	ctx->ctxvals_pos += 0x400; /* padding... no idea why you need it */
-- 
1.7.8.3


[-- Attachment #3: Type: text/plain, Size: 181 bytes --]

_______________________________________________
Nouveau mailing list
Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: "enable ctxprog xfer only when we need it to save power" introduces big performance regression
       [not found]                                 ` <4F10A18E.1030908-GANU6spQydw@public.gmane.org>
@ 2012-01-17 20:55                                   ` Lucas Stach
  2012-01-17 21:17                                     ` Martin Peres
  0 siblings, 1 reply; 16+ messages in thread
From: Lucas Stach @ 2012-01-17 20:55 UTC (permalink / raw)
  To: Martin Peres; +Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Am Freitag, den 13.01.2012, 22:26 +0100 schrieb Martin Peres:
> Hi Marcin,
> 
> As promised, I started working on your performance problem.
> 
> Le 29/12/2011 02:38, Marcin Slusarz a écrit :
> > I tried to implement xfers runtime switching, but couldn't figure out 
> > how to change ctxprog behaviour without rebuilding and reuploading 
> > whole thing. Reading host-writable memory from ctxprog would be enough. 
> Well, I did that for you (see the patch enclosed). The patch uses the 
> 4th ctxprog flag register's bit 0 to store the wanted behaviour.
> Mwk suggested me to do so and it worked perfectly.
> 
> You have all the instructions in the commit message, please report on 
> the actual results!
> 
> By the mean time, I'll plug it to a PM brain so as it would switch back 
> and forth between the two modes according to the load or the perflvl.

Isn't it possible that the performance regression seen with xfer
disabled by default is caused by slow memory clock speed? Martin, you
saw only a 1% performance drop on your 8600 which is running with full
speed by default. Marcins nv92 is likely running at a much lower clock
speed. It would be nice to know if the perf regression is still as big
when running with full speed. We could stop caring about this when the
perf regression is not observable in the highest perf level.

> 
> Take care,
> 
> Martin
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau


_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: "enable ctxprog xfer only when we need it to save power" introduces big performance regression
  2012-01-17 20:55                                   ` Lucas Stach
@ 2012-01-17 21:17                                     ` Martin Peres
  0 siblings, 0 replies; 16+ messages in thread
From: Martin Peres @ 2012-01-17 21:17 UTC (permalink / raw)
  To: Lucas Stach; +Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Le 17/01/2012 21:55, Lucas Stach a écrit :
> Isn't it possible that the performance regression seen with xfer 
> disabled by default is caused by slow memory clock speed? Martin, you 
> saw only a 1% performance drop on your 8600 which is running with full 
> speed by default. Marcins nv92 is likely running at a much lower clock 
> speed. It would be nice to know if the perf regression is still as big 
> when running with full speed. We could stop caring about this when the 
> perf regression is not observable in the highest perf level.
Well, actually, the performance hit should be higher when running at a 
highest memory clock since we context switch more often (there is no 
logic here, just a personal experience).

Anyway, I tried again last week end at every possible speed and I 
haven't been able to spot any performance hit *at all*.
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2012-01-17 21:17 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-29 17:08 "enable ctxprog xfer only when we need it to save power" introduces big performance regression Marcin Slusarz
     [not found] ` <20111029170801.GA3219-OI9uyE9O0yo@public.gmane.org>
2011-10-29 17:29   ` Marcin Slusarz
     [not found]     ` <20111029172923.GA3110-OI9uyE9O0yo@public.gmane.org>
2011-10-29 18:00       ` Maarten Maathuis
     [not found]         ` <CAGZ4FETt6QzMkxrMQwJsszjuQq0wdt85ujRZbxkdurkduaHh2A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-10-30  1:15           ` Ben Skeggs
2011-10-29 19:09       ` Marcin Slusarz
     [not found]         ` <20111029190938.GA3105-OI9uyE9O0yo@public.gmane.org>
2011-10-30  0:25           ` Maxim Levitsky
2011-10-30 10:33             ` Marcin Slusarz
     [not found]               ` <20111030103349.GA3157-OI9uyE9O0yo@public.gmane.org>
2011-10-30 10:37                 ` Marcin Kościelnicki
2011-11-09 22:10           ` Marcin Slusarz
     [not found]             ` <20111109221009.GD3402-OI9uyE9O0yo@public.gmane.org>
2011-11-10  7:10               ` Martin Peres
     [not found]                 ` <4EBB78F4.2020902-Iz16wY1oaNPLSKGbIzaifA@public.gmane.org>
2011-12-28 21:39                   ` Marcin Slusarz
     [not found]                     ` <20111228213902.GA4275-OI9uyE9O0yo@public.gmane.org>
2011-12-28 23:58                       ` Martin Peres
     [not found]                         ` <4EFBAD2B.8080207-Iz16wY1oaNPLSKGbIzaifA@public.gmane.org>
2011-12-29  1:38                           ` Marcin Slusarz
     [not found]                             ` <20111229013850.GA4076-OI9uyE9O0yo@public.gmane.org>
2012-01-13 21:26                               ` Martin Peres
     [not found]                                 ` <4F10A18E.1030908-GANU6spQydw@public.gmane.org>
2012-01-17 20:55                                   ` Lucas Stach
2012-01-17 21:17                                     ` Martin Peres

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.