Re: [PATCH 09/16] drm/radeon: make cp init on cayman more robust

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09.07.2012 16:43, Jerome Glisse wrote:
On Mon, Jul 9, 2012 at 6:41 AM, Christian König <deathsimple@xxxxxxxxxxx> wrote:
It's not critical, but the current code isn't
100% correct.

Signed-off-by: Christian König <deathsimple@xxxxxxxxxxx>
---
  drivers/gpu/drm/radeon/ni.c |  133 ++++++++++++++++++-------------------------
  1 file changed, 56 insertions(+), 77 deletions(-)

diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
index 32a6082..8b1df33 100644
--- a/drivers/gpu/drm/radeon/ni.c
+++ b/drivers/gpu/drm/radeon/ni.c
@@ -987,10 +987,33 @@ static void cayman_cp_fini(struct radeon_device *rdev)

  int cayman_cp_resume(struct radeon_device *rdev)
  {
+       static const int ridx[] = {
+               RADEON_RING_TYPE_GFX_INDEX,
+               CAYMAN_RING_TYPE_CP1_INDEX,
+               CAYMAN_RING_TYPE_CP2_INDEX
+       };
+       static const unsigned cp_rb_cntl[] = {
+               CP_RB0_CNTL,
+               CP_RB1_CNTL,
+               CP_RB2_CNTL,
+       };
+       static const unsigned cp_rb_rptr_addr[] = {
+               CP_RB0_RPTR_ADDR,
+               CP_RB1_RPTR_ADDR,
+               CP_RB2_RPTR_ADDR
+       };
+       static const unsigned cp_rb_rptr_addr_hi[] = {
+               CP_RB0_RPTR_ADDR_HI,
+               CP_RB1_RPTR_ADDR_HI,
+               CP_RB2_RPTR_ADDR_HI
+       };
+       static const unsigned cp_rb_base[] = {
+               CP_RB0_BASE,
+               CP_RB1_BASE,
+               CP_RB2_BASE
+       };
         struct radeon_ring *ring;
-       u32 tmp;
-       u32 rb_bufsz;
-       int r;
+       int i, r;

         /* Reset cp; if cp is reset, then PA, SH, VGT also need to be reset */
         WREG32(GRBM_SOFT_RESET, (SOFT_RESET_CP |
@@ -1012,91 +1035,47 @@ int cayman_cp_resume(struct radeon_device *rdev)

         WREG32(CP_DEBUG, (1 << 27));

-       /* ring 0 - compute and gfx */
-       /* Set ring buffer size */
-       ring = &rdev->ring[RADEON_RING_TYPE_GFX_INDEX];
-       rb_bufsz = drm_order(ring->ring_size / 8);
-       tmp = (drm_order(RADEON_GPU_PAGE_SIZE/8) << 8) | rb_bufsz;
-#ifdef __BIG_ENDIAN
-       tmp |= BUF_SWAP_32BIT;
-#endif
-       WREG32(CP_RB0_CNTL, tmp);
-
-       /* Initialize the ring buffer's read and write pointers */
-       WREG32(CP_RB0_CNTL, tmp | RB_RPTR_WR_ENA);
-       ring->wptr = 0;
-       WREG32(CP_RB0_WPTR, ring->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);
-       WREG32(CP_RB0_RPTR_ADDR_HI, upper_32_bits(rdev->wb.gpu_addr + RADEON_WB_CP_RPTR_OFFSET) & 0xFF);
         WREG32(SCRATCH_ADDR, ((rdev->wb.gpu_addr + RADEON_WB_SCRATCH_OFFSET) >> 8) & 0xFFFFFFFF);
+       WREG32(SCRATCH_UMSK, 0xff);
This looks wrong you set the mask unconditionaly even if writeback is disabled.
writeback is always enabled for NI and APUs, but the register docs say that this feature isn't validated for NI and shouldn't be used so I'm not sure if enabling it is the right thing to do.

Anyway, it doesn't matter at all since we don't use the scratch register writeback anymore and use EOP instead.

Christian.




-       if (rdev->wb.enabled)
-               WREG32(SCRATCH_UMSK, 0xff);
-       else {
-               tmp |= RB_NO_UPDATE;
-               WREG32(SCRATCH_UMSK, 0);
-       }
-
-       mdelay(1);
-       WREG32(CP_RB0_CNTL, tmp);
-
-       WREG32(CP_RB0_BASE, ring->gpu_addr >> 8);
-
-       ring->rptr = RREG32(CP_RB0_RPTR);
+       for (i = 0; i < 3; ++i) {
+               uint32_t rb_cntl;
+               uint64_t addr;

-       /* ring1  - compute only */
-       /* Set ring buffer size */
-       ring = &rdev->ring[CAYMAN_RING_TYPE_CP1_INDEX];
-       rb_bufsz = drm_order(ring->ring_size / 8);
-       tmp = (drm_order(RADEON_GPU_PAGE_SIZE/8) << 8) | rb_bufsz;
+               /* Set ring buffer size */
+               ring = &rdev->ring[ridx[i]];
+               rb_cntl = drm_order(ring->ring_size / 8);
+               rb_cntl |= drm_order(RADEON_GPU_PAGE_SIZE/8) << 8;
  #ifdef __BIG_ENDIAN
-       tmp |= BUF_SWAP_32BIT;
+               rb_cntl |= BUF_SWAP_32BIT;
  #endif
-       WREG32(CP_RB1_CNTL, tmp);
+               WREG32(cp_rb_cntl[i], rb_cntl);

-       /* Initialize the ring buffer's read and write pointers */
-       WREG32(CP_RB1_CNTL, tmp | RB_RPTR_WR_ENA);
-       ring->wptr = 0;
-       WREG32(CP_RB1_WPTR, ring->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);
-       WREG32(CP_RB1_RPTR_ADDR_HI, upper_32_bits(rdev->wb.gpu_addr + RADEON_WB_CP1_RPTR_OFFSET) & 0xFF);
-
-       mdelay(1);
-       WREG32(CP_RB1_CNTL, tmp);
-
-       WREG32(CP_RB1_BASE, ring->gpu_addr >> 8);
-
-       ring->rptr = RREG32(CP_RB1_RPTR);
-
-       /* ring2 - compute only */
-       /* Set ring buffer size */
-       ring = &rdev->ring[CAYMAN_RING_TYPE_CP2_INDEX];
-       rb_bufsz = drm_order(ring->ring_size / 8);
-       tmp = (drm_order(RADEON_GPU_PAGE_SIZE/8) << 8) | rb_bufsz;
-#ifdef __BIG_ENDIAN
-       tmp |= BUF_SWAP_32BIT;
-#endif
-       WREG32(CP_RB2_CNTL, tmp);
-
-       /* Initialize the ring buffer's read and write pointers */
-       WREG32(CP_RB2_CNTL, tmp | RB_RPTR_WR_ENA);
-       ring->wptr = 0;
-       WREG32(CP_RB2_WPTR, ring->wptr);
+               /* set the wb address wether it's enabled or not */
+               addr = rdev->wb.gpu_addr + RADEON_WB_CP_RPTR_OFFSET;
+               WREG32(cp_rb_rptr_addr[i], addr & 0xFFFFFFFC);
+               WREG32(cp_rb_rptr_addr_hi[i], upper_32_bits(addr) & 0xFF);
+       }

-       /* 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);
-       WREG32(CP_RB2_RPTR_ADDR_HI, upper_32_bits(rdev->wb.gpu_addr + RADEON_WB_CP2_RPTR_OFFSET) & 0xFF);
+       /* set the rb base addr, this causes an internal reset of ALL rings */
+       for (i = 0; i < 3; ++i) {
+               ring = &rdev->ring[ridx[i]];
+               WREG32(cp_rb_base[i], ring->gpu_addr >> 8);
+       }

-       mdelay(1);
-       WREG32(CP_RB2_CNTL, tmp);
+       for (i = 0; i < 3; ++i) {
+               /* Initialize the ring buffer's read and write pointers */
+               ring = &rdev->ring[ridx[i]];
+               WREG32_P(cp_rb_cntl[i], RB_RPTR_WR_ENA, ~RB_RPTR_WR_ENA);

-       WREG32(CP_RB2_BASE, ring->gpu_addr >> 8);
+               ring->rptr = ring->wptr = 0;
+               WREG32(ring->rptr_reg, ring->rptr);
+               WREG32(ring->wptr_reg, ring->wptr);

-       ring->rptr = RREG32(CP_RB2_RPTR);
+               mdelay(1);
+               WREG32_P(cp_rb_cntl[i], 0, ~RB_RPTR_WR_ENA);
+       }

         /* start the rings */
         cayman_cp_start(rdev);
--
1.7.9.5

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux