On Wed, Nov 12, 2014 at 12:13:03PM +0000, Nick Hoath wrote:
> On 12/11/2014 11:24, Chris Wilson wrote:
> >On Wed, Nov 12, 2014 at 10:53:26AM +0000, Nick Hoath wrote:
> > seq_putc(m, '\n');
> >>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >>index afa9c35..0fe238c 100644
> >>--- a/drivers/gpu/drm/i915/i915_drv.h
> >>+++ b/drivers/gpu/drm/i915/i915_drv.h
> >>@@ -2027,6 +2027,28 @@ struct drm_i915_gem_request {
> >> struct list_head free_list;
> >>
> >> uint32_t uniq;
> >>+
> >>+ /**
> >>+ * The ELSP only accepts two elements at a time, so we queue context/tail
> >>+ * pairs on a given queue (ring->execlist_queue) until the hardware is
> >>+ * available. The queue serves a double purpose: we also use it to keep track
> >>+ * of the up to 2 contexts currently in the hardware (usually one in execution
> >>+ * and the other queued up by the GPU): We only remove elements from the head
> >>+ * of the queue when the hardware informs us that an element has been
> >>+ * completed.
> >>+ *
> >>+ * All accesses to the queue are mediated by a spinlock (ring->execlist_lock).
> >>+ */
> >>+
> >>+ /** Execlist link in the submission queue.*/
> >>+ struct list_head execlist_link;
> >
> >This is redundant. The request should only be one of the pending or active
> >lists at any time.
> >
> This is used by the pending execlist requests list owned by the
> intel_engine_cs. The request isn't in both the active and pending execlist
> engine lists.
I guess we'll eventually need to subsume this into the scheduler's list of
ctxs. Or reuse it there. Imo ok for now, might just need a rename
longterm.
> >>+ /** Execlists workqueue for processing this request in a bottom half */
> >>+ struct work_struct work;
> >
> >For what purpose? This is not needed.
> This worker is currently used to free up execlist requests. This goes away
> when Thomas Daniel's patchset is merged.
> I have spotted a bug in the cleanup handler with the merged
> requests/execlists cleanup though.
I guess this will drop out on a rebase now that everything has landed?
> >>+ /** Execlists no. of times this request has been sent to the ELSP */
> >>+ int elsp_submitted;
> >
> >A request can only be submitted exactly once at any time. This
> >bookkeeping is not part of the request.
> This is a refcount to preserve the request if it has been resubmitted due to
> preemption or TDR, due to a race condition between the HW finishing with the
> item and the cleanup/resubmission. Have a look at
> e1fee72c2ea2e9c0c6e6743d32a6832f21337d6c which contains a much better
> description of why this exists.
Yeah this one is still a bit crazy but guess that's just how it is ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
