[Intel-gfx] [PATCH 00/14] drm-intel-collector - update

[Daniel Vetter]

On Fri, Apr 25, 2014 at 12:19 PM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> On Fri, Apr 25, 2014 at 12:07:48PM +0200, Daniel Vetter wrote:
>> On Fri, Apr 25, 2014 at 10:24:45AM +0100, Chris Wilson wrote:
>> > On Fri, Apr 25, 2014 at 11:04:22AM +0200, Daniel Vetter wrote:
>> > > > Patch     drm/i915: Upgrade execbuffer fail after resume failure to EIO - Reviewer:
>> > >
>> > > Do we still need this on top of what I've merged. Chris?
>> >
>> > Yes. We can still start the device without initializing all of the
>> > available rings and so hit this before we fail with an EIO.
>>
>> Hm, so we'd need to adjust the commit message a bit since with
>>
>> commit 5582e8c3c49150c0e7398688b5ed167d6c3d44fd
>> Author: Chris Wilson <chris at chris-wilson.co.uk>
>> Date:   Wed Apr 9 09:19:41 2014 +0100
>>
>>     drm/i915: Preserve ring buffers objects across resume
>>
>> this is just a driver load issue and no longer a resume issue. Or do I
>> miss something else?
>>
>> Or should we frob the ring init some more and simply leave the ring->obj
>> hanging around, eat the -EIO and wedge the gpu?
>
> No, because we abort i915_gem_init_rings(). This is an exceptional error
> path and I think just fixing up the errno here is simplest. All other
> paths should barf before they touch the ring thanks to
> intel_ring_begin(). The only difficulty here is that we need to
> differentiate between two different types of error. (Note that even UXA
> should be checking for a dead GPU before it tries execbuffer, in this
> case, at least since it started trying to not die randomly.) So I am
> just arguing from an interface correctness pov.

Yeah I've read around a bit in our gem_init_hw functions and this
seems to be the sanest option. Can you please update the patch's
commit message to reflect our discussion here? And maybe throw a t-b
in in case we have that.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch