Re: [Mesa-dev] [PATCH 3/4] radeonsi: Catch more cases that can't be handled by si_dma_copy_buffer/tile

2014-10-07 Thread Michel Dänzer 

On 01.10.2014 23:13, Grigori Goronzy wrote:

On 30.09.2014 05:58, Michel Dänzer wrote:

}
/* the x test here are currently useless (because we don't support 
partial blit)
 * but keep them around so we don't forget about those
 */
-   if ((src_pitch % 8) || (src_box-x % 8) || (dst_x % 8) || (src_box-y % 
8) || (dst_y % 8)) {
+   if ((src_pitch % 8) || (src_box-x % 8) || (dst_x % 8) ||
+   (src_box-y % 8) || (dst_y % 8) || (src_box-height % 8)) {
goto fallback;
}


That will only allow DMA copies for heights with a multiple of 8. That
isn't requirement of the DMA engines AFAICT, size is always specified in
DWs anyway.


I think the intended purpose of this code is to make sure things are 
aligned to tile boundaries. That's still necessary when using 
si_dma_copy_buffer for tiled resources, but it's not strict enough for 
2D tiled resources. See v4 of my 'Add CIK SDMA support' patch.



--
Earthling Michel Dänzer|  http://www.amd.com
Libre software enthusiast  |Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/4] radeonsi: Catch more cases that can't be handled by si_dma_copy_buffer/tile

2014-10-01 Thread Grigori Goronzy 
On 30.09.2014 05:58, Michel Dänzer wrote:
 diff --git a/src/gallium/drivers/radeonsi/si_dma.c 
 b/src/gallium/drivers/radeonsi/si_dma.c
 index ff64722..643ce3f 100644
 --- a/src/gallium/drivers/radeonsi/si_dma.c
 +++ b/src/gallium/drivers/radeonsi/si_dma.c
 @@ -251,7 +251,9 @@ void si_dma_copy(struct pipe_context *ctx,
   }
  
   if (src-format != dst-format || src_box-depth  1 ||
 - rdst-dirty_level_mask != 0) {
 + rdst-dirty_level_mask != 0 ||
 + rdst-cmask.size || rdst-fmask.size ||
 + rsrc-cmask.size || rsrc-fmask.size) {
   goto fallback;
   }

Does the existence of the cmask alone really matter? We shouldn't copy
from or to fast cleared surfaces, but this change will disable DMA
copies even if the fast clear has been eliminated. Isn't that handled
elsewhere already?

   }
   /* the x test here are currently useless (because we don't support 
 partial blit)
* but keep them around so we don't forget about those
*/
 - if ((src_pitch % 8) || (src_box-x % 8) || (dst_x % 8) || (src_box-y % 
 8) || (dst_y % 8)) {
 + if ((src_pitch % 8) || (src_box-x % 8) || (dst_x % 8) ||
 + (src_box-y % 8) || (dst_y % 8) || (src_box-height % 8)) {
   goto fallback;
   }

That will only allow DMA copies for heights with a multiple of 8. That
isn't requirement of the DMA engines AFAICT, size is always specified in
DWs anyway.

Grigori




signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/4] radeonsi: Catch more cases that can't be handled by si_dma_copy_buffer/tile

2014-10-01 Thread Marek Olšák 
On Wed, Oct 1, 2014 at 4:13 PM, Grigori Goronzy g...@chown.ath.cx wrote:
 On 30.09.2014 05:58, Michel Dänzer wrote:
 diff --git a/src/gallium/drivers/radeonsi/si_dma.c 
 b/src/gallium/drivers/radeonsi/si_dma.c
 index ff64722..643ce3f 100644
 --- a/src/gallium/drivers/radeonsi/si_dma.c
 +++ b/src/gallium/drivers/radeonsi/si_dma.c
 @@ -251,7 +251,9 @@ void si_dma_copy(struct pipe_context *ctx,
   }

   if (src-format != dst-format || src_box-depth  1 ||
 - rdst-dirty_level_mask != 0) {
 + rdst-dirty_level_mask != 0 ||
 + rdst-cmask.size || rdst-fmask.size ||
 + rsrc-cmask.size || rsrc-fmask.size) {
   goto fallback;
   }

 Does the existence of the cmask alone really matter? We shouldn't copy
 from or to fast cleared surfaces, but this change will disable DMA
 copies even if the fast clear has been eliminated. Isn't that handled
 elsewhere already?

