[PATCH 01/11] drm/amdgpu/gfx9: manually control gfxoff for CS on RV

[PATCH 01/11] drm/amdgpu/gfx9: manually control gfxoff for CS on RV

[Alex Deucher]

When mesa started using compute queues more often
we started seeing additional hangs with compute queues.
Disabling gfxoff seems to mitigate that.  Manually
control gfxoff and gfx pg with command submissions to avoid
any issues related to gfxoff.  KFD already does the same
thing for these chips.

v2: limit to compute
v3: limit to APUs
v4: limit to Raven/PCO
v5: only update the compute ring_funcs
v6: Disable GFX PG
v7: adjust order

Suggested-by: Błażej Szczygieł <mumei6102@gmail.com>
Suggested-by: Sergey Kovalenko <seryoga.engineering@gmail.com>
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3861
Link: https://lists.freedesktop.org/archives/amd-gfx/2025-January/119116.html
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 36 +++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 6aa713cfa2f3e..a666832ecefea 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -7443,6 +7443,38 @@ static void gfx_v9_0_ring_emit_cleaner_shader(struct amdgpu_ring *ring)
 	amdgpu_ring_write(ring, 0);  /* RESERVED field, programmed to zero */
 }
 
+static void gfx_v9_0_ring_begin_use_compute(struct amdgpu_ring *ring)
+{
+	struct amdgpu_device *adev = ring->adev;
+	struct amdgpu_ip_block *gfx_block =
+		amdgpu_device_ip_get_ip_block(adev, AMD_IP_BLOCK_TYPE_GFX);
+
+	amdgpu_gfx_enforce_isolation_ring_begin_use(ring);
+
+	/* Raven and PCO APUs seem to have stability issues
+	 * with compute and gfxoff and gfx pg.  Disable gfx pg during
+	 * submission and allow again afterwards.
+	 */
+	if (gfx_block && amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 1, 0))
+		gfx_v9_0_set_powergating_state(gfx_block, AMD_PG_STATE_UNGATE);
+}
+
+static void gfx_v9_0_ring_end_use_compute(struct amdgpu_ring *ring)
+{
+	struct amdgpu_device *adev = ring->adev;
+	struct amdgpu_ip_block *gfx_block =
+		amdgpu_device_ip_get_ip_block(adev, AMD_IP_BLOCK_TYPE_GFX);
+
+	/* Raven and PCO APUs seem to have stability issues
+	 * with compute and gfxoff and gfx pg.  Disable gfx pg during
+	 * submission and allow again afterwards.
+	 */
+	if (gfx_block && amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 1, 0))
+		gfx_v9_0_set_powergating_state(gfx_block, AMD_PG_STATE_GATE);
+
+	amdgpu_gfx_enforce_isolation_ring_end_use(ring);
+}
+
 static const struct amd_ip_funcs gfx_v9_0_ip_funcs = {
 	.name = "gfx_v9_0",
 	.early_init = gfx_v9_0_early_init,
@@ -7619,8 +7651,8 @@ static const struct amdgpu_ring_funcs gfx_v9_0_ring_funcs_compute = {
 	.emit_wave_limit = gfx_v9_0_emit_wave_limit,
 	.reset = gfx_v9_0_reset_kcq,
 	.emit_cleaner_shader = gfx_v9_0_ring_emit_cleaner_shader,
-	.begin_use = amdgpu_gfx_enforce_isolation_ring_begin_use,
-	.end_use = amdgpu_gfx_enforce_isolation_ring_end_use,
+	.begin_use = gfx_v9_0_ring_begin_use_compute,
+	.end_use = gfx_v9_0_ring_end_use_compute,
 };
 
 static const struct amdgpu_ring_funcs gfx_v9_0_ring_funcs_kiq = {
-- 
2.48.1

[PATCH 02/11] drm/amdgpu: bump version for RV/PCO compute fix

[Alex Deucher]

Bump the driver version for RV/PCO compute stability fix
so mesa can use this check to enable compute queues on
RV/PCO.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 69e532e34950e..95dd2fbc26c54 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -122,9 +122,10 @@
  * - 3.58.0 - Add GFX12 DCC support
  * - 3.59.0 - Cleared VRAM
  * - 3.60.0 - Add AMDGPU_TILING_GFX12_DCC_WRITE_COMPRESS_DISABLE (Vulkan requirement)
+ * - 3.61.0 - Contains fix for RV/PCO compute queues
  */
 #define KMS_DRIVER_MAJOR	3
-#define KMS_DRIVER_MINOR	60
+#define KMS_DRIVER_MINOR	61
 #define KMS_DRIVER_PATCHLEVEL	0
 
 /*
-- 
2.48.1

[PATCH 03/11] drm/amdgpu/gfx: add amdgpu_gfx_off_ctrl_immediate()

[Alex Deucher]

Same as amdgpu_gfx_off_ctrl(), but without the delay
for gfxoff disallow.

Suggested-by: Błażej Szczygieł <mumei6102@gmail.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 53 +++++++++++++++++++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h |  1 +
 2 files changed, 41 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index 645efe002d068..27f5318c3a26c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -771,18 +771,8 @@ int amdgpu_gfx_enable_kgq(struct amdgpu_device *adev, int xcc_id)
 	return r;
 }
 
-/* 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)
+static void amdgpu_gfx_do_off_ctrl(struct amdgpu_device *adev, bool enable,
+				   bool no_delay)
 {
 	unsigned long delay = GFX_OFF_DELAY_ENABLE;
 
@@ -804,7 +794,7 @@ void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable)
 		if (adev->gfx.gfx_off_req_count == 0 &&
 		    !adev->gfx.gfx_off_state) {
 			/* If going to s2idle, no need to wait */
-			if (adev->in_s0ix) {
+			if (no_delay) {
 				if (!amdgpu_dpm_set_powergating_by_smu(adev,
 						AMD_IP_BLOCK_TYPE_GFX, true, 0))
 					adev->gfx.gfx_off_state = true;
@@ -836,6 +826,43 @@ void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable)
 	mutex_unlock(&adev->gfx.gfx_off_mutex);
 }
 
+/* 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 pg 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.
+ *
+ * gfx off allow will be delayed by GFX_OFF_DELAY_ENABLE ms.
+ */
+void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable)
+{
+	/* If going to s2idle, no need to wait */
+	bool no_delay = adev->in_s0ix ? true : false;
+
+	amdgpu_gfx_do_off_ctrl(adev, enable, no_delay);
+}
+
+/* amdgpu_gfx_off_ctrl_immediate - 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 pg 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.
+ *
+ * gfx off allow will be issued immediately.
+ */
+void amdgpu_gfx_off_ctrl_immediate(struct amdgpu_device *adev, bool enable)
+{
+	amdgpu_gfx_do_off_ctrl(adev, enable, true);
+}
+
 int amdgpu_set_gfx_off_residency(struct amdgpu_device *adev, bool value)
 {
 	int r = 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
index 6c84598caec21..ddf4533614bac 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -554,6 +554,7 @@ int amdgpu_gfx_me_queue_to_bit(struct amdgpu_device *adev, int me,
 bool amdgpu_gfx_is_me_queue_enabled(struct amdgpu_device *adev, int me,
 				    int pipe, int queue);
 void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable);
+void amdgpu_gfx_off_ctrl_immediate(struct amdgpu_device *adev, bool enable);
 int amdgpu_get_gfx_off_status(struct amdgpu_device *adev, uint32_t *value);
 int amdgpu_gfx_ras_late_init(struct amdgpu_device *adev, struct ras_common_if *ras_block);
 void amdgpu_gfx_ras_fini(struct amdgpu_device *adev);
-- 
2.48.1

[PATCH 04/11] drm/amdgpu/gfx9: use amdgpu_gfx_off_ctrl_immediate() for PG

[Alex Deucher]

Use amdgpu_gfx_off_ctrl_immediate() when powergating.
There's no need for the delay in gfx off allow.  The
powergating is dynamically disabled/enabled as for
RV/PCO on compute queues and allowing gfx off again as
soon the job is submitted improves power savings.

Suggested-by: Błażej Szczygieł <mumei6102@gmail.com>
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3861
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index a666832ecefea..dbb9df7913316 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -5241,7 +5241,7 @@ static int gfx_v9_0_set_powergating_state(struct amdgpu_ip_block *ip_block,
 	case IP_VERSION(9, 1, 0):
 	case IP_VERSION(9, 3, 0):
 		if (!enable)
-			amdgpu_gfx_off_ctrl(adev, false);
+			amdgpu_gfx_off_ctrl_immediate(adev, false);
 
 		if (adev->pg_flags & AMD_PG_SUPPORT_RLC_SMU_HS) {
 			gfx_v9_0_enable_sck_slow_down_on_power_up(adev, true);
@@ -5263,10 +5263,10 @@ static int gfx_v9_0_set_powergating_state(struct amdgpu_ip_block *ip_block,
 		gfx_v9_0_update_gfx_mg_power_gating(adev, enable);
 
 		if (enable)
-			amdgpu_gfx_off_ctrl(adev, true);
+			amdgpu_gfx_off_ctrl_immediate(adev, true);
 		break;
 	case IP_VERSION(9, 2, 1):
-		amdgpu_gfx_off_ctrl(adev, enable);
+		amdgpu_gfx_off_ctrl_immediate(adev, enable);
 		break;
 	default:
 		break;
-- 
2.48.1

Re: [PATCH 04/11] drm/amdgpu/gfx9: use amdgpu_gfx_off_ctrl_immediate() for PG

[Lazar, Lijo]

On 2/4/2025 3:13 AM, Alex Deucher wrote:
> Use amdgpu_gfx_off_ctrl_immediate() when powergating.
> There's no need for the delay in gfx off allow.  The
> powergating is dynamically disabled/enabled as for
> RV/PCO on compute queues and allowing gfx off again as
> soon the job is submitted improves power savings.
> 
> Suggested-by: Błażej Szczygieł <mumei6102@gmail.com>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3861
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

Patches 1-4 are

	Reviewed-by: Lijo Lazar <lijo.lazar@amd.com>

with a special note that the CGCG and PG settings may be compared with
Windows source later. Skimming over, noticed one change.

Thanks,
Lijo
> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index a666832ecefea..dbb9df7913316 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -5241,7 +5241,7 @@ static int gfx_v9_0_set_powergating_state(struct amdgpu_ip_block *ip_block,
>  	case IP_VERSION(9, 1, 0):
>  	case IP_VERSION(9, 3, 0):
>  		if (!enable)
> -			amdgpu_gfx_off_ctrl(adev, false);
> +			amdgpu_gfx_off_ctrl_immediate(adev, false);
>  
>  		if (adev->pg_flags & AMD_PG_SUPPORT_RLC_SMU_HS) {
>  			gfx_v9_0_enable_sck_slow_down_on_power_up(adev, true);
> @@ -5263,10 +5263,10 @@ static int gfx_v9_0_set_powergating_state(struct amdgpu_ip_block *ip_block,
>  		gfx_v9_0_update_gfx_mg_power_gating(adev, enable);
>  
>  		if (enable)
> -			amdgpu_gfx_off_ctrl(adev, true);
> +			amdgpu_gfx_off_ctrl_immediate(adev, true);
>  		break;
>  	case IP_VERSION(9, 2, 1):
> -		amdgpu_gfx_off_ctrl(adev, enable);
> +		amdgpu_gfx_off_ctrl_immediate(adev, enable);
>  		break;
>  	default:
>  		break;

[PATCH 05/11] drm/amdgpu/sdma5.2: use amdgpu_gfx_off_ctrl_immediate()

[Alex Deucher]

In begin_use/end_use use amdgpu_gfx_off_ctrl_immediate()
rather than amdgpu_gfx_off_ctrl() as we don't need the
extra delay before we allow gfxoff again.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
index b1818e87889a2..7f2e1962b5755 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
@@ -1882,7 +1882,7 @@ static void sdma_v5_2_ring_begin_use(struct amdgpu_ring *ring)
 	 * doorbells when entering PG.  If you remove this, update
 	 * sdma_v5_2_ring_set_wptr() as well!
 	 */
-	amdgpu_gfx_off_ctrl(adev, false);
+	amdgpu_gfx_off_ctrl_immediate(adev, false);
 }
 
 static void sdma_v5_2_ring_end_use(struct amdgpu_ring *ring)
@@ -1893,7 +1893,7 @@ static void sdma_v5_2_ring_end_use(struct amdgpu_ring *ring)
 	 * disallow GFXOFF in some cases leading to
 	 * hangs in SDMA.  Allow GFXOFF when SDMA is complete.
 	 */
-	amdgpu_gfx_off_ctrl(adev, true);
+	amdgpu_gfx_off_ctrl_immediate(adev, true);
 }
 
 static void sdma_v5_2_print_ip_state(struct amdgpu_ip_block *ip_block, struct drm_printer *p)
-- 
2.48.1

Re: [PATCH 05/11] drm/amdgpu/sdma5.2: use amdgpu_gfx_off_ctrl_immediate()

[Alex Deucher]

ping on patches 1-5 of this series.

Alex

On Mon, Feb 3, 2025 at 4:52 PM Alex Deucher <alexander.deucher@amd.com> wrote:
>
> In begin_use/end_use use amdgpu_gfx_off_ctrl_immediate()
> rather than amdgpu_gfx_off_ctrl() as we don't need the
> extra delay before we allow gfxoff again.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> index b1818e87889a2..7f2e1962b5755 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> @@ -1882,7 +1882,7 @@ static void sdma_v5_2_ring_begin_use(struct amdgpu_ring *ring)
>          * doorbells when entering PG.  If you remove this, update
>          * sdma_v5_2_ring_set_wptr() as well!
>          */
> -       amdgpu_gfx_off_ctrl(adev, false);
> +       amdgpu_gfx_off_ctrl_immediate(adev, false);
>  }
>
>  static void sdma_v5_2_ring_end_use(struct amdgpu_ring *ring)
> @@ -1893,7 +1893,7 @@ static void sdma_v5_2_ring_end_use(struct amdgpu_ring *ring)
>          * disallow GFXOFF in some cases leading to
>          * hangs in SDMA.  Allow GFXOFF when SDMA is complete.
>          */
> -       amdgpu_gfx_off_ctrl(adev, true);
> +       amdgpu_gfx_off_ctrl_immediate(adev, true);
>  }
>
>  static void sdma_v5_2_print_ip_state(struct amdgpu_ip_block *ip_block, struct drm_printer *p)
> --
> 2.48.1
>

Re: [PATCH 05/11] drm/amdgpu/sdma5.2: use amdgpu_gfx_off_ctrl_immediate()

[Lazar, Lijo]

On 2/4/2025 3:13 AM, Alex Deucher wrote:
> In begin_use/end_use use amdgpu_gfx_off_ctrl_immediate()
> rather than amdgpu_gfx_off_ctrl() as we don't need the
> extra delay before we allow gfxoff again.
> 
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

Won't this cause unnecessary GFX allows since sdma jobs could also be
used for clearing the buffers?

Thanks,
Lijo

> ---
>  drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> index b1818e87889a2..7f2e1962b5755 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> @@ -1882,7 +1882,7 @@ static void sdma_v5_2_ring_begin_use(struct amdgpu_ring *ring)
>  	 * doorbells when entering PG.  If you remove this, update
>  	 * sdma_v5_2_ring_set_wptr() as well!
>  	 */
> -	amdgpu_gfx_off_ctrl(adev, false);
> +	amdgpu_gfx_off_ctrl_immediate(adev, false);
>  }
>  
>  static void sdma_v5_2_ring_end_use(struct amdgpu_ring *ring)
> @@ -1893,7 +1893,7 @@ static void sdma_v5_2_ring_end_use(struct amdgpu_ring *ring)
>  	 * disallow GFXOFF in some cases leading to
>  	 * hangs in SDMA.  Allow GFXOFF when SDMA is complete.
>  	 */
> -	amdgpu_gfx_off_ctrl(adev, true);
> +	amdgpu_gfx_off_ctrl_immediate(adev, true);
>  }
>  
>  static void sdma_v5_2_print_ip_state(struct amdgpu_ip_block *ip_block, struct drm_printer *p)

Re: [PATCH 05/11] drm/amdgpu/sdma5.2: use amdgpu_gfx_off_ctrl_immediate()

[Alex Deucher]

On Thu, Feb 6, 2025 at 10:17 AM Lazar, Lijo <lijo.lazar@amd.com> wrote:
>
>
>
> On 2/4/2025 3:13 AM, Alex Deucher wrote:
> > In begin_use/end_use use amdgpu_gfx_off_ctrl_immediate()
> > rather than amdgpu_gfx_off_ctrl() as we don't need the
> > extra delay before we allow gfxoff again.
> >
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>
> Won't this cause unnecessary GFX allows since sdma jobs could also be
> used for clearing the buffers?

The calls are already there, this just switches to gfxoff allow
sooner, reducing the time where gfxoff is disallowed.  Should save a
bit of power.

Alex

>
> Thanks,
> Lijo
>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > index b1818e87889a2..7f2e1962b5755 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > @@ -1882,7 +1882,7 @@ static void sdma_v5_2_ring_begin_use(struct amdgpu_ring *ring)
> >        * doorbells when entering PG.  If you remove this, update
> >        * sdma_v5_2_ring_set_wptr() as well!
> >        */
> > -     amdgpu_gfx_off_ctrl(adev, false);
> > +     amdgpu_gfx_off_ctrl_immediate(adev, false);
> >  }
> >
> >  static void sdma_v5_2_ring_end_use(struct amdgpu_ring *ring)
> > @@ -1893,7 +1893,7 @@ static void sdma_v5_2_ring_end_use(struct amdgpu_ring *ring)
> >        * disallow GFXOFF in some cases leading to
> >        * hangs in SDMA.  Allow GFXOFF when SDMA is complete.
> >        */
> > -     amdgpu_gfx_off_ctrl(adev, true);
> > +     amdgpu_gfx_off_ctrl_immediate(adev, true);
> >  }
> >
> >  static void sdma_v5_2_print_ip_state(struct amdgpu_ip_block *ip_block, struct drm_printer *p)
>

