Comment 17 for bug 820746

Revision history for this message
a.r.karthick@gmail.com (a-r-karthick) wrote :

@brad-figg

Yes we did mail lkml and dri-devel. They had ack'ed it back then and proposed to resolve it by just resetting the write ring buffer index to 0 on resume to be safe.

But not sure it got submitted upstream as it had asked for confirmation. I am sure the patch would have worked but I debugged the issue which was reproduced by @mynk in his hardware setup as it is typical to his setup. Since I didn't have access or could reproduce locally, I just debugged it with the objdump. Maybe you should pitch for it upstream if it hasn't been merged.

Here is the mail that was sent back in response to our submission:

From c564bc8e6d449216d74ee134d5bf470221f79e8d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michel=20D=C3=A4nzer?= <email address hidden>
Date: Thu, 8 Sep 2011 11:09:39 +0200
Subject: [PATCH] drm/radeon: Don't read from CP ring write pointer registers.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

"
The patch below is what I had in mind. Does this fix the problem above?
"

Apparently this doesn't always work reliably, e.g. at resume time.

Just initialize to 0, so the ring is considered empty.

Tested with hibernation on Sumo and Cayman cards.

Should fix https://bugs.launchpad.net/ubuntu/+source/linux/+bug/820746/ .

Signed-off-by: Michel Dänzer <email address hidden>
---
 drivers/gpu/drm/radeon/evergreen.c | 4 ++--
 drivers/gpu/drm/radeon/ni.c | 12 ++++++------
 drivers/gpu/drm/radeon/r100.c | 6 ++----
 drivers/gpu/drm/radeon/r600.c | 4 ++--
 4 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
index 15bd047..f2bd90a 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -1378,7 +1378,8 @@ int evergreen_cp_resume(struct radeon_device *rdev)
       /* Initialize the ring buffer's read and write pointers */
       WREG32(CP_RB_CNTL, tmp | RB_RPTR_WR_ENA);
       WREG32(CP_RB_RPTR_WR, 0);
