[Mesa-dev] [PATCH resend 5/7] i965: Upload binding tables in hw-generated binding table format.

[Kenneth Graunke]

On Monday, June 01, 2015 03:14:28 PM Abdiel Janulgue wrote:
> When hardware-generated binding tables are enabled, use the hw-generated
> binding table format when uploading binding table state.
> 
> Normally, the CS will will just consume the binding table pointer commands
> as pipelined state. When the RS is enabled however, the RS flushes whatever
> edited surface state entries of our on-chip binding table to the binding
> table pool before passing the command on to the CS.
> 
> Note that the the binding table pointer offset is relative to the binding table
> pool base address when resource streamer instead of the surface state base address.
> 
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_binding_tables.c | 27 ++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_binding_tables.c b/src/mesa/drivers/dri/i965/brw_binding_tables.c
> index ddf68fe..d436c43 100644
> --- a/src/mesa/drivers/dri/i965/brw_binding_tables.c
> +++ b/src/mesa/drivers/dri/i965/brw_binding_tables.c
> @@ -86,7 +86,12 @@ brw_upload_binding_table(struct brw_context *brw,
>           return;
>  
>        stage_state->bind_bo_offset = 0;
> -   } else {
> +   }
> +
> +   /* If resource streamer is enabled, skip manual binding table upload */
> +   if (!brw->hw_bt_pool.bo) {
> +      /* CACHE_NEW_*_PROG */

That was renamed in November, you mean /* BRW_NEW_*_PROG_DATA */.
Seems odd to put this here though - putting at the top of the function
would make more sense, as it's the first use of prog_data.  But that's
also a separate change.

> +
>        /* Upload a new binding table. */
>        if (INTEL_DEBUG & DEBUG_SHADER_TIME) {
>           brw->vtbl.emit_buffer_surface_state(
> @@ -103,15 +108,33 @@ brw_upload_binding_table(struct brw_context *brw,
>        /* BRW_NEW_SURFACES and BRW_NEW_*_CONSTBUF */
>        memcpy(bind, stage_state->surf_offset,
>               prog_data->binding_table.size_bytes);
> +   } else {
> +      gen7_update_binding_table_from_array(brw, stage_state->stage,
> +                                           stage_state->surf_offset,
> +                                           prog_data->binding_table.size_bytes /
> +                                           4);
>     }
>  
>     brw->ctx.NewDriverState |= brw_new_binding_table;
>  
>     if (brw->gen >= 7) {
> +
> +      if (brw->has_resource_streamer)
> +         stage_state->bind_bo_offset = brw->hw_bt_pool.next_offset;
> +
>        BEGIN_BATCH(2);
>        OUT_BATCH(packet_name << 16 | (2 - 2));
> -      OUT_BATCH(stage_state->bind_bo_offset);
> +      /* Align SurfaceStateOffset[16:6] format to [15:5] PS Binding Table field
> +       * when hw-generated binding table is enabled.
> +       */
> +      OUT_BATCH(brw->has_resource_streamer ?
> +                (stage_state->bind_bo_offset >> 1) :
> +                stage_state->bind_bo_offset);
>        ADVANCE_BATCH();
> +

Might be nice to cite the 3DSTATE_BINDING_TABLE_POINTERS_XS
documentation that says "If HW Binding Table is enabled [...] the
alignment is 64B."

> +      if (brw->has_resource_streamer)
> +         brw->hw_bt_pool.next_offset += ALIGN(prog_data->binding_table.size_bytes,
> +                                              64);
>     }
>  }

As I mentioned in my reply to patch 3, I think you need some sort of
"did the buffer fill up?" check.  Perhaps make a function to reserve
space, advancing next_offset and returning the offset to use:

static uint32_t
reserve_hw_bt_space(struct brw_context *brw, unsigned bytes)
{
   if (brw->hw_bt_pool.next_offset + bytes >= hw_bt_pool.bo->size) {
      gen7_reset_hw_bt_pool_offsets();
   }

   uint32_t offset = brw->hw_bt_pool.next_offset;

   brw->hw_bt_pool.next_offset += ALIGN(bytes, 64);

   return offset;
}

That would reduce the number of if (brw->has_resource_streamer) checks a
bit too...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150602/6736daea/attachment.sig>