[Mesa-dev] [PATCH 3/3] r600g: handle unbind of constant buffers

[Keith Whitwell]

Dave,

Sorry for being confusing, but this patch doesn't actually work - this
is what I think should be happening, but at the moment I have to
return early to avoid segfaults on unbinding constant buffers, ie:

+       if (buffer == NULL) {
+                return;
+        }
+        else {
                width = buffer->width0;
                offset = r600_bo_offset(rbuffer->bo);
                bo = rbuffer->bo;


Otherwise I end up with a crash here:

#0  r600_context_block_emit_dirty (ctx=0x837cc20, draw=0xbffff1c4)
    at src/gallium/winsys/r600/drm/r600_priv.h:153
153                             r600_context_bo_reloc(ctx,
(gdb) list
148
149             for (int j = 0; j < block->nreg; j++) {
150                     if (block->pm4_bo_index[j]) {
151                             /* find relocation */
152                             id = block->pm4_bo_index[j];
153                             r600_context_bo_reloc(ctx,
154
&block->pm4[block->reloc[id].bo_pm4_index],
155                                             block->reloc[id].bo);
156                             r600_context_bo_flush(ctx,
157                                             block->reloc[id].flush_flags,


I haven't really figured out the state emit mechanism in this driver
yet...  Have you got any guidance what needs to be done here?

Keith


On Thu, Oct 14, 2010 at 4:42 PM, Keith Whitwell <keithw at vmware.com> wrote:
> Statetrackers can unbind a constant buffer slot by calling
>
>   pipe->set_constant_buffer(pipe, shader, slot, NULL)
>
> The driver should unbind the buffer and potentially allow its storage
> to be released.
> ---
>  src/gallium/drivers/r600/r600_state.c |   20 ++++++++++++++++----
>  1 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/src/gallium/drivers/r600/r600_state.c b/src/gallium/drivers/r600/r600_state.c
> index 2c0a200..b5ae187 100644
> --- a/src/gallium/drivers/r600/r600_state.c
> +++ b/src/gallium/drivers/r600/r600_state.c
> @@ -1160,28 +1160,40 @@ static void r600_set_constant_buffer(struct pipe_context *ctx, uint shader, uint
>  {
>        struct r600_pipe_context *rctx = (struct r600_pipe_context *)ctx;
>        struct r600_resource *rbuffer = (struct r600_resource*)buffer;
> +       unsigned width = 0;
> +       unsigned offset = 0;
> +       struct r600_bo *bo = NULL;
> +
> +       /* Note that the state tracker can unbind constant buffers by
> +        * passing NULL here.
> +        */
> +       if (buffer != NULL) {
> +               width = buffer->width0;
> +               offset = r600_bo_offset(rbuffer->bo);
> +               bo = rbuffer->bo;
> +       }
>
>        switch (shader) {
>        case PIPE_SHADER_VERTEX:
>                rctx->vs_const_buffer.nregs = 0;
>                r600_pipe_state_add_reg(&rctx->vs_const_buffer,
>                                        R_028180_ALU_CONST_BUFFER_SIZE_VS_0,
> -                                       ALIGN_DIVUP(buffer->width0 >> 4, 16),
> +                                       ALIGN_DIVUP(width >> 4, 16),
>                                        0xFFFFFFFF, NULL);
>                r600_pipe_state_add_reg(&rctx->vs_const_buffer,
>                                        R_028980_ALU_CONST_CACHE_VS_0,
> -                                       r600_bo_offset(rbuffer->bo) >> 8, 0xFFFFFFFF, rbuffer->bo);
> +                                       offset >> 8, 0xFFFFFFFF, bo);
>                r600_context_pipe_state_set(&rctx->ctx, &rctx->vs_const_buffer);
>                break;
>        case PIPE_SHADER_FRAGMENT:
>                rctx->ps_const_buffer.nregs = 0;
>                r600_pipe_state_add_reg(&rctx->ps_const_buffer,
>                                        R_028140_ALU_CONST_BUFFER_SIZE_PS_0,
> -                                       ALIGN_DIVUP(buffer->width0 >> 4, 16),
> +                                       ALIGN_DIVUP(width >> 4, 16),
>                                        0xFFFFFFFF, NULL);
>                r600_pipe_state_add_reg(&rctx->ps_const_buffer,
>                                        R_028940_ALU_CONST_CACHE_PS_0,
> -                                       r600_bo_offset(rbuffer->bo) >> 8, 0xFFFFFFFF, rbuffer->bo);
> +                                       offset >> 8, 0xFFFFFFFF, bo);
>                r600_context_pipe_state_set(&rctx->ctx, &rctx->ps_const_buffer);
>                break;
>        default:
> --
> 1.7.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>