Re: [PATCH] drm/amdgpu: nuke the ih reentrant lock

Christian König Fri, 12 Mar 2021 06:28:23 -0800


Am 12.03.21 um 15:04 schrieb Daniel Vetter:

On Fri, Mar 12, 2021 at 02:59:06PM +0100, Christian König wrote:

Interrupts on are non-reentrant on linux. This is just an ancient
leftover from radeon where irq processing was kicked of from different
places.

Signed-off-by: Christian König <christian.koe...@amd.com>

Man you tricked me into grepping this on radeon and it looks horrible.
atomic_t is unordered in linux, so whatever was built there for radeon
does not wokr like a lock. It's missing all the barriers afiui. Good
riddance at least for amdgpu.


Hui? atomic_xchg() is perfectly ordered as far as I know.

IIRC Alex added this for R600 because we had boards where interruptswhere sporadically swallowed during driver startup and we had to kick ofring buffer processing manually.

Anyway we have a fence processing fallback timer in amdgpu instead andthat stuff is probably unused on any modern hardware.

Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>


Thanks,
Christian.

---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c     | 5 -----
  drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h     | 1 -
  3 files changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index a15f1b604733..886625fb464b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3284,7 +3284,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,

/* mutex initialization are all done here so we

         * can recall function without having locking issues */
-       atomic_set(&adev->irq.ih.lock, 0);
        mutex_init(&adev->firmware.mutex);
        mutex_init(&adev->pm.mutex);
        mutex_init(&adev->gfx.gpu_clock_mutex);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
index 1024065f1f03..faaa6aa2faaf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
@@ -228,10 +228,6 @@ int amdgpu_ih_process(struct amdgpu_device *adev, struct 
amdgpu_ih_ring *ih)
        wptr = amdgpu_ih_get_wptr(adev, ih);

restart_ih:

-       /* is somebody else already processing irqs? */
-       if (atomic_xchg(&ih->lock, 1))
-               return IRQ_NONE;
-
        DRM_DEBUG("%s: rptr %d, wptr %d\n", __func__, ih->rptr, wptr);

/* Order reading of wptr vs. reading of IH ring data */

@@ -244,7 +240,6 @@ int amdgpu_ih_process(struct amdgpu_device *adev, struct 
amdgpu_ih_ring *ih)

amdgpu_ih_set_rptr(adev, ih);

        wake_up_all(&ih->wait_process);
-       atomic_set(&ih->lock, 0);

/* make sure wptr hasn't changed while processing */

        wptr = amdgpu_ih_get_wptr(adev, ih);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
index 87ec6d20dbe0..0649b59830a5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
@@ -64,7 +64,6 @@ struct amdgpu_ih_ring {

bool enabled;

        unsigned                rptr;
-       atomic_t                lock;
        struct amdgpu_ih_regs   ih_regs;

/* For waiting on IH processing at checkpoint. */

--
2.25.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/amdgpu: nuke the ih reentrant lock

Reply via email to