| Message ID | 1393001548-2883-3-git-send-email-przanoni@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 21 Feb 2014 13:52:19 -0300 Paulo Zanoni <przanoni@gmail.com> wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Because intel_mark_idle still touches some registers: it needs the > machine to be awake. If you set both the autosuspend and PC8 delays to > zero, you can get a "Device suspended" WARN when gen6_rps_idle touches > registers. > > This is not easy to reproduce, but happens once in a while when > running pm_pc8. > > Testcase: igt/pm_pc8 > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index dd416f2..10ec401 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -8210,10 +8210,8 @@ void intel_mark_idle(struct drm_device *dev) > > dev_priv->mm.busy = false; > > - hsw_package_c8_gpu_idle(dev_priv); > - > if (!i915.powersave) > - return; > + goto out; > > list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { > if (!crtc->fb) > @@ -8224,6 +8222,9 @@ void intel_mark_idle(struct drm_device *dev) > > if (INTEL_INFO(dev)->gen >= 6) > gen6_rps_idle(dev->dev_private); > + > +out: > + hsw_package_c8_gpu_idle(dev_priv); > } > > void intel_mark_fb_busy(struct drm_i915_gem_object *obj, Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
On Fri, Feb 21, 2014 at 09:28:25AM -0800, Jesse Barnes wrote: > On Fri, 21 Feb 2014 13:52:19 -0300 > Paulo Zanoni <przanoni@gmail.com> wrote: > > > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > Because intel_mark_idle still touches some registers: it needs the > > machine to be awake. If you set both the autosuspend and PC8 delays to > > zero, you can get a "Device suspended" WARN when gen6_rps_idle touches > > registers. > > > > This is not easy to reproduce, but happens once in a while when > > running pm_pc8. > > > > Testcase: igt/pm_pc8 > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index dd416f2..10ec401 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -8210,10 +8210,8 @@ void intel_mark_idle(struct drm_device *dev) > > > > dev_priv->mm.busy = false; > > > > - hsw_package_c8_gpu_idle(dev_priv); > > - > > if (!i915.powersave) > > - return; > > + goto out; > > > > list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { > > if (!crtc->fb) > > @@ -8224,6 +8222,9 @@ void intel_mark_idle(struct drm_device *dev) > > > > if (INTEL_INFO(dev)->gen >= 6) > > gen6_rps_idle(dev->dev_private); > > + > > +out: > > + hsw_package_c8_gpu_idle(dev_priv); > > } > > > > void intel_mark_fb_busy(struct drm_i915_gem_object *obj, > > Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> Smells like another good reason for Jesse's idea with a register range based power domain checker. Especially with suspend delays we'll have a really hard time catching these kinds of bugs otherwise ... -Daniel
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index dd416f2..10ec401 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8210,10 +8210,8 @@ void intel_mark_idle(struct drm_device *dev) dev_priv->mm.busy = false; - hsw_package_c8_gpu_idle(dev_priv); - if (!i915.powersave) - return; + goto out; list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { if (!crtc->fb) @@ -8224,6 +8222,9 @@ void intel_mark_idle(struct drm_device *dev) if (INTEL_INFO(dev)->gen >= 6) gen6_rps_idle(dev->dev_private); + +out: + hsw_package_c8_gpu_idle(dev_priv); } void intel_mark_fb_busy(struct drm_i915_gem_object *obj,