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 at 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 at 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 at 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 at ffwll.ch> wrote: >> > >> > > On Tue, Mar 26, 2013 at 5:25 PM, Rodrigo Vivi <rodrigo.vivi at 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 -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20130326/ba4b5280/attachment.html>