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

[Jason Ekstrand]

On Mon, Jan 23, 2017 at 7:47 AM, Pohjolainen, Topi <
topi.pohjolainen at gmail.com> wrote:

> 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
>

Yes, those.


> >    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.
>

Cool


> >
> > 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170123/96707191/attachment.html>