[05/12] panfrost: Determine framebuffer format bits late

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

Details

Message ID 20190310065026.3994-6-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.
Again, these formats are only properly known at the time of fragment job
emit. Rather than hardcoding the format, at least for MFBD we begin to
construct the format bits on-demand. This cleans up the code,
futureproofs for ES3 framebuffer formats, and should fix bugs regarding
FBO colour swizzles.

Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
---
 src/gallium/drivers/panfrost/pan_context.c | 59 +++++++++++++++-------
 1 file changed, 42 insertions(+), 17 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 d54b3df5962..9db667d8287 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -143,6 +143,35 @@  panfrost_enable_checksum(struct panfrost_context *ctx, struct panfrost_resource
         rsrc->bo->has_checksum = true;
 }
 
+static unsigned
+panfrost_sfbd_format_for_surface(struct pipe_surface *surf)
+{
+        /* TODO */
+        return 0xb84e0281; /* RGB32, no MSAA */
+}
+
+static struct mali_rt_format
+panfrost_mfbd_format_for_surface(struct pipe_surface *surf)
+{
+        /* Explode details on the format */
+
+        const struct util_format_description *desc =
+                util_format_description(surf->texture->format);
+
+        /* Fill in accordingly */
+
+        struct mali_rt_format fmt = {
+                .unk1 = 0x4000000,
+                .unk2 = 0x1,
+                .nr_channels = MALI_POSITIVE(desc->nr_channels),
+                .flags = 0x444,
+                .swizzle = panfrost_translate_swizzle_4(desc->swizzle),
+                .unk4 = 0x8
+        };
+
+        return fmt;
+}
+
 static bool panfrost_is_scanout(struct panfrost_context *ctx);
 
 /* These routines link a fragment job with the bound surface, accounting for the
@@ -159,6 +188,18 @@  panfrost_set_fragment_target_cbuf(
         signed stride =
                 util_format_get_stride(surf->format, surf->texture->width0);
 
+        /* First, we set the format bits */
+
+        if (require_sfbd) {
+                ctx->fragment_sfbd.format =
+                        panfrost_sfbd_format_for_surface(surf);
+        } else {
+                ctx->fragment_rts[cb].format =
+                        panfrost_mfbd_format_for_surface(surf);
+        }
+
+        /* Now, we set the layout specific pieces */
+
         if (rsrc->bo->layout == PAN_LINEAR) {
                 mali_ptr framebuffer = rsrc->bo->gpu[0];
 
@@ -392,7 +433,6 @@  panfrost_new_frag_framebuffer(struct panfrost_context *ctx)
 {
         if (require_sfbd) {
                 struct mali_single_framebuffer fb = panfrost_emit_sfbd(ctx);
-                fb.format = 0xb84e0281; /* RGB32, no MSAA */
                 memcpy(&ctx->fragment_sfbd, &fb, sizeof(fb));
         } else {
                 struct bifrost_framebuffer fb = panfrost_emit_mfbd(ctx);
@@ -401,24 +441,9 @@  panfrost_new_frag_framebuffer(struct panfrost_context *ctx)
                 fb.rt_count_2 = 1;
                 fb.unk3 = 0x100;
 
-                /* By default, Gallium seems to need a BGR framebuffer */
-                unsigned char bgra[4] = {
-                        PIPE_SWIZZLE_Z, PIPE_SWIZZLE_Y, PIPE_SWIZZLE_X, PIPE_SWIZZLE_W
-                };
-
-                struct bifrost_render_target rt = {
-                        .format = {
-                                .unk1 = 0x4000000,
-                                .unk2 = 0x1,
-                                .nr_channels = MALI_POSITIVE(4),
-                                .flags = 0x444,
-                                .swizzle = panfrost_translate_swizzle_4(bgra),
-                                .unk4 = 0x8
-                        },
-                };
+                struct bifrost_render_target rt = {};
 
                 memcpy(&ctx->fragment_rts[0], &rt, sizeof(rt));
-
                 memset(&ctx->fragment_extra, 0, sizeof(ctx->fragment_extra));
                 memcpy(&ctx->fragment_mfbd, &fb, sizeof(fb));
         }

Comments

On Sun, 10 Mar 2019 at 07:50, Alyssa Rosenzweig <alyssa@rosenzweig.io> wrote:
>
> Again, these formats are only properly known at the time of fragment job
> emit. Rather than hardcoding the format, at least for MFBD we begin to
> construct the format bits on-demand. This cleans up the code,
> futureproofs for ES3 framebuffer formats, and should fix bugs regarding
> FBO colour swizzles.
>
> Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
> ---
>  src/gallium/drivers/panfrost/pan_context.c | 59 +++++++++++++++-------
>  1 file changed, 42 insertions(+), 17 deletions(-)
>
> diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c
> index d54b3df5962..9db667d8287 100644
> --- a/src/gallium/drivers/panfrost/pan_context.c
> +++ b/src/gallium/drivers/panfrost/pan_context.c
> @@ -143,6 +143,35 @@ panfrost_enable_checksum(struct panfrost_context *ctx, struct panfrost_resource
>          rsrc->bo->has_checksum = true;
>  }
>
> +static unsigned
> +panfrost_sfbd_format_for_surface(struct pipe_surface *surf)
> +{
> +        /* TODO */
> +        return 0xb84e0281; /* RGB32, no MSAA */

Can we use a constant instead?

In any case,

Reviewed-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

Thanks,

Tomeu

> +}
> +
> +static struct mali_rt_format
> +panfrost_mfbd_format_for_surface(struct pipe_surface *surf)
> +{
> +        /* Explode details on the format */
> +
> +        const struct util_format_description *desc =
> +                util_format_description(surf->texture->format);
> +
> +        /* Fill in accordingly */
> +
> +        struct mali_rt_format fmt = {
> +                .unk1 = 0x4000000,
> +                .unk2 = 0x1,
> +                .nr_channels = MALI_POSITIVE(desc->nr_channels),
> +                .flags = 0x444,
> +                .swizzle = panfrost_translate_swizzle_4(desc->swizzle),
> +                .unk4 = 0x8
> +        };
> +
> +        return fmt;
> +}
> +
>  static bool panfrost_is_scanout(struct panfrost_context *ctx);
>
>  /* These routines link a fragment job with the bound surface, accounting for the
> @@ -159,6 +188,18 @@ panfrost_set_fragment_target_cbuf(
>          signed stride =
>                  util_format_get_stride(surf->format, surf->texture->width0);
>
> +        /* First, we set the format bits */
> +
> +        if (require_sfbd) {
> +                ctx->fragment_sfbd.format =
> +                        panfrost_sfbd_format_for_surface(surf);
> +        } else {
> +                ctx->fragment_rts[cb].format =
> +                        panfrost_mfbd_format_for_surface(surf);
> +        }
> +
> +        /* Now, we set the layout specific pieces */
> +
>          if (rsrc->bo->layout == PAN_LINEAR) {
>                  mali_ptr framebuffer = rsrc->bo->gpu[0];
>
> @@ -392,7 +433,6 @@ panfrost_new_frag_framebuffer(struct panfrost_context *ctx)
>  {
>          if (require_sfbd) {
>                  struct mali_single_framebuffer fb = panfrost_emit_sfbd(ctx);
> -                fb.format = 0xb84e0281; /* RGB32, no MSAA */
>                  memcpy(&ctx->fragment_sfbd, &fb, sizeof(fb));
>          } else {
>                  struct bifrost_framebuffer fb = panfrost_emit_mfbd(ctx);
> @@ -401,24 +441,9 @@ panfrost_new_frag_framebuffer(struct panfrost_context *ctx)
>                  fb.rt_count_2 = 1;
>                  fb.unk3 = 0x100;
>
> -                /* By default, Gallium seems to need a BGR framebuffer */
> -                unsigned char bgra[4] = {
> -                        PIPE_SWIZZLE_Z, PIPE_SWIZZLE_Y, PIPE_SWIZZLE_X, PIPE_SWIZZLE_W
> -                };
> -
> -                struct bifrost_render_target rt = {
> -                        .format = {
> -                                .unk1 = 0x4000000,
> -                                .unk2 = 0x1,
> -                                .nr_channels = MALI_POSITIVE(4),
> -                                .flags = 0x444,
> -                                .swizzle = panfrost_translate_swizzle_4(bgra),
> -                                .unk4 = 0x8
> -                        },
> -                };
> +                struct bifrost_render_target rt = {};
>
>                  memcpy(&ctx->fragment_rts[0], &rt, sizeof(rt));
> -
>                  memset(&ctx->fragment_extra, 0, sizeof(ctx->fragment_extra));
>                  memcpy(&ctx->fragment_mfbd, &fb, sizeof(fb));
>          }
> --
> 2.20.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> Can we use a constant instead?

The right solution is to actually RE the format bits for SFBD (which
will be necessary if we're serious about supporting <T760); what we have
here is no worse than before..