All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/4] drm/amdgpu: Add amdgpu_gfx_off_ctrl function
@ 2018-07-30  9:23 Rex Zhu
       [not found] ` <1532942625-26876-1-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Rex Zhu @ 2018-07-30  9:23 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Rex Zhu

v2:
   1. drop the special handling for the hw IP
      suggested by hawking and Christian.
   2. refine the variable name suggested by Flora.

This funciton as the entry of gfx off ctrl feature.
we arbitrat gfx off feature enable/disable in this
function.

Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  5 +++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c    | 36 ++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 5b7bb58..188bc53 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -988,6 +988,10 @@ struct amdgpu_gfx {
 	/* NGG */
 	struct amdgpu_ngg		ngg;
 
+	/* gfx off */
+	bool                            gfx_off_state; /* true: enabled, false: disabled */
+	struct mutex                    gfx_off_mutex;
+	uint32_t                        gfx_off_req_count; /* default 1, enable gfx off: dec 1, disable gfx off: add 1 */
 	/* pipe reservation */
 	struct mutex			pipe_reserve_mutex;
 	DECLARE_BITMAP			(pipe_reserve_bitmap, AMDGPU_MAX_COMPUTE_QUEUES);
@@ -1815,6 +1819,7 @@ void amdgpu_device_program_register_sequence(struct amdgpu_device *adev,
 					     const u32 array_size);
 
 bool amdgpu_device_is_px(struct drm_device *dev);
+void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable);
 /* atpx handler */
 #if defined(CONFIG_VGA_SWITCHEROO)
 void amdgpu_register_atpx_handler(void);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 745f760..122653f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2367,6 +2367,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 	mutex_init(&adev->gfx.gpu_clock_mutex);
 	mutex_init(&adev->srbm_mutex);
 	mutex_init(&adev->gfx.pipe_reserve_mutex);
+	mutex_init(&adev->gfx.gfx_off_mutex);
 	mutex_init(&adev->grbm_idx_mutex);
 	mutex_init(&adev->mn_lock);
 	mutex_init(&adev->virt.vf_errors.lock);
@@ -2394,6 +2395,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 	INIT_DELAYED_WORK(&adev->late_init_work,
 			  amdgpu_device_ip_late_init_func_handler);
 
+	adev->gfx.gfx_off_req_count = 1;
 	adev->pm.ac_power = power_supply_is_system_supplied() > 0 ? true : false;
 
 	/* Registers mapping */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index 239bf2a..1cdb264 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -340,3 +340,39 @@ void amdgpu_gfx_compute_mqd_sw_fini(struct amdgpu_device *adev)
 			      &ring->mqd_gpu_addr,
 			      &ring->mqd_ptr);
 }
+
+/* amdgpu_gfx_off_ctrl - Handle gfx off feature enable/disable
+ *
+ * @adev: amdgpu_device pointer
+ * @bool enable true: enable gfx off feature, false: disable gfx off feature
+ *
+ * 1. gfx off feature will be enabled by gfx ip after gfx cg gp enabled.
+ * 2. other client can send request to disable gfx off feature, the request should be honored.
+ * 3. other client can cancel their request of disable gfx off feature
+ * 4. other client should not send request to enable gfx off feature before disable gfx off feature.
+ */
+
+void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable)
+{
+	if (!(adev->powerplay.pp_feature & PP_GFXOFF_MASK))
+		return;
+
+	if (!adev->powerplay.pp_funcs->set_powergating_by_smu)
+		return;
+
+	mutex_lock(&adev->gfx.gfx_off_mutex);
+
+	if (!enable)
+		adev->gfx.gfx_off_req_count++;
+	else if (adev->gfx.gfx_off_req_count > 0)
+		adev->gfx.gfx_off_req_count--;
+
+	if (enable && !adev->gfx.gfx_off_state && !adev->gfx.gfx_off_req_count) {
+		if (!amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, true))
+			adev->gfx.gfx_off_state = true;
+	} else if (!enable && adev->gfx.gfx_off_state) {
+		if (!amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, false))
+			adev->gfx.gfx_off_state = false;
+	}
+	mutex_unlock(&adev->gfx.gfx_off_mutex);
+}
-- 
1.9.1

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

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

* [PATCH v2 2/4] drm/amdgpu: Put enable gfx off feature to a delay thread
       [not found] ` <1532942625-26876-1-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
