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

Daniel Vetter daniel at ffwll.ch
Fri Mar 12 14:04:33 UTC 2021


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.koenig at 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.

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

> ---
>  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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the dri-devel mailing list