Re: [PATCH] drm/i915: Reinstate order of operations in {intel, logical}_ring_begin()

[Dave Gordon <david.s.gordon@intel.com>]

On 15/06/15 21:41, Chris Wilson wrote:
> On Mon, Jun 15, 2015 at 07:11:37PM +0100, Dave Gordon wrote:
>>> It still applies. If you submit say 1024 interrupted execbuffers they
>>
>> What is an interrupted execbuffer? AFAICT we hold the struct_mutex while
>> stuffing the ringbuffer so we can only ever be in the process of adding
>> instructions to one ringbuffer at a time, and we don't (now) interleave
>> any flip commands (execlists mode requires mmio flip). Is there still
>> something that just adds random stuff to someone else's OLR?
> 
> Write commands to stream, fail to add the request because the wait for
> ring space fails due to a pending signals. Repeat until the entire ring
> is filled by a single pending request.
> -Chris

Uuuurgh ... I always said there was something wrong with leaving partial
command streams in the ringbuffer :(

In the Android version we make sure that can't happen by checking that
there's going to be enough space for all the commands generated by the
request before we start writing into the ringbuffer, so no ring_begin()
call thereafter can ever have to wait for more space.

Of course, having a global lock doesn't help. With multiple ringbuffers
per engine, one should really only need a per-ringbuffer lock for
writing into the ringbuffer ... and we should reset the tail pointer if
writing the request is interrupted or aborted.

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