[Mesa-dev] [PATCH 04/10] freedreno: implement user constant buffers

[Rob Clark]

On Wed, Jan 10, 2018 at 4:41 PM, Marek Olšák <maraeo at gmail.com> wrote:
> On Wed, Jan 10, 2018 at 9:36 PM, Rob Clark <robdclark at gmail.com> wrote:
>> On Wed, Jan 10, 2018 at 3:33 PM, Rob Clark <robdclark at gmail.com> wrote:
>>> On Wed, Jan 10, 2018 at 2:49 PM, Marek Olšák <maraeo at gmail.com> wrote:
>>>> From: Marek Olšák <marek.olsak at amd.com>
>>>>
>>>> ---
>>>>  src/gallium/drivers/freedreno/freedreno_screen.c |  4 +---
>>>>  src/gallium/drivers/freedreno/freedreno_state.c  | 13 +++++++++++++
>>>>  2 files changed, 14 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c b/src/gallium/drivers/freedreno/freedreno_screen.c
>>>> index 315b73a..639fbe7 100644
>>>> --- a/src/gallium/drivers/freedreno/freedreno_screen.c
>>>> +++ b/src/gallium/drivers/freedreno/freedreno_screen.c
>>>> @@ -177,28 +177,26 @@ fd_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
>>>>         case PIPE_CAP_VERTEX_COLOR_UNCLAMPED:
>>>>         case PIPE_CAP_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION:
>>>>         case PIPE_CAP_VERTEX_BUFFER_OFFSET_4BYTE_ALIGNED_ONLY:
>>>>         case PIPE_CAP_VERTEX_BUFFER_STRIDE_4BYTE_ALIGNED_ONLY:
>>>>         case PIPE_CAP_VERTEX_ELEMENT_SRC_OFFSET_4BYTE_ALIGNED_ONLY:
>>>>         case PIPE_CAP_BUFFER_MAP_PERSISTENT_COHERENT:
>>>>         case PIPE_CAP_STRING_MARKER:
>>>>         case PIPE_CAP_MIXED_COLOR_DEPTH_BITS:
>>>>         case PIPE_CAP_TEXTURE_BARRIER:
>>>>         case PIPE_CAP_INVALIDATE_BUFFER:
>>>> +       case PIPE_CAP_USER_CONSTANT_BUFFERS:
>>>>                 return 1;
>>>>
>>>>         case PIPE_CAP_VERTEXID_NOBASE:
>>>>                 return is_a3xx(screen) || is_a4xx(screen);
>>>>
>>>> -       case PIPE_CAP_USER_CONSTANT_BUFFERS:
>>>> -               return is_a4xx(screen) ? 0 : 1;
>>>> -
>>>>         case PIPE_CAP_COMPUTE:
>>>>                 return has_compute(screen);
>>>>
>>>>         case PIPE_CAP_SHADER_STENCIL_EXPORT:
>>>>         case PIPE_CAP_TGSI_TEXCOORD:
>>>>         case PIPE_CAP_PREFER_BLIT_BASED_TEXTURE_TRANSFER:
>>>>         case PIPE_CAP_TEXTURE_MULTISAMPLE:
>>>>         case PIPE_CAP_TEXTURE_MIRROR_CLAMP:
>>>>         case PIPE_CAP_QUERY_MEMORY_INFO:
>>>>         case PIPE_CAP_PCI_GROUP:
>>>> diff --git a/src/gallium/drivers/freedreno/freedreno_state.c b/src/gallium/drivers/freedreno/freedreno_state.c
>>>> index dd42aa0..9f0c490 100644
>>>> --- a/src/gallium/drivers/freedreno/freedreno_state.c
>>>> +++ b/src/gallium/drivers/freedreno/freedreno_state.c
>>>> @@ -24,20 +24,21 @@
>>>>   *
>>>>   * Authors:
>>>>   *    Rob Clark <robclark at freedesktop.org>
>>>>   */
>>>>
>>>>  #include "pipe/p_state.h"
>>>>  #include "util/u_dual_blend.h"
>>>>  #include "util/u_string.h"
>>>>  #include "util/u_memory.h"
>>>>  #include "util/u_helpers.h"
>>>> +#include "util/u_upload_mgr.h"
>>>>
>>>>  #include "freedreno_state.h"
>>>>  #include "freedreno_context.h"
>>>>  #include "freedreno_resource.h"
>>>>  #include "freedreno_texture.h"
>>>>  #include "freedreno_gmem.h"
>>>>  #include "freedreno_query_hw.h"
>>>>  #include "freedreno_util.h"
>>>>
>>>>  /* All the generic state handling.. In case of CSO's that are specific
>>>> @@ -88,20 +89,32 @@ fd_set_sample_mask(struct pipe_context *pctx, unsigned sample_mask)
>>>>   * previous values.
>>>>   * index>0 will be UBO's.. well, I'll worry about that later
>>>>   */
>>>>  static void
>>>>  fd_set_constant_buffer(struct pipe_context *pctx,
>>>>                 enum pipe_shader_type shader, uint index,
>>>>                 const struct pipe_constant_buffer *cb)
>>>>  {
>>>>         struct fd_context *ctx = fd_context(pctx);
>>>>         struct fd_constbuf_stateobj *so = &ctx->constbuf[shader];
>>>> +       struct pipe_constant_buffer c;
>>>> +
>>>> +       if (is_a4xx(ctx->screen) && cb && cb->user_buffer) {
>>>> +               c.buffer = NULL;
>>>> +               c.buffer_size = cb->buffer_size;
>>>> +               c.user_buffer = NULL;
>>>> +
>>>> +               u_upload_data(pctx->const_uploader, 0, cb->buffer_size, 64,
>>>> +                             cb->user_buffer, &c.buffer_offset, &c.buffer);
>>>> +               u_upload_unmap(pctx->const_uploader);
>>>> +               cb = &c;
>>>> +       }
>>>
>>> It's probably ok to just drop this.. actually all of a3xx+ can do
>>> non-user uniform buffer (I just haven't worked out the alignment
>>> constraints on other gens)..
>>>
>>> Really I just wanted a way to say kinda the opposite thing as this cap
>>> to the state tracker (ie. that I could do both user and non-user)
>>>
>>
>> btw, wouldn't u_threaded prefer non-user cb's?  That way you get a
>> refcnt'd ptr to pass thru the queue between threads..
>>
>> (That was kinda why I originally added non-user cb support in a4xx,
>> when I was experimenting w/ a generic render reordering pipe driver
>> shim thing)
>
> u_threaded takes care of uploading user constant buffers.
>

yeah, so same approach that I took..

I do kinda think we should probably have two different caps, one for
whether driver supports user buffers, and one for whether it supports
uploaded buffers.  (Or is adreno the only one that can either upload
uniforms via cmdstream or via pointer in cmdstream?)

BR,
-R