On 02/28/2013 09:08 AM, Eric Anholt wrote:
Chad Versace <chad.vers...@linux.intel.com> writes:
On 02/27/2013 11:39 AM, Eric Anholt wrote:
Chad Versace <chad.vers...@linux.intel.com> writes:
On 02/26/2013 11:15 PM, Eric Anholt wrote:
I have some debug of HiZ rendering that looks like some rendering is not
landing in my HiZ buffer. Unfortunately, fulsim choking on us violating
hiz rendering rules was preventing me from using it as a debug aid.
Once we get things reliable, we'll also be able to take advantage of this
to get fast clears on modes like 1366x768.
---
src/mesa/drivers/dri/i965/brw_blorp.cpp | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/brw_blorp.cpp
b/src/mesa/drivers/dri/i965/brw_blorp.cpp
index 5f72b5d..49dcacb 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp.cpp
+++ b/src/mesa/drivers/dri/i965/brw_blorp.cpp
@@ -181,6 +181,16 @@ brw_hiz_op_params::brw_hiz_op_params(struct
intel_mipmap_tree *mt,
this->hiz_op = op;
depth.set(mt, level, layer);
+
+ /* HiZ operations require alignment to 8x4. Our depth and hiz miplevels
+ * should have their start offsets aligned to that (except for a bug on
+ * non-Z16) so we won't draw into a neighboring miplevel, and we allocate
+ * memory aligned to pages (128bytesx32), so we won't draw into memory not
+ * owned by our miptree.
+ */
+ depth.width = ALIGN(depth.width, 8);
+ depth.height = ALIGN(depth.height, 4);
+
This should be moved into the brw_hiz_op_params() constructor.
That's where this is. Were you thinking of a version that was posted in
my tree at one point instead?
No, I was talking with my foot in my mouth. :/
I meant it should be set by call to depth.set() above, which is
brw_blorp_mip_info::set(), like this:
this->width = ALIGN(mt->level[level].width, 8);
this->height = ALIGN(mt->level[level].height, 4);
The responsibility of setting brw_blorp_mip_info fields should
belong to that method.
I disagree -- you only want this behavior for HiZ ops on depth/stencil,
not blorp ops in general (like blits with color buffers).
My vote is with Eric on this one. I wrote an identical patch, and put
it in brw_hiz_op_params for that very reason.
However, in order for this to not break horribly, don't you need to edit
the intel/brw_tex_layout code to actually pad the buffer to have that
extra width? Otherwise you could access outside the buffer (and I
could've sworn I actually hit that case while trying this).
I believe Chad has patches to do that...
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
