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. > - 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