On 07/18/2011 08:57 AM, Eric Anholt wrote:
> On Mon, 18 Jul 2011 00:55:03 -0700, Chad Versace <c...@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.Good call. Does changing the hunk like this look good to you? + 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; \ + int y_scale = flip ? -1 : 1; \ + int y_bias = flip ? (height - 1) : 0; \ -/* Don't flip y. */ #undef Y_FLIP -#define Y_FLIP(y) y +#define Y_FLIP(y) (y_scale * (y) + y_bias) -- Chad Versace c...@chad-versace.us
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
