From mboxrd@z Thu Jan  1 00:00:00 1970
From: Daniel Vetter <daniel@ffwll.ch>
Subject: Re: [PATCH 66/66] drm/i915: runtime PM support for DPMS
Date: Sat, 17 May 2014 00:19:09 +0200
Message-ID: <20140516221909.GY8790@phenom.ffwll.local>
References: <1398376542-27825-1-git-send-email-daniel.vetter@ffwll.ch>
 <1398376542-27825-67-git-send-email-daniel.vetter@ffwll.ch>
 <20140516144827.155fac84@jbarnes-desktop>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <intel-gfx-bounces@lists.freedesktop.org>
Received: from mail-ee0-f47.google.com (mail-ee0-f47.google.com [74.125.83.47])
 by gabe.freedesktop.org (Postfix) with ESMTP id 0B0C06E246
 for <intel-gfx@lists.freedesktop.org>; Fri, 16 May 2014 15:19:13 -0700 (PDT)
Received: by mail-ee0-f47.google.com with SMTP id c13so1899960eek.20
 for <intel-gfx@lists.freedesktop.org>; Fri, 16 May 2014 15:19:13 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <20140516144827.155fac84@jbarnes-desktop>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
 <mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
 <mailto:intel-gfx-request@lists.freedesktop.org?subject=subscribe>
Errors-To: intel-gfx-bounces@lists.freedesktop.org
Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>
To: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>, Intel Graphics Development <intel-gfx@lists.freedesktop.org>
List-Id: intel-gfx@lists.freedesktop.org

On Fri, May 16, 2014 at 02:48:27PM -0700, Jesse Barnes wrote:
> On Thu, 24 Apr 2014 23:55:42 +0200
> Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> 
> > +	if (enable) {
> > +		if (!intel_crtc->active) {
> > +			domains = get_crtc_power_domains(crtc);
> > +			for_each_power_domain(domain, domains)
> > +				intel_display_power_get(dev_priv, domain);
> > +			intel_crtc->enabled_power_domains = domains;
> > +
> > +			dev_priv->display.crtc_enable(crtc);
> > +		}
> > +	} else {
> > +		if (intel_crtc->active) {
> > +			dev_priv->display.crtc_disable(crtc);
> > +
> > +			domains = intel_crtc->enabled_power_domains;
> > +			for_each_power_domain(domain, domains)
> > +				intel_display_power_put(dev_priv, domain);
> > +			intel_crtc->enabled_power_domains = 0;
> > +		}
> > +	}
> 
> These branches could probably be cleaned up a bit.
> 
> But the power domain bits here got me thinking that maybe we can push
> them down into the crtc_enable/disable functions instead.  That would
> let us do the right thing per-platform and save us the
> "get_crtc_power_domains" call which may not make sense on all platforms.
> 
> I haven't thought it through for the other power wells, but that type
> of approach may make more sense than trying to abstract the wells at
> the high level we're doing today, especially since things are likely to
> get finer grained over time rather than coarser.

Had the same idea but then things get ugly since since the power domain
grabbing in the modeset sequence is a bit convoluted (for historical
reasons). So would require a bit of unwinding.

Also this gives us a much clearer bisect point imo.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch