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

[Pohjolainen, Topi]

On Fri, Jan 20, 2017 at 08:40:50AM -0800, Jason Ekstrand wrote:
>    On Thu, Jan 19, 2017 at 11:48 PM, Pohjolainen, Topi
>    <[1]topi.pohjolainen at gmail.com> wrote:
> 
>      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][2]currojerez at riseup.net> wrote:
>      >
>      >      "Pohjolainen, Topi" <[2][3]topi.pohjolainen at gmail.com>
>      writes:
>      >      > On Thu, Jan 19, 2017 at 12:10:02PM -0800, Francisco Jerez
>      wrote:
>      >      >> Topi Pohjolainen <[3][4]topi.pohjolainen at gmail.com>
>      writes:
>      >      >>
>      >      >> > CC: Francisco Jerez <[4][5]currojerez at riseup.net>
>      >      >> > CC: Kenneth Graunke <[5][6]kenneth at whitecape.org>
>      >      >> > CC: Jason Ekstrand <[6][7]jason at jlekstrand.net>
>      >      >> > Signed-off-by: Topi Pohjolainen
>      <[7][8]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?
> 
>    Ken, Curro, and I had a little chat about this in the office.  I think
>    the conclusion we came to was the following:
>    1) The patches to add flushing around HiZ ops and fast clear ops should
>    get back-ported all the way to 13.0.  They fix potentially serious bugs
>    that could cause problems.

You mean thse two? They apply on 13.0 without any tweaks:

i965/gen6: Issue direct depth stall and flush after depth clear
i965: Make depth clear flushing more explicit

>    2) The patches that switch us over to the render cache should get
>    backported to 17.0.  They aren't so much a bug fix as an enhancement
>    but keeping 17.0 consistent with future will help in backporting other
>    fixes.  For the record, this was me and Ken; Curro preferred to not
>    backport these.

And all the four apply clean on 17.0.

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