All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: add JPEG check to VCN idle handler and begin use
@ 2019-12-11 19:48 Leo Liu
  2019-12-11 19:52 ` Alex Deucher
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Leo Liu @ 2019-12-11 19:48 UTC (permalink / raw)
  To: amd-gfx; +Cc: Leo Liu

Since it's only needed with VCN1.0 when HW has no its
own JPEG HW IP block

Signed-off-by: Leo Liu <leo.liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 29 +++++++++++++++----------
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  2 ++
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 428cfd58b37d..95ac721f2de0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -186,6 +186,9 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
 		}
 	}
 
+	adev->vcn.has_jpeg_block = (amdgpu_device_ip_get_ip_block(adev, AMD_IP_BLOCK_TYPE_JPEG)) ?
+		true : false;
+
 	return 0;
 }
 
@@ -306,15 +309,17 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
 			else
 				new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
 
-			if (amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec))
-				new_state.jpeg = VCN_DPG_STATE__PAUSE;
-			else
-				new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
-
+			if (!adev->vcn.has_jpeg_block) {
+				if (amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec))
+					new_state.jpeg = VCN_DPG_STATE__PAUSE;
+				else
+					new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
+			}
 			adev->vcn.pause_dpg_mode(adev, &new_state);
 		}
 
-		fence[j] += amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec);
+		if (!adev->vcn.has_jpeg_block)
+			fence[j] += amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec);
 		fence[j] += amdgpu_fence_count_emitted(&adev->vcn.inst[j].ring_dec);
 		fences += fence[j];
 	}
@@ -358,14 +363,16 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
 		else
 			new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
 
-		if (amdgpu_fence_count_emitted(&adev->jpeg.inst[ring->me].ring_dec))
-			new_state.jpeg = VCN_DPG_STATE__PAUSE;
-		else
-			new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
+		if (!adev->vcn.has_jpeg_block) {
+			if (amdgpu_fence_count_emitted(&adev->jpeg.inst[ring->me].ring_dec))
+				new_state.jpeg = VCN_DPG_STATE__PAUSE;
+			else
+				new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
+		}
 
 		if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC)
 			new_state.fw_based = VCN_DPG_STATE__PAUSE;
-		else if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_JPEG)
+		else if (!adev->vcn.has_jpeg_block && ring->funcs->type == AMDGPU_RING_TYPE_VCN_JPEG)
 			new_state.jpeg = VCN_DPG_STATE__PAUSE;
 
 		adev->vcn.pause_dpg_mode(adev, &new_state);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index 402a5046b985..9a2381d006c6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -192,6 +192,8 @@ struct amdgpu_vcn {
 	unsigned	harvest_config;
 	int (*pause_dpg_mode)(struct amdgpu_device *adev,
 		struct dpg_pause_state *new_state);
+
+	bool has_jpeg_block;
 };
 
 int amdgpu_vcn_sw_init(struct amdgpu_device *adev);
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/amdgpu: add JPEG check to VCN idle handler and begin use
  2019-12-11 19:48 [PATCH] drm/amdgpu: add JPEG check to VCN idle handler and begin use Leo Liu
@ 2019-12-11 19:52 ` Alex Deucher
  2019-12-11 20:01 ` Zhang, Boyuan
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Alex Deucher @ 2019-12-11 19:52 UTC (permalink / raw)
  To: Leo Liu; +Cc: amd-gfx list

On Wed, Dec 11, 2019 at 2:49 PM Leo Liu <leo.liu@amd.com> wrote:
>
> Since it's only needed with VCN1.0 when HW has no its
> own JPEG HW IP block

typo "HW does not have its own"
With that fixed:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

>
> Signed-off-by: Leo Liu <leo.liu@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 29 +++++++++++++++----------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  2 ++
>  2 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 428cfd58b37d..95ac721f2de0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -186,6 +186,9 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
>                 }
>         }
>
> +       adev->vcn.has_jpeg_block = (amdgpu_device_ip_get_ip_block(adev, AMD_IP_BLOCK_TYPE_JPEG)) ?
> +               true : false;
> +
>         return 0;
>  }
>
> @@ -306,15 +309,17 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
>                         else
>                                 new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
>
> -                       if (amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec))
> -                               new_state.jpeg = VCN_DPG_STATE__PAUSE;
> -                       else
> -                               new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
> -
> +                       if (!adev->vcn.has_jpeg_block) {
> +                               if (amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec))
> +                                       new_state.jpeg = VCN_DPG_STATE__PAUSE;
> +                               else
> +                                       new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
> +                       }
>                         adev->vcn.pause_dpg_mode(adev, &new_state);
>                 }
>
> -               fence[j] += amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec);
> +               if (!adev->vcn.has_jpeg_block)
> +                       fence[j] += amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec);
>                 fence[j] += amdgpu_fence_count_emitted(&adev->vcn.inst[j].ring_dec);
>                 fences += fence[j];
>         }
> @@ -358,14 +363,16 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
>                 else
>                         new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
>
> -               if (amdgpu_fence_count_emitted(&adev->jpeg.inst[ring->me].ring_dec))
> -                       new_state.jpeg = VCN_DPG_STATE__PAUSE;
> -               else
> -                       new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
> +               if (!adev->vcn.has_jpeg_block) {
> +                       if (amdgpu_fence_count_emitted(&adev->jpeg.inst[ring->me].ring_dec))
> +                               new_state.jpeg = VCN_DPG_STATE__PAUSE;
> +                       else
> +                               new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
> +               }
>
>                 if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC)
>                         new_state.fw_based = VCN_DPG_STATE__PAUSE;
> -               else if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_JPEG)
> +               else if (!adev->vcn.has_jpeg_block && ring->funcs->type == AMDGPU_RING_TYPE_VCN_JPEG)
>                         new_state.jpeg = VCN_DPG_STATE__PAUSE;
>
>                 adev->vcn.pause_dpg_mode(adev, &new_state);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> index 402a5046b985..9a2381d006c6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -192,6 +192,8 @@ struct amdgpu_vcn {
>         unsigned        harvest_config;
>         int (*pause_dpg_mode)(struct amdgpu_device *adev,
>                 struct dpg_pause_state *new_state);
> +
> +       bool has_jpeg_block;
>  };
>
>  int amdgpu_vcn_sw_init(struct amdgpu_device *adev);
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH] drm/amdgpu: add JPEG check to VCN idle handler and begin use
  2019-12-11 19:48 [PATCH] drm/amdgpu: add JPEG check to VCN idle handler and begin use Leo Liu
  2019-12-11 19:52 ` Alex Deucher
@ 2019-12-11 20:01 ` Zhang, Boyuan
  2019-12-12  8:18 ` Christian König
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Zhang, Boyuan @ 2019-12-11 20:01 UTC (permalink / raw)
  To: Liu, Leo, amd-gfx; +Cc: Liu, Leo

This patch is 
Reviewed-by: Boyuan Zhang <boyuan.zhang@amd.com>


-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Leo Liu
Sent: December 11, 2019 2:48 PM
To: amd-gfx@lists.freedesktop.org
Cc: Liu, Leo <Leo.Liu@amd.com>
Subject: [PATCH] drm/amdgpu: add JPEG check to VCN idle handler and begin use

Since it's only needed with VCN1.0 when HW has no its own JPEG HW IP block

Signed-off-by: Leo Liu <leo.liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 29 +++++++++++++++----------  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  2 ++
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 428cfd58b37d..95ac721f2de0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -186,6 +186,9 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
 		}
 	}
 
+	adev->vcn.has_jpeg_block = (amdgpu_device_ip_get_ip_block(adev, AMD_IP_BLOCK_TYPE_JPEG)) ?
+		true : false;
+
 	return 0;
 }
 
@@ -306,15 +309,17 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
 			else
 				new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
 
-			if (amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec))
-				new_state.jpeg = VCN_DPG_STATE__PAUSE;
-			else
-				new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
-
+			if (!adev->vcn.has_jpeg_block) {
+				if (amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec))
+					new_state.jpeg = VCN_DPG_STATE__PAUSE;
+				else
+					new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
+			}
 			adev->vcn.pause_dpg_mode(adev, &new_state);
 		}
 
-		fence[j] += amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec);
+		if (!adev->vcn.has_jpeg_block)
+			fence[j] += 
+amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec);
 		fence[j] += amdgpu_fence_count_emitted(&adev->vcn.inst[j].ring_dec);
 		fences += fence[j];
 	}
@@ -358,14 +363,16 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
 		else
 			new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
 
-		if (amdgpu_fence_count_emitted(&adev->jpeg.inst[ring->me].ring_dec))
-			new_state.jpeg = VCN_DPG_STATE__PAUSE;
-		else
-			new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
+		if (!adev->vcn.has_jpeg_block) {
+			if (amdgpu_fence_count_emitted(&adev->jpeg.inst[ring->me].ring_dec))
+				new_state.jpeg = VCN_DPG_STATE__PAUSE;
+			else
+				new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
+		}
 
 		if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC)
 			new_state.fw_based = VCN_DPG_STATE__PAUSE;
-		else if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_JPEG)
+		else if (!adev->vcn.has_jpeg_block && ring->funcs->type == 
+AMDGPU_RING_TYPE_VCN_JPEG)
 			new_state.jpeg = VCN_DPG_STATE__PAUSE;
 
 		adev->vcn.pause_dpg_mode(adev, &new_state); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index 402a5046b985..9a2381d006c6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -192,6 +192,8 @@ struct amdgpu_vcn {
 	unsigned	harvest_config;
 	int (*pause_dpg_mode)(struct amdgpu_device *adev,
 		struct dpg_pause_state *new_state);
+
+	bool has_jpeg_block;
 };
 
 int amdgpu_vcn_sw_init(struct amdgpu_device *adev);
--
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cboyuan.zhang%40amd.com%7C141d98480bb4442c6dff08d77e733651%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637116905879102653&amp;sdata=6Ce6RR2jvpsESZs1I%2FOsYTX1cmImmftEBqfqpN8cp4s%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/amdgpu: add JPEG check to VCN idle handler and begin use
  2019-12-11 19:48 [PATCH] drm/amdgpu: add JPEG check to VCN idle handler and begin use Leo Liu
  2019-12-11 19:52 ` Alex Deucher
  2019-12-11 20:01 ` Zhang, Boyuan
@ 2019-12-12  8:18 ` Christian König
  2019-12-12 15:57   ` Leo Liu
  2019-12-12 16:06 ` [PATCH 1/2] drm/amdgpu/vcn1.0: use its own idle handler and begin use funcs Leo Liu
  2019-12-13 14:42 ` Leo Liu
  4 siblings, 1 reply; 12+ messages in thread
From: Christian König @ 2019-12-12  8:18 UTC (permalink / raw)
  To: Leo Liu, amd-gfx

Am 11.12.19 um 20:48 schrieb Leo Liu:
> Since it's only needed with VCN1.0 when HW has no its
> own JPEG HW IP block

Wouldn't it be simpler/cleaner to just define a 
vcn_v1_0_ring_begin_use() and vcn_v1_0_idle_work_handler() instead?

Regards,
Christian.

>
> Signed-off-by: Leo Liu <leo.liu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 29 +++++++++++++++----------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  2 ++
>   2 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 428cfd58b37d..95ac721f2de0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -186,6 +186,9 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
>   		}
>   	}
>   
> +	adev->vcn.has_jpeg_block = (amdgpu_device_ip_get_ip_block(adev, AMD_IP_BLOCK_TYPE_JPEG)) ?
> +		true : false;
> +
>   	return 0;
>   }
>   
> @@ -306,15 +309,17 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
>   			else
>   				new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
>   
> -			if (amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec))
> -				new_state.jpeg = VCN_DPG_STATE__PAUSE;
> -			else
> -				new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
> -
> +			if (!adev->vcn.has_jpeg_block) {
> +				if (amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec))
> +					new_state.jpeg = VCN_DPG_STATE__PAUSE;
> +				else
> +					new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
> +			}
>   			adev->vcn.pause_dpg_mode(adev, &new_state);
>   		}
>   
> -		fence[j] += amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec);
> +		if (!adev->vcn.has_jpeg_block)
> +			fence[j] += amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec);
>   		fence[j] += amdgpu_fence_count_emitted(&adev->vcn.inst[j].ring_dec);
>   		fences += fence[j];
>   	}
> @@ -358,14 +363,16 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
>   		else
>   			new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
>   
> -		if (amdgpu_fence_count_emitted(&adev->jpeg.inst[ring->me].ring_dec))
> -			new_state.jpeg = VCN_DPG_STATE__PAUSE;
> -		else
> -			new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
> +		if (!adev->vcn.has_jpeg_block) {
> +			if (amdgpu_fence_count_emitted(&adev->jpeg.inst[ring->me].ring_dec))
> +				new_state.jpeg = VCN_DPG_STATE__PAUSE;
> +			else
> +				new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
> +		}
>   
>   		if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC)
>   			new_state.fw_based = VCN_DPG_STATE__PAUSE;
> -		else if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_JPEG)
> +		else if (!adev->vcn.has_jpeg_block && ring->funcs->type == AMDGPU_RING_TYPE_VCN_JPEG)
>   			new_state.jpeg = VCN_DPG_STATE__PAUSE;
>   
>   		adev->vcn.pause_dpg_mode(adev, &new_state);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> index 402a5046b985..9a2381d006c6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -192,6 +192,8 @@ struct amdgpu_vcn {
>   	unsigned	harvest_config;
>   	int (*pause_dpg_mode)(struct amdgpu_device *adev,
>   		struct dpg_pause_state *new_state);
> +
> +	bool has_jpeg_block;
>   };
>   
>   int amdgpu_vcn_sw_init(struct amdgpu_device *adev);

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/amdgpu: add JPEG check to VCN idle handler and begin use
  2019-12-12  8:18 ` Christian König
@ 2019-12-12 15:57   ` Leo Liu
  2019-12-12 15:58     ` Christian König
  0 siblings, 1 reply; 12+ messages in thread
From: Leo Liu @ 2019-12-12 15:57 UTC (permalink / raw)
  To: christian.koenig, amd-gfx


On 2019-12-12 3:18 a.m., Christian König wrote:
> Am 11.12.19 um 20:48 schrieb Leo Liu:
>> Since it's only needed with VCN1.0 when HW has no its
>> own JPEG HW IP block
>
> Wouldn't it be simpler/cleaner to just define a 
> vcn_v1_0_ring_begin_use() and vcn_v1_0_idle_work_handler() instead?

Yeah, this way should be cleaner, even though the changes got bigger, 
the new set will be sent shortly.

Thanks,

Leo



>
> Regards,
> Christian.
>
>>
>> Signed-off-by: Leo Liu <leo.liu@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 29 +++++++++++++++----------
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  2 ++
>>   2 files changed, 20 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> index 428cfd58b37d..95ac721f2de0 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> @@ -186,6 +186,9 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
>>           }
>>       }
>>   +    adev->vcn.has_jpeg_block = 
>> (amdgpu_device_ip_get_ip_block(adev, AMD_IP_BLOCK_TYPE_JPEG)) ?
>> +        true : false;
>> +
>>       return 0;
>>   }
>>   @@ -306,15 +309,17 @@ static void 
>> amdgpu_vcn_idle_work_handler(struct work_struct *work)
>>               else
>>                   new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
>>   -            if 
>> (amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec))
>> -                new_state.jpeg = VCN_DPG_STATE__PAUSE;
>> -            else
>> -                new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
>> -
>> +            if (!adev->vcn.has_jpeg_block) {
>> +                if 
>> (amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec))
>> +                    new_state.jpeg = VCN_DPG_STATE__PAUSE;
>> +                else
>> +                    new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
>> +            }
>>               adev->vcn.pause_dpg_mode(adev, &new_state);
>>           }
>>   -        fence[j] += 
>> amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec);
>> +        if (!adev->vcn.has_jpeg_block)
>> +            fence[j] += 
>> amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec);
>>           fence[j] += 
>> amdgpu_fence_count_emitted(&adev->vcn.inst[j].ring_dec);
>>           fences += fence[j];
>>       }
>> @@ -358,14 +363,16 @@ void amdgpu_vcn_ring_begin_use(struct 
>> amdgpu_ring *ring)
>>           else
>>               new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
>>   -        if 
>> (amdgpu_fence_count_emitted(&adev->jpeg.inst[ring->me].ring_dec))
>> -            new_state.jpeg = VCN_DPG_STATE__PAUSE;
>> -        else
>> -            new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
>> +        if (!adev->vcn.has_jpeg_block) {
>> +            if 
>> (amdgpu_fence_count_emitted(&adev->jpeg.inst[ring->me].ring_dec))
>> +                new_state.jpeg = VCN_DPG_STATE__PAUSE;
>> +            else
>> +                new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
>> +        }
>>             if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC)
>>               new_state.fw_based = VCN_DPG_STATE__PAUSE;
>> -        else if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_JPEG)
>> +        else if (!adev->vcn.has_jpeg_block && ring->funcs->type == 
>> AMDGPU_RING_TYPE_VCN_JPEG)
>>               new_state.jpeg = VCN_DPG_STATE__PAUSE;
>>             adev->vcn.pause_dpg_mode(adev, &new_state);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> index 402a5046b985..9a2381d006c6 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> @@ -192,6 +192,8 @@ struct amdgpu_vcn {
>>       unsigned    harvest_config;
>>       int (*pause_dpg_mode)(struct amdgpu_device *adev,
>>           struct dpg_pause_state *new_state);
>> +
>> +    bool has_jpeg_block;
>>   };
>>     int amdgpu_vcn_sw_init(struct amdgpu_device *adev);
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/amdgpu: add JPEG check to VCN idle handler and begin use
  2019-12-12 15:57   ` Leo Liu