Re: [PATCH 05/11] drm/amdgpu/sdma5.2: use amdgpu_gfx_off_ctrl_immediate()

[Lazar, Lijo]

[-- Attachment #1: Type: text/plain, Size: 2682 bytes --]

[Public]

Specifically, was talking of examples like delayed bo deletes (don't know how they could be in real world). That is an example where buffer deletion is queued up and buffer clearing jobs will start sequentially. With the new sequence, allow will be sent and could immediately be followed by a disallow. With the delayed ones that won't happen.

Or is the gfxoff delay that long which removes considerable power saving?

Thanks,
Lijo
________________________________
From: Alex Deucher <alexdeucher@gmail.com>
Sent: Thursday, February 6, 2025 8:55:13 PM
To: Lazar, Lijo <Lijo.Lazar@amd.com>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 05/11] drm/amdgpu/sdma5.2: use amdgpu_gfx_off_ctrl_immediate()

On Thu, Feb 6, 2025 at 10:17 AM Lazar, Lijo <lijo.lazar@amd.com> wrote:
>
>
>
> On 2/4/2025 3:13 AM, Alex Deucher wrote:
> > In begin_use/end_use use amdgpu_gfx_off_ctrl_immediate()
> > rather than amdgpu_gfx_off_ctrl() as we don't need the
> > extra delay before we allow gfxoff again.
> >
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>
> Won't this cause unnecessary GFX allows since sdma jobs could also be
> used for clearing the buffers?

The calls are already there, this just switches to gfxoff allow
sooner, reducing the time where gfxoff is disallowed.  Should save a
bit of power.

Alex

>
> Thanks,
> Lijo
>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > index b1818e87889a2..7f2e1962b5755 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > @@ -1882,7 +1882,7 @@ static void sdma_v5_2_ring_begin_use(struct amdgpu_ring *ring)
> >        * doorbells when entering PG.  If you remove this, update
> >        * sdma_v5_2_ring_set_wptr() as well!
> >        */
> > -     amdgpu_gfx_off_ctrl(adev, false);
> > +     amdgpu_gfx_off_ctrl_immediate(adev, false);
> >  }
> >
> >  static void sdma_v5_2_ring_end_use(struct amdgpu_ring *ring)
> > @@ -1893,7 +1893,7 @@ static void sdma_v5_2_ring_end_use(struct amdgpu_ring *ring)
> >        * disallow GFXOFF in some cases leading to
> >        * hangs in SDMA.  Allow GFXOFF when SDMA is complete.
> >        */
> > -     amdgpu_gfx_off_ctrl(adev, true);
> > +     amdgpu_gfx_off_ctrl_immediate(adev, true);
> >  }
> >
> >  static void sdma_v5_2_print_ip_state(struct amdgpu_ip_block *ip_block, struct drm_printer *p)
>

