On Tuesday, 2018-10-23 10:49:26 -0400, mesa-dev-boun...@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. > > 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.) > > 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/) > > [1] "ringbuffer" is probably a bad name, the only level of cmdstream > buffer that is actually a ring is RB managed by kernel. User- > space cmdstream is all IB1/IB2 and state-groups. > > Signed-off-by: Rob Clark <robdcl...@gmail.com> > --- > Note one "benchmark" that I was at while working on this is webgl > aquarium. It isn't a terribly clever gl app, doing one draw per > fish, uploading uniforms (and usually not making any other state > changes) between each draw. The "softpin" implementation drops > CPU utilization by about 20%, and once you get CPU limited, it is > worth ~50% fps boost. > > Also note, android build probably needs some attention (mostly > replacing libdrm_freedreno dependency with libdrm + valgrind). Might be best to do that in the same commit, to avoid breaking bisects. > > configure.ac | 2 - > meson.build | 2 - > src/gallium/drivers/freedreno/Makefile.am | 7 +- > .../drivers/freedreno/Makefile.sources | 17 + > .../drivers/freedreno/a3xx/fd3_context.h | 2 - > .../drivers/freedreno/a4xx/fd4_context.h | 2 - > .../drivers/freedreno/a5xx/fd5_context.h | 2 - > src/gallium/drivers/freedreno/a5xx/fd5_draw.c | 3 +- > .../drivers/freedreno/a6xx/fd6_context.h | 2 - > src/gallium/drivers/freedreno/a6xx/fd6_draw.c | 3 +- > src/gallium/drivers/freedreno/a6xx/fd6_emit.c | 26 +- > .../drivers/freedreno/drm/freedreno_bo.c | 361 +++++++++ > .../freedreno/drm/freedreno_bo_cache.c | 218 ++++++ > .../drivers/freedreno/drm/freedreno_device.c | 156 ++++ > .../drivers/freedreno/drm/freedreno_drmif.h | 126 +++ > .../drivers/freedreno/drm/freedreno_pipe.c | 100 +++ > .../drivers/freedreno/drm/freedreno_priv.h | 258 +++++++ > .../freedreno/drm/freedreno_ringbuffer.c | 114 +++ > .../freedreno/drm/freedreno_ringbuffer.h | 159 ++++ > src/gallium/drivers/freedreno/drm/msm_bo.c | 170 +++++ > .../drivers/freedreno/drm/msm_device.c | 61 ++ > src/gallium/drivers/freedreno/drm/msm_drm.h | 308 ++++++++ > src/gallium/drivers/freedreno/drm/msm_pipe.c | 223 ++++++ > src/gallium/drivers/freedreno/drm/msm_priv.h | 140 ++++ > .../drivers/freedreno/drm/msm_ringbuffer.c | 721 ++++++++++++++++++ > .../drivers/freedreno/drm/msm_ringbuffer_sp.c | 551 +++++++++++++ > .../drivers/freedreno/freedreno_batch.c | 25 +- > .../drivers/freedreno/freedreno_batch.h | 2 + > .../drivers/freedreno/freedreno_gmem.c | 8 +- > .../drivers/freedreno/freedreno_screen.c | 1 + > .../drivers/freedreno/freedreno_screen.h | 4 +- > .../drivers/freedreno/freedreno_util.h | 30 +- > .../drivers/freedreno/ir3/ir3_shader.c | 11 +- > src/gallium/drivers/freedreno/meson.build | 25 +- > src/gallium/winsys/freedreno/drm/meson.build | 2 +- > 35 files changed, 3765 insertions(+), 77 deletions(-) > create mode 100644 src/gallium/drivers/freedreno/drm/freedreno_bo.c > create mode 100644 src/gallium/drivers/freedreno/drm/freedreno_bo_cache.c > create mode 100644 src/gallium/drivers/freedreno/drm/freedreno_device.c > create mode 100644 src/gallium/drivers/freedreno/drm/freedreno_drmif.h > create mode 100644 src/gallium/drivers/freedreno/drm/freedreno_pipe.c > create mode 100644 src/gallium/drivers/freedreno/drm/freedreno_priv.h > create mode 100644 src/gallium/drivers/freedreno/drm/freedreno_ringbuffer.c > create mode 100644 src/gallium/drivers/freedreno/drm/freedreno_ringbuffer.h > create mode 100644 src/gallium/drivers/freedreno/drm/msm_bo.c > create mode 100644 src/gallium/drivers/freedreno/drm/msm_device.c > create mode 100644 src/gallium/drivers/freedreno/drm/msm_drm.h > create mode 100644 src/gallium/drivers/freedreno/drm/msm_pipe.c > create mode 100644 src/gallium/drivers/freedreno/drm/msm_priv.h > create mode 100644 src/gallium/drivers/freedreno/drm/msm_ringbuffer.c > create mode 100644 src/gallium/drivers/freedreno/drm/msm_ringbuffer_sp.c > > diff --git a/configure.ac b/configure.ac > index ab9bdce885f..15ac8b0d1b9 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -78,7 +78,6 @@ LIBDRM_AMDGPU_REQUIRED=2.4.93 > LIBDRM_INTEL_REQUIRED=2.4.75 > LIBDRM_NVVIEUX_REQUIRED=2.4.66 > LIBDRM_NOUVEAU_REQUIRED=2.4.66 > -LIBDRM_FREEDRENO_REQUIRED=2.4.96 > LIBDRM_ETNAVIV_REQUIRED=2.4.89 > LIBDRM_VC4_REQUIRED=2.4.89 > > @@ -2722,7 +2721,6 @@ if test -n "$with_gallium_drivers"; then > ;; > xfreedreno) > HAVE_GALLIUM_FREEDRENO=yes > - PKG_CHECK_MODULES([FREEDRENO], [libdrm >= > $LIBDRM_FREEDRENO_REQUIRED libdrm_freedreno >= $LIBDRM_FREEDRENO_REQUIRED]) > require_libdrm "freedreno" > ;; > xetnaviv) > diff --git a/meson.build b/meson.build > index 505cc6c79bd..d483c61f5c9 100644 > --- a/meson.build > +++ b/meson.build > @@ -1112,7 +1112,6 @@ _drm_amdgpu_ver = '2.4.93' Just above this: ----8<---- diff --git a/meson.build b/meson.build index 6d274d9f4a404c29a8c8..4640a04ddbd899e42a69 100644 --- a/meson.build +++ b/meson.build @@ -1098,7 +1098,6 @@ dep_libdrm_amdgpu = null_dep dep_libdrm_radeon = null_dep dep_libdrm_nouveau = null_dep dep_libdrm_etnaviv = null_dep -dep_libdrm_freedreno = null_dep dep_libdrm_intel = null_dep _drm_amdgpu_ver = '2.4.95' ---->8---- With that, the meson side of this patch is: Reviewed-by: Eric Engestrom <eric.engest...@intel.com> > _drm_radeon_ver = '2.4.71' > _drm_nouveau_ver = '2.4.66' > _drm_etnaviv_ver = '2.4.89' > -_drm_freedreno_ver = '2.4.96' > _drm_intel_ver = '2.4.75' > _drm_ver = '2.4.75' > > @@ -1123,7 +1122,6 @@ _libdrm_checks = [ > with_gallium_r300 or with_gallium_r600)], > ['nouveau', (with_gallium_nouveau or with_dri_nouveau)], > ['etnaviv', with_gallium_etnaviv], > - ['freedreno', with_gallium_freedreno], > ] > > # VC4 only needs core libdrm support of this version, not a libdrm_vc4 [snip] > diff --git a/src/gallium/drivers/freedreno/meson.build > b/src/gallium/drivers/freedreno/meson.build > index 9272602e547..d26aa0942e6 100644 > --- a/src/gallium/drivers/freedreno/meson.build > +++ b/src/gallium/drivers/freedreno/meson.build > @@ -71,6 +71,21 @@ files_libfreedreno = files( > 'freedreno_texture.h', > 'freedreno_util.c', > 'freedreno_util.h', > + 'drm/freedreno_bo.c', > + 'drm/freedreno_bo_cache.c', > + 'drm/freedreno_device.c', > + 'drm/freedreno_drmif.h', > + 'drm/freedreno_pipe.c', > + 'drm/freedreno_priv.h', > + 'drm/freedreno_ringbuffer.c', > + 'drm/freedreno_ringbuffer.h', > + 'drm/msm_bo.c', > + 'drm/msm_device.c', > + 'drm/msm_drm.h', > + 'drm/msm_pipe.c', > + 'drm/msm_priv.h', > + 'drm/msm_ringbuffer.c', > + 'drm/msm_ringbuffer_sp.c', > 'a2xx/a2xx.xml.h', > 'a2xx/disasm-a2xx.c', > 'a2xx/fd2_blend.c', > @@ -259,7 +274,12 @@ libfreedreno = static_library( > include_directories : freedreno_includes, > c_args : [freedreno_c_args, c_vis_args], > cpp_args : [freedreno_cpp_args, cpp_vis_args], > - dependencies : [dep_libdrm, dep_libdrm_freedreno, idep_nir_headers], > + dependencies : [ > + dep_libdrm, > + dep_libdrm, Harmless, but duplicate `dep_libdrm` here and in the next hunk below. > + dep_valgrind, > + idep_nir_headers > + ], > ) > > driver_freedreno = declare_dependency( > @@ -274,7 +294,8 @@ ir3_compiler = executable( > include_directories : freedreno_includes, > dependencies : [ > dep_libdrm, > - dep_libdrm_freedreno, > + dep_libdrm, > + dep_valgrind, > dep_thread, > idep_nir, > ], > diff --git a/src/gallium/winsys/freedreno/drm/meson.build > b/src/gallium/winsys/freedreno/drm/meson.build > index 34aff635dde..0fc02897ddd 100644 > --- a/src/gallium/winsys/freedreno/drm/meson.build > +++ b/src/gallium/winsys/freedreno/drm/meson.build > @@ -25,5 +25,5 @@ libfreedrenowinsys = static_library( > inc_src, inc_include, inc_gallium, inc_gallium_aux, inc_gallium_drivers, > ], > c_args : [c_vis_args], > - dependencies : [dep_libdrm_freedreno], > + dependencies : [dep_libdrm], > ) > -- > 2.17.2 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
