Summary: | HiZ buffers are much larger than necessary | ||
---|---|---|---|
Product: | Mesa | Reporter: | Paul Berry <stereotype441> |
Component: | Drivers/DRI/i965 | Assignee: | Jordan Justen <jljusten> |
Status: | RESOLVED FIXED | QA Contact: | Intel 3D Bugs Mailing List <intel-3d-bugs> |
Severity: | normal | ||
Priority: | medium | CC: | ben, bugs.freedesktop.org, chadversary, jljusten |
Version: | git | ||
Hardware: | x86-64 (AMD64) | ||
OS: | All | ||
Whiteboard: | |||
i915 platform: | i915 features: |
Description
Paul Berry
2013-07-31 01:35:26 UTC
Gen7 should get fixed first, because there the required fixes will be less intrusive than in Gen6. For reference, the discussion of the correct size for the HiZ buffer is located on page [3D-Media-GPGPU Engine > 3D Pipeline Stages > Pixel > Depth and Stencil > Hierarchical Depth Buffer]. To understand the discussion, [Memory Views > Common Surface Formats > Surface Layout] is a prerequisite. On Gen7, the root cause of this waste of memory is that Mesa re-uses ``struct intel_mipmap_tree`` and to hold the HiZ buffer's layout (even though the HiZ buffer isn't a true miptree) and inappropriately re-uses the vanilla miptree constructors to determine the HiZ buffer's size. On Gen6, the issue is more complex, so let's focus on solving Gen7 first. There is an additional undesired consequence of re-using ``struct intel_mipmap_tree`` to hold the HiZ buffer, though this consequence causes no bugs. The struct contains many fields that are completely irrelevant and meaningless to HiZ, such as ``format``, which we set to the same format as parent depth buffer; and the layout fields, which are never used on Gen7. Because we construct the HiZ miptree with the vanilla miptree constructors, we set many of these fields to arbitrary values, and other fields lie unused. I propose the below approach to fixing the memory waste. Paul and Jordan, what do you think of this plan? By the way, Ben is expressing interest in picking up a low-priority, straightforward Mesa task close to the metal, so I suggested this. I believe the Gen7 fixes fit his criteria, though the Gen6 fixes may not. Proposed Fixes for Gen7 ======================= 1. Create a new type, ``struct intel_hiz_buffer``, that initially is identical to ``struct intel_mipmap_tree``. Change the type of ``intel_mipmap_tree::hiz_mt`` to this new type. Initially, intel_hiz_buffer_create() should replicate the current behavior of intel_miptree_alloc_hiz(). 2. Fix intel_hiz_buffer to allocate a correctly sized HiZ buffer on Gen7. 3. Delete all unused or irrelevant fields in intel_hiz_buffer. 4. Since we now have a separate type for the HiZ buffer, do some related clean-ups in intel_mipmap_tree.c ... 4a. The layout data in ``struct intel_mipmap_slice::x/y_offset`` is used only for Gen6, so let's not initialize that data on Gen7. 4b. Move intel_mipmap_tree::hiz_map into intel intel_hiz_buffer. 5. Validate on Ivybridge and Haswell, especially Haswell because it's more sensitive to HiZ hangs than Ivybridge. My "i965/gen7+ HiZ cleanup + gen8 hiz improvements" series may help with this, but I did not test the hiz buffer sizes. (I am trying to use the formulas in the docs to size the hiz buffers though.) Since you're updating this one... for posterity, here was the last spin I did on a quick search: http://lists.freedesktop.org/archives/mesa-dev/2013-September/045491.html I tested the hiz buffer size allocated for this piglit command: bin/depthstencil-render-miplevels 1024 d=z24 When using patches 1-5 of my bwd-aux-hiz branch in git://people.freedesktop.org/~jljusten/mesa For gen7 and gen8: master=7340032 patches1-5=917504 This seems to match Paul's comment that it is roughly 8 times larger than necessary. Fixed upstream in Mesa. gen7: 81124aefe8a2f61d5af4257e5b9989019bfed518 gen8: 6626e3548b5365dcb284504ea863d2ccdba2c7a5 |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.