@ 2018-07-30  9:23   ` Rex Zhu
       [not found]     ` <1532942625-26876-2-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
  2018-07-30  9:23   ` [PATCH v2 3/4] drm/amdgpu: gfx ip ctrl gfx off via amdgpu_gfx_off_ctrl Rex Zhu
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Rex Zhu @ 2018-07-30  9:23 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Rex Zhu

delay to enable gfx off feature to avoid gfx on/off frequently
suggested by Alex and Evan.

Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 15 +++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c    |  8 ++++++--
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 188bc53..ff16689 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -992,6 +992,8 @@ struct amdgpu_gfx {
 	bool                            gfx_off_state; /* true: enabled, false: disabled */
 	struct mutex                    gfx_off_mutex;
 	uint32_t                        gfx_off_req_count; /* default 1, enable gfx off: dec 1, disable gfx off: add 1 */
+	struct delayed_work             gfx_off_delay_work;
+
 	/* pipe reservation */
 	struct mutex			pipe_reserve_mutex;
 	DECLARE_BITMAP			(pipe_reserve_bitmap, AMDGPU_MAX_COMPUTE_QUEUES);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 122653f..f6a6fb5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1925,6 +1925,19 @@ static void amdgpu_device_ip_late_init_func_handler(struct work_struct *work)
 		DRM_ERROR("ib ring test failed (%d).\n", r);
 }
 
+static void amdgpu_device_delay_enable_gfx_off(struct work_struct *work)
+{
+	struct amdgpu_device *adev =
+		container_of(work, struct amdgpu_device, gfx.gfx_off_delay_work.work);
+
+	mutex_lock(&adev->gfx.gfx_off_mutex);
+	if (!adev->gfx.gfx_off_state && !adev->gfx.gfx_off_req_count) {
+		if (!amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, true))
+			adev->gfx.gfx_off_state = true;
+	}
+	mutex_unlock(&adev->gfx.gfx_off_mutex);
+}
+
 /**
  * amdgpu_device_ip_suspend_phase1 - run suspend for hardware IPs (phase 1)
  *
@@ -2394,6 +2407,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 
 	INIT_DELAYED_WORK(&adev->late_init_work,
 			  amdgpu_device_ip_late_init_func_handler);
+	INIT_DELAYED_WORK(&adev->gfx.gfx_off_delay_work,
+			  amdgpu_device_delay_enable_gfx_off);
 
 	adev->gfx.gfx_off_req_count = 1;
 	adev->pm.ac_power = power_supply_is_system_supplied() > 0 ? true : false;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index 1cdb264..11d4d9f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -26,6 +26,9 @@
 #include "amdgpu.h"
 #include "amdgpu_gfx.h"
 
+/* 0.5 second timeout */
+#define GFX_OFF_DELAY_ENABLE         msecs_to_jiffies(500)
+
 /*
  * GPU scratch registers helpers function.
  */
@@ -360,6 +363,7 @@ void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable)
 	if (!adev->powerplay.pp_funcs->set_powergating_by_smu)
 		return;
 
+
 	mutex_lock(&adev->gfx.gfx_off_mutex);
 
 	if (!enable)
@@ -368,11 +372,11 @@ void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable)
 		adev->gfx.gfx_off_req_count--;
 
 	if (enable && !adev->gfx.gfx_off_state && !adev->gfx.gfx_off_req_count) {
-		if (!amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, true))
-			adev->gfx.gfx_off_state = true;
+		schedule_delayed_work(&adev->gfx.gfx_off_delay_work, GFX_OFF_DELAY_ENABLE);
 	} else if (!enable && adev->gfx.gfx_off_state) {
 		if (!amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, false))
 			adev->gfx.gfx_off_state = false;
 	}
+
 	mutex_unlock(&adev->gfx.gfx_off_mutex);
 }
-- 
1.9.1

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

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

* [PATCH v2 3/4] drm/amdgpu: gfx ip ctrl gfx off via amdgpu_gfx_off_ctrl
       [not found] ` <1532942625-26876-1-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
  2018-07-30  9:23   ` [PATCH v2 2/4] drm/amdgpu: Put enable gfx off feature to a delay thread Rex Zhu
@ 2018-07-30  9:23   ` Rex Zhu
  2018-07-30  9:23   ` [PATCH v2 4/4] drm/amdgpu: Disable gfx off if VCN is busy Rex Zhu
  2018-07-30 19:25   ` [PATCH v2 1/4] drm/amdgpu: Add amdgpu_gfx_off_ctrl function Felix Kuehling
  3 siblings, 0 replies; 9+ messages in thread
From: Rex Zhu @ 2018-07-30  9:23 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Rex Zhu

use amdgpu_gfx_off_ctrl function so driver can arbitrate
whether the gfx ip can be power off or power on.

Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 ++----
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c      | 6 ++----
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index f6a6fb5..1d933db 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1828,8 +1828,7 @@ static int amdgpu_device_ip_fini(struct amdgpu_device *adev)
 					  adev->ip_blocks[i].version->funcs->name, r);
 				return r;
 			}
-			if (adev->powerplay.pp_funcs->set_powergating_by_smu)
-				amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, false);
+			amdgpu_gfx_off_ctrl(adev, false);
 			r = adev->ip_blocks[i].version->funcs->hw_fini((void *)adev);
 			/* XXX handle errors */
 			if (r) {
@@ -2012,8 +2011,7 @@ static int amdgpu_device_ip_suspend_phase2(struct amdgpu_device *adev)
 	}
 
 	/* call smu to disable gfx off feature first when suspend */
