[Mesa-dev] [PATCH] i965/blorp: Add also depth buffer to render cache

[Pohjolainen, Topi]

On Thu, Jan 19, 2017 at 01:39:49PM -0800, Jason Ekstrand wrote:
>    On Thu, Jan 19, 2017 at 12:40 PM, Francisco Jerez
>    <[1]currojerez at riseup.net> wrote:
> 
>      "Pohjolainen, Topi" <[2]topi.pohjolainen at gmail.com> writes:
>      > On Thu, Jan 19, 2017 at 12:10:02PM -0800, Francisco Jerez wrote:
>      >> Topi Pohjolainen <[3]topi.pohjolainen at gmail.com> writes:
>      >>
>      >> > CC: Francisco Jerez <[4]currojerez at riseup.net>
>      >> > CC: Kenneth Graunke <[5]kenneth at whitecape.org>
>      >> > CC: Jason Ekstrand <[6]jason at jlekstrand.net>
>      >> > Signed-off-by: Topi Pohjolainen <[7]topi.pohjolainen at intel.com>
>      >> > ---
>      >> >  src/mesa/drivers/dri/i965/genX_blorp_exec.c | 3 +++
>      >> >  1 file changed, 3 insertions(+)
>      >> >
>      >> > diff --git a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
>      b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
>      >> > index 647a362..594bd5a 100644
>      >> > --- a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
>      >> > +++ b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
>      >> > @@ -261,4 +261,7 @@ retry:
>      >> >
>      >> >     if (params->dst.enabled)
>      >> >        brw_render_cache_set_add_bo(brw,
>      params->dst.addr.buffer);
>      >> > +
>      >> > +   if (params->depth.enabled)
>      >> > +      brw_render_cache_set_add_bo(brw,
>      params->depth.addr.buffer);
>      >>
>      >> What about the stencil buffer?  Stencil texturing is likely to be
>      >> unhappy unless you mark it as pending flush as well...
>      >
>      > As far as I know i965 only clears depth and color using blorp,
>      stencil gets
>      > cleared using meta. Blits in turn have it as destination.
>      >
>      That doesn't sound like a safe assumption to rely on looking forward
>      if
>      the blorp api already exposes support for stencil writes -- Tracking
>      down the ultimate cause of a memory coherency bugs can be really
>      hard,
>      why make our future lives more intentionally difficult by
>      introducing
>      buggy corner cases like this?  The extra check is not going to hurt
>      performance or cause any other harmful side effects unless stencil
>      writes are used...
> 
>    Agreed.  Let's stick it in there for stencil too.  I've got patches to
>    switch i965 over to blorp for depth/stencil blits.  I never landed them
>    because of what was most likely flushing bugs.  I'm hoping that you've
>    fixed those and I'll revive the patches.
>    Also, please make sure these fixes hit stable.

This sits on top the four earlier patches. Rebasing this alone against stable
requires manual work but can be done. How do you want to handle that?

>    --Jason
> 
> References
> 
>    1. mailto:currojerez at riseup.net
>    2. mailto:topi.pohjolainen at gmail.com
>    3. mailto:topi.pohjolainen at gmail.com
>    4. mailto:currojerez at riseup.net
>    5. mailto:kenneth at whitecape.org
>    6. mailto:jason at jlekstrand.net
>    7. mailto:topi.pohjolainen at intel.com