Re: [PATCH 1/1] drm/i915: split power-related items into intel_pm module

[Ben Widawsky <ben@bwidawsk.net>]

On Mon, 16 Apr 2012 21:08:22 -0300
Eugeni Dodonov <eugeni@dodonov.net> wrote:

> On Mon, Apr 16, 2012 at 20:23, Chris Wilson
> <chris@chris-wilson.co.uk>wrote:
> 
> > On Mon, 16 Apr 2012 20:06:01 -0300, Eugeni Dodonov <
> > eugeni.dodonov@intel.com> wrote:
> > > As previously discussed on irc with Daniel, Ben and Jesse, This
> > > patch moves the power-related functionality into intel_pm module,
> > > aiming at simplifying the intel_display code and make it less
> > > cluttered.
> > >
> > > The functionality affected by this move are: clock gating, rc6
> > > and rps, display watermarks, display fifo-related items, cxsr and
> > > FBC.
> > >
> > > This simplifies intel_display by +/- 2800 lines of code and
> > > centralizes most of the power-related items in one place to
> > > simplify future hardware enablement and subsystems interaction.
> >
> > Diff is making this really difficult to verify that no functional
> > change is taking place. Suggestions? A series of small commits?
> >
> 
> I am fine with making it into a small series of commits which move 1
> subsystem at a time (fbc, cxsr, rc6, fifo, wm, ...). Would everyone be
> happy with that? I think Daniel wanted to move everything in 1 commit,
> hence this initial approach from my side, but yes, as functions are
> spread all around the place it makes very hard to review the patch.
> 
> But in general, does this idea of intel_pm.c looks interesting, or
> anyone has anything against it?
> 

I like intel_pm.c
I don't have an opinion yet on 1 big commit or many small commits.
I'd like if you made a description not just of what has been moved, but
what belongs in this file (as a comment in the .c file).

Also in the bikeshed category:
I don't feel like FBC belongs here.
I also don't know about display watermarks, and certain fifo things.
Similarly with fifo related stuff, some may belong, some may not - I
haven't actually looked at the patch.

Anyway, when/if danvet deals with this, I like the idea very much so
it's
Acked-by: Ben Widawsky <benjamin.widawsky@intel.com>