From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Deucher Subject: Re: [PATCH 3/6] drm/radeon: Writeback endian fixes Date: Wed, 13 Jul 2011 10:42:29 -0400 Message-ID: References: <1310538497.4968.93.camel@pasglop> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-iw0-f177.google.com (mail-iw0-f177.google.com [209.85.214.177]) by gabe.freedesktop.org (Postfix) with ESMTP id E04779F655 for ; Wed, 13 Jul 2011 07:42:29 -0700 (PDT) Received: by iwn35 with SMTP id 35so6565829iwn.36 for ; Wed, 13 Jul 2011 07:42:29 -0700 (PDT) In-Reply-To: <1310538497.4968.93.camel@pasglop> 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: Benjamin Herrenschmidt Cc: xorg-driver-ati@lists.x.org, dri-devel@lists.freedesktop.org, Cedric Cano List-Id: dri-devel@lists.freedesktop.org On Wed, Jul 13, 2011 at 2:28 AM, Benjamin Herrenschmidt wrote: > The writeback ring pointer and IH ring pointer are read using le32_to_cpu > so we do not want the chip to byteswap them on big-endian. > > We still want to byteswap the ring itself and the IBs, so we don't touch > that but we remove setting of the byteswap bits in CP_RB_RPTR_ADDR and > IH_CNTL. > > In general, for things like that where we control all the accessors easil= y, > we are better off doing the swap in SW rather than HW. Paradoxally, it do= es > keep the code closer to x86 and avoid using poorly tested HW features. > > I also changed the use of RADEON_ to R600_ in a couple of cases to be more > consistent with the surrounding code. > > Signed-off-by: Benjamin Herrenschmidt evergreen.c and ni.c will need similar fixes. Reviewed-by: Alex Deucher > --- > > (resent adding dri-devel to the CC list to hit patchwork) > > =A0drivers/gpu/drm/radeon/evergreen.c | =A0 =A03 --- > =A0drivers/gpu/drm/radeon/r600.c =A0 =A0 =A0| =A0 =A07 ------- > =A0drivers/gpu/drm/radeon/r600_cp.c =A0 | =A0 23 +++++++++-------------- > =A03 files changed, 9 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/= evergreen.c > index e8a5ffb..23cf089 100644 > --- a/drivers/gpu/drm/radeon/evergreen.c > +++ b/drivers/gpu/drm/radeon/evergreen.c > @@ -1359,9 +1359,6 @@ int evergreen_cp_resume(struct radeon_device *rdev) > > =A0 =A0 =A0 =A0/* set the wb address wether it's enabled or not */ > =A0 =A0 =A0 =A0WREG32(CP_RB_RPTR_ADDR, > -#ifdef __BIG_ENDIAN > - =A0 =A0 =A0 =A0 =A0 =A0 =A0RB_RPTR_SWAP(2) | > -#endif > =A0 =A0 =A0 =A0 =A0 =A0 =A0 ((rdev->wb.gpu_addr + RADEON_WB_CP_RPTR_OFFSE= T) & 0xFFFFFFFC)); > =A0 =A0 =A0 =A0WREG32(CP_RB_RPTR_ADDR_HI, upper_32_bits(rdev->wb.gpu_addr= + RADEON_WB_CP_RPTR_OFFSET) & 0xFF); > =A0 =A0 =A0 =A0WREG32(SCRATCH_ADDR, ((rdev->wb.gpu_addr + RADEON_WB_SCRAT= CH_OFFSET) >> 8) & 0xFFFFFFFF); > diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c > index f79d2cc..3c86b15 100644 > --- a/drivers/gpu/drm/radeon/r600.c > +++ b/drivers/gpu/drm/radeon/r600.c > @@ -2212,9 +2212,6 @@ int r600_cp_resume(struct radeon_device *rdev) > > =A0 =A0 =A0 =A0/* set the wb address whether it's enabled or not */ > =A0 =A0 =A0 =A0WREG32(CP_RB_RPTR_ADDR, > -#ifdef __BIG_ENDIAN > - =A0 =A0 =A0 =A0 =A0 =A0 =A0RB_RPTR_SWAP(2) | > -#endif > =A0 =A0 =A0 =A0 =A0 =A0 =A0 ((rdev->wb.gpu_addr + RADEON_WB_CP_RPTR_OFFSE= T) & 0xFFFFFFFC)); > =A0 =A0 =A0 =A0WREG32(CP_RB_RPTR_ADDR_HI, upper_32_bits(rdev->wb.gpu_addr= + RADEON_WB_CP_RPTR_OFFSET) & 0xFF); > =A0 =A0 =A0 =A0WREG32(SCRATCH_ADDR, ((rdev->wb.gpu_addr + RADEON_WB_SCRAT= CH_OFFSET) >> 8) & 0xFFFFFFFF); > @@ -2993,10 +2990,6 @@ int r600_irq_init(struct radeon_device *rdev) > =A0 =A0 =A0 =A0/* RPTR_REARM only works if msi's are enabled */ > =A0 =A0 =A0 =A0if (rdev->msi_enabled) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ih_cntl |=3D RPTR_REARM; > - > -#ifdef __BIG_ENDIAN > - =A0 =A0 =A0 ih_cntl |=3D IH_MC_SWAP(IH_MC_SWAP_32BIT); > -#endif > =A0 =A0 =A0 =A0WREG32(IH_CNTL, ih_cntl); > > =A0 =A0 =A0 =A0/* force the active interrupt state to all disabled */ > diff --git a/drivers/gpu/drm/radeon/r600_cp.c b/drivers/gpu/drm/radeon/r6= 00_cp.c > index c3ab959..45fd592 100644 > --- a/drivers/gpu/drm/radeon/r600_cp.c > +++ b/drivers/gpu/drm/radeon/r600_cp.c > @@ -1802,8 +1802,8 @@ static void r600_cp_init_ring_buffer(struct drm_dev= ice *dev, > =A0 =A0 =A0 =A0/* Set ring buffer size */ > =A0#ifdef __BIG_ENDIAN > =A0 =A0 =A0 =A0RADEON_WRITE(R600_CP_RB_CNTL, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0RADEON_BUF_SWAP_32BIT | > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0RADEON_RB_NO_UPDATE | > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0R600_BUF_SWAP_32BIT | > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0R600_RB_NO_UPDATE | > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (dev_priv->ring.rptr_update_l2qw = << 8) | > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_priv->ring.size_l2qw); > =A0#else > @@ -1820,15 +1820,15 @@ static void r600_cp_init_ring_buffer(struct drm_d= evice *dev, > > =A0#ifdef __BIG_ENDIAN > =A0 =A0 =A0 =A0RADEON_WRITE(R600_CP_RB_CNTL, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0RADEON_BUF_SWAP_32BIT | > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0RADEON_RB_NO_UPDATE | > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0RADEON_RB_RPTR_WR_ENA | > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0R600_BUF_SWAP_32BIT | > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0R600_RB_NO_UPDATE | > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0R600_RB_RPTR_WR_ENA | > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (dev_priv->ring.rptr_update_l2qw = << 8) | > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_priv->ring.size_l2qw); > =A0#else > =A0 =A0 =A0 =A0RADEON_WRITE(R600_CP_RB_CNTL, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0RADEON_RB_NO_UPDATE | > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0RADEON_RB_RPTR_WR_ENA | > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0R600_RB_NO_UPDATE | > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0R600_RB_RPTR_WR_ENA | > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (dev_priv->ring.rptr_update_l2qw = << 8) | > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_priv->ring.size_l2qw); > =A0#endif > @@ -1851,13 +1851,8 @@ static void r600_cp_init_ring_buffer(struct drm_de= vice *dev, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0- ((unsigned long) dev->sg= ->virtual) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0+ dev_priv->gart_vm_start; > =A0 =A0 =A0 =A0} > - =A0 =A0 =A0 RADEON_WRITE(R600_CP_RB_RPTR_ADDR, > -#ifdef __BIG_ENDIAN > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(2 << 0) | > -#endif > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(rptr_addr & 0xfffffffc)); > - =A0 =A0 =A0 RADEON_WRITE(R600_CP_RB_RPTR_ADDR_HI, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0upper_32_bits(rptr_addr)); > + =A0 =A0 =A0 RADEON_WRITE(R600_CP_RB_RPTR_ADDR, (rptr_addr & 0xfffffffc)= ); > + =A0 =A0 =A0 RADEON_WRITE(R600_CP_RB_RPTR_ADDR_HI, upper_32_bits(rptr_ad= dr)); > > =A0#ifdef __BIG_ENDIAN > =A0 =A0 =A0 =A0RADEON_WRITE(R600_CP_RB_CNTL, > > > >