[PATCH 0/3] Fixed amdgpu resume failure from suspend

[PATCH 0/3] Fixed amdgpu resume failure from suspend

[Zhu, James]

UVD session info size can be upto 128K, we need to allocate a large
enough buffer for the session info, otherwise the IB test can overwrite
other's memory.

Bug: https://bugzilla.kernel.org/show_bug.cgi?id=204241

James Zhu (3):
  drm/amdgpu/uvd:Add uvd enc session bo
  drm/amdgpu/uvd:Allocate enc session bo for uvd6.0 ring IB test
  drm/amdgpu/uvd:Allocate enc session bo for uvd7.0 ring IB test

 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h |  4 ++++
 drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c   | 16 ++++++++++++++--
 drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c   | 16 ++++++++++++++--
 3 files changed, 32 insertions(+), 4 deletions(-)

-- 
2.7.4

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

[PATCH 1/3] drm/amdgpu/uvd:Add uvd enc session bo

[Zhu, James]

Add uvd enc session bo for uvd encode IB test.

Signed-off-by: James Zhu <James.Zhu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
index 5eb6328..1e39c8a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
@@ -67,6 +67,10 @@ struct amdgpu_uvd {
 	unsigned		harvest_config;
 	/* store image width to adjust nb memory state */
 	unsigned		decode_image_width;
+
+	struct amdgpu_bo *enc_session_bo;
+	void			 *enc_session_cpu_addr;
+	uint64_t		  enc_session_gpu_addr;
 };
 
 int amdgpu_uvd_sw_init(struct amdgpu_device *adev);
-- 
2.7.4

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

[PATCH 2/3] drm/amdgpu/uvd:Allocate enc session bo for uvd6.0 ring IB test

[Zhu, James]

