Re: Move kiq ring lock out of virt strucure

[Andres Rodriguez <andresx7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

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@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@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@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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx