Re: [PATCH 3/6] drm/radeon: Writeback endian fixes

[Alex Deucher <alexdeucher@gmail.com>]

On Wed, Jul 13, 2011 at 2:28 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> 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 easily,
> we are better off doing the swap in SW rather than HW. Paradoxally, it does
> 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 <benh@kernel.crashing.org>

evergreen.c and ni.c will need similar fixes.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>
> (resent adding dri-devel to the CC list to hit patchwork)
>
>  drivers/gpu/drm/radeon/evergreen.c |    3 ---
>  drivers/gpu/drm/radeon/r600.c      |    7 -------
>  drivers/gpu/drm/radeon/r600_cp.c   |   23 +++++++++--------------
>  3 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)
>
>        /* set the wb address wether it's enabled or not */
>        WREG32(CP_RB_RPTR_ADDR,
> -#ifdef __BIG_ENDIAN
> -              RB_RPTR_SWAP(2) |
> -#endif
>               ((rdev->wb.gpu_addr + RADEON_WB_CP_RPTR_OFFSET) & 0xFFFFFFFC));
>        WREG32(CP_RB_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);
> 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)
>
>        /* set the wb address whether it's enabled or not */
>        WREG32(CP_RB_RPTR_ADDR,
> -#ifdef __BIG_ENDIAN
> -              RB_RPTR_SWAP(2) |
> -#endif
>               ((rdev->wb.gpu_addr + RADEON_WB_CP_RPTR_OFFSET) & 0xFFFFFFFC));
>        WREG32(CP_RB_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);
> @@ -2993,10 +2990,6 @@ int r600_irq_init(struct radeon_device *rdev)
>        /* RPTR_REARM only works if msi's are enabled */
>        if (rdev->msi_enabled)
>                ih_cntl |= RPTR_REARM;
> -
> -#ifdef __BIG_ENDIAN
> -       ih_cntl |= IH_MC_SWAP(IH_MC_SWAP_32BIT);
> -#endif
>        WREG32(IH_CNTL, ih_cntl);
>
>        /* force the active interrupt state to all disabled */
> diff --git a/drivers/gpu/drm/radeon/r600_cp.c b/drivers/gpu/drm/radeon/r600_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_device *dev,
>        /* Set ring buffer size */
>  #ifdef __BIG_ENDIAN
>        RADEON_WRITE(R600_CP_RB_CNTL,
> -                    RADEON_BUF_SWAP_32BIT |
> -                    RADEON_RB_NO_UPDATE |
> +                    R600_BUF_SWAP_32BIT |
> +                    R600_RB_NO_UPDATE |
>                     (dev_priv->ring.rptr_update_l2qw << 8) |
>                     dev_priv->ring.size_l2qw);
>  #else
> @@ -1820,15 +1820,15 @@ static void r600_cp_init_ring_buffer(struct drm_device *dev,
>
>  #ifdef __BIG_ENDIAN
>        RADEON_WRITE(R600_CP_RB_CNTL,
> -                    RADEON_BUF_SWAP_32BIT |
> -                    RADEON_RB_NO_UPDATE |
> -                    RADEON_RB_RPTR_WR_ENA |
> +                    R600_BUF_SWAP_32BIT |
> +                    R600_RB_NO_UPDATE |
> +                    R600_RB_RPTR_WR_ENA |
>                     (dev_priv->ring.rptr_update_l2qw << 8) |
>                     dev_priv->ring.size_l2qw);
>  #else
>        RADEON_WRITE(R600_CP_RB_CNTL,
> -                    RADEON_RB_NO_UPDATE |
> -                    RADEON_RB_RPTR_WR_ENA |
> +                    R600_RB_NO_UPDATE |
> +                    R600_RB_RPTR_WR_ENA |
>                     (dev_priv->ring.rptr_update_l2qw << 8) |
>                     dev_priv->ring.size_l2qw);
>  #endif
> @@ -1851,13 +1851,8 @@ static void r600_cp_init_ring_buffer(struct drm_device *dev,
>                        - ((unsigned long) dev->sg->virtual)
>                        + dev_priv->gart_vm_start;
>        }
> -       RADEON_WRITE(R600_CP_RB_RPTR_ADDR,
> -#ifdef __BIG_ENDIAN
> -                    (2 << 0) |
> -#endif
> -                    (rptr_addr & 0xfffffffc));
> -       RADEON_WRITE(R600_CP_RB_RPTR_ADDR_HI,
> -                    upper_32_bits(rptr_addr));
> +       RADEON_WRITE(R600_CP_RB_RPTR_ADDR, (rptr_addr & 0xfffffffc));
> +       RADEON_WRITE(R600_CP_RB_RPTR_ADDR_HI, upper_32_bits(rptr_addr));
>
>  #ifdef __BIG_ENDIAN
>        RADEON_WRITE(R600_CP_RB_CNTL,
>
>
>
>