[Mesa-dev] [PATCH] i965: make emit_dwords non-static and reuse it in blorp

[Rafael Antognolli]

On Fri, May 05, 2017 at 11:47:09AM +0100, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov at collabora.com>
> 
> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> ---
>  src/mesa/drivers/dri/i965/brw_state.h         |  1 +
>  src/mesa/drivers/dri/i965/genX_blorp_exec.c   |  6 +----
>  src/mesa/drivers/dri/i965/genX_state_upload.c | 32 +++++++++++++--------------
>  3 files changed, 18 insertions(+), 21 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_state.h b/src/mesa/drivers/dri/i965/brw_state.h
> index d2d3d7c2c0d..06052fc9b02 100644
> --- a/src/mesa/drivers/dri/i965/brw_state.h
> +++ b/src/mesa/drivers/dri/i965/brw_state.h
> @@ -357,6 +357,7 @@ void brw_copy_pipeline_atoms(struct brw_context *brw,
>                               enum brw_pipeline pipeline,
>                               const struct brw_tracked_state **atoms,
>                               int num_atoms);
> +void *brw_emit_dwords(struct brw_context *brw, unsigned n);
>  void gen4_init_atoms(struct brw_context *brw);
>  void gen45_init_atoms(struct brw_context *brw);
>  void gen5_init_atoms(struct brw_context *brw);
> diff --git a/src/mesa/drivers/dri/i965/genX_blorp_exec.c b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> index 7157420328f..0a93c31334a 100644
> --- a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> +++ b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> @@ -40,11 +40,7 @@ blorp_emit_dwords(struct blorp_batch *batch, unsigned n)
>     assert(batch->blorp->driver_ctx == batch->driver_batch);
>     struct brw_context *brw = batch->driver_batch;
>  
> -   intel_batchbuffer_begin(brw, n, RENDER_RING);
> -   uint32_t *map = brw->batch.map_next;
> -   brw->batch.map_next += n;
> -   intel_batchbuffer_advance(brw);
> -   return map;
> +   return brw_emit_dwords(brw, n);
>  }
>  
>  static uint64_t
> diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c
> index ceeacfcdef7..4e8cf4f6b06 100644
> --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
> @@ -55,8 +55,8 @@
>  #include "main/transformfeedback.h"
>  #include "main/viewport.h"
>  
> -UNUSED static void *
> -emit_dwords(struct brw_context *brw, unsigned n)
> +void *
> +brw_emit_dwords(struct brw_context *brw, unsigned n)
>  {

Hi Emil,

This is going to cause several brw_emit_dwords() symbols to be exposed
(one for gen4, one for gen45, one for gen5,... one for gen9). And
consequently, the build fails because of the multiple definitions.

Assuming you want to really expose this function several times, you
should use genX(emit_dwords)(...), which would generate
gen4_emit_dwords() and all the others.

That said, imho it would be better to move this function outside of this
file and have only one version of it.

Rafael

>     intel_batchbuffer_begin(brw, n, RENDER_RING);
>     uint32_t *map = brw->batch.map_next;
> @@ -149,22 +149,22 @@ vertex_bo(struct brw_bo *bo, uint32_t offset)
>  #define _brw_cmd_header(cmd) cmd ## _header
>  #define _brw_cmd_pack(cmd) cmd ## _pack
>  
> -#define brw_batch_emit(brw, cmd, name)                  \
> -   for (struct cmd name = { _brw_cmd_header(cmd) },     \
> -        *_dst = emit_dwords(brw, _brw_cmd_length(cmd)); \
> -        __builtin_expect(_dst != NULL, 1);              \
> -        _brw_cmd_pack(cmd)(brw, (void *)_dst, &name),   \
> +#define brw_batch_emit(brw, cmd, name)                      \
> +   for (struct cmd name = { _brw_cmd_header(cmd) },         \
> +        *_dst = brw_emit_dwords(brw, _brw_cmd_length(cmd)); \
> +        __builtin_expect(_dst != NULL, 1);                  \
> +        _brw_cmd_pack(cmd)(brw, (void *)_dst, &name),       \
>          _dst = NULL)
>  
> -#define brw_batch_emitn(brw, cmd, n, ...) ({           \
> -      uint32_t *_dw = emit_dwords(brw, n);             \
> -      struct cmd template = {                          \
> -         _brw_cmd_header(cmd),                         \
> -         .DWordLength = n - _brw_cmd_length_bias(cmd), \
> -         __VA_ARGS__                                   \
> -      };                                               \
> -      _brw_cmd_pack(cmd)(brw, _dw, &template);         \
> -      _dw + 1; /* Array starts at dw[1] */             \
> +#define brw_batch_emitn(brw, cmd, n, ...) ({                \
> +      uint32_t *_dw = brw_emit_dwords(brw, n);              \
> +      struct cmd template = {                               \
> +         _brw_cmd_header(cmd),                              \
> +         .DWordLength = n - _brw_cmd_length_bias(cmd),      \
> +         __VA_ARGS__                                        \
> +      };                                                    \
> +      _brw_cmd_pack(cmd)(brw, _dw, &template);              \
> +      _dw + 1; /* Array starts at dw[1] */                  \
>     })
>  
>  #define brw_state_emit(brw, cmd, align, offset, name)              \
> -- 
> 2.12.2
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev