[PATCH 1/2] drm/amd/amdgpu: enable clockgating only after late init

[PATCH 1/2] drm/amd/amdgpu: enable clockgating only after late init

[Alex Deucher]

From: Arindam Nath <arindam.nath@amd.com>

Sometimes during multiple reboots, the system hangs
during bootup. The issue is very random and happens
once in around 50 reboots or so.

It seems if clockgating is enabled before late init,
the GFX engine sometimes does not respond.

This patch changes the ordering a little so that
both powergating and clockgating are enabled only
after late init calls.

Signed-off-by: Arindam Nath <arindam.nath@amd.com>
Tested-by: Sunil Uttarwar <Sunil.Uttarwar1@amd.com>
Reviewed-by: Tom St Denis <tom.stdenis@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 575a84b..e43779f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1437,13 +1437,6 @@ static int amdgpu_late_init(struct amdgpu_device *adev)
 		if (adev->ip_blocks[i].type == AMD_IP_BLOCK_TYPE_UVD ||
 			adev->ip_blocks[i].type == AMD_IP_BLOCK_TYPE_VCE)
 			continue;
-		/* enable clockgating to save power */
-		r = adev->ip_blocks[i].funcs->set_clockgating_state((void *)adev,
-								    AMD_CG_STATE_GATE);
-		if (r) {
-			DRM_ERROR("set_clockgating_state(gate) of IP block <%s> failed %d\n", adev->ip_blocks[i].funcs->name, r);
-			return r;
-		}
 		if (adev->ip_blocks[i].funcs->late_init) {
 			r = adev->ip_blocks[i].funcs->late_init((void *)adev);
 			if (r) {
@@ -1452,6 +1445,13 @@ static int amdgpu_late_init(struct amdgpu_device *adev)
 			}
 			adev->ip_block_status[i].late_initialized = true;
 		}
+		/* enable clockgating to save power */
+		r = adev->ip_blocks[i].funcs->set_clockgating_state((void *)adev,
+								    AMD_CG_STATE_GATE);
+		if (r) {
+			DRM_ERROR("set_clockgating_state(gate) of IP block <%s> failed %d\n", adev->ip_blocks[i].funcs->name, r);
+			return r;
+		}
 	}
 
 	return 0;
-- 
2.5.5

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

[PATCH 2/2] drm/amdgpu: clarify UVD/VCE special handling for CG

[Alex Deucher]

UVD and VCE CG are handled specially, however the previous
fix for this skipped late init for those blocks rather than
just CG.  Just protect the CG function call.  No functional
change since UVD and VCE don't currently utilize a late_init
function.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index e43779f..bfca676 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1434,9 +1434,6 @@ static int amdgpu_late_init(struct amdgpu_device *adev)
 	for (i = 0; i < adev->num_ip_blocks; i++) {
 		if (!adev->ip_block_status[i].valid)
 			continue;
-		if (adev->ip_blocks[i].type == AMD_IP_BLOCK_TYPE_UVD ||
-			adev->ip_blocks[i].type == AMD_IP_BLOCK_TYPE_VCE)
-			continue;
 		if (adev->ip_blocks[i].funcs->late_init) {
 			r = adev->ip_blocks[i].funcs->late_init((void *)adev);
 			if (r) {
@@ -1445,12 +1442,17 @@ static int amdgpu_late_init(struct amdgpu_device *adev)
 			}
 			adev->ip_block_status[i].late_initialized = true;
 		}
-		/* enable clockgating to save power */
-		r = adev->ip_blocks[i].funcs->set_clockgating_state((void *)adev,
-								    AMD_CG_STATE_GATE);
-		if (r) {
-			DRM_ERROR("set_clockgating_state(gate) of IP block <%s> failed %d\n", adev->ip_blocks[i].funcs->name, r);
-			return r;
+		/* skip CG for VCE/UVD, it's handled specially */
+		if (adev->ip_blocks[i].type != AMD_IP_BLOCK_TYPE_UVD &&
+		    adev->ip_blocks[i].type != AMD_IP_BLOCK_TYPE_VCE) {
+			/* enable clockgating to save power */
+			r = adev->ip_blocks[i].funcs->set_clockgating_state((void *)adev,
+									    AMD_CG_STATE_GATE);
+			if (r) {
+				DRM_ERROR("set_clockgating_state(gate) of IP block <%s> failed %d\n",
+					  adev->ip_blocks[i].funcs->name, r);
+				return r;
+			}
 		}
 	}
 
-- 
2.5.5

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

Re: [PATCH 1/2] drm/amd/amdgpu: enable clockgating only after late init

[Christian König]

Am 07.10.2016 um 20:50 schrieb Alex Deucher:
> From: Arindam Nath <arindam.nath@amd.com>
>
> Sometimes during multiple reboots, the system hangs
> during bootup. The issue is very random and happens
> once in around 50 reboots or so.
>
> It seems if clockgating is enabled before late init,
> the GFX engine sometimes does not respond.
>
> This patch changes the ordering a little so that
> both powergating and clockgating are enabled only
> after late init calls.
>
> Signed-off-by: Arindam Nath <arindam.nath@amd.com>
> Tested-by: Sunil Uttarwar <Sunil.Uttarwar1@amd.com>
> Reviewed-by: Tom St Denis <tom.stdenis@amd.com>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

For patch #2 I suggest that we clean up those things in the long run 
with some sort of per IP flags instead of checking the type directly.

But for now that should be sufficient and both patches are Reviewed-by: 
Christian König <christian.koenig@amd.com>.

Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 575a84b..e43779f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1437,13 +1437,6 @@ static int amdgpu_late_init(struct amdgpu_device *adev)
>   		if (adev->ip_blocks[i].type == AMD_IP_BLOCK_TYPE_UVD ||
>   			adev->ip_blocks[i].type == AMD_IP_BLOCK_TYPE_VCE)
>   			continue;
> -		/* enable clockgating to save power */
> -		r = adev->ip_blocks[i].funcs->set_clockgating_state((void *)adev,
> -								    AMD_CG_STATE_GATE);
> -		if (r) {
> -			DRM_ERROR("set_clockgating_state(gate) of IP block <%s> failed %d\n", adev->ip_blocks[i].funcs->name, r);
> -			return r;
> -		}
>   		if (adev->ip_blocks[i].funcs->late_init) {
>   			r = adev->ip_blocks[i].funcs->late_init((void *)adev);
>   			if (r) {
> @@ -1452,6 +1445,13 @@ static int amdgpu_late_init(struct amdgpu_device *adev)
>   			}
>   			adev->ip_block_status[i].late_initialized = true;
>   		}
> +		/* enable clockgating to save power */
> +		r = adev->ip_blocks[i].funcs->set_clockgating_state((void *)adev,
> +								    AMD_CG_STATE_GATE);
> +		if (r) {
> +			DRM_ERROR("set_clockgating_state(gate) of IP block <%s> failed %d\n", adev->ip_blocks[i].funcs->name, r);
> +			return r;
> +		}
>   	}
>   
>   	return 0;


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