[Mesa-dev] [PATCH 04/12] r600g: fix texture offset computation for mapped MSAA depth buffers
Christian König
deathsimple at vodafone.de
Mon Jul 1 05:12:54 PDT 2013
Ok, then I guess it is explanation number two. Your bug description just
sounded like you steeped to that specific memory write with a debugger
and executing this specific write was causing a lockup...
Then sorry for the noise,
Christian.
Am 01.07.2013 13:25, schrieb Marek Olšák:
> 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