-	if (adev->powerplay.pp_funcs->set_powergating_by_smu)
-		amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, false);
+	amdgpu_gfx_off_ctrl(adev, false);
 
 	for (i = adev->num_ip_blocks - 1; i >= 0; i--) {
 		if (!adev->ip_blocks[i].status.valid)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index ef00d14..fd31d3b 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -3783,13 +3783,11 @@ static int gfx_v9_0_set_powergating_state(void *handle,
 		gfx_v9_0_update_gfx_mg_power_gating(adev, enable);
 
 		/* set gfx off through smu */
-		if (enable && adev->powerplay.pp_funcs->set_powergating_by_smu)
-			amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, true);
+		amdgpu_gfx_off_ctrl(adev, true);
 		break;
 	case CHIP_VEGA12:
 		/* set gfx off through smu */
-		if (enable && adev->powerplay.pp_funcs->set_powergating_by_smu)
-			amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, true);
+		amdgpu_gfx_off_ctrl(adev, true);
 		break;
 	default:
 		break;
-- 
1.9.1

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

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

* [PATCH v2 4/4] drm/amdgpu: Disable gfx off if VCN is busy
       [not found] ` <1532942625-26876-1-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
  2018-07-30  9:23   ` [PATCH v2 2/4] drm/amdgpu: Put enable gfx off feature to a delay thread Rex Zhu
  2018-07-30  9:23   ` [PATCH v2 3/4] drm/amdgpu: gfx ip ctrl gfx off via amdgpu_gfx_off_ctrl Rex Zhu
@ 2018-07-30  9:23   ` Rex Zhu
  2018-07-30 19:25   ` [PATCH v2 1/4] drm/amdgpu: Add amdgpu_gfx_off_ctrl function Felix Kuehling
  3 siblings, 0 replies; 9+ messages in thread
From: Rex Zhu @ 2018-07-30  9:23 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Rex Zhu

this patch is a workaround for the gpu hang
at video end time if play video with gfx off enabled.

Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 798648a..878f62c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -214,6 +214,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
 	fences += amdgpu_fence_count_emitted(&adev->vcn.ring_jpeg);
 
 	if (fences == 0) {
+		amdgpu_gfx_off_ctrl(adev, true);
 		if (adev->pm.dpm_enabled)
 			amdgpu_dpm_enable_uvd(adev, false);
 		else
@@ -230,6 +231,7 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
 	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
-- 
1.9.1

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

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

* Re: [PATCH v2 2/4] drm/amdgpu: Put enable gfx off feature to a delay thread
       [not found]     ` <1532942625-26876-2-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
