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

From: Daniel Vetter <daniel@ffwll.ch>
To: Rodrigo Vivi <rodrigo.vivi@gmail.com>,
	Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 4/4] drm/i915: creating Haswell rc6 function
Date: Tue, 26 Mar 2013 20:00:16 +0100	[thread overview]
Message-ID: <CAKMK7uGrN4oViZCL9VvmexMQ3YxWKCQrhNsdiiLYsZp-xUraag@mail.gmail.com> (raw)
In-Reply-To: <CABVU7+v8A0oik+ExBP_g4sRtAwRteO8pJo=VBZxeWECrf0T36Q@mail.gmail.com>

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