On Fri, 2 Aug 2019 08:12:15 -0700 Alyssa Rosenzweig <alyssa.rosenzw...@collabora.com> wrote:
> 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. Sounds quite interesting :-). > > 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. Yep, I can imagine this will lead to some heavy refactoring. > > 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.brezil...@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 > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
