From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH 07/15] drm/i915: vlv: check port power domain instead of only D0 for eDP VDD on Date: Wed, 09 Apr 2014 17:50:05 +0300 Message-ID: <1397055005.2552.23.camel@ideak-mobl> References: <1396976276-32714-1-git-send-email-imre.deak@intel.com> <1396976276-32714-8-git-send-email-imre.deak@intel.com> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id CF81E6EB8A for ; Wed, 9 Apr 2014 07:52:43 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Paulo Zanoni Cc: Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org On Wed, 2014-04-09 at 11:32 -0300, Paulo Zanoni wrote: > 2014-04-08 13:57 GMT-03:00 Imre Deak : > > Some platforms need additional power domains to be on in addition to the > > device D0 state to access the panel registers. > > > > Suggested by Daniel. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76987 > > Signed-off-by: Imre Deak > > --- > > drivers/gpu/drm/i915/intel_dp.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index e48d47c..c7a52d1 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -313,8 +313,12 @@ static bool edp_have_panel_vdd(struct intel_dp *intel_dp) > > { > > struct drm_device *dev = intel_dp_to_dev(intel_dp); > > struct drm_i915_private *dev_priv = dev->dev_private; > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); > > + struct intel_encoder *intel_encoder = &intel_dig_port->base; > > + enum intel_display_power_domain power_domain; > > > > - return !dev_priv->pm.suspended && > > + power_domain = intel_display_port_power_domain(intel_encoder); > > + return intel_display_power_enabled(dev_priv, power_domain) && > > (I915_READ(_pp_ctrl_reg(intel_dp)) & EDP_FORCE_VDD) != 0; > > This is an interesting problem. The patch looks like the correct fix > for VLV, but it creates the problem that it will enable unneeded power > wells on HSW/BDW. I guess you mean port A here. In that case power_domain is POWER_DOMAIN_PORT_DDI_A_4_LANES, which will map both for HSW and BDW to the always-on power well, hence we only get an RPM ref for them? > My crystal ball tells me we're gonna have many > instances of this type of "problem" in the future. In this case, > shouldn't we create a new power domain, e.g., POWER_DOMAIN_EDP_VDD, > that grabs the appropriate power wells on VLV, but just gets runtime > PM on HSW+? Yea, well it's always a possibility that the current granularity is not enough for a new platform and then we have no choice than to extend the set of power domains. This is what happened now for the AUX stuff on SKL, we need to add the new AUX domains. We can also decide that the power saving doesn't justify the complexity and just grab the coarser domain for a given functionality. --Imre > > > } > > > > -- > > 1.8.4 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > >