[PATCH 1/3] drm/amdgpu: provide a generic function interface for reading/writing register by KIQ

Alex Deucher alexdeucher at gmail.com
Thu Jan 16 19:21:25 UTC 2020


On Thu, Jan 16, 2020 at 7:44 AM Christian König
<ckoenig.leichtzumerken at gmail.com> wrote:
>
> Am 16.01.20 um 11:00 schrieb chen gong:
> > Move amdgpu_virt_kiq_rreg/amdgpu_virt_kiq_wreg function to amdgpu_gfx.c,
> > and rename them to amdgpu_kiq_rreg/amdgpu_kiq_wreg.Make it generic and
> > flexible.
> >
> > Signed-off-by: chen gong <curry.gong at amd.com>
>
> Alex has the last word on this since I'm not so deep into the KIQ, but
> at least to me that looks like a rather nice cleanup.
>
> Feel free to add an Acked-by: Christian König <christian.koenig at amd.com>
> to the series.

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

>
> Christian.
>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 +-
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c    | 96 +++++++++++++++++++++++++++++-
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h    |  3 +
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c   | 92 ----------------------------
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h   |  2 -
> >   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c     |  5 +-
> >   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c      |  5 +-
> >   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c      |  5 +-
> >   8 files changed, 108 insertions(+), 104 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 2c64d2a..963ea46 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -218,7 +218,7 @@ uint32_t amdgpu_mm_rreg(struct amdgpu_device *adev, uint32_t reg,
> >       uint32_t ret;
> >
> >       if (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev))
> > -             return amdgpu_virt_kiq_rreg(adev, reg);
> > +             return amdgpu_kiq_rreg(adev, reg);
> >
> >       if ((reg * 4) < adev->rmmio_size && !(acc_flags & AMDGPU_REGS_IDX))
> >               ret = readl(((void __iomem *)adev->rmmio) + (reg * 4));
> > @@ -296,7 +296,7 @@ void amdgpu_mm_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v,
> >       }
> >
> >       if (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev))
> > -             return amdgpu_virt_kiq_wreg(adev, reg, v);
> > +             return amdgpu_kiq_wreg(adev, reg, v);
> >
> >       if ((reg * 4) < adev->rmmio_size && !(acc_flags & AMDGPU_REGS_IDX))
> >               writel(v, ((void __iomem *)adev->rmmio) + (reg * 4));
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > index b88b8b8..0f960b4 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > @@ -296,7 +296,7 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
> >
> >       spin_lock_init(&kiq->ring_lock);
> >
> > -     r = amdgpu_device_wb_get(adev, &adev->virt.reg_val_offs);
> > +     r = amdgpu_device_wb_get(adev, &kiq->reg_val_offs);
> >       if (r)
> >               return r;
> >
> > @@ -321,7 +321,7 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
> >
> >   void amdgpu_gfx_kiq_free_ring(struct amdgpu_ring *ring)
> >   {
> > -     amdgpu_device_wb_free(ring->adev, ring->adev->virt.reg_val_offs);
> > +     amdgpu_device_wb_free(ring->adev, ring->adev->gfx.kiq.reg_val_offs);
> >       amdgpu_ring_fini(ring);
> >   }
> >
> > @@ -658,3 +658,95 @@ int amdgpu_gfx_cp_ecc_error_irq(struct amdgpu_device *adev,
> >       amdgpu_ras_interrupt_dispatch(adev, &ih_data);
> >       return 0;
> >   }
> > +
> > +uint32_t amdgpu_kiq_rreg(struct amdgpu_device *adev, uint32_t reg)
> > +{
> > +     signed long r, cnt = 0;
> > +     unsigned long flags;
> > +     uint32_t seq;
> > +     struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> > +     struct amdgpu_ring *ring = &kiq->ring;
> > +
> > +     BUG_ON(!ring->funcs->emit_rreg);
> > +
> > +     spin_lock_irqsave(&kiq->ring_lock, flags);
> > +     amdgpu_ring_alloc(ring, 32);
> > +     amdgpu_ring_emit_rreg(ring, reg);
> > +     amdgpu_fence_emit_polling(ring, &seq);
> > +     amdgpu_ring_commit(ring);
> > +     spin_unlock_irqrestore(&kiq->ring_lock, flags);
> > +
> > +     r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> > +
> > +     /* don't wait anymore for gpu reset case because this way may
> > +      * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
> > +      * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
> > +      * never return if we keep waiting in virt_kiq_rreg, which cause
> > +      * gpu_recover() hang there.
> > +      *
> > +      * also don't wait anymore for IRQ context
> > +      * */
> > +     if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
> > +             goto failed_kiq_read;
> > +
> > +     might_sleep();
> > +     while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
> > +             msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
> > +             r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> > +     }
> > +
> > +     if (cnt > MAX_KIQ_REG_TRY)
> > +             goto failed_kiq_read;
> > +
> > +     return adev->wb.wb[kiq->reg_val_offs];
> > +
> > +failed_kiq_read:
> > +     pr_err("failed to read reg:%x\n", reg);
> > +     return ~0;
> > +}
> > +
> > +void amdgpu_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v)
> > +{
> > +     signed long r, cnt = 0;
> > +     unsigned long flags;
> > +     uint32_t seq;
> > +     struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> > +     struct amdgpu_ring *ring = &kiq->ring;
> > +
> > +     BUG_ON(!ring->funcs->emit_wreg);
> > +
> > +     spin_lock_irqsave(&kiq->ring_lock, flags);
> > +     amdgpu_ring_alloc(ring, 32);
> > +     amdgpu_ring_emit_wreg(ring, reg, v);
> > +     amdgpu_fence_emit_polling(ring, &seq);
> > +     amdgpu_ring_commit(ring);
> > +     spin_unlock_irqrestore(&kiq->ring_lock, flags);
> > +
> > +     r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> > +
> > +     /* don't wait anymore for gpu reset case because this way may
> > +      * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
> > +      * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
> > +      * never return if we keep waiting in virt_kiq_rreg, which cause
> > +      * gpu_recover() hang there.
> > +      *
> > +      * also don't wait anymore for IRQ context
> > +      * */
> > +     if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
> > +             goto failed_kiq_write;
> > +
> > +     might_sleep();
> > +     while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
> > +
> > +             msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
> > +             r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> > +     }
> > +
> > +     if (cnt > MAX_KIQ_REG_TRY)
> > +             goto failed_kiq_write;
> > +
> > +     return;
> > +
> > +failed_kiq_write:
> > +     pr_err("failed to write reg:%x\n", reg);
> > +}
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> > index af4bd27..ca17ffb 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> > @@ -94,6 +94,7 @@ struct amdgpu_kiq {
> >       struct amdgpu_ring      ring;
> >       struct amdgpu_irq_src   irq;
> >       const struct kiq_pm4_funcs *pmf;
> > +     uint32_t                        reg_val_offs;
> >   };
> >
> >   /*
> > @@ -375,4 +376,6 @@ int amdgpu_gfx_process_ras_data_cb(struct amdgpu_device *adev,
> >   int amdgpu_gfx_cp_ecc_error_irq(struct amdgpu_device *adev,
> >                                 struct amdgpu_irq_src *source,
> >                                 struct amdgpu_iv_entry *entry);
> > +uint32_t amdgpu_kiq_rreg(struct amdgpu_device *adev, uint32_t reg);
> > +void amdgpu_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v);
> >   #endif
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > index 103033f..adc813c 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > @@ -45,98 +45,6 @@ void amdgpu_virt_init_setting(struct amdgpu_device *adev)
> >       adev->pg_flags = 0;
> >   }
> >
> > -uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg)
> > -{
> > -     signed long r, cnt = 0;
> > -     unsigned long flags;
> > -     uint32_t seq;
> > -     struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> > -     struct amdgpu_ring *ring = &kiq->ring;
> > -
> > -     BUG_ON(!ring->funcs->emit_rreg);
> > -
> > -     spin_lock_irqsave(&kiq->ring_lock, flags);
> > -     amdgpu_ring_alloc(ring, 32);
> > -     amdgpu_ring_emit_rreg(ring, reg);
> > -     amdgpu_fence_emit_polling(ring, &seq);
> > -     amdgpu_ring_commit(ring);
> > -     spin_unlock_irqrestore(&kiq->ring_lock, flags);
> > -
> > -     r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> > -
> > -     /* don't wait anymore for gpu reset case because this way may
> > -      * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
> > -      * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
> > -      * never return if we keep waiting in virt_kiq_rreg, which cause
> > -      * gpu_recover() hang there.
> > -      *
> > -      * also don't wait anymore for IRQ context
> > -      * */
> > -     if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
> > -             goto failed_kiq_read;
> > -
> > -     might_sleep();
> > -     while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
> > -             msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
> > -             r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> > -     }
> > -
> > -     if (cnt > MAX_KIQ_REG_TRY)
> > -             goto failed_kiq_read;
> > -
> > -     return adev->wb.wb[adev->virt.reg_val_offs];
> > -
> > -failed_kiq_read:
> > -     pr_err("failed to read reg:%x\n", reg);
> > -     return ~0;
> > -}
> > -
> > -void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v)
> > -{
> > -     signed long r, cnt = 0;
> > -     unsigned long flags;
> > -     uint32_t seq;
> > -     struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> > -     struct amdgpu_ring *ring = &kiq->ring;
> > -
> > -     BUG_ON(!ring->funcs->emit_wreg);
> > -
> > -     spin_lock_irqsave(&kiq->ring_lock, flags);
> > -     amdgpu_ring_alloc(ring, 32);
> > -     amdgpu_ring_emit_wreg(ring, reg, v);
> > -     amdgpu_fence_emit_polling(ring, &seq);
> > -     amdgpu_ring_commit(ring);
> > -     spin_unlock_irqrestore(&kiq->ring_lock, flags);
> > -
> > -     r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> > -
> > -     /* don't wait anymore for gpu reset case because this way may
> > -      * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
> > -      * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
> > -      * never return if we keep waiting in virt_kiq_rreg, which cause
> > -      * gpu_recover() hang there.
> > -      *
> > -      * also don't wait anymore for IRQ context
> > -      * */
> > -     if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
> > -             goto failed_kiq_write;
> > -
> > -     might_sleep();
> > -     while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
> > -
> > -             msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
> > -             r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> > -     }
> > -
> > -     if (cnt > MAX_KIQ_REG_TRY)
> > -             goto failed_kiq_write;
> > -
> > -     return;
> > -
> > -failed_kiq_write:
> > -     pr_err("failed to write reg:%x\n", reg);
> > -}
> > -
> >   void amdgpu_virt_kiq_reg_write_reg_wait(struct amdgpu_device *adev,
> >                                       uint32_t reg0, uint32_t reg1,
> >                                       uint32_t ref, uint32_t mask)
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> > index 4d1ac76..daaf909 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> > @@ -287,8 +287,6 @@ static inline bool is_virtual_machine(void)
> >
> >   bool amdgpu_virt_mmio_blocked(struct amdgpu_device *adev);
> >   void amdgpu_virt_init_setting(struct amdgpu_device *adev);
> > -uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg);
> > -void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v);
> >   void amdgpu_virt_kiq_reg_write_reg_wait(struct amdgpu_device *adev,
> >                                       uint32_t reg0, uint32_t rreg1,
> >                                       uint32_t ref, uint32_t mask);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> > index 1cfc508..e74ed06 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> > @@ -4747,6 +4747,7 @@ static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
> >   static void gfx_v10_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
> >   {
> >       struct amdgpu_device *adev = ring->adev;
> > +     struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> >
> >       amdgpu_ring_write(ring, PACKET3(PACKET3_COPY_DATA, 4));
> >       amdgpu_ring_write(ring, 0 |     /* src: register*/
> > @@ -4755,9 +4756,9 @@ static void gfx_v10_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
> >       amdgpu_ring_write(ring, reg);
> >       amdgpu_ring_write(ring, 0);
> >       amdgpu_ring_write(ring, lower_32_bits(adev->wb.gpu_addr +
> > -                             adev->virt.reg_val_offs * 4));
> > +                             kiq->reg_val_offs * 4));
> >       amdgpu_ring_write(ring, upper_32_bits(adev->wb.gpu_addr +
> > -                             adev->virt.reg_val_offs * 4));
> > +                             kiq->reg_val_offs * 4));
> >   }
> >
> >   static void gfx_v10_0_ring_emit_wreg(struct amdgpu_ring *ring, uint32_t reg,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > index 306ee61..a46ec1c 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > @@ -6450,6 +6450,7 @@ static void gfx_v8_0_ring_emit_patch_cond_exec(struct amdgpu_ring *ring, unsigne
> >   static void gfx_v8_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
> >   {
> >       struct amdgpu_device *adev = ring->adev;
> > +     struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> >
> >       amdgpu_ring_write(ring, PACKET3(PACKET3_COPY_DATA, 4));
> >       amdgpu_ring_write(ring, 0 |     /* src: register*/
> > @@ -6458,9 +6459,9 @@ static void gfx_v8_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
> >       amdgpu_ring_write(ring, reg);
> >       amdgpu_ring_write(ring, 0);
> >       amdgpu_ring_write(ring, lower_32_bits(adev->wb.gpu_addr +
> > -                             adev->virt.reg_val_offs * 4));
> > +                             kiq->reg_val_offs * 4));
> >       amdgpu_ring_write(ring, upper_32_bits(adev->wb.gpu_addr +
> > -                             adev->virt.reg_val_offs * 4));
> > +                             kiq->reg_val_offs * 4));
> >   }
> >
> >   static void gfx_v8_0_ring_emit_wreg(struct amdgpu_ring *ring, uint32_t reg,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > index 0dfdc86..d9d7ee9 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > @@ -5221,6 +5221,7 @@ static void gfx_v9_0_ring_emit_patch_cond_exec(struct amdgpu_ring *ring, unsigne
> >   static void gfx_v9_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
> >   {
> >       struct amdgpu_device *adev = ring->adev;
> > +     struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> >
> >       amdgpu_ring_write(ring, PACKET3(PACKET3_COPY_DATA, 4));
> >       amdgpu_ring_write(ring, 0 |     /* src: register*/
> > @@ -5229,9 +5230,9 @@ static void gfx_v9_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
> >       amdgpu_ring_write(ring, reg);
> >       amdgpu_ring_write(ring, 0);
> >       amdgpu_ring_write(ring, lower_32_bits(adev->wb.gpu_addr +
> > -                             adev->virt.reg_val_offs * 4));
> > +                             kiq->reg_val_offs * 4));
> >       amdgpu_ring_write(ring, upper_32_bits(adev->wb.gpu_addr +
> > -                             adev->virt.reg_val_offs * 4));
> > +                             kiq->reg_val_offs * 4));
> >   }
> >
> >   static void gfx_v9_0_ring_emit_wreg(struct amdgpu_ring *ring, uint32_t reg,
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


More information about the amd-gfx mailing list