From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH 5/6] drm/i915: enable only the needed power domains during modeset Date: Fri, 18 Oct 2013 11:53:08 -0700 Message-ID: <20131018115308.6a2b3da2@jbarnes-desktop> References: <1381933553-19529-1-git-send-email-imre.deak@intel.com> <1381933553-19529-6-git-send-email-imre.deak@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from qproxy1-pub.mail.unifiedlayer.com (qproxy1-pub.mail.unifiedlayer.com [173.254.64.10]) by gabe.freedesktop.org (Postfix) with SMTP id 1E97DE74AE for ; Fri, 18 Oct 2013 11:52:52 -0700 (PDT) In-Reply-To: <1381933553-19529-6-git-send-email-imre.deak@intel.com> 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: Imre Deak Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, 16 Oct 2013 17:25:52 +0300 Imre Deak wrote: > So far the modeset code enabled all power domains if it needed any. It > wasn't a problem since HW generations so far only had one always-on > power well and one dynamic power well that can be enabled/disabled. For > domains powered by always-on power wells (panel fitter on pipe A and the > eDP transcoder) we didn't do anything, for all other domains we just > enabled the single dynamic power well. > > Future HW generations will change this, as they add multiple dynamic > power wells. Support for these will be added later, this patch prepares > for those by making sure we only enable the required domains. > > Note that after this change on HSW we'll enable all power domains even > if it was the domain for the panel fitter on pipe A or the eDP > transcoder. This isn't a problem since the power domain framework > already checks if the domain is on an always-on power well and doesn't > do anything in this case. > > Signed-off-by: Imre Deak > --- > drivers/gpu/drm/i915/intel_display.c | 46 ++++++++++++++++++++++++++++++++---- > drivers/gpu/drm/i915/intel_drv.h | 1 + > 2 files changed, 42 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 8e734f2..e2a4f3b 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -6561,21 +6561,57 @@ static void hsw_package_c8_gpu_busy(struct drm_i915_private *dev_priv) > } > } > > +#define for_each_power_domain(domain, mask) \ > + for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++) \ > + if ((1 << (domain)) & (mask)) > + > +static unsigned long get_pipe_power_domains(struct drm_device *dev, > + enum pipe pipe, bool pfit_enabled) > +{ > + unsigned long mask; > + enum transcoder transcoder; > + > + transcoder = intel_pipe_to_cpu_transcoder(dev->dev_private, pipe); > + > + mask = BIT(POWER_DOMAIN_PIPE(pipe)); > + mask |= BIT(POWER_DOMAIN_TRANSCODER(transcoder)); > + if (pfit_enabled) > + mask |= BIT(POWER_DOMAIN_PIPE_PANEL_FITTER(pipe)); > + > + return mask; > +} > + > static void modeset_update_power_wells(struct drm_device *dev) > { > - bool enable = false; > + unsigned long pipe_domains[I915_MAX_PIPES] = { 0, }; > struct intel_crtc *crtc; > > + /* > + * First get all needed power domains, then put all unneeded, to avoid > + * any unnecessary toggling of the power wells. > + */ > list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) { > + enum intel_display_power_domain domain; > + > if (!crtc->base.enabled) > continue; > > - if (crtc->pipe != PIPE_A || crtc->config.pch_pfit.enabled || > - crtc->config.cpu_transcoder != TRANSCODER_EDP) > - enable = true; > + pipe_domains[crtc->pipe] = get_pipe_power_domains(dev, > + crtc->pipe, > + crtc->config.pch_pfit.enabled); > + > + for_each_power_domain(domain, pipe_domains[crtc->pipe]) > + intel_display_power_get(dev, domain); > } > > - intel_set_power_well(dev, enable); > + list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) { > + enum intel_display_power_domain domain; > + > + for_each_power_domain(domain, crtc->enabled_power_domains) > + intel_display_power_put(dev, domain); > + > + crtc->enabled_power_domains = pipe_domains[crtc->pipe]; > + } > } > > static void haswell_modeset_global_resources(struct drm_device *dev) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 189257d..63a5bfd 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -320,6 +320,7 @@ struct intel_crtc { > * some outputs connected to this crtc. > */ > bool active; > + unsigned long enabled_power_domains; > bool eld_vld; > bool primary_enabled; /* is the primary plane (partially) visible? */ > bool lowfreq_avail; Reviewed-by: Jesse Barnes Hope we can get rid of the set_power_well() function soon... -- Jesse Barnes, Intel Open Source Technology Center