diff mbox

[4/4] drm/i915: Cleanup instdone state when idle

Message ID 1344922871-2248-4-git-send-email-ben@bwidawsk.net (mailing list archive)
State Superseded
Headers show

Commit Message

Ben Widawsky Aug. 14, 2012, 5:41 a.m. UTC 
The previous state is bogus when we've gone into idle. Actually there
would be a known state of idle (ie. all units are done), but since it
differs for every platform, we can just set 0, and let the hangcheck
progress as normal.

References: https://bugs.freedesktop.org/show_bug.cgi?id=52429
Tested-by: Guang A Yang <guang.a.yang@intel.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_gem.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Chris Wilson Aug. 14, 2012, 7:39 a.m. UTC | #1 
On Mon, 13 Aug 2012 22:41:11 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> The previous state is bogus when we've gone into idle. Actually there
> would be a known state of idle (ie. all units are done), but since it
> differs for every platform, we can just set 0, and let the hangcheck
> progress as normal.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=52429
> Tested-by: Guang A Yang <guang.a.yang@intel.com>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

I think you have a point, but I don't think this should be just on idle.
Everytime we kick off the hangcheck, we should not be comparing to stale
state.
-Chris
Ben Widawsky Aug. 14, 2012, 4:42 p.m. UTC | #2 
On 2012-08-14 00:39, Chris Wilson wrote:
> On Mon, 13 Aug 2012 22:41:11 -0700, Ben Widawsky <ben@bwidawsk.net> 
> wrote:
>> The previous state is bogus when we've gone into idle. Actually 
>> there
>> would be a known state of idle (ie. all units are done), but since 
>> it
>> differs for every platform, we can just set 0, and let the hangcheck
>> progress as normal.
>>
>> References: https://bugs.freedesktop.org/show_bug.cgi?id=52429
>> Tested-by: Guang A Yang <guang.a.yang@intel.com>
>> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
>
> I think you have a point, but I don't think this should be just on 
> idle.
> Everytime we kick off the hangcheck, we should not be comparing to 
> stale
> state.
> -Chris

Are you suggesting this is a patch which already exists somewhere?
Chris Wilson Aug. 14, 2012, 4:48 p.m. UTC | #3 
On Tue, 14 Aug 2012 09:42:07 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> On 2012-08-14 00:39, Chris Wilson wrote:
> > On Mon, 13 Aug 2012 22:41:11 -0700, Ben Widawsky <ben@bwidawsk.net> 
> > wrote:
> >> The previous state is bogus when we've gone into idle. Actually 
> >> there
> >> would be a known state of idle (ie. all units are done), but since 
> >> it
> >> differs for every platform, we can just set 0, and let the hangcheck
> >> progress as normal.
> >>
> >> References: https://bugs.freedesktop.org/show_bug.cgi?id=52429
> >> Tested-by: Guang A Yang <guang.a.yang@intel.com>
> >> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> >
> > I think you have a point, but I don't think this should be just on 
> > idle.
> > Everytime we kick off the hangcheck, we should not be comparing to 
> > stale
> > state.
> 
> Are you suggesting this is a patch which already exists somewhere?

Not yet. :) Just that I agree with you that this a problem, and it is a
bigger problem than first thought.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 31054fa..5f116a1 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3624,6 +3624,9 @@  i915_gem_idle(struct drm_device *dev)
 	/* Cancel the retire work handler, which should be idle now. */
 	cancel_delayed_work_sync(&dev_priv->mm.retire_work);
 
+	dev_priv->last_instdone = 0;
+	dev_priv->last_instdone1 = 0;
+
 	return 0;
 }