- WREG32(CP_RB_WPTR, 0);
+ rdev->cp.wptr = 0;
+ WREG32(CP_RB_WPTR, rdev->cp.wptr);

       /* set the wb address wether it's enabled or not */
       WREG32(CP_RB_RPTR_ADDR,
@@ -1403,7 +1404,6 @@ int evergreen_cp_resume(struct radeon_device *rdev)
       WREG32(CP_DEBUG, (1 << 27) | (1 << 28));

       rdev->cp.rptr = RREG32(CP_RB_RPTR);
- rdev->cp.wptr = RREG32(CP_RB_WPTR);

       evergreen_cp_start(rdev);
       rdev->cp.ready = true;
diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
index 559dbd4..e3489ee 100644
--- a/drivers/gpu/drm/radeon/ni.c
+++ b/drivers/gpu/drm/radeon/ni.c
@@ -1182,7 +1182,8 @@ int cayman_cp_resume(struct radeon_device *rdev)

       /* Initialize the ring buffer's read and write pointers */
       WREG32(CP_RB0_CNTL, tmp | RB_RPTR_WR_ENA);
- WREG32(CP_RB0_WPTR, 0);
+ rdev->cp.wptr = 0;
+ WREG32(CP_RB0_WPTR, rdev->cp.wptr);

       /* set the wb address wether it's enabled or not */
       WREG32(CP_RB0_RPTR_ADDR, (rdev->wb.gpu_addr + RADEON_WB_CP_RPTR_OFFSET) & 0xFFFFFFFC);
@@ -1202,7 +1203,6 @@ int cayman_cp_resume(struct radeon_device *rdev)
       WREG32(CP_RB0_BASE, rdev->cp.gpu_addr >> 8);

       rdev->cp.rptr = RREG32(CP_RB0_RPTR);
- rdev->cp.wptr = RREG32(CP_RB0_WPTR);

       /* ring1 - compute only */
       /* Set ring buffer size */
@@ -1215,7 +1215,8 @@ int cayman_cp_resume(struct radeon_device *rdev)

       /* Initialize the ring buffer's read and write pointers */
       WREG32(CP_RB1_CNTL, tmp | RB_RPTR_WR_ENA);
- WREG32(CP_RB1_WPTR, 0);
+ rdev->cp1.wptr = 0;
+ WREG32(CP_RB1_WPTR, rdev->cp1.wptr);

       /* set the wb address wether it's enabled or not */
       WREG32(CP_RB1_RPTR_ADDR, (rdev->wb.gpu_addr + RADEON_WB_CP1_RPTR_OFFSET) & 0xFFFFFFFC);
@@ -1227,7 +1228,6 @@ int cayman_cp_resume(struct radeon_device *rdev)
       WREG32(CP_RB1_BASE, rdev->cp1.gpu_addr >> 8);

       rdev->cp1.rptr = RREG32(CP_RB1_RPTR);
- rdev->cp1.wptr = RREG32(CP_RB1_WPTR);

       /* ring2 - compute only */
       /* Set ring buffer size */
@@ -1240,7 +1240,8 @@ int cayman_cp_resume(struct radeon_device *rdev)

       /* Initialize the ring buffer's read and write pointers */
       WREG32(CP_RB2_CNTL, tmp | RB_RPTR_WR_ENA);
- WREG32(CP_RB2_WPTR, 0);
+ rdev->cp2.wptr = 0;
+ WREG32(CP_RB2_WPTR, rdev->cp2.wptr);

       /* set the wb address wether it's enabled or not */
       WREG32(CP_RB2_RPTR_ADDR, (rdev->wb.gpu_addr + RADEON_WB_CP2_RPTR_OFFSET) & 0xFFFFFFFC);
@@ -1252,7 +1253,6 @@ int cayman_cp_resume(struct radeon_device *rdev)
       WREG32(CP_RB2_BASE, rdev->cp2.gpu_addr >> 8);

       rdev->cp2.rptr = RREG32(CP_RB2_RPTR);
- rdev->cp2.wptr = RREG32(CP_RB2_WPTR);

       /* start the rings */
       cayman_cp_start(rdev);
diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
index f2204cb..11e44a3 100644
--- a/drivers/gpu/drm/radeon/r100.c
+++ b/drivers/gpu/drm/radeon/r100.c
@@ -990,7 +990,8 @@ int r100_cp_init(struct radeon_device *rdev, unsigned ring_size)
       /* Force read & write ptr to 0 */
       WREG32(RADEON_CP_RB_CNTL, tmp | RADEON_RB_RPTR_WR_ENA | RADEON_RB_NO_UPDATE);
       WREG32(RADEON_CP_RB_RPTR_WR, 0);
- WREG32(RADEON_CP_RB_WPTR, 0);
+ rdev->cp.wptr = 0;
+ WREG32(RADEON_CP_RB_WPTR, rdev->cp.wptr);

       /* set the wb address whether it's enabled or not */
       WREG32(R_00070C_CP_RB_RPTR_ADDR,
@@ -1007,9 +1008,6 @@ int r100_cp_init(struct radeon_device *rdev, unsigned ring_size)
       WREG32(RADEON_CP_RB_CNTL, tmp);
       udelay(10);
       rdev->cp.rptr = RREG32(RADEON_CP_RB_RPTR);
- rdev->cp.wptr = RREG32(RADEON_CP_RB_WPTR);
- /* protect against crazy HW on resume */
- rdev->cp.wptr &= rdev->cp.ptr_mask;
       /* Set cp mode to bus mastering & enable cp*/
       WREG32(RADEON_CP_CSQ_MODE,
              REG_SET(RADEON_INDIRECT2_START, indirect2_start) |
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index bc54b26..8ca098d 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -2208,7 +2208,8 @@ int r600_cp_resume(struct radeon_device *rdev)
       /* Initialize the ring buffer's read and write pointers */
       WREG32(CP_RB_CNTL, tmp | RB_RPTR_WR_ENA);
       WREG32(CP_RB_RPTR_WR, 0);
- WREG32(CP_RB_WPTR, 0);
+ rdev->cp.wptr = 0;
+ WREG32(CP_RB_WPTR, rdev->cp.wptr);

       /* set the wb address whether it's enabled or not */
       WREG32(CP_RB_RPTR_ADDR,
@@ -2233,7 +2234,6 @@ int r600_cp_resume(struct radeon_device *rdev)
       WREG32(CP_DEBUG, (1 << 27) | (1 << 28));

       rdev->cp.rptr = RREG32(CP_RB_RPTR);
- rdev->cp.wptr = RREG32(CP_RB_WPTR);