[-- Attachment #2: Type: text/html, Size: 4313 bytes --]

Re: [PATCH 05/11] drm/amdgpu/sdma5.2: use amdgpu_gfx_off_ctrl_immediate()

[Alex Deucher]

On Thu, Feb 6, 2025 at 10:36 AM Lazar, Lijo <Lijo.Lazar@amd.com> wrote:
>
> [Public]
>
>
> Specifically, was talking of examples like delayed bo deletes (don't know how they could be in real world). That is an example where buffer deletion is queued up and buffer clearing jobs will start sequentially. With the new sequence, allow will be sent and could immediately be followed by a disallow. With the delayed ones that won't happen.
>
> Or is the gfxoff delay that long which removes considerable power saving?

It's 100ms, so it's pretty long.

Alex

>
> Thanks,
> Lijo
> ________________________________
> From: Alex Deucher <alexdeucher@gmail.com>
> Sent: Thursday, February 6, 2025 8:55:13 PM
> To: Lazar, Lijo <Lijo.Lazar@amd.com>
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
> Subject: Re: [PATCH 05/11] drm/amdgpu/sdma5.2: use amdgpu_gfx_off_ctrl_immediate()
>
> On Thu, Feb 6, 2025 at 10:17 AM Lazar, Lijo <lijo.lazar@amd.com> wrote:
> >
> >
> >
> > On 2/4/2025 3:13 AM, Alex Deucher wrote:
> > > In begin_use/end_use use amdgpu_gfx_off_ctrl_immediate()
> > > rather than amdgpu_gfx_off_ctrl() as we don't need the
> > > extra delay before we allow gfxoff again.
> > >
> > > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> >
> > Won't this cause unnecessary GFX allows since sdma jobs could also be
> > used for clearing the buffers?
>
> The calls are already there, this just switches to gfxoff allow
> sooner, reducing the time where gfxoff is disallowed.  Should save a
> bit of power.
>
> Alex
>
> >
> > Thanks,
> > Lijo
> >
> > > ---
> > >  drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > > index b1818e87889a2..7f2e1962b5755 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > > @@ -1882,7 +1882,7 @@ static void sdma_v5_2_ring_begin_use(struct amdgpu_ring *ring)
> > >        * doorbells when entering PG.  If you remove this, update
> > >        * sdma_v5_2_ring_set_wptr() as well!
> > >        */
> > > -     amdgpu_gfx_off_ctrl(adev, false);
> > > +     amdgpu_gfx_off_ctrl_immediate(adev, false);
> > >  }
> > >
> > >  static void sdma_v5_2_ring_end_use(struct amdgpu_ring *ring)
> > > @@ -1893,7 +1893,7 @@ static void sdma_v5_2_ring_end_use(struct amdgpu_ring *ring)
> > >        * disallow GFXOFF in some cases leading to
> > >        * hangs in SDMA.  Allow GFXOFF when SDMA is complete.
> > >        */
> > > -     amdgpu_gfx_off_ctrl(adev, true);
> > > +     amdgpu_gfx_off_ctrl_immediate(adev, true);
> > >  }
> > >
> > >  static void sdma_v5_2_print_ip_state(struct amdgpu_ip_block *ip_block, struct drm_printer *p)
> >

[PATCH 06/11] drm/amdgpu/gfx10: manually control gfxoff for CS

[Alex Deucher]

Manually disallow and then allow gfxoff in begin_use
and end_use to avoid any potential FW races when
ringing the doorbell.  There are no known issues
with gfxoff that this solves, but it shouldn't hurt anything
and shouldn't affect power usage since we are only
toggling it around the doorbell update.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 7b01828eea8dc..96346a19950b9 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -9826,14 +9826,14 @@ static void gfx_v10_0_ring_emit_cleaner_shader(struct amdgpu_ring *ring)
 static void gfx_v10_0_ring_begin_use(struct amdgpu_ring *ring)
 {
 	amdgpu_gfx_profile_ring_begin_use(ring);
-
 	amdgpu_gfx_enforce_isolation_ring_begin_use(ring);
+	amdgpu_gfx_off_ctrl_immediate(ring->adev, false);
 }
 
 static void gfx_v10_0_ring_end_use(struct amdgpu_ring *ring)
 {
+	amdgpu_gfx_off_ctrl_immediate(ring->adev, true);
 	amdgpu_gfx_profile_ring_end_use(ring);
-
 	amdgpu_gfx_enforce_isolation_ring_end_use(ring);
 }
 
-- 
2.48.1

Re: [PATCH 06/11] drm/amdgpu/gfx10: manually control gfxoff for CS