Allocate 256K enc session bo for uvd6.0 ring IB test to fix S3 resume
corruption issue.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
index 670784a..c79ce73 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
@@ -220,7 +220,7 @@ static int uvd_v6_0_enc_get_create_msg(struct amdgpu_ring *ring, uint32_t handle
 		return r;
 
 	ib = &job->ibs[0];
-	dummy = ib->gpu_addr + 1024;
+	dummy = ring->adev->vcn.enc_session_gpu_addr;
 
 	ib->length_dw = 0;
 	ib->ptr[ib->length_dw++] = 0x00000018;
@@ -282,7 +282,7 @@ static int uvd_v6_0_enc_get_destroy_msg(struct amdgpu_ring *ring,
 		return r;
 
 	ib = &job->ibs[0];
-	dummy = ib->gpu_addr + 1024;
+	dummy = ring->adev->vcn.enc_session_gpu_addr + 128 * PAGE_SIZE;
 
 	ib->length_dw = 0;
 	ib->ptr[ib->length_dw++] = 0x00000018;
@@ -326,9 +326,16 @@ static int uvd_v6_0_enc_get_destroy_msg(struct amdgpu_ring *ring,
  */
 static int uvd_v6_0_enc_ring_test_ib(struct amdgpu_ring *ring, long timeout)
 {
+	struct amdgpu_device *adev = ring->adev;
 	struct dma_fence *fence = NULL;
 	long r;
 
+	r = amdgpu_bo_create_kernel(adev, 2 * 128, PAGE_SIZE,
+			AMDGPU_GEM_DOMAIN_VRAM, &adev->vcn.enc_session_bo,
+			&adev->vcn.enc_session_gpu_addr, &adev->vcn.enc_session_cpu_addr);
+	if (r)
+		return r;
+
 	r = uvd_v6_0_enc_get_create_msg(ring, 1, NULL);
 	if (r)
 		goto error;
@@ -345,6 +352,11 @@ static int uvd_v6_0_enc_ring_test_ib(struct amdgpu_ring *ring, long timeout)
 
 error:
 	dma_fence_put(fence);
+
+	amdgpu_bo_free_kernel(&adev->vcn.enc_session_bo,
+				  &adev->vcn.enc_session_gpu_addr,
+				  (void **)&adev->vcn.enc_session_cpu_addr);
+
 	return r;
 }
 
-- 
2.7.4

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

[PATCH 3/3] drm/amdgpu/uvd:Allocate enc session bo for uvd7.0 ring IB test

[Zhu, James]

Allocate 256K enc session bo for uvd6.0 ring IB test to fix S3 resume
corruption issue.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
index 01f658f..1b17fc9 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
@@ -228,7 +228,7 @@ static int uvd_v7_0_enc_get_create_msg(struct amdgpu_ring *ring, uint32_t handle
 		return r;
 
 	ib = &job->ibs[0];
-	dummy = ib->gpu_addr + 1024;
+	dummy = ring->adev->vcn.enc_session_gpu_addr;
 
 	ib->length_dw = 0;
 	ib->ptr[ib->length_dw++] = 0x00000018;
@@ -289,7 +289,7 @@ static int uvd_v7_0_enc_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handl
 		return r;
 
 	ib = &job->ibs[0];
-	dummy = ib->gpu_addr + 1024;
+	dummy = ring->adev->vcn.enc_session_gpu_addr + 128 * PAGE_SIZE;
 
 	ib->length_dw = 0;
 	ib->ptr[ib->length_dw++] = 0x00000018;
@@ -333,9 +333,16 @@ static int uvd_v7_0_enc_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handl
  */
 static int uvd_v7_0_enc_ring_test_ib(struct amdgpu_ring *ring, long timeout)
 {
+	struct amdgpu_device *adev = ring->adev;
 	struct dma_fence *fence = NULL;
 	long r;
 
+	r = amdgpu_bo_create_kernel(adev, 2 * 128, PAGE_SIZE,
+			AMDGPU_GEM_DOMAIN_VRAM, &adev->vcn.enc_session_bo,
+			&adev->vcn.enc_session_gpu_addr, &adev->vcn.enc_session_cpu_addr);
+	if (r)
+		return r;
+
 	r = uvd_v7_0_enc_get_create_msg(ring, 1, NULL);
 	if (r)
 		goto error;
@@ -352,6 +359,11 @@ static int uvd_v7_0_enc_ring_test_ib(struct amdgpu_ring *ring, long timeout)
 
 error:
 	dma_fence_put(fence);
+
+	amdgpu_bo_free_kernel(&adev->vcn.enc_session_bo,
+				  &adev->vcn.enc_session_gpu_addr,
+				  (void **)&adev->vcn.enc_session_cpu_addr);
+
 	return r;
 }
 
-- 
2.7.4

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

Re: [PATCH 2/3] drm/amdgpu/uvd:Allocate enc session bo for uvd6.0 ring IB test

[Alex Deucher]

On Tue, Oct 15, 2019 at 6:08 PM Zhu, James <James.Zhu@amd.com> wrote:
>
> Allocate 256K enc session bo for uvd6.0 ring IB test to fix S3 resume
> corruption issue.
>
> Signed-off-by: James Zhu <James.Zhu@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> index 670784a..c79ce73 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> @@ -220,7 +220,7 @@ static int uvd_v6_0_enc_get_create_msg(struct amdgpu_ring *ring, uint32_t handle
>                 return r;
>
>         ib = &job->ibs[0];
> -       dummy = ib->gpu_addr + 1024;
> +       dummy = ring->adev->vcn.enc_session_gpu_addr;
>
>         ib->length_dw = 0;
>         ib->ptr[ib->length_dw++] = 0x00000018;
> @@ -282,7 +282,7 @@ static int uvd_v6_0_enc_get_destroy_msg(struct amdgpu_ring *ring,
>                 return r;
>
>         ib = &job->ibs[0];
> -       dummy = ib->gpu_addr + 1024;
> +       dummy = ring->adev->vcn.enc_session_gpu_addr + 128 * PAGE_SIZE;
>
>         ib->length_dw = 0;
>         ib->ptr[ib->length_dw++] = 0x00000018;
> @@ -326,9 +326,16 @@ static int uvd_v6_0_enc_get_destroy_msg(struct amdgpu_ring *ring,
>   */
>  static int uvd_v6_0_enc_ring_test_ib(struct amdgpu_ring *ring, long timeout)
>  {
> +       struct amdgpu_device *adev = ring->adev;
>         struct dma_fence *fence = NULL;
>         long r;
>
> +       r = amdgpu_bo_create_kernel(adev, 2 * 128, PAGE_SIZE,

I just sent a similar patch set.  Note that the amdgpu_bo functions
takes the size in bytes, so this is only 256 bytes (well, probably 4K
due to page alignment).  I think VCN also needs this fix.  Do the
create and destroy need to reference the same session info?

Alex

> +                       AMDGPU_GEM_DOMAIN_VRAM, &adev->vcn.enc_session_bo,
> +                       &adev->vcn.enc_session_gpu_addr, &adev->vcn.enc_session_cpu_addr);
> +       if (r)
> +               return r;
> +
>         r = uvd_v6_0_enc_get_create_msg(ring, 1, NULL);
>         if (r)
>                 goto error;
> @@ -345,6 +352,11 @@ static int uvd_v6_0_enc_ring_test_ib(struct amdgpu_ring *ring, long timeout)
>
>  error:
>         dma_fence_put(fence);
> +
> +       amdgpu_bo_free_kernel(&adev->vcn.enc_session_bo,
> +                                 &adev->vcn.enc_session_gpu_addr,
> +                                 (void **)&adev->vcn.enc_session_cpu_addr);
> +
>         return r;
>  }
>
> --
> 2.7.4
>
> _______________________________________________
> 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

Re: [PATCH 1/3] drm/amdgpu/uvd:Add uvd enc session bo

[Christian König]

Am 16.10.19 um 00:08 schrieb Zhu, James:
> Add uvd enc session bo for uvd encode IB test.
>
> Signed-off-by: James Zhu <James.Zhu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
> index 5eb6328..1e39c8a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
> @@ -67,6 +67,10 @@ struct amdgpu_uvd {
>   	unsigned		harvest_config;
>   	/* store image width to adjust nb memory state */
>   	unsigned		decode_image_width;
> +
> +	struct amdgpu_bo *enc_session_bo;
> +	void			 *enc_session_cpu_addr;
> +	uint64_t		  enc_session_gpu_addr;

Please don't keep that allocated all the time, but rather only allocate 
it on demand during the IB test.

Regards,
Christian.

>   };
>   
>   int amdgpu_uvd_sw_init(struct amdgpu_device *adev);

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

Re: [PATCH 3/3] drm/amdgpu/uvd:Allocate enc session bo for uvd7.0 ring IB test

[Christian König]

Am 16.10.19 um 00:08 schrieb Zhu, James:
> Allocate 256K enc session bo for uvd6.0 ring IB test to fix S3 resume
> corruption issue.
>
> Signed-off-by: James Zhu <James.Zhu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c | 16 ++++++++++++++--
>   1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> index 01f658f..1b17fc9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> @@ -228,7 +228,7 @@ static int uvd_v7_0_enc_get_create_msg(struct amdgpu_ring *ring, uint32_t handle
>   		return r;
>   
>   	ib = &job->ibs[0];
> -	dummy = ib->gpu_addr + 1024;
> +	dummy = ring->adev->vcn.enc_session_gpu_addr;
>   
>   	ib->length_dw = 0;
>   	ib->ptr[ib->length_dw++] = 0x00000018;
> @@ -289,7 +289,7 @@ static int uvd_v7_0_enc_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handl
>   		return r;
>   
>   	ib = &job->ibs[0];
> -	dummy = ib->gpu_addr + 1024;
> +	dummy = ring->adev->vcn.enc_session_gpu_addr + 128 * PAGE_SIZE;
>   
>   	ib->length_dw = 0;
>   	ib->ptr[ib->length_dw++] = 0x00000018;
> @@ -333,9 +333,16 @@ static int uvd_v7_0_enc_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handl
>    */
>   static int uvd_v7_0_enc_ring_test_ib(struct amdgpu_ring *ring, long timeout)
>   {
> +	struct amdgpu_device *adev = ring->adev;
>   	struct dma_fence *fence = NULL;
>   	long r;
>   
> +	r = amdgpu_bo_create_kernel(adev, 2 * 128, PAGE_SIZE,
> +			AMDGPU_GEM_DOMAIN_VRAM, &adev->vcn.enc_session_bo,
> +			&adev->vcn.enc_session_gpu_addr, &adev->vcn.enc_session_cpu_addr);
> +	if (r)
> +		return r;
> +

Looks like you actually do allocate that on demand, but please don't put 
the bo and addresses into the adev->vcn structure. It is only valid 
temporary.

Regards,
Christian.

>   	r = uvd_v7_0_enc_get_create_msg(ring, 1, NULL);
>   	if (r)
>   		goto error;
> @@ -352,6 +359,11 @@ static int uvd_v7_0_enc_ring_test_ib(struct amdgpu_ring *ring, long timeout)
>   
>   error:
>   	dma_fence_put(fence);
> +
> +	amdgpu_bo_free_kernel(&adev->vcn.enc_session_bo,
> +				  &adev->vcn.enc_session_gpu_addr,
> +				  (void **)&adev->vcn.enc_session_cpu_addr);
> +
>   	return r;
>   }
>   

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

Re: [PATCH 1/3] drm/amdgpu/uvd:Add uvd enc session bo

[James Zhu]

Thanks for your comments!

Alex has a new patch submit, I am verifying it.

Thanks!

James Zhu

On 2019-10-16 4:59 a.m., Christian König wrote:
> Am 16.10.19 um 00:08 schrieb Zhu, James:
>> Add uvd enc session bo for uvd encode IB test.
>>
>> Signed-off-by: James Zhu <James.Zhu@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
>> index 5eb6328..1e39c8a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
>> @@ -67,6 +67,10 @@ struct amdgpu_uvd {
>>       unsigned        harvest_config;
>>       /* store image width to adjust nb memory state */
>>       unsigned        decode_image_width;
>> +
>> +    struct amdgpu_bo *enc_session_bo;
>> +    void             *enc_session_cpu_addr;
>> +    uint64_t          enc_session_gpu_addr;
>
> Please don't keep that allocated all the time, but rather only 
> allocate it on demand during the IB test.
>
> Regards,
> Christian.
>
>>   };
>>     int amdgpu_uvd_sw_init(struct amdgpu_device *adev);
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx