From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH 2/4] drm/i915: leave rc6 enabled at suspend time Date: Fri, 30 May 2014 15:54:37 +0300 Message-ID: <1401454477.24060.32.camel@intelbox> References: <1401397897-4655-1-git-send-email-jbarnes@virtuousgeek.org> <1401397897-4655-2-git-send-email-jbarnes@virtuousgeek.org> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0901047286==" Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id E246F6E4B3 for ; Fri, 30 May 2014 05:54:40 -0700 (PDT) In-Reply-To: <1401397897-4655-2-git-send-email-jbarnes@virtuousgeek.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Jesse Barnes Cc: intel-gfx@lists.freedesktop.org, kristen@linux.intel.com List-Id: intel-gfx@lists.freedesktop.org --===============0901047286== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-dB31ln+75e41nlXN9Nfl" --=-dB31ln+75e41nlXN9Nfl Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 2014-05-29 at 14:11 -0700, Jesse Barnes wrote: > From: Kristen Carlson Accardi >=20 > This allows the system to enter the lowest power mode during system freez= e. >=20 > Signed-off-by: Jesse Barnes > --- > drivers/gpu/drm/i915/i915_drv.c | 3 --- > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_pm.c | 16 +++++++++++----- > 3 files changed, 12 insertions(+), 8 deletions(-) >=20 > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_= drv.c > index 66c6ffb..433bdfa 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -521,8 +521,6 @@ static int i915_drm_freeze(struct drm_device *dev) > drm_irq_uninstall(dev); > dev_priv->enable_hotplug_processing =3D false; > =20 > - intel_disable_gt_powersave(dev); > - I wonder what was the reason for this call. One possibility is that i915_save_state() depends on it to save the correct registers, but it would be good to clarify this. It also cancels some deferred works which we do need here. But we could also add that to intel_enable_gt_powersave_sync() in this patch. > /* > * Disable CRTCs directly since we want to preserve sw state > * for _thaw. > @@ -543,7 +541,6 @@ static int i915_drm_freeze(struct drm_device *dev) > i915_save_state(dev); > =20 > intel_opregion_fini(dev); > - intel_uncore_fini(dev); Some stuff in the above call is unrelated to this patch, like intel_uncore_forcewake_reset(). At least canceling force_wake_timer there seems to be needed here. In any case it would be better to have the above two changes in a separate patch. With that fixed this patch looks ok to me. The original patch was from me, so fwiw: Reviewed-by: Imre Deak =20 > console_lock(); > intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED); > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/inte= l_drv.h > index c597b0d..bf90e7d 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -956,6 +956,7 @@ void intel_power_domains_init_hw(struct drm_i915_priv= ate *dev_priv); > void intel_init_gt_powersave(struct drm_device *dev); > void intel_cleanup_gt_powersave(struct drm_device *dev); > void intel_enable_gt_powersave(struct drm_device *dev); > +void intel_enable_gt_powersave_sync(struct drm_device *dev); > void intel_disable_gt_powersave(struct drm_device *dev); > void intel_reset_gt_powersave(struct drm_device *dev); > void ironlake_teardown_rc6(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel= _pm.c > index 1840d15..8d9e036 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4891,12 +4891,9 @@ void intel_disable_gt_powersave(struct drm_device = *dev) > } > } > =20 > -static void intel_gen6_powersave_work(struct work_struct *work) > +void intel_enable_gt_powersave_sync(struct drm_device *dev) > { > - struct drm_i915_private *dev_priv =3D > - container_of(work, struct drm_i915_private, > - rps.delayed_resume_work.work); > - struct drm_device *dev =3D dev_priv->dev; > + struct drm_i915_private *dev_priv =3D dev->dev_private; > =20 > mutex_lock(&dev_priv->rps.hw_lock); > =20 > @@ -4917,6 +4914,15 @@ static void intel_gen6_powersave_work(struct work_= struct *work) > intel_runtime_pm_put(dev_priv); > } > =20 > +static void intel_gen6_powersave_work(struct work_struct *work) > +{ > + struct drm_i915_private *dev_priv =3D > + container_of(work, struct drm_i915_private, > + rps.delayed_resume_work.work); > + > + intel_enable_gt_powersave_sync(dev_priv->dev); > +} > + > void intel_enable_gt_powersave(struct drm_device *dev) > { > struct drm_i915_private *dev_priv =3D dev->dev_private; --=-dB31ln+75e41nlXN9Nfl 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) iQEcBAABAgAGBQJTiH+NAAoJEORIIAnNuWDFA1YH/2d3UtjL1uRPgjyC2mZ8R9vF e5m1Z8UPc6GiSgSu4v+0F2aGQUkBhhI4vvSXnt76dAZ87eCKO9juH5NQ8i8F5/U5 79Kc3LsKTCfSvfi8rVV4saBBo0Xj+1VEz8jzdYeoUmwXBitFbieP7geeir0y9vgi BJ6s54TyoygozTprXaE6FVryDEgsA+juNcy9uBy65sA5nviJnZAu0660s9wd9kdj NdTlUU7k3RAgoTqADQgCBsBZWKFVXU/T0RTUVL42MtM9hvDRUOsTEapS3xDvgRaq g1mRZVboP0H95VWM4n9dLKiHIiU3ZFPQluhx6sok/RvM0ZFPTAx7LOPYD0I7ky0= =KCfg -----END PGP SIGNATURE----- --=-dB31ln+75e41nlXN9Nfl-- --===============0901047286== 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 --===============0901047286==--