[10/12] panfrost: Document "depth buffer writeback" bit

Submitted by Alyssa Rosenzweig on March 10, 2019, 6:50 a.m.

Details

Message ID 20190310065026.3994-11-alyssa@rosenzweig.io
State New
Headers show
Series "Refactors related to BO layouts" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Alyssa Rosenzweig March 10, 2019, 6:50 a.m.
We were setting this bit as a magic value for a while when using depth
FBOs, but it is only now clear what the meaning is.

Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
---
 src/gallium/drivers/panfrost/include/panfrost-job.h | 8 ++++++++
 src/gallium/drivers/panfrost/pan_context.c          | 5 ++---
 2 files changed, 10 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/gallium/drivers/panfrost/include/panfrost-job.h b/src/gallium/drivers/panfrost/include/panfrost-job.h
index d719325d07b..3c5ed2bc802 100644
--- a/src/gallium/drivers/panfrost/include/panfrost-job.h
+++ b/src/gallium/drivers/panfrost/include/panfrost-job.h
@@ -1457,6 +1457,14 @@  struct bifrost_fb_extra {
 } __attribute__((packed));
 
 /* flags for unk3 */
+
+/* Enables writing depth results back to main memory (rather than keeping them
+ * on-chip in the tile buffer and then discarding) */
+
+#define MALI_MFBD_DEPTH_WRITE (1 << 10)
+
+/* The MFBD contains the extra bifrost_fb_extra section*/
+
 #define MALI_MFBD_EXTRA (1 << 13)
 
 struct bifrost_framebuffer {
diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c
index 37f6ce4910f..419c1a4eb6f 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -270,7 +270,7 @@  panfrost_set_fragment_target_zsbuf(
 
                 ctx->fragment_extra.unk = 0x400 | 0x20 | 0x10 | 0x5;
 
-                ctx->fragment_mfbd.unk3 |= 0x400;
+                ctx->fragment_mfbd.unk3 |= MALI_MFBD_DEPTH_WRITE;
         } else if (rsrc->bo->layout == PAN_LINEAR) {
                 /* TODO: explicit depth on SFBD */
                 assert(!require_sfbd);
@@ -282,8 +282,7 @@  panfrost_set_fragment_target_zsbuf(
                 ctx->fragment_extra.ds_linear.depth = rsrc->bo->gpu[0];
                 ctx->fragment_extra.ds_linear.depth_stride = stride;
 
-                /* Seemingly means "write to depth buffer" */
-                ctx->fragment_mfbd.unk3 |= 0x400;
+                ctx->fragment_mfbd.unk3 |= MALI_MFBD_DEPTH_WRITE;
         } else {
                 fprintf(stderr, "Invalid render layout (zsbuf)");
                 assert(0);

Comments

On Sun, 10 Mar 2019 at 07:50, Alyssa Rosenzweig <alyssa@rosenzweig.io> wrote:
>
> We were setting this bit as a magic value for a while when using depth
> FBOs, but it is only now clear what the meaning is.
>
> Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
> ---
>  src/gallium/drivers/panfrost/include/panfrost-job.h | 8 ++++++++
>  src/gallium/drivers/panfrost/pan_context.c          | 5 ++---
>  2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/drivers/panfrost/include/panfrost-job.h b/src/gallium/drivers/panfrost/include/panfrost-job.h
> index d719325d07b..3c5ed2bc802 100644
> --- a/src/gallium/drivers/panfrost/include/panfrost-job.h
> +++ b/src/gallium/drivers/panfrost/include/panfrost-job.h
> @@ -1457,6 +1457,14 @@ struct bifrost_fb_extra {
>  } __attribute__((packed));
>
>  /* flags for unk3 */
> +
> +/* Enables writing depth results back to main memory (rather than keeping them
> + * on-chip in the tile buffer and then discarding) */
> +
> +#define MALI_MFBD_DEPTH_WRITE (1 << 10)
> +
> +/* The MFBD contains the extra bifrost_fb_extra section*/
> +
>  #define MALI_MFBD_EXTRA (1 << 13)
>
>  struct bifrost_framebuffer {
> diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c
> index 37f6ce4910f..419c1a4eb6f 100644
> --- a/src/gallium/drivers/panfrost/pan_context.c
> +++ b/src/gallium/drivers/panfrost/pan_context.c
> @@ -270,7 +270,7 @@ panfrost_set_fragment_target_zsbuf(
>
>                  ctx->fragment_extra.unk = 0x400 | 0x20 | 0x10 | 0x5;
>
> -                ctx->fragment_mfbd.unk3 |= 0x400;
> +                ctx->fragment_mfbd.unk3 |= MALI_MFBD_DEPTH_WRITE;
>          } else if (rsrc->bo->layout == PAN_LINEAR) {
>                  /* TODO: explicit depth on SFBD */
>                  assert(!require_sfbd);
> @@ -282,8 +282,7 @@ panfrost_set_fragment_target_zsbuf(
>                  ctx->fragment_extra.ds_linear.depth = rsrc->bo->gpu[0];
>                  ctx->fragment_extra.ds_linear.depth_stride = stride;
>
> -                /* Seemingly means "write to depth buffer" */
> -                ctx->fragment_mfbd.unk3 |= 0x400;
> +                ctx->fragment_mfbd.unk3 |= MALI_MFBD_DEPTH_WRITE;

When testing this series with the DRM driver, every other subtest run
in glmark2-drm will trigger JS faults and fail to render:

[build] use-vbo=false: FPS: 59 FrameTime: 16.949 ms
[build] use-vbo=true: FPS: 1 FrameTime: 1000.000 ms
[texture] texture-filter=nearest: FPS: 60 FrameTime: 16.667 ms
[texture] texture-filter=linear: FPS: 1 FrameTime: 1000.000 ms
[texture] texture-filter=mipmap: FPS: 60 FrameTime: 16.667 ms
etc.

If I comment the line immediately above, things work as expected.

Cheers,

Tomeu

>          } else {
>                  fprintf(stderr, "Invalid render layout (zsbuf)");
>                  assert(0);
> --
> 2.20.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> If I comment the line immediately above, things work as expected.

Interesting, okay, this is definitely a regression on kbase too. I'll
investigate this evening -- I think I need to be checking the depth mask
before setting this bit..?