From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH 1/1] drm/i915: split power-related items into intel_pm module Date: Mon, 16 Apr 2012 19:17:08 -0700 Message-ID: <20120416191708.59dd795c@bwidawsk.net> References: <1334617561-4400-1-git-send-email-eugeni.dodonov@intel.com> <1334618657_9970@CP5-2952> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from cloud01.chad-versace.us (184-106-247-128.static.cloud-ips.com [184.106.247.128]) by gabe.freedesktop.org (Postfix) with ESMTP id 2C52E9E815 for ; Mon, 16 Apr 2012 19:18:38 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Eugeni Dodonov Cc: Daniel Vetter , intel-gfx@lists.freedesktop.org, Eugeni Dodonov List-Id: intel-gfx@lists.freedesktop.org On Mon, 16 Apr 2012 21:08:22 -0300 Eugeni Dodonov wrote: > On Mon, Apr 16, 2012 at 20:23, Chris Wilson > 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