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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 10.12.20 um 05:49 schrieb Shashank Sharma:
On 09/12/20 11:00 pm, Alex Deucher wrote:
On Fri, Dec 4, 2020 at 3:41 PM Alex Deucher <alexdeucher@xxxxxxxxx> 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@xxxxxxx>
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 ?

I'm not sure why pm_runtime_get_sync() could fail, but not attaching the DMA-buf is certainly the best we could do here in that moment.

Otherwise we could end up in accessing the PCIe BAR with power disabled and that's indeed kind of bad.

Christian.

         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 ?

- 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@xxxxxxxxxxxxxxxxxxxxx
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cshashank.sharma%40amd.com%7Cd5fccf427c34414ff4e708d89c682898%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637431318483043257%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=NpCTY%2FVKd%2FBDi1wsFC79qSUTmNHx3nBp0HUj3m0cFeM%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux