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

[Eric Anholt]

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.  Doesn't
seem like 128 pitch requirement would be needed -- has it been tested?

> diff --git a/src/mesa/drivers/dri/intel/intel_span.c b/src/mesa/drivers/dri/intel/intel_span.c
> index 153803f..d306432 100644
> --- a/src/mesa/drivers/dri/intel/intel_span.c
> +++ b/src/mesa/drivers/dri/intel/intel_span.c
> @@ -131,38 +131,77 @@ intel_set_span_functions(struct intel_context *intel,
>     int miny = 0;							\
>     int maxx = rb->Width;						\
>     int maxy = rb->Height;						\
> -   int stride = rb->RowStride;						\
> -   uint8_t *buf = rb->Data;						\
> +									\
> +   /*									\
> +    * Here we ignore rb->Data and rb->RowStride as set by		\
> +    * intelSpanRenderStart. Since intel_offset_S8 decodes the W tile	\
> +    * manually, the region's *real* base address and stride is		\
> +    * required.								\
> +    */									\
> +   struct intel_renderbuffer *irb = intel_renderbuffer(rb);		\
> +   uint8_t *buf = irb->region->buffer->virtual;				\
> +   unsigned stride = irb->region->pitch;				\
> +   unsigned height = 2 * irb->region->height;				\
> +   bool flip = rb->Name == 0;						\
>  
> -/* Don't flip y. */
>  #undef Y_FLIP
> -#define Y_FLIP(y) y
> +#define Y_FLIP(y) ((1 - flip) * y + flip * (height - 1 - y))

The flip is usually handled by a scale and bias variable, so that Y_FLIP
is ((y) * scale + bias).  I think it'll produce less code, since Y_FLIP
is used a lot.

-------------- 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/20110718/7c374c13/attachment.pgp>