All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ben Widawsky <ben@bwidawsk.net>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 0/4] [RFC] use HW watchdog timer
Date: Tue, 17 Jul 2012 11:51:18 -0700	[thread overview]
Message-ID: <20120717115118.00e3da0b@bwidawsk.net> (raw)
In-Reply-To: <1342523579_7970@CP5-2952>

On Tue, 17 Jul 2012 12:12:39 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On Mon, 16 Jul 2012 11:51:55 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> > Pros:
> > * Potential for per batch, or ring watchdog values. I believe when/if we
> > get to GPGPU workloads, this is particularly interesting.
> > * Batch granularity hang detection. This mostly just makes hang
> > detection and recovery a bit easier IMO.
> > 
> > Cons:
> > * Blit ring doesn't have an interrupt. This means we still need the
> > software watchdog, and it makes hang detection more complex. I've been
> > led to believe future HW *may* have this interrupt.
> > * Semaphores 
> 
> Replacing the black magic for INSTDONE hang detection does seem like a
> sensible plan, but as long as we require the hangcheck timer we are only
> adding code complexity. So there really needs to a be a compelling
> advantage for the watchdoy, something that we cannot acheive with the
> existing method.

Just to be clear, INSTDONE can go away. I don't think it's valuable for
the blitter.

> 
> For me, the criteria is whether we ever miss a hang or falsely accuse
> the hw of stopping. If I understand the watchdog correctly, it basically
> ensures the batch completes within a certain interval which we can
> codify into the existing hangcheck, so no USP.

Yeah. If we follow the windows model, I think we just tweak the value
until we find something, "good" and just always reset on the timeout
instead of doing instdone-foo.

> 
> Or is there more magic waiting in the wings?
> -Chris
> 

The magic was only a more straightforward way of finding the batch to
blame, and as I said on IRC, when I started I was planning to gut the
whole SW watchdog; that was the magic.

FWIW I think we may see the interrupt in future products; so it may
still be worth considering whether we want to move in this direction.

-- 
Ben Widawsky, Intel Open Source Technology Center

      reply	other threads:[~2012-07-17 18:51 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-16 18:51 [PATCH 0/4] [RFC] use HW watchdog timer Ben Widawsky
2012-07-16 18:51 ` [PATCH 1/4] drm/i915: Use HW watchdog for each batch Ben Widawsky
2012-07-16 18:51 ` [PATCH 2/4] drm/i915: Turn on watchdog interrupts Ben Widawsky
2012-07-16 18:51 ` [PATCH 3/4] drm/i915: Add a breadcrumb Ben Widawsky
2012-07-16 18:51 ` [PATCH 4/4] drm/i915: Display the failing seqno Ben Widawsky
2012-07-16 20:16 ` [PATCH 0/4] [RFC] use HW watchdog timer Daniel Vetter
2012-07-17 11:12 ` Chris Wilson
2012-07-17 18:51   ` Ben Widawsky [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120717115118.00e3da0b@bwidawsk.net \
    --to=ben@bwidawsk.net \
    --cc=chris@chris-wilson.co.uk \
    --cc=daniel.vetter@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.