[Intel-gfx] [PATCH 07/32] drm/i915: Store the reset counter when constructing a request

[Dave Gordon]

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.