[04/12] panfrost: Cleanup zsbuf emit in fragment job

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

Details

Message ID 20190310065026.3994-5-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.
This emit is only implemented for AFBC depth/stencil buffers, so
conceptually there is no change here, but we follow the style of the
previous patch to improve robustness and clarity.

Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
---
 src/gallium/drivers/panfrost/pan_context.c | 71 +++++++++++++---------
 1 file changed, 42 insertions(+), 29 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c
index 71634c781a3..d54b3df5962 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -199,46 +199,61 @@  panfrost_set_fragment_target_cbuf(
 }
 
 static void
-panfrost_set_fragment_target(struct panfrost_context *ctx)
+panfrost_set_fragment_target_zsbuf(
+                struct panfrost_context *ctx,
+                struct pipe_surface *surf)
 {
-        for (int cb = 0; cb < ctx->pipe_framebuffer.nr_cbufs; ++cb) {
-                struct pipe_surface *surf = ctx->pipe_framebuffer.cbufs[cb];
-                panfrost_set_fragment_target_cbuf(ctx, surf, cb);
-        }
+        struct panfrost_resource *rsrc = pan_resource(surf->texture);
 
-        /* Enable depth/stencil AFBC for the framebuffer (not the render target) */
-        if (ctx->pipe_framebuffer.zsbuf) {
-                struct panfrost_resource *rsrc = (struct panfrost_resource *) ctx->pipe_framebuffer.zsbuf->texture;
+        if (rsrc->bo->layout == PAN_AFBC) {
+                /* TODO: AFBC on SFBD */
+                assert(!require_sfbd);
 
-                if (rsrc->bo->layout == PAN_AFBC) {
-                        if (require_sfbd) {
-                                fprintf(stderr, "Depth AFBC not supported on SFBD\n");
-                                assert(0);
-                        }
+                ctx->fragment_mfbd.unk3 |= MALI_MFBD_EXTRA;
 
-                        ctx->fragment_mfbd.unk3 |= MALI_MFBD_EXTRA;
+                ctx->fragment_extra.ds_afbc.depth_stencil_afbc_metadata = rsrc->bo->afbc_slab.gpu;
+                ctx->fragment_extra.ds_afbc.depth_stencil_afbc_stride = 0;
 
-                        ctx->fragment_extra.ds_afbc.depth_stencil_afbc_metadata = rsrc->bo->afbc_slab.gpu;
-                        ctx->fragment_extra.ds_afbc.depth_stencil_afbc_stride = 0;
+                ctx->fragment_extra.ds_afbc.depth_stencil = rsrc->bo->afbc_slab.gpu + rsrc->bo->afbc_metadata_size;
 
-                        ctx->fragment_extra.ds_afbc.depth_stencil = rsrc->bo->afbc_slab.gpu + rsrc->bo->afbc_metadata_size;
+                ctx->fragment_extra.ds_afbc.zero1 = 0x10009;
+                ctx->fragment_extra.ds_afbc.padding = 0x1000;
 
-                        ctx->fragment_extra.ds_afbc.zero1 = 0x10009;
-                        ctx->fragment_extra.ds_afbc.padding = 0x1000;
+                /* There's a general 0x400 in all versions of this field scene.
+                 * ORed with 0x5 for depth/stencil. ORed 0x10 for AFBC encoded
+                 * depth stencil. It's unclear where the remaining 0x20 bit is
+                 * from */
 
-                        ctx->fragment_extra.unk = 0x435; /* General 0x400 in all unks. 0x5 for depth/stencil. 0x10 for AFBC encoded depth stencil. Unclear where the 0x20 is from */
+                ctx->fragment_extra.unk = 0x400 | 0x20 | 0x10 | 0x5;
 
-                        ctx->fragment_mfbd.unk3 |= 0x400;
-                }
+                ctx->fragment_mfbd.unk3 |= 0x400;
+        } else if (rsrc->bo->layout == PAN_LINEAR) {
+                /* TODO */
+        } else {
+                fprintf(stderr, "Invalid render layout (zsbuf)");
+                assert(0);
         }
+}
 
-        /* For the special case of a depth-only FBO, we need to attach a dummy render target */
+static void
+panfrost_set_fragment_target(struct panfrost_context *ctx)
+{
+        for (int cb = 0; cb < ctx->pipe_framebuffer.nr_cbufs; ++cb) {
+                struct pipe_surface *surf = ctx->pipe_framebuffer.cbufs[cb];
+                panfrost_set_fragment_target_cbuf(ctx, surf, cb);
+        }
+
+        if (ctx->pipe_framebuffer.zsbuf) {
+                struct pipe_surface *surf = ctx->pipe_framebuffer.zsbuf;
+                panfrost_set_fragment_target_zsbuf(ctx, surf);
+
+       }
+
+        /* There must always be at least one render-target, so attach a dummy
+         * if necessary */
 
         if (ctx->pipe_framebuffer.nr_cbufs == 0) {
-                if (require_sfbd) {
-                        fprintf(stderr, "Depth-only FBO not supported on SFBD\n");
-                        assert(0);
-                }
+                assert(!require_sfbd);
 
                 struct mali_rt_format null_rt = {
                         .unk1 = 0x4000000,
@@ -246,8 +261,6 @@  panfrost_set_fragment_target(struct panfrost_context *ctx)
                 };
 
                 ctx->fragment_rts[0].format = null_rt;
-                ctx->fragment_rts[0].framebuffer = 0;
-                ctx->fragment_rts[0].framebuffer_stride = 0;
         }
 }
 

Comments

On Sun, 10 Mar 2019 at 07:50, Alyssa Rosenzweig <alyssa@rosenzweig.io> wrote:
>
> This emit is only implemented for AFBC depth/stencil buffers, so
> conceptually there is no change here, but we follow the style of the
> previous patch to improve robustness and clarity.
>
> Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
> ---
>  src/gallium/drivers/panfrost/pan_context.c | 71 +++++++++++++---------
>  1 file changed, 42 insertions(+), 29 deletions(-)
>
> diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c
> index 71634c781a3..d54b3df5962 100644
> --- a/src/gallium/drivers/panfrost/pan_context.c
> +++ b/src/gallium/drivers/panfrost/pan_context.c
> @@ -199,46 +199,61 @@ panfrost_set_fragment_target_cbuf(
>  }
>
>  static void
> -panfrost_set_fragment_target(struct panfrost_context *ctx)
> +panfrost_set_fragment_target_zsbuf(
> +                struct panfrost_context *ctx,
> +                struct pipe_surface *surf)
>  {
> -        for (int cb = 0; cb < ctx->pipe_framebuffer.nr_cbufs; ++cb) {
> -                struct pipe_surface *surf = ctx->pipe_framebuffer.cbufs[cb];
> -                panfrost_set_fragment_target_cbuf(ctx, surf, cb);
> -        }
> +        struct panfrost_resource *rsrc = pan_resource(surf->texture);
>
> -        /* Enable depth/stencil AFBC for the framebuffer (not the render target) */
> -        if (ctx->pipe_framebuffer.zsbuf) {
> -                struct panfrost_resource *rsrc = (struct panfrost_resource *) ctx->pipe_framebuffer.zsbuf->texture;
> +        if (rsrc->bo->layout == PAN_AFBC) {
> +                /* TODO: AFBC on SFBD */
> +                assert(!require_sfbd);
>
> -                if (rsrc->bo->layout == PAN_AFBC) {
> -                        if (require_sfbd) {
> -                                fprintf(stderr, "Depth AFBC not supported on SFBD\n");
> -                                assert(0);
> -                        }
> +                ctx->fragment_mfbd.unk3 |= MALI_MFBD_EXTRA;
>
> -                        ctx->fragment_mfbd.unk3 |= MALI_MFBD_EXTRA;
> +                ctx->fragment_extra.ds_afbc.depth_stencil_afbc_metadata = rsrc->bo->afbc_slab.gpu;
> +                ctx->fragment_extra.ds_afbc.depth_stencil_afbc_stride = 0;
>
> -                        ctx->fragment_extra.ds_afbc.depth_stencil_afbc_metadata = rsrc->bo->afbc_slab.gpu;
> -                        ctx->fragment_extra.ds_afbc.depth_stencil_afbc_stride = 0;
> +                ctx->fragment_extra.ds_afbc.depth_stencil = rsrc->bo->afbc_slab.gpu + rsrc->bo->afbc_metadata_size;
>
> -                        ctx->fragment_extra.ds_afbc.depth_stencil = rsrc->bo->afbc_slab.gpu + rsrc->bo->afbc_metadata_size;
> +                ctx->fragment_extra.ds_afbc.zero1 = 0x10009;
> +                ctx->fragment_extra.ds_afbc.padding = 0x1000;
>
> -                        ctx->fragment_extra.ds_afbc.zero1 = 0x10009;
> -                        ctx->fragment_extra.ds_afbc.padding = 0x1000;
> +                /* There's a general 0x400 in all versions of this field scene.
> +                 * ORed with 0x5 for depth/stencil. ORed 0x10 for AFBC encoded
> +                 * depth stencil. It's unclear where the remaining 0x20 bit is
> +                 * from */
>
> -                        ctx->fragment_extra.unk = 0x435; /* General 0x400 in all unks. 0x5 for depth/stencil. 0x10 for AFBC encoded depth stencil. Unclear where the 0x20 is from */
> +                ctx->fragment_extra.unk = 0x400 | 0x20 | 0x10 | 0x5;
>
> -                        ctx->fragment_mfbd.unk3 |= 0x400;

The patch looks good to me, but this line is causing failures here
with the DRM driver (haven't tested with Arm's).

Regards,

Tomeu

> -                }
> +                ctx->fragment_mfbd.unk3 |= 0x400;
> +        } else if (rsrc->bo->layout == PAN_LINEAR) {
> +                /* TODO */
> +        } else {
> +                fprintf(stderr, "Invalid render layout (zsbuf)");
> +                assert(0);
>          }
> +}
>
> -        /* For the special case of a depth-only FBO, we need to attach a dummy render target */
> +static void
> +panfrost_set_fragment_target(struct panfrost_context *ctx)
> +{
> +        for (int cb = 0; cb < ctx->pipe_framebuffer.nr_cbufs; ++cb) {
> +                struct pipe_surface *surf = ctx->pipe_framebuffer.cbufs[cb];
> +                panfrost_set_fragment_target_cbuf(ctx, surf, cb);
> +        }
> +
> +        if (ctx->pipe_framebuffer.zsbuf) {
> +                struct pipe_surface *surf = ctx->pipe_framebuffer.zsbuf;
> +                panfrost_set_fragment_target_zsbuf(ctx, surf);
> +
> +       }
> +
> +        /* There must always be at least one render-target, so attach a dummy
> +         * if necessary */
>
>          if (ctx->pipe_framebuffer.nr_cbufs == 0) {
> -                if (require_sfbd) {
> -                        fprintf(stderr, "Depth-only FBO not supported on SFBD\n");
> -                        assert(0);
> -                }
> +                assert(!require_sfbd);
>
>                  struct mali_rt_format null_rt = {
>                          .unk1 = 0x4000000,
> @@ -246,8 +261,6 @@ panfrost_set_fragment_target(struct panfrost_context *ctx)
>                  };
>
>                  ctx->fragment_rts[0].format = null_rt;
> -                ctx->fragment_rts[0].framebuffer = 0;
> -                ctx->fragment_rts[0].framebuffer_stride = 0;
>          }
>  }
>
> --
> 2.20.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev