[PATCH 1/3] drm/radeon: drop drivers copy of the rptr

Alex Deucher alexdeucher at gmail.com
Tue Feb 18 08:25:43 PST 2014


On Tue, Feb 18, 2014 at 9:05 AM, Christian König
<deathsimple at vodafone.de> wrote:
> From: Christian König <christian.koenig at amd.com>
>
> In all cases where it really matters we are using the read functions anyway.
>
> Signed-off-by: Christian König <christian.koenig at amd.com>

For the series:

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

> ---
>  drivers/gpu/drm/radeon/cik.c           |  7 ++-----
>  drivers/gpu/drm/radeon/cik_sdma.c      |  4 +---
>  drivers/gpu/drm/radeon/evergreen.c     |  4 +---
>  drivers/gpu/drm/radeon/evergreen_dma.c |  2 +-
>  drivers/gpu/drm/radeon/ni.c            |  6 +++---
>  drivers/gpu/drm/radeon/ni_dma.c        |  4 +---
>  drivers/gpu/drm/radeon/r100.c          |  3 +--
>  drivers/gpu/drm/radeon/r600.c          |  4 +---
>  drivers/gpu/drm/radeon/r600_dma.c      |  4 +---
>  drivers/gpu/drm/radeon/radeon.h        |  4 ++--
>  drivers/gpu/drm/radeon/radeon_ring.c   | 27 +++++++++++++--------------
>  drivers/gpu/drm/radeon/si.c            |  8 +-------
>  drivers/gpu/drm/radeon/si_dma.c        |  2 +-
>  drivers/gpu/drm/radeon/uvd_v1_0.c      |  2 +-
>  drivers/gpu/drm/radeon/vce_v1_0.c      |  4 ++--
>  15 files changed, 32 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
> index 2b31c32..835dcfb 100644
> --- a/drivers/gpu/drm/radeon/cik.c
> +++ b/drivers/gpu/drm/radeon/cik.c
> @@ -4031,8 +4031,6 @@ static int cik_cp_gfx_resume(struct radeon_device *rdev)
>         WREG32(CP_RB0_BASE, rb_addr);
>         WREG32(CP_RB0_BASE_HI, upper_32_bits(rb_addr));
>
> -       ring->rptr = RREG32(CP_RB0_RPTR);
> -
>         /* start the ring */
>         cik_cp_gfx_start(rdev);
>         rdev->ring[RADEON_RING_TYPE_GFX_INDEX].ready = true;
> @@ -4587,8 +4585,7 @@ static int cik_cp_compute_resume(struct radeon_device *rdev)
>                 rdev->ring[idx].wptr = 0;
>                 mqd->queue_state.cp_hqd_pq_wptr = rdev->ring[idx].wptr;
>                 WREG32(CP_HQD_PQ_WPTR, mqd->queue_state.cp_hqd_pq_wptr);
> -               rdev->ring[idx].rptr = RREG32(CP_HQD_PQ_RPTR);
> -               mqd->queue_state.cp_hqd_pq_rptr = rdev->ring[idx].rptr;
> +               mqd->queue_state.cp_hqd_pq_rptr = RREG32(CP_HQD_PQ_RPTR);
>
>                 /* set the vmid for the queue */
>                 mqd->queue_state.cp_hqd_vmid = 0;
> @@ -5118,7 +5115,7 @@ bool cik_gfx_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
>         if (!(reset_mask & (RADEON_RESET_GFX |
>                             RADEON_RESET_COMPUTE |
>                             RADEON_RESET_CP))) {
> -               radeon_ring_lockup_update(ring);
> +               radeon_ring_lockup_update(rdev, ring);
>                 return false;
>         }
>         /* force CP activities */
> diff --git a/drivers/gpu/drm/radeon/cik_sdma.c b/drivers/gpu/drm/radeon/cik_sdma.c
> index 1ecb3f1..e474760 100644
> --- a/drivers/gpu/drm/radeon/cik_sdma.c
> +++ b/drivers/gpu/drm/radeon/cik_sdma.c
> @@ -362,8 +362,6 @@ static int cik_sdma_gfx_resume(struct radeon_device *rdev)
>                 ring->wptr = 0;
>                 WREG32(SDMA0_GFX_RB_WPTR + reg_offset, ring->wptr << 2);
>
> -               ring->rptr = RREG32(SDMA0_GFX_RB_RPTR + reg_offset) >> 2;
> -
>                 /* enable DMA RB */
>                 WREG32(SDMA0_GFX_RB_CNTL + reg_offset, rb_cntl | SDMA_RB_ENABLE);
>
> @@ -713,7 +711,7 @@ bool cik_sdma_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
>                 mask = RADEON_RESET_DMA1;
>
>         if (!(reset_mask & mask)) {
> -               radeon_ring_lockup_update(ring);
> +               radeon_ring_lockup_update(rdev, ring);
>                 return false;
>         }
>         /* force ring activities */
> diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
> index f2b9e21..d9156be 100644
> --- a/drivers/gpu/drm/radeon/evergreen.c
> +++ b/drivers/gpu/drm/radeon/evergreen.c
> @@ -2990,8 +2990,6 @@ static int evergreen_cp_resume(struct radeon_device *rdev)
>         WREG32(CP_RB_BASE, ring->gpu_addr >> 8);
>         WREG32(CP_DEBUG, (1 << 27) | (1 << 28));
>
> -       ring->rptr = RREG32(CP_RB_RPTR);
> -
>         evergreen_cp_start(rdev);
>         ring->ready = true;
>         r = radeon_ring_test(rdev, RADEON_RING_TYPE_GFX_INDEX, ring);
> @@ -3952,7 +3950,7 @@ bool evergreen_gfx_is_lockup(struct radeon_device *rdev, struct radeon_ring *rin
>         if (!(reset_mask & (RADEON_RESET_GFX |
>                             RADEON_RESET_COMPUTE |
>                             RADEON_RESET_CP))) {
> -               radeon_ring_lockup_update(ring);
> +               radeon_ring_lockup_update(rdev, ring);
>                 return false;
>         }
>         /* force CP activities */
> diff --git a/drivers/gpu/drm/radeon/evergreen_dma.c b/drivers/gpu/drm/radeon/evergreen_dma.c
> index a37b544..d448961 100644
> --- a/drivers/gpu/drm/radeon/evergreen_dma.c
> +++ b/drivers/gpu/drm/radeon/evergreen_dma.c
> @@ -174,7 +174,7 @@ bool evergreen_dma_is_lockup(struct radeon_device *rdev, struct radeon_ring *rin
>         u32 reset_mask = evergreen_gpu_check_soft_reset(rdev);
>
>         if (!(reset_mask & RADEON_RESET_DMA)) {
> -               radeon_ring_lockup_update(ring);
> +               radeon_ring_lockup_update(rdev, ring);
>                 return false;
>         }
>         /* force ring activities */
> diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
> index ea932ac..7601532 100644
> --- a/drivers/gpu/drm/radeon/ni.c
> +++ b/drivers/gpu/drm/radeon/ni.c
> @@ -1642,8 +1642,8 @@ static int cayman_cp_resume(struct radeon_device *rdev)
>                 ring = &rdev->ring[ridx[i]];
>                 WREG32_P(cp_rb_cntl[i], RB_RPTR_WR_ENA, ~RB_RPTR_WR_ENA);
>
> -               ring->rptr = ring->wptr = 0;
> -               WREG32(cp_rb_rptr[i], ring->rptr);
> +               ring->wptr = 0;
> +               WREG32(cp_rb_rptr[i], 0);
>                 WREG32(cp_rb_wptr[i], ring->wptr);
>
>                 mdelay(1);
> @@ -1917,7 +1917,7 @@ bool cayman_gfx_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
>         if (!(reset_mask & (RADEON_RESET_GFX |
>                             RADEON_RESET_COMPUTE |
>                             RADEON_RESET_CP))) {
> -               radeon_ring_lockup_update(ring);
> +               radeon_ring_lockup_update(rdev, ring);
>                 return false;
>         }
>         /* force CP activities */
> diff --git a/drivers/gpu/drm/radeon/ni_dma.c b/drivers/gpu/drm/radeon/ni_dma.c
> index 7cf96b1..95e533c 100644
> --- a/drivers/gpu/drm/radeon/ni_dma.c
> +++ b/drivers/gpu/drm/radeon/ni_dma.c
> @@ -248,8 +248,6 @@ int cayman_dma_resume(struct radeon_device *rdev)
>                 ring->wptr = 0;
>                 WREG32(DMA_RB_WPTR + reg_offset, ring->wptr << 2);
>
> -               ring->rptr = RREG32(DMA_RB_RPTR + reg_offset) >> 2;
> -
>                 WREG32(DMA_RB_CNTL + reg_offset, rb_cntl | DMA_RB_ENABLE);
>
>                 ring->ready = true;
> @@ -302,7 +300,7 @@ bool cayman_dma_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
>                 mask = RADEON_RESET_DMA1;
>
>         if (!(reset_mask & mask)) {
> -               radeon_ring_lockup_update(ring);
> +               radeon_ring_lockup_update(rdev, ring);
>                 return false;
>         }
>         /* force ring activities */
> diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
> index ef024ce..3a74381 100644
> --- a/drivers/gpu/drm/radeon/r100.c
> +++ b/drivers/gpu/drm/radeon/r100.c
> @@ -1193,7 +1193,6 @@ int r100_cp_init(struct radeon_device *rdev, unsigned ring_size)
>
>         WREG32(RADEON_CP_RB_CNTL, tmp);
>         udelay(10);
> -       ring->rptr = RREG32(RADEON_CP_RB_RPTR);
>         /* Set cp mode to bus mastering & enable cp*/
>         WREG32(RADEON_CP_CSQ_MODE,
>                REG_SET(RADEON_INDIRECT2_START, indirect2_start) |
> @@ -2523,7 +2522,7 @@ bool r100_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
>
>         rbbm_status = RREG32(R_000E40_RBBM_STATUS);
>         if (!G_000E40_GUI_ACTIVE(rbbm_status)) {
> -               radeon_ring_lockup_update(ring);
> +               radeon_ring_lockup_update(rdev, ring);
>                 return false;
>         }
>         /* force CP activities */
> diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
> index cdbc417..085e025 100644
> --- a/drivers/gpu/drm/radeon/r600.c
> +++ b/drivers/gpu/drm/radeon/r600.c
> @@ -1748,7 +1748,7 @@ bool r600_gfx_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
>         if (!(reset_mask & (RADEON_RESET_GFX |
>                             RADEON_RESET_COMPUTE |
>                             RADEON_RESET_CP))) {
> -               radeon_ring_lockup_update(ring);
> +               radeon_ring_lockup_update(rdev, ring);
>                 return false;
>         }
>         /* force CP activities */
> @@ -2604,8 +2604,6 @@ int r600_cp_resume(struct radeon_device *rdev)
>         WREG32(CP_RB_BASE, ring->gpu_addr >> 8);
>         WREG32(CP_DEBUG, (1 << 27) | (1 << 28));
>
> -       ring->rptr = RREG32(CP_RB_RPTR);
> -
>         r600_cp_start(rdev);
>         ring->ready = true;
>         r = radeon_ring_test(rdev, RADEON_RING_TYPE_GFX_INDEX, ring);
> diff --git a/drivers/gpu/drm/radeon/r600_dma.c b/drivers/gpu/drm/radeon/r600_dma.c
> index b2d4c91..6944e19 100644
> --- a/drivers/gpu/drm/radeon/r600_dma.c
> +++ b/drivers/gpu/drm/radeon/r600_dma.c
> @@ -176,8 +176,6 @@ int r600_dma_resume(struct radeon_device *rdev)
>         ring->wptr = 0;
>         WREG32(DMA_RB_WPTR, ring->wptr << 2);
>
> -       ring->rptr = RREG32(DMA_RB_RPTR) >> 2;
> -
>         WREG32(DMA_RB_CNTL, rb_cntl | DMA_RB_ENABLE);
>
>         ring->ready = true;
> @@ -221,7 +219,7 @@ bool r600_dma_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
>         u32 reset_mask = r600_gpu_check_soft_reset(rdev);
>
>         if (!(reset_mask & RADEON_RESET_DMA)) {
> -               radeon_ring_lockup_update(ring);
> +               radeon_ring_lockup_update(rdev, ring);
>                 return false;
>         }
>         /* force ring activities */
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 63777e4..425491f 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -791,7 +791,6 @@ struct radeon_ib {
>  struct radeon_ring {
>         struct radeon_bo        *ring_obj;
>         volatile uint32_t       *ring;
> -       unsigned                rptr;
>         unsigned                rptr_offs;
>         unsigned                rptr_save_reg;
>         u64                     next_rptr_gpu_addr;
> @@ -956,7 +955,8 @@ void radeon_ring_undo(struct radeon_ring *ring);
>  void radeon_ring_unlock_undo(struct radeon_device *rdev, struct radeon_ring *cp);
>  int radeon_ring_test(struct radeon_device *rdev, struct radeon_ring *cp);
>  void radeon_ring_force_activity(struct radeon_device *rdev, struct radeon_ring *ring);
> -void radeon_ring_lockup_update(struct radeon_ring *ring);
> +void radeon_ring_lockup_update(struct radeon_device *rdev,
> +                              struct radeon_ring *ring);
>  bool radeon_ring_test_lockup(struct radeon_device *rdev, struct radeon_ring *ring);
>  unsigned radeon_ring_backup(struct radeon_device *rdev, struct radeon_ring *ring,
>                             uint32_t **data);
> diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
> index 149cd63..42b712e 100644
> --- a/drivers/gpu/drm/radeon/radeon_ring.c
> +++ b/drivers/gpu/drm/radeon/radeon_ring.c
> @@ -342,9 +342,10 @@ bool radeon_ring_supports_scratch_reg(struct radeon_device *rdev,
>   */
>  void radeon_ring_free_size(struct radeon_device *rdev, struct radeon_ring *ring)
>  {
> -       ring->rptr = radeon_ring_get_rptr(rdev, ring);
> +       uint32_t rptr = radeon_ring_get_rptr(rdev, ring);
> +
>         /* This works because ring_size is a power of 2 */
> -       ring->ring_free_dw = (ring->rptr + (ring->ring_size / 4));
> +       ring->ring_free_dw = rptr + (ring->ring_size / 4);
>         ring->ring_free_dw -= ring->wptr;
>         ring->ring_free_dw &= ring->ptr_mask;
>         if (!ring->ring_free_dw) {
> @@ -376,7 +377,7 @@ int radeon_ring_alloc(struct radeon_device *rdev, struct radeon_ring *ring, unsi
>                 /* This is an empty ring update lockup info to avoid
>                  * false positive.
>                  */
> -               radeon_ring_lockup_update(ring);
> +               radeon_ring_lockup_update(rdev, ring);
>         }
>         ndw = (ndw + ring->align_mask) & ~ring->align_mask;
>         while (ndw > (ring->ring_free_dw - 1)) {
> @@ -490,8 +491,7 @@ void radeon_ring_force_activity(struct radeon_device *rdev, struct radeon_ring *
>  {
>         int r;
>
> -       radeon_ring_free_size(rdev, ring);
> -       if (ring->rptr == ring->wptr) {
> +       if (radeon_ring_get_rptr(rdev, ring) == ring->wptr) {
>                 r = radeon_ring_alloc(rdev, ring, 1);
>                 if (!r) {
>                         radeon_ring_write(ring, ring->nop);
> @@ -507,9 +507,10 @@ void radeon_ring_force_activity(struct radeon_device *rdev, struct radeon_ring *
>   *
>   * Update the last rptr value and timestamp (all asics).
>   */
> -void radeon_ring_lockup_update(struct radeon_ring *ring)
> +void radeon_ring_lockup_update(struct radeon_device *rdev,
> +                              struct radeon_ring *ring)
>  {
> -       ring->last_rptr = ring->rptr;
> +       ring->last_rptr = radeon_ring_get_rptr(rdev, ring);
>         ring->last_activity = jiffies;
>  }
>
> @@ -535,18 +536,18 @@ void radeon_ring_lockup_update(struct radeon_ring *ring)
>   **/
>  bool radeon_ring_test_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
>  {
> +       uint32_t rptr = radeon_ring_get_rptr(rdev, ring);
>         unsigned long cjiffies, elapsed;
>
>         cjiffies = jiffies;
>         if (!time_after(cjiffies, ring->last_activity)) {
>                 /* likely a wrap around */
> -               radeon_ring_lockup_update(ring);
> +               radeon_ring_lockup_update(rdev, ring);
>                 return false;
>         }
> -       ring->rptr = radeon_ring_get_rptr(rdev, ring);
> -       if (ring->rptr != ring->last_rptr) {
> +       if (rptr != ring->last_rptr) {
>                 /* CP is still working no lockup */
> -               radeon_ring_lockup_update(ring);
> +               radeon_ring_lockup_update(rdev, ring);
>                 return false;
>         }
>         elapsed = jiffies_to_msecs(cjiffies - ring->last_activity);
> @@ -709,7 +710,7 @@ int radeon_ring_init(struct radeon_device *rdev, struct radeon_ring *ring, unsig
>         if (radeon_debugfs_ring_init(rdev, ring)) {
>                 DRM_ERROR("Failed to register debugfs file for rings !\n");
>         }
> -       radeon_ring_lockup_update(ring);
> +       radeon_ring_lockup_update(rdev, ring);
>         return 0;
>  }
>
> @@ -780,8 +781,6 @@ static int radeon_debugfs_ring_info(struct seq_file *m, void *data)
>
>         seq_printf(m, "driver's copy of the wptr: 0x%08x [%5d]\n",
>                    ring->wptr, ring->wptr);
> -       seq_printf(m, "driver's copy of the rptr: 0x%08x [%5d]\n",
> -                  ring->rptr, ring->rptr);
>         seq_printf(m, "last semaphore signal addr : 0x%016llx\n",
>                    ring->last_semaphore_signal_addr);
>         seq_printf(m, "last semaphore wait addr   : 0x%016llx\n",
> diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
> index 8357832..b406a48 100644
> --- a/drivers/gpu/drm/radeon/si.c
> +++ b/drivers/gpu/drm/radeon/si.c
> @@ -3434,8 +3434,6 @@ static int si_cp_resume(struct radeon_device *rdev)
>
>         WREG32(CP_RB0_BASE, ring->gpu_addr >> 8);
>
> -       ring->rptr = RREG32(CP_RB0_RPTR);
> -
>         /* ring1  - compute only */
>         /* Set ring buffer size */
>         ring = &rdev->ring[CAYMAN_RING_TYPE_CP1_INDEX];
> @@ -3460,8 +3458,6 @@ static int si_cp_resume(struct radeon_device *rdev)
>
>         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];
> @@ -3486,8 +3482,6 @@ static int si_cp_resume(struct radeon_device *rdev)
>
>         WREG32(CP_RB2_BASE, ring->gpu_addr >> 8);
>
> -       ring->rptr = RREG32(CP_RB2_RPTR);
> -
>         /* start the rings */
>         si_cp_start(rdev);
>         rdev->ring[RADEON_RING_TYPE_GFX_INDEX].ready = true;
> @@ -3872,7 +3866,7 @@ bool si_gfx_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
>         if (!(reset_mask & (RADEON_RESET_GFX |
>                             RADEON_RESET_COMPUTE |
>                             RADEON_RESET_CP))) {
> -               radeon_ring_lockup_update(ring);
> +               radeon_ring_lockup_update(rdev, ring);
>                 return false;
>         }
>         /* force CP activities */
> diff --git a/drivers/gpu/drm/radeon/si_dma.c b/drivers/gpu/drm/radeon/si_dma.c
> index 59be2cf..c75f533 100644
> --- a/drivers/gpu/drm/radeon/si_dma.c
> +++ b/drivers/gpu/drm/radeon/si_dma.c
> @@ -49,7 +49,7 @@ bool si_dma_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
>                 mask = RADEON_RESET_DMA1;
>
>         if (!(reset_mask & mask)) {
> -               radeon_ring_lockup_update(ring);
> +               radeon_ring_lockup_update(rdev, ring);
>                 return false;
>         }
>         /* force ring activities */
> diff --git a/drivers/gpu/drm/radeon/uvd_v1_0.c b/drivers/gpu/drm/radeon/uvd_v1_0.c
> index d4a68af..0a243f0 100644
> --- a/drivers/gpu/drm/radeon/uvd_v1_0.c
> +++ b/drivers/gpu/drm/radeon/uvd_v1_0.c
> @@ -262,7 +262,7 @@ int uvd_v1_0_start(struct radeon_device *rdev)
>         /* Initialize the ring buffer's read and write pointers */
>         WREG32(UVD_RBC_RB_RPTR, 0x0);
>
> -       ring->wptr = ring->rptr = RREG32(UVD_RBC_RB_RPTR);
> +       ring->wptr = RREG32(UVD_RBC_RB_RPTR);
>         WREG32(UVD_RBC_RB_WPTR, ring->wptr);
>
>         /* set the ring address */
> diff --git a/drivers/gpu/drm/radeon/vce_v1_0.c b/drivers/gpu/drm/radeon/vce_v1_0.c
> index e0c3534..b44d9c8 100644
> --- a/drivers/gpu/drm/radeon/vce_v1_0.c
> +++ b/drivers/gpu/drm/radeon/vce_v1_0.c
> @@ -98,14 +98,14 @@ int vce_v1_0_start(struct radeon_device *rdev)
>         WREG32_P(VCE_STATUS, 1, ~1);
>
>         ring = &rdev->ring[TN_RING_TYPE_VCE1_INDEX];
> -       WREG32(VCE_RB_RPTR, ring->rptr);
> +       WREG32(VCE_RB_RPTR, ring->wptr);
>         WREG32(VCE_RB_WPTR, ring->wptr);
>         WREG32(VCE_RB_BASE_LO, ring->gpu_addr);
>         WREG32(VCE_RB_BASE_HI, upper_32_bits(ring->gpu_addr));
>         WREG32(VCE_RB_SIZE, ring->ring_size / 4);
>
>         ring = &rdev->ring[TN_RING_TYPE_VCE2_INDEX];
> -       WREG32(VCE_RB_RPTR2, ring->rptr);
> +       WREG32(VCE_RB_RPTR2, ring->wptr);
>         WREG32(VCE_RB_WPTR2, ring->wptr);
>         WREG32(VCE_RB_BASE_LO2, ring->gpu_addr);
>         WREG32(VCE_RB_BASE_HI2, upper_32_bits(ring->gpu_addr));
> --
> 1.8.3.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel


More information about the dri-devel mailing list