[Mesa-dev] [PATCH 2/3] intel/blorp: Make KSP a blorp_address instead of an offset.
Jason Ekstrand
jason at jlekstrand.net
Fri Dec 7 22:38:28 UTC 2018
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 at 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 at 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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181207/b5f01d50/attachment-0001.html>
More information about the mesa-dev
mailing list