[Mesa-dev] [PATCH 1/4] i965: Fix fulsim assertion failures by aligning HiZ ops to 8x4.

[Chad Versace]

On 02/27/2013 11:39 AM, Eric Anholt wrote:
> Chad Versace <chad.versace at 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.