| Message ID | 1403049653-31480-1-git-send-email-deepak.s@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 18, 2014 at 05:30:53AM +0530, deepak.s@linux.intel.com wrote: > From: Deepak S <deepak.s@linux.intel.com> > > We might be leaving the GPU Frequency (and thus vnn) high during the suspend. > Flush the delayed work queue should take care of this. > > v2: Fixed typo in commit message (Deepak) > > Signed-off-by: Deepak S <deepak.s@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_gem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 7f643db..8d5ae82 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4541,7 +4541,7 @@ i915_gem_suspend(struct drm_device *dev) > > del_timer_sync(&dev_priv->gpu_error.hangcheck_timer); > cancel_delayed_work_sync(&dev_priv->mm.retire_work); > - cancel_delayed_work_sync(&dev_priv->mm.idle_work); > + flush_delayed_work(&dev_priv->mm.idle_work); Shouldn't we do that in suspend_gt_powersave instead? Also if we cancel the retire work the idle work won't necessarily get armed and we might miss the window. Just forcing the gt to the lowest freq in suspend_gt_powersave should be more reliable. -Daniel
On Thu, Jun 19, 2014 at 08:53:24AM +0530, Deepak S wrote: > > On Wednesday 18 June 2014 03:47 AM, Daniel Vetter wrote: > >On Wed, Jun 18, 2014 at 05:30:53AM +0530, deepak.s@linux.intel.com wrote: > >>From: Deepak S <deepak.s@linux.intel.com> > >> > >>We might be leaving the GPU Frequency (and thus vnn) high during the suspend. > >>Flush the delayed work queue should take care of this. > >> > >>v2: Fixed typo in commit message (Deepak) > >> > >>Signed-off-by: Deepak S <deepak.s@linux.intel.com> > >>--- > >> drivers/gpu/drm/i915/i915_gem.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >>diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > >>index 7f643db..8d5ae82 100644 > >>--- a/drivers/gpu/drm/i915/i915_gem.c > >>+++ b/drivers/gpu/drm/i915/i915_gem.c > >>@@ -4541,7 +4541,7 @@ i915_gem_suspend(struct drm_device *dev) > >> del_timer_sync(&dev_priv->gpu_error.hangcheck_timer); > >> cancel_delayed_work_sync(&dev_priv->mm.retire_work); > >>- cancel_delayed_work_sync(&dev_priv->mm.idle_work); > >>+ flush_delayed_work(&dev_priv->mm.idle_work); > >Shouldn't we do that in suspend_gt_powersave instead? Also if we cancel > >the retire work the idle work won't necessarily get armed and we might > >miss the window. Just forcing the gt to the lowest freq in > >suspend_gt_powersave should be more reliable. > >-Daniel > > Since we a calling suspend_gt_powersave after i915_gem_suspend, i added the flush in suspend. I'm confused: So you're doing the change I've suggested or it doesn't work? Please unconfuse me ;-) > Yes i agree forcing the gt freq us more reliable So new patch or not? -Daniel
On Wednesday 18 June 2014 03:47 AM, Daniel Vetter wrote: > On Wed, Jun 18, 2014 at 05:30:53AM +0530, deepak.s@linux.intel.com wrote: >> From: Deepak S <deepak.s@linux.intel.com> >> >> We might be leaving the GPU Frequency (and thus vnn) high during the suspend. >> Flush the delayed work queue should take care of this. >> >> v2: Fixed typo in commit message (Deepak) >> >> Signed-off-by: Deepak S <deepak.s@linux.intel.com> >> --- >> drivers/gpu/drm/i915/i915_gem.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c >> index 7f643db..8d5ae82 100644 >> --- a/drivers/gpu/drm/i915/i915_gem.c >> +++ b/drivers/gpu/drm/i915/i915_gem.c >> @@ -4541,7 +4541,7 @@ i915_gem_suspend(struct drm_device *dev) >> >> del_timer_sync(&dev_priv->gpu_error.hangcheck_timer); >> cancel_delayed_work_sync(&dev_priv->mm.retire_work); >> - cancel_delayed_work_sync(&dev_priv->mm.idle_work); >> + flush_delayed_work(&dev_priv->mm.idle_work); > Shouldn't we do that in suspend_gt_powersave instead? Also if we cancel > the retire work the idle work won't necessarily get armed and we might > miss the window. Just forcing the gt to the lowest freq in > suspend_gt_powersave should be more reliable. > -Daniel Since we a calling suspend_gt_powersave after i915_gem_suspend, i added the flush in suspend. Yes i agree forcing the gt freq us more reliable
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 7f643db..8d5ae82 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4541,7 +4541,7 @@ i915_gem_suspend(struct drm_device *dev) del_timer_sync(&dev_priv->gpu_error.hangcheck_timer); cancel_delayed_work_sync(&dev_priv->mm.retire_work); - cancel_delayed_work_sync(&dev_priv->mm.idle_work); + flush_delayed_work(&dev_priv->mm.idle_work); return 0;