RE: Move kiq ring lock out of virt strucure

2017-04-28 Thread Liu, Shaoyun 
Thanks , Andres . 
Changed as suggested  .

Regards
Shaoyun.liu


-Original Message-
From: Andres Rodriguez [mailto:andre...@gmail.com] 
Sent: Friday, April 28, 2017 5:41 PM
To: Liu, Shaoyun; amd-gfx list (amd-gfx@lists.freedesktop.org); Deucher, 
Alexander
Cc: brahma_sw_dev
Subject: Re: Move kiq ring lock out of virt strucure

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 <andre...@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@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@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 mutexlock_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(>virt.lock_kiq);
>   mutex_init(>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(>virt.lock_kiq);
> + mutex_lock(>lock_ring);
>   amdgpu_ring_alloc(ring, 32);
>   amdgpu_ring_emit_rreg(ring, reg);
>   amdgpu_fence_emit(ring, );
>   amdgpu_ring_commit(ring);
> - mutex_unlock(>virt.lock_kiq);
> + mutex_unlock(>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(>virt.lock_kiq);
> + mutex_lock(>lock_ring);
>   amdgpu_ring_alloc(ring, 32);
>   amdgpu_ring_emit_wreg(ring, reg, v);
>   amdgpu_fence_emit(ring, );
>   amdgpu_ring_commit(ring);
> - mutex_unlock(>virt.lock_kiq);
> + mutex_unlock(>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_tcsa_vmid0_addr;
>   bool chained_ib_support;
>   uint32_treg_val_offs;
> - struct mutexlock_kiq;
>   struct mutexlock_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 = >gfx.kiq;
>   int r = 0;
>
> + mutex_init(>lock_ring);
> +
>   r = amdgpu_wb_get(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

Re: Move kiq ring lock out of virt strucure

2017-04-28 Thread Andres Rodriguez 

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 

Regards,
Andres

On 2017-04-28 05:26 PM, Liu, Shaoyun wrote:

From c520110807459bdd3fa4d6c86b2f2ab291051ebc Mon Sep 17 00:00:00 2001
From: Shaoyun Liu 
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 
---
 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 mutexlock_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(>virt.lock_kiq);
mutex_init(>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(>virt.lock_kiq);
+   mutex_lock(>lock_ring);
amdgpu_ring_alloc(ring, 32);
amdgpu_ring_emit_rreg(ring, reg);
amdgpu_fence_emit(ring, );
amdgpu_ring_commit(ring);
-   mutex_unlock(>virt.lock_kiq);
+   mutex_unlock(>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(>virt.lock_kiq);
+   mutex_lock(>lock_ring);
amdgpu_ring_alloc(ring, 32);
amdgpu_ring_emit_wreg(ring, reg, v);
amdgpu_fence_emit(ring, );
amdgpu_ring_commit(ring);
-   mutex_unlock(>virt.lock_kiq);
+   mutex_unlock(>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_tcsa_vmid0_addr;
bool chained_ib_support;
uint32_treg_val_offs;
-   struct mutexlock_kiq;
struct mutexlock_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 = >gfx.kiq;
int r = 0;

+   mutex_init(>lock_ring);
+
r = amdgpu_wb_get(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 = >gfx.kiq;
int r = 0;

+   mutex_init(>lock_ring);
+
r = amdgpu_wb_get(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