From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-1?Q?Christian_K=F6nig?= Subject: Re: [PATCH 09/16] drm/radeon: make cp init on cayman more robust Date: Mon, 09 Jul 2012 17:11:40 +0200 Message-ID: <4FFAF4AC.2050602@vodafone.de> References: <1341830523-30320-1-git-send-email-deathsimple@vodafone.de> <1341830523-30320-10-git-send-email-deathsimple@vodafone.de> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: Received: from outgoing.email.vodafone.de (outgoing.email.vodafone.de [139.7.28.128]) by gabe.freedesktop.org (Postfix) with ESMTP id CB5AB9FD68 for ; Mon, 9 Jul 2012 08:11:44 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Jerome Glisse Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On 09.07.2012 16:43, Jerome Glisse wrote: > On Mon, Jul 9, 2012 at 6:41 AM, Christian K=F6nig wrote: >> It's not critical, but the current code isn't >> 100% correct. >> >> Signed-off-by: Christian K=F6nig >> --- >> 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 *r= dev) >> >> int cayman_cp_resume(struct radeon_device *rdev) >> { >> + static const int ridx[] =3D { >> + RADEON_RING_TYPE_GFX_INDEX, >> + CAYMAN_RING_TYPE_CP1_INDEX, >> + CAYMAN_RING_TYPE_CP2_INDEX >> + }; >> + static const unsigned cp_rb_cntl[] =3D { >> + CP_RB0_CNTL, >> + CP_RB1_CNTL, >> + CP_RB2_CNTL, >> + }; >> + static const unsigned cp_rb_rptr_addr[] =3D { >> + CP_RB0_RPTR_ADDR, >> + CP_RB1_RPTR_ADDR, >> + CP_RB2_RPTR_ADDR >> + }; >> + static const unsigned cp_rb_rptr_addr_hi[] =3D { >> + CP_RB0_RPTR_ADDR_HI, >> + CP_RB1_RPTR_ADDR_HI, >> + CP_RB2_RPTR_ADDR_HI >> + }; >> + static const unsigned cp_rb_base[] =3D { >> + 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 r= eset */ >> 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 =3D &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]; >> - rb_bufsz =3D drm_order(ring->ring_size / 8); >> - tmp =3D (drm_order(RADEON_GPU_PAGE_SIZE/8) << 8) | rb_bufsz; >> -#ifdef __BIG_ENDIAN >> - tmp |=3D 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 =3D 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 + RA= DEON_WB_CP_RPTR_OFFSET) & 0xFF); >> WREG32(SCRATCH_ADDR, ((rdev->wb.gpu_addr + RADEON_WB_SCRATCH_OF= FSET) >> 8) & 0xFFFFFFFF); >> + WREG32(SCRATCH_UMSK, 0xff); > This looks wrong you set the mask unconditionaly even if writeback is dis= abled. 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 |=3D RB_NO_UPDATE; >> - WREG32(SCRATCH_UMSK, 0); >> - } >> - >> - mdelay(1); >> - WREG32(CP_RB0_CNTL, tmp); >> - >> - WREG32(CP_RB0_BASE, ring->gpu_addr >> 8); >> - >> - ring->rptr =3D RREG32(CP_RB0_RPTR); >> + for (i =3D 0; i < 3; ++i) { >> + uint32_t rb_cntl; >> + uint64_t addr; >> >> - /* ring1 - compute only */ >> - /* Set ring buffer size */ >> - ring =3D &rdev->ring[CAYMAN_RING_TYPE_CP1_INDEX]; >> - rb_bufsz =3D drm_order(ring->ring_size / 8); >> - tmp =3D (drm_order(RADEON_GPU_PAGE_SIZE/8) << 8) | rb_bufsz; >> + /* Set ring buffer size */ >> + ring =3D &rdev->ring[ridx[i]]; >> + rb_cntl =3D drm_order(ring->ring_size / 8); >> + rb_cntl |=3D drm_order(RADEON_GPU_PAGE_SIZE/8) << 8; >> #ifdef __BIG_ENDIAN >> - tmp |=3D BUF_SWAP_32BIT; >> + rb_cntl |=3D 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 =3D 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 + RA= DEON_WB_CP1_RPTR_OFFSET) & 0xFF); >> - >> - mdelay(1); >> - WREG32(CP_RB1_CNTL, tmp); >> - >> - WREG32(CP_RB1_BASE, ring->gpu_addr >> 8); >> - >> - ring->rptr =3D RREG32(CP_RB1_RPTR); >> - >> - /* ring2 - compute only */ >> - /* Set ring buffer size */ >> - ring =3D &rdev->ring[CAYMAN_RING_TYPE_CP2_INDEX]; >> - rb_bufsz =3D drm_order(ring->ring_size / 8); >> - tmp =3D (drm_order(RADEON_GPU_PAGE_SIZE/8) << 8) | rb_bufsz; >> -#ifdef __BIG_ENDIAN >> - tmp |=3D 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 =3D 0; >> - WREG32(CP_RB2_WPTR, ring->wptr); >> + /* set the wb address wether it's enabled or not */ >> + addr =3D 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 + RA= DEON_WB_CP2_RPTR_OFFSET) & 0xFF); >> + /* set the rb base addr, this causes an internal reset of ALL ri= ngs */ >> + for (i =3D 0; i < 3; ++i) { >> + ring =3D &rdev->ring[ridx[i]]; >> + WREG32(cp_rb_base[i], ring->gpu_addr >> 8); >> + } >> >> - mdelay(1); >> - WREG32(CP_RB2_CNTL, tmp); >> + for (i =3D 0; i < 3; ++i) { >> + /* Initialize the ring buffer's read and write pointers = */ >> + ring =3D &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 =3D ring->wptr =3D 0; >> + WREG32(ring->rptr_reg, ring->rptr); >> + WREG32(ring->wptr_reg, ring->wptr); >> >> - ring->rptr =3D 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@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel