[Mesa-dev] [PATCH 06/20] i965: Assign hw-binding table index for each UBO constant buffer.

[Francisco Jerez]

Abdiel Janulgue <abdiel.janulgue at linux.intel.com> writes:

> To be able to refer to a constant buffer, the resource streamer needs
> to index it with a hardware binding table entry. This blankets the ubo
> buffers with hardware binding table indices.
>
> Gather constants hardware fetches in 16-entry binding table blocks.
> So we need to use a block that is unused.
>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_context.h          | 11 +++++++++++
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c |  6 ++++++
>  2 files changed, 17 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index de0db5a..58edaf4 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -762,6 +762,17 @@ struct brw_vs_prog_data {
>  
>  #define SURF_INDEX_GEN6_SOL_BINDING(t) (t)
>  
> +/** Start of hardware binding table index for uniform gather constant entries.
> + *  This must be aligned to the start of a hardware binding table block (a block
> + *  is a group 16 binding table entries).
> + */
> +#define BRW_UNIFORM_GATHER_INDEX_START 32
> +
Does this mean that you use a fixed range of binding table indices (32
to 47) for gather?  Isn't that going to clash with entries allocated for
other surfaces like textures and render buffers if there are enough of
them?  Shouldn't you allocate this block dynamically as is done for
other surfaces in backend_shader::assign_common_binding_table_offsets?

I suggest you allocate the binding table block used for gather before
all other surfaces (i.e. at index 0) so you don't need to waste entries
as padding in order to guarantee that the block has the expected
alignment.

> +/** Appended to the end of the binding table index for uniform constant buffers
> + *  to indicate start of the UBO gather constant binding table.
> + */
> +#define BRW_UBO_GATHER_INDEX_APPEND 2

What does this do?  Two entries of the gather block of surfaces are
reserved for something?  What for?

> +
>  /* Note: brw_gs_prog_data_compare() must be updated when adding fields to
>   * this struct!
>   */
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index 8213f4e..fab553b 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -917,6 +917,12 @@ brw_upload_ubo_surfaces(struct brw_context *brw,
>                                    bo->size - binding->Offset,
>                                    &surf_offsets[i],
>                                    dword_pitch);
> +      if (brw->gather_pool.bo) {
> +         int bt_idx = BRW_UNIFORM_GATHER_INDEX_START +
> +            BRW_UBO_GATHER_INDEX_APPEND + i;
> +         gen7_edit_hw_binding_table_entry(brw, stage_state->stage,
> +                                          bt_idx, surf_offsets[i]);

Is there any reason you need to edit the binding table at this point?
Couldn't you just assign the surface state offsets to the entries
allocated for gather in the brw_stage_state::surf_offset array, so that
they're uploaded in one block together with the other surface state
entries in the binding table by brw_upload_binding_table()?

> +      }
>     }
>  
>     if (shader->NumUniformBlocks)
> -- 
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151023/5f2de912/attachment.sig>