LGTM, but I have a comments below.

Grigori

On 10.09.2014 10:54, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daen...@amd.com>
> 
> Signed-off-by: Michel Dänzer <michel.daen...@amd.com>
> ---
> 
> This might help for investigating DMA related bugs.
> 
>  src/gallium/drivers/radeonsi/si_dma.c | 103 
> ++++++++++++++--------------------
>  1 file changed, 41 insertions(+), 62 deletions(-)
> 
> diff --git a/src/gallium/drivers/radeonsi/si_dma.c 
> b/src/gallium/drivers/radeonsi/si_dma.c
> index f6e2a78..c067cd9 100644
> --- a/src/gallium/drivers/radeonsi/si_dma.c
> +++ b/src/gallium/drivers/radeonsi/si_dma.c
> @@ -130,8 +130,11 @@ static void si_dma_copy_tile(struct si_context *ctx,
>       struct si_screen *sscreen = ctx->screen;
>       struct r600_texture *rsrc = (struct r600_texture*)src;
>       struct r600_texture *rdst = (struct r600_texture*)dst;
> +     struct r600_texture *rlinear, *rtiled;
> +     unsigned linear_lvl, tiled_lvl;
>       unsigned array_mode, lbpp, pitch_tile_max, slice_tile_max, size;
> -     unsigned ncopy, height, cheight, detile, i, x, y, z, src_mode, dst_mode;
> +     unsigned ncopy, height, cheight, detile, i, src_mode, dst_mode;
> +     unsigned linear_x, linear_y, linear_z,  tiled_x, tiled_y, tiled_z;
>       unsigned sub_cmd, bank_h, bank_w, mt_aspect, nbanks, tile_split, mt;
>       uint64_t base, addr;
>       unsigned pipe_config, tile_mode_index;
> @@ -143,68 +146,44 @@ static void si_dma_copy_tile(struct si_context *ctx,
>       dst_mode = dst_mode == RADEON_SURF_MODE_LINEAR_ALIGNED ? 
> RADEON_SURF_MODE_LINEAR : dst_mode;
>       assert(dst_mode != src_mode);
>  
> -     y = 0;
>       sub_cmd = SI_DMA_COPY_TILED;
>       lbpp = util_logbase2(bpp);
>       pitch_tile_max = ((pitch / bpp) / 8) - 1;
>  
> -     if (dst_mode == RADEON_SURF_MODE_LINEAR) {
> -             /* T2L */
> -             array_mode = si_array_mode(src_mode);
> -             slice_tile_max = (rsrc->surface.level[src_level].nblk_x * 
> rsrc->surface.level[src_level].nblk_y) / (8*8);
> -             slice_tile_max = slice_tile_max ? slice_tile_max - 1 : 0;
> -             /* linear height must be the same as the slice tile max height, 
> it's ok even
> -              * if the linear destination/source have smaller heigh as the 
> size of the
> -              * dma packet will be using the copy_height which is always 
> smaller or equal
> -              * to the linear height
> -              */
> -             height = rsrc->surface.level[src_level].npix_y;
> -             detile = 1;
> -             x = src_x;
> -             y = src_y;
> -             z = src_z;
> -             base = rsrc->surface.level[src_level].offset;
> -             addr = rdst->surface.level[dst_level].offset;
> -             addr += rdst->surface.level[dst_level].slice_size * dst_z;
> -             addr += dst_y * pitch + dst_x * bpp;
> -             bank_h = cik_bank_wh(rsrc->surface.bankh);
> -             bank_w = cik_bank_wh(rsrc->surface.bankw);
> -             mt_aspect = cik_macro_tile_aspect(rsrc->surface.mtilea);
> -             tile_split = cik_tile_split(rsrc->surface.tile_split);
> -             tile_mode_index = si_tile_mode_index(rsrc, src_level,
> -                                                  
> util_format_has_stencil(util_format_description(src->format)));
> -             nbanks = si_num_banks(sscreen, rsrc);
> -             base += rsrc->resource.gpu_address;
> -             addr += rdst->resource.gpu_address;
> -     } else {
> -             /* L2T */
> -             array_mode = si_array_mode(dst_mode);
> -             slice_tile_max = (rdst->surface.level[dst_level].nblk_x * 
> rdst->surface.level[dst_level].nblk_y) / (8*8);
> -             slice_tile_max = slice_tile_max ? slice_tile_max - 1 : 0;
> -             /* linear height must be the same as the slice tile max height, 
> it's ok even
> -              * if the linear destination/source have smaller heigh as the 
> size of the
> -              * dma packet will be using the copy_height which is always 
> smaller or equal
> -              * to the linear height
> -              */
> -             height = rdst->surface.level[dst_level].npix_y;
> -             detile = 0;
> -             x = dst_x;
> -             y = dst_y;
> -             z = dst_z;
> -             base = rdst->surface.level[dst_level].offset;
> -             addr = rsrc->surface.level[src_level].offset;
> -             addr += rsrc->surface.level[src_level].slice_size * src_z;
> -             addr += src_y * pitch + src_x * bpp;
> -             bank_h = cik_bank_wh(rdst->surface.bankh);
> -             bank_w = cik_bank_wh(rdst->surface.bankw);
> -             mt_aspect = cik_macro_tile_aspect(rdst->surface.mtilea);
> -             tile_split = cik_tile_split(rdst->surface.tile_split);
> -             tile_mode_index = si_tile_mode_index(rdst, dst_level,
> -                                                  
> util_format_has_stencil(util_format_description(dst->format)));
> -             nbanks = si_num_banks(sscreen, rdst);
> -             base += rdst->resource.gpu_address;
> -             addr += rsrc->resource.gpu_address;
> -     }
> +     detile = dst_mode == RADEON_SURF_MODE_LINEAR;
> +     rlinear = detile ? rdst : rsrc;
> +     rtiled = detile ? rsrc : rdst;
> +     linear_lvl = detile ? dst_level : src_level;
> +     tiled_lvl = detile ? src_level : dst_level;
> +     linear_x = detile ? dst_x : src_x;
> +     linear_y = detile ? dst_y : src_y;
> +     linear_z = detile ? dst_z : src_z;
> +     tiled_x = detile ? src_x : dst_x;
> +     tiled_y = detile ? src_y : dst_y;
> +     tiled_z = detile ? src_z : dst_z;
> +
> +     array_mode = si_array_mode(rtiled->surface.level[tiled_lvl].mode);

Wouldn't it be more consistent to pull the array_mode from the tile mode
array like other parameters? That will break old kernels, but I think it
should be okay by now. We could maybe disable tiled DMA copies
completely on those.

> +     slice_tile_max = (rtiled->surface.level[tiled_lvl].nblk_x *
> +                       rtiled->surface.level[tiled_lvl].nblk_y) / (8*8) - 1;
> +     /* linear height must be the same as the slice tile max height, it's ok 
> even
> +      * if the linear destination/source have smaller heigh as the size of 
> the
> +      * dma packet will be using the copy_height which is always smaller or 
> equal
> +      * to the linear height
> +      */
> +     height = rtiled->surface.level[tiled_lvl].npix_y;
> +     base = rtiled->surface.level[tiled_lvl].offset;
> +     addr = rlinear->surface.level[linear_lvl].offset;
> +     addr += rlinear->surface.level[linear_lvl].slice_size * linear_z;
> +     addr += linear_y * pitch + linear_x * bpp;
> +     bank_h = cik_bank_wh(rtiled->surface.bankh);
> +     bank_w = cik_bank_wh(rtiled->surface.bankw);
> +     mt_aspect = cik_macro_tile_aspect(rtiled->surface.mtilea);
> +     tile_split = cik_tile_split(rtiled->surface.tile_split);
> +     tile_mode_index = si_tile_mode_index(rtiled, tiled_lvl,
> +                                          
> util_format_has_stencil(util_format_description(rtiled->resource.b.b.format)));
> +     nbanks = si_num_banks(sscreen, rtiled);
> +     base += rtiled->resource.gpu_address;
> +     addr += rlinear->resource.gpu_address;
>  
>       pipe_config = cik_db_pipe_config(sscreen, tile_mode_index);
>       mt = si_micro_tile_mode(sscreen, tile_mode_index);
> @@ -230,13 +209,13 @@ static void si_dma_copy_tile(struct si_context *ctx,
>                                       (bank_w << 18) | (mt_aspect << 16);
>               cs->buf[cs->cdw++] = (pitch_tile_max << 0) | ((height - 1) << 
> 16);
>               cs->buf[cs->cdw++] = (slice_tile_max << 0) | (pipe_config << 
> 26);
> -             cs->buf[cs->cdw++] = (x << 0) | (z << 18);
> -             cs->buf[cs->cdw++] = (y << 0) | (tile_split << 21) | (nbanks << 
> 25) | (mt << 27);
> +             cs->buf[cs->cdw++] = (tiled_x << 0) | (tiled_z << 18);
> +             cs->buf[cs->cdw++] = (tiled_y << 0) | (tile_split << 21) | 
> (nbanks << 25) | (mt << 27);
>               cs->buf[cs->cdw++] = addr & 0xfffffffc;
>               cs->buf[cs->cdw++] = (addr >> 32UL) & 0xff;
>               copy_height -= cheight;
>               addr += cheight * pitch;
> -             y += cheight;
> +             tiled_y += cheight;
>       }
>  }
>  
> 


Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to