[Mesa-dev] [PATCH] intel: Fix stencil buffer to be W tiled

[Chad Versace]

On 07/18/2011 08:57 AM, Eric Anholt wrote:
> On Mon, 18 Jul 2011 00:55:03 -0700, Chad Versace <chad at chad-versace.us> wrote:
>> diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c b/src/mesa/drivers/dri/intel/intel_fbo.c
>> index 1669af2..507cc33 100644
>> --- a/src/mesa/drivers/dri/intel/intel_fbo.c
>> +++ b/src/mesa/drivers/dri/intel/intel_fbo.c
>> @@ -173,6 +173,9 @@ intel_alloc_renderbuffer_storage(struct gl_context * ctx, struct gl_renderbuffer
>>  
>>     if (irb->Base.Format == MESA_FORMAT_S8) {
>>        /*
>> +       * The stencil buffer is W tiled. However, we request from the kernel a
>> +       * non-tiled buffer because the GTT is incapable of W fencing.
>> +       *
>>         * The stencil buffer has quirky pitch requirements.  From Vol 2a,
>>         * 11.5.6.2.1 3DSTATE_STENCIL_BUFFER, field "Surface Pitch":
>>         *    The pitch must be set to 2x the value computed based on width, as
>> @@ -180,14 +183,14 @@ intel_alloc_renderbuffer_storage(struct gl_context * ctx, struct gl_renderbuffer
>>         * To accomplish this, we resort to the nasty hack of doubling the drm
>>         * region's cpp and halving its height.
>>         *
>> -       * If we neglect to double the pitch, then drm_intel_gem_bo_map_gtt()
>> -       * maps the memory incorrectly.
>> +       * If we neglect to double the pitch, then render corruption occurs.
>>         */
>>        irb->region = intel_region_alloc(intel->intelScreen,
>> -				       I915_TILING_Y,
>> +				       I915_TILING_NONE,
>>  				       cpp * 2,
>> -				       width,
>> -				       height / 2,
>> +				       ALIGN(width, 64),
>> +				       /* XXX: Maybe align to 128? */
>> +				       ALIGN(height / 2, 64),
>>  				       GL_TRUE);
>>        if (!irb->region)
>>  	return false;
> 
> This looks like it would fail on a buffer with height = 129.

Oops. It does fail for height = 129. Accordingly, I've changed this hunk to:

       irb->region = intel_region_alloc(intel->intelScreen,
-				       I915_TILING_Y,
+				       I915_TILING_NONE,
 				       cpp * 2,
-				       width,
-				       height / 2,
+				       ALIGN(width, 64),
+				       ALIGN((height + 1)/ 2, 64),
 				       GL_TRUE);

And changed the same line in xf86-video-intel too.

> Doesn't
> seem like 128 pitch requirement would be needed -- has it been tested?

The XXX was left in the patch accidentally. 128-alignment has been tested, and was
found unnecessary.

-- 
Chad Versace
chad at chad-versace.us

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 900 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20110718/48a788ff/attachment.pgp>