Re: [Mesa-dev] [PATCH v2 02/31] i965/blorp: Expose the shader cache through function pointers

[Pohjolainen, Topi]

On Fri, Aug 26, 2016 at 06:58:27PM -0700, Jason Ekstrand wrote:
> This sanitizes blorp's access to the i965 driver's shader cache by patching
> it through the blorp_context.  When we start using blorp in Vulkan, we will
> simply have to implement such a caching interface in the Vulkan driver.
> 
> Note: In my first attempt at this, I simplified it down to a single
> upload_shader entrypoint and implemented the caching inside of blorp.  This
> doesn't work, however, because the i965 driver will, on occation, dump its
> entire cache and start over.  When this happens, blorp needs to be able to
> recompile its shaders and re-upload them.  It's easiest to just expose the
> caching interface.

Reviewed-by: Topi Pohjolainen 

> 
> Signed-off-by: Jason Ekstrand 
> Cc: Topi Pohjolainen 
> 
> ---
> 
> This replaces patch 2 from the original series.
> 
>  src/mesa/drivers/dri/i965/blorp.h   |  9 +
>  src/mesa/drivers/dri/i965/blorp_blit.c  | 15 ++-
>  src/mesa/drivers/dri/i965/blorp_clear.c | 15 ++-
>  src/mesa/drivers/dri/i965/brw_blorp.c   | 26 ++
>  4 files changed, 47 insertions(+), 18 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/blorp.h 
> b/src/mesa/drivers/dri/i965/blorp.h
> index 7dbf022..602d97e 100644
> --- a/src/mesa/drivers/dri/i965/blorp.h
> +++ b/src/mesa/drivers/dri/i965/blorp.h
> @@ -41,6 +41,15 @@ struct blorp_context {
> void *driver_ctx;
>  
> const struct isl_device *isl_dev;
> +
> +   bool (*lookup_shader)(struct blorp_context *blorp,
> + const void *key, uint32_t key_size,
> + uint32_t *kernel_out, void *prog_data_out);
> +   void (*upload_shader)(struct blorp_context *blorp,
> + const void *key, uint32_t key_size,
> + const void *kernel, uint32_t kernel_size,
> + const void *prog_data, uint32_t prog_data_size,
> + uint32_t *kernel_out, void *prog_data_out);
>  };
>  
>  void blorp_init(struct blorp_context *blorp, void *driver_ctx,
> diff --git a/src/mesa/drivers/dri/i965/blorp_blit.c 
> b/src/mesa/drivers/dri/i965/blorp_blit.c
> index a4b3fe0..0291e01 100644
> --- a/src/mesa/drivers/dri/i965/blorp_blit.c
> +++ b/src/mesa/drivers/dri/i965/blorp_blit.c
> @@ -32,7 +32,6 @@
>  
>  #include "blorp_priv.h"
>  #include "brw_context.h"
> -#include "brw_state.h"
>  #include "brw_meta_util.h"
>  
>  #define FILE_DEBUG_FLAG DEBUG_BLORP
> @@ -1196,9 +1195,8 @@ brw_blorp_get_blit_kernel(struct brw_context *brw,
>struct brw_blorp_params *params,
>const struct brw_blorp_blit_prog_key *prog_key)
>  {
> -   if (brw_search_cache(>cache, BRW_CACHE_BLORP_PROG,
> -prog_key, sizeof(*prog_key),
> ->wm_prog_kernel, >wm_prog_data))
> +   if (brw->blorp.lookup_shader(>blorp, prog_key, sizeof(*prog_key),
> +>wm_prog_kernel, 
> >wm_prog_data))
>return;
>  
> const unsigned *program;
> @@ -1219,11 +1217,10 @@ brw_blorp_get_blit_kernel(struct brw_context *brw,
> program = brw_blorp_compile_nir_shader(brw, nir, _key, false,
>_data, _size);
>  
> -   brw_upload_cache(>cache, BRW_CACHE_BLORP_PROG,
> -prog_key, sizeof(*prog_key),
> -program, program_size,
> -_data, sizeof(prog_data),
> ->wm_prog_kernel, >wm_prog_data);
> +   brw->blorp.upload_shader(>blorp, prog_key, sizeof(*prog_key),
> +program, program_size,
> +_data, sizeof(prog_data),
> +>wm_prog_kernel, >wm_prog_data);
>  }
>  
>  static void
> diff --git a/src/mesa/drivers/dri/i965/blorp_clear.c 
> b/src/mesa/drivers/dri/i965/blorp_clear.c
> index 2da08f8..5b8ceec 100644
> --- a/src/mesa/drivers/dri/i965/blorp_clear.c
> +++ b/src/mesa/drivers/dri/i965/blorp_clear.c
> @@ -35,7 +35,6 @@
>  #include "brw_meta_util.h"
>  #include "brw_context.h"
>  #include "brw_eu.h"
> -#include "brw_state.h"
>  
>  #include "nir_builder.h"
>  
> @@ -56,9 +55,8 @@ brw_blorp_params_get_clear_kernel(struct brw_context *brw,
> memset(_key, 0, sizeof(blorp_key));
> blorp_key.use_simd16_replicated_data = use_replicated_data;
>  
> -   if (brw_search_cache(>cache, BRW_CACHE_BLORP_PROG,
> -_key, sizeof(blorp_key),
> ->wm_prog_kernel, >wm_prog_data))
> +   if (brw->blorp.lookup_shader(>blorp, _key, sizeof(blorp_key),
> +>wm_prog_kernel, 
> >wm_prog_data))
>return;
>  
> void *mem_ctx = ralloc_context(NULL);
> @@ -88,11 +86,10 @@ brw_blorp_params_get_clear_kernel(struct brw_context *brw,
>brw_blorp_compile_nir_shader(brw, b.shader, _key, 
> 

[Mesa-dev] [PATCH v2 02/31] i965/blorp: Expose the shader cache through function pointers

[Jason Ekstrand]

This sanitizes blorp's access to the i965 driver's shader cache by patching
it through the blorp_context.  When we start using blorp in Vulkan, we will
simply have to implement such a caching interface in the Vulkan driver.

Note: In my first attempt at this, I simplified it down to a single
upload_shader entrypoint and implemented the caching inside of blorp.  This
doesn't work, however, because the i965 driver will, on occation, dump its
entire cache and start over.  When this happens, blorp needs to be able to
recompile its shaders and re-upload them.  It's easiest to just expose the
caching interface.

Signed-off-by: Jason Ekstrand 
Cc: Topi Pohjolainen 

---

This replaces patch 2 from the original series.

 src/mesa/drivers/dri/i965/blorp.h   |  9 +
 src/mesa/drivers/dri/i965/blorp_blit.c  | 15 ++-
 src/mesa/drivers/dri/i965/blorp_clear.c | 15 ++-
 src/mesa/drivers/dri/i965/brw_blorp.c   | 26 ++
 4 files changed, 47 insertions(+), 18 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/blorp.h 
b/src/mesa/drivers/dri/i965/blorp.h
index 7dbf022..602d97e 100644
--- a/src/mesa/drivers/dri/i965/blorp.h
+++ b/src/mesa/drivers/dri/i965/blorp.h
@@ -41,6 +41,15 @@ struct blorp_context {
void *driver_ctx;
 
const struct isl_device *isl_dev;
+
+   bool (*lookup_shader)(struct blorp_context *blorp,
+ const void *key, uint32_t key_size,
+ uint32_t *kernel_out, void *prog_data_out);
+   void (*upload_shader)(struct blorp_context *blorp,
+ const void *key, uint32_t key_size,
+ const void *kernel, uint32_t kernel_size,
+ const void *prog_data, uint32_t prog_data_size,
+ uint32_t *kernel_out, void *prog_data_out);
 };
 
 void blorp_init(struct blorp_context *blorp, void *driver_ctx,
diff --git a/src/mesa/drivers/dri/i965/blorp_blit.c 
b/src/mesa/drivers/dri/i965/blorp_blit.c
index a4b3fe0..0291e01 100644
--- a/src/mesa/drivers/dri/i965/blorp_blit.c
+++ b/src/mesa/drivers/dri/i965/blorp_blit.c
@@ -32,7 +32,6 @@
 
 #include "blorp_priv.h"
 #include "brw_context.h"
-#include "brw_state.h"
 #include "brw_meta_util.h"
 
 #define FILE_DEBUG_FLAG DEBUG_BLORP
@@ -1196,9 +1195,8 @@ brw_blorp_get_blit_kernel(struct brw_context *brw,
   struct brw_blorp_params *params,
   const struct brw_blorp_blit_prog_key *prog_key)
 {
-   if (brw_search_cache(>cache, BRW_CACHE_BLORP_PROG,
-prog_key, sizeof(*prog_key),
->wm_prog_kernel, >wm_prog_data))
+   if (brw->blorp.lookup_shader(>blorp, prog_key, sizeof(*prog_key),
+>wm_prog_kernel, 
>wm_prog_data))
   return;
 
const unsigned *program;
@@ -1219,11 +1217,10 @@ brw_blorp_get_blit_kernel(struct brw_context *brw,
program = brw_blorp_compile_nir_shader(brw, nir, _key, false,
   _data, _size);
 
-   brw_upload_cache(>cache, BRW_CACHE_BLORP_PROG,
-prog_key, sizeof(*prog_key),
-program, program_size,
-_data, sizeof(prog_data),
->wm_prog_kernel, >wm_prog_data);
+   brw->blorp.upload_shader(>blorp, prog_key, sizeof(*prog_key),
+program, program_size,
+_data, sizeof(prog_data),
+>wm_prog_kernel, >wm_prog_data);
 }
 
 static void
diff --git a/src/mesa/drivers/dri/i965/blorp_clear.c 
b/src/mesa/drivers/dri/i965/blorp_clear.c
index 2da08f8..5b8ceec 100644
--- a/src/mesa/drivers/dri/i965/blorp_clear.c
+++ b/src/mesa/drivers/dri/i965/blorp_clear.c
@@ -35,7 +35,6 @@
 #include "brw_meta_util.h"
 #include "brw_context.h"
 #include "brw_eu.h"
-#include "brw_state.h"
 
 #include "nir_builder.h"
 
@@ -56,9 +55,8 @@ brw_blorp_params_get_clear_kernel(struct brw_context *brw,
memset(_key, 0, sizeof(blorp_key));
blorp_key.use_simd16_replicated_data = use_replicated_data;
 
-   if (brw_search_cache(>cache, BRW_CACHE_BLORP_PROG,
-_key, sizeof(blorp_key),
->wm_prog_kernel, >wm_prog_data))
+   if (brw->blorp.lookup_shader(>blorp, _key, sizeof(blorp_key),
+>wm_prog_kernel, 
>wm_prog_data))
   return;
 
void *mem_ctx = ralloc_context(NULL);
@@ -88,11 +86,10 @@ brw_blorp_params_get_clear_kernel(struct brw_context *brw,
   brw_blorp_compile_nir_shader(brw, b.shader, _key, use_replicated_data,
_data, _size);
 
-   brw_upload_cache(>cache, BRW_CACHE_BLORP_PROG,
-_key, sizeof(blorp_key),
-program, program_size,
-_data, sizeof(prog_data),
->wm_prog_kernel, >wm_prog_data);
+   brw->blorp.upload_shader(>blorp, _key,