From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 3/7] drm/i915: add intel_display_power_enabled Date: Fri, 19 Apr 2013 17:45:00 +0200 Message-ID: <20130419154500.GW6169@phenom.ffwll.local> References: <1366313746-3659-1-git-send-email-przanoni@gmail.com> <1366313746-3659-4-git-send-email-przanoni@gmail.com> <20130419054447.GC23752@strange.amr.corp.intel.com> <20130419081553.GQ6169@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f45.google.com (mail-ee0-f45.google.com [74.125.83.45]) by gabe.freedesktop.org (Postfix) with ESMTP id AC381E5C67 for ; Fri, 19 Apr 2013 08:42:00 -0700 (PDT) Received: by mail-ee0-f45.google.com with SMTP id l10so320132eei.4 for ; Fri, 19 Apr 2013 08:42:00 -0700 (PDT) Content-Disposition: inline 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: Paulo Zanoni Cc: Intel Graphics Development , Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Fri, Apr 19, 2013 at 12:28:30PM -0300, Paulo Zanoni wrote: > Hi > > 2013/4/19 Daniel Vetter : > > On Fri, Apr 19, 2013 at 06:44:47AM +0100, Damien Lespiau wrote: > >> On Thu, Apr 18, 2013 at 04:35:42PM -0300, Paulo Zanoni wrote: > >> > From: Paulo Zanoni > >> > > >> > This should replace intel_using_power_well. The idea is that we're > >> > adding the requested power domain as an argument, so this might enable > >> > the code to look less platform-specific and also allows us to easily > >> > add new domains in case we need. > >> > > >> > Requested-by: Daniel Vetter > >> > Signed-off-by: Paulo Zanoni > >> > --- > >> > drivers/gpu/drm/i915/intel_display.c | 16 +++++++++++----- > >> > drivers/gpu/drm/i915/intel_drv.h | 10 +++++++++- > >> > drivers/gpu/drm/i915/intel_pm.c | 23 ++++++++++++++++++----- > >> > 3 files changed, 38 insertions(+), 11 deletions(-) > >> > > >> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > >> > index b86023d..b492eaa 100644 > >> > --- a/drivers/gpu/drm/i915/intel_display.c > >> > +++ b/drivers/gpu/drm/i915/intel_display.c > >> > @@ -1227,8 +1227,8 @@ void assert_pipe(struct drm_i915_private *dev_priv, > >> > if (pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) > >> > state = true; > >> > > >> > - if (!intel_using_power_well(dev_priv->dev) && > >> > - cpu_transcoder != TRANSCODER_EDP) { > >> > + if (cpu_transcoder != TRANSCODER_EDP && > >> > + !intel_display_power_enabled(dev_priv->dev, POWER_DOMAIN_OTHER)) { > >> > >> I'm wondering if we could simplify the look of this and try to avoid > >> using POWER_DOMAIN_OTHER, for instance having a: > >> > >> if (!intel_display_power_enabled(dev, POWER_DOMAIN_TRANSCODER(cpu_transcoder))) > >> return false; > >> > >> playing a bit with the POWER_DOMAIN_TRANSCODER_* enums to encode the > >> the cpu_transcoder in them. > > > > Well I have my own bikeshed - imo POWER_DOMAIN_TRANSCODER_EDP should be > > called just POWER_DOMAIN_EDP since it also includes the eDP port. So the > > transcoder part in the name is a bit misleading. And I suspect that we > > still miss some DDI A vs everything else checks in the hw state readout > > code ... > > I think POWER_DOMAIN_EDP can be misleading. We can have "PIPE_B + > TRANSCODER_EDP" configured (who can guarantee what the xrandr apps > really do?), and we can also have the "eDP on port D" which will never > use TRANSCODER_EDP, it will use PIPE_X + CPU_TRANSCODER_X. Yeah, I guess that makes sense. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch