drm/amdgpu: revert "take runtime pm reference when we attach a buffer" v2

This reverts commit b8c415e3bf98 ("drm/amdgpu: take runtime pm reference
when we attach a buffer") and commit 425285d39afd ("drm/amdgpu: add amdgpu
runpm usage trace for separate funcs").

Taking a runtime pm reference for DMA-buf is actually completely
unnecessary and even dangerous.

The problem is that calling pm_runtime_get_sync() from the DMA-buf
callbacks is illegal because we have the reservation locked here
which is also taken during resume. So this would deadlock.

When the buffer is in GTT it is still accessible even when the GPU
is powered down and when it is in VRAM the buffer gets migrated to
GTT before powering down.

The only use case which would make it mandatory to keep the runtime
pm reference would be if we pin the buffer into VRAM, and that's not
something we currently do.

v2: improve the commit message

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
CC: stable@vger.kernel.org

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 0b3b10d219529f1dc4078883e42a35d4f84c356c..8e81a83d37d846cc927306739f8c5b7eaa222076 100644 (file)
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -41,8 +41,6 @@
 #include <linux/dma-buf.h>
 #include <linux/dma-fence-array.h>
 #include <linux/pci-p2pdma.h>
-#include <linux/pm_runtime.h>
-#include "amdgpu_trace.h"
 
 /**
  * amdgpu_dma_buf_attach - &dma_buf_ops.attach implementation
@@ -58,42 +56,11 @@ static int amdgpu_dma_buf_attach(struct dma_buf *dmabuf,
        struct drm_gem_object *obj = dmabuf->priv;
        struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
        struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
-       int r;
 
        if (pci_p2pdma_distance(adev->pdev, attach->dev, false) < 0)
                attach->peer2peer = false;
 
-       r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
-       trace_amdgpu_runpm_reference_dumps(1, __func__);
-       if (r < 0)
-               goto out;
-
        return 0;
-
-out:
-       pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
-       trace_amdgpu_runpm_reference_dumps(0, __func__);
-       return r;
-}
-
-/**
- * amdgpu_dma_buf_detach - &dma_buf_ops.detach implementation
- *
- * @dmabuf: DMA-buf where we remove the attachment from
- * @attach: the attachment to remove
- *
- * Called when an attachment is removed from the DMA-buf.
- */
-static void amdgpu_dma_buf_detach(struct dma_buf *dmabuf,
-                                 struct dma_buf_attachment *attach)
-{
-       struct drm_gem_object *obj = dmabuf->priv;
-       struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
-       struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
-
-       pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
-       pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
-       trace_amdgpu_runpm_reference_dumps(0, __func__);
 }
 
 /**
@@ -266,7 +233,6 @@ static int amdgpu_dma_buf_begin_cpu_access(struct dma_buf *dma_buf,
 
 const struct dma_buf_ops amdgpu_dmabuf_ops = {
        .attach = amdgpu_dma_buf_attach,
-       .detach = amdgpu_dma_buf_detach,
        .pin = amdgpu_dma_buf_pin,
        .unpin = amdgpu_dma_buf_unpin,
        .map_dma_buf = amdgpu_dma_buf_map,

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 3f492277d7d39a4c4bd441f8264704a830f2af36..2f24a6aa13bf6e6ea66b2489bda7b43947e7578d 100644 (file)
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -181,7 +181,6 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring, struct dma_fence **f, struct amd
        amdgpu_ring_emit_fence(ring, ring->fence_drv.gpu_addr,
                               seq, flags | AMDGPU_FENCE_FLAG_INT);
        pm_runtime_get_noresume(adev_to_drm(adev)->dev);
-       trace_amdgpu_runpm_reference_dumps(1, __func__);
        ptr = &ring->fence_drv.fences[seq & ring->fence_drv.num_fences_mask];
        if (unlikely(rcu_dereference_protected(*ptr, 1))) {
                struct dma_fence *old;
@@ -309,7 +308,6 @@ bool amdgpu_fence_process(struct amdgpu_ring *ring)
                dma_fence_put(fence);
                pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
                pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
-               trace_amdgpu_runpm_reference_dumps(0, __func__);
        } while (last_seq != seq);
 
        return true;

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
index f539b1d002343ea2a79af93595919a067da52742..2fd1bfb35916fe63f2865d4bfad4f96ecd547627 100644 (file)
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
@@ -554,21 +554,6 @@ TRACE_EVENT(amdgpu_reset_reg_dumps,
                      __entry->value)
 );
 
-TRACE_EVENT(amdgpu_runpm_reference_dumps,
-           TP_PROTO(uint32_t index, const char *func),
-           TP_ARGS(index, func),
-           TP_STRUCT__entry(
-                            __field(uint32_t, index)
-                            __string(func, func)
-                            ),
-           TP_fast_assign(
-                          __entry->index = index;
-                          __assign_str(func, func);
-                          ),
-           TP_printk("amdgpu runpm reference dump 0x%x: 0x%s\n",
-                     __entry->index,
-                     __get_str(func))
-);
 #undef AMDGPU_JOB_GET_TIMELINE_NAME
 #endif