Re: [PATCH 08/43] drm/i915/bdw: Add a context and an engine pointers to the ringbuffer

Daniel Vetter
2014-08-14T15:37:00Z

On Thu, Aug 14, 2014 at 05:32:28PM +0200, Daniel Vetter wrote:
> On Thu, Aug 14, 2014 at 03:09:45PM +0000, Daniel, Thomas wrote:
> > When it comes to the execlist submission (actually as early as the execlist
> > request queueing), the engine and context are indeed used and required.
> > intel_logical_ring_advance_and_submit() is the lrc function analogous to
> > __intel_ring_advance() and I believe the initial creation of intel_lrc.c
> > was actually done by copying intel_ringbuffer.c. This explains why some of
> > the lrc code is perhaps not as it would have been if this had been designed
> > from scratch, and there is room for future improvement.
> > advance_and_submit therefore only gets the ringbuffer struct and uses
> > the context pointer in that struct to get the logical ring context itself. At
> > that point the engine, context and new tail pointer are handed over to the
> > execlist queue backend.
>
> I guess I need to clarify: Does it make sense to move the ELSP
> respectively the submission to the execlist scheduler queue out of there
> up a few levels into the execlist cmd submission function? Is it possible
> or is there some technical reason that I'm overlooking?
>
> I want to know what exactly I'm dealing with here before I sign up for it
> by merging the patches as-is and asking for a cleanup. I doesn't look bad
> really, but there's always a good chance that I've overlooked a bigger
> dragon.
>
> Since you have the patches and worked with them I'm asking you such
> explorative questions. Ofc I can do this checking myself, but that takes
> time ... This doesn't mean that you have to implement the changes, just be
> reasonable confident that it will work out as a cleanup on top.

To clarify more the context: Currently you're replies sound like "This is
what it looks like and I don't really know why nor whether we can change
that". That's not confidence instilling and that makes maintainers
reluctant to merge patches for fear of needing to fix things themselves
;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch