[Mesa-dev] i965: Use isl for hiz and stencil

[Jason Ekstrand]

On Tue, May 9, 2017 at 9:43 AM, Pohjolainen, Topi <
topi.pohjolainen at gmail.com> wrote:

> On Tue, May 09, 2017 at 08:45:55AM -0700, Jason Ekstrand wrote:
> > On Mon, May 8, 2017 at 11:10 PM, Pohjolainen, Topi <
> > topi.pohjolainen at gmail.com> wrote:
> >
> > > On Mon, May 08, 2017 at 04:51:35PM -0700, Jason Ekstrand wrote:
> > > > On Wed, May 3, 2017 at 2:22 AM, Topi Pohjolainen <
> > > topi.pohjolainen at gmail.com
> > > > > wrote:
> > > >
> > > > > Patches 1-17 are revision that
> > > > >
> > > > >   - rework hiz on gen6 to use on-demand offset calculator allowing
> > > > >     one to drop dependency to miptree structure and
> > > > >   - rework all auxiliary surfaces to be created against isl
> directly.
> > > > >
> > > > > Patches 18 and 19 introduce new surface layout in ISL. This is
> called
> > > > > back-to-back and similar to layout ALL_SLICES_AT_EACH_LOD found in
> > > > > i965 for gen6 hiz and stencil. This layout stacks slices for each
> level
> > > > > after one and other, or back to back. All slices ate each lod is
> almost
> > > > > the same except that it places levels one and two side-by-side
> trying
> > > > > to preserve space. Back-to-back wastes a little more memory but
> aligns
> > > > > each level on page boundary simplifying driver logic.
> > > > >
> > > >
> > > > My primary gripe here is that you seem to have half-added
> back-to-back to
> > > > ISL.  If this layout is a long-term thing, then we should add a new
> > > > ISL_DIM_LAYOUT_GEN6_BACK_TO_BACK layout and plumb your offset
> function
> > > > through isl_surf_get_image_offset_sa.  Is this intended to be a
> permanent
> > > > solution?  I think eventually, I'd like us to go with one surf per
> > > miplevel
> > > > (which is almost the same) but I can see how this is easier at the
> > > moment.
> > > > However, I think this works sufficiently well that I'm ok with doing
> the
> > > > back-to-back thing for a while.
> > >
> > > I thought about adding new layout type but couldn't decide which way is
> > > better. It is easy to buy your arguments in favor, and I'm happy to
> give it
> > > a go.
> > > If miptree per level is your number one choice, then lets go with that.
> >
> >
> > I said "one surf per miplevel".  I see no reason why we need N miptrees.
>
> Ah, right, my mistake. We need a little more than isl_surf instance though,
> at least we need offset per level (unless we calculate that on-demand).
> Of course we could use the current level/slice table but I'm hoping to get
> rid
> of that.
>

Yeah, something similar to anv_surface is probably needed.  Though I would
be inclined to call it brw_per_lod_surf or something more descriptive like
that.


> >
> >
> > > I just
> > > need to check a few things first about the actual solution. I would see
> > > something in these lines:
> > >
> > > 1) Add a dynamically allocated array of miptrees into miptree. This
> would
> > >    contain miptree instance per level.
> > >
> > > 2) Still uses one buffer object containing space for all levels. The
> > > instances
> > >    in the array would either have their ::bo pointer zero or pointing
> to
> > > the
> > >    parent ::bo. In both cases ::offset would point the start of the
> level.
> > >
> >
> > Yes
> >
> >
> > > 3) Instances in the array are not reference counted and therefore
> deleted
> > >    simply by deallocating the malloced chunk underneath.
> > >
> >
> > If we have one isl_surf per miplevel and not a miptree per level, then I
> > don't think this is an issue.
> >
> >
> > > 4) Add similar dynamically allocated array of intel_miptree_aux_buffer
> > >    instances for hiz. Here also use one ::bo which would need to added
> to
> > >    miptree I think cause there ins't one in miptree. Or perhaps add the
> > >    array of aux buffers to aux buffer?
> > >
> >
> > Looking at intel_miptree_aux_buffer, I think what we would end up with is
> > an array of aux_buffers
> >
> >
> > > 5) ISL doesn't need to know about this and hence we would add the total
> > > space
> > >    calculator along with ::offset initialization in i965
> (brw_tex_layout,
> > >    I think).
> > >
> >
> > That's fine.  We already do that in Vulkan with anv_surface.  ::offset
> > calculation can be done easily enough by just adding sizes.
> >
> >
> > > 6) In Vulkan <-> GL interop, we'd pass single level arrays only as ISL
> > > didn't
> > >    know about back-2-back. Or we simply don't care about gen6 as Vulkan
> > >    doesn't support it anyhow?
> > >
> >
> > Yeah, we don't care about gen6.
> >
> > --Jason
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170509/1e3d38bc/attachment.html>