[Mesa-dev] [PATCH v2 02/14] i965/gen7: Don't rely directly on the hiz miptree structure

[Ben Widawsky]

On Tue, Nov 11, 2014 at 03:06:03PM -0800, Jordan Justen wrote:
> On 2014-11-10 10:51:17, Ben Widawsky wrote:
> > On Mon, Jul 21, 2014 at 11:00:51PM -0700, Jordan Justen wrote:
> > > We are still allocating a miptree for hiz, but we only use fields from
> > > intel_miptree_aux_buffer. This will allow us to switch over to not
> > > allocating a miptree.
> > > 
> > > Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> > > Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> > > ---
> > >  src/mesa/drivers/dri/i965/gen7_blorp.cpp    | 6 +++---
> > >  src/mesa/drivers/dri/i965/gen7_misc_state.c | 7 ++++---
> > >  2 files changed, 7 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > > index d0d623d..ebfe6d1 100644
> > > --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > > +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > > @@ -752,13 +752,13 @@ gen7_blorp_emit_depth_stencil_config(struct brw_context *brw,
> > >  
> > >     /* 3DSTATE_HIER_DEPTH_BUFFER */
> > >     {
> > > -      struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_buf->mt;
> > > +      struct intel_miptree_aux_buffer *hiz_buf = params->depth.mt->hiz_buf;
> > >  
> > >        BEGIN_BATCH(3);
> > >        OUT_BATCH((GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16) | (3 - 2));
> > >        OUT_BATCH((mocs << 25) |
> > > -                (hiz_mt->pitch - 1));
> > > -      OUT_RELOC(hiz_mt->bo,
> > > +                (hiz_buf->pitch - 1));
> > > +      OUT_RELOC(hiz_buf->bo,
> > >                  I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
> > >                  0);
> > >        ADVANCE_BATCH();
> > > diff --git a/src/mesa/drivers/dri/i965/gen7_misc_state.c b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> > > index 6c6a79b..8f8c33e 100644
> > > --- a/src/mesa/drivers/dri/i965/gen7_misc_state.c
> > > +++ b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> > > @@ -145,12 +145,13 @@ gen7_emit_depth_stencil_hiz(struct brw_context *brw,
> > >        OUT_BATCH(0);
> > >        ADVANCE_BATCH();
> > >     } else {
> > > -      struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_buf->mt;
> > > +      struct intel_miptree_aux_buffer *hiz_buf = depth_mt->hiz_buf;
> > > +
> > >        BEGIN_BATCH(3);
> > >        OUT_BATCH(GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16 | (3 - 2));
> > >        OUT_BATCH((mocs << 25) |
> > > -                (hiz_mt->pitch - 1));
> > > -      OUT_RELOC(hiz_mt->bo,
> > > +                (hiz_buf->pitch - 1));
> > > +      OUT_RELOC(hiz_buf->bo,
> > >                  I915_GEM_DOMAIN_RENDER,
> > >                  I915_GEM_DOMAIN_RENDER,
> > >                  0);
> > > -- 
> > 
> > Just a simple grep turns up a few places that weren't modified.  I am
> > not sure if you intentionally left out the <= gen6 state, but just for
> > the pedant:
> > 
> > brw_emit_depth_stencil_hiz()
> > gen6_blorp_emit_depth_stencil_config()
> > gen6_emit_depth_stencil_hiz()
> 
> Yeah, this is intentional. Gen6 will have to keep using the miptree
> structure because of limitations of hiz on gen6. To support layered
> rendering (and thus GL 3.2) we'll need to stick with this.
> 

As we discussed in my cube yesterday, I was sort of hoping there would
be one data structure for both gen6 and > gen6, but I think we agreed it
doesn't really work out well. So it seems okay to me.

> For gen < 6, I'm not sure. We might be able to rework some of how the
> texturing works there because I don't think they will have to support
> layered rendering. (Except, I may be veering off track from hiz
> now...)
> 
> > If it was intentional, maybe adjust the commit message?
> 
> The subject does say gen7. :)
> 
> Any suggestions on what to add? Maybe just mention that gen <= 6 will
> not be updated similarly?
> 
> -Jordan

You're right. The subject says enough.

You did already have my r-b...

> 
> > I see nothing that would break as a result here though.
> > Reviewed-by: Ben Widawsky <ben at bwidawsk.net>
> > 
> > -- 
> > Ben Widawsky, Intel Open Source Technology Center

-- 
Ben Widawsky, Intel Open Source Technology Center