* [PATCH 2/3] drm/radeon: implement bo copy callback using CP DMA (v2)
2013-07-11 19:35 [PATCH 1/3] drm/radeon: Disable dma rings for bo moves on r6xx alexdeucher
@ 2013-07-11 19:35 ` alexdeucher
2013-07-18 12:29 ` Marek Olšák
2013-07-11 19:35 ` [PATCH 3/3] drm/radeon: use CP DMA on r6xx for bo moves alexdeucher
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: alexdeucher @ 2013-07-11 19:35 UTC (permalink / raw)
To: dri-devel; +Cc: Alex Deucher
From: Alex Deucher <alexander.deucher@amd.com>
Lighter weight than using the 3D engine.
v2: fix ring count
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/radeon/r600.c | 81 ++++++++++++++++++++++++++++++++++
drivers/gpu/drm/radeon/r600d.h | 1 +
drivers/gpu/drm/radeon/radeon_asic.h | 3 +
3 files changed, 85 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index 2d3655f..f7d494f 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -3145,6 +3145,87 @@ int r600_copy_blit(struct radeon_device *rdev,
}
/**
+ * r600_copy_cpdma - copy pages using the CP DMA engine
+ *
+ * @rdev: radeon_device pointer
+ * @src_offset: src GPU address
+ * @dst_offset: dst GPU address
+ * @num_gpu_pages: number of GPU pages to xfer
+ * @fence: radeon fence object
+ *
+ * Copy GPU paging using the CP DMA engine (r6xx+).
+ * Used by the radeon ttm implementation to move pages if
+ * registered as the asic copy callback.
+ */
+int r600_copy_cpdma(struct radeon_device *rdev,
+ uint64_t src_offset, uint64_t dst_offset,
+ unsigned num_gpu_pages,
+ struct radeon_fence **fence)
+{
+ struct radeon_semaphore *sem = NULL;
+ int ring_index = rdev->asic->copy.blit_ring_index;
+ struct radeon_ring *ring = &rdev->ring[ring_index];
+ u32 size_in_bytes, cur_size_in_bytes, tmp;
+ int i, num_loops;
+ int r = 0;
+
+ r = radeon_semaphore_create(rdev, &sem);
+ if (r) {
+ DRM_ERROR("radeon: moving bo (%d).\n", r);
+ return r;
+ }
+
+ size_in_bytes = (num_gpu_pages << RADEON_GPU_PAGE_SHIFT);
+ num_loops = DIV_ROUND_UP(size_in_bytes, 0x1fffff);
+ r = radeon_ring_lock(rdev, ring, num_loops * 6 + 21);
+ if (r) {
+ DRM_ERROR("radeon: moving bo (%d).\n", r);
+ radeon_semaphore_free(rdev, &sem, NULL);
+ return r;
+ }
+
+ if (radeon_fence_need_sync(*fence, ring->idx)) {
+ radeon_semaphore_sync_rings(rdev, sem, (*fence)->ring,
+ ring->idx);
+ radeon_fence_note_sync(*fence, ring->idx);
+ } else {
+ radeon_semaphore_free(rdev, &sem, NULL);
+ }
+
+ for (i = 0; i < num_loops; i++) {
+ cur_size_in_bytes = size_in_bytes;
+ if (cur_size_in_bytes > 0x1fffff)
+ cur_size_in_bytes = 0x1fffff;
+ size_in_bytes -= cur_size_in_bytes;
+ tmp = upper_32_bits(src_offset) & 0xff;
+ if (size_in_bytes == 0)
+ tmp |= PACKET3_CP_DMA_CP_SYNC;
+ radeon_ring_write(ring, PACKET3(PACKET3_CP_DMA, 4));
+ radeon_ring_write(ring, src_offset & 0xffffffff);
+ radeon_ring_write(ring, tmp);
+ radeon_ring_write(ring, dst_offset & 0xffffffff);
+ radeon_ring_write(ring, upper_32_bits(dst_offset) & 0xff);
+ radeon_ring_write(ring, cur_size_in_bytes);
+ src_offset += cur_size_in_bytes;
+ dst_offset += cur_size_in_bytes;
+ }
+ radeon_ring_write(ring, PACKET3(PACKET3_SET_CONFIG_REG, 1));
+ radeon_ring_write(ring, (WAIT_UNTIL - PACKET3_SET_CONFIG_REG_OFFSET) >> 2);
+ radeon_ring_write(ring, WAIT_CP_DMA_IDLE_bit);
+
+ r = radeon_fence_emit(rdev, fence, ring->idx);
+ if (r) {
+ radeon_ring_unlock_undo(rdev, ring);
+ return r;
+ }
+
+ radeon_ring_unlock_commit(rdev, ring);
+ radeon_semaphore_free(rdev, &sem, *fence);
+
+ return r;
+}
+
+/**
* r600_copy_dma - copy pages using the DMA engine
*
* @rdev: radeon_device pointer
diff --git a/drivers/gpu/drm/radeon/r600d.h b/drivers/gpu/drm/radeon/r600d.h
index f1b3084..8e3fe81 100644
--- a/drivers/gpu/drm/radeon/r600d.h
+++ b/drivers/gpu/drm/radeon/r600d.h
@@ -602,6 +602,7 @@
#define L2_BUSY (1 << 0)
#define WAIT_UNTIL 0x8040
+#define WAIT_CP_DMA_IDLE_bit (1 << 8)
#define WAIT_2D_IDLE_bit (1 << 14)
#define WAIT_3D_IDLE_bit (1 << 15)
#define WAIT_2D_IDLECLEAN_bit (1 << 16)
diff --git a/drivers/gpu/drm/radeon/radeon_asic.h b/drivers/gpu/drm/radeon/radeon_asic.h
index 45d0693..b04b578 100644
--- a/drivers/gpu/drm/radeon/radeon_asic.h
+++ b/drivers/gpu/drm/radeon/radeon_asic.h
@@ -340,6 +340,9 @@ int r600_uvd_ring_test(struct radeon_device *rdev, struct radeon_ring *ring);
int r600_copy_blit(struct radeon_device *rdev,
uint64_t src_offset, uint64_t dst_offset,
unsigned num_gpu_pages, struct radeon_fence **fence);
+int r600_copy_cpdma(struct radeon_device *rdev,
+ uint64_t src_offset, uint64_t dst_offset,
+ unsigned num_gpu_pages, struct radeon_fence **fence);
int r600_copy_dma(struct radeon_device *rdev,
uint64_t src_offset, uint64_t dst_offset,
unsigned num_gpu_pages, struct radeon_fence **fence);
--
1.7.7.5
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/3] drm/radeon: implement bo copy callback using CP DMA (v2)
2013-07-11 19:35 ` [PATCH 2/3] drm/radeon: implement bo copy callback using CP DMA (v2) alexdeucher
@ 2013-07-18 12:29 ` Marek Olšák
0 siblings, 0 replies; 10+ messages in thread
From: Marek Olšák @ 2013-07-18 12:29 UTC (permalink / raw)
To: Alex Deucher; +Cc: Alex Deucher, dri-devel
I have just discovered that WAIT_UNTIL=WAIT_3D_IDLE must be set before using
CP DMA.
Marek
On Thu, Jul 11, 2013 at 9:35 PM, <alexdeucher@gmail.com> wrote:
> From: Alex Deucher <alexander.deucher@amd.com>
>
> Lighter weight than using the 3D engine.
>
> v2: fix ring count
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/radeon/r600.c | 81 ++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/radeon/r600d.h | 1 +
> drivers/gpu/drm/radeon/radeon_asic.h | 3 +
> 3 files changed, 85 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
> index 2d3655f..f7d494f 100644
> --- a/drivers/gpu/drm/radeon/r600.c
> +++ b/drivers/gpu/drm/radeon/r600.c
> @@ -3145,6 +3145,87 @@ int r600_copy_blit(struct radeon_device *rdev,
> }
>
> /**
> + * r600_copy_cpdma - copy pages using the CP DMA engine
> + *
> + * @rdev: radeon_device pointer
> + * @src_offset: src GPU address
> + * @dst_offset: dst GPU address
> + * @num_gpu_pages: number of GPU pages to xfer
> + * @fence: radeon fence object
> + *
> + * Copy GPU paging using the CP DMA engine (r6xx+).
> + * Used by the radeon ttm implementation to move pages if
> + * registered as the asic copy callback.
> + */
> +int r600_copy_cpdma(struct radeon_device *rdev,
> + uint64_t src_offset, uint64_t dst_offset,
> + unsigned num_gpu_pages,
> + struct radeon_fence **fence)
> +{
> + struct radeon_semaphore *sem = NULL;
> + int ring_index = rdev->asic->copy.blit_ring_index;
> + struct radeon_ring *ring = &rdev->ring[ring_index];
> + u32 size_in_bytes, cur_size_in_bytes, tmp;
> + int i, num_loops;
> + int r = 0;
> +
> + r = radeon_semaphore_create(rdev, &sem);
> + if (r) {
> + DRM_ERROR("radeon: moving bo (%d).\n", r);
> + return r;
> + }
> +
> + size_in_bytes = (num_gpu_pages << RADEON_GPU_PAGE_SHIFT);
> + num_loops = DIV_ROUND_UP(size_in_bytes, 0x1fffff);
> + r = radeon_ring_lock(rdev, ring, num_loops * 6 + 21);
> + if (r) {
> + DRM_ERROR("radeon: moving bo (%d).\n", r);
> + radeon_semaphore_free(rdev, &sem, NULL);
> + return r;
> + }
> +
> + if (radeon_fence_need_sync(*fence, ring->idx)) {
> + radeon_semaphore_sync_rings(rdev, sem, (*fence)->ring,
> + ring->idx);
> + radeon_fence_note_sync(*fence, ring->idx);
> + } else {
> + radeon_semaphore_free(rdev, &sem, NULL);
> + }
> +
> + for (i = 0; i < num_loops; i++) {
> + cur_size_in_bytes = size_in_bytes;
> + if (cur_size_in_bytes > 0x1fffff)
> + cur_size_in_bytes = 0x1fffff;
> + size_in_bytes -= cur_size_in_bytes;
> + tmp = upper_32_bits(src_offset) & 0xff;
> + if (size_in_bytes == 0)
> + tmp |= PACKET3_CP_DMA_CP_SYNC;
> + radeon_ring_write(ring, PACKET3(PACKET3_CP_DMA, 4));
> + radeon_ring_write(ring, src_offset & 0xffffffff);
> + radeon_ring_write(ring, tmp);
> + radeon_ring_write(ring, dst_offset & 0xffffffff);
> + radeon_ring_write(ring, upper_32_bits(dst_offset) & 0xff);
> + radeon_ring_write(ring, cur_size_in_bytes);
> + src_offset += cur_size_in_bytes;
> + dst_offset += cur_size_in_bytes;
> + }
> + radeon_ring_write(ring, PACKET3(PACKET3_SET_CONFIG_REG, 1));
> + radeon_ring_write(ring, (WAIT_UNTIL - PACKET3_SET_CONFIG_REG_OFFSET) >> 2);
> + radeon_ring_write(ring, WAIT_CP_DMA_IDLE_bit);
> +
> + r = radeon_fence_emit(rdev, fence, ring->idx);
> + if (r) {
> + radeon_ring_unlock_undo(rdev, ring);
> + return r;
> + }
> +
> + radeon_ring_unlock_commit(rdev, ring);
> + radeon_semaphore_free(rdev, &sem, *fence);
> +
> + return r;
> +}
> +
> +/**
> * r600_copy_dma - copy pages using the DMA engine
> *
> * @rdev: radeon_device pointer
> diff --git a/drivers/gpu/drm/radeon/r600d.h b/drivers/gpu/drm/radeon/r600d.h
> index f1b3084..8e3fe81 100644
> --- a/drivers/gpu/drm/radeon/r600d.h
> +++ b/drivers/gpu/drm/radeon/r600d.h
> @@ -602,6 +602,7 @@
> #define L2_BUSY (1 << 0)
>
> #define WAIT_UNTIL 0x8040
> +#define WAIT_CP_DMA_IDLE_bit (1 << 8)
> #define WAIT_2D_IDLE_bit (1 << 14)
> #define WAIT_3D_IDLE_bit (1 << 15)
> #define WAIT_2D_IDLECLEAN_bit (1 << 16)
> diff --git a/drivers/gpu/drm/radeon/radeon_asic.h b/drivers/gpu/drm/radeon/radeon_asic.h
> index 45d0693..b04b578 100644
> --- a/drivers/gpu/drm/radeon/radeon_asic.h
> +++ b/drivers/gpu/drm/radeon/radeon_asic.h
> @@ -340,6 +340,9 @@ int r600_uvd_ring_test(struct radeon_device *rdev, struct radeon_ring *ring);
> int r600_copy_blit(struct radeon_device *rdev,
> uint64_t src_offset, uint64_t dst_offset,
> unsigned num_gpu_pages, struct radeon_fence **fence);
> +int r600_copy_cpdma(struct radeon_device *rdev,
> + uint64_t src_offset, uint64_t dst_offset,
> + unsigned num_gpu_pages, struct radeon_fence **fence);
> int r600_copy_dma(struct radeon_device *rdev,
> uint64_t src_offset, uint64_t dst_offset,
> unsigned num_gpu_pages, struct radeon_fence **fence);
> --
> 1.7.7.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] drm/radeon: use CP DMA on r6xx for bo moves
2013-07-11 19:35 [PATCH 1/3] drm/radeon: Disable dma rings for bo moves on r6xx alexdeucher
2013-07-11 19:35 ` [PATCH 2/3] drm/radeon: implement bo copy callback using CP DMA (v2) alexdeucher
@ 2013-07-11 19:35 ` alexdeucher
2013-07-11 19:59 ` [PATCH 1/3] drm/radeon: Disable dma rings for bo moves on r6xx Ilija Hadzic
2013-07-12 6:50 ` Christian König
3 siblings, 0 replies; 10+ messages in thread
From: alexdeucher @ 2013-07-11 19:35 UTC (permalink / raw)
To: dri-devel; +Cc: Alex Deucher
From: Alex Deucher <alexander.deucher@amd.com>
Lighter weight than using the 3D engine.
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/radeon/radeon_asic.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_asic.c b/drivers/gpu/drm/radeon/radeon_asic.c
index ea5c52b..fea997e 100644
--- a/drivers/gpu/drm/radeon/radeon_asic.c
+++ b/drivers/gpu/drm/radeon/radeon_asic.c
@@ -1026,7 +1026,7 @@ static struct radeon_asic r600_asic = {
.blit_ring_index = RADEON_RING_TYPE_GFX_INDEX,
.dma = &r600_copy_dma,
.dma_ring_index = R600_RING_TYPE_DMA_INDEX,
- .copy = &r600_copy_blit,
+ .copy = &r600_copy_cpdma,
.copy_ring_index = RADEON_RING_TYPE_GFX_INDEX,
},
.surface = {
@@ -1119,7 +1119,7 @@ static struct radeon_asic rv6xx_asic = {
.blit_ring_index = RADEON_RING_TYPE_GFX_INDEX,
.dma = &r600_copy_dma,
.dma_ring_index = R600_RING_TYPE_DMA_INDEX,
- .copy = &r600_copy_blit,
+ .copy = &r600_copy_cpdma,
.copy_ring_index = RADEON_RING_TYPE_GFX_INDEX,
},
.surface = {
@@ -1229,7 +1229,7 @@ static struct radeon_asic rs780_asic = {
.blit_ring_index = RADEON_RING_TYPE_GFX_INDEX,
.dma = &r600_copy_dma,
.dma_ring_index = R600_RING_TYPE_DMA_INDEX,
- .copy = &r600_copy_blit,
+ .copy = &r600_copy_cpdma,
.copy_ring_index = RADEON_RING_TYPE_GFX_INDEX,
},
.surface = {
--
1.7.7.5
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 1/3] drm/radeon: Disable dma rings for bo moves on r6xx
2013-07-11 19:35 [PATCH 1/3] drm/radeon: Disable dma rings for bo moves on r6xx alexdeucher
2013-07-11 19:35 ` [PATCH 2/3] drm/radeon: implement bo copy callback using CP DMA (v2) alexdeucher
2013-07-11 19:35 ` [PATCH 3/3] drm/radeon: use CP DMA on r6xx for bo moves alexdeucher
@ 2013-07-11 19:59 ` Ilija Hadzic
2013-07-11 20:05 ` Alex Deucher
2013-07-12 6:53 ` Christian König
2013-07-12 6:50 ` Christian König
3 siblings, 2 replies; 10+ messages in thread
From: Ilija Hadzic @ 2013-07-11 19:59 UTC (permalink / raw)
To: alexdeucher; +Cc: Alex Deucher, dri-devel
Alex,
Can you please share some details about the nature or symptom of the
"instability". One problem that I have been seeing on my end is that when
I use the DMA ring intensively (by intensively I mean, calling the copy
function every frame), combined with some 3D rendering that causes a lot
of cs_ioctl calls, that I can provoke a corruption of the olist field in
radeon_sa_bo structure, consequently causing an oops in
radeon_sa_bo_try_free(). I have also found that I can suppress the problem
if I add some padding fields at the beginnig of the radeon_sa_bo structure
(essentially moving the olist field down).
So I speculate that the use of DMA somehow results in corrupting that
structure, but I have not yet done enough experiments to prove or disprove
that theory (so I have not spoke up yet). Also my kernel is full of my own
internal hacks, and I have not yet taken the time to reproduce the problem
with the "stock" kernel. However, after seeing your patch series, I am
wondering if the instability you are referring to may be of the same
or similar nature.
thanks,
Ilija
On Thu, 11 Jul 2013, alexdeucher@gmail.com wrote:
> From: Alex Deucher <alexander.deucher@amd.com>
>
> They still seem to cause instability on some r6xx parts.
> As a follow up, we can switch to using CP DMA for bo
> moves on r6xx as a lighter weight alternative to using
> the 3D engine.
>
> A version of this patch should also go to stable kernels.
>
> Tested-by: J.N. <golden.fleeced@gmail.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/radeon/radeon_asic.c | 12 ++++++------
> 1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_asic.c b/drivers/gpu/drm/radeon/radeon_asic.c
> index 0970774..ea5c52b 100644
> --- a/drivers/gpu/drm/radeon/radeon_asic.c
> +++ b/drivers/gpu/drm/radeon/radeon_asic.c
> @@ -1026,8 +1026,8 @@ static struct radeon_asic r600_asic = {
> .blit_ring_index = RADEON_RING_TYPE_GFX_INDEX,
> .dma = &r600_copy_dma,
> .dma_ring_index = R600_RING_TYPE_DMA_INDEX,
> - .copy = &r600_copy_dma,
> - .copy_ring_index = R600_RING_TYPE_DMA_INDEX,
> + .copy = &r600_copy_blit,
> + .copy_ring_index = RADEON_RING_TYPE_GFX_INDEX,
> },
> .surface = {
> .set_reg = r600_set_surface_reg,
> @@ -1119,8 +1119,8 @@ static struct radeon_asic rv6xx_asic = {
> .blit_ring_index = RADEON_RING_TYPE_GFX_INDEX,
> .dma = &r600_copy_dma,
> .dma_ring_index = R600_RING_TYPE_DMA_INDEX,
> - .copy = &r600_copy_dma,
> - .copy_ring_index = R600_RING_TYPE_DMA_INDEX,
> + .copy = &r600_copy_blit,
> + .copy_ring_index = RADEON_RING_TYPE_GFX_INDEX,
> },
> .surface = {
> .set_reg = r600_set_surface_reg,
> @@ -1229,8 +1229,8 @@ static struct radeon_asic rs780_asic = {
> .blit_ring_index = RADEON_RING_TYPE_GFX_INDEX,
> .dma = &r600_copy_dma,
> .dma_ring_index = R600_RING_TYPE_DMA_INDEX,
> - .copy = &r600_copy_dma,
> - .copy_ring_index = R600_RING_TYPE_DMA_INDEX,
> + .copy = &r600_copy_blit,
> + .copy_ring_index = RADEON_RING_TYPE_GFX_INDEX,
> },
> .surface = {
> .set_reg = r600_set_surface_reg,
> --
> 1.7.7.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/3] drm/radeon: Disable dma rings for bo moves on r6xx
2013-07-11 19:59 ` [PATCH 1/3] drm/radeon: Disable dma rings for bo moves on r6xx Ilija Hadzic
@ 2013-07-11 20:05 ` Alex Deucher
2013-07-12 6:53 ` Christian König
1 sibling, 0 replies; 10+ messages in thread
From: Alex Deucher @ 2013-07-11 20:05 UTC (permalink / raw)
To: Ilija Hadzic; +Cc: Alex Deucher, dri-devel
On Thu, Jul 11, 2013 at 3:59 PM, Ilija Hadzic
<ihadzic@research.bell-labs.com> wrote:
>
> Alex,
>
> Can you please share some details about the nature or symptom of the
> "instability". One problem that I have been seeing on my end is that when I
> use the DMA ring intensively (by intensively I mean, calling the copy
> function every frame), combined with some 3D rendering that causes a lot of
> cs_ioctl calls, that I can provoke a corruption of the olist field in
> radeon_sa_bo structure, consequently causing an oops in
> radeon_sa_bo_try_free(). I have also found that I can suppress the problem
> if I add some padding fields at the beginnig of the radeon_sa_bo structure
> (essentially moving the olist field down).
>
> So I speculate that the use of DMA somehow results in corrupting that
> structure, but I have not yet done enough experiments to prove or disprove
> that theory (so I have not spoke up yet). Also my kernel is full of my own
> internal hacks, and I have not yet taken the time to reproduce the problem
> with the "stock" kernel. However, after seeing your patch series, I am
> wondering if the instability you are referring to may be of the same or
> similar nature.
Basically using the DMA engine on r6xx seems to lead to hangs on
certain chips when the bo copy callback gets triggered a lot (e.g., a
web page with lots of images, etc.). Switching back to the 3D engine
fixes the issue. R6xx was the first asic family with the async dma
engines, so there are likely some hw bugs at play. Unfortunately, the
hw guys haven't looked at 6xx in probably 8 years, so I don't hold out
much chance of getting to the root of the problem at this point.
Alex
>
> thanks,
>
> Ilija
>
>
>
> On Thu, 11 Jul 2013, alexdeucher@gmail.com wrote:
>
>> From: Alex Deucher <alexander.deucher@amd.com>
>>
>> They still seem to cause instability on some r6xx parts.
>> As a follow up, we can switch to using CP DMA for bo
>> moves on r6xx as a lighter weight alternative to using
>> the 3D engine.
>>
>> A version of this patch should also go to stable kernels.
>>
>> Tested-by: J.N. <golden.fleeced@gmail.com>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> ---
>> drivers/gpu/drm/radeon/radeon_asic.c | 12 ++++++------
>> 1 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_asic.c
>> b/drivers/gpu/drm/radeon/radeon_asic.c
>> index 0970774..ea5c52b 100644
>> --- a/drivers/gpu/drm/radeon/radeon_asic.c
>> +++ b/drivers/gpu/drm/radeon/radeon_asic.c
>> @@ -1026,8 +1026,8 @@ static struct radeon_asic r600_asic = {
>> .blit_ring_index = RADEON_RING_TYPE_GFX_INDEX,
>> .dma = &r600_copy_dma,
>> .dma_ring_index = R600_RING_TYPE_DMA_INDEX,
>> - .copy = &r600_copy_dma,
>> - .copy_ring_index = R600_RING_TYPE_DMA_INDEX,
>> + .copy = &r600_copy_blit,
>> + .copy_ring_index = RADEON_RING_TYPE_GFX_INDEX,
>> },
>> .surface = {
>> .set_reg = r600_set_surface_reg,
>> @@ -1119,8 +1119,8 @@ static struct radeon_asic rv6xx_asic = {
>> .blit_ring_index = RADEON_RING_TYPE_GFX_INDEX,
>> .dma = &r600_copy_dma,
>> .dma_ring_index = R600_RING_TYPE_DMA_INDEX,
>> - .copy = &r600_copy_dma,
>> - .copy_ring_index = R600_RING_TYPE_DMA_INDEX,
>> + .copy = &r600_copy_blit,
>> + .copy_ring_index = RADEON_RING_TYPE_GFX_INDEX,
>> },
>> .surface = {
>> .set_reg = r600_set_surface_reg,
>> @@ -1229,8 +1229,8 @@ static struct radeon_asic rs780_asic = {
>> .blit_ring_index = RADEON_RING_TYPE_GFX_INDEX,
>> .dma = &r600_copy_dma,
>> .dma_ring_index = R600_RING_TYPE_DMA_INDEX,
>> - .copy = &r600_copy_dma,
>> - .copy_ring_index = R600_RING_TYPE_DMA_INDEX,
>> + .copy = &r600_copy_blit,
>> + .copy_ring_index = RADEON_RING_TYPE_GFX_INDEX,
>> },
>> .surface = {
>> .set_reg = r600_set_surface_reg,
>> --
>> 1.7.7.5
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/3] drm/radeon: Disable dma rings for bo moves on r6xx
2013-07-11 19:59 ` [PATCH 1/3] drm/radeon: Disable dma rings for bo moves on r6xx Ilija Hadzic
2013-07-11 20:05 ` Alex Deucher
@ 2013-07-12 6:53 ` Christian König
2013-07-12 14:23 ` Ilija Hadzic
1 sibling, 1 reply; 10+ messages in thread
From: Christian König @ 2013-07-12 6:53 UTC (permalink / raw)
To: Ilija Hadzic; +Cc: Alex Deucher, dri-devel
Am 11.07.2013 21:59, schrieb Ilija Hadzic:
>
> Alex,
>
> Can you please share some details about the nature or symptom of the
> "instability". One problem that I have been seeing on my end is that
> when I use the DMA ring intensively (by intensively I mean, calling
> the copy function every frame), combined with some 3D rendering that
> causes a lot of cs_ioctl calls, that I can provoke a corruption of the
> olist field in radeon_sa_bo structure, consequently causing an oops in
> radeon_sa_bo_try_free(). I have also found that I can suppress the
> problem if I add some padding fields at the beginnig of the
> radeon_sa_bo structure (essentially moving the olist field down).
>
> So I speculate that the use of DMA somehow results in corrupting that
> structure, but I have not yet done enough experiments to prove or
> disprove that theory (so I have not spoke up yet). Also my kernel is
> full of my own internal hacks, and I have not yet taken the time to
> reproduce the problem with the "stock" kernel. However, after seeing
> your patch series, I am wondering if the instability you are referring
> to may be of the same or similar nature.
>
Hi Ilija,
well that's very interesting and no it's quite unlikely that this is
cause by the DMA ring because the radeon_sa_bo structure should be
allocated on system memory and the GPU can usually only access it if you
map it through GART.
Is that easily reproducible for you? Is there already an open bugreport?
Thanks,
Christian.
> thanks,
>
> Ilija
>
>
> On Thu, 11 Jul 2013, alexdeucher@gmail.com wrote:
>
>> From: Alex Deucher <alexander.deucher@amd.com>
>>
>> They still seem to cause instability on some r6xx parts.
>> As a follow up, we can switch to using CP DMA for bo
>> moves on r6xx as a lighter weight alternative to using
>> the 3D engine.
>>
>> A version of this patch should also go to stable kernels.
>>
>> Tested-by: J.N. <golden.fleeced@gmail.com>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> ---
>> drivers/gpu/drm/radeon/radeon_asic.c | 12 ++++++------
>> 1 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_asic.c
>> b/drivers/gpu/drm/radeon/radeon_asic.c
>> index 0970774..ea5c52b 100644
>> --- a/drivers/gpu/drm/radeon/radeon_asic.c
>> +++ b/drivers/gpu/drm/radeon/radeon_asic.c
>> @@ -1026,8 +1026,8 @@ static struct radeon_asic r600_asic = {
>> .blit_ring_index = RADEON_RING_TYPE_GFX_INDEX,
>> .dma = &r600_copy_dma,
>> .dma_ring_index = R600_RING_TYPE_DMA_INDEX,
>> - .copy = &r600_copy_dma,
>> - .copy_ring_index = R600_RING_TYPE_DMA_INDEX,
>> + .copy = &r600_copy_blit,
>> + .copy_ring_index = RADEON_RING_TYPE_GFX_INDEX,
>> },
>> .surface = {
>> .set_reg = r600_set_surface_reg,
>> @@ -1119,8 +1119,8 @@ static struct radeon_asic rv6xx_asic = {
>> .blit_ring_index = RADEON_RING_TYPE_GFX_INDEX,
>> .dma = &r600_copy_dma,
>> .dma_ring_index = R600_RING_TYPE_DMA_INDEX,
>> - .copy = &r600_copy_dma,
>> - .copy_ring_index = R600_RING_TYPE_DMA_INDEX,
>> + .copy = &r600_copy_blit,
>> + .copy_ring_index = RADEON_RING_TYPE_GFX_INDEX,
>> },
>> .surface = {
>> .set_reg = r600_set_surface_reg,
>> @@ -1229,8 +1229,8 @@ static struct radeon_asic rs780_asic = {
>> .blit_ring_index = RADEON_RING_TYPE_GFX_INDEX,
>> .dma = &r600_copy_dma,
>> .dma_ring_index = R600_RING_TYPE_DMA_INDEX,
>> - .copy = &r600_copy_dma,
>> - .copy_ring_index = R600_RING_TYPE_DMA_INDEX,
>> + .copy = &r600_copy_blit,
>> + .copy_ring_index = RADEON_RING_TYPE_GFX_INDEX,
>> },
>> .surface = {
>> .set_reg = r600_set_surface_reg,
>> --
>> 1.7.7.5
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/3] drm/radeon: Disable dma rings for bo moves on r6xx
2013-07-12 6:53 ` Christian König
@ 2013-07-12 14:23 ` Ilija Hadzic
0 siblings, 0 replies; 10+ messages in thread
From: Ilija Hadzic @ 2013-07-12 14:23 UTC (permalink / raw)
To: Christian König; +Cc: Alex Deucher, dri-devel
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1074 bytes --]
On Fri, 12 Jul 2013, Christian König wrote:
> Hi Ilija,
>
> well that's very interesting and no it's quite unlikely that this is cause by
> the DMA ring because the radeon_sa_bo structure should be allocated on system
> memory and the GPU can usually only access it if you map it through GART.
>
OK that's good information to know. I'll do some more investigation on my
end.
> Is that easily reproducible for you? Is there already an open bugreport?
>
It typically happens out of the blue after several days of running, but
it happens consistently (just like padding the radeon_sa_bo structure
consistently suppresses the problem).
However, like I said, my kernel is full of my own patches that don't live
in the upstream tree, so I am not excluding the possibility that one of my
hacks introduced the problem (though, I find it unlikely).
I have not yet opened the bug report, because I first want to reproduce
the problem with the upstream kernel that is free of my hacks. As soon as
I do that, I'll write the ticket.
-- Ilija
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] drm/radeon: Disable dma rings for bo moves on r6xx
2013-07-11 19:35 [PATCH 1/3] drm/radeon: Disable dma rings for bo moves on r6xx alexdeucher
` (2 preceding siblings ...)
2013-07-11 19:59 ` [PATCH 1/3] drm/radeon: Disable dma rings for bo moves on r6xx Ilija Hadzic
@ 2013-07-12 6:50 ` Christian König
2013-07-12 13:04 ` Alex Deucher
3 siblings, 1 reply; 10+ messages in thread
From: Christian König @ 2013-07-12 6:50 UTC (permalink / raw)
To: alexdeucher; +Cc: Alex Deucher, dri-devel
Am 11.07.2013 21:35, schrieb alexdeucher@gmail.com:
> From: Alex Deucher <alexander.deucher@amd.com>
>
> They still seem to cause instability on some r6xx parts.
> As a follow up, we can switch to using CP DMA for bo
> moves on r6xx as a lighter weight alternative to using
> the 3D engine.
>
> A version of this patch should also go to stable kernels.
>
> Tested-by: J.N. <golden.fleeced@gmail.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Well since we have an easy to use alternative, isn't it time to kill off
all the blitter code?
Anyway this patch series is:
Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/radeon/radeon_asic.c | 12 ++++++------
> 1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_asic.c b/drivers/gpu/drm/radeon/radeon_asic.c
> index 0970774..ea5c52b 100644
> --- a/drivers/gpu/drm/radeon/radeon_asic.c
> +++ b/drivers/gpu/drm/radeon/radeon_asic.c
> @@ -1026,8 +1026,8 @@ static struct radeon_asic r600_asic = {
> .blit_ring_index = RADEON_RING_TYPE_GFX_INDEX,
> .dma = &r600_copy_dma,
> .dma_ring_index = R600_RING_TYPE_DMA_INDEX,
> - .copy = &r600_copy_dma,
> - .copy_ring_index = R600_RING_TYPE_DMA_INDEX,
> + .copy = &r600_copy_blit,
> + .copy_ring_index = RADEON_RING_TYPE_GFX_INDEX,
> },
> .surface = {
> .set_reg = r600_set_surface_reg,
> @@ -1119,8 +1119,8 @@ static struct radeon_asic rv6xx_asic = {
> .blit_ring_index = RADEON_RING_TYPE_GFX_INDEX,
> .dma = &r600_copy_dma,
> .dma_ring_index = R600_RING_TYPE_DMA_INDEX,
> - .copy = &r600_copy_dma,
> - .copy_ring_index = R600_RING_TYPE_DMA_INDEX,
> + .copy = &r600_copy_blit,
> + .copy_ring_index = RADEON_RING_TYPE_GFX_INDEX,
> },
> .surface = {
> .set_reg = r600_set_surface_reg,
> @@ -1229,8 +1229,8 @@ static struct radeon_asic rs780_asic = {
> .blit_ring_index = RADEON_RING_TYPE_GFX_INDEX,
> .dma = &r600_copy_dma,
> .dma_ring_index = R600_RING_TYPE_DMA_INDEX,
> - .copy = &r600_copy_dma,
> - .copy_ring_index = R600_RING_TYPE_DMA_INDEX,
> + .copy = &r600_copy_blit,
> + .copy_ring_index = RADEON_RING_TYPE_GFX_INDEX,
> },
> .surface = {
> .set_reg = r600_set_surface_reg,
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/3] drm/radeon: Disable dma rings for bo moves on r6xx
2013-07-12 6:50 ` Christian König
@ 2013-07-12 13:04 ` Alex Deucher
0 siblings, 0 replies; 10+ messages in thread
From: Alex Deucher @ 2013-07-12 13:04 UTC (permalink / raw)
To: Christian König; +Cc: Alex Deucher, dri-devel
On Fri, Jul 12, 2013 at 2:50 AM, Christian König
<deathsimple@vodafone.de> wrote:
> Am 11.07.2013 21:35, schrieb alexdeucher@gmail.com:
>
>> From: Alex Deucher <alexander.deucher@amd.com>
>>
>> They still seem to cause instability on some r6xx parts.
>> As a follow up, we can switch to using CP DMA for bo
>> moves on r6xx as a lighter weight alternative to using
>> the 3D engine.
>>
>> A version of this patch should also go to stable kernels.
>>
>> Tested-by: J.N. <golden.fleeced@gmail.com>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>
>
> Well since we have an easy to use alternative, isn't it time to kill off all
> the blitter code?
I was thinking that could be done as a clean-up in 3.12.
Alex
>
> Anyway this patch series is:
>
> Reviewed-by: Christian König <christian.koenig@amd.com>
>
>
>> ---
>> drivers/gpu/drm/radeon/radeon_asic.c | 12 ++++++------
>> 1 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_asic.c
>> b/drivers/gpu/drm/radeon/radeon_asic.c
>> index 0970774..ea5c52b 100644
>> --- a/drivers/gpu/drm/radeon/radeon_asic.c
>> +++ b/drivers/gpu/drm/radeon/radeon_asic.c
>> @@ -1026,8 +1026,8 @@ static struct radeon_asic r600_asic = {
>> .blit_ring_index = RADEON_RING_TYPE_GFX_INDEX,
>> .dma = &r600_copy_dma,
>> .dma_ring_index = R600_RING_TYPE_DMA_INDEX,
>> - .copy = &r600_copy_dma,
>> - .copy_ring_index = R600_RING_TYPE_DMA_INDEX,
>> + .copy = &r600_copy_blit,
>> + .copy_ring_index = RADEON_RING_TYPE_GFX_INDEX,
>> },
>> .surface = {
>> .set_reg = r600_set_surface_reg,
>> @@ -1119,8 +1119,8 @@ static struct radeon_asic rv6xx_asic = {
>> .blit_ring_index = RADEON_RING_TYPE_GFX_INDEX,
>> .dma = &r600_copy_dma,
>> .dma_ring_index = R600_RING_TYPE_DMA_INDEX,
>> - .copy = &r600_copy_dma,
>> - .copy_ring_index = R600_RING_TYPE_DMA_INDEX,
>> + .copy = &r600_copy_blit,
>> + .copy_ring_index = RADEON_RING_TYPE_GFX_INDEX,
>> },
>> .surface = {
>> .set_reg = r600_set_surface_reg,
>> @@ -1229,8 +1229,8 @@ static struct radeon_asic rs780_asic = {
>> .blit_ring_index = RADEON_RING_TYPE_GFX_INDEX,
>> .dma = &r600_copy_dma,
>> .dma_ring_index = R600_RING_TYPE_DMA_INDEX,
>> - .copy = &r600_copy_dma,
>> - .copy_ring_index = R600_RING_TYPE_DMA_INDEX,
>> + .copy = &r600_copy_blit,
>> + .copy_ring_index = RADEON_RING_TYPE_GFX_INDEX,
>> },
>> .surface = {
>> .set_reg = r600_set_surface_reg,
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread