thr3ads.net - Nouveau - [Nouveau] [Mesa-dev] [PATCH] nv50, nvc0: only clear out the buffers that we were asked to clear [Jan 2014]

If this information is useful, please help other people find it:
Share via:

Ilia Mirkin

2014-Jan-17 02:23 UTC

[Nouveau] [PATCH] nv50, nvc0: only clear out the buffers that we were asked to clear

Fixes fbo-drawbuffers-none glClearBuffer piglit test.

Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
---

Only tested on nv50, but implementations seem similar enough.

 src/gallium/drivers/nouveau/nv50/nv50_surface.c | 17 +++++++++--------
 src/gallium/drivers/nouveau/nvc0/nvc0_surface.c | 17 +++++++++--------
 2 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_surface.c
b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
index 358f57a..eb27429 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_surface.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
@@ -408,9 +408,6 @@ nv50_clear(struct pipe_context *pipe, unsigned buffers,
       PUSH_DATAf(push, color->f[1]);
       PUSH_DATAf(push, color->f[2]);
       PUSH_DATAf(push, color->f[3]);
-      mode -         NV50_3D_CLEAR_BUFFERS_R | NV50_3D_CLEAR_BUFFERS_G |
-         NV50_3D_CLEAR_BUFFERS_B | NV50_3D_CLEAR_BUFFERS_A;
    }
 
    if (buffers & PIPE_CLEAR_DEPTH) {
@@ -425,12 +422,16 @@ nv50_clear(struct pipe_context *pipe, unsigned buffers,
       mode |= NV50_3D_CLEAR_BUFFERS_S;
    }
 
-   BEGIN_NV04(push, NV50_3D(CLEAR_BUFFERS), 1);
-   PUSH_DATA (push, mode);
-
-   for (i = 1; i < fb->nr_cbufs; i++) {
+   if (mode) {
       BEGIN_NV04(push, NV50_3D(CLEAR_BUFFERS), 1);
-      PUSH_DATA (push, (i << 6) | 0x3c);
+      PUSH_DATA (push, mode);
+   }
+
+   for (i = 0; i < fb->nr_cbufs; i++) {
+      if (buffers & (PIPE_CLEAR_COLOR0 << i)) {
+         BEGIN_NV04(push, NV50_3D(CLEAR_BUFFERS), 1);
+         PUSH_DATA (push, (i << 6) | 0x3c);
+      }
    }
 }
 
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
b/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
index 5375bd4..0c12bce 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
@@ -427,9 +427,6 @@ nvc0_clear(struct pipe_context *pipe, unsigned buffers,
       PUSH_DATAf(push, color->f[1]);
       PUSH_DATAf(push, color->f[2]);
       PUSH_DATAf(push, color->f[3]);
-      mode -         NVC0_3D_CLEAR_BUFFERS_R | NVC0_3D_CLEAR_BUFFERS_G |
-         NVC0_3D_CLEAR_BUFFERS_B | NVC0_3D_CLEAR_BUFFERS_A;
    }
 
    if (buffers & PIPE_CLEAR_DEPTH) {
@@ -444,12 +441,16 @@ nvc0_clear(struct pipe_context *pipe, unsigned buffers,
       mode |= NVC0_3D_CLEAR_BUFFERS_S;
    }
 
-   BEGIN_NVC0(push, NVC0_3D(CLEAR_BUFFERS), 1);
-   PUSH_DATA (push, mode);
-
-   for (i = 1; i < fb->nr_cbufs; i++) {
+   if (mode) {
       BEGIN_NVC0(push, NVC0_3D(CLEAR_BUFFERS), 1);
-      PUSH_DATA (push, (i << 6) | 0x3c);
+      PUSH_DATA (push, mode);
+   }
+
+   for (i = 0; i < fb->nr_cbufs; i++) {
+      if (buffers & (PIPE_CLEAR_COLOR0 << i)) {
+         BEGIN_NVC0(push, NVC0_3D(CLEAR_BUFFERS), 1);
+         PUSH_DATA (push, (i << 6) | 0x3c);
+      }
    }
 }
 
-- 
1.8.3.2

Emil Velikov

2014-Jan-23 20:11 UTC

head link

[Nouveau] [Mesa-dev] [PATCH] nv50, nvc0: only clear out the buffers that we were asked to clear

On 17/01/14 02:23, Ilia Mirkin wrote:> Fixes fbo-drawbuffers-none glClearBuffer piglit test.
> 
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
> 
> Only tested on nv50, but implementations seem similar enough.
> 
>  src/gallium/drivers/nouveau/nv50/nv50_surface.c | 17 +++++++++--------
>  src/gallium/drivers/nouveau/nvc0/nvc0_surface.c | 17 +++++++++--------
>  2 files changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_surface.c
b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
> index 358f57a..eb27429 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_surface.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
> @@ -408,9 +408,6 @@ nv50_clear(struct pipe_context *pipe, unsigned buffers,
>        PUSH_DATAf(push, color->f[1]);
>        PUSH_DATAf(push, color->f[2]);
>        PUSH_DATAf(push, color->f[3]);
> -      mode > -         NV50_3D_CLEAR_BUFFERS_R |
NV50_3D_CLEAR_BUFFERS_G |
> -         NV50_3D_CLEAR_BUFFERS_B | NV50_3D_CLEAR_BUFFERS_A;
>     }
>  I'm not sure why you've dropped the mode from above. I'm guessing
that
the initial assumption was that if there is a color buffer it must be at
cbuf[0].

The corrected version in your github branch looks alot better, it
handles the above case and does not overwrite the clear buffer on rt0.

That one is Reviewed-by: Emil Velikov <emil.l.velikov at
gmail.com>>     if (buffers & PIPE_CLEAR_DEPTH) {
> @@ -425,12 +422,16 @@ nv50_clear(struct pipe_context *pipe, unsigned
buffers,
>        mode |= NV50_3D_CLEAR_BUFFERS_S;
>     }
>  
> -   BEGIN_NV04(push, NV50_3D(CLEAR_BUFFERS), 1);
> -   PUSH_DATA (push, mode);
> -
> -   for (i = 1; i < fb->nr_cbufs; i++) {
> +   if (mode) {
>        BEGIN_NV04(push, NV50_3D(CLEAR_BUFFERS), 1);
> -      PUSH_DATA (push, (i << 6) | 0x3c);
> +      PUSH_DATA (push, mode);
> +   }
> +
> +   for (i = 0; i < fb->nr_cbufs; i++) {
> +      if (buffers & (PIPE_CLEAR_COLOR0 << i)) {
> +         BEGIN_NV04(push, NV50_3D(CLEAR_BUFFERS), 1);
> +         PUSH_DATA (push, (i << 6) | 0x3c);
> +      }
>     }
>  }
>  
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
b/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
> index 5375bd4..0c12bce 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
> @@ -427,9 +427,6 @@ nvc0_clear(struct pipe_context *pipe, unsigned buffers,
>        PUSH_DATAf(push, color->f[1]);
>        PUSH_DATAf(push, color->f[2]);
>        PUSH_DATAf(push, color->f[3]);
> -      mode > -         NVC0_3D_CLEAR_BUFFERS_R |
NVC0_3D_CLEAR_BUFFERS_G |
> -         NVC0_3D_CLEAR_BUFFERS_B | NVC0_3D_CLEAR_BUFFERS_A;
>     }
>  
>     if (buffers & PIPE_CLEAR_DEPTH) {
> @@ -444,12 +441,16 @@ nvc0_clear(struct pipe_context *pipe, unsigned
buffers,
>        mode |= NVC0_3D_CLEAR_BUFFERS_S;
>     }
>  
> -   BEGIN_NVC0(push, NVC0_3D(CLEAR_BUFFERS), 1);
> -   PUSH_DATA (push, mode);
> -
> -   for (i = 1; i < fb->nr_cbufs; i++) {
> +   if (mode) {
>        BEGIN_NVC0(push, NVC0_3D(CLEAR_BUFFERS), 1);
> -      PUSH_DATA (push, (i << 6) | 0x3c);
> +      PUSH_DATA (push, mode);
> +   }
> +
> +   for (i = 0; i < fb->nr_cbufs; i++) {
> +      if (buffers & (PIPE_CLEAR_COLOR0 << i)) {
> +         BEGIN_NVC0(push, NVC0_3D(CLEAR_BUFFERS), 1);
> +         PUSH_DATA (push, (i << 6) | 0x3c);
> +      }
>     }
>  }
>  
>

Ilia Mirkin

2014-Jan-23 20:15 UTC

head link

[Nouveau] [Mesa-dev] [PATCH] nv50, nvc0: only clear out the buffers that we were asked to clear

On Thu, Jan 23, 2014 at 3:11 PM, Emil Velikov <emil.l.velikov at
gmail.com> wrote:> On 17/01/14 02:23, Ilia Mirkin wrote:
>> Fixes fbo-drawbuffers-none glClearBuffer piglit test.
>>
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>>
>> Only tested on nv50, but implementations seem similar enough.
>>
>>  src/gallium/drivers/nouveau/nv50/nv50_surface.c | 17 +++++++++--------
>>  src/gallium/drivers/nouveau/nvc0/nvc0_surface.c | 17 +++++++++--------
>>  2 files changed, 18 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_surface.c
b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
>> index 358f57a..eb27429 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_surface.c
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
>> @@ -408,9 +408,6 @@ nv50_clear(struct pipe_context *pipe, unsigned
buffers,
>>        PUSH_DATAf(push, color->f[1]);
>>        PUSH_DATAf(push, color->f[2]);
>>        PUSH_DATAf(push, color->f[3]);
>> -      mode >> -         NV50_3D_CLEAR_BUFFERS_R |
NV50_3D_CLEAR_BUFFERS_G |
>> -         NV50_3D_CLEAR_BUFFERS_B | NV50_3D_CLEAR_BUFFERS_A;
>>     }
>>
> I'm not sure why you've dropped the mode from above. I'm
guessing that
> the initial assumption was that if there is a color buffer it must be at
> cbuf[0].
Because I cleared the cbufs separately in a for loop below (the 0x3c
== the RGBA mask). The first RT may not have been there, and that
seemed simpler than the thing that I have now (as evidenced by the
code I have now which has more complex logic).
>
> The corrected version in your github branch looks alot better, it
> handles the above case and does not overwrite the clear buffer on rt0.
Christoph was really keen on doing the common-case color/depth clear
all in one clear buffers command, so yeah -- I redid it that way.
Enjoy the layered version of that in a later commit.
>
> That one is Reviewed-by: Emil Velikov <emil.l.velikov at gmail.com>
>>     if (buffers & PIPE_CLEAR_DEPTH) {
>> @@ -425,12 +422,16 @@ nv50_clear(struct pipe_context *pipe, unsigned
buffers,
>>        mode |= NV50_3D_CLEAR_BUFFERS_S;
>>     }
>>
>> -   BEGIN_NV04(push, NV50_3D(CLEAR_BUFFERS), 1);
>> -   PUSH_DATA (push, mode);
>> -
>> -   for (i = 1; i < fb->nr_cbufs; i++) {
>> +   if (mode) {
>>        BEGIN_NV04(push, NV50_3D(CLEAR_BUFFERS), 1);
>> -      PUSH_DATA (push, (i << 6) | 0x3c);
>> +      PUSH_DATA (push, mode);
>> +   }
>> +
>> +   for (i = 0; i < fb->nr_cbufs; i++) {
>> +      if (buffers & (PIPE_CLEAR_COLOR0 << i)) {
>> +         BEGIN_NV04(push, NV50_3D(CLEAR_BUFFERS), 1);
>> +         PUSH_DATA (push, (i << 6) | 0x3c);
>> +      }
>>     }
>>  }
>>
>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
b/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
>> index 5375bd4..0c12bce 100644
>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
>> @@ -427,9 +427,6 @@ nvc0_clear(struct pipe_context *pipe, unsigned
buffers,
>>        PUSH_DATAf(push, color->f[1]);
>>        PUSH_DATAf(push, color->f[2]);
>>        PUSH_DATAf(push, color->f[3]);
>> -      mode >> -         NVC0_3D_CLEAR_BUFFERS_R |
NVC0_3D_CLEAR_BUFFERS_G |
>> -         NVC0_3D_CLEAR_BUFFERS_B | NVC0_3D_CLEAR_BUFFERS_A;
>>     }
>>
>>     if (buffers & PIPE_CLEAR_DEPTH) {
>> @@ -444,12 +441,16 @@ nvc0_clear(struct pipe_context *pipe, unsigned
buffers,
>>        mode |= NVC0_3D_CLEAR_BUFFERS_S;
>>     }
>>
>> -   BEGIN_NVC0(push, NVC0_3D(CLEAR_BUFFERS), 1);
>> -   PUSH_DATA (push, mode);
>> -
>> -   for (i = 1; i < fb->nr_cbufs; i++) {
>> +   if (mode) {
>>        BEGIN_NVC0(push, NVC0_3D(CLEAR_BUFFERS), 1);
>> -      PUSH_DATA (push, (i << 6) | 0x3c);
>> +      PUSH_DATA (push, mode);
>> +   }
>> +
>> +   for (i = 0; i < fb->nr_cbufs; i++) {
>> +      if (buffers & (PIPE_CLEAR_COLOR0 << i)) {
>> +         BEGIN_NVC0(push, NVC0_3D(CLEAR_BUFFERS), 1);
>> +         PUSH_DATA (push, (i << 6) | 0x3c);
>> +      }
>>     }
>>  }
>>
>>
>

Apparently Analagous Threads

Search for more possibly parallel threads

Nouveau - Jan 2014 - [Mesa-dev] [PATCH] nv50, nvc0: only clear out the buffers that we were asked to clear

[Nouveau] [PATCH] nv50, nvc0: only clear out the buffers that we were asked to clear

[Nouveau] [Mesa-dev] [PATCH] nv50, nvc0: only clear out the buffers that we were asked to clear

[Nouveau] [Mesa-dev] [PATCH] nv50, nvc0: only clear out the buffers that we were asked to clear

Apparently Analagous Threads