On Fri, 20 Sep 2019 15:45:33 -0400
Alyssa Rosenzweig <aly...@rosenzweig.io> wrote:
> To be clear, if we have a batch and do the following operations:
>
> clear red
> draw 1
> clear green
> draw 2
> flush
>
> All we should see is #2 on a green background, which this patch handles
> by the second clear invalidating all the clears/draws that came before
> it (provided there is no flush in between).
>
> I might just be tripped up by the "freeze" name. That really means throw
> away / free here, I guess?
Nope. Freeze means "stop queuing new draws to this batch". I guess we
could free the batch as well if the result of the previous draws/clear
are really overwritten by this new clear, but that implies checking the
new clear flags to make sure they target the same depth/stencil/color
combination. On the other hand, I'm wondering if it's really the
driver's job to try to optimize silly things the apps might do. I mean,
the sequence you describe does not look like a wise thing to do since
the "clear red+draw 1" end up being overwritten by "clear green + draw
2".
>
> Provided that's the idea (and we're not somehow saving the original draw
> 1), it's Reviewed-by A R <a...@c.com>
>
> On Fri, Sep 20, 2019 at 04:53:37PM +0200, Boris Brezillon wrote:
> > glClear()s are expected to be the first thing GL apps do before drawing
> > new things. If there's already an existing batch targetting the same
> > FBO that has draws attached to it, we should make sure the new clear
> > gets a new batch assigned to guaranteed that the FB content is actually
> > cleared with the requested color/depth/stencil values.
> >
> > We create a panfrost_get_fresh_batch_for_fbo() helper for that and
> > call it from panfrost_clear().
> >
> > Signed-off-by: Boris Brezillon <boris.brezil...@collabora.com>
> > ---
> > src/gallium/drivers/panfrost/pan_context.c | 2 +-
> > src/gallium/drivers/panfrost/pan_job.c | 21 +++++++++++++++++++++
> > src/gallium/drivers/panfrost/pan_job.h | 3 +++
> > 3 files changed, 25 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/gallium/drivers/panfrost/pan_context.c
> > b/src/gallium/drivers/panfrost/pan_context.c
> > index ac01461a07fe..b2f2a9da7a51 100644
> > --- a/src/gallium/drivers/panfrost/pan_context.c
> > +++ b/src/gallium/drivers/panfrost/pan_context.c
> > @@ -162,7 +162,7 @@ panfrost_clear(
> > double depth, unsigned stencil)
> > {
> > struct panfrost_context *ctx = pan_context(pipe);
> > - struct panfrost_batch *batch = panfrost_get_batch_for_fbo(ctx);
> > + struct panfrost_batch *batch =
> > panfrost_get_fresh_batch_for_fbo(ctx);
> >
> > panfrost_batch_add_fbo_bos(batch);
> > panfrost_batch_clear(batch, buffers, color, depth, stencil);
> > diff --git a/src/gallium/drivers/panfrost/pan_job.c
> > b/src/gallium/drivers/panfrost/pan_job.c
> > index d8330bc133a6..4ec2aa0565d7 100644
> > --- a/src/gallium/drivers/panfrost/pan_job.c
> > +++ b/src/gallium/drivers/panfrost/pan_job.c
> > @@ -298,6 +298,27 @@ panfrost_get_batch_for_fbo(struct panfrost_context
> > *ctx)
> > return batch;
> > }
> >
> > +struct panfrost_batch *
> > +panfrost_get_fresh_batch_for_fbo(struct panfrost_context *ctx)
> > +{
> > + struct panfrost_batch *batch;
> > +
> > + batch = panfrost_get_batch(ctx, &ctx->pipe_framebuffer);
> > +
> > + /* The batch has no draw/clear queued, let's return it directly.
> > + * Note that it's perfectly fine to re-use a batch with an
> > + * existing clear, we'll just update it with the new clear request.
> > + */
> > + if (!batch->last_job.gpu)
> > + return batch;
> > +
> > + /* Otherwise, we need to freeze the existing one and instantiate a
> > new
> > + * one.
> > + */
> > + panfrost_freeze_batch(batch);
> > + return panfrost_get_batch(ctx, &ctx->pipe_framebuffer);
> > +}
> > +
> > static bool
> > panfrost_batch_fence_is_signaled(struct panfrost_batch_fence *fence)
> > {
> > diff --git a/src/gallium/drivers/panfrost/pan_job.h
> > b/src/gallium/drivers/panfrost/pan_job.h
> > index e1b1f56a2e64..0bd78bba267a 100644
> > --- a/src/gallium/drivers/panfrost/pan_job.h
> > +++ b/src/gallium/drivers/panfrost/pan_job.h
> > @@ -172,6 +172,9 @@ panfrost_batch_fence_reference(struct
> > panfrost_batch_fence *batch);
> > struct panfrost_batch *
> > panfrost_get_batch_for_fbo(struct panfrost_context *ctx);
> >
> > +struct panfrost_batch *
> > +panfrost_get_fresh_batch_for_fbo(struct panfrost_context *ctx);
> > +
> > void
> > panfrost_batch_init(struct panfrost_context *ctx);
> >
> > --
> > 2.21.0
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
