First I reported bug #24781 but that was invalid report, there was not bug in code where I thought. However there are for sure some issues in current ring info debugfs. ********** 1) We count buffer's elements in following way: r600.c::r600_debugfs_cp_ring_info count = (rdp + rdev->cp.ring_size - wdp) & rdev->cp.ptr_mask; a) We don't divide cp.ring_size by 4 but fortunately this doesn't cause problems as we use bit multiplication by mask. However maybe we should do so for code readability? b) What we count is actually amount of *free* dwords in buffer. This is the same what gets done in radeon_ring_free_size. You can see results of this mistake in bug #24535 comment #6 - amount of free and used dwords is the same which is incorrect. The only exception when this work is empty ring situation. In radeon_ring.c::radeon_ring_free_size we treat 0 as (rdev->cp.ring_size / 4) so we get nice (correct): > 262128 free dwords in ring > 0 dwords in ring then. Ideally this probably should be: count = (wdp + rdev->cp.ring_size - rdp) & rdev->cp.ptr_mask; ********** 2) We have inconsistent in reading CP_RB_WPTR and CP_RB_RPTR. This leads to situations like: > 262127 free dwords in ring > 0 dwords in ring where 262127 + 0 != 262144 (and 262144 is buffer size ofc). That happens because we call radeon_ring_free_size where we calculate amount of free dwords using rdev->cp.rptr and rdev->cp.wptr. Then we read CP_RB_RPTR and CP_RB_WPTR and calculate used dwords. As ring changes often this causes mentioned inconsistency. I belive we should use rdev->cp.rptr and rdev->cp.wptr in r600_debugfs_cp_ring_info and eventually consider updating rdev->cp.wptr in radeon_ring_free_size. ********** 3) Finally we don't detect ring end in printing ring's content in r600_debugfs_cp_ring_info.
Created attachment 30893 [details] [review] fix ring info in debugfs on r600+ With this patch applied I get: # cat r600_ring_info CP_STAT 0x800000c1 CP_RB_WPTR 0x0000ade0 CP_RB_RPTR 0x0000add0 262128 free dwords in ring 16 dwords in ring r[44497]=0x10181000 r[44498]=0x00000000 r[44499]=0x000002a0 r[44500]=0xc0016800 r[44501]=0x00000140 r[44502]=0x00000489 r[44503]=0x80000000 r[44504]=0x80000000 r[44505]=0x80000000 r[44506]=0x80000000 r[44507]=0x80000000 r[44508]=0x80000000 r[44509]=0x80000000 r[44510]=0x80000000 r[44511]=0x80000000 r[44512]=0x0901000d r[44513]=0x000dc1ea you can compare this to /broken/ data in bug #24535 comment #6
Created attachment 30901 [details] [review] fix ring info in debugfs on r600+ V2 First version was dumping ring from CP_RB_RPTR + 1 to CP_RB_WPTR + 1
Created attachment 30995 [details] [review] fix ring info in debugfs on r600+ V3 Both: V1 and V2 missed printing actual value of CP_RB_WPTR and CP_RB_RPTR. Now we print that registers and local value of them.
Example of debug info with patch V3 applied: CP_STAT 0x800000c1 CP_RB_WPTR 0x000126a0 CP_RB_RPTR 0x000126a0 driver's copy of the CP_RB_WPTR 0x000126a0 driver's copy of the CP_RB_RPTR 0x00012690 262128 free dwords in ring 16 dwords in ring r[75408]=0xc0023200 r[75409]=0x10101000 r[75410]=0x00000000 r[75411]=0x0000028d r[75412]=0xc0016800 r[75413]=0x00000140 r[75414]=0x00000e21 r[75415]=0x80000000 r[75416]=0x80000000 r[75417]=0x80000000 r[75418]=0x80000000 r[75419]=0x80000000 r[75420]=0x80000000 r[75421]=0x80000000 r[75422]=0x80000000 r[75423]=0x80000000 r[75424]=0x0030189a
I believe the patch is upstream now
(In reply to comment #5) > I believe the patch is upstream now It didn't hit drm-next or drm-radeon-testing, not to mention mainline. But OK, let's keep it resolved, I'll post commit when Dave take this patch to some branch.
http://git.kernel.org/?p=linux/kernel/git/airlied/drm-2.6.git;a=commit;h=d684076627a4561ea698bf7652a1a1baabdcdbdc
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.