[2/3] intel/blorp: Make KSP a blorp_address instead of an offset.

Submitted by Kenneth Graunke on Nov. 29, 2018, 8:24 a.m.

Details

Message ID 20181129082440.2397-2-kenneth@whitecape.org
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Kenneth Graunke Nov. 29, 2018, 8:24 a.m.
In i965, shader programs live in a single buffer, and every batch emits
a STATE_BASE_ADDRESS packet pointing to that buffer.  This takes care of
pinning the buffer for the batch; from then on, we can use an offset.

In the upcoming Iris driver, shader programs can live in multiple
buffers, and those buffers need to be pinned when shader assembly is
referenced.  To aid in this, we turn KSP into a blorp_address rather
than a 32-bit offset.  This lets us also pass a buffer to pin.

For now, we simply assert the BO is NULL and return the offset, as
we did before - nothing should behave differently.
---
 src/intel/blorp/blorp.h                     |  7 ++++--
 src/intel/blorp/blorp_genX_exec.h           | 27 +++++++++++++--------
 src/intel/blorp/blorp_priv.h                |  6 ++---
 src/intel/vulkan/anv_blorp.c                | 10 +++++---
 src/mesa/drivers/dri/i965/brw_blorp.c       | 18 ++++++++++----
 src/mesa/drivers/dri/i965/gen4_blorp_exec.h | 12 ++++-----
 6 files changed, 50 insertions(+), 30 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/intel/blorp/blorp.h b/src/intel/blorp/blorp.h
index 1e22712602d..da0c9ac205c 100644
--- a/src/intel/blorp/blorp.h
+++ b/src/intel/blorp/blorp.h
@@ -35,6 +35,7 @@  struct brw_stage_prog_data;
 extern "C" {
 #endif
 
+struct blorp_address;
 struct blorp_batch;
 struct blorp_params;
 
@@ -47,13 +48,15 @@  struct blorp_context {
 
    bool (*lookup_shader)(struct blorp_context *blorp,
                          const void *key, uint32_t key_size,
-                         uint32_t *kernel_out, void *prog_data_out);
+                         struct blorp_address *kernel_out,
+                         void *prog_data_out);
    bool (*upload_shader)(struct blorp_context *blorp,
                          const void *key, uint32_t key_size,
                          const void *kernel, uint32_t kernel_size,
                          const struct brw_stage_prog_data *prog_data,
                          uint32_t prog_data_size,
-                         uint32_t *kernel_out, void *prog_data_out);
+                         struct blorp_address *kernel_out,
+                         void *prog_data_out);
    void (*exec)(struct blorp_batch *batch, const struct blorp_params *params);
 };
 
diff --git a/src/intel/blorp/blorp_genX_exec.h b/src/intel/blorp/blorp_genX_exec.h
index 065980616ec..20f30c7116d 100644
--- a/src/intel/blorp/blorp_genX_exec.h
+++ b/src/intel/blorp/blorp_genX_exec.h
@@ -108,6 +108,13 @@  _blorp_combine_address(struct blorp_batch *batch, void *location,
    }
 }
 
+static uint64_t
+KSP(struct blorp_batch *batch, struct blorp_address address)
+{
+   assert(address.buffer == NULL);
+   return address.offset;
+}
+
 #define __gen_address_type struct blorp_address
 #define __gen_user_data struct blorp_batch
 #define __gen_combine_address _blorp_combine_address
