<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, May 9, 2017 at 9:43 AM, Pohjolainen, Topi <<a href="mailto:topi.pohjolainen@gmail.com" target="_blank">topi.pohjolainen@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, May 09, 2017 at 08:45:55AM -0700, Jason Ekstrand wrote: > On Mon, May 8, 2017 at 11:10 PM, Pohjolainen, Topi < > <a href="mailto:topi.pohjolainen@gmail.com">topi.pohjolainen@gmail.com</a>> 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 < > > <a href="mailto:topi.pohjolainen@gmail.com">topi.pohjolainen@gmail.com</a> > > > > 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. </div></div>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. </blockquote><div> </div><div>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. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"> > > > > 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 </div></div></blockquote></div> </div></div>