Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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

authored by

Christian König and committed by
Alex Deucher
8bd82363 49c9ffab

-51
-34
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
··· 41 41 #include <linux/dma-buf.h> 42 42 #include <linux/dma-fence-array.h> 43 43 #include <linux/pci-p2pdma.h> 44 - #include <linux/pm_runtime.h> 45 - #include "amdgpu_trace.h" 46 44 47 45 /** 48 46 * amdgpu_dma_buf_attach - &dma_buf_ops.attach implementation ··· 56 58 struct drm_gem_object *obj = dmabuf->priv; 57 59 struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj); 58 60 struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev); 59 - int r; 60 61 61 62 if (pci_p2pdma_distance(adev->pdev, attach->dev, false) < 0) 62 63 attach->peer2peer = false; 63 64 64 - r = pm_runtime_get_sync(adev_to_drm(adev)->dev); 65 - trace_amdgpu_runpm_reference_dumps(1, __func__); 66 - if (r < 0) 67 - goto out; 68 - 69 65 return 0; 70 - 71 - out: 72 - pm_runtime_put_autosuspend(adev_to_drm(adev)->dev); 73 - trace_amdgpu_runpm_reference_dumps(0, __func__); 74 - return r; 75 - } 76 - 77 - /** 78 - * amdgpu_dma_buf_detach - &dma_buf_ops.detach implementation 79 - * 80 - * @dmabuf: DMA-buf where we remove the attachment from 81 - * @attach: the attachment to remove 82 - * 83 - * Called when an attachment is removed from the DMA-buf. 84 - */ 85 - static void amdgpu_dma_buf_detach(struct dma_buf *dmabuf, 86 - struct dma_buf_attachment *attach) 87 - { 88 - struct drm_gem_object *obj = dmabuf->priv; 89 - struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj); 90 - struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev); 91 - 92 - pm_runtime_mark_last_busy(adev_to_drm(adev)->dev); 93 - pm_runtime_put_autosuspend(adev_to_drm(adev)->dev); 94 - trace_amdgpu_runpm_reference_dumps(0, __func__); 95 66 } 96 67 97 68 /** ··· 234 267 235 268 const struct dma_buf_ops amdgpu_dmabuf_ops = { 236 269 .attach = amdgpu_dma_buf_attach, 237 - .detach = amdgpu_dma_buf_detach, 238 270 .pin = amdgpu_dma_buf_pin, 239 271 .unpin = amdgpu_dma_buf_unpin, 240 272 .map_dma_buf = amdgpu_dma_buf_map,
-2
drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
··· 181 181 amdgpu_ring_emit_fence(ring, ring->fence_drv.gpu_addr, 182 182 seq, flags | AMDGPU_FENCE_FLAG_INT); 183 183 pm_runtime_get_noresume(adev_to_drm(adev)->dev); 184 - trace_amdgpu_runpm_reference_dumps(1, __func__); 185 184 ptr = &ring->fence_drv.fences[seq & ring->fence_drv.num_fences_mask]; 186 185 if (unlikely(rcu_dereference_protected(*ptr, 1))) { 187 186 struct dma_fence *old; ··· 308 309 dma_fence_put(fence); 309 310 pm_runtime_mark_last_busy(adev_to_drm(adev)->dev); 310 311 pm_runtime_put_autosuspend(adev_to_drm(adev)->dev); 311 - trace_amdgpu_runpm_reference_dumps(0, __func__); 312 312 } while (last_seq != seq); 313 313 314 314 return true;
-15
drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
··· 554 554 __entry->value) 555 555 ); 556 556 557 - TRACE_EVENT(amdgpu_runpm_reference_dumps, 558 - TP_PROTO(uint32_t index, const char *func), 559 - TP_ARGS(index, func), 560 - TP_STRUCT__entry( 561 - __field(uint32_t, index) 562 - __string(func, func) 563 - ), 564 - TP_fast_assign( 565 - __entry->index = index; 566 - __assign_str(func); 567 - ), 568 - TP_printk("amdgpu runpm reference dump 0x%x: 0x%s\n", 569 - __entry->index, 570 - __get_str(func)) 571 - ); 572 557 #undef AMDGPU_JOB_GET_TIMELINE_NAME 573 558 #endif 574 559