[PATCH 1/3] drm/amdgpu/sdma: add begin_use/end_use helpers

[PATCH 1/3] drm/amdgpu/sdma: add begin_use/end_use helpers

[Alex Deucher]

Add helper functions to disallow GFXOFF while SDMA has work.
This should avoid corner cases where GFXOFF is erroneously
entered when SDMA is still active.  For now just allow/disallow
GFXOFF in the begin and end helpers until we root cause the
issue.  This should not impact power as SDMA usage is pretty
minimal and GFXOSS should not be active when SDMA is active
anyway, this just makes it explicit.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 14 ++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h |  3 +++
 2 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
index 1d9d187de6ee..d4b08d03970c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
@@ -326,3 +326,17 @@ int amdgpu_sdma_ras_sw_init(struct amdgpu_device *adev)
 
 	return 0;
 }
+
+void amdgpu_sdma_ring_begin_use(struct amdgpu_ring *ring)
+{
+	struct amdgpu_device *adev = ring->adev;
+
+	amdgpu_gfx_off_ctrl(adev, false);
+}
+
+void amdgpu_sdma_ring_end_use(struct amdgpu_ring *ring)
+{
+	struct amdgpu_device *adev = ring->adev;
+
+	amdgpu_gfx_off_ctrl(adev, true);
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
index 173a2a308078..b52d16829204 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
@@ -171,4 +171,7 @@ void amdgpu_sdma_destroy_inst_ctx(struct amdgpu_device *adev,
         bool duplicate);
 int amdgpu_sdma_ras_sw_init(struct amdgpu_device *adev);
 
+void amdgpu_sdma_ring_begin_use(struct amdgpu_ring *ring);
+void amdgpu_sdma_ring_end_use(struct amdgpu_ring *ring);
+
 #endif
-- 
2.42.0

[PATCH 2/3] drm/amdgpu/sdma5.0: add begin/end_use ring callbacks

[Alex Deucher]

Add begin/end_use ring callbacks to disallow GFXOFF when
SDMA work is submitted and allow it again afterward.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
index 5c1bb6d07a76..1a68cd2de522 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
@@ -1790,6 +1790,8 @@ static const struct amdgpu_ring_funcs sdma_v5_0_ring_funcs = {
 	.test_ib = sdma_v5_0_ring_test_ib,
 	.insert_nop = sdma_v5_0_ring_insert_nop,
 	.pad_ib = sdma_v5_0_ring_pad_ib,
+	.begin_use = amdgpu_sdma_ring_begin_use,
+	.end_use = amdgpu_sdma_ring_end_use,
 	.emit_wreg = sdma_v5_0_ring_emit_wreg,
 	.emit_reg_wait = sdma_v5_0_ring_emit_reg_wait,
 	.emit_reg_write_reg_wait = sdma_v5_0_ring_emit_reg_write_reg_wait,
-- 
2.42.0

[PATCH 3/3] drm/amdgpu/sdma5.2: add begin/end_use ring callbacks

[Alex Deucher]

Add begin/end_use ring callbacks to disallow GFXOFF when
SDMA work is submitted and allow it again afterward.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
index 2e35f3571774..988238f20315 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
@@ -1690,6 +1690,8 @@ static const struct amdgpu_ring_funcs sdma_v5_2_ring_funcs = {
 	.test_ib = sdma_v5_2_ring_test_ib,
 	.insert_nop = sdma_v5_2_ring_insert_nop,
 	.pad_ib = sdma_v5_2_ring_pad_ib,
+	.begin_use = amdgpu_sdma_ring_begin_use,
+	.end_use = amdgpu_sdma_ring_end_use,
 	.emit_wreg = sdma_v5_2_ring_emit_wreg,
 	.emit_reg_wait = sdma_v5_2_ring_emit_reg_wait,
 	.emit_reg_write_reg_wait = sdma_v5_2_ring_emit_reg_write_reg_wait,
-- 
2.42.0

Re: [PATCH 1/3] drm/amdgpu/sdma: add begin_use/end_use helpers

[Mario Limonciello]

On 12/8/2023 16:19, Alex Deucher wrote:
> Add helper functions to disallow GFXOFF while SDMA has work.
> This should avoid corner cases where GFXOFF is erroneously
> entered when SDMA is still active.  For now just allow/disallow
> GFXOFF in the begin and end helpers until we root cause the
> issue.  This should not impact power as SDMA usage is pretty
> minimal and GFXOSS should not be active when SDMA is active
> anyway, this just makes it explicit.
> 
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---

For the series:

Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2220
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Tested-by: Mario Limonciello <mario.limonciello@amd.com>

>   drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 14 ++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h |  3 +++
>   2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> index 1d9d187de6ee..d4b08d03970c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> @@ -326,3 +326,17 @@ int amdgpu_sdma_ras_sw_init(struct amdgpu_device *adev)
>   
>   	return 0;
>   }
> +
> +void amdgpu_sdma_ring_begin_use(struct amdgpu_ring *ring)
> +{
> +	struct amdgpu_device *adev = ring->adev;
> +
> +	amdgpu_gfx_off_ctrl(adev, false);
> +}
> +
> +void amdgpu_sdma_ring_end_use(struct amdgpu_ring *ring)
> +{
> +	struct amdgpu_device *adev = ring->adev;
> +
> +	amdgpu_gfx_off_ctrl(adev, true);
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
> index 173a2a308078..b52d16829204 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
> @@ -171,4 +171,7 @@ void amdgpu_sdma_destroy_inst_ctx(struct amdgpu_device *adev,
>           bool duplicate);
>   int amdgpu_sdma_ras_sw_init(struct amdgpu_device *adev);
>   
> +void amdgpu_sdma_ring_begin_use(struct amdgpu_ring *ring);
> +void amdgpu_sdma_ring_end_use(struct amdgpu_ring *ring);
> +
>   #endif

RE: [PATCH 2/3] drm/amdgpu/sdma5.0: add begin/end_use ring callbacks

[Deucher, Alexander]

[Public]

> -----Original Message-----
> From: Deucher, Alexander <Alexander.Deucher@amd.com>
> Sent: Friday, December 8, 2023 5:19 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
> Subject: [PATCH 2/3] drm/amdgpu/sdma5.0: add begin/end_use ring
> callbacks
>
> Add begin/end_use ring callbacks to disallow GFXOFF when SDMA work is
> submitted and allow it again afterward.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

This one can probably be dropped.  It's only needed if anyone on navi1x is experiencing a similar issue.

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> index 5c1bb6d07a76..1a68cd2de522 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> @@ -1790,6 +1790,8 @@ static const struct amdgpu_ring_funcs
> sdma_v5_0_ring_funcs = {
>       .test_ib = sdma_v5_0_ring_test_ib,
>       .insert_nop = sdma_v5_0_ring_insert_nop,
>       .pad_ib = sdma_v5_0_ring_pad_ib,
> +     .begin_use = amdgpu_sdma_ring_begin_use,
> +     .end_use = amdgpu_sdma_ring_end_use,
>       .emit_wreg = sdma_v5_0_ring_emit_wreg,
>       .emit_reg_wait = sdma_v5_0_ring_emit_reg_wait,
>       .emit_reg_write_reg_wait =
> sdma_v5_0_ring_emit_reg_write_reg_wait,
> --
> 2.42.0

Re: [PATCH 1/3] drm/amdgpu/sdma: add begin_use/end_use helpers

[Alex Deucher]

On Fri, Dec 8, 2023 at 5:19 PM Alex Deucher <alexander.deucher@amd.com> wrote:
>
> Add helper functions to disallow GFXOFF while SDMA has work.
> This should avoid corner cases where GFXOFF is erroneously
> entered when SDMA is still active.  For now just allow/disallow
> GFXOFF in the begin and end helpers until we root cause the
> issue.  This should not impact power as SDMA usage is pretty
> minimal and GFXOSS should not be active when SDMA is active
> anyway, this just makes it explicit.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

Since sdma5.2 is the only version that currently needs this I think we
can just squash this all into sdma5.2.  There may even be 5.2.x
variants that don't need this.  Better to keep it all in that code.
Will send out a v2.

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 14 ++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h |  3 +++
>  2 files changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> index 1d9d187de6ee..d4b08d03970c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> @@ -326,3 +326,17 @@ int amdgpu_sdma_ras_sw_init(struct amdgpu_device *adev)
>
>         return 0;
>  }
> +
> +void amdgpu_sdma_ring_begin_use(struct amdgpu_ring *ring)
> +{
> +       struct amdgpu_device *adev = ring->adev;
> +
> +       amdgpu_gfx_off_ctrl(adev, false);
> +}
> +
> +void amdgpu_sdma_ring_end_use(struct amdgpu_ring *ring)
> +{
> +       struct amdgpu_device *adev = ring->adev;
> +
> +       amdgpu_gfx_off_ctrl(adev, true);
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
> index 173a2a308078..b52d16829204 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
> @@ -171,4 +171,7 @@ void amdgpu_sdma_destroy_inst_ctx(struct amdgpu_device *adev,
>          bool duplicate);
>  int amdgpu_sdma_ras_sw_init(struct amdgpu_device *adev);
>
> +void amdgpu_sdma_ring_begin_use(struct amdgpu_ring *ring);
> +void amdgpu_sdma_ring_end_use(struct amdgpu_ring *ring);
> +
>  #endif
> --
> 2.42.0
>