Re: [PATCH 4/4] drm/i915: creating Haswell rc6 function

From: Rodrigo Vivi <rodrigo.vivi@gmail.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 4/4] drm/i915: creating Haswell rc6 function
Date: Tue, 26 Mar 2013 16:37:49 -0300	[thread overview]
Message-ID: <CABVU7+uVp0CpMYb1KzCKTKZ3Rokh_6p6_w5c_UWmNKex5EZu3g@mail.gmail.com> (raw)
In-Reply-To: <CABVU7+vGLz7-S-1K8fwy2h4br-Fxzo9gQPCV3SxdnkXJWY4saw@mail.gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 3337 bytes --]

I forgot to say that I tested patch by patch today on my HSW ULT and just
noticed real improvement with the patch you already queued.

Where improvements are: at least 0.2W and also less oscillation and more %
on RC and package C states.

Although I'd like to see the rest of the series applied to have the code as
clean, organized and matching documentation as possible.

On Tue, Mar 26, 2013 at 4:32 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com>wrote:

> I'm in favor of this revert. Although I don't have any argument in values,
> I always guessed that many of rc6 bugs we have on snb came from the gap
> between the threashold values used and documented for snb.
>
>
> On Tue, Mar 26, 2013 at 4:00 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>
>> On Tue, Mar 26, 2013 at 01:32:51PM -0300, Rodrigo Vivi wrote:
>> > ah... got your point...
>> > I just split later because Ben wanted the frequency patch as the first
>> one
>> > so I decided to let split at last patch to be really optional...
>> > so, you suggestion is to revert the order of this two latest patches or
>> the
>> > 3?
>>
>> Yeah, that's the idea. But since I've merged the first one already I get
>> minus points for inconsistency, too :(
>>
>> > I guess frequency one was already queued right?
>>
>> Yeah, frequency one is already queued. That one looked more like a real
>> bugfix to me, since it essentially changes what we're writing into
>> functional registers. Hence why I've picked it right away.
>>
>> Another patch which is still dangling around is Chris' revert of
>>
>> commit 1ee9ae3244c4789f3184c5123f3b2d7e405b3f4c
>> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Date:   Wed Aug 15 10:41:45 2012 +0200
>>
>>     drm/i915: use hsw rps tuning values everywhere on gen6+
>>
>> With the split-up hsw rps stuff that's imo something we should look into
>> again I think. Chris?
>> -Daniel
>>
>> >
>> >
>> > On Tue, Mar 26, 2013 at 1:30 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> >
>> > > On Tue, Mar 26, 2013 at 5:25 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com
>> >
>> > > wrote:
>> > > > I just checked the code and this patch looks right for me.
>> > > > it doesn't add any if block... just remove them.
>> > > > What am I missing?
>> > >
>> > > You've added it right in the previous patch ;-)
>> > >
>> > > Which means if someone tries to understand the history of a given
>> > > piece of code with git blame, they now have to jump through these 2
>> > > patches which change nothing and are right following each another. But
>> > > in the usual recursive git blame mode you don't see that (or at least
>> > > I don't check for that by default), so you end up reading both patches
>> > > to make sure you still see where the code is moving around.
>> > >
>> > > So if you want to split (and I agree that it starts to make sense),
>> > > pls split first, then apply hsw changes to the hsw rps code only.
>> > > -Daniel
>> > > --
>> > > Daniel Vetter
>> > > Software Engineer, Intel Corporation
>> > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>> > >
>> >
>> >
>> >
>> > --
>> > Rodrigo Vivi
>> > Blog: http://blog.vivi.eng.br
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>>
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
>
>

-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

[-- Attachment #1.2: Type: text/html, Size: 5273 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx