RE: [PATCH 3/3] drm/amdgpu: add saved_bo to save vce 4.0 context when suspend

["Deucher, Alexander" <Alexander.Deucher-5C7GfCeVMHo@public.gmane.org>]

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Leo Liu
> Sent: Wednesday, May 31, 2017 3:28 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Liu, Leo
> Subject: [PATCH 3/3] drm/amdgpu: add saved_bo to save vce 4.0 context
> when suspend
> 
> We are using PSP to resume firmware after suspend, and it is
> resumed at where it got suspended, so we'd better save the
> the context.
> 
> Signed-off-by: Leo Liu <leo.liu@amd.com>

Patches 1, 2 are:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
A few comments below on patch 3.


> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h |  1 +
>  drivers/gpu/drm/amd/amdgpu/vce_v4_0.c   | 39
> ++++++++++++++++++++++++++++-----
>  2 files changed, 35 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> index c93f74a..5ce54cd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> @@ -34,6 +34,7 @@ struct amdgpu_vce {
>  	struct amdgpu_bo	*vcpu_bo;
>  	uint64_t		gpu_addr;
>  	void			*cpu_addr;
> +	void			*saved_bo;
>  	unsigned		fw_version;
>  	unsigned		fb_version;
>  	atomic_t		handles[AMDGPU_MAX_VCE_HANDLES];
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> index 0b7fcc1..2230a99 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> @@ -522,8 +522,22 @@ static int vce_v4_0_hw_fini(void *handle)
> 
>  static int vce_v4_0_suspend(void *handle)
>  {
> -	int r;
>  	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +	int r;
> +
> +	if (adev->vce.vcpu_bo == NULL)
> +		return 0;
> +
> +	if (adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) {
> +		unsigned size = amdgpu_bo_size(adev->vce.vcpu_bo);
> +		void *ptr = adev->vce.cpu_addr;
> +
> +		adev->vce.saved_bo = kmalloc(size, GFP_KERNEL);

Might want to avoid malloc/free in the suspend/resume paths.  Just malloc/free the memory at sw_init/fini time.

> +		if (!adev->vce.saved_bo)
> +			return -ENOMEM;
> +
> +		memcpy_fromio(adev->vce.saved_bo, ptr, size);
> +	}
> 
>  	r = vce_v4_0_hw_fini(adev);
>  	if (r)
> @@ -534,12 +548,27 @@ static int vce_v4_0_suspend(void *handle)
> 
>  static int vce_v4_0_resume(void *handle)
>  {
> -	int r;
>  	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +	int r;
> 
> -	r = amdgpu_vce_resume(adev);
> -	if (r)
> -		return r;
> +	if (adev->vce.vcpu_bo == NULL)
> +		return -EINVAL;
> +
> +	if (adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) {
> +		unsigned size = amdgpu_bo_size(adev->vce.vcpu_bo);
> +		void *ptr = adev->vce.cpu_addr;
> +
> +		if (adev->vce.saved_bo != NULL) {
> +			memcpy_toio(ptr, adev->vce.saved_bo, size);
> +			kfree(adev->vce.saved_bo);
> +			adev->vce.saved_bo = NULL;
> +		} else

Kernel coding style; should be } else { ... } to match the chunk above.

> +			memset_io(ptr, 0, size);
> +	} else {
> +		r = amdgpu_vce_resume(adev);
> +		if (r)
> +			return r;
> +	}
> 
>  	return vce_v4_0_hw_init(adev);
>  }
> --
> 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