@ 2018-07-30 10:25       ` Zhu, Rex
       [not found]         ` <CY4PR12MB168727FC303FDB625880C440FB2F0-rpdhrqHFk06Y0SjTqZDccQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
  2018-07-30 19:33       ` Felix Kuehling
  1 sibling, 1 reply; 9+ messages in thread
From: Zhu, Rex @ 2018-07-30 10:25 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW


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

One problem,


I delay 500ms to enable gfx off feature.


Any concern about the delay time?


Best Regards

Rex

________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
Sent: Monday, July 30, 2018 5:23 PM
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Cc: Zhu, Rex
Subject: [PATCH v2 2/4] drm/amdgpu: Put enable gfx off feature to a delay thread

delay to enable gfx off feature to avoid gfx on/off frequently
suggested by Alex and Evan.

Signed-off-by: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 15 +++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c    |  8 ++++++--
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 188bc53..ff16689 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -992,6 +992,8 @@ struct amdgpu_gfx {
         bool                            gfx_off_state; /* true: enabled, false: disabled */
         struct mutex                    gfx_off_mutex;
         uint32_t                        gfx_off_req_count; /* default 1, enable gfx off: dec 1, disable gfx off: add 1 */
+       struct delayed_work             gfx_off_delay_work;
+
         /* pipe reservation */
         struct mutex                    pipe_reserve_mutex;
         DECLARE_BITMAP                  (pipe_reserve_bitmap, AMDGPU_MAX_COMPUTE_QUEUES);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 122653f..f6a6fb5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1925,6 +1925,19 @@ static void amdgpu_device_ip_late_init_func_handler(struct work_struct *work)
                 DRM_ERROR("ib ring test failed (%d).\n", r);
 }

+static void amdgpu_device_delay_enable_gfx_off(struct work_struct *work)
+{
+       struct amdgpu_device *adev =
+               container_of(work, struct amdgpu_device, gfx.gfx_off_delay_work.work);
+
+       mutex_lock(&adev->gfx.gfx_off_mutex);
+       if (!adev->gfx.gfx_off_state && !adev->gfx.gfx_off_req_count) {
+               if (!amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, true))
+                       adev->gfx.gfx_off_state = true;
+       }
+       mutex_unlock(&adev->gfx.gfx_off_mutex);
+}
+
 /**
  * amdgpu_device_ip_suspend_phase1 - run suspend for hardware IPs (phase 1)
  *
@@ -2394,6 +2407,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,

         INIT_DELAYED_WORK(&adev->late_init_work,
                           amdgpu_device_ip_late_init_func_handler);
+       INIT_DELAYED_WORK(&adev->gfx.gfx_off_delay_work,
+                         amdgpu_device_delay_enable_gfx_off);

         adev->gfx.gfx_off_req_count = 1;
         adev->pm.ac_power = power_supply_is_system_supplied() > 0 ? true : false;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index 1cdb264..11d4d9f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -26,6 +26,9 @@
 #include "amdgpu.h"
 #include "amdgpu_gfx.h"

+/* 0.5 second timeout */
+#define GFX_OFF_DELAY_ENABLE         msecs_to_jiffies(500)
+
 /*
  * GPU scratch registers helpers function.
  */
@@ -360,6 +363,7 @@ void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable)
         if (!adev->powerplay.pp_funcs->set_powergating_by_smu)
                 return;

+
         mutex_lock(&adev->gfx.gfx_off_mutex);

         if (!enable)
@@ -368,11 +372,11 @@ void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable)
                 adev->gfx.gfx_off_req_count--;

         if (enable && !adev->gfx.gfx_off_state && !adev->gfx.gfx_off_req_count) {
-               if (!amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, true))
-                       adev->gfx.gfx_off_state = true;
+               schedule_delayed_work(&adev->gfx.gfx_off_delay_work, GFX_OFF_DELAY_ENABLE);
         } else if (!enable && adev->gfx.gfx_off_state) {
                 if (!amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, false))
                         adev->gfx.gfx_off_state = false;
         }
+
         mutex_unlock(&adev->gfx.gfx_off_mutex);
 }
--
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
amd-gfx Info Page - freedesktop.org<https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
lists.freedesktop.org
Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the following form. Use of all freedesktop.org lists is subject to our Code of Conduct.



[-- Attachment #1.2: Type: text/html, Size: 11802 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

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

* Re: [PATCH v2 2/4] drm/amdgpu: Put enable gfx off feature to a delay thread
       [not found]         ` <CY4PR12MB168727FC303FDB625880C440FB2F0-rpdhrqHFk06Y0SjTqZDccQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2018-07-30 10:49           ` Christian König
  0 siblings, 0 replies; 9+ messages in thread
From: Christian König @ 2018-07-30 10:49 UTC (permalink / raw)
  To: Zhu, Rex, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW


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

A bit large, maybe 100ms is enough.

Apart from that the set now looks good on first glance, but I really 
don't know that code that well.

Christian.

Am 30.07.2018 um 12:25 schrieb Zhu, Rex:
>
> One problem,
>
>
> I delay 500ms to enable gfx off feature.
>
>
> Any concern about the delay time?
>
>
> Best Regards
>
> Rex
>
>
> ------------------------------------------------------------------------
> *From:* amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of 
> Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
> *Sent:* Monday, July 30, 2018 5:23 PM
> *To:* amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> *Cc:* Zhu, Rex
> *Subject:* [PATCH v2 2/4] drm/amdgpu: Put enable gfx off feature to a 
> delay thread
> delay to enable gfx off feature to avoid gfx on/off frequently
> suggested by Alex and Evan.
>
> Signed-off-by: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  2 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 15 +++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c    |  8 ++++++--
>  3 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 188bc53..ff16689 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -992,6 +992,8 @@ struct amdgpu_gfx {
>          bool gfx_off_state; /* true: enabled, false: disabled */
>          struct mutex gfx_off_mutex;
>          uint32_t gfx_off_req_count; /* default 1, enable gfx off: dec 
> 1, disable gfx off: add 1 */
> +       struct delayed_work gfx_off_delay_work;
> +
>          /* pipe reservation */
>          struct mutex pipe_reserve_mutex;
>          DECLARE_BITMAP (pipe_reserve_bitmap, AMDGPU_MAX_COMPUTE_QUEUES);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 122653f..f6a6fb5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1925,6 +1925,19 @@ static void 
> amdgpu_device_ip_late_init_func_handler(struct work_struct *work)
>                  DRM_ERROR("ib ring test failed (%d).\n", r);
>  }
>
> +static void amdgpu_device_delay_enable_gfx_off(struct work_struct *work)
> +{
> +       struct amdgpu_device *adev =
> +               container_of(work, struct amdgpu_device, 
> gfx.gfx_off_delay_work.work);
> +
> +       mutex_lock(&adev->gfx.gfx_off_mutex);
> +       if (!adev->gfx.gfx_off_state && !adev->gfx.gfx_off_req_count) {
> +               if (!amdgpu_dpm_set_powergating_by_smu(adev, 
> AMD_IP_BLOCK_TYPE_GFX, true))
> +                       adev->gfx.gfx_off_state = true;
> +       }
> +       mutex_unlock(&adev->gfx.gfx_off_mutex);
> +}
> +
>  /**
>   * amdgpu_device_ip_suspend_phase1 - run suspend for hardware IPs 
> (phase 1)
>   *
> @@ -2394,6 +2407,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>
> INIT_DELAYED_WORK(&adev->late_init_work,
> amdgpu_device_ip_late_init_func_handler);
> + INIT_DELAYED_WORK(&adev->gfx.gfx_off_delay_work,
> + amdgpu_device_delay_enable_gfx_off);
>
>          adev->gfx.gfx_off_req_count = 1;
>          adev->pm.ac_power = power_supply_is_system_supplied() > 0 ? 
> true : false;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index 1cdb264..11d4d9f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -26,6 +26,9 @@
>  #include "amdgpu.h"
>  #include "amdgpu_gfx.h"
>
> +/* 0.5 second timeout */
> +#define GFX_OFF_DELAY_ENABLE msecs_to_jiffies(500)
> +
>  /*
>   * GPU scratch registers helpers function.
>   */
> @@ -360,6 +363,7 @@ void amdgpu_gfx_off_ctrl(struct amdgpu_device 
> *adev, bool enable)
>          if (!adev->powerplay.pp_funcs->set_powergating_by_smu)
>                  return;
>
> +
>          mutex_lock(&adev->gfx.gfx_off_mutex);
>
>          if (!enable)
> @@ -368,11 +372,11 @@ void amdgpu_gfx_off_ctrl(struct amdgpu_device 
> *adev, bool enable)
>                  adev->gfx.gfx_off_req_count--;
>
>          if (enable && !adev->gfx.gfx_off_state && 
> !adev->gfx.gfx_off_req_count) {
> -               if (!amdgpu_dpm_set_powergating_by_smu(adev, 
> AMD_IP_BLOCK_TYPE_GFX, true))
> -                       adev->gfx.gfx_off_state = true;
> + schedule_delayed_work(&adev->gfx.gfx_off_delay_work, 
> GFX_OFF_DELAY_ENABLE);
>          } else if (!enable && adev->gfx.gfx_off_state) {
>                  if (!amdgpu_dpm_set_powergating_by_smu(adev, 
> AMD_IP_BLOCK_TYPE_GFX, false))
>                          adev->gfx.gfx_off_state = false;
>          }
> +
> mutex_unlock(&adev->gfx.gfx_off_mutex);
>  }
> -- 
> 1.9.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> amd-gfx Info Page - freedesktop.org 
> <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
> lists.freedesktop.org
> Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the 
> following form. Use of all freedesktop.org lists is subject to our 
> Code of Conduct.
>
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[-- Attachment #1.2: Type: text/html, Size: 14926 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

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

* Re: [PATCH v2 1/4] drm/amdgpu: Add amdgpu_gfx_off_ctrl function
       [not found] ` <1532942625-26876-1-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
                     ` (2 preceding siblings ...)
  2018-07-30  9:23   ` [PATCH v2 4/4] drm/amdgpu: Disable gfx off if VCN is busy Rex Zhu
