[Mesa-dev] [PATCH 00/31] i965: Separate blorp from the rest of the driver

[Jason Ekstrand]

On Thu, Aug 25, 2016 at 10:15 PM, Jason Ekstrand <jason at jlekstrand.net>
wrote:

> On Aug 25, 2016 9:41 PM, "Pohjolainen, Topi" <topi.pohjolainen at gmail.com>
> wrote:
> >
> > On Thu, Aug 25, 2016 at 02:34:37PM -0700, Jason Ekstrand wrote:
> > >    On Thu, Aug 25, 2016 at 1:10 AM, Pohjolainen, Topi
> > >    <[1]topi.pohjolainen at gmail.com> wrote:
> > >
> > >    On Fri, Aug 19, 2016 at 09:55:37AM -0700, Jason Ekstrand wrote:
> > >    > This little patch series is even more blorp code churn.  The end
> > >    objective
> > >    > is in patch 31 which pulls blorp into its own directory completely
> > >    separate
> > >    > from the i965 dri driver.
> > >    >
> > >    > Jason Ekstrand (31):
> > >    >Â  Â i965/blorp: Add a blorp_context struct and init/finish funcs
> > >    >Â  Â i965/blorp: Add an internal shader cache
> > >    >Â  Â i965/blorp/genX: Add helpers for allocating various bits of
> state
> > >    >Â  Â i965/blorp/genX: Pull emit_3dstate_multisample into a helper
> > >    >Â  Â i965/gen6: Refactor gen6_upload_urb
> > >    >Â  Â i965/blorp: Use gen6_upload_urb
> > >    >Â  Â i965/blorp/genX: Move emit_urb_config into another helper
> > >    >Â  Â i965/blorp: Add driver mocs settings to the context
> > >    >Â  Â i965/blorp: Pull emit_surface_state into genX_blorp_exec.c
> > >    >Â  Â i965/blorp: Use blorp_address in brw_blorp_surface instead of
> > >    >Â  Â  Â bo+offset
> > >    >Â  Â i965/blorp/genX: Add a blorp_surface_reloc helper
> > >    >Â  Â i965/blorp: Shorten binding table index enum names
> > >    >Â  Â i965/blorp: Use BT_INDEX enums for setting up the binding
> table
> > >    >Â  Â i965/blorp: Add a helper for allocating binding tables and
> > >    surface
> > >    >Â  Â  Â states
> > >    >Â  Â i965/blorp/exec: Refactor to use blorp_context and a void
> *batch
> > >    >Â  Â i965/blorp: Pull the guts of blorp_exec into a driver-agnostic
> > >    header
> > >    >Â  Â i965/blorp: Move the guts of brw_blorp_exec into
> > >    genX_blorp_exec.c
> > >    >Â  Â i965/blorp: Add an "exec" function pointer to blorp_context
> > >    >Â  Â i965/meta_util: Take an isl_device in get_fast_clear_rect
> > >    >Â  Â i965/blorp: Take a blorp_context in compile_nir_shader
> > >    >Â  Â i965/blorp: Get rid of brw_context
> > >    >Â  Â i965/blorp: Make blorp_addres::buffer a void*
> > >    >Â  Â i965/blorp: Add a fast_clear_op enum
> > >    >Â  Â i965: Move the hiz_op enum to blorp
> > >    >Â  Â i965/blorp: Get rid of most brw and mesa includes
> > >    >Â  Â i965: Roll brw_get_ccs_resolve_rect into blorp_ccs_resolve
> > >    >Â  Â i965: Move get_fast_clear_rect to blorp_clear.c
> > >    >Â  Â i965: Move the type_size function declartaions to brw_nir.h
> > >    >Â  Â i965/blorp: Use isl_format_get_depth_format for setting depth
> > >    formats
> > >    >Â  Â i965/blorp: Remove the remaining brw prefixes from the blorp.h
> > >    API
> > >    >Â  Â i965: Move blorp into src/intel/blorp
> > >
> > >      Patches 28-31 also look good to me and are:
> > >      Reviewed-by: Topi Pohjolainen <[2]topi.pohjolainen at intel.com>
> > >      We discussed a few things offline. Among other things we agreed to
> > >      add as a
> > >      follow-up a word or two about the low 12 bits of aux address in
> > >      patch 11
> > >      giving rational to the delta argument of blorp_surface_reloc(). In
> > >      any case
> > >      I think all patches are now reviewed.
> > >
> > >    I'm still missing a couple.  You can find everything here:
> > >    [3]https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=
> wip/blorp-vulkan
> > >    In particular, a review of the first patch to flatten makefiles
> would
> > >    be nice.
> >
> > Right, that wasn't in the series in the list and I missed it. It looks
> good
> > though:
> >
> > Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> >
> > In the patch split from the original series "i965/blorp: Remove some
> i965-isms
> > from genX_blorp_exec.h" you still have the wm_prog_data::inputs_read type
> > change left in the other patch. It doesn't effect compilation or produced
> > binary, and therefore:
> >
> > Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
>
Fixed and thanks.

> >
> > Then there is still left "i965/blorp: Use gen6_upload_urb" where I wasn't
> > 100% sure if the produced batch for gen6 remained the same as before.
> I'll
> > double check that.
> >
> > Did I miss any other?
>
There's another one in there "i965: Move type_size function declarations to
brw_nir.h" that I don't know if it ever landed on the list.

> Not that I know of off hand.
>
> One quick note (to myself as much as anyone): before this can land I need
> to go back and rework things a bit so that the shader upload API is a cache
> API.  Apparently, the i965 driver will, on occasion, drop all it's shaders
> on the floor and recompile them all.  When this happens, blorp needs to
> recompile too.  In order to handle this, we need to expose the shader
> caching API directly to the blorp interface rather than the simplified
> upload_shader function pointer.  The little cache I wrote will have to be
> part of the anv blorp implementation.
>
I'm about to send a patch for this out.  It replaces patch 1.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160826/ce7989b2/attachment.html>