On 02/28/2013 11:01 AM, Kenneth Graunke wrote: > 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...
Eric, you're right. I agree with Ken on the padding issue, and I do have patches for it. I'm doing a full piglit run now, and will submit them if the results look good. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