@ 2019-12-12 15:58     ` Christian König
  0 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2019-12-12 15:58 UTC (permalink / raw)
  To: Leo Liu, amd-gfx

Am 12.12.19 um 16:57 schrieb Leo Liu:
>
> On 2019-12-12 3:18 a.m., Christian König wrote:
>> Am 11.12.19 um 20:48 schrieb Leo Liu:
>>> Since it's only needed with VCN1.0 when HW has no its
>>> own JPEG HW IP block
>>
>> Wouldn't it be simpler/cleaner to just define a 
>> vcn_v1_0_ring_begin_use() and vcn_v1_0_idle_work_handler() instead?
>
> Yeah, this way should be cleaner, even though the changes got bigger, 
> the new set will be sent shortly.

Keep in mind that you don't need to fully clone the code.

You probably can still call the common VCN helper code quite a bit.

Christian.

>
> Thanks,
>
> Leo
>
>
>
>>
>> Regards,
>> Christian.
>>
>>>
>>> Signed-off-by: Leo Liu <leo.liu@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 29 
>>> +++++++++++++++----------
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  2 ++
>>>   2 files changed, 20 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>> index 428cfd58b37d..95ac721f2de0 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>> @@ -186,6 +186,9 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
>>>           }
>>>       }
>>>   +    adev->vcn.has_jpeg_block = 
>>> (amdgpu_device_ip_get_ip_block(adev, AMD_IP_BLOCK_TYPE_JPEG)) ?
>>> +        true : false;
>>> +
>>>       return 0;
>>>   }
>>>   @@ -306,15 +309,17 @@ static void 
>>> amdgpu_vcn_idle_work_handler(struct work_struct *work)
>>>               else
>>>                   new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
>>>   -            if 
>>> (amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec))
>>> -                new_state.jpeg = VCN_DPG_STATE__PAUSE;
>>> -            else
>>> -                new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
>>> -
>>> +            if (!adev->vcn.has_jpeg_block) {
>>> +                if 
>>> (amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec))
>>> +                    new_state.jpeg = VCN_DPG_STATE__PAUSE;
>>> +                else
>>> +                    new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
>>> +            }
>>>               adev->vcn.pause_dpg_mode(adev, &new_state);
>>>           }
>>>   -        fence[j] += 
>>> amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec);
>>> +        if (!adev->vcn.has_jpeg_block)
>>> +            fence[j] += 
>>> amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec);
>>>           fence[j] += 
>>> amdgpu_fence_count_emitted(&adev->vcn.inst[j].ring_dec);
>>>           fences += fence[j];
>>>       }
>>> @@ -358,14 +363,16 @@ void amdgpu_vcn_ring_begin_use(struct 
>>> amdgpu_ring *ring)
>>>           else
>>>               new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
>>>   -        if 
>>> (amdgpu_fence_count_emitted(&adev->jpeg.inst[ring->me].ring_dec))
>>> -            new_state.jpeg = VCN_DPG_STATE__PAUSE;
>>> -        else
>>> -            new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
>>> +        if (!adev->vcn.has_jpeg_block) {
>>> +            if 
>>> (amdgpu_fence_count_emitted(&adev->jpeg.inst[ring->me].ring_dec))
>>> +                new_state.jpeg = VCN_DPG_STATE__PAUSE;
>>> +            else
>>> +                new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
>>> +        }
>>>             if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC)
>>>               new_state.fw_based = VCN_DPG_STATE__PAUSE;
>>> -        else if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_JPEG)
>>> +        else if (!adev->vcn.has_jpeg_block && ring->funcs->type == 
>>> AMDGPU_RING_TYPE_VCN_JPEG)
>>>               new_state.jpeg = VCN_DPG_STATE__PAUSE;
>>>             adev->vcn.pause_dpg_mode(adev, &new_state);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>> index 402a5046b985..9a2381d006c6 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>> @@ -192,6 +192,8 @@ struct amdgpu_vcn {
>>>       unsigned    harvest_config;
>>>       int (*pause_dpg_mode)(struct amdgpu_device *adev,
>>>           struct dpg_pause_state *new_state);
>>> +
>>> +    bool has_jpeg_block;
>>>   };
>>>     int amdgpu_vcn_sw_init(struct amdgpu_device *adev);
>>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] drm/amdgpu/vcn1.0: use its own idle handler and begin use funcs
  2019-12-11 19:48 [PATCH] drm/amdgpu: add JPEG check to VCN idle handler and begin use Leo Liu
                   ` (2 preceding siblings ...)
  2019-12-12  8:18 ` Christian König
@ 2019-12-12 16:06 ` Leo Liu
  2019-12-12 16:06   ` [PATCH 2/2] drm/amdgpu/vcn: remove JPEG related code from idle handler and begin use Leo Liu
                     ` (2 more replies)
  2019-12-13 14:42 ` Leo Liu
  4 siblings, 3 replies; 12+ messages in thread
From: Leo Liu @ 2019-12-12 16:06 UTC (permalink / raw)
  To: amd-gfx; +Cc: Leo Liu

Because VCN1.0 power management and DPG mode are managed together with
JPEG1.0 under both HW and FW, so separated them from general VCN code.
Also the multiple instances case got removed, since VCN1.0 HW just have
a single instance.

Signed-off-by: Leo Liu <leo.liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c |  7 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  3 +
 drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c  |  3 +-
 drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c   | 88 ++++++++++++++++++++++++-
 drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h   |  2 +
 5 files changed, 96 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 428cfd58b37d..e962c87d04cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -39,9 +39,6 @@
 #include "vcn/vcn_1_0_offset.h"
 #include "vcn/vcn_1_0_sh_mask.h"
 
-/* 1 second timeout */
-#define VCN_IDLE_TIMEOUT	msecs_to_jiffies(1000)
-
 /* Firmware Names */
 #define FIRMWARE_RAVEN		"amdgpu/raven_vcn.bin"
 #define FIRMWARE_PICASSO	"amdgpu/picasso_vcn.bin"
@@ -71,7 +68,9 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
 	unsigned char fw_check;
 	int i, r;
 
-	INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
+	/* For VCN2.0 and above */
+	if (adev->asic_type >= CHIP_ARCTURUS)
+		INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
 
 	switch (adev->asic_type) {
 	case CHIP_RAVEN:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index 402a5046b985..3484ead62046 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -56,6 +56,9 @@
 #define VCN_VID_IP_ADDRESS_2_0		0x0
 #define VCN_AON_IP_ADDRESS_2_0		0x30000
 
+/* 1 second timeout */
+#define VCN_IDLE_TIMEOUT	msecs_to_jiffies(1000)
+
 #define RREG32_SOC15_DPG_MODE(ip, inst, reg, mask, sram_sel) 				\
 	({	WREG32_SOC15(ip, inst, mmUVD_DPG_LMA_MASK, mask); 			\
 		WREG32_SOC15(ip, inst, mmUVD_DPG_LMA_CTL, 				\
diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c
index a141408dfb23..0debfd9f428c 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c
@@ -25,6 +25,7 @@
 #include "amdgpu_jpeg.h"
 #include "soc15.h"
 #include "soc15d.h"
+#include "vcn_v1_0.h"
 
 #include "vcn/vcn_1_0_offset.h"
 #include "vcn/vcn_1_0_sh_mask.h"
@@ -561,7 +562,7 @@ static const struct amdgpu_ring_funcs jpeg_v1_0_decode_ring_vm_funcs = {
 	.insert_start = jpeg_v1_0_decode_ring_insert_start,
 	.insert_end = jpeg_v1_0_decode_ring_insert_end,
 	.pad_ib = amdgpu_ring_generic_pad_ib,
-	.begin_use = amdgpu_vcn_ring_begin_use,
+	.begin_use = vcn_v1_0_ring_begin_use,
 	.end_use = amdgpu_vcn_ring_end_use,
 	.emit_wreg = jpeg_v1_0_decode_ring_emit_wreg,
 	.emit_reg_wait = jpeg_v1_0_decode_ring_emit_reg_wait,
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
index 652cecc030b3..7395286540e1 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
@@ -25,6 +25,7 @@
 
 #include "amdgpu.h"
 #include "amdgpu_vcn.h"
+#include "amdgpu_pm.h"
 #include "soc15.h"
 #include "soc15d.h"
 #include "soc15_common.h"
@@ -51,6 +52,8 @@ static int vcn_v1_0_set_powergating_state(void *handle, enum amd_powergating_sta
 static int vcn_v1_0_pause_dpg_mode(struct amdgpu_device *adev,
 				struct dpg_pause_state *new_state);
 
+static void vcn_v1_0_idle_work_handler(struct work_struct *work);
+
 /**
  * vcn_v1_0_early_init - set function pointers
  *
@@ -101,6 +104,7 @@ static int vcn_v1_0_sw_init(void *handle)
 			return r;
 	}
 
+	INIT_DELAYED_WORK(&adev->vcn.idle_work, vcn_v1_0_idle_work_handler);
 	r = amdgpu_vcn_sw_init(adev);
 	if (r)
 		return r;
@@ -1758,6 +1762,86 @@ static int vcn_v1_0_set_powergating_state(void *handle,
 	return ret;
 }
 
+static void vcn_v1_0_idle_work_handler(struct work_struct *work)
+{
+	struct amdgpu_device *adev =
+		container_of(work, struct amdgpu_device, vcn.idle_work.work);
+	unsigned int fences = 0, i;
+
+	for (i = 0; i < adev->vcn.num_enc_rings; ++i)
+		fences += amdgpu_fence_count_emitted(&adev->vcn.inst->ring_enc[i]);
+
+	if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
+		struct dpg_pause_state new_state;
+
+		if (fences)
+			new_state.fw_based = VCN_DPG_STATE__PAUSE;
+		else
+			new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
+
+		if (amdgpu_fence_count_emitted(&adev->jpeg.inst->ring_dec))
+			new_state.jpeg = VCN_DPG_STATE__PAUSE;
+		else
+			new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
+
+		adev->vcn.pause_dpg_mode(adev, &new_state);
+	}
+
+	fences += amdgpu_fence_count_emitted(&adev->jpeg.inst->ring_dec);
+	fences += amdgpu_fence_count_emitted(&adev->vcn.inst->ring_dec);
+
+	if (fences == 0) {
+		amdgpu_gfx_off_ctrl(adev, true);
+		if (adev->pm.dpm_enabled)
+			amdgpu_dpm_enable_uvd(adev, false);
+		else
+			amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
+			       AMD_PG_STATE_GATE);
+	} else {
+		schedule_delayed_work(&adev->vcn.idle_work, VCN_IDLE_TIMEOUT);
+	}
+}
+
+void vcn_v1_0_ring_begin_use(struct amdgpu_ring *ring)
+{
+	struct amdgpu_device *adev = ring->adev;
+	bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);
+
+	if (set_clocks) {
+		amdgpu_gfx_off_ctrl(adev, false);
+		if (adev->pm.dpm_enabled)
+			amdgpu_dpm_enable_uvd(adev, true);
+		else
+			amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
+			       AMD_PG_STATE_UNGATE);
+	}
+
+	if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
+		struct dpg_pause_state new_state;
+		unsigned int fences = 0, i;
+
+		for (i = 0; i < adev->vcn.num_enc_rings; ++i)
+			fences += amdgpu_fence_count_emitted(&adev->vcn.inst->ring_enc[i]);
+
+		if (fences)
+			new_state.fw_based = VCN_DPG_STATE__PAUSE;
+		else
+			new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
+
+		if (amdgpu_fence_count_emitted(&adev->jpeg.inst->ring_dec))
+			new_state.jpeg = VCN_DPG_STATE__PAUSE;
+		else
+			new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
+
+		if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC)
+			new_state.fw_based = VCN_DPG_STATE__PAUSE;
+		else if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_JPEG)
+			new_state.jpeg = VCN_DPG_STATE__PAUSE;
+
+		adev->vcn.pause_dpg_mode(adev, &new_state);
+	}
+}
+
 static const struct amd_ip_funcs vcn_v1_0_ip_funcs = {
 	.name = "vcn_v1_0",
 	.early_init = vcn_v1_0_early_init,
@@ -1804,7 +1888,7 @@ static const struct amdgpu_ring_funcs vcn_v1_0_dec_ring_vm_funcs = {
 	.insert_start = vcn_v1_0_dec_ring_insert_start,
 	.insert_end = vcn_v1_0_dec_ring_insert_end,
 	.pad_ib = amdgpu_ring_generic_pad_ib,
-	.begin_use = amdgpu_vcn_ring_begin_use,
+	.begin_use = vcn_v1_0_ring_begin_use,
 	.end_use = amdgpu_vcn_ring_end_use,
 	.emit_wreg = vcn_v1_0_dec_ring_emit_wreg,
 	.emit_reg_wait = vcn_v1_0_dec_ring_emit_reg_wait,
@@ -1836,7 +1920,7 @@ static const struct amdgpu_ring_funcs vcn_v1_0_enc_ring_vm_funcs = {
 	.insert_nop = amdgpu_ring_insert_nop,
 	.insert_end = vcn_v1_0_enc_ring_insert_end,
 	.pad_ib = amdgpu_ring_generic_pad_ib,
-	.begin_use = amdgpu_vcn_ring_begin_use,
+	.begin_use = vcn_v1_0_ring_begin_use,
 	.end_use = amdgpu_vcn_ring_end_use,
 	.emit_wreg = vcn_v1_0_enc_ring_emit_wreg,
 	.emit_reg_wait = vcn_v1_0_enc_ring_emit_reg_wait,
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h
index 2a497a7a4840..f67d7391fc21 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h
@@ -24,6 +24,8 @@
 #ifndef __VCN_V1_0_H__
 #define __VCN_V1_0_H__
 
+void vcn_v1_0_ring_begin_use(struct amdgpu_ring *ring);
+
 extern const struct amdgpu_ip_block_version vcn_v1_0_ip_block;
 
 #endif
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] drm/amdgpu/vcn: remove JPEG related code from idle handler and begin use
  2019-12-12 16:06 ` [PATCH 1/2] drm/amdgpu/vcn1.0: use its own idle handler and begin use funcs Leo Liu
@ 2019-12-12 16:06   ` Leo Liu
  2019-12-12 17:59   ` [PATCH 1/2] drm/amdgpu/vcn1.0: use its own idle handler and begin use funcs James Zhu
  2019-12-13 10:15   ` Christian König
  2 siblings, 0 replies; 12+ messages in thread
From: Leo Liu @ 2019-12-12 16:06 UTC (permalink / raw)
  To: amd-gfx; +Cc: Leo Liu

For VCN2.0 and above, VCN has been separated from JPEG

Signed-off-by: Leo Liu <leo.liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 28 +++++--------------------
 1 file changed, 5 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index e962c87d04cf..2ff04d0047ed 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -293,6 +293,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
 	for (j = 0; j < adev->vcn.num_vcn_inst; ++j) {
 		if (adev->vcn.harvest_config & (1 << j))
 			continue;
+
 		for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
 			fence[j] += amdgpu_fence_count_emitted(&adev->vcn.inst[j].ring_enc[i]);
 		}
@@ -305,26 +306,17 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
 			else
 				new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
 
-			if (amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec))
-				new_state.jpeg = VCN_DPG_STATE__PAUSE;
-			else
-				new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
-
 			adev->vcn.pause_dpg_mode(adev, &new_state);
 		}
 
-		fence[j] += amdgpu_fence_count_emitted(&adev->jpeg.inst[j].ring_dec);
 		fence[j] += amdgpu_fence_count_emitted(&adev->vcn.inst[j].ring_dec);
 		fences += fence[j];
 	}
 
 	if (fences == 0) {
 		amdgpu_gfx_off_ctrl(adev, true);
-		if (adev->asic_type < CHIP_ARCTURUS && adev->pm.dpm_enabled)
-			amdgpu_dpm_enable_uvd(adev, false);
-		else
-			amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
-							       AMD_PG_STATE_GATE);
+		amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
+		       AMD_PG_STATE_GATE);
 	} else {
 		schedule_delayed_work(&adev->vcn.idle_work, VCN_IDLE_TIMEOUT);
 	}
@@ -337,11 +329,8 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
 
 	if (set_clocks) {
 		amdgpu_gfx_off_ctrl(adev, false);
-		if (adev->asic_type < CHIP_ARCTURUS && adev->pm.dpm_enabled)
-			amdgpu_dpm_enable_uvd(adev, true);
-		else
-			amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
-							       AMD_PG_STATE_UNGATE);
+		amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
+		       AMD_PG_STATE_UNGATE);
 	}
 
 	if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG)	{
@@ -357,15 +346,8 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
 		else
 			new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
 
-		if (amdgpu_fence_count_emitted(&adev->jpeg.inst[ring->me].ring_dec))
-			new_state.jpeg = VCN_DPG_STATE__PAUSE;
-		else
-			new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
-
 		if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC)
 			new_state.fw_based = VCN_DPG_STATE__PAUSE;
-		else if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_JPEG)
-			new_state.jpeg = VCN_DPG_STATE__PAUSE;
 
 		adev->vcn.pause_dpg_mode(adev, &new_state);
 	}
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] drm/amdgpu/vcn1.0: use its own idle handler and begin use funcs
  2019-12-12 16:06 ` [PATCH 1/2] drm/amdgpu/vcn1.0: use its own idle handler and begin use funcs Leo Liu
  2019-12-12 16:06   ` [PATCH 2/2] drm/amdgpu/vcn: remove JPEG related code from idle handler and begin use Leo Liu
@ 2019-12-12 17:59   ` James Zhu
  2019-12-13 10:15   ` Christian König
  2 siblings, 0 replies; 12+ messages in thread
From: James Zhu @ 2019-12-12 17:59 UTC (permalink / raw)
  To: amd-gfx

Reviewed-by: James Zhu <James.Zhu@amd.com> for the series.

On 2019-12-12 11:06 a.m., Leo Liu wrote:
> Because VCN1.0 power management and DPG mode are managed together with
> JPEG1.0 under both HW and FW, so separated them from general VCN code.
> Also the multiple instances case got removed, since VCN1.0 HW just have
> a single instance.
>
> Signed-off-by: Leo Liu <leo.liu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c |  7 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  3 +
>   drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c  |  3 +-
>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c   | 88 ++++++++++++++++++++++++-
>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h   |  2 +
>   5 files changed, 96 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 428cfd58b37d..e962c87d04cf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -39,9 +39,6 @@
>   #include "vcn/vcn_1_0_offset.h"
>   #include "vcn/vcn_1_0_sh_mask.h"
>   
> -/* 1 second timeout */
> -#define VCN_IDLE_TIMEOUT	msecs_to_jiffies(1000)
> -
>   /* Firmware Names */
>   #define FIRMWARE_RAVEN		"amdgpu/raven_vcn.bin"
>   #define FIRMWARE_PICASSO	"amdgpu/picasso_vcn.bin"
> @@ -71,7 +68,9 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
>   	unsigned char fw_check;
>   	int i, r;
>   
> -	INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
> +	/* For VCN2.0 and above */
> +	if (adev->asic_type >= CHIP_ARCTURUS)
> +		INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
>   
>   	switch (adev->asic_type) {
>   	case CHIP_RAVEN:
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> index 402a5046b985..3484ead62046 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -56,6 +56,9 @@
>   #define VCN_VID_IP_ADDRESS_2_0		0x0
>   #define VCN_AON_IP_ADDRESS_2_0		0x30000
>   
> +/* 1 second timeout */
> +#define VCN_IDLE_TIMEOUT	msecs_to_jiffies(1000)
> +
>   #define RREG32_SOC15_DPG_MODE(ip, inst, reg, mask, sram_sel) 				\
>   	({	WREG32_SOC15(ip, inst, mmUVD_DPG_LMA_MASK, mask); 			\
>   		WREG32_SOC15(ip, inst, mmUVD_DPG_LMA_CTL, 				\
> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c
> index a141408dfb23..0debfd9f428c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c
> @@ -25,6 +25,7 @@
>   #include "amdgpu_jpeg.h"
>   #include "soc15.h"
>   #include "soc15d.h"
> +#include "vcn_v1_0.h"
>   
>   #include "vcn/vcn_1_0_offset.h"
>   #include "vcn/vcn_1_0_sh_mask.h"
> @@ -561,7 +562,7 @@ static const struct amdgpu_ring_funcs jpeg_v1_0_decode_ring_vm_funcs = {
>   	.insert_start = jpeg_v1_0_decode_ring_insert_start,
>   	.insert_end = jpeg_v1_0_decode_ring_insert_end,
>   	.pad_ib = amdgpu_ring_generic_pad_ib,
> -	.begin_use = amdgpu_vcn_ring_begin_use,
> +	.begin_use = vcn_v1_0_ring_begin_use,
>   	.end_use = amdgpu_vcn_ring_end_use,
>   	.emit_wreg = jpeg_v1_0_decode_ring_emit_wreg,
>   	.emit_reg_wait = jpeg_v1_0_decode_ring_emit_reg_wait,
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> index 652cecc030b3..7395286540e1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> @@ -25,6 +25,7 @@
>   
>   #include "amdgpu.h"
>   #include "amdgpu_vcn.h"
> +#include "amdgpu_pm.h"
>   #include "soc15.h"
>   #include "soc15d.h"
>   #include "soc15_common.h"
> @@ -51,6 +52,8 @@ static int vcn_v1_0_set_powergating_state(void *handle, enum amd_powergating_sta
>   static int vcn_v1_0_pause_dpg_mode(struct amdgpu_device *adev,
>   				struct dpg_pause_state *new_state);
>   
> +static void vcn_v1_0_idle_work_handler(struct work_struct *work);
> +
>   /**
>    * vcn_v1_0_early_init - set function pointers
>    *
> @@ -101,6 +104,7 @@ static int vcn_v1_0_sw_init(void *handle)
>   			return r;
>   	}
>   
> +	INIT_DELAYED_WORK(&adev->vcn.idle_work, vcn_v1_0_idle_work_handler);
>   	r = amdgpu_vcn_sw_init(adev);
>   	if (r)
>   		return r;
> @@ -1758,6 +1762,86 @@ static int vcn_v1_0_set_powergating_state(void *handle,
>   	return ret;
>   }
>   
> +static void vcn_v1_0_idle_work_handler(struct work_struct *work)
> +{
> +	struct amdgpu_device *adev =
> +		container_of(work, struct amdgpu_device, vcn.idle_work.work);
> +	unsigned int fences = 0, i;
> +
> +	for (i = 0; i < adev->vcn.num_enc_rings; ++i)
> +		fences += amdgpu_fence_count_emitted(&adev->vcn.inst->ring_enc[i]);
> +
> +	if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
> +		struct dpg_pause_state new_state;
> +
> +		if (fences)
> +			new_state.fw_based = VCN_DPG_STATE__PAUSE;
> +		else
> +			new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
> +
> +		if (amdgpu_fence_count_emitted(&adev->jpeg.inst->ring_dec))
> +			new_state.jpeg = VCN_DPG_STATE__PAUSE;
> +		else
> +			new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
> +
> +		adev->vcn.pause_dpg_mode(adev, &new_state);
> +	}
> +
> +	fences += amdgpu_fence_count_emitted(&adev->jpeg.inst->ring_dec);
> +	fences += amdgpu_fence_count_emitted(&adev->vcn.inst->ring_dec);
> +
> +	if (fences == 0) {
> +		amdgpu_gfx_off_ctrl(adev, true);
> +		if (adev->pm.dpm_enabled)
> +			amdgpu_dpm_enable_uvd(adev, false);
> +		else
> +			amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> +			       AMD_PG_STATE_GATE);
> +	} else {
> +		schedule_delayed_work(&adev->vcn.idle_work, VCN_IDLE_TIMEOUT);
> +	}
> +}
> +
> +void vcn_v1_0_ring_begin_use(struct amdgpu_ring *ring)
> +{
> +	struct amdgpu_device *adev = ring->adev;
> +	bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);
> +
> +	if (set_clocks) {
> +		amdgpu_gfx_off_ctrl(adev, false);
> +		if (adev->pm.dpm_enabled)
> +			amdgpu_dpm_enable_uvd(adev, true);
> +		else
> +			amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> +			       AMD_PG_STATE_UNGATE);
> +	}
> +
> +	if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
> +		struct dpg_pause_state new_state;
> +		unsigned int fences = 0, i;
> +
> +		for (i = 0; i < adev->vcn.num_enc_rings; ++i)
> +			fences += amdgpu_fence_count_emitted(&adev->vcn.inst->ring_enc[i]);
> +
> +		if (fences)
> +			new_state.fw_based = VCN_DPG_STATE__PAUSE;
> +		else
> +			new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
> +
> +		if (amdgpu_fence_count_emitted(&adev->jpeg.inst->ring_dec))
> +			new_state.jpeg = VCN_DPG_STATE__PAUSE;
> +		else
> +			new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
> +
> +		if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC)
> +			new_state.fw_based = VCN_DPG_STATE__PAUSE;
> +		else if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_JPEG)
> +			new_state.jpeg = VCN_DPG_STATE__PAUSE;
> +
> +		adev->vcn.pause_dpg_mode(adev, &new_state);
> +	}
> +}
> +
>   static const struct amd_ip_funcs vcn_v1_0_ip_funcs = {
>   	.name = "vcn_v1_0",
>   	.early_init = vcn_v1_0_early_init,
> @@ -1804,7 +1888,7 @@ static const struct amdgpu_ring_funcs vcn_v1_0_dec_ring_vm_funcs = {
>   	.insert_start = vcn_v1_0_dec_ring_insert_start,
>   	.insert_end = vcn_v1_0_dec_ring_insert_end,
>   	.pad_ib = amdgpu_ring_generic_pad_ib,
> -	.begin_use = amdgpu_vcn_ring_begin_use,
> +	.begin_use = vcn_v1_0_ring_begin_use,
>   	.end_use = amdgpu_vcn_ring_end_use,
>   	.emit_wreg = vcn_v1_0_dec_ring_emit_wreg,
>   	.emit_reg_wait = vcn_v1_0_dec_ring_emit_reg_wait,
> @@ -1836,7 +1920,7 @@ static const struct amdgpu_ring_funcs vcn_v1_0_enc_ring_vm_funcs = {
>   	.insert_nop = amdgpu_ring_insert_nop,
>   	.insert_end = vcn_v1_0_enc_ring_insert_end,
>   	.pad_ib = amdgpu_ring_generic_pad_ib,
> -	.begin_use = amdgpu_vcn_ring_begin_use,
> +	.begin_use = vcn_v1_0_ring_begin_use,
>   	.end_use = amdgpu_vcn_ring_end_use,
>   	.emit_wreg = vcn_v1_0_enc_ring_emit_wreg,
>   	.emit_reg_wait = vcn_v1_0_enc_ring_emit_reg_wait,
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h
> index 2a497a7a4840..f67d7391fc21 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h
> @@ -24,6 +24,8 @@
>   #ifndef __VCN_V1_0_H__
>   #define __VCN_V1_0_H__
>   
> +void vcn_v1_0_ring_begin_use(struct amdgpu_ring *ring);
> +
>   extern const struct amdgpu_ip_block_version vcn_v1_0_ip_block;
>   
>   #endif
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] drm/amdgpu/vcn1.0: use its own idle handler and begin use funcs
  2019-12-12 16:06 ` [PATCH 1/2] drm/amdgpu/vcn1.0: use its own idle handler and begin use funcs Leo Liu
  2019-12-12 16:06   ` [PATCH 2/2] drm/amdgpu/vcn: remove JPEG related code from idle handler and begin use Leo Liu
  2019-12-12 17:59   ` [PATCH 1/2] drm/amdgpu/vcn1.0: use its own idle handler and begin use funcs James Zhu
@ 2019-12-13 10:15   ` Christian König
  2 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2019-12-13 10:15 UTC (permalink / raw)
  To: Leo Liu, amd-gfx

Am 12.12.19 um 17:06 schrieb Leo Liu:
> Because VCN1.0 power management and DPG mode are managed together with
> JPEG1.0 under both HW and FW, so separated them from general VCN code.
> Also the multiple instances case got removed, since VCN1.0 HW just have
> a single instance.
>
> Signed-off-by: Leo Liu <leo.liu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c |  7 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  3 +
>   drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c  |  3 +-
>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c   | 88 ++++++++++++++++++++++++-
>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h   |  2 +
>   5 files changed, 96 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 428cfd58b37d..e962c87d04cf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -39,9 +39,6 @@
>   #include "vcn/vcn_1_0_offset.h"
>   #include "vcn/vcn_1_0_sh_mask.h"
>   
> -/* 1 second timeout */
> -#define VCN_IDLE_TIMEOUT	msecs_to_jiffies(1000)
> -
>   /* Firmware Names */
>   #define FIRMWARE_RAVEN		"amdgpu/raven_vcn.bin"
>   #define FIRMWARE_PICASSO	"amdgpu/picasso_vcn.bin"
> @@ -71,7 +68,9 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
>   	unsigned char fw_check;
>   	int i, r;
>   
> -	INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
> +	/* For VCN2.0 and above */
> +	if (adev->asic_type >= CHIP_ARCTURUS)
> +		INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);

Initializing the work structure twice is indeed not a good idea, but I 
think we could just override the work function in vcn_v1_0_sw_init() 
after calling vcn_v1_0_sw_init().

Apart from that the series looks good to me,
Christian.

>   
>   	switch (adev->asic_type) {
>   	case CHIP_RAVEN:
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> index 402a5046b985..3484ead62046 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -56,6 +56,9 @@
>   #define VCN_VID_IP_ADDRESS_2_0		0x0
>   #define VCN_AON_IP_ADDRESS_2_0		0x30000
>   
> +/* 1 second timeout */
> +#define VCN_IDLE_TIMEOUT	msecs_to_jiffies(1000)
> +
>   #define RREG32_SOC15_DPG_MODE(ip, inst, reg, mask, sram_sel) 				\
>   	({	WREG32_SOC15(ip, inst, mmUVD_DPG_LMA_MASK, mask); 			\
>   		WREG32_SOC15(ip, inst, mmUVD_DPG_LMA_CTL, 				\
> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c
> index a141408dfb23..0debfd9f428c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c
> @@ -25,6 +25,7 @@
>   #include "amdgpu_jpeg.h"
>   #include "soc15.h"
>   #include "soc15d.h"
> +#include "vcn_v1_0.h"
>   
>   #include "vcn/vcn_1_0_offset.h"
>   #include "vcn/vcn_1_0_sh_mask.h"
> @@ -561,7 +562,7 @@ static const struct amdgpu_ring_funcs jpeg_v1_0_decode_ring_vm_funcs = {
>   	.insert_start = jpeg_v1_0_decode_ring_insert_start,
>   	.insert_end = jpeg_v1_0_decode_ring_insert_end,
>   	.pad_ib = amdgpu_ring_generic_pad_ib,
> -	.begin_use = amdgpu_vcn_ring_begin_use,
> +	.begin_use = vcn_v1_0_ring_begin_use,
>   	.end_use = amdgpu_vcn_ring_end_use,
>   	.emit_wreg = jpeg_v1_0_decode_ring_emit_wreg,
>   	.emit_reg_wait = jpeg_v1_0_decode_ring_emit_reg_wait,
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> index 652cecc030b3..7395286540e1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> @@ -25,6 +25,7 @@
>   
>   #include "amdgpu.h"
>   #include "amdgpu_vcn.h"
> +#include "amdgpu_pm.h"
>   #include "soc15.h"
>   #include "soc15d.h"
>   #include "soc15_common.h"
> @@ -51,6 +52,8 @@ static int vcn_v1_0_set_powergating_state(void *handle, enum amd_powergating_sta
>   static int vcn_v1_0_pause_dpg_mode(struct amdgpu_device *adev,
>   				struct dpg_pause_state *new_state);
>   
> +static void vcn_v1_0_idle_work_handler(struct work_struct *work);
> +
>   /**
>    * vcn_v1_0_early_init - set function pointers
>    *
> @@ -101,6 +104,7 @@ static int vcn_v1_0_sw_init(void *handle)
>   			return r;
>   	}
>   
> +	INIT_DELAYED_WORK(&adev->vcn.idle_work, vcn_v1_0_idle_work_handler);
>   	r = amdgpu_vcn_sw_init(adev);
>   	if (r)
>   		return r;
> @@ -1758,6 +1762,86 @@ static int vcn_v1_0_set_powergating_state(void *handle,
>   	return ret;
>   }
>   
> +static void vcn_v1_0_idle_work_handler(struct work_struct *work)
> +{
> +	struct amdgpu_device *adev =
> +		container_of(work, struct amdgpu_device, vcn.idle_work.work);
> +	unsigned int fences = 0, i;
> +
> +	for (i = 0; i < adev->vcn.num_enc_rings; ++i)
> +		fences += amdgpu_fence_count_emitted(&adev->vcn.inst->ring_enc[i]);
> +
> +	if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
> +		struct dpg_pause_state new_state;
> +
> +		if (fences)
> +			new_state.fw_based = VCN_DPG_STATE__PAUSE;
> +		else
> +			new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
> +
> +		if (amdgpu_fence_count_emitted(&adev->jpeg.inst->ring_dec))
> +			new_state.jpeg = VCN_DPG_STATE__PAUSE;
> +		else
> +			new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
> +
> +		adev->vcn.pause_dpg_mode(adev, &new_state);
> +	}
> +
> +	fences += amdgpu_fence_count_emitted(&adev->jpeg.inst->ring_dec);
> +	fences += amdgpu_fence_count_emitted(&adev->vcn.inst->ring_dec);
> +
> +	if (fences == 0) {
> +		amdgpu_gfx_off_ctrl(adev, true);
> +		if (adev->pm.dpm_enabled)
> +			amdgpu_dpm_enable_uvd(adev, false);
> +		else
> +			amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> +			       AMD_PG_STATE_GATE);
> +	} else {
> +		schedule_delayed_work(&adev->vcn.idle_work, VCN_IDLE_TIMEOUT);
> +	}
> +}
> +
> +void vcn_v1_0_ring_begin_use(struct amdgpu_ring *ring)
> +{
> +	struct amdgpu_device *adev = ring->adev;
> +	bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);
> +
> +	if (set_clocks) {
> +		amdgpu_gfx_off_ctrl(adev, false);
> +		if (adev->pm.dpm_enabled)
> +			amdgpu_dpm_enable_uvd(adev, true);
> +		else
> +			amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> +			       AMD_PG_STATE_UNGATE);
> +	}
> +
> +	if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
> +		struct dpg_pause_state new_state;
> +		unsigned int fences = 0, i;
> +
> +		for (i = 0; i < adev->vcn.num_enc_rings; ++i)
> +			fences += amdgpu_fence_count_emitted(&adev->vcn.inst->ring_enc[i]);
> +
> +		if (fences)
> +			new_state.fw_based = VCN_DPG_STATE__PAUSE;
> +		else
> +			new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
> +
> +		if (amdgpu_fence_count_emitted(&adev->jpeg.inst->ring_dec))
> +			new_state.jpeg = VCN_DPG_STATE__PAUSE;
> +		else
> +			new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
> +
> +		if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC)
> +			new_state.fw_based = VCN_DPG_STATE__PAUSE;
> +		else if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_JPEG)
> +			new_state.jpeg = VCN_DPG_STATE__PAUSE;
> +
> +		adev->vcn.pause_dpg_mode(adev, &new_state);
> +	}
> +}
> +
>   static const struct amd_ip_funcs vcn_v1_0_ip_funcs = {
>   	.name = "vcn_v1_0",
>   	.early_init = vcn_v1_0_early_init,
> @@ -1804,7 +1888,7 @@ static const struct amdgpu_ring_funcs vcn_v1_0_dec_ring_vm_funcs = {
>   	.insert_start = vcn_v1_0_dec_ring_insert_start,
>   	.insert_end = vcn_v1_0_dec_ring_insert_end,
>   	.pad_ib = amdgpu_ring_generic_pad_ib,
> -	.begin_use = amdgpu_vcn_ring_begin_use,
> +	.begin_use = vcn_v1_0_ring_begin_use,
>   	.end_use = amdgpu_vcn_ring_end_use,
>   	.emit_wreg = vcn_v1_0_dec_ring_emit_wreg,
>   	.emit_reg_wait = vcn_v1_0_dec_ring_emit_reg_wait,
> @@ -1836,7 +1920,7 @@ static const struct amdgpu_ring_funcs vcn_v1_0_enc_ring_vm_funcs = {
>   	.insert_nop = amdgpu_ring_insert_nop,
>   	.insert_end = vcn_v1_0_enc_ring_insert_end,
>   	.pad_ib = amdgpu_ring_generic_pad_ib,
> -	.begin_use = amdgpu_vcn_ring_begin_use,
> +	.begin_use = vcn_v1_0_ring_begin_use,
>   	.end_use = amdgpu_vcn_ring_end_use,
>   	.emit_wreg = vcn_v1_0_enc_ring_emit_wreg,
>   	.emit_reg_wait = vcn_v1_0_enc_ring_emit_reg_wait,
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h
> index 2a497a7a4840..f67d7391fc21 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h
> @@ -24,6 +24,8 @@
>   #ifndef __VCN_V1_0_H__
>   #define __VCN_V1_0_H__
>   
> +void vcn_v1_0_ring_begin_use(struct amdgpu_ring *ring);
> +
>   extern const struct amdgpu_ip_block_version vcn_v1_0_ip_block;
>   
>   #endif

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] drm/amdgpu/vcn1.0: use its own idle handler and begin use funcs
  2019-12-11 19:48 [PATCH] drm/amdgpu: add JPEG check to VCN idle handler and begin use Leo Liu
                   ` (3 preceding siblings ...)
  2019-12-12 16:06 ` [PATCH 1/2] drm/amdgpu/vcn1.0: use its own idle handler and begin use funcs Leo Liu
@ 2019-12-13 14:42 ` Leo Liu
  2019-12-13 15:30   ` Christian König
  4 siblings, 1 reply; 12+ messages in thread
From: Leo Liu @ 2019-12-13 14:42 UTC (permalink / raw)
  To: amd-gfx; +Cc: Leo Liu

Because VCN1.0 power management and DPG mode are managed together with
JPEG1.0 under both HW and FW, so separated them from general VCN code.
Also the multiple instances case got removed, since VCN1.0 HW just have
a single instance.

v2: override work func with vcn1.0's own

Signed-off-by: Leo Liu <leo.liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c |  3 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  3 +
 drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c  |  3 +-
 drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c   | 90 ++++++++++++++++++++++++-
 drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h   |  2 +
 5 files changed, 95 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 428cfd58b37d..717f0a218c5d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -39,9 +39,6 @@
 #include "vcn/vcn_1_0_offset.h"
 #include "vcn/vcn_1_0_sh_mask.h"
 
-/* 1 second timeout */
-#define VCN_IDLE_TIMEOUT	msecs_to_jiffies(1000)
-
 /* Firmware Names */
 #define FIRMWARE_RAVEN		"amdgpu/raven_vcn.bin"
 #define FIRMWARE_PICASSO	"amdgpu/picasso_vcn.bin"
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index 402a5046b985..3484ead62046 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -56,6 +56,9 @@
 #define VCN_VID_IP_ADDRESS_2_0		0x0
 #define VCN_AON_IP_ADDRESS_2_0		0x30000
 
+/* 1 second timeout */
+#define VCN_IDLE_TIMEOUT	msecs_to_jiffies(1000)
+
 #define RREG32_SOC15_DPG_MODE(ip, inst, reg, mask, sram_sel) 				\
 	({	WREG32_SOC15(ip, inst, mmUVD_DPG_LMA_MASK, mask); 			\
 		WREG32_SOC15(ip, inst, mmUVD_DPG_LMA_CTL, 				\
diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c
index a141408dfb23..0debfd9f428c 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c
@@ -25,6 +25,7 @@
 #include "amdgpu_jpeg.h"
 #include "soc15.h"
 #include "soc15d.h"
+#include "vcn_v1_0.h"
 
 #include "vcn/vcn_1_0_offset.h"
 #include "vcn/vcn_1_0_sh_mask.h"
@@ -561,7 +562,7 @@ static const struct amdgpu_ring_funcs jpeg_v1_0_decode_ring_vm_funcs = {
 	.insert_start = jpeg_v1_0_decode_ring_insert_start,
 	.insert_end = jpeg_v1_0_decode_ring_insert_end,
 	.pad_ib = amdgpu_ring_generic_pad_ib,
-	.begin_use = amdgpu_vcn_ring_begin_use,
+	.begin_use = vcn_v1_0_ring_begin_use,
 	.end_use = amdgpu_vcn_ring_end_use,
 	.emit_wreg = jpeg_v1_0_decode_ring_emit_wreg,
 	.emit_reg_wait = jpeg_v1_0_decode_ring_emit_reg_wait,
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
index 652cecc030b3..3b025a3f8c7d 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
@@ -25,6 +25,7 @@
 
 #include "amdgpu.h"
 #include "amdgpu_vcn.h"
+#include "amdgpu_pm.h"
 #include "soc15.h"
 #include "soc15d.h"
 #include "soc15_common.h"
@@ -51,6 +52,8 @@ static int vcn_v1_0_set_powergating_state(void *handle, enum amd_powergating_sta
 static int vcn_v1_0_pause_dpg_mode(struct amdgpu_device *adev,
 				struct dpg_pause_state *new_state);
 
+static void vcn_v1_0_idle_work_handler(struct work_struct *work);
+
 /**
  * vcn_v1_0_early_init - set function pointers
  *
@@ -105,6 +108,9 @@ static int vcn_v1_0_sw_init(void *handle)
 	if (r)
 		return r;
 
+	/* Override the work func */
+	adev->vcn.idle_work.work.func = vcn_v1_0_idle_work_handler;
+
 	if (adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) {
 		const struct common_firmware_header *hdr;
 		hdr = (const struct common_firmware_header *)adev->vcn.fw->data;
@@ -1758,6 +1764,86 @@ static int vcn_v1_0_set_powergating_state(void *handle,
 	return ret;
 }
 
+static void vcn_v1_0_idle_work_handler(struct work_struct *work)
+{
+	struct amdgpu_device *adev =
+		container_of(work, struct amdgpu_device, vcn.idle_work.work);
+	unsigned int fences = 0, i;
+
+	for (i = 0; i < adev->vcn.num_enc_rings; ++i)
+		fences += amdgpu_fence_count_emitted(&adev->vcn.inst->ring_enc[i]);
+
+	if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
+		struct dpg_pause_state new_state;
+
+		if (fences)
+			new_state.fw_based = VCN_DPG_STATE__PAUSE;
+		else
+			new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
+
+		if (amdgpu_fence_count_emitted(&adev->jpeg.inst->ring_dec))
+			new_state.jpeg = VCN_DPG_STATE__PAUSE;
+		else
+			new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
+
+		adev->vcn.pause_dpg_mode(adev, &new_state);
+	}
+
+	fences += amdgpu_fence_count_emitted(&adev->jpeg.inst->ring_dec);
+	fences += amdgpu_fence_count_emitted(&adev->vcn.inst->ring_dec);
+
+	if (fences == 0) {
+		amdgpu_gfx_off_ctrl(adev, true);
+		if (adev->pm.dpm_enabled)
+			amdgpu_dpm_enable_uvd(adev, false);
+		else
+			amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
+			       AMD_PG_STATE_GATE);
+	} else {
+		schedule_delayed_work(&adev->vcn.idle_work, VCN_IDLE_TIMEOUT);
+	}
+}
+
+void vcn_v1_0_ring_begin_use(struct amdgpu_ring *ring)
+{
+	struct amdgpu_device *adev = ring->adev;
+	bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);
+
+	if (set_clocks) {
+		amdgpu_gfx_off_ctrl(adev, false);
+		if (adev->pm.dpm_enabled)
+			amdgpu_dpm_enable_uvd(adev, true);
+		else
+			amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
+			       AMD_PG_STATE_UNGATE);
+	}
+
+	if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
+		struct dpg_pause_state new_state;
+		unsigned int fences = 0, i;
+
+		for (i = 0; i < adev->vcn.num_enc_rings; ++i)
+			fences += amdgpu_fence_count_emitted(&adev->vcn.inst->ring_enc[i]);
+
+		if (fences)
+			new_state.fw_based = VCN_DPG_STATE__PAUSE;
+		else
+			new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
+
+		if (amdgpu_fence_count_emitted(&adev->jpeg.inst->ring_dec))
+			new_state.jpeg = VCN_DPG_STATE__PAUSE;
+		else
+			new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
+
+		if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC)
+			new_state.fw_based = VCN_DPG_STATE__PAUSE;
+		else if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_JPEG)
+			new_state.jpeg = VCN_DPG_STATE__PAUSE;
+
+		adev->vcn.pause_dpg_mode(adev, &new_state);
+	}
+}
+
 static const struct amd_ip_funcs vcn_v1_0_ip_funcs = {
 	.name = "vcn_v1_0",
 	.early_init = vcn_v1_0_early_init,
@@ -1804,7 +1890,7 @@ static const struct amdgpu_ring_funcs vcn_v1_0_dec_ring_vm_funcs = {
 	.insert_start = vcn_v1_0_dec_ring_insert_start,
 	.insert_end = vcn_v1_0_dec_ring_insert_end,
 	.pad_ib = amdgpu_ring_generic_pad_ib,
-	.begin_use = amdgpu_vcn_ring_begin_use,
+	.begin_use = vcn_v1_0_ring_begin_use,
 	.end_use = amdgpu_vcn_ring_end_use,
 	.emit_wreg = vcn_v1_0_dec_ring_emit_wreg,
 	.emit_reg_wait = vcn_v1_0_dec_ring_emit_reg_wait,
@@ -1836,7 +1922,7 @@ static const struct amdgpu_ring_funcs vcn_v1_0_enc_ring_vm_funcs = {
 	.insert_nop = amdgpu_ring_insert_nop,
 	.insert_end = vcn_v1_0_enc_ring_insert_end,
 	.pad_ib = amdgpu_ring_generic_pad_ib,
-	.begin_use = amdgpu_vcn_ring_begin_use,
+	.begin_use = vcn_v1_0_ring_begin_use,
 	.end_use = amdgpu_vcn_ring_end_use,
 	.emit_wreg = vcn_v1_0_enc_ring_emit_wreg,
 	.emit_reg_wait = vcn_v1_0_enc_ring_emit_reg_wait,
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h
index 2a497a7a4840..f67d7391fc21 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h
@@ -24,6 +24,8 @@
 #ifndef __VCN_V1_0_H__
 #define __VCN_V1_0_H__
 
+void vcn_v1_0_ring_begin_use(struct amdgpu_ring *ring);
+
 extern const struct amdgpu_ip_block_version vcn_v1_0_ip_block;
 
 #endif
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] drm/amdgpu/vcn1.0: use its own idle handler and begin use funcs
  2019-12-13 14:42 ` Leo Liu