Here's some clarification on FMASK and CMASK.

If FMASK is present, DMA cannot be used, because textures with FMASK
cannot be fully decompressed and only shaders know how to read them.
Also, FMASK is only used with MSAA, so if the code falls back to 3D
when nr_samples  1, you don't need to check if FMASK is present.

If (tex-dirty_level_mask  (1  level)) is 0 and the surface doesn't
have FMASK, then CMASK has no effect and DMA can be used. If
(tex-dirty_level_mask  (1  level)) is non-zero, then color
decompression must be done for that level before using DMA.

If rdst needs color decompression, it's better to use the 3D engine
for the texture to stay compressed and that's why the code falls back
to 3D if rdst-dirty_level_mask != 0.

If rsrc needs color decompression, it's better to decompress it and
use DMA. I think that's what the code did, but I don't remember it
anymore.

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

[Mesa-dev] [PATCH 3/4] radeonsi: Catch more cases that can't be handled by si_dma_copy_buffer/tile

2014-09-29 Thread Michel Dänzer 
From: Michel Dänzer michel.daen...@amd.com

Signed-off-by: Michel Dänzer michel.daen...@amd.com
---
 src/gallium/drivers/radeonsi/si_dma.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_dma.c 
b/src/gallium/drivers/radeonsi/si_dma.c
index ff64722..643ce3f 100644
--- a/src/gallium/drivers/radeonsi/si_dma.c
+++ b/src/gallium/drivers/radeonsi/si_dma.c
@@ -251,7 +251,9 @@ void si_dma_copy(struct pipe_context *ctx,
}
 
if (src-format != dst-format || src_box-depth  1 ||
-   rdst-dirty_level_mask != 0) {
+   rdst-dirty_level_mask != 0 ||
+   rdst-cmask.size || rdst-fmask.size ||
+   rsrc-cmask.size || rsrc-fmask.size) {
goto fallback;
}
 
@@ -277,14 +279,20 @@ void si_dma_copy(struct pipe_context *ctx,
src_mode = src_mode == RADEON_SURF_MODE_LINEAR_ALIGNED ? 
RADEON_SURF_MODE_LINEAR : src_mode;
dst_mode = dst_mode == RADEON_SURF_MODE_LINEAR_ALIGNED ? 
RADEON_SURF_MODE_LINEAR : dst_mode;
 
-   if (src_pitch != dst_pitch || src_box-x || dst_x || src_w != dst_w) {
+   if (src_pitch != dst_pitch || src_box-x || dst_x || src_w != dst_w ||
+   src_box-width != src_w ||
+   src_box-height != rsrc-surface.level[src_level].npix_y ||
+   src_box-height != rdst-surface.level[dst_level].npix_y ||
+   rsrc-surface.level[src_level].nblk_y !=
+   rdst-surface.level[dst_level].nblk_y) {
/* FIXME si can do partial blit */
goto fallback;
}
/* the x test here are currently useless (because we don't support 
partial blit)
 * but keep them around so we don't forget about those
 */
-   if ((src_pitch % 8) || (src_box-x % 8) || (dst_x % 8) || (src_box-y % 
8) || (dst_y % 8)) {
+   if ((src_pitch % 8) || (src_box-x % 8) || (dst_x % 8) ||
+   (src_box-y % 8) || (dst_y % 8) || (src_box-height % 8)) {
goto fallback;
}
 
-- 
2.1.1

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