On Aug 25, 2016 9:41 PM, "Pohjolainen, Topi" <<a href="mailto:topi.pohjolainen@gmail.com">topi.pohjolainen@gmail.com</a>> 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]<a href="mailto:topi.pohjolainen@gmail.com">topi.pohjolainen@gmail.com</a>> 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]<a href="mailto:topi.pohjolainen@intel.com">topi.pohjolainen@intel.com</a>> > > 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]<a href="https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=wip/blorp-vulkan">https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=wip/blorp-vulkan</a> > > 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 <<a href="mailto:topi.pohjolainen@intel.com">topi.pohjolainen@intel.com</a>> > > 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 <<a href="mailto:topi.pohjolainen@intel.com">topi.pohjolainen@intel.com</a>> > > 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? 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.