@ 2019-12-13 15:30   ` Christian König
  0 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2019-12-13 15:30 UTC (permalink / raw)
  To: Leo Liu, amd-gfx

Am 13.12.19 um 15:42 schrieb Leo Liu:
> Because VCN1.0 power management and DPG mode are managed together with
> JPEG1.0 under both HW and FW, so separated them from general VCN code.
> Also the multiple instances case got removed, since VCN1.0 HW just have
> a single instance.
>
> v2: override work func with vcn1.0's own
>
> Signed-off-by: Leo Liu <leo.liu@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c |  3 -
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  3 +
>   drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c  |  3 +-
>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c   | 90 ++++++++++++++++++++++++-
>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h   |  2 +
>   5 files changed, 95 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 428cfd58b37d..717f0a218c5d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -39,9 +39,6 @@
>   #include "vcn/vcn_1_0_offset.h"
>   #include "vcn/vcn_1_0_sh_mask.h"
>   
> -/* 1 second timeout */
> -#define VCN_IDLE_TIMEOUT	msecs_to_jiffies(1000)
> -
>   /* Firmware Names */
>   #define FIRMWARE_RAVEN		"amdgpu/raven_vcn.bin"
>   #define FIRMWARE_PICASSO	"amdgpu/picasso_vcn.bin"
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> index 402a5046b985..3484ead62046 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -56,6 +56,9 @@
>   #define VCN_VID_IP_ADDRESS_2_0		0x0
>   #define VCN_AON_IP_ADDRESS_2_0		0x30000
>   
> +/* 1 second timeout */
> +#define VCN_IDLE_TIMEOUT	msecs_to_jiffies(1000)
> +
>   #define RREG32_SOC15_DPG_MODE(ip, inst, reg, mask, sram_sel) 				\
>   	({	WREG32_SOC15(ip, inst, mmUVD_DPG_LMA_MASK, mask); 			\
>   		WREG32_SOC15(ip, inst, mmUVD_DPG_LMA_CTL, 				\
> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c
> index a141408dfb23..0debfd9f428c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c
> @@ -25,6 +25,7 @@
>   #include "amdgpu_jpeg.h"
>   #include "soc15.h"
>   #include "soc15d.h"
> +#include "vcn_v1_0.h"
>   
>   #include "vcn/vcn_1_0_offset.h"
>   #include "vcn/vcn_1_0_sh_mask.h"
> @@ -561,7 +562,7 @@ static const struct amdgpu_ring_funcs jpeg_v1_0_decode_ring_vm_funcs = {
>   	.insert_start = jpeg_v1_0_decode_ring_insert_start,
>   	.insert_end = jpeg_v1_0_decode_ring_insert_end,
>   	.pad_ib = amdgpu_ring_generic_pad_ib,
> -	.begin_use = amdgpu_vcn_ring_begin_use,
> +	.begin_use = vcn_v1_0_ring_begin_use,
>   	.end_use = amdgpu_vcn_ring_end_use,
>   	.emit_wreg = jpeg_v1_0_decode_ring_emit_wreg,
>   	.emit_reg_wait = jpeg_v1_0_decode_ring_emit_reg_wait,
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> index 652cecc030b3..3b025a3f8c7d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> @@ -25,6 +25,7 @@
>   
>   #include "amdgpu.h"
>   #include "amdgpu_vcn.h"
> +#include "amdgpu_pm.h"
>   #include "soc15.h"
>   #include "soc15d.h"
>   #include "soc15_common.h"
> @@ -51,6 +52,8 @@ static int vcn_v1_0_set_powergating_state(void *handle, enum amd_powergating_sta
>   static int vcn_v1_0_pause_dpg_mode(struct amdgpu_device *adev,
>   				struct dpg_pause_state *new_state);
>   
> +static void vcn_v1_0_idle_work_handler(struct work_struct *work);
> +
>   /**
>    * vcn_v1_0_early_init - set function pointers
>    *
> @@ -105,6 +108,9 @@ static int vcn_v1_0_sw_init(void *handle)
>   	if (r)
>   		return r;
>   
> +	/* Override the work func */
> +	adev->vcn.idle_work.work.func = vcn_v1_0_idle_work_handler;
> +
>   	if (adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) {
>   		const struct common_firmware_header *hdr;
>   		hdr = (const struct common_firmware_header *)adev->vcn.fw->data;
> @@ -1758,6 +1764,86 @@ static int vcn_v1_0_set_powergating_state(void *handle,
>   	return ret;
>   }
>   
> +static void vcn_v1_0_idle_work_handler(struct work_struct *work)
> +{
> +	struct amdgpu_device *adev =
> +		container_of(work, struct amdgpu_device, vcn.idle_work.work);
> +	unsigned int fences = 0, i;
> +
> +	for (i = 0; i < adev->vcn.num_enc_rings; ++i)
> +		fences += amdgpu_fence_count_emitted(&adev->vcn.inst->ring_enc[i]);
> +
> +	if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
> +		struct dpg_pause_state new_state;
> +
> +		if (fences)
> +			new_state.fw_based = VCN_DPG_STATE__PAUSE;
> +		else
> +			new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
> +
> +		if (amdgpu_fence_count_emitted(&adev->jpeg.inst->ring_dec))
> +			new_state.jpeg = VCN_DPG_STATE__PAUSE;
> +		else
> +			new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
> +
> +		adev->vcn.pause_dpg_mode(adev, &new_state);
> +	}
> +
> +	fences += amdgpu_fence_count_emitted(&adev->jpeg.inst->ring_dec);
> +	fences += amdgpu_fence_count_emitted(&adev->vcn.inst->ring_dec);
> +
> +	if (fences == 0) {
> +		amdgpu_gfx_off_ctrl(adev, true);
> +		if (adev->pm.dpm_enabled)
> +			amdgpu_dpm_enable_uvd(adev, false);
> +		else
> +			amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> +			       AMD_PG_STATE_GATE);
> +	} else {
> +		schedule_delayed_work(&adev->vcn.idle_work, VCN_IDLE_TIMEOUT);
> +	}
> +}
> +
> +void vcn_v1_0_ring_begin_use(struct amdgpu_ring *ring)
> +{
> +	struct amdgpu_device *adev = ring->adev;
> +	bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);
> +
> +	if (set_clocks) {
> +		amdgpu_gfx_off_ctrl(adev, false);
> +		if (adev->pm.dpm_enabled)
> +			amdgpu_dpm_enable_uvd(adev, true);
> +		else
> +			amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> +			       AMD_PG_STATE_UNGATE);
> +	}
> +
> +	if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
> +		struct dpg_pause_state new_state;
> +		unsigned int fences = 0, i;
> +
> +		for (i = 0; i < adev->vcn.num_enc_rings; ++i)
> +			fences += amdgpu_fence_count_emitted(&adev->vcn.inst->ring_enc[i]);
> +
> +		if (fences)
> +			new_state.fw_based = VCN_DPG_STATE__PAUSE;
> +		else
> +			new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
> +
> +		if (amdgpu_fence_count_emitted(&adev->jpeg.inst->ring_dec))
> +			new_state.jpeg = VCN_DPG_STATE__PAUSE;
> +		else
> +			new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
> +
> +		if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC)
> +			new_state.fw_based = VCN_DPG_STATE__PAUSE;
> +		else if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_JPEG)
> +			new_state.jpeg = VCN_DPG_STATE__PAUSE;
> +
> +		adev->vcn.pause_dpg_mode(adev, &new_state);
> +	}
> +}
> +
>   static const struct amd_ip_funcs vcn_v1_0_ip_funcs = {
>   	.name = "vcn_v1_0",
>   	.early_init = vcn_v1_0_early_init,
> @@ -1804,7 +1890,7 @@ static const struct amdgpu_ring_funcs vcn_v1_0_dec_ring_vm_funcs = {
>   	.insert_start = vcn_v1_0_dec_ring_insert_start,
>   	.insert_end = vcn_v1_0_dec_ring_insert_end,
>   	.pad_ib = amdgpu_ring_generic_pad_ib,
> -	.begin_use = amdgpu_vcn_ring_begin_use,
> +	.begin_use = vcn_v1_0_ring_begin_use,
>   	.end_use = amdgpu_vcn_ring_end_use,
>   	.emit_wreg = vcn_v1_0_dec_ring_emit_wreg,
>   	.emit_reg_wait = vcn_v1_0_dec_ring_emit_reg_wait,
> @@ -1836,7 +1922,7 @@ static const struct amdgpu_ring_funcs vcn_v1_0_enc_ring_vm_funcs = {
>   	.insert_nop = amdgpu_ring_insert_nop,
>   	.insert_end = vcn_v1_0_enc_ring_insert_end,
>   	.pad_ib = amdgpu_ring_generic_pad_ib,
> -	.begin_use = amdgpu_vcn_ring_begin_use,
> +	.begin_use = vcn_v1_0_ring_begin_use,
>   	.end_use = amdgpu_vcn_ring_end_use,
>   	.emit_wreg = vcn_v1_0_enc_ring_emit_wreg,
>   	.emit_reg_wait = vcn_v1_0_enc_ring_emit_reg_wait,
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h
> index 2a497a7a4840..f67d7391fc21 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h
> @@ -24,6 +24,8 @@
>   #ifndef __VCN_V1_0_H__
>   #define __VCN_V1_0_H__
>   
> +void vcn_v1_0_ring_begin_use(struct amdgpu_ring *ring);
> +
>   extern const struct amdgpu_ip_block_version vcn_v1_0_ip_block;
>   
>   #endif

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-12-13 21:34 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-11 19:48 [PATCH] drm/amdgpu: add JPEG check to VCN idle handler and begin use Leo Liu
2019-12-11 19:52 ` Alex Deucher
2019-12-11 20:01 ` Zhang, Boyuan
2019-12-12  8:18 ` Christian König
2019-12-12 15:57   ` Leo Liu
2019-12-12 15:58     ` Christian König
2019-12-12 16:06 ` [PATCH 1/2] drm/amdgpu/vcn1.0: use its own idle handler and begin use funcs Leo Liu
2019-12-12 16:06   ` [PATCH 2/2] drm/amdgpu/vcn: remove JPEG related code from idle handler and begin use Leo Liu
2019-12-12 17:59   ` [PATCH 1/2] drm/amdgpu/vcn1.0: use its own idle handler and begin use funcs James Zhu
2019-12-13 10:15   ` Christian König
2019-12-13 14:42 ` Leo Liu
2019-12-13 15:30   ` Christian König

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.