[PART1 PATCH v4 2/8] drm/amdgpu: add parse clock gating state

[PART1 PATCH v4 2/8] drm/amdgpu: add parse clock gating state

[Huang Rui]

Suggested-by: Felix Kuehling <Felix.Kuehling@amd.com>
Signed-off-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Cc: William Lewis <minutemaidpark@hotmail.com>
---

Changes from V3 -> V4:
- fix some typo

---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 33 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h |  6 ++++++
 2 files changed, 39 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 1203e9a..dba462f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -34,6 +34,28 @@
 
 static int amdgpu_debugfs_pm_init(struct amdgpu_device *adev);
 
+static const struct cg_flag_name clocks[] = {
+	{AMD_CG_SUPPORT_GFX_MGCG, "Graphics Medium Grain Clock Gating"},
+	{AMD_CG_SUPPORT_GFX_MGLS, "Graphics Medium Grain memory Light Sleep"},
+	{AMD_CG_SUPPORT_GFX_CGCG, "Graphics Coarse Grain Clock Gating"},
+	{AMD_CG_SUPPORT_GFX_CGLS, "Graphics Coarse Grain memory Light Sleep"},
+	{AMD_CG_SUPPORT_GFX_CGTS, "Graphics Coarse Grain Tree Shader Light Sleep"},
+	{AMD_CG_SUPPORT_GFX_CGTS_LS, "Graphics Coarse Grain Tree Shader Light Sleep"},
+	{AMD_CG_SUPPORT_GFX_CP_LS, "Graphics Command Processor Light Sleep"},
+	{AMD_CG_SUPPORT_GFX_RLC_LS, "Graphics Run List Controller Light Sleep"},
+	{AMD_CG_SUPPORT_MC_LS, "Memory Controller Light Sleep"},
+	{AMD_CG_SUPPORT_MC_MGCG, "Memory Controller Medium Grain Clock Gating"},
+	{AMD_CG_SUPPORT_SDMA_LS, "System Direct Memory Access Light Sleep"},
+	{AMD_CG_SUPPORT_SDMA_MGCG, "System Direct Memory Access Medium Grain Clock Gating"},
+	{AMD_CG_SUPPORT_BIF_LS, "Bus Interface Light Sleep"},
+	{AMD_CG_SUPPORT_UVD_MGCG, "Universal Video Decoder Medium Grain Clock Gating"},
+	{AMD_CG_SUPPORT_VCE_MGCG, "Video Compression Engine Medium Grain Clock Gating"},
+	{AMD_CG_SUPPORT_HDP_LS, "Host Data Path Light Sleep"},
+	{AMD_CG_SUPPORT_HDP_MGCG, "Host Data Path Medium Grain Clock Gating"},
+	{AMD_CG_SUPPORT_ROM_MGCG, "Rom Medium Grain Clock Gating"},
+	{0, NULL},
+};
+
 void amdgpu_pm_acpi_event_handler(struct amdgpu_device *adev)
 {
 	if (adev->pp_enabled)
@@ -1536,6 +1558,15 @@ static int amdgpu_debugfs_pm_info_pp(struct seq_file *m, struct amdgpu_device *a
 	return 0;
 }
 
+static void amdgpu_parse_cg_state(struct seq_file *m, u32 flags)
+{
+	int i;
+
+	for (i = 0; clocks[i].flag; i++)
+		seq_printf(m, "\t%s: %s\n", clocks[i].name,
+			   (flags & clocks[i].flag) ? "On" : "Off");
+}
+
 static int amdgpu_debugfs_pm_info(struct seq_file *m, void *data)
 {
 	struct drm_info_node *node = (struct drm_info_node *) m->private;
@@ -1546,6 +1577,8 @@ static int amdgpu_debugfs_pm_info(struct seq_file *m, void *data)
 
 	amdgpu_get_clockgating_state(adev, &flags);
 	seq_printf(m, "Clock Gating Flags Mask: 0x%x\n", flags);
+	amdgpu_parse_cg_state(m, flags);
+	seq_printf(m, "\n");
 
 	if (!adev->pm.dpm_enabled) {
 		seq_printf(m, "dpm not enabled\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h
index 5fd7734..c19c4d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h
@@ -24,6 +24,12 @@
 #ifndef __AMDGPU_PM_H__
 #define __AMDGPU_PM_H__
 
+struct cg_flag_name
+{
+	u32 flag;
+	const char *name;
+};
+
 int amdgpu_pm_sysfs_init(struct amdgpu_device *adev);
 void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev);
 void amdgpu_pm_print_power_states(struct amdgpu_device *adev);
-- 
2.7.4

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

[PART1 PATCH v4 7/8] drm/amdgpu: add get clockgating_state method for uvd v5&v6

[Huang Rui]

Signed-off-by: Huang Rui <ray.huang@amd.com>
---

Changes from V3 -> V4:
- use pm mutex to protect UVD clock gating status

---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h   |  1 +
 drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c | 37 +++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c | 37 +++++++++++++++++++++++++++++++++--
 3 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 530549b..31054c7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1183,6 +1183,7 @@ struct amdgpu_uvd {
 	bool			use_ctx_buf;
 	struct amd_sched_entity entity;
 	uint32_t                srbm_soft_reset;
+	bool			is_powergated;
 };
 
 /*
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
index 03a35d9..e647d3e 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
@@ -781,16 +781,48 @@ static int uvd_v5_0_set_powergating_state(void *handle,
 	 * the smc and the hw blocks
 	 */
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int ret = 0;
 
 	if (!(adev->pg_flags & AMD_PG_SUPPORT_UVD))
 		return 0;
 
+	mutex_lock(&adev->pm.mutex);
+
 	if (state == AMD_PG_STATE_GATE) {
+		adev->uvd.is_powergated = true;
 		uvd_v5_0_stop(adev);
-		return 0;
 	} else {
-		return uvd_v5_0_start(adev);
+		ret = uvd_v5_0_start(adev);
+		if (ret)
+			goto out;
+		adev->uvd.is_powergated = false;
+	}
+
+out:
+	mutex_unlock(&adev->pm.mutex);
+
+	return ret;
+}
+
+static void uvd_v5_0_get_clockgating_state(void *handle, u32 *flags)
+{
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int data;
+
+	mutex_lock(&adev->pm.mutex);
+
+	if (adev->uvd.is_powergated) {
+		DRM_INFO("Cannot get clockgating state when UVD is powergated.\n");
+		goto out;
 	}
+
+	/* AMD_CG_SUPPORT_UVD_MGCG */
+	data = RREG32(mmUVD_CGC_CTRL);
+	if (data & UVD_CGC_CTRL__DYN_CLOCK_MODE_MASK)
+		*flags |= AMD_CG_SUPPORT_UVD_MGCG;
+
+out:
+	mutex_unlock(&adev->pm.mutex);
 }
 
 static const struct amd_ip_funcs uvd_v5_0_ip_funcs = {
@@ -808,6 +840,7 @@ static const struct amd_ip_funcs uvd_v5_0_ip_funcs = {
 	.soft_reset = uvd_v5_0_soft_reset,
 	.set_clockgating_state = uvd_v5_0_set_clockgating_state,
 	.set_powergating_state = uvd_v5_0_set_powergating_state,
+	.get_clockgating_state = uvd_v5_0_get_clockgating_state,
 };
 
 static const struct amdgpu_ring_funcs uvd_v5_0_ring_funcs = {
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
index 8779d4b..2585ae9 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
@@ -987,18 +987,50 @@ static int uvd_v6_0_set_powergating_state(void *handle,
 	 * the smc and the hw blocks
 	 */
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int ret = 0;
 
 	if (!(adev->pg_flags & AMD_PG_SUPPORT_UVD))
 		return 0;
 
 	WREG32(mmUVD_POWER_STATUS, UVD_POWER_STATUS__UVD_PG_EN_MASK);
 
+	mutex_lock(&adev->pm.mutex);
+
 	if (state == AMD_PG_STATE_GATE) {
+		adev->uvd.is_powergated = true;
 		uvd_v6_0_stop(adev);
-		return 0;
 	} else {
-		return uvd_v6_0_start(adev);
+		ret = uvd_v6_0_start(adev);
+		if (ret)
+			goto out;
+		adev->uvd.is_powergated = false;
+	}
+
+out:
+	mutex_unlock(&adev->pm.mutex);
+
+	return ret;
+}
+
+static void uvd_v6_0_get_clockgating_state(void *handle, u32 *flags)
+{
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int data;
+
+	mutex_lock(&adev->pm.mutex);
+
+	if (adev->uvd.is_powergated) {
+		DRM_INFO("Cannot get clockgating state when UVD is powergated.\n");
+		goto out;
 	}
+
+	/* AMD_CG_SUPPORT_UVD_MGCG */
+	data = RREG32(mmUVD_CGC_CTRL);
+	if (data & UVD_CGC_CTRL__DYN_CLOCK_MODE_MASK)
+		*flags |= AMD_CG_SUPPORT_UVD_MGCG;
+
+out:
+	mutex_unlock(&adev->pm.mutex);
 }
 
 static const struct amd_ip_funcs uvd_v6_0_ip_funcs = {
@@ -1019,6 +1051,7 @@ static const struct amd_ip_funcs uvd_v6_0_ip_funcs = {
 	.post_soft_reset = uvd_v6_0_post_soft_reset,
 	.set_clockgating_state = uvd_v6_0_set_clockgating_state,
 	.set_powergating_state = uvd_v6_0_set_powergating_state,
+	.get_clockgating_state = uvd_v6_0_get_clockgating_state,
 };
 
 static const struct amdgpu_ring_funcs uvd_v6_0_ring_phys_funcs = {
-- 
2.7.4

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

[PART1 PATCH v4 8/8] drm/amdgpu: add get clockgating_state method for vce v3

[Huang Rui]

Signed-off-by: Huang Rui <ray.huang@amd.com>
---

Changes from V3 -> V4:
- use pm mutex to protect VCE clock gating status

---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h   |  1 +
 drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 44 +++++++++++++++++++++++++++++++----
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 31054c7..3c52b81 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1212,6 +1212,7 @@ struct amdgpu_vce {
 	struct amd_sched_entity	entity;
 	uint32_t                srbm_soft_reset;
 	unsigned		num_rings;
+	bool			is_powergated;
 };
 
 /*
diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
index 35ff1c3..dde46ea 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
@@ -770,15 +770,50 @@ static int vce_v3_0_set_powergating_state(void *handle,
 	 * the smc and the hw blocks
 	 */
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int ret = 0;
 
 	if (!(adev->pg_flags & AMD_PG_SUPPORT_VCE))
 		return 0;
 
-	if (state == AMD_PG_STATE_GATE)
+	mutex_lock(&adev->pm.mutex);
+
+	if (state == AMD_PG_STATE_GATE) {
+		adev->vce.is_powergated = true;
 		/* XXX do we need a vce_v3_0_stop()? */
-		return 0;
-	else
-		return vce_v3_0_start(adev);
+	} else {
+		ret = vce_v3_0_start(adev);
+		if (ret)
+			goto out;
+		adev->vce.is_powergated = false;
+	}
+
+out:
+	mutex_unlock(&adev->pm.mutex);
+
+	return ret;
+}
+
+static void vce_v3_0_get_clockgating_state(void *handle, u32 *flags)
+{
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int data;
+
+	mutex_lock(&adev->pm.mutex);
+
+	if (adev->vce.is_powergated) {
+		DRM_INFO("Cannot get clockgating state when VCE is powergated.\n");
+		goto out;
+	}
+
+	WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, 0);
+
+	/* AMD_CG_SUPPORT_VCE_MGCG */
+	data = RREG32(mmVCE_CLOCK_GATING_A);
+	if (data & (0x04 << 4))
+		*flags |= AMD_CG_SUPPORT_VCE_MGCG;
+
+out:
+	mutex_unlock(&adev->pm.mutex);
 }
 
 static void vce_v3_0_ring_emit_ib(struct amdgpu_ring *ring,
@@ -832,6 +867,7 @@ static const struct amd_ip_funcs vce_v3_0_ip_funcs = {
 	.post_soft_reset = vce_v3_0_post_soft_reset,
 	.set_clockgating_state = vce_v3_0_set_clockgating_state,
 	.set_powergating_state = vce_v3_0_set_powergating_state,
+	.get_clockgating_state = vce_v3_0_get_clockgating_state,
 };
 
 static const struct amdgpu_ring_funcs vce_v3_0_ring_phys_funcs = {
-- 
2.7.4

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

Re: [PART1 PATCH v4 7/8] drm/amdgpu: add get clockgating_state method for uvd v5&v6

[Christian König]

Am 09.01.2017 um 04:00 schrieb Huang Rui:
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> ---
>
> Changes from V3 -> V4:
> - use pm mutex to protect UVD clock gating status
>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h   |  1 +
>   drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c | 37 +++++++++++++++++++++++++++++++++--
>   drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c | 37 +++++++++++++++++++++++++++++++++--
>   3 files changed, 71 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 530549b..31054c7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1183,6 +1183,7 @@ struct amdgpu_uvd {
>   	bool			use_ctx_buf;
>   	struct amd_sched_entity entity;
>   	uint32_t                srbm_soft_reset;
> +	bool			is_powergated;
>   };
>   
>   /*
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> index 03a35d9..e647d3e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> @@ -781,16 +781,48 @@ static int uvd_v5_0_set_powergating_state(void *handle,
>   	 * the smc and the hw blocks
>   	 */
>   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +	int ret = 0;
>   
>   	if (!(adev->pg_flags & AMD_PG_SUPPORT_UVD))
>   		return 0;
>   
> +	mutex_lock(&adev->pm.mutex);

Might be that I'm wrong, but didn't Tom said the mutex is taken anyway 
when this function is called?

If that's true we would certainly run into problem when we try to 
acquire it again.

Christian.

> +
>   	if (state == AMD_PG_STATE_GATE) {
> +		adev->uvd.is_powergated = true;
>   		uvd_v5_0_stop(adev);
> -		return 0;
>   	} else {
> -		return uvd_v5_0_start(adev);
> +		ret = uvd_v5_0_start(adev);
> +		if (ret)
> +			goto out;
> +		adev->uvd.is_powergated = false;
> +	}
> +
> +out:
> +	mutex_unlock(&adev->pm.mutex);
> +
> +	return ret;
> +}
> +
> +static void uvd_v5_0_get_clockgating_state(void *handle, u32 *flags)
> +{
> +	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +	int data;
> +
> +	mutex_lock(&adev->pm.mutex);
> +
> +	if (adev->uvd.is_powergated) {
> +		DRM_INFO("Cannot get clockgating state when UVD is powergated.\n");
> +		goto out;
>   	}
> +
> +	/* AMD_CG_SUPPORT_UVD_MGCG */
> +	data = RREG32(mmUVD_CGC_CTRL);
> +	if (data & UVD_CGC_CTRL__DYN_CLOCK_MODE_MASK)
> +		*flags |= AMD_CG_SUPPORT_UVD_MGCG;
> +
> +out:
> +	mutex_unlock(&adev->pm.mutex);
>   }
>   
>   static const struct amd_ip_funcs uvd_v5_0_ip_funcs = {
> @@ -808,6 +840,7 @@ static const struct amd_ip_funcs uvd_v5_0_ip_funcs = {
>   	.soft_reset = uvd_v5_0_soft_reset,
>   	.set_clockgating_state = uvd_v5_0_set_clockgating_state,
>   	.set_powergating_state = uvd_v5_0_set_powergating_state,
> +	.get_clockgating_state = uvd_v5_0_get_clockgating_state,
>   };
>   
>   static const struct amdgpu_ring_funcs uvd_v5_0_ring_funcs = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> index 8779d4b..2585ae9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> @@ -987,18 +987,50 @@ static int uvd_v6_0_set_powergating_state(void *handle,
>   	 * the smc and the hw blocks
>   	 */
>   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +	int ret = 0;
>   
>   	if (!(adev->pg_flags & AMD_PG_SUPPORT_UVD))
>   		return 0;
>   
>   	WREG32(mmUVD_POWER_STATUS, UVD_POWER_STATUS__UVD_PG_EN_MASK);
>   
> +	mutex_lock(&adev->pm.mutex);
> +
>   	if (state == AMD_PG_STATE_GATE) {
> +		adev->uvd.is_powergated = true;
>   		uvd_v6_0_stop(adev);
> -		return 0;
>   	} else {
> -		return uvd_v6_0_start(adev);
> +		ret = uvd_v6_0_start(adev);
> +		if (ret)
> +			goto out;
> +		adev->uvd.is_powergated = false;
> +	}
> +
> +out:
> +	mutex_unlock(&adev->pm.mutex);
> +
> +	return ret;
> +}
> +
> +static void uvd_v6_0_get_clockgating_state(void *handle, u32 *flags)
> +{
> +	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +	int data;
> +
> +	mutex_lock(&adev->pm.mutex);
> +
> +	if (adev->uvd.is_powergated) {
> +		DRM_INFO("Cannot get clockgating state when UVD is powergated.\n");
> +		goto out;
>   	}
> +
> +	/* AMD_CG_SUPPORT_UVD_MGCG */
> +	data = RREG32(mmUVD_CGC_CTRL);
> +	if (data & UVD_CGC_CTRL__DYN_CLOCK_MODE_MASK)
> +		*flags |= AMD_CG_SUPPORT_UVD_MGCG;
> +
> +out:
> +	mutex_unlock(&adev->pm.mutex);
>   }
>   
>   static const struct amd_ip_funcs uvd_v6_0_ip_funcs = {
> @@ -1019,6 +1051,7 @@ static const struct amd_ip_funcs uvd_v6_0_ip_funcs = {
>   	.post_soft_reset = uvd_v6_0_post_soft_reset,
>   	.set_clockgating_state = uvd_v6_0_set_clockgating_state,
>   	.set_powergating_state = uvd_v6_0_set_powergating_state,
> +	.get_clockgating_state = uvd_v6_0_get_clockgating_state,
>   };
>   
>   static const struct amdgpu_ring_funcs uvd_v6_0_ring_phys_funcs = {


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

Re: [PART1 PATCH v4 7/8] drm/amdgpu: add get clockgating_state method for uvd v5&v6

[StDenis, Tom]

[-- Attachment #1.1: Type: text/plain, Size: 6029 bytes --]

Yup it's held by both amdgpu_dpm_enable_uvd() and amdgpu_dpm_enable_vce()


Tom


________________________________
From: Koenig, Christian
Sent: Monday, January 9, 2017 05:32
To: Huang, Ray; Deucher, Alexander; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org; StDenis, Tom
Cc: Zhu, Rex; Mao, David; Fu, Ping; Zhang, Hawking; Kuehling, Felix
Subject: Re: [PART1 PATCH v4 7/8] drm/amdgpu: add get clockgating_state method for uvd v5&v6

Am 09.01.2017 um 04:00 schrieb Huang Rui:
> Signed-off-by: Huang Rui <ray.huang-5C7GfCeVMHo@public.gmane.org>
> ---
>
> Changes from V3 -> V4:
> - use pm mutex to protect UVD clock gating status
>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h   |  1 +
>   drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c | 37 +++++++++++++++++++++++++++++++++--
>   drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c | 37 +++++++++++++++++++++++++++++++++--
>   3 files changed, 71 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 530549b..31054c7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1183,6 +1183,7 @@ struct amdgpu_uvd {
>        bool                    use_ctx_buf;
>        struct amd_sched_entity entity;
>        uint32_t                srbm_soft_reset;
> +     bool                    is_powergated;
>   };
>
>   /*
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> index 03a35d9..e647d3e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> @@ -781,16 +781,48 @@ static int uvd_v5_0_set_powergating_state(void *handle,
>         * the smc and the hw blocks
>         */
>        struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +     int ret = 0;
>
>        if (!(adev->pg_flags & AMD_PG_SUPPORT_UVD))
>                return 0;
>
> +     mutex_lock(&adev->pm.mutex);

Might be that I'm wrong, but didn't Tom said the mutex is taken anyway
when this function is called?

If that's true we would certainly run into problem when we try to
acquire it again.

Christian.

> +
>        if (state == AMD_PG_STATE_GATE) {
> +             adev->uvd.is_powergated = true;
>                uvd_v5_0_stop(adev);
> -             return 0;
>        } else {
> -             return uvd_v5_0_start(adev);
> +             ret = uvd_v5_0_start(adev);
> +             if (ret)
> +                     goto out;
> +             adev->uvd.is_powergated = false;
> +     }
> +
> +out:
> +     mutex_unlock(&adev->pm.mutex);
> +
> +     return ret;
> +}
> +
> +static void uvd_v5_0_get_clockgating_state(void *handle, u32 *flags)
> +{
> +     struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +     int data;
> +
> +     mutex_lock(&adev->pm.mutex);
> +
> +     if (adev->uvd.is_powergated) {
> +             DRM_INFO("Cannot get clockgating state when UVD is powergated.\n");
> +             goto out;
>        }
> +
> +     /* AMD_CG_SUPPORT_UVD_MGCG */
> +     data = RREG32(mmUVD_CGC_CTRL);
> +     if (data & UVD_CGC_CTRL__DYN_CLOCK_MODE_MASK)
> +             *flags |= AMD_CG_SUPPORT_UVD_MGCG;
> +
> +out:
> +     mutex_unlock(&adev->pm.mutex);
>   }
>
>   static const struct amd_ip_funcs uvd_v5_0_ip_funcs = {
> @@ -808,6 +840,7 @@ static const struct amd_ip_funcs uvd_v5_0_ip_funcs = {
>        .soft_reset = uvd_v5_0_soft_reset,
>        .set_clockgating_state = uvd_v5_0_set_clockgating_state,
>        .set_powergating_state = uvd_v5_0_set_powergating_state,
> +     .get_clockgating_state = uvd_v5_0_get_clockgating_state,
>   };
>
>   static const struct amdgpu_ring_funcs uvd_v5_0_ring_funcs = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> index 8779d4b..2585ae9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> @@ -987,18 +987,50 @@ static int uvd_v6_0_set_powergating_state(void *handle,
>         * the smc and the hw blocks
>         */
>        struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +     int ret = 0;
>
>        if (!(adev->pg_flags & AMD_PG_SUPPORT_UVD))
>                return 0;
>
>        WREG32(mmUVD_POWER_STATUS, UVD_POWER_STATUS__UVD_PG_EN_MASK);
>
> +     mutex_lock(&adev->pm.mutex);
> +
>        if (state == AMD_PG_STATE_GATE) {
> +             adev->uvd.is_powergated = true;
>                uvd_v6_0_stop(adev);
> -             return 0;
>        } else {
> -             return uvd_v6_0_start(adev);
> +             ret = uvd_v6_0_start(adev);
> +             if (ret)
> +                     goto out;
> +             adev->uvd.is_powergated = false;
> +     }
> +
> +out:
> +     mutex_unlock(&adev->pm.mutex);
> +
> +     return ret;
> +}
> +
> +static void uvd_v6_0_get_clockgating_state(void *handle, u32 *flags)
> +{
> +     struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +     int data;
> +
> +     mutex_lock(&adev->pm.mutex);
> +
> +     if (adev->uvd.is_powergated) {
> +             DRM_INFO("Cannot get clockgating state when UVD is powergated.\n");
> +             goto out;
>        }
> +
> +     /* AMD_CG_SUPPORT_UVD_MGCG */
> +     data = RREG32(mmUVD_CGC_CTRL);
> +     if (data & UVD_CGC_CTRL__DYN_CLOCK_MODE_MASK)
> +             *flags |= AMD_CG_SUPPORT_UVD_MGCG;
> +
> +out:
> +     mutex_unlock(&adev->pm.mutex);
>   }
>
>   static const struct amd_ip_funcs uvd_v6_0_ip_funcs = {
> @@ -1019,6 +1051,7 @@ static const struct amd_ip_funcs uvd_v6_0_ip_funcs = {
>        .post_soft_reset = uvd_v6_0_post_soft_reset,
>        .set_clockgating_state = uvd_v6_0_set_clockgating_state,
>        .set_powergating_state = uvd_v6_0_set_powergating_state,
> +     .get_clockgating_state = uvd_v6_0_get_clockgating_state,
>   };
>
>   static const struct amdgpu_ring_funcs uvd_v6_0_ring_phys_funcs = {



[-- Attachment #1.2: Type: text/html, Size: 12311 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

RE: [PART1 PATCH v4 2/8] drm/amdgpu: add parse clock gating state

[Deucher, Alexander]

> -----Original Message-----
> From: Huang Rui [mailto:ray.huang@amd.com]
> Sent: Sunday, January 08, 2017 10:00 PM
> To: Deucher, Alexander; amd-gfx@lists.freedesktop.org; Koenig, Christian;
> StDenis, Tom
> Cc: Zhu, Rex; Mao, David; Fu, Ping; Zhang, Hawking; Kuehling, Felix; Huang,
> Ray; William Lewis
> Subject: [PART1 PATCH v4 2/8] drm/amdgpu: add parse clock gating state
> 
> Suggested-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> Cc: William Lewis <minutemaidpark@hotmail.com>
> ---
> 
> Changes from V3 -> V4:
> - fix some typo
> 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 33
> +++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h |  6 ++++++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index 1203e9a..dba462f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -34,6 +34,28 @@
> 
>  static int amdgpu_debugfs_pm_init(struct amdgpu_device *adev);
> 
> +static const struct cg_flag_name clocks[] = {
> +	{AMD_CG_SUPPORT_GFX_MGCG, "Graphics Medium Grain Clock
> Gating"},
> +	{AMD_CG_SUPPORT_GFX_MGLS, "Graphics Medium Grain memory
> Light Sleep"},
> +	{AMD_CG_SUPPORT_GFX_CGCG, "Graphics Coarse Grain Clock
> Gating"},
> +	{AMD_CG_SUPPORT_GFX_CGLS, "Graphics Coarse Grain memory
> Light Sleep"},
> +	{AMD_CG_SUPPORT_GFX_CGTS, "Graphics Coarse Grain Tree
> Shader Light Sleep"},
> +	{AMD_CG_SUPPORT_GFX_CGTS_LS, "Graphics Coarse Grain Tree
> Shader Light Sleep"},
> +	{AMD_CG_SUPPORT_GFX_CP_LS, "Graphics Command Processor
> Light Sleep"},
> +	{AMD_CG_SUPPORT_GFX_RLC_LS, "Graphics Run List Controller
> Light Sleep"},
> +	{AMD_CG_SUPPORT_MC_LS, "Memory Controller Light Sleep"},
> +	{AMD_CG_SUPPORT_MC_MGCG, "Memory Controller Medium
> Grain Clock Gating"},
> +	{AMD_CG_SUPPORT_SDMA_LS, "System Direct Memory Access
> Light Sleep"},
> +	{AMD_CG_SUPPORT_SDMA_MGCG, "System Direct Memory Access
> Medium Grain Clock Gating"},
> +	{AMD_CG_SUPPORT_BIF_LS, "Bus Interface Light Sleep"},
> +	{AMD_CG_SUPPORT_UVD_MGCG, "Universal Video Decoder
> Medium Grain Clock Gating"},

Universal -> Unified
With that fixed:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> +	{AMD_CG_SUPPORT_VCE_MGCG, "Video Compression Engine
> Medium Grain Clock Gating"},
> +	{AMD_CG_SUPPORT_HDP_LS, "Host Data Path Light Sleep"},
> +	{AMD_CG_SUPPORT_HDP_MGCG, "Host Data Path Medium Grain
> Clock Gating"},
> +	{AMD_CG_SUPPORT_ROM_MGCG, "Rom Medium Grain Clock
> Gating"},
> +	{0, NULL},
> +};
> +
>  void amdgpu_pm_acpi_event_handler(struct amdgpu_device *adev)
>  {
>  	if (adev->pp_enabled)
> @@ -1536,6 +1558,15 @@ static int amdgpu_debugfs_pm_info_pp(struct
> seq_file *m, struct amdgpu_device *a
>  	return 0;
>  }
> 
> +static void amdgpu_parse_cg_state(struct seq_file *m, u32 flags)
> +{
> +	int i;
> +
> +	for (i = 0; clocks[i].flag; i++)
> +		seq_printf(m, "\t%s: %s\n", clocks[i].name,
> +			   (flags & clocks[i].flag) ? "On" : "Off");
> +}
> +
>  static int amdgpu_debugfs_pm_info(struct seq_file *m, void *data)
>  {
>  	struct drm_info_node *node = (struct drm_info_node *) m-
> >private;
> @@ -1546,6 +1577,8 @@ static int amdgpu_debugfs_pm_info(struct seq_file
> *m, void *data)
> 
>  	amdgpu_get_clockgating_state(adev, &flags);
>  	seq_printf(m, "Clock Gating Flags Mask: 0x%x\n", flags);
> +	amdgpu_parse_cg_state(m, flags);
> +	seq_printf(m, "\n");
> 
>  	if (!adev->pm.dpm_enabled) {
>  		seq_printf(m, "dpm not enabled\n");
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h
> index 5fd7734..c19c4d1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h
> @@ -24,6 +24,12 @@
>  #ifndef __AMDGPU_PM_H__
>  #define __AMDGPU_PM_H__
> 
> +struct cg_flag_name
> +{
> +	u32 flag;
> +	const char *name;
> +};
> +
>  int amdgpu_pm_sysfs_init(struct amdgpu_device *adev);
>  void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev);
>  void amdgpu_pm_print_power_states(struct amdgpu_device *adev);
> --
> 2.7.4

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

Re: [PART1 PATCH v4 7/8] drm/amdgpu: add get clockgating_state method for uvd v5&v6

[Christian König]

Am 09.01.2017 um 15:46 schrieb Huang Rui:
> On Mon, Jan 09, 2017 at 07:29:00PM +0800, StDenis, Tom wrote:
>> Yup it's held by both amdgpu_dpm_enable_uvd() and amdgpu_dpm_enable_vce()
>>
>> Tom
>>
> <snip>
>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c b/drivers/gpu/drm/amd/
>> amdgpu/uvd_v5_0.c
>>> index 03a35d9..e647d3e 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
>>> @@ -781,16 +781,48 @@ static int uvd_v5_0_set_powergating_state(void *handle,
>>>          * the smc and the hw blocks
>>>          */
>>>         struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>>> +     int ret = 0;
>>>   
>>>         if (!(adev->pg_flags & AMD_PG_SUPPORT_UVD))
>>>                 return 0;
>>>   
>>> +     mutex_lock(&adev->pm.mutex);
>> Might be that I'm wrong, but didn't Tom said the mutex is taken anyway
>> when this function is called?
>>
>> If that's true we would certainly run into problem when we try to
>> acquire it again.
>>
> OK, I see. Actually, pm.mutex is already held on upper layer call
> (amdgpu_dpm_enable_uvd). So we should not hold it again here.
>
>>> +
>>> +static void uvd_v5_0_get_clockgating_state(void *handle, u32 *flags)
>>> +{
>>> +     struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>>> +     int data;
>>> +
>>> +     mutex_lock(&adev->pm.mutex);
>>> +
> We just need keep pm.mutex here in set_clockgating_state, that's enough.
>
> Christian, Tom, am I right?

Yes, I think so.

Christian.

>
> Thanks,
> Rui


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

Re: [PART1 PATCH v4 7/8] drm/amdgpu: add get clockgating_state method for uvd v5&v6

[Huang Rui]

On Mon, Jan 09, 2017 at 07:29:00PM +0800, StDenis, Tom wrote:
> Yup it's held by both amdgpu_dpm_enable_uvd() and amdgpu_dpm_enable_vce()
> 
> Tom
> 

<snip>

> > diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c b/drivers/gpu/drm/amd/
> amdgpu/uvd_v5_0.c
> > index 03a35d9..e647d3e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> > @@ -781,16 +781,48 @@ static int uvd_v5_0_set_powergating_state(void *handle,
> >         * the smc and the hw blocks
> >         */
> >        struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> > +     int ret = 0;
> >  
> >        if (!(adev->pg_flags & AMD_PG_SUPPORT_UVD))
> >                return 0;
> >  
> > +     mutex_lock(&adev->pm.mutex);
> 
> Might be that I'm wrong, but didn't Tom said the mutex is taken anyway
> when this function is called?
> 
> If that's true we would certainly run into problem when we try to
> acquire it again.
> 

OK, I see. Actually, pm.mutex is already held on upper layer call
(amdgpu_dpm_enable_uvd). So we should not hold it again here.

> > +
> > +static void uvd_v5_0_get_clockgating_state(void *handle, u32 *flags)
> > +{
> > +     struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> > +     int data;
> > +
> > +     mutex_lock(&adev->pm.mutex);
> > +

We just need keep pm.mutex here in set_clockgating_state, that's enough.

Christian, Tom, am I right?

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

Re: [PART1 PATCH v4 7/8] drm/amdgpu: add get clockgating_state method for uvd v5&v6

[StDenis, Tom]

[-- Attachment #1.1: Type: text/plain, Size: 2267 bytes --]

Yes, holding pm.mutex will prevent PP/DPM from entering/leaving PG states and is used as a means to probe power/clock related signals reliably (except for VCE clock signals which aren't part of an AON tile).


You just can't take it from a "set" state function since the higher up API will take it.


Tom


________________________________
From: Koenig, Christian
Sent: Monday, January 9, 2017 09:45
To: Huang, Ray; StDenis, Tom
Cc: Deucher, Alexander; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org; Zhu, Rex; Mao, David; Fu, Ping; Zhang, Hawking; Kuehling, Felix
Subject: Re: [PART1 PATCH v4 7/8] drm/amdgpu: add get clockgating_state method for uvd v5&v6

Am 09.01.2017 um 15:46 schrieb Huang Rui:
> On Mon, Jan 09, 2017 at 07:29:00PM +0800, StDenis, Tom wrote:
>> Yup it's held by both amdgpu_dpm_enable_uvd() and amdgpu_dpm_enable_vce()
>>
>> Tom
>>
> <snip>
>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c b/drivers/gpu/drm/amd/
>> amdgpu/uvd_v5_0.c
>>> index 03a35d9..e647d3e 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
>>> @@ -781,16 +781,48 @@ static int uvd_v5_0_set_powergating_state(void *handle,
>>>          * the smc and the hw blocks
>>>          */
>>>         struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>>> +     int ret = 0;
>>>
>>>         if (!(adev->pg_flags & AMD_PG_SUPPORT_UVD))
>>>                 return 0;
>>>
>>> +     mutex_lock(&adev->pm.mutex);
>> Might be that I'm wrong, but didn't Tom said the mutex is taken anyway
>> when this function is called?
>>
>> If that's true we would certainly run into problem when we try to
>> acquire it again.
>>
> OK, I see. Actually, pm.mutex is already held on upper layer call
> (amdgpu_dpm_enable_uvd). So we should not hold it again here.
>
>>> +
>>> +static void uvd_v5_0_get_clockgating_state(void *handle, u32 *flags)
>>> +{
>>> +     struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>>> +     int data;
>>> +
>>> +     mutex_lock(&adev->pm.mutex);
>>> +
> We just need keep pm.mutex here in set_clockgating_state, that's enough.
>
> Christian, Tom, am I right?

Yes, I think so.

Christian.

>
> Thanks,
> Rui



[-- Attachment #1.2: Type: text/html, Size: 4043 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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