From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH 19/19] drm/i915: power domains: add vlv power wells Date: Wed, 26 Feb 2014 20:02:19 +0200 Message-ID: <1393437739.29921.10.camel@intelbox> References: <1392674540-10915-1-git-send-email-imre.deak@intel.com> <1392674540-10915-20-git-send-email-imre.deak@intel.com> <20140219122944.GC3852@intel.com> <20140220115832.54d27680@jbarnes-desktop> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0374970923==" Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id EB511FA6CB for ; Wed, 26 Feb 2014 10:03:55 -0800 (PST) In-Reply-To: <20140220115832.54d27680@jbarnes-desktop> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Jesse Barnes Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============0374970923== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-N5OEsfIxusDQ97iXq/dp" --=-N5OEsfIxusDQ97iXq/dp Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 2014-02-20 at 11:58 -0800, Jesse Barnes wrote: > On Wed, 19 Feb 2014 14:29:44 +0200 > Ville Syrj=C3=A4l=C3=A4 wrote: >=20 > > On Tue, Feb 18, 2014 at 12:02:20AM +0200, Imre Deak wrote: > > > Based on an early draft from Jesse. > > >=20 > > > Add support for powering on/off the dynamic power wells on VLV by > > > registering its display and dpio dynamic power wells with the power > > > domain framework. > > >=20 > > > For now power on all PHY TX lanes regardless of the actual lane > > > configuration. Later this can be optimized when the PHY side setup > > > enables only the required lanes. Atm, it enables all lanes in all > > > cases. > > >=20 > > > Signed-off-by: Imre Deak > > > --- > > > drivers/gpu/drm/i915/i915_dma.c | 1 - > > > drivers/gpu/drm/i915/i915_drv.h | 2 +- > > > drivers/gpu/drm/i915/intel_display.c | 1 + > > > drivers/gpu/drm/i915/intel_pm.c | 228 +++++++++++++++++++++++++= ++++++++++ > > > 4 files changed, 230 insertions(+), 2 deletions(-) > > >=20 > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i= 915_dma.c > > > index dca4dc3..f8f7a59 100644 > > > --- a/drivers/gpu/drm/i915/i915_dma.c > > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > > @@ -1668,7 +1668,6 @@ int i915_driver_load(struct drm_device *dev, un= signed long flags) > > > goto out_mtrrfree; > > > } > > > =20 > > > - dev_priv->display_irqs_enabled =3D true; > > > intel_irq_init(dev); > > > intel_uncore_sanitize(dev); > > > =20 > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i= 915_drv.h > > > index 227c349..804334e 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -1053,7 +1053,7 @@ struct i915_power_well { > > > /* power well enable/disable usage count */ > > > int count; > > > unsigned long domains; > > > - void *data; > > > + unsigned long data; > > > const struct i915_power_well_ops *ops; > > > }; > > > =20 > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i= 915/intel_display.c > > > index ea00878..d6661c4 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -4224,6 +4224,7 @@ static void valleyview_modeset_global_resources= (struct drm_device *dev) > > > =20 > > > if (req_cdclk !=3D cur_cdclk) > > > valleyview_set_cdclk(dev, req_cdclk); > > > + modeset_update_power_wells(dev); > > > } > > > =20 > > > static void valleyview_crtc_enable(struct drm_crtc *crtc) > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/i= ntel_pm.c > > > index 68f58e5..e4416a7 100644 > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > @@ -5344,6 +5344,133 @@ static void hsw_power_well_disable(struct drm= _i915_private *dev_priv, > > > hsw_enable_package_c8(dev_priv); > > > } > > > =20 > > > +static void vlv_set_power_well(struct drm_i915_private *dev_priv, > > > + struct i915_power_well *power_well, bool enable) > > > +{ > > > + enum punit_power_well power_well_id =3D power_well->data; > > > + u32 mask; > > > + u32 state; > > > + u32 ctrl; > > > + > > > + mask =3D PUNIT_PWRGT_MASK(power_well_id); > > > + state =3D enable ? PUNIT_PWRGT_PWR_ON(power_well_id) : > > > + PUNIT_PWRGT_PWR_GATE(power_well_id); > > > + > > > + mutex_lock(&dev_priv->rps.hw_lock); > > > + > > > +#define COND \ > > > + ((vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_STATUS) & mask) =3D=3D s= tate) > > > + > > > + if (COND) > > > + goto out; > > > + > > > + ctrl =3D vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_CTRL); > > > + ctrl &=3D ~mask; > > > + ctrl |=3D state; > > > + vlv_punit_write(dev_priv, PUNIT_REG_PWRGT_CTRL, ctrl); > > > + > > > + if (wait_for(COND, 100)) > > > + DRM_ERROR("timout setting power well state %08x (%08x)\n", > > > + state, > > > + vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_CTRL)); > >=20 > > #undef COND somewhere to avoid suprises further down in the code? > >=20 > > > + > > > +out: > > > + mutex_unlock(&dev_priv->rps.hw_lock); > > > +} > > > + > > > >=20 >=20 > I'd like to see the code for re-enabling the display state land > eventually too, so we can get savings when userspace uses DPMS instead > of NULL mode sets for things. But to do that nicely requires some more > work in the mode set code to pull out more PLL bits (also needed for > atomic mode setting). I guess you meant here the drm connector->funcs->dpms() callback, that's called when setting the connector's dpms property. But I'm not sure what you meant by re-enabling the display state. I was thinking that we need to get/put the power domains around setting DPMS off/on via the above property, but we actually don't need that. Internally the dpms callback just calls the display.disable_crtc/enable_crtc() which will disable/enable the pipes but won't do anything with the power domains (which is only updated during a normal modeset through display.modeset_global_resources(). This isn't optimal power-wise, but it's a separate issue. I think Daniel had the idea of converting the dpms callback to be a proper NULL modeset. In that case too the power domains will be get/put correctly. --Imre --=-N5OEsfIxusDQ97iXq/dp Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.14 (GNU/Linux) iQEcBAABAgAGBQJTDiwrAAoJEORIIAnNuWDF3y8IAKT1tVsCmdOUHjWV+91DIALH JBO1tkd4eQ5cWKKIcZXQxh3BxWjJ++jH+3275ex8WFcR62Wp3SSPhob0V+yimq9j M+bpyhiXCzcEzFLcvFccztAyEaztJ+3RJ8j9qsTyE62NgINvizixIG8LCLSeHz5Y znde2r2loIY5GcdLuf+wSSgjfhVo+vMSXKDhgyJ07+6/aWEFy2YMgkX5Y9k0wXGN MsKMTE2o/JpWadpFVjNNW5216hqa769vMJ9HhfCluqaVUIX1gvq/yGDd24Rq7rvl q8xQYBtyfFVgz1SLnIjQSimKRMYG+da7oefqQYQXvusR+KY3vCYrhRtkBKejxX4= =STRH -----END PGP SIGNATURE----- --=-N5OEsfIxusDQ97iXq/dp-- --===============0374970923== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============0374970923==--