[PATCH v10 6/6] drm/amdgpu: Enable userq fence interrupt support

[Christian König]

Am 10.05.24 um 10:50 schrieb Arunpravin Paneer Selvam:
> Add support to handle the userqueue protected fence signal hardware
> interrupt.
>
> Create a xarray which maps the doorbell index to the fence driver address.
> This would help to retrieve the fence driver information when an userq fence
> interrupt is triggered. Firmware sends the doorbell offset value and
> this info is compared with the queue's mqd doorbell offset value.
> If they are same, we process the userq fence interrupt.
>
> v1:(Christian)
>    - use xa_load() instead of going over all entries
>    - keep the xa_lock until the fence driver process completes
>    - create a separate patch to remove the MES self test function
>      call.
>
> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  2 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |  2 ++
>   .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c   | 15 ++++++++++++
>   drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c        | 23 +++++++++----------
>   4 files changed, 30 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 4ca14b02668b..2d5ef2e74c71 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1043,6 +1043,8 @@ struct amdgpu_device {
>   	struct amdgpu_mqd               mqds[AMDGPU_HW_IP_NUM];
>   	const struct amdgpu_userq_funcs *userq_funcs[AMDGPU_HW_IP_NUM];
>   
> +	struct xarray			userq_xa;
> +
>   	/* df */
>   	struct amdgpu_df                df;
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 2d9fa3d0d4a4..fd919105a181 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -3982,6 +3982,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>   	spin_lock_init(&adev->audio_endpt_idx_lock);
>   	spin_lock_init(&adev->mm_stats.lock);
>   
> +	xa_init_flags(&adev->userq_xa, XA_FLAGS_LOCK_IRQ);
> +
>   	INIT_LIST_HEAD(&adev->shadow_list);
>   	mutex_init(&adev->shadow_list_lock);
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> index 339d82d5808f..4cbc25595226 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> @@ -70,6 +70,7 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
>   				    struct amdgpu_usermode_queue *userq)
>   {
>   	struct amdgpu_userq_fence_driver *fence_drv;
> +	unsigned long flags;
>   	int r;
>   
>   	fence_drv = kzalloc(sizeof(*fence_drv), GFP_KERNEL);
> @@ -97,6 +98,11 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
>   	fence_drv->context = dma_fence_context_alloc(1);
>   	get_task_comm(fence_drv->timeline_name, current);
>   
> +	xa_lock_irqsave(&adev->userq_xa, flags);
> +	__xa_store(&adev->userq_xa, userq->doorbell_index,
> +		   fence_drv, GFP_KERNEL);
> +	xa_unlock_irqrestore(&adev->userq_xa, flags);
> +
>   	userq->fence_drv = fence_drv;
>   
>   	return 0;
> @@ -147,8 +153,11 @@ void amdgpu_userq_fence_driver_destroy(struct kref *ref)
>   	struct amdgpu_userq_fence_driver *fence_drv = container_of(ref,
>   					 struct amdgpu_userq_fence_driver,
>   					 refcount);
> +	struct amdgpu_userq_fence_driver *xa_fence_drv;
>   	struct amdgpu_device *adev = fence_drv->adev;
>   	struct amdgpu_userq_fence *fence, *tmp;
> +	struct xarray *xa = &adev->userq_xa;
> +	unsigned long index;
>   	struct dma_fence *f;
>   
>   	spin_lock(&fence_drv->fence_list_lock);
> @@ -165,6 +174,12 @@ void amdgpu_userq_fence_driver_destroy(struct kref *ref)
>   	}
>   	spin_unlock(&fence_drv->fence_list_lock);
>   
> +	xa_lock(xa);
> +	xa_for_each(xa, index, xa_fence_drv)
> +		if (xa_fence_drv == fence_drv)
> +			__xa_erase(xa, index);
> +	xa_unlock(xa);

That is rather inefficient. We should probably move registering a 
fence_drv for a certain doorbell into the userq code instead.

> +
>   	/* Free seq64 memory */
>   	amdgpu_seq64_free(adev, fence_drv->gpu_addr);
>   	kfree(fence_drv);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> index a786e25432ae..0a206f484240 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> @@ -49,6 +49,7 @@
>   #include "gfx_v11_0_3.h"
>   #include "nbio_v4_3.h"
>   #include "mes_v11_0.h"
> +#include "amdgpu_userq_fence.h"
>   
>   #define GFX11_NUM_GFX_RINGS		1
>   #define GFX11_MEC_HPD_SIZE	2048
> @@ -5939,25 +5940,23 @@ static int gfx_v11_0_eop_irq(struct amdgpu_device *adev,
>   			     struct amdgpu_irq_src *source,
>   			     struct amdgpu_iv_entry *entry)
>   {
> -	int i;
> +	u32 doorbell_offset = entry->src_data[0];
>   	u8 me_id, pipe_id, queue_id;
>   	struct amdgpu_ring *ring;
> -	uint32_t mes_queue_id = entry->src_data[0];
> +	int i;
>   
>   	DRM_DEBUG("IH: CP EOP\n");
>   
> -	if (adev->enable_mes && (mes_queue_id & AMDGPU_FENCE_MES_QUEUE_FLAG)) {
> -		struct amdgpu_mes_queue *queue;
> +	if (adev->enable_mes && doorbell_offset) {
> +		struct amdgpu_userq_fence_driver *fence_drv = NULL;
> +		struct xarray *xa = &adev->userq_xa;
> +		unsigned long flags;
>   
> -		mes_queue_id &= AMDGPU_FENCE_MES_QUEUE_ID_MASK;
> +		xa_lock_irqsave(xa, flags);
>   
> -		spin_lock(&adev->mes.queue_id_lock);
> -		queue = idr_find(&adev->mes.queue_id_idr, mes_queue_id);
> -		if (queue) {
> -			DRM_DEBUG("process mes queue id = %d\n", mes_queue_id);
> -			amdgpu_fence_process(queue->ring);
> -		}
> -		spin_unlock(&adev->mes.queue_id_lock);
> +		fence_drv = xa_load(xa, doorbell_offset);

It's perfectly possible that the doorbell_offset is invalid and the 
fence_drv is NULL here.

You should probably check for that.

Regards,
Christian.

> +		amdgpu_userq_fence_driver_process(fence_drv);
> +		xa_unlock_irqrestore(xa, flags);
>   	} else {
>   		me_id = (entry->ring_id & 0x0c) >> 2;
>   		pipe_id = (entry->ring_id & 0x03) >> 0;