[04/10] freedreno: implement user constant buffers

Submitted by Marek Olšák on Jan. 10, 2018, 7:49 p.m.

Details

Message ID 1515613774-2056-4-git-send-email-maraeo@gmail.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Marek Olšák Jan. 10, 2018, 7:49 p.m.
From: Marek Olšák <marek.olsak@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(-)

Patch hide | download patch | download mbox

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@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;
+	}
 
 	util_copy_constant_buffer(&so->cb[index], cb);
 
 	/* Note that the state tracker can unbind constant buffers by
 	 * passing NULL here.
 	 */
 	if (unlikely(!cb)) {
 		so->enabled_mask &= ~(1 << index);
 		so->dirty_mask &= ~(1 << index);
 		return;

Comments

On Wed, Jan 10, 2018 at 2:49 PM, Marek Olšák <maraeo@gmail.com> wrote:
> From: Marek Olšák <marek.olsak@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@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)

BR,
-R

>
>         util_copy_constant_buffer(&so->cb[index], cb);
>
>         /* Note that the state tracker can unbind constant buffers by
>          * passing NULL here.
>          */
>         if (unlikely(!cb)) {
>                 so->enabled_mask &= ~(1 << index);
>                 so->dirty_mask &= ~(1 << index);
>                 return;
> --
> 2.7.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On Wed, Jan 10, 2018 at 3:33 PM, Rob Clark <robdclark@gmail.com> wrote:
> On Wed, Jan 10, 2018 at 2:49 PM, Marek Olšák <maraeo@gmail.com> wrote:
>> From: Marek Olšák <marek.olsak@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@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)


> BR,
> -R
>
>>
>>         util_copy_constant_buffer(&so->cb[index], cb);
>>
>>         /* Note that the state tracker can unbind constant buffers by
>>          * passing NULL here.
>>          */
>>         if (unlikely(!cb)) {
>>                 so->enabled_mask &= ~(1 << index);
>>                 so->dirty_mask &= ~(1 << index);
>>                 return;
>> --
>> 2.7.4
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On Wed, Jan 10, 2018 at 9:36 PM, Rob Clark <robdclark@gmail.com> wrote:
> On Wed, Jan 10, 2018 at 3:33 PM, Rob Clark <robdclark@gmail.com> wrote:
>> On Wed, Jan 10, 2018 at 2:49 PM, Marek Olšák <maraeo@gmail.com> wrote:
>>> From: Marek Olšák <marek.olsak@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@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.

Marek
On Wed, Jan 10, 2018 at 4:41 PM, Marek Olšák <maraeo@gmail.com> wrote:
> On Wed, Jan 10, 2018 at 9:36 PM, Rob Clark <robdclark@gmail.com> wrote:
>> On Wed, Jan 10, 2018 at 3:33 PM, Rob Clark <robdclark@gmail.com> wrote:
>>> On Wed, Jan 10, 2018 at 2:49 PM, Marek Olšák <maraeo@gmail.com> wrote:
>>>> From: Marek Olšák <marek.olsak@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@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
On Jan 11, 2018 1:45 PM, "Rob Clark" <robdclark@gmail.com> wrote:

On Wed, Jan 10, 2018 at 4:41 PM, Marek Olšák <maraeo@gmail.com> wrote:
> On Wed, Jan 10, 2018 at 9:36 PM, Rob Clark <robdclark@gmail.com> wrote:
>> On Wed, Jan 10, 2018 at 3:33 PM, Rob Clark <robdclark@gmail.com> wrote:
>>> On Wed, Jan 10, 2018 at 2:49 PM, Marek Olšák <maraeo@gmail.com> wrote:
>>>> From: Marek Olšák <marek.olsak@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@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?)


I prefer to have no caps. All drivers should support both.

M.


BR,
-R