[Mesa-dev] [PATCH 8/8] intel/hsw: Enable hiz

[Kenneth Graunke]

On 04/09/2013 10:49 PM, Chad Versace wrote:
> On 04/09/2013 04:15 PM, Eric Anholt wrote:
>> Kenneth Graunke <kenneth at whitecape.org> writes:
>>
>>> From: Chad Versace <chad.versace at linux.intel.com>
>>>
>>> Enable hiz by setting intel_context::has_hiz.  However, to work around
>>> a hardware bug, we selectively enable hiz for only nicely aligned
>>> miptree
>>> slices.
>>>
>>> No Piglit regressions on Haswell 0x0d26 rev07 when based atop
>>> mesa-master-97e40a5.
>>>
>>> Improves the performance of GLB27_TRex_C24Z16_FixedTimeStep by 18.52%
>>> (hsw-0x0d26-rev07; kernel-3.9.0-rc1; GLBenchmark 2.7.0 Release a68901;
>>> samples=3).
>>
>>> diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
>>> b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
>>> index 6c27bab..654f0be 100644
>>> --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
>>> +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
>>> @@ -29,6 +29,7 @@
>>>   #include <GL/internal/dri_interface.h>
>>>
>>>   #include "intel_batchbuffer.h"
>>> +#include "intel_chipset.h"
>>>   #include "intel_context.h"
>>>   #include "intel_mipmap_tree.h"
>>>   #include "intel_regions.h"
>>> @@ -1036,7 +1037,38 @@ intel_miptree_slice_has_hiz(struct
>>> intel_mipmap_tree *mt,
>>>                               uint32_t layer)
>>>   {
>>>      intel_miptree_check_level_layer(mt, level, layer);
>>> -   return mt->hiz_mt != NULL;
>>> +
>>> +   if (!mt->hiz_mt)
>>> +      return false;
>>> +
>>> +   int devid = drm_intel_bufmgr_gem_get_devid(mt->region->bo->bufmgr);
>>> +   if (IS_HASWELL(devid)) {
>>
>> This conditional is checking for about 37 different PCI IDs, and
>> shouldn't appear in a hot path.  Please use intel->is_haswell, by
>> passing the intel_context into this function.
>
> Passing the intel_context into this function would require changing
> the signature of many functions. Rather than change all those
> signatures, I'd prefer to store a boolean flag 'has_hiz' in
> intel_mipmap_slice. What do you think?

And then just check that, rather than calling this function?  Yeah, that 
seems reasonable to me.