<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<p><br>
</p>
<br>
<div class="moz-cite-prefix">On 05/31/2017 03:54 PM, Deucher,
Alexander wrote:<br>
</div>
<blockquote
cite="mid:BN6PR12MB1652D083E4A3123CFE3B3628F7F10@BN6PR12MB1652.namprd12.prod.outlook.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Exchange Server">
<!-- converted from text -->
<style><!-- .EmailQuote { margin-left: 1pt; padding-left: 4pt; border-left: #800000 2px solid; } --></style>
<font size="2"><span style="font-size:10pt;">
<div class="PlainText">> -----Original Message-----<br>
> From: amd-gfx [<a moz-do-not-send="true"
href="mailto:amd-gfx-bounces@lists.freedesktop.org">mailto:amd-gfx-bounces@lists.freedesktop.org</a>]
On Behalf<br>
> Of Leo Liu<br>
> Sent: Wednesday, May 31, 2017 3:28 PM<br>
> To: <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a><br>
> Cc: Liu, Leo<br>
> Subject: [PATCH 3/3] drm/amdgpu: add saved_bo to save
vce 4.0 context<br>
> when suspend<br>
> <br>
> We are using PSP to resume firmware after suspend, and
it is<br>
> resumed at where it got suspended, so we'd better save
the<br>
> the context.<br>
> <br>
> Signed-off-by: Leo Liu <a class="moz-txt-link-rfc2396E" href="mailto:leo.liu@amd.com"><leo.liu@amd.com></a><br>
<br>
Patches 1, 2 are:<br>
Reviewed-by: Alex Deucher <a class="moz-txt-link-rfc2396E" href="mailto:alexander.deucher@amd.com"><alexander.deucher@amd.com></a><br>
A few comments below on patch 3.<br>
<br>
<br>
> ---<br>
> drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h | 1 +<br>
> drivers/gpu/drm/amd/amdgpu/vce_v4_0.c | 39<br>
> ++++++++++++++++++++++++++++-----<br>
> 2 files changed, 35 insertions(+), 5 deletions(-)<br>
> <br>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h<br>
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h<br>
> index c93f74a..5ce54cd 100644<br>
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h<br>
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h<br>
> @@ -34,6 +34,7 @@ struct amdgpu_vce {<br>
> struct amdgpu_bo *vcpu_bo;<br>
> uint64_t gpu_addr;<br>
> void *cpu_addr;<br>
> + void *saved_bo;<br>
> unsigned fw_version;<br>
> unsigned fb_version;<br>
> atomic_t
handles[AMDGPU_MAX_VCE_HANDLES];<br>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c<br>
> b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c<br>
> index 0b7fcc1..2230a99 100644<br>
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c<br>
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c<br>
> @@ -522,8 +522,22 @@ static int vce_v4_0_hw_fini(void
*handle)<br>
> <br>
> static int vce_v4_0_suspend(void *handle)<br>
> {<br>
> - int r;<br>
> struct amdgpu_device *adev = (struct
amdgpu_device *)handle;<br>
> + int r;<br>
> +<br>
> + if (adev->vce.vcpu_bo == NULL)<br>
> + return 0;<br>
> +<br>
> + if (adev->firmware.load_type ==
AMDGPU_FW_LOAD_PSP) {<br>
> + unsigned size =
amdgpu_bo_size(adev->vce.vcpu_bo);<br>
> + void *ptr = adev->vce.cpu_addr;<br>
> +<br>
> + adev->vce.saved_bo = kmalloc(size,
GFP_KERNEL);<br>
<br>
Might want to avoid malloc/free in the suspend/resume
paths. Just malloc/free the memory at sw_init/fini time.<br>
</div>
</span></font></blockquote>
<br>
<font size="2">Just waste a bit memory if it never goes to suspend,
but sure, I will fix it in v2.<br>
<br>
Thanks,<br>
Leo<br>
<br>
</font>
<blockquote
cite="mid:BN6PR12MB1652D083E4A3123CFE3B3628F7F10@BN6PR12MB1652.namprd12.prod.outlook.com"
type="cite"><font size="2"><span style="font-size:10pt;">
<div class="PlainText">
<br>
> + if (!adev->vce.saved_bo)<br>
> + return -ENOMEM;<br>
> +<br>
> + memcpy_fromio(adev->vce.saved_bo, ptr,
size);<br>
> + }<br>
> <br>
> r = vce_v4_0_hw_fini(adev);<br>
> if (r)<br>
> @@ -534,12 +548,27 @@ static int vce_v4_0_suspend(void
*handle)<br>
> <br>
> static int vce_v4_0_resume(void *handle)<br>
> {<br>
> - int r;<br>
> struct amdgpu_device *adev = (struct
amdgpu_device *)handle;<br>
> + int r;<br>
> <br>
> - r = amdgpu_vce_resume(adev);<br>
> - if (r)<br>
> - return r;<br>
> + if (adev->vce.vcpu_bo == NULL)<br>
> + return -EINVAL;<br>
> +<br>
> + if (adev->firmware.load_type ==
AMDGPU_FW_LOAD_PSP) {<br>
> + unsigned size =
amdgpu_bo_size(adev->vce.vcpu_bo);<br>
> + void *ptr = adev->vce.cpu_addr;<br>
> +<br>
> + if (adev->vce.saved_bo != NULL) {<br>
> + memcpy_toio(ptr,
adev->vce.saved_bo, size);<br>
> + kfree(adev->vce.saved_bo);<br>
> + adev->vce.saved_bo = NULL;<br>
> + } else<br>
<br>
Kernel coding style; should be } else { ... } to match the
chunk above.<br>
<br>
> + memset_io(ptr, 0, size);<br>
> + } else {<br>
> + r = amdgpu_vce_resume(adev);<br>
> + if (r)<br>
> + return r;<br>
> + }<br>
> <br>
> return vce_v4_0_hw_init(adev);<br>
> }<br>
> --<br>
> 2.7.4<br>
> <br>
> _______________________________________________<br>
> amd-gfx mailing list<br>
> <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a><br>
> <a moz-do-not-send="true"
href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a><br>
</div>
</span></font>
</blockquote>
<br>
</body>
</html>