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

Submitted by Leo Liu on May 31, 2017, 7:28 p.m.

Details

Message ID 20170531192826.17571-3-leo.liu@amd.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Leo Liu May 31, 2017, 7:28 p.m.
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>
---
 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(-)

Patch hide | download patch | download mbox

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

Comments

> -----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
On 05/31/2017 03:54 PM, Deucher, Alexander wrote:
> > -----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.

Just waste a bit memory if it never goes to suspend, but sure, I will 
fix it in v2.

Thanks,
Leo

>
> > +             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