All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Gordon <david.s.gordon@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 07/32] drm/i915: Store the reset counter when constructing a request
Date: Mon, 4 Jan 2016 17:57:56 +0000	[thread overview]
Message-ID: <568AB2A4.5070809@intel.com> (raw)
In-Reply-To: <20160104161047.GA9682@nuc-i3427.alporthouse.com>

On 04/01/16 16:10, Chris Wilson wrote:
> On Mon, Jan 04, 2016 at 03:58:21PM +0000, Dave Gordon wrote:
>> On 16/12/15 10:19, Chris Wilson wrote:
>>> On Wed, Dec 16, 2015 at 10:44:19AM +0100, Daniel Vetter wrote:
>>>> On Fri, Dec 11, 2015 at 11:33:03AM +0000, Chris Wilson wrote:
>>>>> @@ -1288,7 +1286,7 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
>>>>>
>>>>>   		/* We need to check whether any gpu reset happened in between
>>>>>   		 * the caller grabbing the seqno and now ... */
>>>>> -		if (reset_counter != i915_reset_counter(&dev_priv->gpu_error)) {
>>>>> +		if (req->reset_counter != i915_reset_counter(&dev_priv->gpu_error)) {
>>>>
>>>> READ_ONCE(req->reset) as a future proofing maybe for TDR? Or punt that to
>>>> TDR? Could be confusing to READ_ONCE something that's (with the current
>>>> code at least) invariant.
>>>
>>> Right, my plan was to solve this in TDR! At the moment, I am thinking
>>> that we regenerate a new request for each one resubmitted (that requires
>>> patching the seqno embedded into the ring before restarting the ring),
>>> but otherwise we can just duplicate the contents of the request.
>>
>> With the scheduler, there's no need to patch the contents of the
>> ringbuffer; we just reset it (so it's empty) and then have the
>> scheduler re-emit the requests, starting after the one that hung.
>
> Which breaks __i915_wait_request, since it has already noticed that the
> reset counter has changed. Hence the discussion.
> -Chris

No, I'm saying that it's simpler for TDR to update the requests that are 
going to be re-emitted into an empty ring, rather than patch up the 
ringbuffer contents. Then by the time those requests reach 
__i915_wait_request() they will be seen as within the "current reset 
epoch" and proceed normally. I don't think there's any need to generate 
any new (duplicate) requests.

But maybe you can't take this approach without the scheduler ...

.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2016-01-04 17:57 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-11 11:32 Slaughter the thundering i915_wait_request, v3? Chris Wilson
2015-12-11 11:32 ` [PATCH 01/32] drm/i915: Break busywaiting for requests on pending signals Chris Wilson
2015-12-11 11:32 ` [PATCH 02/32] drm/i915: Limit the busy wait on requests to 5us not 10ms! Chris Wilson
2015-12-11 11:32 ` [PATCH 03/32] drm/i915: Only spin whilst waiting on the current request Chris Wilson
2015-12-18 16:12   ` Daniel Vetter
2015-12-18 16:12     ` Daniel Vetter
2015-12-11 11:33 ` [PATCH 04/32] drm/i915: Hide the atomic_read(reset_counter) behind a helper Chris Wilson
2015-12-16  9:31   ` Daniel Vetter
2015-12-16  9:33   ` Daniel Vetter
2015-12-16  9:36     ` Daniel Vetter
2015-12-16 10:26     ` Chris Wilson
2015-12-11 11:33 ` [PATCH 05/32] drm/i915: Simplify checking of GPU reset_counter in display pageflips Chris Wilson
2015-12-16  9:31   ` Daniel Vetter
2015-12-11 11:33 ` [PATCH 06/32] drm/i915: Tighten reset_counter for reset status Chris Wilson
2015-12-16  9:35   ` Daniel Vetter
2015-12-11 11:33 ` [PATCH 07/32] drm/i915: Store the reset counter when constructing a request Chris Wilson
2015-12-16  9:44   ` Daniel Vetter
2015-12-16 10:19     ` Chris Wilson
2016-01-04 15:58       ` Dave Gordon
2016-01-04 16:10         ` Chris Wilson
2016-01-04 17:57           ` Dave Gordon [this message]
2015-12-11 11:33 ` [PATCH 08/32] drm/i915: Simplify reset_counter handling during atomic modesetting Chris Wilson
2015-12-16  9:46   ` Daniel Vetter
2015-12-11 11:33 ` [PATCH 09/32] drm/i915: Prevent leaking of -EIO from i915_wait_request() Chris Wilson
2015-12-16  9:52   ` Daniel Vetter
2015-12-16 11:06     ` Chris Wilson
2015-12-16 12:53       ` Daniel Vetter
2015-12-11 11:33 ` [PATCH 10/32] drm/i915: Suppress error message when GPU resets are disabled Chris Wilson
2015-12-16  9:53   ` Daniel Vetter
2015-12-16 10:06     ` Chris Wilson
2015-12-11 11:33 ` [PATCH 11/32] drm/i915: Delay queuing hangcheck to wait-request Chris Wilson
2015-12-11 11:33 ` [PATCH 12/32] drm/i915: Remove the dedicated hangcheck workqueue Chris Wilson
2015-12-11 11:33 ` [PATCH 13/32] drm/i915: Make queueing the hangcheck work inline Chris Wilson
2015-12-11 11:33 ` [PATCH 14/32] drm/i915: Remove forcewake dance from seqno/irq barrier on legacy gen6+ Chris Wilson
2016-01-05 12:45   ` Dave Gordon
2015-12-11 11:33 ` [PATCH 15/32] drm/i915: Slaughter the thundering i915_wait_request herd Chris Wilson
2015-12-14 12:21   ` Tvrtko Ursulin
2015-12-14 13:18     ` Chris Wilson
2015-12-18 10:01     ` [PATCH] " Chris Wilson
2015-12-21 11:23       ` [PATCH v16] " Chris Wilson
2015-12-11 11:33 ` [PATCH 16/32] drm/i915: Separate out the seqno-barrier from engine->get_seqno Chris Wilson
2015-12-11 11:33 ` [PATCH 17/32] drm/i915: Remove the lazy_coherency parameter from request-completed? Chris Wilson
2015-12-14 14:59   ` Tvrtko Ursulin
2015-12-14 15:11     ` Chris Wilson
2016-01-04 11:16       ` Dave Gordon
2016-01-04 11:26         ` Chris Wilson
2016-01-04 13:02           ` Dave Gordon
2016-01-04 13:11             ` Chris Wilson
2016-01-04 14:09             ` Dave Gordon
2016-01-04 14:20               ` Chris Wilson
2016-01-04 17:28                 ` Dave Gordon
2015-12-11 11:33 ` [PATCH 18/32] drm/i915: Use HWS for seqno tracking everywhere Chris Wilson
2016-01-04 18:11   ` Dave Gordon
2016-01-04 19:37     ` Chris Wilson
2015-12-11 11:33 ` [PATCH 19/32] drm/i915: Check the CPU cached value of seqno after waking the waiter Chris Wilson
2015-12-11 11:33 ` [PATCH 20/32] drm/i915: Replace manual barrier() with READ_ONCE() in HWS accessor Chris Wilson
2015-12-11 11:33 ` [PATCH 21/32] drm/i915: Broadwell execlists needs exactly the same seqno w/a as legacy Chris Wilson
2016-01-04 21:34   ` Jesse Barnes
2016-01-05 10:20     ` Chris Wilson
2015-12-11 11:33 ` [PATCH 22/32] drm/i915: Stop setting wraparound seqno on initialisation Chris Wilson
2015-12-11 11:33 ` [PATCH 23/32] drm/i915: Only query timestamp when measuring elapsed time Chris Wilson
2015-12-11 11:33 ` [PATCH 24/32] drm/i915: On GPU reset, set the HWS breadcrumb to the last seqno Chris Wilson
2015-12-11 11:33 ` [PATCH 25/32] drm/i915: Convert trace-irq to the breadcrumb waiter Chris Wilson
2015-12-12 15:20   ` [PATCH v2] " Chris Wilson
2015-12-12 15:34     ` [PATCH 1/3] drm/i915: Move GEM request routines to i915_gem_request.c Chris Wilson
2015-12-12 15:34       ` [PATCH 2/3] drm/i915: Move releasing of the GEM request from free to retire/cancel Chris Wilson
2015-12-12 15:34       ` [PATCH 3/3] drm/i915: Derive GEM requests from dma-fence Chris Wilson
2016-01-04 12:17         ` Dave Gordon
2016-01-04 12:22           ` Chris Wilson
2015-12-11 11:33 ` [PATCH 26/32] drm/i915: Move the get/put irq locking into the caller Chris Wilson
2015-12-11 11:33 ` [PATCH 27/32] drm/i915: Harden detection of missed interrupts Chris Wilson
2015-12-11 11:33 ` [PATCH 28/32] drm/i915: Remove debug noise on detecting fault-injection " Chris Wilson
2015-12-11 11:33 ` [PATCH 29/32] drm/i915: Only start retire worker when idle Chris Wilson
2015-12-15  9:26   ` [PATCH] " Chris Wilson
2015-12-11 11:33 ` [PATCH 30/32] drm/i915: Restore waitboost credit to the synchronous waiter Chris Wilson
2015-12-11 11:33 ` [PATCH 31/32] drm/i915: Add background commentary to "waitboosting" Chris Wilson
2015-12-11 11:33 ` [PATCH 32/32] drm/i915: Flush the RPS bottom-half when the GPU idles Chris Wilson

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=568AB2A4.5070809@intel.com \
    --to=david.s.gordon@intel.com \
    --cc=chris@chris-wilson.co.uk \
    --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.