[Mesa-dev] [PATCH 4/5] r600g/compute: Use a constant buffer to store kernel parameters

[Marek Olšák]

On Tue, Apr 23, 2013 at 5:24 AM, Tom Stellard <tom at stellard.net> wrote:
> From: Tom Stellard <thomas.stellard at amd.com>
>
> ---
>  src/gallium/drivers/r600/evergreen_compute.c       | 47 +++++++++++++++-------
>  .../drivers/r600/evergreen_compute_internal.h      |  2 +-
>  2 files changed, 33 insertions(+), 16 deletions(-)
>
> diff --git a/src/gallium/drivers/r600/evergreen_compute.c b/src/gallium/drivers/r600/evergreen_compute.c
> index 05e5e3d..9d3c1d9 100644
> --- a/src/gallium/drivers/r600/evergreen_compute.c
> +++ b/src/gallium/drivers/r600/evergreen_compute.c
> @@ -102,6 +102,25 @@ static void evergreen_cs_set_vertex_buffer(
>         state->atom.dirty = true;
>  }
>
> +static void evergreen_cs_set_constant_buffer(
> +       struct r600_context * rctx,
> +       unsigned cb_index,
> +       unsigned offset,
> +       unsigned size,
> +       struct pipe_resource * buffer)
> +

Superfluous empty line?

> +{
> +       struct r600_constbuf_state *state =
> +                               &rctx->constbuf_state[PIPE_SHADER_COMPUTE];
> +       struct pipe_constant_buffer *cb = &state->cb[cb_index];
> +       cb->buffer_size = size;
> +       cb->buffer_offset = offset;
> +       cb->buffer = buffer;
> +       cb->user_buffer = NULL;
> +
> +       rctx->context.set_constant_buffer(&rctx->context, PIPE_SHADER_COMPUTE, cb_index, cb);

I don't think this will reference the resource properly in
set_constant_buffer and it's wrong usage anyway, because
set_constant_buffer is responsible for updating the r600 context
state, not the caller. There should just be a local variable "struct
pipe_constant_buffer cb;" which is passed to set_constant_buffer.

> +}
> +
>  static const struct u_resource_vtbl r600_global_buffer_vtbl =
>  {
>         u_default_resource_get_handle, /* get_handle */
> @@ -189,7 +208,10 @@ void evergreen_compute_upload_input(
>         struct r600_context *ctx = (struct r600_context *)ctx_;
>         struct r600_pipe_compute *shader = ctx->cs_shader_state.shader;
>         int i;
> -       unsigned kernel_parameters_offset_bytes = 36;
> +       /* We need to reserve 8 dwords (36 bytes) for implicit kernel
> +        * parameters.
> +        */
> +       unsigned input_size = shader->input_size + 36;

8 dwords is 32 bytes, not 36. :)

Marek