From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 2/4] drm/i915: add intel_encoder_power_get/put Date: Thu, 24 Apr 2014 17:14:27 +0200 Message-ID: <20140424151427.GE26374@phenom.ffwll.local> References: <20140423094504.GF18465@intel.com> <1398347368-2459-1-git-send-email-przanoni@gmail.com> <20140424141110.GR18465@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-ee0-f53.google.com (mail-ee0-f53.google.com [74.125.83.53]) by gabe.freedesktop.org (Postfix) with ESMTP id B2EA96E00C for ; Thu, 24 Apr 2014 08:14:33 -0700 (PDT) Received: by mail-ee0-f53.google.com with SMTP id b57so1981162eek.40 for ; Thu, 24 Apr 2014 08:14:31 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140424141110.GR18465@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Thu, Apr 24, 2014 at 05:11:10PM +0300, Ville Syrj=E4l=E4 wrote: > On Thu, Apr 24, 2014 at 10:49:28AM -0300, Paulo Zanoni wrote: > > From: Paulo Zanoni > > = > > This was suggested by Chris on his review to the first version of > > "drm/i915: get power domain in case the BIOS enabled eDP VDD". Well, > > at least that's what I understood from his comment :) > > = > > v2: - Make it intel_encoder_power_get/put > > - Don't call the new functions on functions where we need both > > get() and put(), so we don't end up calling > > intel_display_port_power_domain() more than needed. > > = > > Signed-off-by: Paulo Zanoni > > --- > > drivers/gpu/drm/i915/intel_dp.c | 19 +++++-------------- > > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > > drivers/gpu/drm/i915/intel_pm.c | 24 ++++++++++++++++++++++++ > > 3 files changed, 31 insertions(+), 14 deletions(-) > > = > > = > > I am not really sure if this patch is worth it... I certainly won't be = mad if we > > drop it. The diffstat is not good. > = > I'm not sure either. In fact I had this same idea already when I saw > Imre's power well patches for the first time, but then I thought that > it doesn't help that much and decided to keep the idea to myself. If you guys ask about my color choices I have to admit that I'm not a huge fan of intel_display_port_power_domain. In roughly object-oriented code functions with giant switch statements which use the object class to decide what to do tend to be a red flag ... In case you care for a reference see the switch statement code smell in Martin Fowler's refactoring. But that was just my bikeshed, tbh I don't care strongly enough here. -Daniel -- = Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch