[PATCH 1/3] drm/amdgpu: add vcpu_bo cpu address

[PATCH 1/3] drm/amdgpu: add vcpu_bo cpu address

[Leo Liu]

Signed-off-by: Leo Liu <leo.liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
index 0a7f18c..c93f74a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
@@ -33,6 +33,7 @@
 struct amdgpu_vce {
 	struct amdgpu_bo	*vcpu_bo;
 	uint64_t		gpu_addr;
+	void			*cpu_addr;
 	unsigned		fw_version;
 	unsigned		fb_version;
 	atomic_t		handles[AMDGPU_MAX_VCE_HANDLES];
-- 
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: use existing function amdgpu_bo_create_kernel

[Leo Liu]

To simplify vce bo create

Signed-off-by: Leo Liu <leo.liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c | 27 +++------------------------
 1 file changed, 3 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
index 735c38d..b9b0906 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
@@ -165,35 +165,14 @@ int amdgpu_vce_sw_init(struct amdgpu_device *adev, unsigned long size)
 	adev->vce.fw_version = ((version_major << 24) | (version_minor << 16) |
 				(binary_id << 8));
 
-	/* allocate firmware, stack and heap BO */
-
-	r = amdgpu_bo_create(adev, size, PAGE_SIZE, true,
-			     AMDGPU_GEM_DOMAIN_VRAM,
-			     AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED |
-			     AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS,
-			     NULL, NULL, &adev->vce.vcpu_bo);
+	r = amdgpu_bo_create_kernel(adev, size, PAGE_SIZE,
+				    AMDGPU_GEM_DOMAIN_VRAM, &adev->vce.vcpu_bo,
+				    &adev->vce.gpu_addr, &adev->vce.cpu_addr);
 	if (r) {
 		dev_err(adev->dev, "(%d) failed to allocate VCE bo\n", r);
 		return r;
 	}
 
-	r = amdgpu_bo_reserve(adev->vce.vcpu_bo, false);
-	if (r) {
-		amdgpu_bo_unref(&adev->vce.vcpu_bo);
-		dev_err(adev->dev, "(%d) failed to reserve VCE bo\n", r);
-		return r;
-	}
-
-	r = amdgpu_bo_pin(adev->vce.vcpu_bo, AMDGPU_GEM_DOMAIN_VRAM,
-			  &adev->vce.gpu_addr);
-	amdgpu_bo_unreserve(adev->vce.vcpu_bo);
-	if (r) {
-		amdgpu_bo_unref(&adev->vce.vcpu_bo);
-		dev_err(adev->dev, "(%d) VCE bo pin failed\n", r);
-		return r;
-	}
-
-
 	ring = &adev->vce.ring[0];
 	rq = &ring->sched.sched_rq[AMD_SCHED_PRIORITY_NORMAL];
 	r = amd_sched_entity_init(&ring->sched, &adev->vce.entity,
-- 
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: add saved_bo to save vce 4.0 context when suspend

[Leo Liu]

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(-)

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);
 }
-- 
2.7.4

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

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

[Deucher, Alexander]

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

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

[Leo Liu]

[-- Attachment #1.1: Type: text/plain, Size: 4216 bytes --]



On 05/31/2017 03:54 PM, Deucher, Alexander wrote:
> > -----Original Message-----
> > From: amd-gfx [mailto:amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org] On Behalf
> > Of Leo Liu
> > Sent: Wednesday, May 31, 2017 3:28 PM
> > To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.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-5C7GfCeVMHo@public.gmane.org>
>
> Patches 1, 2 are:
> Reviewed-by: Alex Deucher <alexander.deucher-5C7GfCeVMHo@public.gmane.org>
> 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-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[-- Attachment #1.2: Type: text/html, Size: 8783 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

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

[Leo Liu]

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   | 45 +++++++++++++++++++++++++++------
 2 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
index f5f35cc..f83e0c3 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 ada2d0a..5229593 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
@@ -419,15 +419,19 @@ static int vce_v4_0_sw_init(void *handle)
 
 	if (adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) {
 		const struct common_firmware_header *hdr;
+		unsigned size = amdgpu_bo_size(adev->vce.vcpu_bo);
+
+		adev->vce.saved_bo = kmalloc(size, GFP_KERNEL);
+		if (!adev->vce.saved_bo)
+			return -ENOMEM;
+
 		hdr = (const struct common_firmware_header *)adev->vce.fw->data;
 		adev->firmware.ucode[AMDGPU_UCODE_ID_VCE].ucode_id = AMDGPU_UCODE_ID_VCE;
 		adev->firmware.ucode[AMDGPU_UCODE_ID_VCE].fw = adev->vce.fw;
 		adev->firmware.fw_size +=
 			ALIGN(le32_to_cpu(hdr->ucode_size_bytes), PAGE_SIZE);
 		DRM_INFO("PSP loading VCE firmware\n");
-	}
-
-	if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP) {
+	} else {
 		r = amdgpu_vce_resume(adev);
 		if (r)
 			return r;
@@ -466,6 +470,11 @@ static int vce_v4_0_sw_fini(void *handle)
 	/* free MM table */
 	amdgpu_virt_free_mm_table(adev);
 
+	if (adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) {
+		kfree(adev->vce.saved_bo);
+		adev->vce.saved_bo = NULL;
+	}
+
 	r = amdgpu_vce_suspend(adev);
 	if (r)
 		return r;
@@ -522,8 +531,18 @@ 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;
+
+		memcpy_fromio(adev->vce.saved_bo, ptr, size);
+	}
 
 	r = vce_v4_0_hw_fini(adev);
 	if (r)
@@ -534,12 +553,22 @@ 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;
+
+		memcpy_toio(ptr, adev->vce.saved_bo, 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

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

[Deucher, Alexander]

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Leo Liu
> Sent: Wednesday, May 31, 2017 5:01 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>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h |  1 +
>  drivers/gpu/drm/amd/amdgpu/vce_v4_0.c   | 45
> +++++++++++++++++++++++++++------
>  2 files changed, 38 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> index f5f35cc..f83e0c3 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 ada2d0a..5229593 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> @@ -419,15 +419,19 @@ static int vce_v4_0_sw_init(void *handle)
> 
>  	if (adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) {
>  		const struct common_firmware_header *hdr;
> +		unsigned size = amdgpu_bo_size(adev->vce.vcpu_bo);
> +
> +		adev->vce.saved_bo = kmalloc(size, GFP_KERNEL);
> +		if (!adev->vce.saved_bo)
> +			return -ENOMEM;
> +
>  		hdr = (const struct common_firmware_header *)adev-
> >vce.fw->data;
>  		adev-
> >firmware.ucode[AMDGPU_UCODE_ID_VCE].ucode_id =
> AMDGPU_UCODE_ID_VCE;
>  		adev->firmware.ucode[AMDGPU_UCODE_ID_VCE].fw =
> adev->vce.fw;
>  		adev->firmware.fw_size +=
>  			ALIGN(le32_to_cpu(hdr->ucode_size_bytes),
> PAGE_SIZE);
>  		DRM_INFO("PSP loading VCE firmware\n");
> -	}
> -
> -	if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP) {
> +	} else {
>  		r = amdgpu_vce_resume(adev);
>  		if (r)
>  			return r;
> @@ -466,6 +470,11 @@ static int vce_v4_0_sw_fini(void *handle)
>  	/* free MM table */
>  	amdgpu_virt_free_mm_table(adev);
> 
> +	if (adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) {
> +		kfree(adev->vce.saved_bo);
> +		adev->vce.saved_bo = NULL;
> +	}
> +
>  	r = amdgpu_vce_suspend(adev);
>  	if (r)
>  		return r;
> @@ -522,8 +531,18 @@ 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;
> +
> +		memcpy_fromio(adev->vce.saved_bo, ptr, size);
> +	}
> 
>  	r = vce_v4_0_hw_fini(adev);
>  	if (r)
> @@ -534,12 +553,22 @@ 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;
> +
> +		memcpy_toio(ptr, adev->vce.saved_bo, 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

Re: [PATCH 2/3] drm/amdgpu: use existing function amdgpu_bo_create_kernel

[Christian König]

Am 31.05.2017 um 21:28 schrieb Leo Liu:
> To simplify vce bo create
>
> Signed-off-by: Leo Liu <leo.liu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c | 27 +++------------------------
>   1 file changed, 3 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> index 735c38d..b9b0906 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> @@ -165,35 +165,14 @@ int amdgpu_vce_sw_init(struct amdgpu_device *adev, unsigned long size)
>   	adev->vce.fw_version = ((version_major << 24) | (version_minor << 16) |
>   				(binary_id << 8));
>   
> -	/* allocate firmware, stack and heap BO */
> -
> -	r = amdgpu_bo_create(adev, size, PAGE_SIZE, true,
> -			     AMDGPU_GEM_DOMAIN_VRAM,
> -			     AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED |
> -			     AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS,
> -			     NULL, NULL, &adev->vce.vcpu_bo);
> +	r = amdgpu_bo_create_kernel(adev, size, PAGE_SIZE,
> +				    AMDGPU_GEM_DOMAIN_VRAM, &adev->vce.vcpu_bo,
> +				    &adev->vce.gpu_addr, &adev->vce.cpu_addr);

When you now CPU map the BO you also need to make sure that this mapping 
is properly destroyed in amdgpu_vce_sw_fini().

In other words use amdgpu_bo_free_kernel() in amdgpu_vce_sw_fini() as 
well :)

Apart from that the set looks good to me,
Christian.

>   	if (r) {
>   		dev_err(adev->dev, "(%d) failed to allocate VCE bo\n", r);
>   		return r;
>   	}
>   
> -	r = amdgpu_bo_reserve(adev->vce.vcpu_bo, false);
> -	if (r) {
> -		amdgpu_bo_unref(&adev->vce.vcpu_bo);
> -		dev_err(adev->dev, "(%d) failed to reserve VCE bo\n", r);
> -		return r;
> -	}
> -
> -	r = amdgpu_bo_pin(adev->vce.vcpu_bo, AMDGPU_GEM_DOMAIN_VRAM,
> -			  &adev->vce.gpu_addr);
> -	amdgpu_bo_unreserve(adev->vce.vcpu_bo);
> -	if (r) {
> -		amdgpu_bo_unref(&adev->vce.vcpu_bo);
> -		dev_err(adev->dev, "(%d) VCE bo pin failed\n", r);
> -		return r;
> -	}
> -
> -
>   	ring = &adev->vce.ring[0];
>   	rq = &ring->sched.sched_rq[AMD_SCHED_PRIORITY_NORMAL];
>   	r = amd_sched_entity_init(&ring->sched, &adev->vce.entity,


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

Re: [PATCH 2/3] drm/amdgpu: use existing function amdgpu_bo_create_kernel

[Leo Liu]

On 06/01/2017 06:55 AM, Christian König wrote:
> Am 31.05.2017 um 21:28 schrieb Leo Liu:
>> To simplify vce bo create
>>
>> Signed-off-by: Leo Liu <leo.liu@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c | 27 
>> +++------------------------
>>   1 file changed, 3 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
>> index 735c38d..b9b0906 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
>> @@ -165,35 +165,14 @@ int amdgpu_vce_sw_init(struct amdgpu_device 
>> *adev, unsigned long size)
>>       adev->vce.fw_version = ((version_major << 24) | (version_minor 
>> << 16) |
>>                   (binary_id << 8));
>>   -    /* allocate firmware, stack and heap BO */
>> -
>> -    r = amdgpu_bo_create(adev, size, PAGE_SIZE, true,
>> -                 AMDGPU_GEM_DOMAIN_VRAM,
>> -                 AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED |
>> -                 AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS,
>> -                 NULL, NULL, &adev->vce.vcpu_bo);
>> +    r = amdgpu_bo_create_kernel(adev, size, PAGE_SIZE,
>> +                    AMDGPU_GEM_DOMAIN_VRAM, &adev->vce.vcpu_bo,
>> +                    &adev->vce.gpu_addr, &adev->vce.cpu_addr);
>
> When you now CPU map the BO you also need to make sure that this 
> mapping is properly destroyed in amdgpu_vce_sw_fini().
>
> In other words use amdgpu_bo_free_kernel() in amdgpu_vce_sw_fini() as 
> well :)

Thanks for pointing this out. I will add it following your comment.

Thanks,
Leo

>
> Apart from that the set looks good to me,
> Christian.
>
>>       if (r) {
>>           dev_err(adev->dev, "(%d) failed to allocate VCE bo\n", r);
>>           return r;
>>       }
>>   -    r = amdgpu_bo_reserve(adev->vce.vcpu_bo, false);
>> -    if (r) {
>> -        amdgpu_bo_unref(&adev->vce.vcpu_bo);
>> -        dev_err(adev->dev, "(%d) failed to reserve VCE bo\n", r);
>> -        return r;
>> -    }
>> -
>> -    r = amdgpu_bo_pin(adev->vce.vcpu_bo, AMDGPU_GEM_DOMAIN_VRAM,
>> -              &adev->vce.gpu_addr);
>> -    amdgpu_bo_unreserve(adev->vce.vcpu_bo);
>> -    if (r) {
>> -        amdgpu_bo_unref(&adev->vce.vcpu_bo);
>> -        dev_err(adev->dev, "(%d) VCE bo pin failed\n", r);
>> -        return r;
>> -    }
>> -
>> -
>>       ring = &adev->vce.ring[0];
>>       rq = &ring->sched.sched_rq[AMD_SCHED_PRIORITY_NORMAL];
>>       r = amd_sched_entity_init(&ring->sched, &adev->vce.entity,
>
>

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