[Mesa-dev] [PATCH 04/12] r600g: fix texture offset computation for mapped MSAA depth buffers
Marek Olšák
maraeo at gmail.com
Mon Jul 1 04:25:31 PDT 2013
I only know that the offset was wrong and the state tracker was
writing outside of a mapped resource. The lockup disappeared by
commenting out the call to _mesa_pack_ubyte_stencil_row, which was
reading from and writing to the mapped resource.
Two possible explanations are:
- the memory controller hangs
- some other resource which happens to be mapped next to the
depth-stencil buffer is corrupted by the writes (e.g. shaders)
Marek
On Mon, Jul 1, 2013 at 9:47 AM, Christian König <deathsimple at vodafone.de> wrote:
> Am 01.07.2013 03:53, schrieb Marek Olšák:
>
>> It was wrong, because the offset shouldn't be applied to MSAA depth
>> buffers.
>> This small cleanup should prevent such issues in the future.
>>
>> This fixes a lockup in "piglit/fbo-depthstencil default_fb -samples=n".
>>
>> The lockup was special, because it was triggered by the CPU writing to
>> memory
>> outside of the buffer range. I didn't know it was possible to hang a
>> machine
>> with the CPU. This is nuts.
>
>
> WTF? Well if my understanding of our memory controller is correct it is
> possible to hang it when the CPU tries to access some non existing memory,
> but to make this happen the kernel driver must program the PCI BAR somehow
> incorrectly.
>
> Anyway it's good that this is fixed, but I'm pretty sure we should take a
> closer ĺook what's happening here.
>
> Christian.
>
>
>> ---
>> src/gallium/drivers/r600/r600_pipe.h | 2 --
>> src/gallium/drivers/r600/r600_texture.c | 28
>> ++++++++++++++--------------
>> 2 files changed, 14 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/gallium/drivers/r600/r600_pipe.h
>> b/src/gallium/drivers/r600/r600_pipe.h
>> index 8485cce..64a90c3 100644
>> --- a/src/gallium/drivers/r600/r600_pipe.h
>> +++ b/src/gallium/drivers/r600/r600_pipe.h
>> @@ -787,8 +787,6 @@ void r600_init_surface_functions(struct r600_context
>> *r600);
>> uint32_t r600_translate_texformat(struct pipe_screen *screen, enum
>> pipe_format format,
>> const unsigned char *swizzle_view,
>> uint32_t *word4_p, uint32_t
>> *yuv_format_p);
>> -unsigned r600_texture_get_offset(struct r600_texture *rtex,
>> - unsigned level, unsigned layer);
>> struct pipe_surface *r600_create_surface_custom(struct pipe_context
>> *pipe,
>> struct pipe_resource
>> *texture,
>> const struct pipe_surface
>> *templ,
>> diff --git a/src/gallium/drivers/r600/r600_texture.c
>> b/src/gallium/drivers/r600/r600_texture.c
>> index 60d8c36..c2feb52 100644
>> --- a/src/gallium/drivers/r600/r600_texture.c
>> +++ b/src/gallium/drivers/r600/r600_texture.c
>> @@ -116,11 +116,15 @@ static void r600_copy_from_staging_texture(struct
>> pipe_context *ctx, struct r600
>> }
>> }
>> -unsigned r600_texture_get_offset(struct r600_texture *rtex,
>> - unsigned level, unsigned layer)
>> +static unsigned r600_texture_get_offset(struct r600_texture *rtex,
>> unsigned level,
>> + const struct pipe_box *box)
>> {
>> + enum pipe_format format = rtex->resource.b.b.format;
>> +
>> return rtex->surface.level[level].offset +
>> - layer * rtex->surface.level[level].slice_size;
>> + box->z * rtex->surface.level[level].slice_size +
>> + box->y / util_format_get_blockheight(format) *
>> rtex->surface.level[level].pitch_bytes +
>> + box->x / util_format_get_blockwidth(format) *
>> util_format_get_blocksize(format);
>> }
>> static int r600_init_surface(struct r600_screen *rscreen,
>> @@ -805,7 +809,6 @@ static void *r600_texture_transfer_map(struct
>> pipe_context *ctx,
>> struct r600_texture *rtex = (struct r600_texture*)texture;
>> struct r600_transfer *trans;
>> boolean use_staging_texture = FALSE;
>> - enum pipe_format format = texture->format;
>> struct r600_resource *buf;
>> unsigned offset = 0;
>> char *map;
>> @@ -849,8 +852,6 @@ static void *r600_texture_transfer_map(struct
>> pipe_context *ctx,
>> trans->transfer.box = *box;
>> if (rtex->is_depth) {
>> - /* XXX: only readback the rectangle which is being mapped?
>> */
>> - /* XXX: when discard is true, no need to read back from
>> depth texture */
>> struct r600_texture *staging_depth;
>> if (rtex->resource.b.b.nr_samples > 1) {
>> @@ -861,6 +862,8 @@ static void *r600_texture_transfer_map(struct
>> pipe_context *ctx,
>> *
>> * First downsample the depth buffer to a
>> temporary texture,
>> * then decompress the temporary one to staging.
>> + *
>> + * Only the region being mapped is transfered.
>> */
>> struct pipe_resource *temp;
>> struct pipe_resource resource;
>> @@ -880,9 +883,10 @@ static void *r600_texture_transfer_map(struct
>> pipe_context *ctx,
>> 0, 0, 0, box->depth, 0,
>> 0);
>> pipe_resource_reference((struct
>> pipe_resource**)&temp, NULL);
>> - trans->offset = 0;
>> }
>> else {
>> + /* XXX: only readback the rectangle which is being
>> mapped? */
>> + /* XXX: when discard is true, no need to read back
>> from depth texture */
>> if (!r600_init_flushed_depth_texture(ctx, texture,
>> &staging_depth)) {
>> R600_ERR("failed to create temporary
>> texture to hold untiled copy\n");
>> FREE(trans);
>> @@ -894,7 +898,7 @@ static void *r600_texture_transfer_map(struct
>> pipe_context *ctx,
>> box->z, box->z +
>> box->depth - 1,
>> 0, 0);
>> - trans->offset =
>> r600_texture_get_offset(staging_depth, level, box->z);
>> + offset = r600_texture_get_offset(staging_depth,
>> level, box);
>> }
>> trans->transfer.stride =
>> staging_depth->surface.level[level].pitch_bytes;
>> @@ -926,9 +930,10 @@ static void *r600_texture_transfer_map(struct
>> pipe_context *ctx,
>> rctx->rings.gfx.flush(rctx, 0);
>> }
>> } else {
>> + /* the resource is mapped directly */
>> trans->transfer.stride =
>> rtex->surface.level[level].pitch_bytes;
>> trans->transfer.layer_stride =
>> rtex->surface.level[level].slice_size;
>> - trans->offset = r600_texture_get_offset(rtex, level,
>> box->z);
>> + offset = r600_texture_get_offset(rtex, level, box);
>> }
>> if (trans->staging) {
>> @@ -937,11 +942,6 @@ static void *r600_texture_transfer_map(struct
>> pipe_context *ctx,
>> buf = &rtex->resource;
>> }
>> - if (rtex->is_depth || !trans->staging)
>> - offset = trans->offset +
>> - box->y / util_format_get_blockheight(format) *
>> trans->transfer.stride +
>> - box->x / util_format_get_blockwidth(format) *
>> util_format_get_blocksize(format);
>> -
>> if (!(map = r600_buffer_mmap_sync_with_rings(rctx, buf, usage))) {
>> pipe_resource_reference((struct
>> pipe_resource**)&trans->staging, NULL);
>> FREE(trans);
>
>
More information about the mesa-dev
mailing list