* [PATCH] nv50, nvc0: don't crash on a null cbuf
@ 2014-01-15 7:30 Ilia Mirkin
[not found] ` <1389771040-29447-1-git-send-email-imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Ilia Mirkin @ 2014-01-15 7:30 UTC (permalink / raw)
To: Christoph Bumiller, mesa-dev-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
This is needed since commit 9baa45f78b (st/mesa: bind NULL colorbuffers
as specified by glDrawBuffers).
Signed-off-by: Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org>
---
Not sure whether something needs to be done to clear out the old RT_* settings
for that index buffer, or if things are cleared out implicitly. Perhaps
instead of skipping indices, RT_CONTROL needs to be adjusted with the
appropriate indices?
src/gallium/drivers/nouveau/nv50/nv50_state_validate.c | 14 +++++++++++---
src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c | 14 +++++++++++---
2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c b/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
index 86b9a23..7d330c9 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
@@ -20,9 +20,17 @@ nv50_validate_fb(struct nv50_context *nv50)
PUSH_DATA (push, fb->height << 16);
for (i = 0; i < fb->nr_cbufs; ++i) {
- struct nv50_miptree *mt = nv50_miptree(fb->cbufs[i]->texture);
- struct nv50_surface *sf = nv50_surface(fb->cbufs[i]);
- struct nouveau_bo *bo = mt->base.bo;
+ struct nv50_miptree *mt;
+ struct nv50_surface *sf;
+ struct nouveau_bo *bo;
+
+ /* Do we need to clear the old RT settings? */
+ if (!fb->cbufs[i])
+ continue;
+
+ mt = nv50_miptree(fb->cbufs[i]->texture);
+ sf = nv50_surface(fb->cbufs[i]);
+ bo = mt->base.bo;
array_size = MIN2(array_size, sf->depth);
if (mt->layout_3d)
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c b/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
index 0ba4bad..9059d76 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
@@ -72,9 +72,17 @@ nvc0_validate_fb(struct nvc0_context *nvc0)
PUSH_DATA (push, fb->height << 16);
for (i = 0; i < fb->nr_cbufs; ++i) {
- struct nv50_surface *sf = nv50_surface(fb->cbufs[i]);
- struct nv04_resource *res = nv04_resource(sf->base.texture);
- struct nouveau_bo *bo = res->bo;
+ struct nv50_surface *sf;
+ struct nv04_resource *res;
+ struct nouveau_bo *bo;
+
+ /* Do we need to clear the old RT settings? */
+ if (!fb->cbufs[i])
+ continue;
+
+ sf = nv50_surface(fb->cbufs[i]);
+ res = nv04_resource(sf->base.texture);
+ bo = res->bo;
BEGIN_NVC0(push, NVC0_3D(RT_ADDRESS_HIGH(i)), 9);
PUSH_DATAh(push, res->address + sf->offset);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2] nv50, nvc0: clear out RT on a null cbuf
[not found] ` <1389771040-29447-1-git-send-email-imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org>
@ 2014-01-17 2:23 ` Ilia Mirkin
2014-01-17 2:23 ` [PATCH] nv50, nvc0: only clear out the buffers that we were asked to clear Ilia Mirkin
2014-01-23 19:40 ` [Nouveau] [PATCH v2] nv50, nvc0: clear out RT on a null cbuf Emil Velikov
0 siblings, 2 replies; 10+ messages in thread
From: Ilia Mirkin @ 2014-01-17 2:23 UTC (permalink / raw)
To: mesa-dev-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
This is needed since commit 9baa45f78b (st/mesa: bind NULL colorbuffers
as specified by glDrawBuffers).
This implementation is highly based on a larger commit by
Christoph Bumiller <e0425955-oe7qfRrRQffzPE21tAIdciO7C/xPubJB@public.gmane.org> in his gallium-nine
branch.
Signed-off-by: Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org>
---
src/gallium/drivers/nouveau/nv50/nv50_defs.xml.h | 1 +
src/gallium/drivers/nouveau/nv50/nv50_formats.c | 1 -
.../drivers/nouveau/nv50/nv50_state_validate.c | 30 +++++++++++++++++++---
.../drivers/nouveau/nvc0/nvc0_state_validate.c | 28 +++++++++++++++++---
4 files changed, 52 insertions(+), 8 deletions(-)
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_defs.xml.h b/src/gallium/drivers/nouveau/nv50/nv50_defs.xml.h
index 2e42843..80de3be 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_defs.xml.h
+++ b/src/gallium/drivers/nouveau/nv50/nv50_defs.xml.h
@@ -78,6 +78,7 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
#define NV50_VSTATUS_BLOCKED 0x00000005
#define NV50_VSTATUS_FAULTED 0x00000006
#define NV50_VSTATUS_PAUSED 0x00000007
+#define NV50_SURFACE_FORMAT_NONE 0x00000000
#define NV50_SURFACE_FORMAT_BITMAP 0x0000001c
#define NV50_SURFACE_FORMAT_UNK1D 0x0000001d
#define NV50_SURFACE_FORMAT_RGBA32_FLOAT 0x000000c0
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_formats.c b/src/gallium/drivers/nouveau/nv50/nv50_formats.c
index 0a7e812..d21905d 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_formats.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_formats.c
@@ -71,7 +71,6 @@
# define U_tV U_V
#endif
-#define NV50_SURFACE_FORMAT_NONE 0
#define NV50_ZETA_FORMAT_NONE 0
/* for vertex buffers: */
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c b/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
index 86b9a23..bb05f03 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
@@ -1,6 +1,19 @@
#include "nv50/nv50_context.h"
-#include "os/os_time.h"
+#include "nv50/nv50_defs.xml.h"
+
+static INLINE void
+nv50_fb_set_null_rt(struct nouveau_pushbuf *push, unsigned i)
+{
+ BEGIN_NV04(push, NV50_3D(RT_ADDRESS_HIGH(i)), 4);
+ PUSH_DATA (push, 0);
+ PUSH_DATA (push, 0);
+ PUSH_DATA (push, NV50_SURFACE_FORMAT_NONE);
+ PUSH_DATA (push, 0);
+ BEGIN_NV04(push, NV50_3D(RT_HORIZ(i)), 2);
+ PUSH_DATA (push, 64);
+ PUSH_DATA (push, 0);
+}
static void
nv50_validate_fb(struct nv50_context *nv50)
@@ -20,9 +33,18 @@ nv50_validate_fb(struct nv50_context *nv50)
PUSH_DATA (push, fb->height << 16);
for (i = 0; i < fb->nr_cbufs; ++i) {
- struct nv50_miptree *mt = nv50_miptree(fb->cbufs[i]->texture);
- struct nv50_surface *sf = nv50_surface(fb->cbufs[i]);
- struct nouveau_bo *bo = mt->base.bo;
+ struct nv50_miptree *mt;
+ struct nv50_surface *sf;
+ struct nouveau_bo *bo;
+
+ if (!fb->cbufs[i]) {
+ nv50_fb_set_null_rt(push, i);
+ continue;
+ }
+
+ mt = nv50_miptree(fb->cbufs[i]->texture);
+ sf = nv50_surface(fb->cbufs[i]);
+ bo = mt->base.bo;
array_size = MIN2(array_size, sf->depth);
if (mt->layout_3d)
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c b/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
index 0ba4bad..dd71c65 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
@@ -2,6 +2,7 @@
#include "util/u_math.h"
#include "nvc0/nvc0_context.h"
+#include "nv50/nv50_defs.xml.h"
#if 0
static void
@@ -54,6 +55,18 @@ nvc0_validate_zcull(struct nvc0_context *nvc0)
}
#endif
+static INLINE void
+nvc0_fb_set_null_rt(struct nouveau_pushbuf *push, unsigned i)
+{
+ BEGIN_NVC0(push, NVC0_3D(RT_ADDRESS_HIGH(i)), 6);
+ PUSH_DATA (push, 0);
+ PUSH_DATA (push, 0);
+ PUSH_DATA (push, 64);
+ PUSH_DATA (push, 0);
+ PUSH_DATA (push, NV50_SURFACE_FORMAT_NONE);
+ PUSH_DATA (push, 0);
+}
+
static void
nvc0_validate_fb(struct nvc0_context *nvc0)
{
@@ -72,9 +85,18 @@ nvc0_validate_fb(struct nvc0_context *nvc0)
PUSH_DATA (push, fb->height << 16);
for (i = 0; i < fb->nr_cbufs; ++i) {
- struct nv50_surface *sf = nv50_surface(fb->cbufs[i]);
- struct nv04_resource *res = nv04_resource(sf->base.texture);
- struct nouveau_bo *bo = res->bo;
+ struct nv50_surface *sf;
+ struct nv04_resource *res;
+ struct nouveau_bo *bo;
+
+ if (!fb->cbufs[i]) {
+ nvc0_fb_set_null_rt(push, i);
+ continue;
+ }
+
+ sf = nv50_surface(fb->cbufs[i]);
+ res = nv04_resource(sf->base.texture);
+ bo = res->bo;
BEGIN_NVC0(push, NVC0_3D(RT_ADDRESS_HIGH(i)), 9);
PUSH_DATAh(push, res->address + sf->offset);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] nv50, nvc0: only clear out the buffers that we were asked to clear
2014-01-17 2:23 ` [PATCH v2] nv50, nvc0: clear out RT " Ilia Mirkin
@ 2014-01-17 2:23 ` Ilia Mirkin
[not found] ` <1389925395-22379-2-git-send-email-imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org>
2014-01-23 19:40 ` [Nouveau] [PATCH v2] nv50, nvc0: clear out RT on a null cbuf Emil Velikov
1 sibling, 1 reply; 10+ messages in thread
From: Ilia Mirkin @ 2014-01-17 2:23 UTC (permalink / raw)
To: mesa-dev; +Cc: nouveau
Fixes fbo-drawbuffers-none glClearBuffer piglit test.
Signed-off-by: Ilia Mirkin <imirkin@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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] nv50, nvc0: clear out RT on a null cbuf
[not found] ` <52E1701A.3060203-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2014-01-23 19:39 ` Ilia Mirkin
[not found] ` <CAKb7UvgB_5omfkrL1mfFUYHL_3Pi8V01NzJJeo2sbKTfuNYOkA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Ilia Mirkin @ 2014-01-23 19:39 UTC (permalink / raw)
To: Emil Velikov
Cc: mesa-dev-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
On Thu, Jan 23, 2014 at 2:40 PM, Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 17/01/14 02:23, Ilia Mirkin wrote:
>> This is needed since commit 9baa45f78b (st/mesa: bind NULL colorbuffers
>> as specified by glDrawBuffers).
>>
>> This implementation is highly based on a larger commit by
>> Christoph Bumiller <e0425955-oe7qfRrRQffzPE21tAIdciO7C/xPubJB@public.gmane.org> in his gallium-nine
>> branch.
>>
> Ilia,
> Do you know why we cannot set the rt height to 64? After all you
> explicitly set the format to NONE.
Because then the RT would have a size, which we don't want. The real
question is why not set the width to 0. It was that way in Christoph's
code, I assumed he did it that way for a reason.
>
> Either way this patch looks good afaics
> Reviewed-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Signed-off-by: Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org>
>> ---
>> src/gallium/drivers/nouveau/nv50/nv50_defs.xml.h | 1 +
>> src/gallium/drivers/nouveau/nv50/nv50_formats.c | 1 -
>> .../drivers/nouveau/nv50/nv50_state_validate.c | 30 +++++++++++++++++++---
>> .../drivers/nouveau/nvc0/nvc0_state_validate.c | 28 +++++++++++++++++---
>> 4 files changed, 52 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_defs.xml.h b/src/gallium/drivers/nouveau/nv50/nv50_defs.xml.h
>> index 2e42843..80de3be 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_defs.xml.h
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_defs.xml.h
>> @@ -78,6 +78,7 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
>> #define NV50_VSTATUS_BLOCKED 0x00000005
>> #define NV50_VSTATUS_FAULTED 0x00000006
>> #define NV50_VSTATUS_PAUSED 0x00000007
>> +#define NV50_SURFACE_FORMAT_NONE 0x00000000
>> #define NV50_SURFACE_FORMAT_BITMAP 0x0000001c
>> #define NV50_SURFACE_FORMAT_UNK1D 0x0000001d
>> #define NV50_SURFACE_FORMAT_RGBA32_FLOAT 0x000000c0
>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_formats.c b/src/gallium/drivers/nouveau/nv50/nv50_formats.c
>> index 0a7e812..d21905d 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_formats.c
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_formats.c
>> @@ -71,7 +71,6 @@
>> # define U_tV U_V
>> #endif
>>
>> -#define NV50_SURFACE_FORMAT_NONE 0
>> #define NV50_ZETA_FORMAT_NONE 0
>>
>> /* for vertex buffers: */
>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c b/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
>> index 86b9a23..bb05f03 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
>> @@ -1,6 +1,19 @@
>>
>> #include "nv50/nv50_context.h"
>> -#include "os/os_time.h"
>> +#include "nv50/nv50_defs.xml.h"
>> +
>> +static INLINE void
>> +nv50_fb_set_null_rt(struct nouveau_pushbuf *push, unsigned i)
>> +{
>> + BEGIN_NV04(push, NV50_3D(RT_ADDRESS_HIGH(i)), 4);
>> + PUSH_DATA (push, 0);
>> + PUSH_DATA (push, 0);
>> + PUSH_DATA (push, NV50_SURFACE_FORMAT_NONE);
>> + PUSH_DATA (push, 0);
>> + BEGIN_NV04(push, NV50_3D(RT_HORIZ(i)), 2);
>> + PUSH_DATA (push, 64);
>> + PUSH_DATA (push, 0);
>> +}
>>
>> static void
>> nv50_validate_fb(struct nv50_context *nv50)
>> @@ -20,9 +33,18 @@ nv50_validate_fb(struct nv50_context *nv50)
>> PUSH_DATA (push, fb->height << 16);
>>
>> for (i = 0; i < fb->nr_cbufs; ++i) {
>> - struct nv50_miptree *mt = nv50_miptree(fb->cbufs[i]->texture);
>> - struct nv50_surface *sf = nv50_surface(fb->cbufs[i]);
>> - struct nouveau_bo *bo = mt->base.bo;
>> + struct nv50_miptree *mt;
>> + struct nv50_surface *sf;
>> + struct nouveau_bo *bo;
>> +
>> + if (!fb->cbufs[i]) {
>> + nv50_fb_set_null_rt(push, i);
>> + continue;
>> + }
>> +
>> + mt = nv50_miptree(fb->cbufs[i]->texture);
>> + sf = nv50_surface(fb->cbufs[i]);
>> + bo = mt->base.bo;
>>
>> array_size = MIN2(array_size, sf->depth);
>> if (mt->layout_3d)
>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c b/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
>> index 0ba4bad..dd71c65 100644
>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
>> @@ -2,6 +2,7 @@
>> #include "util/u_math.h"
>>
>> #include "nvc0/nvc0_context.h"
>> +#include "nv50/nv50_defs.xml.h"
>>
>> #if 0
>> static void
>> @@ -54,6 +55,18 @@ nvc0_validate_zcull(struct nvc0_context *nvc0)
>> }
>> #endif
>>
>> +static INLINE void
>> +nvc0_fb_set_null_rt(struct nouveau_pushbuf *push, unsigned i)
>> +{
>> + BEGIN_NVC0(push, NVC0_3D(RT_ADDRESS_HIGH(i)), 6);
>> + PUSH_DATA (push, 0);
>> + PUSH_DATA (push, 0);
>> + PUSH_DATA (push, 64);
>> + PUSH_DATA (push, 0);
>> + PUSH_DATA (push, NV50_SURFACE_FORMAT_NONE);
>> + PUSH_DATA (push, 0);
>> +}
>> +
>> static void
>> nvc0_validate_fb(struct nvc0_context *nvc0)
>> {
>> @@ -72,9 +85,18 @@ nvc0_validate_fb(struct nvc0_context *nvc0)
>> PUSH_DATA (push, fb->height << 16);
>>
>> for (i = 0; i < fb->nr_cbufs; ++i) {
>> - struct nv50_surface *sf = nv50_surface(fb->cbufs[i]);
>> - struct nv04_resource *res = nv04_resource(sf->base.texture);
>> - struct nouveau_bo *bo = res->bo;
>> + struct nv50_surface *sf;
>> + struct nv04_resource *res;
>> + struct nouveau_bo *bo;
>> +
>> + if (!fb->cbufs[i]) {
>> + nvc0_fb_set_null_rt(push, i);
>> + continue;
>> + }
>> +
>> + sf = nv50_surface(fb->cbufs[i]);
>> + res = nv04_resource(sf->base.texture);
>> + bo = res->bo;
>>
>> BEGIN_NVC0(push, NVC0_3D(RT_ADDRESS_HIGH(i)), 9);
>> PUSH_DATAh(push, res->address + sf->offset);
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Nouveau] [PATCH v2] nv50, nvc0: clear out RT on a null cbuf
2014-01-17 2:23 ` [PATCH v2] nv50, nvc0: clear out RT " Ilia Mirkin
2014-01-17 2:23 ` [PATCH] nv50, nvc0: only clear out the buffers that we were asked to clear Ilia Mirkin
@ 2014-01-23 19:40 ` Emil Velikov
[not found] ` <52E1701A.3060203-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
1 sibling, 1 reply; 10+ messages in thread
From: Emil Velikov @ 2014-01-23 19:40 UTC (permalink / raw)
To: Ilia Mirkin, mesa-dev; +Cc: nouveau, emil.l.velikov
On 17/01/14 02:23, Ilia Mirkin wrote:
> This is needed since commit 9baa45f78b (st/mesa: bind NULL colorbuffers
> as specified by glDrawBuffers).
>
> This implementation is highly based on a larger commit by
> Christoph Bumiller <e0425955@student.tuwien.ac.at> in his gallium-nine
> branch.
>
Ilia,
Do you know why we cannot set the rt height to 64? After all you
explicitly set the format to NONE.
Either way this patch looks good afaics
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
> ---
> src/gallium/drivers/nouveau/nv50/nv50_defs.xml.h | 1 +
> src/gallium/drivers/nouveau/nv50/nv50_formats.c | 1 -
> .../drivers/nouveau/nv50/nv50_state_validate.c | 30 +++++++++++++++++++---
> .../drivers/nouveau/nvc0/nvc0_state_validate.c | 28 +++++++++++++++++---
> 4 files changed, 52 insertions(+), 8 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_defs.xml.h b/src/gallium/drivers/nouveau/nv50/nv50_defs.xml.h
> index 2e42843..80de3be 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_defs.xml.h
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_defs.xml.h
> @@ -78,6 +78,7 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> #define NV50_VSTATUS_BLOCKED 0x00000005
> #define NV50_VSTATUS_FAULTED 0x00000006
> #define NV50_VSTATUS_PAUSED 0x00000007
> +#define NV50_SURFACE_FORMAT_NONE 0x00000000
> #define NV50_SURFACE_FORMAT_BITMAP 0x0000001c
> #define NV50_SURFACE_FORMAT_UNK1D 0x0000001d
> #define NV50_SURFACE_FORMAT_RGBA32_FLOAT 0x000000c0
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_formats.c b/src/gallium/drivers/nouveau/nv50/nv50_formats.c
> index 0a7e812..d21905d 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_formats.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_formats.c
> @@ -71,7 +71,6 @@
> # define U_tV U_V
> #endif
>
> -#define NV50_SURFACE_FORMAT_NONE 0
> #define NV50_ZETA_FORMAT_NONE 0
>
> /* for vertex buffers: */
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c b/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
> index 86b9a23..bb05f03 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
> @@ -1,6 +1,19 @@
>
> #include "nv50/nv50_context.h"
> -#include "os/os_time.h"
> +#include "nv50/nv50_defs.xml.h"
> +
> +static INLINE void
> +nv50_fb_set_null_rt(struct nouveau_pushbuf *push, unsigned i)
> +{
> + BEGIN_NV04(push, NV50_3D(RT_ADDRESS_HIGH(i)), 4);
> + PUSH_DATA (push, 0);
> + PUSH_DATA (push, 0);
> + PUSH_DATA (push, NV50_SURFACE_FORMAT_NONE);
> + PUSH_DATA (push, 0);
> + BEGIN_NV04(push, NV50_3D(RT_HORIZ(i)), 2);
> + PUSH_DATA (push, 64);
> + PUSH_DATA (push, 0);
> +}
>
> static void
> nv50_validate_fb(struct nv50_context *nv50)
> @@ -20,9 +33,18 @@ nv50_validate_fb(struct nv50_context *nv50)
> PUSH_DATA (push, fb->height << 16);
>
> for (i = 0; i < fb->nr_cbufs; ++i) {
> - struct nv50_miptree *mt = nv50_miptree(fb->cbufs[i]->texture);
> - struct nv50_surface *sf = nv50_surface(fb->cbufs[i]);
> - struct nouveau_bo *bo = mt->base.bo;
> + struct nv50_miptree *mt;
> + struct nv50_surface *sf;
> + struct nouveau_bo *bo;
> +
> + if (!fb->cbufs[i]) {
> + nv50_fb_set_null_rt(push, i);
> + continue;
> + }
> +
> + mt = nv50_miptree(fb->cbufs[i]->texture);
> + sf = nv50_surface(fb->cbufs[i]);
> + bo = mt->base.bo;
>
> array_size = MIN2(array_size, sf->depth);
> if (mt->layout_3d)
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c b/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
> index 0ba4bad..dd71c65 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
> @@ -2,6 +2,7 @@
> #include "util/u_math.h"
>
> #include "nvc0/nvc0_context.h"
> +#include "nv50/nv50_defs.xml.h"
>
> #if 0
> static void
> @@ -54,6 +55,18 @@ nvc0_validate_zcull(struct nvc0_context *nvc0)
> }
> #endif
>
> +static INLINE void
> +nvc0_fb_set_null_rt(struct nouveau_pushbuf *push, unsigned i)
> +{
> + BEGIN_NVC0(push, NVC0_3D(RT_ADDRESS_HIGH(i)), 6);
> + PUSH_DATA (push, 0);
> + PUSH_DATA (push, 0);
> + PUSH_DATA (push, 64);
> + PUSH_DATA (push, 0);
> + PUSH_DATA (push, NV50_SURFACE_FORMAT_NONE);
> + PUSH_DATA (push, 0);
> +}
> +
> static void
> nvc0_validate_fb(struct nvc0_context *nvc0)
> {
> @@ -72,9 +85,18 @@ nvc0_validate_fb(struct nvc0_context *nvc0)
> PUSH_DATA (push, fb->height << 16);
>
> for (i = 0; i < fb->nr_cbufs; ++i) {
> - struct nv50_surface *sf = nv50_surface(fb->cbufs[i]);
> - struct nv04_resource *res = nv04_resource(sf->base.texture);
> - struct nouveau_bo *bo = res->bo;
> + struct nv50_surface *sf;
> + struct nv04_resource *res;
> + struct nouveau_bo *bo;
> +
> + if (!fb->cbufs[i]) {
> + nvc0_fb_set_null_rt(push, i);
> + continue;
> + }
> +
> + sf = nv50_surface(fb->cbufs[i]);
> + res = nv04_resource(sf->base.texture);
> + bo = res->bo;
>
> BEGIN_NVC0(push, NVC0_3D(RT_ADDRESS_HIGH(i)), 9);
> PUSH_DATAh(push, res->address + sf->offset);
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Mesa-dev] [PATCH] nv50, nvc0: only clear out the buffers that we were asked to clear
[not found] ` <1389925395-22379-2-git-send-email-imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org>
@ 2014-01-23 20:11 ` Emil Velikov
[not found] ` <52E17757.2010807-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Emil Velikov @ 2014-01-23 20:11 UTC (permalink / raw)
To: Ilia Mirkin, mesa-dev-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On 17/01/14 02:23, Ilia Mirkin wrote:
> Fixes fbo-drawbuffers-none glClearBuffer piglit test.
>
> Signed-off-by: Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org>
> ---
>
> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 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);
> + }
> }
> }
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Mesa-dev] [PATCH] nv50, nvc0: only clear out the buffers that we were asked to clear
[not found] ` <52E17757.2010807-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2014-01-23 20:15 ` Ilia Mirkin
2014-01-23 20:31 ` Emil Velikov
0 siblings, 1 reply; 10+ messages in thread
From: Ilia Mirkin @ 2014-01-23 20:15 UTC (permalink / raw)
To: Emil Velikov
Cc: mesa-dev-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
On Thu, Jan 23, 2014 at 3:11 PM, Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 17/01/14 02:23, Ilia Mirkin wrote:
>> Fixes fbo-drawbuffers-none glClearBuffer piglit test.
>>
>> Signed-off-by: Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org>
>> ---
>>
>> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> 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);
>> + }
>> }
>> }
>>
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] nv50, nvc0: clear out RT on a null cbuf
[not found] ` <CAKb7UvgB_5omfkrL1mfFUYHL_3Pi8V01NzJJeo2sbKTfuNYOkA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2014-01-23 20:18 ` Emil Velikov
0 siblings, 0 replies; 10+ messages in thread
From: Emil Velikov @ 2014-01-23 20:18 UTC (permalink / raw)
To: Ilia Mirkin
Cc: mesa-dev-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
On 23/01/14 19:39, Ilia Mirkin wrote:
>> Ilia,
>> > Do you know why we cannot set the rt height to 64? After all you
>> > explicitly set the format to NONE.
> Because then the RT would have a size, which we don't want. The real
> question is why not set the width to 0. It was that way in Christoph's
> code, I assumed he did it that way for a reason.
>
Err. I meant to say
"Why do we set the width to 64", which you've already covered.
-Emil
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] nv50, nvc0: only clear out the buffers that we were asked to clear
2014-01-23 20:15 ` Ilia Mirkin
@ 2014-01-23 20:31 ` Emil Velikov
[not found] ` <52E17C0C.9050305-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Emil Velikov @ 2014-01-23 20:31 UTC (permalink / raw)
To: Ilia Mirkin
Cc: mesa-dev@lists.freedesktop.org, emil.l.velikov,
nouveau@lists.freedesktop.org
On 23/01/14 20:15, Ilia Mirkin wrote:
> On Thu, Jan 23, 2014 at 3:11 PM, Emil Velikov <emil.l.velikov@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@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).
>
With the original patch, you explicitly set clear_depth and
clear_stencil for the first buffer, regardless of what is being passed
by gallium. I'm not sure that was strictly correct.
-Emil
>>
>> 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@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);
>>> + }
>>> }
>>> }
>>>
>>>
>>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Mesa-dev] [PATCH] nv50, nvc0: only clear out the buffers that we were asked to clear
[not found] ` <52E17C0C.9050305-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2014-01-23 20:33 ` Ilia Mirkin
0 siblings, 0 replies; 10+ messages in thread
From: Ilia Mirkin @ 2014-01-23 20:33 UTC (permalink / raw)
To: Emil Velikov
Cc: mesa-dev-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
On Thu, Jan 23, 2014 at 3:31 PM, Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 23/01/14 20:15, Ilia Mirkin wrote:
>> On Thu, Jan 23, 2014 at 3:11 PM, Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> On 17/01/14 02:23, Ilia Mirkin wrote:
>>>> Fixes fbo-drawbuffers-none glClearBuffer piglit test.
>>>>
>>>> Signed-off-by: Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org>
>>>> ---
>>>>
>>>> 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).
>>
> With the original patch, you explicitly set clear_depth and
> clear_stencil for the first buffer, regardless of what is being passed
> by gallium. I'm not sure that was strictly correct.
Only if buffers & PIPE_CLEAR_DEPTH/STENCIL were set... of which there
can only be one, unlike the color buffers, of which there can be many.
Well, whatever. I think everyone's happy with the new version, right?
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-01-23 20:33 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-15 7:30 [PATCH] nv50, nvc0: don't crash on a null cbuf Ilia Mirkin
[not found] ` <1389771040-29447-1-git-send-email-imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org>
2014-01-17 2:23 ` [PATCH v2] nv50, nvc0: clear out RT " Ilia Mirkin
2014-01-17 2:23 ` [PATCH] nv50, nvc0: only clear out the buffers that we were asked to clear Ilia Mirkin
[not found] ` <1389925395-22379-2-git-send-email-imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org>
2014-01-23 20:11 ` [Mesa-dev] " Emil Velikov
[not found] ` <52E17757.2010807-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-01-23 20:15 ` Ilia Mirkin
2014-01-23 20:31 ` Emil Velikov
[not found] ` <52E17C0C.9050305-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-01-23 20:33 ` [Mesa-dev] " Ilia Mirkin
2014-01-23 19:40 ` [Nouveau] [PATCH v2] nv50, nvc0: clear out RT on a null cbuf Emil Velikov
[not found] ` <52E1701A.3060203-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-01-23 19:39 ` Ilia Mirkin
[not found] ` <CAKb7UvgB_5omfkrL1mfFUYHL_3Pi8V01NzJJeo2sbKTfuNYOkA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-01-23 20:18 ` Emil Velikov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.