From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH 07/11] drm/i915: kill dev_priv->pc8.gpu_idle Date: Fri, 28 Feb 2014 15:50:01 +0200 Message-ID: <1393595401.3479.50.camel@intelbox> References: <1393001548-2883-1-git-send-email-przanoni@gmail.com> <1393001548-2883-8-git-send-email-przanoni@gmail.com> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1572536762==" Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id 187E0FB545 for ; Fri, 28 Feb 2014 05:50:04 -0800 (PST) In-Reply-To: <1393001548-2883-8-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 --===============1572536762== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-GpNKCSzSvbmOUlbxw1vS" --=-GpNKCSzSvbmOUlbxw1vS Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2014-02-21 at 13:52 -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 > --- > 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 34e347f..62d0c0915 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -2014,7 +2014,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 c64fb7f..796a116 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 */ This looks ok, but it's part of "Merge PC8 with runtime PM, v2" along with patch 1/11, so they can be skipped from this patchset. --Imre > 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); --=-GpNKCSzSvbmOUlbxw1vS 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) iQEcBAABAgAGBQJTEJQJAAoJEORIIAnNuWDFaRcH/025yZyBPgeQPP9FYjOnkIDb DVikElh/3oy9aUa+IrG2ZMZiQAYR5V+vyJ9jtQn0aAmzF8Hy4jF9Sa3yC2THbXhr Agzt4c8dDiIBcNqHbZdMXgyW0dLiGUG3XrhsGWP4yyn/TYJIDvJicrhZQSBbor7q REv1xm/vp5AgvzjytbG6j/Yttds5nIZHN1td6EQj7H3SqNpea471xFJ7UtvaooKy vwjLglvWtfrTOPGqWBHosHOhc8+/oo17W34iQ3JR6Nt3izaI+StHnhUYjSVAy7DT 0OVTvixUMM8M3HlBYuvsIKwAMLjCo4nTbamRGI3x1mozybCmT5ptTXgNuB4iifE= =DYlN -----END PGP SIGNATURE----- --=-GpNKCSzSvbmOUlbxw1vS-- --===============1572536762== 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 --===============1572536762==--