@ 2018-07-30 19:25   ` Felix Kuehling
  3 siblings, 0 replies; 9+ messages in thread
From: Felix Kuehling @ 2018-07-30 19:25 UTC (permalink / raw)
  To: Rex Zhu, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Oded Gabbay

[+Oded]

KFD on amd-staging-drm-next doesn't support Raven yet. It's coming soon
via Oded's branch.

To prepare for that, all MMIO access in amdgpu_amdkfd_gfx_v9.c should be
protected by disabling GFXOFF. Since enabling GFXOFF is delayed in your
latest patch series, it shouldn't cause performance issues. It's not
going to do anything on Vega10, which doesn't have the GFXOFF feature.
But it will take effect as soon as Raven support is enabled for KFD.

Regards,
  Felix


On 2018-07-30 05:23 AM, Rex Zhu wrote:
> v2:
>    1. drop the special handling for the hw IP
>       suggested by hawking and Christian.
>    2. refine the variable name suggested by Flora.
>
> This funciton as the entry of gfx off ctrl feature.
> we arbitrat gfx off feature enable/disable in this
> function.
>
> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  5 +++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c    | 36 ++++++++++++++++++++++++++++++
>  3 files changed, 43 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 5b7bb58..188bc53 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -988,6 +988,10 @@ struct amdgpu_gfx {
>  	/* NGG */
>  	struct amdgpu_ngg		ngg;
>  
> +	/* gfx off */
> +	bool                            gfx_off_state; /* true: enabled, false: disabled */
> +	struct mutex                    gfx_off_mutex;
> +	uint32_t                        gfx_off_req_count; /* default 1, enable gfx off: dec 1, disable gfx off: add 1 */
>  	/* pipe reservation */
>  	struct mutex			pipe_reserve_mutex;
>  	DECLARE_BITMAP			(pipe_reserve_bitmap, AMDGPU_MAX_COMPUTE_QUEUES);
> @@ -1815,6 +1819,7 @@ void amdgpu_device_program_register_sequence(struct amdgpu_device *adev,
>  					     const u32 array_size);
>  
>  bool amdgpu_device_is_px(struct drm_device *dev);
> +void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable);
>  /* atpx handler */
>  #if defined(CONFIG_VGA_SWITCHEROO)
>  void amdgpu_register_atpx_handler(void);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 745f760..122653f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2367,6 +2367,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>  	mutex_init(&adev->gfx.gpu_clock_mutex);
>  	mutex_init(&adev->srbm_mutex);
>  	mutex_init(&adev->gfx.pipe_reserve_mutex);
> +	mutex_init(&adev->gfx.gfx_off_mutex);
>  	mutex_init(&adev->grbm_idx_mutex);
>  	mutex_init(&adev->mn_lock);
>  	mutex_init(&adev->virt.vf_errors.lock);
> @@ -2394,6 +2395,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>  	INIT_DELAYED_WORK(&adev->late_init_work,
>  			  amdgpu_device_ip_late_init_func_handler);
>  
> +	adev->gfx.gfx_off_req_count = 1;
>  	adev->pm.ac_power = power_supply_is_system_supplied() > 0 ? true : false;
>  
>  	/* Registers mapping */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index 239bf2a..1cdb264 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -340,3 +340,39 @@ void amdgpu_gfx_compute_mqd_sw_fini(struct amdgpu_device *adev)
>  			      &ring->mqd_gpu_addr,
>  			      &ring->mqd_ptr);
>  }
> +
> +/* amdgpu_gfx_off_ctrl - Handle gfx off feature enable/disable
> + *
> + * @adev: amdgpu_device pointer
> + * @bool enable true: enable gfx off feature, false: disable gfx off feature
> + *
> + * 1. gfx off feature will be enabled by gfx ip after gfx cg gp enabled.
> + * 2. other client can send request to disable gfx off feature, the request should be honored.
> + * 3. other client can cancel their request of disable gfx off feature
> + * 4. other client should not send request to enable gfx off feature before disable gfx off feature.
> + */
> +
> +void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable)
> +{
> +	if (!(adev->powerplay.pp_feature & PP_GFXOFF_MASK))
> +		return;
> +
> +	if (!adev->powerplay.pp_funcs->set_powergating_by_smu)
> +		return;
> +
> +	mutex_lock(&adev->gfx.gfx_off_mutex);
> +
> +	if (!enable)
> +		adev->gfx.gfx_off_req_count++;
> +	else if (adev->gfx.gfx_off_req_count > 0)
> +		adev->gfx.gfx_off_req_count--;
> +
> +	if (enable && !adev->gfx.gfx_off_state && !adev->gfx.gfx_off_req_count) {
> +		if (!amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, true))
> +			adev->gfx.gfx_off_state = true;
> +	} else if (!enable && adev->gfx.gfx_off_state) {
> +		if (!amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, false))
> +			adev->gfx.gfx_off_state = false;
> +	}
> +	mutex_unlock(&adev->gfx.gfx_off_mutex);
> +}

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

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

* Re: [PATCH v2 2/4] drm/amdgpu: Put enable gfx off feature to a delay thread
       [not found]     ` <1532942625-26876-2-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
  2018-07-30 10:25       ` Zhu, Rex
@ 2018-07-30 19:33       ` Felix Kuehling
       [not found]         ` <c0edeb5f-140d-4572-d587-375a6838ca5f-5C7GfCeVMHo@public.gmane.org>
  1 sibling, 1 reply; 9+ messages in thread
From: Felix Kuehling @ 2018-07-30 19:33 UTC (permalink / raw)
  To: Rex Zhu, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

NACK.

See inline ...


On 2018-07-30 05:23 AM, Rex Zhu wrote:
> delay to enable gfx off feature to avoid gfx on/off frequently
> suggested by Alex and Evan.
>
> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  2 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 15 +++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c    |  8 ++++++--
>  3 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 188bc53..ff16689 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -992,6 +992,8 @@ struct amdgpu_gfx {
>  	bool                            gfx_off_state; /* true: enabled, false: disabled */
>  	struct mutex                    gfx_off_mutex;
>  	uint32_t                        gfx_off_req_count; /* default 1, enable gfx off: dec 1, disable gfx off: add 1 */
> +	struct delayed_work             gfx_off_delay_work;
> +
>  	/* pipe reservation */
>  	struct mutex			pipe_reserve_mutex;
>  	DECLARE_BITMAP			(pipe_reserve_bitmap, AMDGPU_MAX_COMPUTE_QUEUES);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 122653f..f6a6fb5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1925,6 +1925,19 @@ static void amdgpu_device_ip_late_init_func_handler(struct work_struct *work)
>  		DRM_ERROR("ib ring test failed (%d).\n", r);
>  }
>  
> +static void amdgpu_device_delay_enable_gfx_off(struct work_struct *work)
> +{
> +	struct amdgpu_device *adev =
> +		container_of(work, struct amdgpu_device, gfx.gfx_off_delay_work.work);
> +
> +	mutex_lock(&adev->gfx.gfx_off_mutex);
> +	if (!adev->gfx.gfx_off_state && !adev->gfx.gfx_off_req_count) {
> +		if (!amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, true))
> +			adev->gfx.gfx_off_state = true;
> +	}
> +	mutex_unlock(&adev->gfx.gfx_off_mutex);
> +}
> +
>  /**
>   * amdgpu_device_ip_suspend_phase1 - run suspend for hardware IPs (phase 1)
>   *
> @@ -2394,6 +2407,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>  
>  	INIT_DELAYED_WORK(&adev->late_init_work,
>  			  amdgpu_device_ip_late_init_func_handler);
> +	INIT_DELAYED_WORK(&adev->gfx.gfx_off_delay_work,
> +			  amdgpu_device_delay_enable_gfx_off);
>  
>  	adev->gfx.gfx_off_req_count = 1;
>  	adev->pm.ac_power = power_supply_is_system_supplied() > 0 ? true : false;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index 1cdb264..11d4d9f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -26,6 +26,9 @@
>  #include "amdgpu.h"
>  #include "amdgpu_gfx.h"
>  
> +/* 0.5 second timeout */
> +#define GFX_OFF_DELAY_ENABLE         msecs_to_jiffies(500)
> +
>  /*
>   * GPU scratch registers helpers function.
>   */
> @@ -360,6 +363,7 @@ void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable)
>  	if (!adev->powerplay.pp_funcs->set_powergating_by_smu)
>  		return;
>  
> +
>  	mutex_lock(&adev->gfx.gfx_off_mutex);
>  
>  	if (!enable)
> @@ -368,11 +372,11 @@ void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable)
>  		adev->gfx.gfx_off_req_count--;
>  
>  	if (enable && !adev->gfx.gfx_off_state && !adev->gfx.gfx_off_req_count) {
> -		if (!amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, true))
> -			adev->gfx.gfx_off_state = true;
> +		schedule_delayed_work(&adev->gfx.gfx_off_delay_work, GFX_OFF_DELAY_ENABLE);
>  	} else if (!enable && adev->gfx.gfx_off_state) {

There may still be delayed work pending to enable GFXOFF. You need to
use cancel_delayed_work_sync to cancel it and make sure GFXOFF doesn't
get enabled again while your caller still needs it disabled.

That may also wait for a worker function that has already started.
Therefore you need to cancel before taking the gfx_off_mutex, otherwise
you can deadlock with the worker function trying to take the lock as well.

Regards,
  Felix

>  		if (!amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, false))
>  			adev->gfx.gfx_off_state = false;
>  	}
> +
>  	mutex_unlock(&adev->gfx.gfx_off_mutex);
>  }

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

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

* Re: [PATCH v2 2/4] drm/amdgpu: Put enable gfx off feature to a delay thread
       [not found]         ` <c0edeb5f-140d-4572-d587-375a6838ca5f-5C7GfCeVMHo@public.gmane.org>
