From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH 04/23] drm/i915: kill dev_priv->pc8.gpu_idle Date: Fri, 28 Feb 2014 17:23:38 +0200 Message-ID: <1393601018.3479.65.camel@intelbox> References: <1393540010-1582-1-git-send-email-przanoni@gmail.com> <1393540010-1582-5-git-send-email-przanoni@gmail.com> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0685244952==" Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id 94E20FBA1F for ; Fri, 28 Feb 2014 07:23:41 -0800 (PST) In-Reply-To: <1393540010-1582-5-git-send-email-przanoni@gmail.com> 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: Paulo Zanoni Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org --===============0685244952== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-kwEwnkx7Avw0U9OezSBn" --=-kwEwnkx7Avw0U9OezSBn Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 2014-02-27 at 19:26 -0300, Paulo Zanoni wrote: > From: Paulo Zanoni >=20 > Since the addition of dev_priv->mm.busy, there's no more need for > dev_priv->pc8.gpu_idle, so kill it. >=20 > Notice that when you remove gpu_idle, hsw_package_c8_gpu_idle and > hsw_package_c8_gpu_busy become identical to hsw_enable_package_c8 and > hsw_disable_package_c8, so just use them. >=20 > Also, when we boot the machine, dev_priv->mm.busy initially considers > the machine as idle. This is opposed to dev_priv->pc8.gpu_idle, which > considered it busy. So dev_priv->pc8.disable_count has to be > initalized to 1 now. >=20 > Signed-off-by: Paulo Zanoni Reviewed-by: Imre Deak > --- > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 10 ++++------ > drivers/gpu/drm/i915/intel_display.c | 30 ++---------------------------- > drivers/gpu/drm/i915/intel_pm.c | 3 +-- > 4 files changed, 8 insertions(+), 37 deletions(-) >=20 > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i= 915_debugfs.c > index d90a707..72cc6d0 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -1997,7 +1997,7 @@ static int i915_pc8_status(struct seq_file *m, void= *unused) > mutex_lock(&dev_priv->pc8.lock); > seq_printf(m, "Requirements met: %s\n", > yesno(dev_priv->pc8.requirements_met)); > - seq_printf(m, "GPU idle: %s\n", yesno(dev_priv->pc8.gpu_idle)); > + seq_printf(m, "GPU idle: %s\n", yesno(!dev_priv->mm.busy)); > seq_printf(m, "Disable count: %d\n", dev_priv->pc8.disable_count); > seq_printf(m, "IRQs disabled: %s\n", > yesno(dev_priv->pc8.irqs_disabled)); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_= drv.h > index a5caa7e..2a2a3a9 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1321,11 +1321,10 @@ struct ilk_wm_values { > * Ideally every piece of our code that needs PC8+ disabled would call > * hsw_disable_package_c8, which would increment disable_count and preve= nt the > * system from reaching PC8+. But we don't have a symmetric way to do th= is for > - * everything, so we have the requirements_met and gpu_idle variables. W= hen we > - * switch requirements_met or gpu_idle to true we decrease disable_count= , and > - * increase it in the opposite case. The requirements_met variable is tr= ue when > - * all the CRTCs, encoders and the power well are disabled. The gpu_idle > - * variable is true when the GPU is idle. > + * everything, so we have the requirements_met variable. When we switch > + * requirements_met to true we decrease disable_count, and increase it i= n the > + * opposite case. The requirements_met variable is true when all the CRT= Cs, > + * encoders and the power well are disabled. > * > * In addition to everything, we only actually enable PC8+ if disable_co= unt > * stays at zero for at least some seconds. This is implemented with the > @@ -1348,7 +1347,6 @@ struct ilk_wm_values { > */ > struct i915_package_c8 { > bool requirements_met; > - bool gpu_idle; > bool irqs_disabled; > /* Only true after the delayed work task actually enables it. */ > bool enabled; > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/= intel_display.c > index 10ec401..0183a34 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -6812,32 +6812,6 @@ done: > mutex_unlock(&dev_priv->pc8.lock); > } > =20 > -static void hsw_package_c8_gpu_idle(struct drm_i915_private *dev_priv) > -{ > - if (!HAS_PC8(dev_priv->dev)) > - return; > - > - mutex_lock(&dev_priv->pc8.lock); > - if (!dev_priv->pc8.gpu_idle) { > - dev_priv->pc8.gpu_idle =3D true; > - __hsw_enable_package_c8(dev_priv); > - } > - mutex_unlock(&dev_priv->pc8.lock); > -} > - > -static void hsw_package_c8_gpu_busy(struct drm_i915_private *dev_priv) > -{ > - if (!HAS_PC8(dev_priv->dev)) > - return; > - > - mutex_lock(&dev_priv->pc8.lock); > - if (dev_priv->pc8.gpu_idle) { > - dev_priv->pc8.gpu_idle =3D false; > - __hsw_disable_package_c8(dev_priv); > - } > - mutex_unlock(&dev_priv->pc8.lock); > -} > - > #define for_each_power_domain(domain, mask) \ > for ((domain) =3D 0; (domain) < POWER_DOMAIN_NUM; (domain)++) \ > if ((1 << (domain)) & (mask)) > @@ -8195,7 +8169,7 @@ void intel_mark_busy(struct drm_device *dev) > if (dev_priv->mm.busy) > return; > =20 > - hsw_package_c8_gpu_busy(dev_priv); > + hsw_disable_package_c8(dev_priv); > i915_update_gfx_val(dev_priv); > dev_priv->mm.busy =3D true; > } > @@ -8224,7 +8198,7 @@ void intel_mark_idle(struct drm_device *dev) > gen6_rps_idle(dev->dev_private); > =20 > out: > - hsw_package_c8_gpu_idle(dev_priv); > + hsw_enable_package_c8(dev_priv); > } > =20 > void intel_mark_fb_busy(struct drm_i915_gem_object *obj, > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel= _pm.c > index a6b877a..50b80bb 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -5786,10 +5786,9 @@ void intel_pm_setup(struct drm_device *dev) > =20 > mutex_init(&dev_priv->pc8.lock); > dev_priv->pc8.requirements_met =3D false; > - dev_priv->pc8.gpu_idle =3D false; > dev_priv->pc8.irqs_disabled =3D false; > dev_priv->pc8.enabled =3D false; > - dev_priv->pc8.disable_count =3D 2; /* requirements_met + gpu_idle */ > + dev_priv->pc8.disable_count =3D 1; /* requirements_met */ > INIT_DELAYED_WORK(&dev_priv->pc8.enable_work, hsw_enable_pc8_work); > INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work, > intel_gen6_powersave_work); --=-kwEwnkx7Avw0U9OezSBn 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) iQEcBAABAgAGBQJTEKn6AAoJEORIIAnNuWDFctEIAN+JTCwteWmPKS8vADQWGFiZ Xy0qyqzJYAlDBbnZu9/UPVfCafCA6W25UuZMidszO2XgaKD4wL3FUbjTwDuWMKvJ osaF19FpQlqtKfElQG1i7t7LQPZl9J8DpJTvNcZVIednEQLjxxBaPdR2jWcpF27J jzKkOu7E5XnO19YD/tzeQGpN2KdgiipTq3PuOMqgaetZU0nHtBj3G26qveqEFoHH z9C6W4rBRBKpoPORh21GKPXsEKZoTNeyeOfFxALlAQJTwTYa8NCrB+CzdeFlVity fO/lf3guu9XNItz7QRaNkg/KJMr3LsMDPZio+ZJuTEe3abRmExXHK3YVzCeYTzg= =g9MM -----END PGP SIGNATURE----- --=-kwEwnkx7Avw0U9OezSBn-- --===============0685244952== 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 --===============0685244952==--