[Mesa-dev] [RFC] freedreno: import libdrm_freedreno + redesign submit

[Rob Clark]

On Tue, Oct 30, 2018 at 11:27 AM Emil Velikov <emil.l.velikov at gmail.com> wrote:
>
> On Thu, 25 Oct 2018 at 10:32, Eric Engestrom <eric.engestrom at intel.com> wrote:
> >
> > On Tuesday, 2018-10-23 10:49:26 -0400, mesa-dev-bounces at lists.freedesktop.org wrote:
> > > In the pursuit of lowering driver overhead, it became clear that some
> > > amount of redesign of how libdrm_freedreno constructs the submit ioctl
> > > would be needed.  In particular, as the gallium driver is starting to
> > > make heavier use of CP_SET_DRAW_STATE state groups/objects, the over-
> > > head of tracking cmd buffers and relocs becomes too much.  And for
> > > "streaming" state, which isn't ever reused (like uniform uploads) the
> > > overhead of allocating/freeing ringbuffer[1] objects is too high.
> > >
> > > This redesign makes two main changes:
> > >
> > >  1) Introduces a fd_submit object for tracking bos and cmds table
> > >     for the submit ioctl, making ringbuffer objects more light-
> > >     weight.  This was previously done in the ringbuffer.  But we
> > >     have many ringbuffer instances involved in a submit (gmem +
> > >     draw + potentially 1000's of state-group rbs), and only need
> > >     a single bos and cmds table.  (Reloc table is still per-rb)
> > >
> > >     The submit is also a convenient place for a slab allocator for
> > >     ringbuffer objects.  Other options would have required locking
> > >     because, while we can guarantee allocations will only happen on
> > >     a single thread, free's could happen either on the application
> > >     thread or the flush_queue thread.  With the slab allocator in
> > >     the submit object, any frees that happen on the flush_queue
> > >     thread happen after we know that the application thread is done
> > >     with the submit.
> > >
> > >  2) Introduce a new "softpin" msm_ringbuffer_sp implementation that
> > >     does not use relocs and only has cmds table entries for IB1 (ie.
> > >     the cmdstream buffers that kernel needs to CP_INDIRECT_BUFFER
> > >     to from the RB).  To do this properly will require some updates
> > >     on the kernel side, so whether you get the softpin or legacy
> > >     submit/ringbuffer implementation at runtime depends on your
> > >     kernel version.
> > >
> > > To make all these changes in libdrm would basically require adding a
> > > libdrm_freedreno2, so this is a good point to just pull the libdrm code
> > > into mesa.  Plus it allows for using mesa's hashtable, slab allocator,
> > > etc.  And it lets us have asserts enabled for debug mesa buids but
> > > omitted for release builds.  And it makes life easier if further API
> > > changes become necessary.
> > >
> libdrm_freedreno made sense when there was more than one user. Since
> xf86-video-freedreno is a dead end it's better to make your life
> easier.
>
> > > At this point I haven't tried to pull in the kgsl backend.  Although
> > > I left the level of vfunc indirection which would make it possible
> > > to have other backends.  (And this was convenient to keep to allow
> > > for the "softpin" ringbuffer to coexist.)
> > >
> Does that code even work with the latest kernel kgsl code?
> I thought wasn't compatible for years.
>
> > > NOTE: if bisecting a build error takes you hear, try a clean build.
> > > There are a bunch of ways things can go wrong if you still have
> > > libdrm_freedreno cflags.
> >
> > Good note!
> > (and s/hear/here/)
> >
> Or to make the note disappear and minimise the chance to even getting
> here you can try the following:
>  - patch 1 - dummy copy, mention the sha used as base
>  - patch 2/3/4 - wire up Autoconf/Android/meson
>  - patch 5/... - polish (remove freedreno_drmif.h includes and others)
>

fwiw, I'm not planning to remove libdrm_freedreno any time soon,
because (a) new libdrm vs old mesa combo, and (b) I do have some other
small utils that use libdrm_freedreno.  I'm just planning to not
change it.

BR,
-R

> It'll make it far easier to verity, read and provide meaningful review.
>
> -Emil