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