[Lazar, Lijo]

On 2/4/2025 3:13 AM, Alex Deucher wrote:
> Manually disallow and then allow gfxoff in begin_use
> and end_use to avoid any potential FW races when
> ringing the doorbell.  There are no known issues
> with gfxoff that this solves, but it shouldn't hurt anything
> and shouldn't affect power usage since we are only
> toggling it around the doorbell update.

I think, this path shouldn't be pursued unless there is a bug identified
in specific SOCs. This really needs to be taken care by
hardware/firmware and driver shouldn't be involved. This may only be
employed as a workaround only if a real solution is not available.

Thanks,
Lijo

> 
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index 7b01828eea8dc..96346a19950b9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -9826,14 +9826,14 @@ static void gfx_v10_0_ring_emit_cleaner_shader(struct amdgpu_ring *ring)
>  static void gfx_v10_0_ring_begin_use(struct amdgpu_ring *ring)
>  {
>  	amdgpu_gfx_profile_ring_begin_use(ring);
> -
>  	amdgpu_gfx_enforce_isolation_ring_begin_use(ring);
> +	amdgpu_gfx_off_ctrl_immediate(ring->adev, false);
>  }
>  
>  static void gfx_v10_0_ring_end_use(struct amdgpu_ring *ring)
>  {
> +	amdgpu_gfx_off_ctrl_immediate(ring->adev, true);
>  	amdgpu_gfx_profile_ring_end_use(ring);
> -
>  	amdgpu_gfx_enforce_isolation_ring_end_use(ring);
>  }
>  

[PATCH 07/11] drm/amdgpu/gfx11: manually control gfxoff for CS

[Alex Deucher]

Manually disallow and then allow gfxoff in begin_use
and end_use to avoid any potential FW races when
ringing the doorbell.  There are no known issues
with gfxoff that this solves, but it shouldn't hurt anything
and shouldn't affect power usage since we are only
toggling it around the doorbell update.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
index 89d17750af04d..51443995eed7f 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
@@ -6871,14 +6871,14 @@ static void gfx_v11_0_ring_emit_cleaner_shader(struct amdgpu_ring *ring)
 static void gfx_v11_0_ring_begin_use(struct amdgpu_ring *ring)
 {
 	amdgpu_gfx_profile_ring_begin_use(ring);
-
 	amdgpu_gfx_enforce_isolation_ring_begin_use(ring);
+	amdgpu_gfx_off_ctrl_immediate(ring->adev, false);
 }
 
 static void gfx_v11_0_ring_end_use(struct amdgpu_ring *ring)
 {
+	amdgpu_gfx_off_ctrl_immediate(ring->adev, true);
 	amdgpu_gfx_profile_ring_end_use(ring);
-
 	amdgpu_gfx_enforce_isolation_ring_end_use(ring);
 }
 
-- 
2.48.1

[PATCH 08/11] drm/amdgpu/gfx12: manually control gfxoff for CS

[Alex Deucher]

Manually disallow and then allow gfxoff in begin_use
and end_use to avoid any potential FW races when
ringing the doorbell.  There are no known issues
with gfxoff that this solves, but it shouldn't hurt anything
and shouldn't affect power usage since we are only
toggling it around the doorbell update.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
index db5cc060de853..9ec2f5fb252a1 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
@@ -5331,14 +5331,14 @@ static int gfx_v12_0_reset_kcq(struct amdgpu_ring *ring, unsigned int vmid)
 static void gfx_v12_0_ring_begin_use(struct amdgpu_ring *ring)
 {
 	amdgpu_gfx_profile_ring_begin_use(ring);
-
 	amdgpu_gfx_enforce_isolation_ring_begin_use(ring);
+	amdgpu_gfx_off_ctrl_immediate(ring->adev, false);
 }
 
 static void gfx_v12_0_ring_end_use(struct amdgpu_ring *ring)
 {
+	amdgpu_gfx_off_ctrl_immediate(ring->adev, true);
 	amdgpu_gfx_profile_ring_end_use(ring);
-
 	amdgpu_gfx_enforce_isolation_ring_end_use(ring);
 }
 
-- 
2.48.1

[PATCH 09/11] drm/amdgpu/sdma5.0: manually control gfxoff for CS

[Alex Deucher]

Manually disallow and then allow gfxoff in begin_use
and end_use to avoid any potential FW races when
ringing the doorbell.  There are no known issues
with gfxoff that this solves, but it shouldn't hurt anything
and shouldn't affect power usage since we are only
toggling it around the doorbell update.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
index b764550834a07..cb4d253b22b2b 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
@@ -1944,6 +1944,16 @@ static void sdma_v5_0_dump_ip_state(struct amdgpu_ip_block *ip_block)
 	amdgpu_gfx_off_ctrl(adev, true);
 }
 
+static void sdma_v5_0_ring_begin_use(struct amdgpu_ring *ring)
+{
+	amdgpu_gfx_off_ctrl_immediate(ring->adev, false);
+}
+
+static void sdma_v5_0_ring_end_use(struct amdgpu_ring *ring)
+{
+	amdgpu_gfx_off_ctrl_immediate(ring->adev, true);
+}
+
 static const struct amd_ip_funcs sdma_v5_0_ip_funcs = {
 	.name = "sdma_v5_0",
 	.early_init = sdma_v5_0_early_init,
@@ -1998,6 +2008,8 @@ static const struct amdgpu_ring_funcs sdma_v5_0_ring_funcs = {
 	.init_cond_exec = sdma_v5_0_ring_init_cond_exec,
 	.preempt_ib = sdma_v5_0_ring_preempt_ib,
 	.reset = sdma_v5_0_reset_queue,
+	.begin_use = sdma_v5_0_ring_begin_use,
+	.end_use = sdma_v5_0_ring_end_use,
 };
 
 static void sdma_v5_0_set_ring_funcs(struct amdgpu_device *adev)
-- 
2.48.1

[PATCH 10/11] drm/amdgpu/sdma6.0: manually control gfxoff for CS

[Alex Deucher]

Manually disallow and then allow gfxoff in begin_use
and end_use to avoid any potential FW races when
ringing the doorbell.  There are no known issues
with gfxoff that this solves, but it shouldn't hurt anything
and shouldn't affect power usage since we are only
toggling it around the doorbell update.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
index b83a0a69ac5b4..a3e5b5ccadbcf 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
@@ -1667,6 +1667,16 @@ static void sdma_v6_0_dump_ip_state(struct amdgpu_ip_block *ip_block)
 	amdgpu_gfx_off_ctrl(adev, true);
 }
 
+static void sdma_v6_0_ring_begin_use(struct amdgpu_ring *ring)
+{
+	amdgpu_gfx_off_ctrl_immediate(ring->adev, false);
+}
+
+static void sdma_v6_0_ring_end_use(struct amdgpu_ring *ring)
+{
+	amdgpu_gfx_off_ctrl_immediate(ring->adev, true);
+}
+
 const struct amd_ip_funcs sdma_v6_0_ip_funcs = {
 	.name = "sdma_v6_0",
 	.early_init = sdma_v6_0_early_init,
@@ -1721,6 +1731,8 @@ static const struct amdgpu_ring_funcs sdma_v6_0_ring_funcs = {
 	.init_cond_exec = sdma_v6_0_ring_init_cond_exec,
 	.preempt_ib = sdma_v6_0_ring_preempt_ib,
 	.reset = sdma_v6_0_reset_queue,
+	.begin_use = sdma_v6_0_ring_begin_use,
+	.end_use = sdma_v6_0_ring_end_use,
 };
 
 static void sdma_v6_0_set_ring_funcs(struct amdgpu_device *adev)
-- 
2.48.1

[PATCH 11/11] drm/amdgpu/sdma7.0: manually control gfxoff for CS

[Alex Deucher]

Manually disallow and then allow gfxoff in begin_use
and end_use to avoid any potential FW races when
ringing the doorbell.  There are no known issues
with gfxoff that this solves, but it shouldn't hurt anything
and shouldn't affect power usage since we are only
toggling it around the doorbell update.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
index 9b40ae67a92f7..53ca808fd1fd7 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
@@ -1646,6 +1646,16 @@ static void sdma_v7_0_dump_ip_state(struct amdgpu_ip_block *ip_block)
 	amdgpu_gfx_off_ctrl(adev, true);
 }
 
+static void sdma_v7_0_ring_begin_use(struct amdgpu_ring *ring)
+{
+	amdgpu_gfx_off_ctrl_immediate(ring->adev, false);
+}
+
+static void sdma_v7_0_ring_end_use(struct amdgpu_ring *ring)
+{
+	amdgpu_gfx_off_ctrl_immediate(ring->adev, true);
+}
+
 const struct amd_ip_funcs sdma_v7_0_ip_funcs = {
 	.name = "sdma_v7_0",
 	.early_init = sdma_v7_0_early_init,
@@ -1701,6 +1711,8 @@ static const struct amdgpu_ring_funcs sdma_v7_0_ring_funcs = {
 	.init_cond_exec = sdma_v7_0_ring_init_cond_exec,
 	.preempt_ib = sdma_v7_0_ring_preempt_ib,
 	.reset = sdma_v7_0_reset_queue,
+	.begin_use = sdma_v7_0_ring_begin_use,
+	.end_use = sdma_v7_0_ring_end_use,
 };
 
 static void sdma_v7_0_set_ring_funcs(struct amdgpu_device *adev)
-- 
2.48.1