[Intel-gfx] [PATCH 09/13] drm/i915: add reset_state for hw_contexts

[Chris Wilson]

On Tue, Feb 26, 2013 at 05:47:12PM -0800, Ian Romanick wrote:
> On 02/26/2013 03:05 AM, Mika Kuoppala wrote:
> >For arb-robustness, every context needs to have it's own
> >reset state tracking. Default context will be handled in a identical
> >way as the no-context case in further down in the patch set.
> >For no-context case, the reset state will be stored in
> >the file_priv part.
> >
> >v2: handle default context inside get_reset_state
> 
> This isn't the interface we want.  I already sent you the patches
> for Mesa, and you seem to have completely ignored them.  Moreover,
> this interface provides no mechanism to query for its existence
> (other than relying on the kernel version), and no method to
> deprecate it.

It's existence, and the existence of any successor, is the only test you
need to check for the interface. Testing the flag field up front also
lets you know if individual flags are known prior to use.
 
> Based on e-mail discussions, I think danvet agrees with me here.
> 
> Putting guilty / innocent counting in kernel puts policy decisions
> in the kernel that belong with the user space API that implements
> them. Putting these choices in the kernel significantly decreases
> how "future proof" the interface is.  Since any kernel/user
> interface has to be kept forever, this is just asking for
> maintenance trouble down the road.

I think you have the policy standpoint reversed.
 
> Also, it's difficult (for me) to tell from the rest of the series
> whether or not a context that was not affected by a reset (had no
> batches in flight at all) can still observe that a reset occurred.

Yes, the context can observe that the global reset increased, its did
not.

> From the GL point of view, once a context has observed a reset, it's
> garbage.  Knowing that it saw 1 reset or 43,000,000 resets is the
> same.  Since it's a count, GL will have to query the values before
> any rendering happens or it won't know whether the value 6 means
> there was a reset or not.
> 
> The right interface is a set of flags indicating the state the
> context was in when it observed a reset.  As this patch series
> stands, Mesa will not use this interface.

This interface conforms to your specifications and interface that you
last posted to the list and were revised on list. There are two
substantive differences; there is *less* policy in the kernel than
before, and the breadcrumb of last reset state was overlooked.

In order to make the reset status work in a multi-threaded environment
the kernel exists in, we have to assume that there will be multiple
actors on the reset status, and they will be comparing the state that
they are interested in. The set of flags that GL understands is
deducible from this interface.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre