R600/700 and Evergreen/NI blit code have a few redundant definitions in respective .c file. Move common definitions into a separate (new) .h file.
Signed-off-by: Ilija Hadzic ihadzic@research.bell-labs.com --- drivers/gpu/drm/radeon/evergreen_blit_kms.c | 12 +------- drivers/gpu/drm/radeon/r600_blit_kms.c | 15 +-------- drivers/gpu/drm/radeon/radeon_blit_common.h | 44 +++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 25 deletions(-) create mode 100644 drivers/gpu/drm/radeon/radeon_blit_common.h
diff --git a/drivers/gpu/drm/radeon/evergreen_blit_kms.c b/drivers/gpu/drm/radeon/evergreen_blit_kms.c index 2379849..4e83fdc 100644 --- a/drivers/gpu/drm/radeon/evergreen_blit_kms.c +++ b/drivers/gpu/drm/radeon/evergreen_blit_kms.c @@ -32,17 +32,7 @@ #include "evergreend.h" #include "evergreen_blit_shaders.h" #include "cayman_blit_shaders.h" - -#define DI_PT_RECTLIST 0x11 -#define DI_INDEX_SIZE_16_BIT 0x0 -#define DI_SRC_SEL_AUTO_INDEX 0x2 - -#define FMT_8 0x1 -#define FMT_5_6_5 0x8 -#define FMT_8_8_8_8 0x1a -#define COLOR_8 0x1 -#define COLOR_5_6_5 0x8 -#define COLOR_8_8_8_8 0x1a +#include "radeon_blit_common.h"
/* emits 17 */ static void diff --git a/drivers/gpu/drm/radeon/r600_blit_kms.c b/drivers/gpu/drm/radeon/r600_blit_kms.c index d996f43..5dcff17 100644 --- a/drivers/gpu/drm/radeon/r600_blit_kms.c +++ b/drivers/gpu/drm/radeon/r600_blit_kms.c @@ -30,20 +30,7 @@
#include "r600d.h" #include "r600_blit_shaders.h" - -#define DI_PT_RECTLIST 0x11 -#define DI_INDEX_SIZE_16_BIT 0x0 -#define DI_SRC_SEL_AUTO_INDEX 0x2 - -#define FMT_8 0x1 -#define FMT_5_6_5 0x8 -#define FMT_8_8_8_8 0x1a -#define COLOR_8 0x1 -#define COLOR_5_6_5 0x8 -#define COLOR_8_8_8_8 0x1a - -#define RECT_UNIT_H 32 -#define RECT_UNIT_W (RADEON_GPU_PAGE_SIZE / 4 / RECT_UNIT_H) +#include "radeon_blit_common.h"
/* emits 21 on rv770+, 23 on r600 */ static void diff --git a/drivers/gpu/drm/radeon/radeon_blit_common.h b/drivers/gpu/drm/radeon/radeon_blit_common.h new file mode 100644 index 0000000..4ecbe72 --- /dev/null +++ b/drivers/gpu/drm/radeon/radeon_blit_common.h @@ -0,0 +1,44 @@ +/* + * Copyright 2009 Advanced Micro Devices, Inc. + * Copyright 2009 Red Hat Inc. + * Copyright 2012 Alcatel-Lucent, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + * + */ + +#ifndef __RADEON_BLIT_COMMON_H__ + +#define DI_PT_RECTLIST 0x11 +#define DI_INDEX_SIZE_16_BIT 0x0 +#define DI_SRC_SEL_AUTO_INDEX 0x2 + +#define FMT_8 0x1 +#define FMT_5_6_5 0x8 +#define FMT_8_8_8_8 0x1a +#define COLOR_8 0x1 +#define COLOR_5_6_5 0x8 +#define COLOR_8_8_8_8 0x1a + +#define RECT_UNIT_H 32 +#define RECT_UNIT_W (RADEON_GPU_PAGE_SIZE / 4 / RECT_UNIT_H) + +#define __RADEON_BLIT_COMMON_H__ +#endif
If a blit copy operation specifies a rectangle whose one dimension is 16384 (max allowed by these chips), the chip will silently drop all commands. Fixed by reducing the maximum dimension by one rectangle unit size. In the mainline kernel, the problem is exposed only when buffers are very large (1G), but it's still a problem. The problem *could* be exposed for smaller buffers if anyone modifies the algorithm for rectangle construction in r600_blit_create_rect() (the reason why someone would modify that algorithm is to tune the performance of buffer moves).
Signed-off-by: Ilija Hadzic ihadzic@research.bell-labs.com --- drivers/gpu/drm/radeon/evergreen_blit_kms.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/radeon/evergreen_blit_kms.c b/drivers/gpu/drm/radeon/evergreen_blit_kms.c index 4e83fdc..24abdf4 100644 --- a/drivers/gpu/drm/radeon/evergreen_blit_kms.c +++ b/drivers/gpu/drm/radeon/evergreen_blit_kms.c @@ -631,7 +631,8 @@ int evergreen_blit_init(struct radeon_device *rdev) if (rdev->family >= CHIP_CAYMAN) rdev->r600_blit.ring_size_per_loop += 9; /* additional DWs for surface sync */
- rdev->r600_blit.max_dim = 16384; + /* Evergreen/NI bug: max dimension 16384 is broken for blit copy */ + rdev->r600_blit.max_dim = 16384 - RECT_UNIT_H;
/* pin copy shader into vram if already initialized */ if (rdev->r600_blit.shader_obj)
On Mit, 2012-02-01 at 11:42 -0500, Ilija Hadzic wrote:
If a blit copy operation specifies a rectangle whose one dimension is 16384 (max allowed by these chips), the chip will silently drop all commands.
What exactly does 'silently drop all commands' mean?
Did you notice the following in i2f():
fraction = (input & 0x3fff) << 10; /* cheat and only handle numbers below 2^^15 */
I suspect that should say 2^14, as 16384 == 0x4000, which results in 0 for the above assignment.
On Wed, 1 Feb 2012, Michel [ISO-8859-1] D�nzer wrote:
On Mit, 2012-02-01 at 11:42 -0500, Ilija Hadzic wrote:
If a blit copy operation specifies a rectangle whose one dimension is 16384 (max allowed by these chips), the chip will silently drop all commands.
What exactly does 'silently drop all commands' mean?
It means that when I look at the transactions on the bus (using the PCIe analyzer instrument), I see that the GPU pulls the stuff from the ring (read transactions come from GPU and completions come from the host), but I see no posted writes in the upstream that correspond to GPU writing to host memory.
Did you notice the following in i2f():
fraction = (input & 0x3fff) << 10; /* cheat and only handle numbers below 2^^15 */I suspect that should say 2^14, as 16384 == 0x4000, which results in 0 for the above assignment.
Thanks for pointing this out. This could be the possible root cause. I'll see if i2f can be easily extended to support up to 2^15 range (I am not good at doing floating point representation in my head, so I need to write out things on paper to make sure, first). If yes, I'll fix i2f. If not, the existing patch will still be the reasonable fix.
-- Ilija
dri-devel@lists.freedesktop.org
-
Ilija Hadzic -
Michel Dänzer