[PATCH] drm/amdgpu: take runtime pm reference when we attach a buffer

[Alex Deucher]

On Wed, Dec 9, 2020 at 11:49 PM Shashank Sharma <shashank.sharma at amd.com> wrote:
>
>
> On 09/12/20 11:00 pm, Alex Deucher wrote:
> > On Fri, Dec 4, 2020 at 3:41 PM Alex Deucher <alexdeucher at gmail.com> wrote:
> >> And drop it when we detach.  If the shared buffer is in vram,
> >> we need to make sure we don't put the device into runtime
> >> suspend.
> >>
> >> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
> >
> > Ping?  Any thoughts on this?  We really only need this for p2p since
> > device memory in involved, but I'm not sure of the best way to handle
> > that.
> >
> > Alex
> >
> >
> >> ---
> >>  drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 16 ++++++++++++++--
> >>  1 file changed, 14 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> >> index 5b465ab774d1..f63f182f37f9 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> >> @@ -40,6 +40,7 @@
> >>  #include <linux/dma-buf.h>
> >>  #include <linux/dma-fence-array.h>
> >>  #include <linux/pci-p2pdma.h>
> >> +#include <linux/pm_runtime.h>
> >>
> >>  /**
> >>   * amdgpu_gem_prime_vmap - &dma_buf_ops.vmap implementation
> >> @@ -187,9 +188,13 @@ static int amdgpu_dma_buf_attach(struct dma_buf *dmabuf,
> >>         if (attach->dev->driver == adev->dev->driver)
> >>                 return 0;
> >>
> >> +       r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
> >> +       if (r < 0)
> >> +               goto out;
> >> +
> I am a bit skeptical if we should fail the BO reserve if we don't get the sync ? I was hoping to continue with it, with a warning maybe, so that it doesn't block the existing functionality ?
> >>         r = amdgpu_bo_reserve(bo, false);
> >>         if (unlikely(r != 0))
> >> -               return r;
> >> +               goto out;
> >>
> >>         /*
> >>          * We only create shared fences for internal use, but importers
> >> @@ -201,11 +206,15 @@ static int amdgpu_dma_buf_attach(struct dma_buf *dmabuf,
> >>          */
> >>         r = __dma_resv_make_exclusive(bo->tbo.base.resv);
> >>         if (r)
> >> -               return r;
> >> +               goto out;
> >>
> >>         bo->prime_shared_count++;
> >>         amdgpu_bo_unreserve(bo);
> >>         return 0;
> >> +
> >> +out:
> >> +       pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
>
> Why not just pm_runtime_put_sync ? Why autosuspend ?

Not sure.  I'm just copying what we do in other cases which happens to
work.  I'm not really a runtime pm expert.

Alex

>
> - Shashank
>
> >> +       return r;
> >>  }
> >>
> >>  /**
> >> @@ -225,6 +234,9 @@ static void amdgpu_dma_buf_detach(struct dma_buf *dmabuf,
> >>
> >>         if (attach->dev->driver != adev->dev->driver && bo->prime_shared_count)
> >>                 bo->prime_shared_count--;
> >> +
> >> +       pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> >> +       pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> >>  }
> >>
> >>  /**
> >> --
> >> 2.25.4
> >>
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx at lists.freedesktop.org
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Cshashank.sharma%40amd.com%7Cd5fccf427c34414ff4e708d89c682898%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637431318483043257%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=NpCTY%2FVKd%2FBDi1wsFC79qSUTmNHx3nBp0HUj3m0cFeM%3D&reserved=0
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx