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

[Eric Anholt]

On Mon, 18 Jul 2011 17:00:54 -0700, Chad Versace <chad at chad-versace.us> wrote:
> 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.

Looks good to me, gets the Reviewed-by: Eric Anholt <eric at anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20110719/8b96d2ea/attachment.pgp>