@ 2018-07-30 20:07           ` Felix Kuehling
  0 siblings, 0 replies; 9+ messages in thread
From: Felix Kuehling @ 2018-07-30 20:07 UTC (permalink / raw)
  To: Rex Zhu, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Sorry, I just saw your discussion with Evan. Checking the conditions in
the worker function should work. Feel free to add my Reviewed-by.

Regards,
  Felix


On 2018-07-30 03:33 PM, Felix Kuehling wrote:
> NACK.
>
> See inline ...
>
>
> On 2018-07-30 05:23 AM, Rex Zhu wrote:
>> delay to enable gfx off feature to avoid gfx on/off frequently
>> suggested by Alex and Evan.
>>
>> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  2 ++
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 15 +++++++++++++++
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c    |  8 ++++++--
>>  3 files changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 188bc53..ff16689 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -992,6 +992,8 @@ struct amdgpu_gfx {
>>  	bool                            gfx_off_state; /* true: enabled, false: disabled */
>>  	struct mutex                    gfx_off_mutex;
>>  	uint32_t                        gfx_off_req_count; /* default 1, enable gfx off: dec 1, disable gfx off: add 1 */
>> +	struct delayed_work             gfx_off_delay_work;
>> +
>>  	/* pipe reservation */
>>  	struct mutex			pipe_reserve_mutex;
>>  	DECLARE_BITMAP			(pipe_reserve_bitmap, AMDGPU_MAX_COMPUTE_QUEUES);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 122653f..f6a6fb5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -1925,6 +1925,19 @@ static void amdgpu_device_ip_late_init_func_handler(struct work_struct *work)
>>  		DRM_ERROR("ib ring test failed (%d).\n", r);
>>  }
>>  
>> +static void amdgpu_device_delay_enable_gfx_off(struct work_struct *work)
>> +{
>> +	struct amdgpu_device *adev =
>> +		container_of(work, struct amdgpu_device, gfx.gfx_off_delay_work.work);
>> +
>> +	mutex_lock(&adev->gfx.gfx_off_mutex);
>> +	if (!adev->gfx.gfx_off_state && !adev->gfx.gfx_off_req_count) {
>> +		if (!amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, true))
>> +			adev->gfx.gfx_off_state = true;
>> +	}
>> +	mutex_unlock(&adev->gfx.gfx_off_mutex);
>> +}
>> +
>>  /**
>>   * amdgpu_device_ip_suspend_phase1 - run suspend for hardware IPs (phase 1)
>>   *
>> @@ -2394,6 +2407,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>>  
>>  	INIT_DELAYED_WORK(&adev->late_init_work,
>>  			  amdgpu_device_ip_late_init_func_handler);
>> +	INIT_DELAYED_WORK(&adev->gfx.gfx_off_delay_work,
>> +			  amdgpu_device_delay_enable_gfx_off);
>>  
>>  	adev->gfx.gfx_off_req_count = 1;
>>  	adev->pm.ac_power = power_supply_is_system_supplied() > 0 ? true : false;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> index 1cdb264..11d4d9f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> @@ -26,6 +26,9 @@
>>  #include "amdgpu.h"
>>  #include "amdgpu_gfx.h"
>>  
>> +/* 0.5 second timeout */
>> +#define GFX_OFF_DELAY_ENABLE         msecs_to_jiffies(500)
>> +
>>  /*
>>   * GPU scratch registers helpers function.
>>   */
>> @@ -360,6 +363,7 @@ void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable)
>>  	if (!adev->powerplay.pp_funcs->set_powergating_by_smu)
>>  		return;
>>  
>> +
>>  	mutex_lock(&adev->gfx.gfx_off_mutex);
>>  
>>  	if (!enable)
>> @@ -368,11 +372,11 @@ void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable)
>>  		adev->gfx.gfx_off_req_count--;
>>  
>>  	if (enable && !adev->gfx.gfx_off_state && !adev->gfx.gfx_off_req_count) {
>> -		if (!amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, true))
>> -			adev->gfx.gfx_off_state = true;
>> +		schedule_delayed_work(&adev->gfx.gfx_off_delay_work, GFX_OFF_DELAY_ENABLE);
>>  	} else if (!enable && adev->gfx.gfx_off_state) {
> There may still be delayed work pending to enable GFXOFF. You need to
> use cancel_delayed_work_sync to cancel it and make sure GFXOFF doesn't
> get enabled again while your caller still needs it disabled.
>
> That may also wait for a worker function that has already started.
> Therefore you need to cancel before taking the gfx_off_mutex, otherwise
> you can deadlock with the worker function trying to take the lock as well.
>
> Regards,
>   Felix
>
>>  		if (!amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, false))
>>  			adev->gfx.gfx_off_state = false;
>>  	}
>> +
>>  	mutex_unlock(&adev->gfx.gfx_off_mutex);
>>  }
> _______________________________________________
> 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] 9+ messages in thread

end of thread, other threads:[~2018-07-30 20:07 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-30  9:23 [PATCH v2 1/4] drm/amdgpu: Add amdgpu_gfx_off_ctrl function Rex Zhu
     [not found] ` <1532942625-26876-1-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
2018-07-30  9:23   ` [PATCH v2 2/4] drm/amdgpu: Put enable gfx off feature to a delay thread Rex Zhu
     [not found]     ` <1532942625-26876-2-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
2018-07-30 10:25       ` Zhu, Rex
     [not found]         ` <CY4PR12MB168727FC303FDB625880C440FB2F0-rpdhrqHFk06Y0SjTqZDccQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2018-07-30 10:49           ` Christian König
2018-07-30 19:33       ` Felix Kuehling
     [not found]         ` <c0edeb5f-140d-4572-d587-375a6838ca5f-5C7GfCeVMHo@public.gmane.org>
2018-07-30 20:07           ` Felix Kuehling
2018-07-30  9:23   ` [PATCH v2 3/4] drm/amdgpu: gfx ip ctrl gfx off via amdgpu_gfx_off_ctrl Rex Zhu
2018-07-30  9:23   ` [PATCH v2 4/4] drm/amdgpu: Disable gfx off if VCN is busy Rex Zhu
2018-07-30 19:25   ` [PATCH v2 1/4] drm/amdgpu: Add amdgpu_gfx_off_ctrl function Felix Kuehling

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.