From: "Christian König" <christian.koenig@amd.com>
To: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>,
amd-gfx@lists.freedesktop.org
Cc: alexander.deucher@amd.com
Subject: Re: [PATCH v10 6/6] drm/amdgpu: Enable userq fence interrupt support
Date: Mon, 13 May 2024 18:00:12 +0200 [thread overview]
Message-ID: <bfa5cf75-8e01-4f81-bf40-d89314373ddd@amd.com> (raw)
In-Reply-To: <20240510085046.2718-6-Arunpravin.PaneerSelvam@amd.com>
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@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;
prev parent reply other threads:[~2024-05-13 16:00 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-10 8:50 [PATCH v10 1/6] drm/amdgpu: Implement a new userqueue fence driver Arunpravin Paneer Selvam
2024-05-10 8:50 ` [PATCH v10 2/6] drm/amdgpu: Add mqd support for the fence address Arunpravin Paneer Selvam
2024-05-10 8:50 ` [PATCH v10 3/6] drm/amdgpu: UAPI headers for userqueue secure semaphore Arunpravin Paneer Selvam
2024-05-10 8:50 ` [PATCH v10 4/6] drm/amdgpu: Implement userqueue signal/wait IOCTL Arunpravin Paneer Selvam
2024-05-13 15:22 ` Christian König
2024-05-10 8:50 ` [PATCH v10 5/6] drm/amdgpu: Remove the MES self test Arunpravin Paneer Selvam
2024-05-13 15:53 ` Christian König
2024-05-10 8:50 ` [PATCH v10 6/6] drm/amdgpu: Enable userq fence interrupt support Arunpravin Paneer Selvam
2024-05-13 16:00 ` Christian König [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bfa5cf75-8e01-4f81-bf40-d89314373ddd@amd.com \
--to=christian.koenig@amd.com \
--cc=Arunpravin.PaneerSelvam@amd.com \
--cc=alexander.deucher@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).