| Message ID | 1393540010-1582-5-git-send-email-przanoni@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2014-02-27 at 19:26 -0300, Paulo Zanoni wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Since the addition of dev_priv->mm.busy, there's no more need for > dev_priv->pc8.gpu_idle, so kill it. > > 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. > > 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. > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: Imre Deak <imre.deak@intel.com> > --- > 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(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_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 prevent the > * system from reaching PC8+. But we don't have a symmetric way to do this for > - * everything, so we have the requirements_met and gpu_idle variables. When 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 true 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 in the > + * opposite case. The requirements_met variable is true when all the CRTCs, > + * encoders and the power well are disabled. > * > * In addition to everything, we only actually enable PC8+ if disable_count > * 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); > } > > -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 = 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 = false; > - __hsw_disable_package_c8(dev_priv); > - } > - mutex_unlock(&dev_priv->pc8.lock); > -} > - > #define for_each_power_domain(domain, mask) \ > for ((domain) = 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; > > - hsw_package_c8_gpu_busy(dev_priv); > + hsw_disable_package_c8(dev_priv); > i915_update_gfx_val(dev_priv); > dev_priv->mm.busy = true; > } > @@ -8224,7 +8198,7 @@ void intel_mark_idle(struct drm_device *dev) > gen6_rps_idle(dev->dev_private); > > out: > - hsw_package_c8_gpu_idle(dev_priv); > + hsw_enable_package_c8(dev_priv); > } > > 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) > > mutex_init(&dev_priv->pc8.lock); > dev_priv->pc8.requirements_met = false; > - dev_priv->pc8.gpu_idle = false; > dev_priv->pc8.irqs_disabled = false; > dev_priv->pc8.enabled = false; > - dev_priv->pc8.disable_count = 2; /* requirements_met + gpu_idle */ > + dev_priv->pc8.disable_count = 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);
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_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 prevent the * system from reaching PC8+. But we don't have a symmetric way to do this for - * everything, so we have the requirements_met and gpu_idle variables. When 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 true 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 in the + * opposite case. The requirements_met variable is true when all the CRTCs, + * encoders and the power well are disabled. * * In addition to everything, we only actually enable PC8+ if disable_count * 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); } -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 = 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 = false; - __hsw_disable_package_c8(dev_priv); - } - mutex_unlock(&dev_priv->pc8.lock); -} - #define for_each_power_domain(domain, mask) \ for ((domain) = 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; - hsw_package_c8_gpu_busy(dev_priv); + hsw_disable_package_c8(dev_priv); i915_update_gfx_val(dev_priv); dev_priv->mm.busy = true; } @@ -8224,7 +8198,7 @@ void intel_mark_idle(struct drm_device *dev) gen6_rps_idle(dev->dev_private); out: - hsw_package_c8_gpu_idle(dev_priv); + hsw_enable_package_c8(dev_priv); } 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) mutex_init(&dev_priv->pc8.lock); dev_priv->pc8.requirements_met = false; - dev_priv->pc8.gpu_idle = false; dev_priv->pc8.irqs_disabled = false; dev_priv->pc8.enabled = false; - dev_priv->pc8.disable_count = 2; /* requirements_met + gpu_idle */ + dev_priv->pc8.disable_count = 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);