@@ -615,7 +622,7 @@  blorp_emit_vs_config(struct blorp_batch *batch,
       if (vs_prog_data) {
          vs.Enable = true;
 
-         vs.KernelStartPointer = params->vs_prog_kernel;
+         vs.KernelStartPointer = KSP(batch, params->vs_prog_kernel);
 
          vs.DispatchGRFStartRegisterForURBData =
             vs_prog_data->base.base.dispatch_grf_start_reg;
@@ -795,11 +802,11 @@  blorp_emit_ps_config(struct blorp_batch *batch,
          ps.DispatchGRFStartRegisterForConstantSetupData2 =
             brw_wm_prog_data_dispatch_grf_start_reg(prog_data, ps, 2);
 
-         ps.KernelStartPointer0 = params->wm_prog_kernel +
+         ps.KernelStartPointer0 = KSP(batch, params->wm_prog_kernel) +
                                   brw_wm_prog_data_prog_offset(prog_data, ps, 0);
-         ps.KernelStartPointer1 = params->wm_prog_kernel +
+         ps.KernelStartPointer1 = KSP(batch, params->wm_prog_kernel) +
                                   brw_wm_prog_data_prog_offset(prog_data, ps, 1);
-         ps.KernelStartPointer2 = params->wm_prog_kernel +
+         ps.KernelStartPointer2 = KSP(batch, params->wm_prog_kernel) +
                                   brw_wm_prog_data_prog_offset(prog_data, ps, 2);
       }
 
@@ -905,11 +912,11 @@  blorp_emit_ps_config(struct blorp_batch *batch,
          ps.DispatchGRFStartRegisterForConstantSetupData2 =
             brw_wm_prog_data_dispatch_grf_start_reg(prog_data, ps, 2);
 
-         ps.KernelStartPointer0 = params->wm_prog_kernel +
+         ps.KernelStartPointer0 = KSP(batch, params->wm_prog_kernel) +
                                   brw_wm_prog_data_prog_offset(prog_data, ps, 0);
-         ps.KernelStartPointer1 = params->wm_prog_kernel +
+         ps.KernelStartPointer1 = KSP(batch, params->wm_prog_kernel) +
                                   brw_wm_prog_data_prog_offset(prog_data, ps, 1);
-         ps.KernelStartPointer2 = params->wm_prog_kernel +
+         ps.KernelStartPointer2 = KSP(batch, params->wm_prog_kernel) +
                                   brw_wm_prog_data_prog_offset(prog_data, ps, 2);
 
          ps.AttributeEnable = prog_data->num_varying_inputs > 0;
@@ -973,11 +980,11 @@  blorp_emit_ps_config(struct blorp_batch *batch,
          wm.DispatchGRFStartRegisterForConstantSetupData2 =
             brw_wm_prog_data_dispatch_grf_start_reg(prog_data, wm, 2);
 
-         wm.KernelStartPointer0 = params->wm_prog_kernel +
+         wm.KernelStartPointer0 = KSP(batch, params->wm_prog_kernel) +
                                   brw_wm_prog_data_prog_offset(prog_data, wm, 0);
-         wm.KernelStartPointer1 = params->wm_prog_kernel +
+         wm.KernelStartPointer1 = KSP(batch, params->wm_prog_kernel) +
                                   brw_wm_prog_data_prog_offset(prog_data, wm, 1);
-         wm.KernelStartPointer2 = params->wm_prog_kernel +
+         wm.KernelStartPointer2 = KSP(batch, params->wm_prog_kernel) +
                                   brw_wm_prog_data_prog_offset(prog_data, wm, 2);
 
          wm.NumberofSFOutputAttributes = prog_data->num_varying_inputs;
diff --git a/src/intel/blorp/blorp_priv.h b/src/intel/blorp/blorp_priv.h
index a6aa2aa4151..b60505ea06a 100644
--- a/src/intel/blorp/blorp_priv.h
+++ b/src/intel/blorp/blorp_priv.h
@@ -203,11 +203,11 @@  struct blorp_params
    unsigned num_samples;
    unsigned num_draw_buffers;
    unsigned num_layers;
-   uint32_t vs_prog_kernel;
+   struct blorp_address vs_prog_kernel;
    struct brw_vs_prog_data *vs_prog_data;
-   uint32_t sf_prog_kernel;
+   struct blorp_address sf_prog_kernel;
    struct brw_sf_prog_data *sf_prog_data;
-   uint32_t wm_prog_kernel;
+   struct blorp_address wm_prog_kernel;
    struct brw_wm_prog_data *wm_prog_data;
 
    bool use_pre_baked_binding_table;
diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
index 478b8e7a3db..e6bcf4b0bc1 100644
--- a/src/intel/vulkan/anv_blorp.c
+++ b/src/intel/vulkan/anv_blorp.c
@@ -26,7 +26,8 @@ 
 static bool
 lookup_blorp_shader(struct blorp_context *blorp,
                     const void *key, uint32_t key_size,
-                    uint32_t *kernel_out, void *prog_data_out)
+                    struct blorp_address *kernel_out,
+                    void *prog_data_out)
 {
    struct anv_device *device = blorp->driver_ctx;
 
@@ -43,7 +44,7 @@  lookup_blorp_shader(struct blorp_context *blorp,
     */
    anv_shader_bin_unref(device, bin);
 
-   *kernel_out = bin->kernel.offset;
+   *kernel_out = (struct blorp_address) { .offset = bin->kernel.offset };
    *(const struct brw_stage_prog_data **)prog_data_out = bin->prog_data;
 
    return true;
@@ -55,7 +56,8 @@  upload_blorp_shader(struct blorp_context *blorp,
                     const void *kernel, uint32_t kernel_size,
                     const struct brw_stage_prog_data *prog_data,
                     uint32_t prog_data_size,
-                    uint32_t *kernel_out, void *prog_data_out)
+                    struct blorp_address *kernel_out,
+                    void *prog_data_out)
 {
    struct anv_device *device = blorp->driver_ctx;
 
@@ -81,7 +83,7 @@  upload_blorp_shader(struct blorp_context *blorp,
     */
    anv_shader_bin_unref(device, bin);
 
-   *kernel_out = bin->kernel.offset;
+   *kernel_out = (struct blorp_address) { .offset = bin->kernel.offset };
    *(const struct brw_stage_prog_data **)prog_data_out = bin->prog_data;
 
    return true;
diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c
index b286b231537..d021e2906fe 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp.c
+++ b/src/mesa/drivers/dri/i965/brw_blorp.c
@@ -45,11 +45,16 @@ 
 static bool
 brw_blorp_lookup_shader(struct blorp_context *blorp,
                         const void *key, uint32_t key_size,
-                        uint32_t *kernel_out, void *prog_data_out)
+                        struct blorp_address *kernel_out,
+                        void *prog_data_out)
 {
    struct brw_context *brw = blorp->driver_ctx;
-   return brw_search_cache(&brw->cache, BRW_CACHE_BLORP_PROG, key, key_size,
-                           kernel_out, prog_data_out, true);
+   uint32_t offset;
+   bool found =
+      brw_search_cache(&brw->cache, BRW_CACHE_BLORP_PROG, key, key_size,
+                       &offset, prog_data_out, true);
+   *kernel_out = (struct blorp_address) { .offset = offset };
+   return found;
 }
 
 static bool
@@ -58,12 +63,15 @@  brw_blorp_upload_shader(struct blorp_context *blorp,
                         const void *kernel, uint32_t kernel_size,
                         const struct brw_stage_prog_data *prog_data,
                         uint32_t prog_data_size,
-                        uint32_t *kernel_out, void *prog_data_out)
+                        struct blorp_address *kernel_out,
+                        void *prog_data_out)
 {
    struct brw_context *brw = blorp->driver_ctx;
+   uint32_t offset;
    brw_upload_cache(&brw->cache, BRW_CACHE_BLORP_PROG, key, key_size,
                     kernel, kernel_size, prog_data, prog_data_size,
-                    kernel_out, prog_data_out);
+                    &offset, prog_data_out);
+   *kernel_out = (struct blorp_address) { .offset = offset };
    return true;
 }
 
diff --git a/src/mesa/drivers/dri/i965/gen4_blorp_exec.h b/src/mesa/drivers/dri/i965/gen4_blorp_exec.h
index 0edc518fa35..3b0f1ea0979 100644
--- a/src/mesa/drivers/dri/i965/gen4_blorp_exec.h
+++ b/src/mesa/drivers/dri/i965/gen4_blorp_exec.h
@@ -77,9 +77,9 @@  blorp_emit_sf_state(struct blorp_batch *batch,
    blorp_emit_dynamic(batch, GENX(SF_STATE), sf, 64, &offset) {
 #if GEN_GEN == 4
       sf.KernelStartPointer =
-         instruction_state_address(batch, params->sf_prog_kernel);
+         instruction_state_address(batch, KSP(batch, params->sf_prog_kernel));
 #else
-      sf.KernelStartPointer = params->sf_prog_kernel;
+      sf.KernelStartPointer = KSP(batch, params->sf_prog_kernel);
 #endif
       sf.GRFRegisterCount = DIV_ROUND_UP(prog_data->total_grf, 16) - 1;
       sf.VertexURBEntryReadLength = prog_data->urb_read_length;
@@ -136,14 +136,14 @@  blorp_emit_wm_state(struct blorp_batch *batch,
 
 #if GEN_GEN == 4
          wm.KernelStartPointer0 =
-            instruction_state_address(batch, params->wm_prog_kernel);
+            instruction_state_address(batch, KSP(batch, params->wm_prog_kernel));
          wm.GRFRegisterCount0 = brw_wm_prog_data_reg_blocks(prog_data, wm, 0);
 #else
-         wm.KernelStartPointer0 = params->wm_prog_kernel +
+         wm.KernelStartPointer0 = KSP(batch, params->wm_prog_kernel) +
                                   brw_wm_prog_data_prog_offset(prog_data, wm, 0);
-         wm.KernelStartPointer1 = params->wm_prog_kernel +
+         wm.KernelStartPointer1 = KSP(batch, params->wm_prog_kernel) +
                                   brw_wm_prog_data_prog_offset(prog_data, wm, 1);
-         wm.KernelStartPointer2 = params->wm_prog_kernel +
+         wm.KernelStartPointer2 = KSP(batch, params->wm_prog_kernel) +
                                   brw_wm_prog_data_prog_offset(prog_data, wm, 2);
          wm.GRFRegisterCount0 = brw_wm_prog_data_reg_blocks(prog_data, wm, 0);
          wm.GRFRegisterCount1 = brw_wm_prog_data_reg_blocks(prog_data, wm, 1);

Comments

I kind-of wonder if we want to allow for relocations and not surface state
base address in which case you'd want to do something "real" with offset.
Making this an address and then ignoring the buffer part entirely seems
like it promises a bit too much.  Then again, I don't think it's really
hurting anything.  Meh; this is probably fine.

On Thu, Nov 29, 2018 at 2:24 AM Kenneth Graunke <kenneth@whitecape.org>
wrote:

> In i965, shader programs live in a single buffer, and every batch emits
> a STATE_BASE_ADDRESS packet pointing to that buffer.  This takes care of
> pinning the buffer for the batch; from then on, we can use an offset.
>
> In the upcoming Iris driver, shader programs can live in multiple
> buffers, and those buffers need to be pinned when shader assembly is
> referenced.  To aid in this, we turn KSP into a blorp_address rather
> than a 32-bit offset.  This lets us also pass a buffer to pin.
>
> For now, we simply assert the BO is NULL and return the offset, as
> we did before - nothing should behave differently.
> ---
>  src/intel/blorp/blorp.h                     |  7 ++++--
>  src/intel/blorp/blorp_genX_exec.h           | 27 +++++++++++++--------
>  src/intel/blorp/blorp_priv.h                |  6 ++---
>  src/intel/vulkan/anv_blorp.c                | 10 +++++---
>  src/mesa/drivers/dri/i965/brw_blorp.c       | 18 ++++++++++----
>  src/mesa/drivers/dri/i965/gen4_blorp_exec.h | 12 ++++-----
>  6 files changed, 50 insertions(+), 30 deletions(-)
>
> diff --git a/src/intel/blorp/blorp.h b/src/intel/blorp/blorp.h
> index 1e22712602d..da0c9ac205c 100644
> --- a/src/intel/blorp/blorp.h
> +++ b/src/intel/blorp/blorp.h
> @@ -35,6 +35,7 @@ struct brw_stage_prog_data;
>  extern "C" {
>  #endif
>
> +struct blorp_address;
>  struct blorp_batch;
>  struct blorp_params;
>
> @@ -47,13 +48,15 @@ struct blorp_context {
>
>     bool (*lookup_shader)(struct blorp_context *blorp,
>                           const void *key, uint32_t key_size,
> -                         uint32_t *kernel_out, void *prog_data_out);
> +                         struct blorp_address *kernel_out,
> +                         void *prog_data_out);
>     bool (*upload_shader)(struct blorp_context *blorp,
>                           const void *key, uint32_t key_size,
>                           const void *kernel, uint32_t kernel_size,
>                           const struct brw_stage_prog_data *prog_data,
>                           uint32_t prog_data_size,
> -                         uint32_t *kernel_out, void *prog_data_out);
> +                         struct blorp_address *kernel_out,
> +                         void *prog_data_out);
>     void (*exec)(struct blorp_batch *batch, const struct blorp_params
> *params);
>  };
>
> diff --git a/src/intel/blorp/blorp_genX_exec.h
> b/src/intel/blorp/blorp_genX_exec.h
> index 065980616ec..20f30c7116d 100644
> --- a/src/intel/blorp/blorp_genX_exec.h
> +++ b/src/intel/blorp/blorp_genX_exec.h
> @@ -108,6 +108,13 @@ _blorp_combine_address(struct blorp_batch *batch,
> void *location,
>     }
>  }
>
> +static uint64_t
> +KSP(struct blorp_batch *batch, struct blorp_address address)
> +{
> +   assert(address.buffer == NULL);
> +   return address.offset;
> +}
> +
>  #define __gen_address_type struct blorp_address
>  #define __gen_user_data struct blorp_batch
>  #define __gen_combine_address _blorp_combine_address
> @@ -615,7 +622,7 @@ blorp_emit_vs_config(struct blorp_batch *batch,
>        if (vs_prog_data) {
>           vs.Enable = true;
>
> -         vs.KernelStartPointer = params->vs_prog_kernel;
> +         vs.KernelStartPointer = KSP(batch, params->vs_prog_kernel);
>
>           vs.DispatchGRFStartRegisterForURBData =
>              vs_prog_data->base.base.dispatch_grf_start_reg;
> @@ -795,11 +802,11 @@ blorp_emit_ps_config(struct blorp_batch *batch,
>           ps.DispatchGRFStartRegisterForConstantSetupData2 =
>              brw_wm_prog_data_dispatch_grf_start_reg(prog_data, ps, 2);
>
> -         ps.KernelStartPointer0 = params->wm_prog_kernel +
> +         ps.KernelStartPointer0 = KSP(batch, params->wm_prog_kernel) +
>                                    brw_wm_prog_data_prog_offset(prog_data,
> ps, 0);
> -         ps.KernelStartPointer1 = params->wm_prog_kernel +
> +         ps.KernelStartPointer1 = KSP(batch, params->wm_prog_kernel) +
>                                    brw_wm_prog_data_prog_offset(prog_data,
> ps, 1);
> -         ps.KernelStartPointer2 = params->wm_prog_kernel +
> +         ps.KernelStartPointer2 = KSP(batch, params->wm_prog_kernel) +
>                                    brw_wm_prog_data_prog_offset(prog_data,
> ps, 2);
>        }
>
> @@ -905,11 +912,11 @@ blorp_emit_ps_config(struct blorp_batch *batch,
>           ps.DispatchGRFStartRegisterForConstantSetupData2 =
>              brw_wm_prog_data_dispatch_grf_start_reg(prog_data, ps, 2);
>
> -         ps.KernelStartPointer0 = params->wm_prog_kernel +
> +         ps.KernelStartPointer0 = KSP(batch, params->wm_prog_kernel) +
>                                    brw_wm_prog_data_prog_offset(prog_data,
> ps, 0);
> -         ps.KernelStartPointer1 = params->wm_prog_kernel +
> +         ps.KernelStartPointer1 = KSP(batch, params->wm_prog_kernel) +
>                                    brw_wm_prog_data_prog_offset(prog_data,
> ps, 1);
> -         ps.KernelStartPointer2 = params->wm_prog_kernel +
> +         ps.KernelStartPointer2 = KSP(batch, params->wm_prog_kernel) +
>                                    brw_wm_prog_data_prog_offset(prog_data,
> ps, 2);
>
>           ps.AttributeEnable = prog_data->num_varying_inputs > 0;
> @@ -973,11 +980,11 @@ blorp_emit_ps_config(struct blorp_batch *batch,
>           wm.DispatchGRFStartRegisterForConstantSetupData2 =
>              brw_wm_prog_data_dispatch_grf_start_reg(prog_data, wm, 2);
>
> -         wm.KernelStartPointer0 = params->wm_prog_kernel +
> +         wm.KernelStartPointer0 = KSP(batch, params->wm_prog_kernel) +
>                                    brw_wm_prog_data_prog_offset(prog_data,
> wm, 0);
> -         wm.KernelStartPointer1 = params->wm_prog_kernel +
> +         wm.KernelStartPointer1 = KSP(batch, params->wm_prog_kernel) +
>                                    brw_wm_prog_data_prog_offset(prog_data,
> wm, 1);
> -         wm.KernelStartPointer2 = params->wm_prog_kernel +
> +         wm.KernelStartPointer2 = KSP(batch, params->wm_prog_kernel) +
>                                    brw_wm_prog_data_prog_offset(prog_data,
> wm, 2);
>
>           wm.NumberofSFOutputAttributes = prog_data->num_varying_inputs;
> diff --git a/src/intel/blorp/blorp_priv.h b/src/intel/blorp/blorp_priv.h
> index a6aa2aa4151..b60505ea06a 100644
> --- a/src/intel/blorp/blorp_priv.h
> +++ b/src/intel/blorp/blorp_priv.h
> @@ -203,11 +203,11 @@ struct blorp_params
>     unsigned num_samples;
>     unsigned num_draw_buffers;
>     unsigned num_layers;
> -   uint32_t vs_prog_kernel;
> +   struct blorp_address vs_prog_kernel;
>     struct brw_vs_prog_data *vs_prog_data;
> -   uint32_t sf_prog_kernel;
> +   struct blorp_address sf_prog_kernel;
>     struct brw_sf_prog_data *sf_prog_data;
> -   uint32_t wm_prog_kernel;
> +   struct blorp_address wm_prog_kernel;
>     struct brw_wm_prog_data *wm_prog_data;
>
>     bool use_pre_baked_binding_table;
> diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> index 478b8e7a3db..e6bcf4b0bc1 100644
> --- a/src/intel/vulkan/anv_blorp.c
> +++ b/src/intel/vulkan/anv_blorp.c
> @@ -26,7 +26,8 @@
>  static bool
>  lookup_blorp_shader(struct blorp_context *blorp,
>                      const void *key, uint32_t key_size,
> -                    uint32_t *kernel_out, void *prog_data_out)
> +                    struct blorp_address *kernel_out,
> +                    void *prog_data_out)
>  {
>     struct anv_device *device = blorp->driver_ctx;
>
> @@ -43,7 +44,7 @@ lookup_blorp_shader(struct blorp_context *blorp,
>      */
>     anv_shader_bin_unref(device, bin);
>
> -   *kernel_out = bin->kernel.offset;
> +   *kernel_out = (struct blorp_address) { .offset = bin->kernel.offset };
>     *(const struct brw_stage_prog_data **)prog_data_out = bin->prog_data;
>
>     return true;
> @@ -55,7 +56,8 @@ upload_blorp_shader(struct blorp_context *blorp,
>                      const void *kernel, uint32_t kernel_size,
>                      const struct brw_stage_prog_data *prog_data,
>                      uint32_t prog_data_size,
> -                    uint32_t *kernel_out, void *prog_data_out)
> +                    struct blorp_address *kernel_out,
> +                    void *prog_data_out)
>  {
>     struct anv_device *device = blorp->driver_ctx;
>
> @@ -81,7 +83,7 @@ upload_blorp_shader(struct blorp_context *blorp,
>      */
>     anv_shader_bin_unref(device, bin);
>
> -   *kernel_out = bin->kernel.offset;
> +   *kernel_out = (struct blorp_address) { .offset = bin->kernel.offset };
>     *(const struct brw_stage_prog_data **)prog_data_out = bin->prog_data;
>
>     return true;
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
> b/src/mesa/drivers/dri/i965/brw_blorp.c
> index b286b231537..d021e2906fe 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> @@ -45,11 +45,16 @@
>  static bool
>  brw_blorp_lookup_shader(struct blorp_context *blorp,
>                          const void *key, uint32_t key_size,
> -                        uint32_t *kernel_out, void *prog_data_out)
> +                        struct blorp_address *kernel_out,
> +                        void *prog_data_out)
>  {
>     struct brw_context *brw = blorp->driver_ctx;
> -   return brw_search_cache(&brw->cache, BRW_CACHE_BLORP_PROG, key,
> key_size,
> -                           kernel_out, prog_data_out, true);
> +   uint32_t offset;
> +   bool found =
> +      brw_search_cache(&brw->cache, BRW_CACHE_BLORP_PROG, key, key_size,
> +                       &offset, prog_data_out, true);
> +   *kernel_out = (struct blorp_address) { .offset = offset };
> +   return found;
>  }
>
>  static bool
> @@ -58,12 +63,15 @@ brw_blorp_upload_shader(struct blorp_context *blorp,
>                          const void *kernel, uint32_t kernel_size,
>                          const struct brw_stage_prog_data *prog_data,
>                          uint32_t prog_data_size,
> -                        uint32_t *kernel_out, void *prog_data_out)
> +                        struct blorp_address *kernel_out,
> +                        void *prog_data_out)
>  {
>     struct brw_context *brw = blorp->driver_ctx;
> +   uint32_t offset;
>     brw_upload_cache(&brw->cache, BRW_CACHE_BLORP_PROG, key, key_size,
>                      kernel, kernel_size, prog_data, prog_data_size,
> -                    kernel_out, prog_data_out);
> +                    &offset, prog_data_out);
> +   *kernel_out = (struct blorp_address) { .offset = offset };
>     return true;
>  }
>
> diff --git a/src/mesa/drivers/dri/i965/gen4_blorp_exec.h
> b/src/mesa/drivers/dri/i965/gen4_blorp_exec.h
> index 0edc518fa35..3b0f1ea0979 100644
> --- a/src/mesa/drivers/dri/i965/gen4_blorp_exec.h
> +++ b/src/mesa/drivers/dri/i965/gen4_blorp_exec.h
> @@ -77,9 +77,9 @@ blorp_emit_sf_state(struct blorp_batch *batch,
>     blorp_emit_dynamic(batch, GENX(SF_STATE), sf, 64, &offset) {
>  #if GEN_GEN == 4
>        sf.KernelStartPointer =
> -         instruction_state_address(batch, params->sf_prog_kernel);
> +         instruction_state_address(batch, KSP(batch,
> params->sf_prog_kernel));
>  #else
> -      sf.KernelStartPointer = params->sf_prog_kernel;
> +      sf.KernelStartPointer = KSP(batch, params->sf_prog_kernel);
>  #endif
>        sf.GRFRegisterCount = DIV_ROUND_UP(prog_data->total_grf, 16) - 1;
>        sf.VertexURBEntryReadLength = prog_data->urb_read_length;
> @@ -136,14 +136,14 @@ blorp_emit_wm_state(struct blorp_batch *batch,
>
>  #if GEN_GEN == 4
>           wm.KernelStartPointer0 =
> -            instruction_state_address(batch, params->wm_prog_kernel);
> +            instruction_state_address(batch, KSP(batch,
> params->wm_prog_kernel));
>           wm.GRFRegisterCount0 = brw_wm_prog_data_reg_blocks(prog_data,
> wm, 0);
>  #else
> -         wm.KernelStartPointer0 = params->wm_prog_kernel +
> +         wm.KernelStartPointer0 = KSP(batch, params->wm_prog_kernel) +
>                                    brw_wm_prog_data_prog_offset(prog_data,
> wm, 0);
> -         wm.KernelStartPointer1 = params->wm_prog_kernel +
> +         wm.KernelStartPointer1 = KSP(batch, params->wm_prog_kernel) +
>                                    brw_wm_prog_data_prog_offset(prog_data,
> wm, 1);
> -         wm.KernelStartPointer2 = params->wm_prog_kernel +
> +         wm.KernelStartPointer2 = KSP(batch, params->wm_prog_kernel) +
>                                    brw_wm_prog_data_prog_offset(prog_data,
> wm, 2);
>           wm.GRFRegisterCount0 = brw_wm_prog_data_reg_blocks(prog_data,
> wm, 0);
>           wm.GRFRegisterCount1 = brw_wm_prog_data_reg_blocks(prog_data,
> wm, 1);
> --
> 2.19.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
I somewhat recant my statements below.  Even in a driver that puts the full
address in the offset field, having the BO pointer may still be useful for
the purpose of adding it to a residency list somewhere.

On Fri, Dec 7, 2018 at 4:13 PM Jason Ekstrand <jason@jlekstrand.net> wrote:

> I kind-of wonder if we want to allow for relocations and not surface state
> base address in which case you'd want to do something "real" with offset.
> Making this an address and then ignoring the buffer part entirely seems
> like it promises a bit too much.  Then again, I don't think it's really
> hurting anything.  Meh; this is probably fine.
>
> On Thu, Nov 29, 2018 at 2:24 AM Kenneth Graunke <kenneth@whitecape.org>
> wrote:
>
>> In i965, shader programs live in a single buffer, and every batch emits
>> a STATE_BASE_ADDRESS packet pointing to that buffer.  This takes care of
>> pinning the buffer for the batch; from then on, we can use an offset.
>>
>> In the upcoming Iris driver, shader programs can live in multiple
>> buffers, and those buffers need to be pinned when shader assembly is
>> referenced.  To aid in this, we turn KSP into a blorp_address rather
>> than a 32-bit offset.  This lets us also pass a buffer to pin.
>>
>> For now, we simply assert the BO is NULL and return the offset, as
>> we did before - nothing should behave differently.
>> ---
>>  src/intel/blorp/blorp.h                     |  7 ++++--
>>  src/intel/blorp/blorp_genX_exec.h           | 27 +++++++++++++--------
>>  src/intel/blorp/blorp_priv.h                |  6 ++---
>>  src/intel/vulkan/anv_blorp.c                | 10 +++++---
>>  src/mesa/drivers/dri/i965/brw_blorp.c       | 18 ++++++++++----
>>  src/mesa/drivers/dri/i965/gen4_blorp_exec.h | 12 ++++-----
>>  6 files changed, 50 insertions(+), 30 deletions(-)
>>
>> diff --git a/src/intel/blorp/blorp.h b/src/intel/blorp/blorp.h
>> index 1e22712602d..da0c9ac205c 100644
>> --- a/src/intel/blorp/blorp.h
>> +++ b/src/intel/blorp/blorp.h
>> @@ -35,6 +35,7 @@ struct brw_stage_prog_data;
>>  extern "C" {
>>  #endif
>>
>> +struct blorp_address;
>>  struct blorp_batch;
>>  struct blorp_params;
>>
>> @@ -47,13 +48,15 @@ struct blorp_context {
>>
>>     bool (*lookup_shader)(struct blorp_context *blorp,
>>                           const void *key, uint32_t key_size,
>> -                         uint32_t *kernel_out, void *prog_data_out);
>> +                         struct blorp_address *kernel_out,
>> +                         void *prog_data_out);
>>     bool (*upload_shader)(struct blorp_context *blorp,
>>                           const void *key, uint32_t key_size,
>>                           const void *kernel, uint32_t kernel_size,
>>                           const struct brw_stage_prog_data *prog_data,
>>                           uint32_t prog_data_size,
>> -                         uint32_t *kernel_out, void *prog_data_out);
>> +                         struct blorp_address *kernel_out,
>> +                         void *prog_data_out);
>>     void (*exec)(struct blorp_batch *batch, const struct blorp_params
>> *params);
>>  };
>>
>> diff --git a/src/intel/blorp/blorp_genX_exec.h
>> b/src/intel/blorp/blorp_genX_exec.h
>> index 065980616ec..20f30c7116d 100644
>> --- a/src/intel/blorp/blorp_genX_exec.h
>> +++ b/src/intel/blorp/blorp_genX_exec.h
>> @@ -108,6 +108,13 @@ _blorp_combine_address(struct blorp_batch *batch,
>> void *location,
>>     }
>>  }
>>
>> +static uint64_t
>> +KSP(struct blorp_batch *batch, struct blorp_address address)
>> +{
>> +   assert(address.buffer == NULL);
>> +   return address.offset;
>> +}
>> +
>>  #define __gen_address_type struct blorp_address
>>  #define __gen_user_data struct blorp_batch
>>  #define __gen_combine_address _blorp_combine_address
>> @@ -615,7 +622,7 @@ blorp_emit_vs_config(struct blorp_batch *batch,
>>        if (vs_prog_data) {
>>           vs.Enable = true;
>>
>> -         vs.KernelStartPointer = params->vs_prog_kernel;
>> +         vs.KernelStartPointer = KSP(batch, params->vs_prog_kernel);
>>
>>           vs.DispatchGRFStartRegisterForURBData =
>>              vs_prog_data->base.base.dispatch_grf_start_reg;
>> @@ -795,11 +802,11 @@ blorp_emit_ps_config(struct blorp_batch *batch,
>>           ps.DispatchGRFStartRegisterForConstantSetupData2 =
>>              brw_wm_prog_data_dispatch_grf_start_reg(prog_data, ps, 2);
>>
>> -         ps.KernelStartPointer0 = params->wm_prog_kernel +
>> +         ps.KernelStartPointer0 = KSP(batch, params->wm_prog_kernel) +
>>
>>  brw_wm_prog_data_prog_offset(prog_data, ps, 0);
>> -         ps.KernelStartPointer1 = params->wm_prog_kernel +
>> +         ps.KernelStartPointer1 = KSP(batch, params->wm_prog_kernel) +
>>
>>  brw_wm_prog_data_prog_offset(prog_data, ps, 1);
>> -         ps.KernelStartPointer2 = params->wm_prog_kernel +
>> +         ps.KernelStartPointer2 = KSP(batch, params->wm_prog_kernel) +
>>
>>  brw_wm_prog_data_prog_offset(prog_data, ps, 2);
>>        }
>>
>> @@ -905,11 +912,11 @@ blorp_emit_ps_config(struct blorp_batch *batch,
>>           ps.DispatchGRFStartRegisterForConstantSetupData2 =
>>              brw_wm_prog_data_dispatch_grf_start_reg(prog_data, ps, 2);
>>
>> -         ps.KernelStartPointer0 = params->wm_prog_kernel +
>> +         ps.KernelStartPointer0 = KSP(batch, params->wm_prog_kernel) +
>>
>>  brw_wm_prog_data_prog_offset(prog_data, ps, 0);
>> -         ps.KernelStartPointer1 = params->wm_prog_kernel +
>> +         ps.KernelStartPointer1 = KSP(batch, params->wm_prog_kernel) +
>>
>>  brw_wm_prog_data_prog_offset(prog_data, ps, 1);
>> -         ps.KernelStartPointer2 = params->wm_prog_kernel +
>> +         ps.KernelStartPointer2 = KSP(batch, params->wm_prog_kernel) +
>>
>>  brw_wm_prog_data_prog_offset(prog_data, ps, 2);
>>
>>           ps.AttributeEnable = prog_data->num_varying_inputs > 0;
>> @@ -973,11 +980,11 @@ blorp_emit_ps_config(struct blorp_batch *batch,
>>           wm.DispatchGRFStartRegisterForConstantSetupData2 =
>>              brw_wm_prog_data_dispatch_grf_start_reg(prog_data, wm, 2);
>>
>> -         wm.KernelStartPointer0 = params->wm_prog_kernel +
>> +         wm.KernelStartPointer0 = KSP(batch, params->wm_prog_kernel) +
>>
>>  brw_wm_prog_data_prog_offset(prog_data, wm, 0);
>> -         wm.KernelStartPointer1 = params->wm_prog_kernel +
>> +         wm.KernelStartPointer1 = KSP(batch, params->wm_prog_kernel) +
>>
>>  brw_wm_prog_data_prog_offset(prog_data, wm, 1);
>> -         wm.KernelStartPointer2 = params->wm_prog_kernel +
>> +         wm.KernelStartPointer2 = KSP(batch, params->wm_prog_kernel) +
>>
>>  brw_wm_prog_data_prog_offset(prog_data, wm, 2);
>>
>>           wm.NumberofSFOutputAttributes = prog_data->num_varying_inputs;
>> diff --git a/src/intel/blorp/blorp_priv.h b/src/intel/blorp/blorp_priv.h
>> index a6aa2aa4151..b60505ea06a 100644
>> --- a/src/intel/blorp/blorp_priv.h
>> +++ b/src/intel/blorp/blorp_priv.h
>> @@ -203,11 +203,11 @@ struct blorp_params
>>     unsigned num_samples;
>>     unsigned num_draw_buffers;
>>     unsigned num_layers;
>> -   uint32_t vs_prog_kernel;
>> +   struct blorp_address vs_prog_kernel;
>>     struct brw_vs_prog_data *vs_prog_data;
>> -   uint32_t sf_prog_kernel;
>> +   struct blorp_address sf_prog_kernel;
>>     struct brw_sf_prog_data *sf_prog_data;
>> -   uint32_t wm_prog_kernel;
>> +   struct blorp_address wm_prog_kernel;
>>     struct brw_wm_prog_data *wm_prog_data;
>>
>>     bool use_pre_baked_binding_table;
>> diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
>> index 478b8e7a3db..e6bcf4b0bc1 100644
>> --- a/src/intel/vulkan/anv_blorp.c
>> +++ b/src/intel/vulkan/anv_blorp.c
>> @@ -26,7 +26,8 @@
>>  static bool
>>  lookup_blorp_shader(struct blorp_context *blorp,
>>                      const void *key, uint32_t key_size,
>> -                    uint32_t *kernel_out, void *prog_data_out)
>> +                    struct blorp_address *kernel_out,
>> +                    void *prog_data_out)
>>  {
>>     struct anv_device *device = blorp->driver_ctx;
>>
>> @@ -43,7 +44,7 @@ lookup_blorp_shader(struct blorp_context *blorp,
>>      */
>>     anv_shader_bin_unref(device, bin);
>>
>> -   *kernel_out = bin->kernel.offset;
>> +   *kernel_out = (struct blorp_address) { .offset = bin->kernel.offset };
>>     *(const struct brw_stage_prog_data **)prog_data_out = bin->prog_data;
>>
>>     return true;
>> @@ -55,7 +56,8 @@ upload_blorp_shader(struct blorp_context *blorp,
>>                      const void *kernel, uint32_t kernel_size,
>>                      const struct brw_stage_prog_data *prog_data,
>>                      uint32_t prog_data_size,
>> -                    uint32_t *kernel_out, void *prog_data_out)
>> +                    struct blorp_address *kernel_out,
>> +                    void *prog_data_out)
>>  {
>>     struct anv_device *device = blorp->driver_ctx;
>>
>> @@ -81,7 +83,7 @@ upload_blorp_shader(struct blorp_context *blorp,
>>      */
>>     anv_shader_bin_unref(device, bin);
>>
>> -   *kernel_out = bin->kernel.offset;
>> +   *kernel_out = (struct blorp_address) { .offset = bin->kernel.offset };
>>     *(const struct brw_stage_prog_data **)prog_data_out = bin->prog_data;
>>
>>     return true;
>> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
>> b/src/mesa/drivers/dri/i965/brw_blorp.c
>> index b286b231537..d021e2906fe 100644
>> --- a/src/mesa/drivers/dri/i965/brw_blorp.c
>> +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
>> @@ -45,11 +45,16 @@
>>  static bool
>>  brw_blorp_lookup_shader(struct blorp_context *blorp,
>>                          const void *key, uint32_t key_size,
>> -                        uint32_t *kernel_out, void *prog_data_out)
>> +                        struct blorp_address *kernel_out,
>> +                        void *prog_data_out)
>>  {
>>     struct brw_context *brw = blorp->driver_ctx;
>> -   return brw_search_cache(&brw->cache, BRW_CACHE_BLORP_PROG, key,
>> key_size,
>> -                           kernel_out, prog_data_out, true);
>> +   uint32_t offset;
>> +   bool found =
>> +      brw_search_cache(&brw->cache, BRW_CACHE_BLORP_PROG, key, key_size,
>> +                       &offset, prog_data_out, true);
>> +   *kernel_out = (struct blorp_address) { .offset = offset };
>> +   return found;
>>  }
>>
>>  static bool
>> @@ -58,12 +63,15 @@ brw_blorp_upload_shader(struct blorp_context *blorp,
>>                          const void *kernel, uint32_t kernel_size,
>>                          const struct brw_stage_prog_data *prog_data,
>>                          uint32_t prog_data_size,
>> -                        uint32_t *kernel_out, void *prog_data_out)
>> +                        struct blorp_address *kernel_out,
>> +                        void *prog_data_out)
>>  {
>>     struct brw_context *brw = blorp->driver_ctx;
>> +   uint32_t offset;
>>     brw_upload_cache(&brw->cache, BRW_CACHE_BLORP_PROG, key, key_size,
>>                      kernel, kernel_size, prog_data, prog_data_size,
>> -                    kernel_out, prog_data_out);
>> +                    &offset, prog_data_out);
>> +   *kernel_out = (struct blorp_address) { .offset = offset };
>>     return true;
>>  }
>>
>> diff --git a/src/mesa/drivers/dri/i965/gen4_blorp_exec.h
>> b/src/mesa/drivers/dri/i965/gen4_blorp_exec.h
>> index 0edc518fa35..3b0f1ea0979 100644
>> --- a/src/mesa/drivers/dri/i965/gen4_blorp_exec.h
>> +++ b/src/mesa/drivers/dri/i965/gen4_blorp_exec.h
>> @@ -77,9 +77,9 @@ blorp_emit_sf_state(struct blorp_batch *batch,
>>     blorp_emit_dynamic(batch, GENX(SF_STATE), sf, 64, &offset) {
>>  #if GEN_GEN == 4
>>        sf.KernelStartPointer =
>> -         instruction_state_address(batch, params->sf_prog_kernel);
>> +         instruction_state_address(batch, KSP(batch,
>> params->sf_prog_kernel));
>>  #else
>> -      sf.KernelStartPointer = params->sf_prog_kernel;
>> +      sf.KernelStartPointer = KSP(batch, params->sf_prog_kernel);
>>  #endif
>>        sf.GRFRegisterCount = DIV_ROUND_UP(prog_data->total_grf, 16) - 1;
>>        sf.VertexURBEntryReadLength = prog_data->urb_read_length;
>> @@ -136,14 +136,14 @@ blorp_emit_wm_state(struct blorp_batch *batch,
>>
>>  #if GEN_GEN == 4
>>           wm.KernelStartPointer0 =
>> -            instruction_state_address(batch, params->wm_prog_kernel);
>> +            instruction_state_address(batch, KSP(batch,
>> params->wm_prog_kernel));
>>           wm.GRFRegisterCount0 = brw_wm_prog_data_reg_blocks(prog_data,
>> wm, 0);
>>  #else
>> -         wm.KernelStartPointer0 = params->wm_prog_kernel +
>> +         wm.KernelStartPointer0 = KSP(batch, params->wm_prog_kernel) +
>>
>>  brw_wm_prog_data_prog_offset(prog_data, wm, 0);
>> -         wm.KernelStartPointer1 = params->wm_prog_kernel +
>> +         wm.KernelStartPointer1 = KSP(batch, params->wm_prog_kernel) +
>>
>>  brw_wm_prog_data_prog_offset(prog_data, wm, 1);
>> -         wm.KernelStartPointer2 = params->wm_prog_kernel +
>> +         wm.KernelStartPointer2 = KSP(batch, params->wm_prog_kernel) +
>>
>>  brw_wm_prog_data_prog_offset(prog_data, wm, 2);
>>           wm.GRFRegisterCount0 = brw_wm_prog_data_reg_blocks(prog_data,
>> wm, 0);
>>           wm.GRFRegisterCount1 = brw_wm_prog_data_reg_blocks(prog_data,
>> wm, 1);
>> --
>> 2.19.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>