<div dir="ltr">All of the below being said, I'm fine with landing things with a BACK_TO_BACK layout and then making the switch to one isl_surf per miplevel later if that's an easier path.<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 9, 2017 at 8:45 AM, Jason Ekstrand <span dir="ltr"><<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">On Mon, May 8, 2017 at 11:10 PM, Pohjolainen, Topi <span dir="ltr"><<a href="mailto:topi.pohjolainen@gmail.com" target="_blank">topi.pohjolainen@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="m_1842991692001462822gmail-">On Mon, May 08, 2017 at 04:51:35PM -0700, Jason Ekstrand wrote:<br>
> On Wed, May 3, 2017 at 2:22 AM, Topi Pohjolainen <<a href="mailto:topi.pohjolainen@gmail.com" target="_blank">topi.pohjolainen@gmail.com</a><br>
> > wrote:<br>
><br>
> > Patches 1-17 are revision that<br>
> ><br>
> >   - rework hiz on gen6 to use on-demand offset calculator allowing<br>
> >     one to drop dependency to miptree structure and<br>
> >   - rework all auxiliary surfaces to be created against isl directly.<br>
> ><br>
> > Patches 18 and 19 introduce new surface layout in ISL. This is called<br>
> > back-to-back and similar to layout ALL_SLICES_AT_EACH_LOD found in<br>
> > i965 for gen6 hiz and stencil. This layout stacks slices for each level<br>
> > after one and other, or back to back. All slices ate each lod is almost<br>
> > the same except that it places levels one and two side-by-side trying<br>
> > to preserve space. Back-to-back wastes a little more memory but aligns<br>
> > each level on page boundary simplifying driver logic.<br>
> ><br>
><br>
> My primary gripe here is that you seem to have half-added back-to-back to<br>
> ISL.  If this layout is a long-term thing, then we should add a new<br>
> ISL_DIM_LAYOUT_GEN6_BACK_TO_BA<wbr>CK layout and plumb your offset function<br>
> through isl_surf_get_image_offset_sa.  Is this intended to be a permanent<br>
> solution?  I think eventually, I'd like us to go with one surf per miplevel<br>
> (which is almost the same) but I can see how this is easier at the moment.<br>
> However, I think this works sufficiently well that I'm ok with doing the<br>
> back-to-back thing for a while.<br>
<br>
</span>I thought about adding new layout type but couldn't decide which way is<br>
better. It is easy to buy your arguments in favor, and I'm happy to give it<br>
a go.<br>
If miptree per level is your number one choice, then lets go with that.</blockquote><div><br></div></div></div><div>I said "one surf per miplevel".  I see no reason why we need N miptrees.<br></div><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> I just<br>
need to check a few things first about the actual solution. I would see<br>
something in these lines:<br>
<br>
1) Add a dynamically allocated array of miptrees into miptree. This would<br>
   contain miptree instance per level.<br>
<br>
2) Still uses one buffer object containing space for all levels. The instances<br>
   in the array would either have their ::bo pointer zero or pointing to the<br>
   parent ::bo. In both cases ::offset would point the start of the level.<br></blockquote><div><br></div></span><div>Yes<br></div><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
3) Instances in the array are not reference counted and therefore deleted<br>
   simply by deallocating the malloced chunk underneath.<br></blockquote></span><div><br>If we have one isl_surf per miplevel and not a miptree per level, then I don't think this is an issue.<br> </div><span class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
4) Add similar dynamically allocated array of intel_miptree_aux_buffer<br>
   instances for hiz. Here also use one ::bo which would need to added to<br>
   miptree I think cause there ins't one in miptree. Or perhaps add the<br>
   array of aux buffers to aux buffer?<br></blockquote><div><br></div></span><div>Looking at intel_miptree_aux_buffer, I think what we would end up with is an array of aux_buffers<br></div><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
5) ISL doesn't need to know about this and hence we would add the total space<br>
   calculator along with ::offset initialization in i965 (brw_tex_layout,<br>
   I think).<br></blockquote><div><br></div></span><div>That's fine.  We already do that in Vulkan with anv_surface.  ::offset calculation can be done easily enough by just adding sizes.<br></div><span class=""><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
6) In Vulkan <-> GL interop, we'd pass single level arrays only as ISL didn't<br>
   know about back-2-back. Or we simply don't care about gen6 as Vulkan<br>
   doesn't support it anyhow?<br>
</blockquote></span></div><br></div><div class="gmail_extra">Yeah, we don't care about gen6.<span class="HOEnZb"><font color="#888888"><br><br></font></span></div><span class="HOEnZb"><font color="#888888"><div class="gmail_extra">--Jason<br></div></font></span></div>
</blockquote></div><br></div>