Re: [Mesa-dev] [PATCH] dri: Do not tile stencil buffer

[Ian Romanick <idr@freedesktop.org>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/18/2011 12:28 PM, Chad Versace wrote:
> On 07/18/2011 11:45 AM, Ian Romanick wrote:
>> On 07/18/2011 12:55 AM, Chad Versace wrote:
>>> Until now, the stencil buffer was allocated as a Y tiled buffer, because
>>> in several locations the PRM states that it is. However, it is actually
>>> W tiled. From the PRM, 2011 Sandy Bridge, Volume 1, Part 2, Section
>>> 4.5.2.1 W-Major Format:
>>>     W-Major Tile Format is used for separate stencil.
>>
>>> The GTT is incapable of W fencing, so we allocate the stencil buffer with
>>> I915_TILING_NONE and decode the tile's layout in software.
>>
>>> This commit mutually depends on the mesa commit:
>>>     intel: Fix stencil buffer to be W tiled
>>>     Author: Chad Versace <chad@chad-versace.us>
>>>     Date:   Mon Jul 18 00:37:45 2011 -0700
>>
>>> CC: Eric Anholt <eric@anholt.net>
>>> CC: Kenneth Graunke <kenneth@whitecape.org>
>>> Signed-off-by: Chad Versace <chad@chad-versace.us>
>>> ---
>>>  src/intel_dri.c |   16 ++++++++++++----
>>>  1 files changed, 12 insertions(+), 4 deletions(-)
>>
>>> diff --git a/src/intel_dri.c b/src/intel_dri.c
>>> index 5ea7c2c..4652dc7 100644
>>> --- a/src/intel_dri.c
>>> +++ b/src/intel_dri.c
>>> @@ -336,7 +336,6 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment,
>>>  			switch (attachment) {
>>>  			case DRI2BufferDepth:
>>>  			case DRI2BufferDepthStencil:
>>> -			case DRI2BufferStencil:
>>>  			case DRI2BufferHiz:
>>>  				if (SUPPORTS_YTILING(intel)) {
>>>  					hint |= INTEL_CREATE_PIXMAP_TILING_Y;
>>> @@ -351,6 +350,14 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment,
>>>  			case DRI2BufferFrontRight:
>>>  				hint |= INTEL_CREATE_PIXMAP_TILING_X;
>>>  				break;
>>> +			case DRI2BufferStencil:
>>> +				/*
>>> +				 * The stencil buffer is W tiled. However, we
>>> +				 * request from the kernel a non-tiled buffer
>>> +				 * because the GTT is incapable of W fencing.
>>> +				 */
>>> +				hint |= INTEL_CREATE_PIXMAP_TILING_NONE;
>>> +				break;
>>>  			default:
>>>  				free(privates);
>>>  				free(buffer);
>>
>> Eh... it seems like this will break compatibility with older versions of
>> Mesa.  I haven't dug around, but there used to be a hack in DRI2 where a
>> client would request a depth buffer and a stencil buffer, but it would
>> get the same packed depth-stencil buffer for both.  I guess that might
>> all be up in the DRI2 layer and not in the driver...
> 
> The 'case DRI2BufferStencil' modified in this hunk was added by me when implementing
> separate stencil support. It was never used for this hack.
> 
> That hack is implemented by an alternate definition of I830DRI2CreateBuffer() which
> is ifdef'd out when DRI2INFOREC_VERSION >= 2. FYI, the line that implements the hack can
> be found by grepping xf86-video-intel:src/intel_dri.c for
>     } else if (attachments[i] == DRI2BufferStencil && pDepthPixmap) {

Ah.  That sounds right.

This patch (with the whitespace fix) is

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>

>>
>>> @@ -368,11 +375,12 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment,
>>>  		 * 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.
>>
>> Mangled whitespace?  Probably mixed tabs and spaces...
> 
> Oops. Will fix.
> 
>>
>>>  		 */
>>>  		if (attachment == DRI2BufferStencil) {
>>> -			pixmap_height /= 2;
>>> +			pixmap_width = ALIGN(pixmap_width, 64);
>>> +			pixmap_height = ALIGN(pixmap_height / 2, 64);
>>>  			pixmap_cpp *= 2;
>>>  		}
> 
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk4knooACgkQX1gOwKyEAw8mrACdFEbKAyQIntzWec6LVA2ZYYjz
8icAn1Fc+vHRyNeygch2QHnH7Vh9Ilqt
=9yZk
-----END PGP SIGNATURE-----