Comment 12 for bug 820746

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

@Herton : Good point regarding a similar fix in r100.c that I wasn't aware.
I didn't even reproduce this as @mynk (reporter and buddy) reproduced this as I don't have a hardware with radeon chipset :)

I debugged this with the objdump disassembly and the OOPs information. Seems to exactly match the fix and the comment in r100.c that corrects the write index.

Masking the write pointer with the ring buffer ptr_mask like in r100.c makes sense even if it doesn't match the exact write index (would write to the last byte before rolling back to 0 in the ring buffer) on resume since I was trying to re-read with a delay. The fact that the retry with mdelay was working for him on a resume implies that it was indeed fetching the right values on a re-read.

But my patch was causing a boot time lockup as it was doing the same thing for ring buffer read index and it seems that the ring buffer read index returned from the hardware is always uninitialized or ~0U during boot.
So maybe a mask for read index makes sense but the fact that its acceptable to issue a read to the iommu at an invalid offset before its corrected in the next read pass that patches it with ring ptr_mask suggests that its not a deal breaker.

So lets play it safe and retain the same fix in r600.c as it is in r100.c:

@Mayank : Please revert the last patch and apply the patch on top of your original un-patched r600.c.
(also attached)

@Herton: Once Mayank re-tests with the patch and confirms that it works, I can push for it upstream quoting this bug as the reference. I am surprised that the bug still exists in 3.0 as well. And I don't believe it has anything to do with regression. It could be that we are plain lucky with resume since this is related to the hardware returning an invalid index on resume.

--- r600_orig.c 2011-08-05 13:39:25.833427436 -0700
+++ r600.c 2011-08-05 14:50:32.037670946 -0700
@@ -2218,6 +2218,8 @@

    rdev->cp.rptr = RREG32(CP_RB_RPTR);
    rdev->cp.wptr = RREG32(CP_RB_WPTR);
+ /* protect against crazy HW on resume */
+ rdev->cp.wptr &= rdev->cp.ptr_mask;

    r600_cp_start(rdev);
    rdev->cp.ready = true;