Re: [Mesa-dev] [PATCH] radeonsi: fix dvec[34] attributes sourced from current attribute state

[Marek Olšák]

Reviewed-by: Marek Olšák 

Marek

On Thu, Mar 23, 2017 at 6:14 PM, Nicolai Hähnle  wrote:
> From: Nicolai Hähnle 
>
> The state tracker no longer uploads those attributes for us,
> so we must conservatively upload the size of the largest
> attribute, which is a dvec4.
>
> Fixes a regression of GL45-CTS.gpu_shader_fp64.varyings and
> GL45-CTS.vertex_attrib_64bit.limits_test.
>
> Fixes: 9b91e0b54cc2 ("radeonsi: allow unaligned vertex buffer offsets and 
> strides on CIK-VI")
> ---
>  src/gallium/drivers/radeonsi/si_state.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_state.c 
> b/src/gallium/drivers/radeonsi/si_state.c
> index 6948a74..0ee4af3 100644
> --- a/src/gallium/drivers/radeonsi/si_state.c
> +++ b/src/gallium/drivers/radeonsi/si_state.c
> @@ -3525,26 +3525,27 @@ static void si_set_vertex_buffers(struct pipe_context 
> *ctx,
>
> if (buffers) {
> for (i = 0; i < count; i++) {
> const struct pipe_vertex_buffer *src = buffers + i;
> struct pipe_vertex_buffer *dsti = dst + i;
>
> if (unlikely(src->user_buffer)) {
> /* Zero-stride attribs only. */
> assert(src->stride == 0);
>
> -   /* Assume the attrib has 4 dwords like the vbo
> -* module. This is also a good upper bound.
> +   /* Assume that the user_buffer comes from
> +* gl_current_attrib, which implies it has
> +* 4 * 8 bytes (for dvec4 attributes).
>  *
>  * Use const_uploader to upload into VRAM 
> directly.
>  */
> -   u_upload_data(sctx->b.b.const_uploader, 0, 
> 16, 16,
> +   u_upload_data(sctx->b.b.const_uploader, 0, 
> 32, 32,
>   src->user_buffer,
>   >buffer_offset,
>   >buffer);
> dsti->stride = 0;
> } else {
> struct pipe_resource *buf = src->buffer;
>
> pipe_resource_reference(>buffer, buf);
> dsti->buffer_offset = src->buffer_offset;
> dsti->stride = src->stride;
> --
> 2.9.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] radeonsi: fix dvec[34] attributes sourced from current attribute state

[Ilia Mirkin]

Ah I see. So we still get 2 vertex attributes, good. This was just
tripping some assumptions in your code about how attributes are laid
out in a constant vertex buffer. I guess I should double-check nouveau
as well, but I think we should be good. I always forget about the
disconnect between attributes and buffers :)

On Thu, Mar 23, 2017 at 1:29 PM, Nicolai Hähnle  wrote:
> On 23.03.2017 18:19, Ilia Mirkin wrote:
>>
>> Wait, you can have constant attribs that are dvec4's? Those don't get
>> split up into multiple constant attribs (2x dvec2, as it would for a
>> regular VBO)? Or am I misunderstanding the circumstances?
>
>
> The draw call will have one vertex buffer for each GL attribute, but the
> attribute is split into two vertex elements, i.e. it occupies two slots in
> the vertex array.
>
> E.g. the CTS has a test where there are 9 vertex buffers and I believe 15
> attributes at the Gallium interface level.
>
> Cheers,
> Nicolai
>
>
>>
>> On Thu, Mar 23, 2017 at 1:14 PM, Nicolai Hähnle 
>> wrote:
>>>
>>> From: Nicolai Hähnle 
>>>
>>> The state tracker no longer uploads those attributes for us,
>>> so we must conservatively upload the size of the largest
>>> attribute, which is a dvec4.
>>>
>>> Fixes a regression of GL45-CTS.gpu_shader_fp64.varyings and
>>> GL45-CTS.vertex_attrib_64bit.limits_test.
>>>
>>> Fixes: 9b91e0b54cc2 ("radeonsi: allow unaligned vertex buffer offsets and
>>> strides on CIK-VI")
>>> ---
>>>  src/gallium/drivers/radeonsi/si_state.c | 7 ---
>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/radeonsi/si_state.c
>>> b/src/gallium/drivers/radeonsi/si_state.c
>>> index 6948a74..0ee4af3 100644
>>> --- a/src/gallium/drivers/radeonsi/si_state.c
>>> +++ b/src/gallium/drivers/radeonsi/si_state.c
>>> @@ -3525,26 +3525,27 @@ static void si_set_vertex_buffers(struct
>>> pipe_context *ctx,
>>>
>>> if (buffers) {
>>> for (i = 0; i < count; i++) {
>>> const struct pipe_vertex_buffer *src = buffers +
>>> i;
>>> struct pipe_vertex_buffer *dsti = dst + i;
>>>
>>> if (unlikely(src->user_buffer)) {
>>> /* Zero-stride attribs only. */
>>> assert(src->stride == 0);
>>>
>>> -   /* Assume the attrib has 4 dwords like
>>> the vbo
>>> -* module. This is also a good upper
>>> bound.
>>> +   /* Assume that the user_buffer comes from
>>> +* gl_current_attrib, which implies it
>>> has
>>> +* 4 * 8 bytes (for dvec4 attributes).
>>>  *
>>>  * Use const_uploader to upload into VRAM
>>> directly.
>>>  */
>>> -   u_upload_data(sctx->b.b.const_uploader,
>>> 0, 16, 16,
>>> +   u_upload_data(sctx->b.b.const_uploader,
>>> 0, 32, 32,
>>>   src->user_buffer,
>>>   >buffer_offset,
>>>   >buffer);
>>> dsti->stride = 0;
>>> } else {
>>> struct pipe_resource *buf = src->buffer;
>>>
>>> pipe_resource_reference(>buffer,
>>> buf);
>>> dsti->buffer_offset = src->buffer_offset;
>>> dsti->stride = src->stride;
>>> --
>>> 2.9.3
>>>
>>> ___
>>> mesa-dev mailing list
>>> mesa-dev@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
>
> --
> Lerne, wie die Welt wirklich ist,
> Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] radeonsi: fix dvec[34] attributes sourced from current attribute state

[Nicolai Hähnle]

On 23.03.2017 18:19, Ilia Mirkin wrote:

Wait, you can have constant attribs that are dvec4's? Those don't get
split up into multiple constant attribs (2x dvec2, as it would for a
regular VBO)? Or am I misunderstanding the circumstances?


The draw call will have one vertex buffer for each GL attribute, but the 
attribute is split into two vertex elements, i.e. it occupies two slots 
in the vertex array.


E.g. the CTS has a test where there are 9 vertex buffers and I believe 
15 attributes at the Gallium interface level.


Cheers,
Nicolai



On Thu, Mar 23, 2017 at 1:14 PM, Nicolai Hähnle  wrote:

From: Nicolai Hähnle 

The state tracker no longer uploads those attributes for us,
so we must conservatively upload the size of the largest
attribute, which is a dvec4.

Fixes a regression of GL45-CTS.gpu_shader_fp64.varyings and
GL45-CTS.vertex_attrib_64bit.limits_test.

Fixes: 9b91e0b54cc2 ("radeonsi: allow unaligned vertex buffer offsets and strides on 
CIK-VI")
---
 src/gallium/drivers/radeonsi/si_state.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_state.c 
b/src/gallium/drivers/radeonsi/si_state.c
index 6948a74..0ee4af3 100644
--- a/src/gallium/drivers/radeonsi/si_state.c
+++ b/src/gallium/drivers/radeonsi/si_state.c
@@ -3525,26 +3525,27 @@ static void si_set_vertex_buffers(struct pipe_context 
*ctx,

if (buffers) {
for (i = 0; i < count; i++) {
const struct pipe_vertex_buffer *src = buffers + i;
struct pipe_vertex_buffer *dsti = dst + i;

if (unlikely(src->user_buffer)) {
/* Zero-stride attribs only. */
assert(src->stride == 0);

-   /* Assume the attrib has 4 dwords like the vbo
-* module. This is also a good upper bound.
+   /* Assume that the user_buffer comes from
+* gl_current_attrib, which implies it has
+* 4 * 8 bytes (for dvec4 attributes).
 *
 * Use const_uploader to upload into VRAM 
directly.
 */
-   u_upload_data(sctx->b.b.const_uploader, 0, 16, 
16,
+   u_upload_data(sctx->b.b.const_uploader, 0, 32, 
32,
  src->user_buffer,
  >buffer_offset,
  >buffer);
dsti->stride = 0;
} else {
struct pipe_resource *buf = src->buffer;

pipe_resource_reference(>buffer, buf);
dsti->buffer_offset = src->buffer_offset;
dsti->stride = src->stride;
--
2.9.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev



--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] radeonsi: fix dvec[34] attributes sourced from current attribute state

[Ilia Mirkin]

Wait, you can have constant attribs that are dvec4's? Those don't get
split up into multiple constant attribs (2x dvec2, as it would for a
regular VBO)? Or am I misunderstanding the circumstances?

On Thu, Mar 23, 2017 at 1:14 PM, Nicolai Hähnle  wrote:
> From: Nicolai Hähnle 
>
> The state tracker no longer uploads those attributes for us,
> so we must conservatively upload the size of the largest
> attribute, which is a dvec4.
>
> Fixes a regression of GL45-CTS.gpu_shader_fp64.varyings and
> GL45-CTS.vertex_attrib_64bit.limits_test.
>
> Fixes: 9b91e0b54cc2 ("radeonsi: allow unaligned vertex buffer offsets and 
> strides on CIK-VI")
> ---
>  src/gallium/drivers/radeonsi/si_state.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_state.c 
> b/src/gallium/drivers/radeonsi/si_state.c
> index 6948a74..0ee4af3 100644
> --- a/src/gallium/drivers/radeonsi/si_state.c
> +++ b/src/gallium/drivers/radeonsi/si_state.c
> @@ -3525,26 +3525,27 @@ static void si_set_vertex_buffers(struct pipe_context 
> *ctx,
>
> if (buffers) {
> for (i = 0; i < count; i++) {
> const struct pipe_vertex_buffer *src = buffers + i;
> struct pipe_vertex_buffer *dsti = dst + i;
>
> if (unlikely(src->user_buffer)) {
> /* Zero-stride attribs only. */
> assert(src->stride == 0);
>
> -   /* Assume the attrib has 4 dwords like the vbo
> -* module. This is also a good upper bound.
> +   /* Assume that the user_buffer comes from
> +* gl_current_attrib, which implies it has
> +* 4 * 8 bytes (for dvec4 attributes).
>  *
>  * Use const_uploader to upload into VRAM 
> directly.
>  */
> -   u_upload_data(sctx->b.b.const_uploader, 0, 
> 16, 16,
> +   u_upload_data(sctx->b.b.const_uploader, 0, 
> 32, 32,
>   src->user_buffer,
>   >buffer_offset,
>   >buffer);
> dsti->stride = 0;
> } else {
> struct pipe_resource *buf = src->buffer;
>
> pipe_resource_reference(>buffer, buf);
> dsti->buffer_offset = src->buffer_offset;
> dsti->stride = src->stride;
> --
> 2.9.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev