Re: [PATCH v5 3/7] drm/amd: Split up UVD suspend into prepare and suspend steps

2023-10-09 Thread Alex Deucher 
On Fri, Oct 6, 2023 at 3:07 PM Mario Limonciello
 wrote:
>
> amdgpu_uvd_suspend() allocates memory and copies objects into that
> allocated memory.  This fails under memory pressure.  Instead move
> majority of this code into a prepare step when swap can still be
> allocated.
>
> Signed-off-by: Mario Limonciello 
> ---
> v4->v5:
>  * New patch
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 12 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h |  1 +
>  drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c   |  9 +
>  drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c   |  9 +
>  drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c   |  9 +
>  drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c   |  9 +
>  drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c   |  9 +
>  7 files changed, 54 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> index b7441654e6fa..a53c4ba8b3fe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> @@ -418,12 +418,11 @@ int amdgpu_uvd_entity_init(struct amdgpu_device *adev)
> return 0;
>  }
>
> -int amdgpu_uvd_suspend(struct amdgpu_device *adev)
> +int amdgpu_uvd_prepare(struct amdgpu_device *adev)

amdgpu_uvd_prepare_suspend() so it's more obvious what this is for.
Other than that, looks good to me.

Alex

>  {
> unsigned int size;
> void *ptr;
> int i, j, idx;
> -   bool in_ras_intr = amdgpu_ras_intr_triggered();
>
> cancel_delayed_work_sync(>uvd.idle_work);
>
> @@ -452,7 +451,7 @@ int amdgpu_uvd_suspend(struct amdgpu_device *adev)
>
> if (drm_dev_enter(adev_to_drm(adev), )) {
> /* re-write 0 since err_event_athub will corrupt VCPU 
> buffer */
> -   if (in_ras_intr)
> +   if (amdgpu_ras_intr_triggered())
> memset(adev->uvd.inst[j].saved_bo, 0, size);
> else
> memcpy_fromio(adev->uvd.inst[j].saved_bo, 
> ptr, size);
> @@ -461,7 +460,12 @@ int amdgpu_uvd_suspend(struct amdgpu_device *adev)
> }
> }
>
> -   if (in_ras_intr)
> +   return 0;
> +}
> +
> +int amdgpu_uvd_suspend(struct amdgpu_device *adev)
> +{
> +   if (amdgpu_ras_intr_triggered())
> DRM_WARN("UVD VCPU state may lost due to RAS 
> ERREVENT_ATHUB_INTERRUPT\n");
>
> return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
> index 9f89bb7cd60b..72228425e021 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
> @@ -74,6 +74,7 @@ struct amdgpu_uvd {
>  int amdgpu_uvd_sw_init(struct amdgpu_device *adev);
>  int amdgpu_uvd_sw_fini(struct amdgpu_device *adev);
>  int amdgpu_uvd_entity_init(struct amdgpu_device *adev);
> +int amdgpu_uvd_prepare(struct amdgpu_device *adev);
>  int amdgpu_uvd_suspend(struct amdgpu_device *adev);
>  int amdgpu_uvd_resume(struct amdgpu_device *adev);
>  int amdgpu_uvd_get_create_msg(struct amdgpu_ring *ring, uint32_t handle,
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c 
> b/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c
> index 5534c769b655..869e9948fa36 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c
> @@ -706,6 +706,14 @@ static int uvd_v3_1_hw_fini(void *handle)
> return 0;
>  }
>
> +static int uvd_v3_1_prepare(void *handle)
> +{
> +   struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +
> +   return amdgpu_uvd_prepare(adev);
> +
> +}
> +
>  static int uvd_v3_1_suspend(void *handle)
>  {
> int r;
> @@ -806,6 +814,7 @@ static const struct amd_ip_funcs uvd_v3_1_ip_funcs = {
> .sw_fini = uvd_v3_1_sw_fini,
> .hw_init = uvd_v3_1_hw_init,
> .hw_fini = uvd_v3_1_hw_fini,
> +   .prepare = uvd_v3_1_prepare,
> .suspend = uvd_v3_1_suspend,
> .resume = uvd_v3_1_resume,
> .is_idle = uvd_v3_1_is_idle,
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c 
> b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> index c108b8381795..e589c17af371 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> @@ -220,6 +220,14 @@ static int uvd_v4_2_hw_fini(void *handle)
> return 0;
>  }
>
> +static int uvd_v4_2_prepare(void *handle)
> +{
> +   struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +
> +   return amdgpu_uvd_prepare(adev);
> +
> +}
> +
>  static int uvd_v4_2_suspend(void *handle)
>  {
> int r;
> @@ -756,6 +764,7 @@ static const struct amd_ip_funcs uvd_v4_2_ip_funcs = {
> .sw_fini = uvd_v4_2_sw_fini,
> .hw_init = uvd_v4_2_hw_init,
> .hw_fini = uvd_v4_2_hw_fini,
> +   .prepare = uvd_v4_2_prepare,
> .suspend = uvd_v4_2_suspend,
> .resume = uvd_v4_2_resume,
> .is_idle =

[PATCH v5 3/7] drm/amd: Split up UVD suspend into prepare and suspend steps

2023-10-06 Thread Mario Limonciello 
amdgpu_uvd_suspend() allocates memory and copies objects into that
allocated memory.  This fails under memory pressure.  Instead move
majority of this code into a prepare step when swap can still be
allocated.

Signed-off-by: Mario Limonciello 
---
v4->v5:
 * New patch
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 12 
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h |  1 +
 drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c   |  9 +
 drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c   |  9 +
 drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c   |  9 +
 drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c   |  9 +
 drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c   |  9 +
 7 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
index b7441654e6fa..a53c4ba8b3fe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
@@ -418,12 +418,11 @@ int amdgpu_uvd_entity_init(struct amdgpu_device *adev)
return 0;
 }
 
-int amdgpu_uvd_suspend(struct amdgpu_device *adev)
+int amdgpu_uvd_prepare(struct amdgpu_device *adev)
 {
unsigned int size;
void *ptr;
int i, j, idx;
-   bool in_ras_intr = amdgpu_ras_intr_triggered();
 
cancel_delayed_work_sync(>uvd.idle_work);
 
@@ -452,7 +451,7 @@ int amdgpu_uvd_suspend(struct amdgpu_device *adev)
 
if (drm_dev_enter(adev_to_drm(adev), )) {
/* re-write 0 since err_event_athub will corrupt VCPU 
buffer */
-   if (in_ras_intr)
+   if (amdgpu_ras_intr_triggered())
memset(adev->uvd.inst[j].saved_bo, 0, size);
else
memcpy_fromio(adev->uvd.inst[j].saved_bo, ptr, 
size);
@@ -461,7 +460,12 @@ int amdgpu_uvd_suspend(struct amdgpu_device *adev)
}
}
 
-   if (in_ras_intr)
+   return 0;
+}
+
+int amdgpu_uvd_suspend(struct amdgpu_device *adev)
+{
+   if (amdgpu_ras_intr_triggered())
DRM_WARN("UVD VCPU state may lost due to RAS 
ERREVENT_ATHUB_INTERRUPT\n");
 
return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
index 9f89bb7cd60b..72228425e021 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
@@ -74,6 +74,7 @@ struct amdgpu_uvd {
 int amdgpu_uvd_sw_init(struct amdgpu_device *adev);
 int amdgpu_uvd_sw_fini(struct amdgpu_device *adev);
 int amdgpu_uvd_entity_init(struct amdgpu_device *adev);
+int amdgpu_uvd_prepare(struct amdgpu_device *adev);
 int amdgpu_uvd_suspend(struct amdgpu_device *adev);
 int amdgpu_uvd_resume(struct amdgpu_device *adev);
 int amdgpu_uvd_get_create_msg(struct amdgpu_ring *ring, uint32_t handle,
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c 
b/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c
index 5534c769b655..869e9948fa36 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c
@@ -706,6 +706,14 @@ static int uvd_v3_1_hw_fini(void *handle)
return 0;
 }
 
+static int uvd_v3_1_prepare(void *handle)
+{
+   struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+
+   return amdgpu_uvd_prepare(adev);
+
+}
+
 static int uvd_v3_1_suspend(void *handle)
 {
int r;
@@ -806,6 +814,7 @@ static const struct amd_ip_funcs uvd_v3_1_ip_funcs = {
.sw_fini = uvd_v3_1_sw_fini,
.hw_init = uvd_v3_1_hw_init,
.hw_fini = uvd_v3_1_hw_fini,
+   .prepare = uvd_v3_1_prepare,
.suspend = uvd_v3_1_suspend,
.resume = uvd_v3_1_resume,
.is_idle = uvd_v3_1_is_idle,
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c 
b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
index c108b8381795..e589c17af371 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
@@ -220,6 +220,14 @@ static int uvd_v4_2_hw_fini(void *handle)
return 0;
 }
 
+static int uvd_v4_2_prepare(void *handle)
+{
+   struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+
+   return amdgpu_uvd_prepare(adev);
+
+}
+
 static int uvd_v4_2_suspend(void *handle)
 {
int r;
@@ -756,6 +764,7 @@ static const struct amd_ip_funcs uvd_v4_2_ip_funcs = {
.sw_fini = uvd_v4_2_sw_fini,
.hw_init = uvd_v4_2_hw_init,
.hw_fini = uvd_v4_2_hw_fini,
+   .prepare = uvd_v4_2_prepare,
.suspend = uvd_v4_2_suspend,
.resume = uvd_v4_2_resume,
.is_idle = uvd_v4_2_is_idle,
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c 
b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
index d7e31e48a2b8..65aa23bc2d91 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
@@ -218,6 +218,14 @@ static int uvd_v5_0_hw_fini(void *handle)
return 0;
 }
 
+static int uvd_v5_0_prepare(void *handle)
+{
+   struct amdgpu_device *adev = (struct amdgpu_device