[Mesa-dev] [PATCH 6/9] panfrost: Don't emit a new FB desc when setting a new FB state

[Alyssa Rosenzweig]

So, in the future, we'll want to have multiple jobs for different
framebuffers independently in flight at the same time. As an
illustrative example, we would like the app to be able to do a sequence
like:

	bind scanout
	clear
	draw something
	bind FBO
	clear
	draw something
	bind scanout
	draw something else
	bind FBO
	draw something else
	bind scanout
	draw the fbo
	flush

Ideally, nothing should happen until the final flush. So the order
submitted to the hardware would be:

	FBO:
	clear
	draw something
	draw something else 
	draw something else

	Scanout:
	clear
	draw something
	draw something else 
	draw the fbo

Unfortunately, right now that panfrost_flush() call makes the order
instead:

	Scanout:
	clear
	draw something
	
	FBO:
	clear
	draw something

	Scanout:
	clear
	wallpaper scanout
	draw something else

	FBO:
	clear
	wallpaper fbo
	draw something else

	Scanout:
	clear
	wallpaper scanout
	draw FBO

Clearly this is extremely suboptimal (I believe the extra wallpaper
blits are related to our poor performance on FBO-heavy apps. glmark's
-bdesktop and -bterrain are suspects here.)

The solution is to be able to have switching framebuffers be a no-op,
since all the important stuff is in the panfrost_job/batch. So
panfrost_set_framebuffer_state is just doing the copy, but no flush and
almost no logic, so we can switch back and forth freely without
incurring a flush/clear/wallpaper cycle each time.

This patch series is not the right time to focus on pipelined
performance (though I'd love if somebody gave it some love at some
point). But I do want to caution from a code smell that will make
whoever that someone is -- quite possibly you! -- quite grumpy when they
work on this in future.

On Fri, Aug 02, 2019 at 12:12:54PM +0200, Boris Brezillon wrote:
> The FB desc will be emitted/attached on the first draw targetting
> this new FB.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon at collabora.com>
> ---
>  src/gallium/drivers/panfrost/pan_context.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c
> index 1091caeb1148..2b7906eea155 100644
> --- a/src/gallium/drivers/panfrost/pan_context.c
> +++ b/src/gallium/drivers/panfrost/pan_context.c
> @@ -2384,6 +2384,9 @@ panfrost_set_framebuffer_state(struct pipe_context *pctx,
>  
>          if (!is_scanout || has_draws)
>                  panfrost_flush(pctx, NULL, PIPE_FLUSH_END_OF_FRAME);
> +        else
> +                assert(!ctx->payloads[PIPE_SHADER_VERTEX].postfix.framebuffer &&
> +                       !ctx->payloads[PIPE_SHADER_FRAGMENT].postfix.framebuffer);
>  
>          util_copy_framebuffer_state(&ctx->pipe_framebuffer, fb);
>  
> @@ -2391,7 +2394,8 @@ panfrost_set_framebuffer_state(struct pipe_context *pctx,
>          struct panfrost_screen *screen = pan_screen(ctx->base.screen);
>  
>          panfrost_hint_afbc(screen, &ctx->pipe_framebuffer);
> -        panfrost_attach_vt_framebuffer(ctx, false);
> +        for (unsigned i = 0; i < PIPE_SHADER_TYPES; ++i)
> +                ctx->payloads[i].postfix.framebuffer = 0;
>  }
>  
>  static void *
> -- 
> 2.21.0
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190802/bc7781d1/attachment.sig>