I actually have a similar patch in my tree at the moment.
Only a minor nitpick (the original base also has the same problem). Can
you rename 'lock_ring' to 'ring_mutex'? Usually the _lock suffix is
used for spinlock_t and _mutex is used for mutexes.
With that fixed you can add:
Reviewed-by: Andres Rodriguez <andresx7 at gmail.com>
Regards,
Andres
On 2017-04-28 05:26 PM, Liu, Shaoyun wrote:
> From c520110807459bdd3fa4d6c86b2f2ab291051ebc Mon Sep 17 00:00:00 2001
> From: Shaoyun Liu <Shaoyun.Liu at amd.com>
> Date: Fri, 28 Apr 2017 17:18:26 -0400
> Subject: [PATCH] drm/amdgpu: Move kiq ring lock out of virt structure
>
> The usage of kiq should not depend on the virtualization.
>
> Change-Id: I39a439383b0c48d8f410cd362325b8404382cd53
> Signed-off-by: Shaoyun Liu <Shaoyun.Liu at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 9 ++++-----
> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 1 -
> drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 2 ++
> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 ++
> 5 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index e330009..dbd0cb2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -909,6 +909,7 @@ struct amdgpu_mec {
> struct amdgpu_kiq {
> u64 eop_gpu_addr;
> struct amdgpu_bo *eop_obj;
> + struct mutex lock_ring;
> struct amdgpu_ring ring;
> struct amdgpu_irq_src irq;
> };
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index 1363239..4da15e8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -108,7 +108,6 @@ void amdgpu_virt_init_setting(struct amdgpu_device *adev)
> adev->cg_flags = 0;
> adev->pg_flags = 0;
>
> - mutex_init(&adev->virt.lock_kiq);
> mutex_init(&adev->virt.lock_reset);
> }
>
> @@ -122,12 +121,12 @@ uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg)
>
> BUG_ON(!ring->funcs->emit_rreg);
>
> - mutex_lock(&adev->virt.lock_kiq);
> + mutex_lock(&kiq->lock_ring);
> amdgpu_ring_alloc(ring, 32);
> amdgpu_ring_emit_rreg(ring, reg);
> amdgpu_fence_emit(ring, &f);
> amdgpu_ring_commit(ring);
> - mutex_unlock(&adev->virt.lock_kiq);
> + mutex_unlock(&kiq->lock_ring);
>
> r = fence_wait(f, false);
> if (r)
> @@ -148,12 +147,12 @@ void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v)
>
> BUG_ON(!ring->funcs->emit_wreg);
>
> - mutex_lock(&adev->virt.lock_kiq);
> + mutex_lock(&kiq->lock_ring);
> amdgpu_ring_alloc(ring, 32);
> amdgpu_ring_emit_wreg(ring, reg, v);
> amdgpu_fence_emit(ring, &f);
> amdgpu_ring_commit(ring);
> - mutex_unlock(&adev->virt.lock_kiq);
> + mutex_unlock(&kiq->lock_ring);
>
> r = fence_wait(f, false);
> if (r)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> index a8ed162..6f2b7df 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> @@ -52,7 +52,6 @@ struct amdgpu_virt {
> uint64_t csa_vmid0_addr;
> bool chained_ib_support;
> uint32_t reg_val_offs;
> - struct mutex lock_kiq;
> struct mutex lock_reset;
> struct amdgpu_irq_src ack_irq;
> struct amdgpu_irq_src rcv_irq;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index ac64e01..d649479 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -1385,6 +1385,8 @@ static int gfx_v8_0_kiq_init_ring(struct amdgpu_device *adev,
> struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> int r = 0;
>
> + mutex_init(&kiq->lock_ring);
> +
> r = amdgpu_wb_get(adev, &adev->virt.reg_val_offs);
> if (r)
> return r;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 08daa3f..9b68d33 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -615,6 +615,8 @@ static int gfx_v9_0_kiq_init_ring(struct amdgpu_device *adev,
> struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> int r = 0;
>
> + mutex_init(&kiq->lock_ring);
> +
> r = amdgpu_wb_get(adev, &adev->virt.reg_val_offs);
> if (r)
> return r;
> -- 1.9.1
