On 12/19/25 09:39, Tvrtko Ursulin wrote:
>
> On 19/12/2025 08:29, Christian König wrote:
>>
>>
>> On 12/18/25 16:04, Tvrtko Ursulin wrote:
>>> Commit
>>> cb17fff3a254 ("drm/amdgpu/mes: remove unused functions")
>>> removed most of the code using these IDRs but forgot to remove the struct
>>> members and init/destroy paths.
>>>
>>> There is also interrupt handling code in SDMA 5.0 and 5.2 which appears to
>>> be using it, but is is unreachable since nothing ever allocates the
>>> relevant IDR. We replace those with one time warnings just to avoid any
>>> functional difference, but it is also possible they should be removed.
>>>
>>> Signed-off-by: Tvrtko Ursulin <[email protected]>
>>> References: cb17fff3a254 ("drm/amdgpu/mes: remove unused functions")
>>> Cc: Alex Deucher <[email protected]>
>>
>> Reviewed-by: Christian König <[email protected]>
>
> Thanks!
>
> Do you think the drm_WARN_ON_ONCE's are worth keeping or I should remove that
> completely?
I wanted to discuss that with Alex after the holidays, but my gut feeling is it
is superflous.
> I wasn't sure if there is a code path or not to enable the required condition:
>
> adev->enable_mes && (entry->src_data[0] & AMDGPU_FENCE_MES_QUEUE_FLAG))
>
> If there is interrupts would get silently not processed. Although that is
> currently in the code as well. Maybe it is impossible?
Yeah that's not so much of a problem. At worst it will create a performance
problem because we only check the fence after a timeout.
Regards,
Christian.
>
> Regards,
>
> Tvrtko
>
>>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 9 ---------
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h | 3 ---
>>> drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 18 +++---------------
>>> drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c | 18 +++---------------
>>> 4 files changed, 6 insertions(+), 42 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
>>> index 9c182ce501af..505619d504ea 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
>>> @@ -94,9 +94,6 @@ int amdgpu_mes_init(struct amdgpu_device *adev)
>>> adev->mes.adev = adev;
>>> - idr_init(&adev->mes.pasid_idr);
>>> - idr_init(&adev->mes.gang_id_idr);
>>> - idr_init(&adev->mes.queue_id_idr);
>>> ida_init(&adev->mes.doorbell_ida);
>>> spin_lock_init(&adev->mes.queue_id_lock);
>>> mutex_init(&adev->mes.mutex_hidden);
>>> @@ -218,9 +215,6 @@ int amdgpu_mes_init(struct amdgpu_device *adev)
>>> adev->mes.query_status_fence_offs[i]);
>>> }
>>> - idr_destroy(&adev->mes.pasid_idr);
>>> - idr_destroy(&adev->mes.gang_id_idr);
>>> - idr_destroy(&adev->mes.queue_id_idr);
>>> ida_destroy(&adev->mes.doorbell_ida);
>>> mutex_destroy(&adev->mes.mutex_hidden);
>>> return r;
>>> @@ -248,9 +242,6 @@ void amdgpu_mes_fini(struct amdgpu_device *adev)
>>> amdgpu_mes_doorbell_free(adev);
>>> - idr_destroy(&adev->mes.pasid_idr);
>>> - idr_destroy(&adev->mes.gang_id_idr);
>>> - idr_destroy(&adev->mes.queue_id_idr);
>>> ida_destroy(&adev->mes.doorbell_ida);
>>> mutex_destroy(&adev->mes.mutex_hidden);
>>> }
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
>>> index e989225b354b..f45129277479 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
>>> @@ -68,9 +68,6 @@ struct amdgpu_mes {
>>> struct mutex mutex_hidden;
>>> - struct idr pasid_idr;
>>> - struct idr gang_id_idr;
>>> - struct idr queue_id_idr;
>>> struct ida doorbell_ida;
>>> spinlock_t queue_id_lock;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
>>> index 8ddc4df06a1f..ab9e6199b01d 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
>>> @@ -1704,24 +1704,12 @@ static int sdma_v5_0_process_trap_irq(struct
>>> amdgpu_device *adev,
>>> struct amdgpu_irq_src *source,
>>> struct amdgpu_iv_entry *entry)
>>> {
>>> - uint32_t mes_queue_id = entry->src_data[0];
>>> -
>>> DRM_DEBUG("IH: SDMA trap\n");
>>> - if (adev->enable_mes && (mes_queue_id &
>>> AMDGPU_FENCE_MES_QUEUE_FLAG)) {
>>> - struct amdgpu_mes_queue *queue;
>>> -
>>> - mes_queue_id &= AMDGPU_FENCE_MES_QUEUE_ID_MASK;
>>> -
>>> - spin_lock(&adev->mes.queue_id_lock);
>>> - queue = idr_find(&adev->mes.queue_id_idr, mes_queue_id);
>>> - if (queue) {
>>> - DRM_DEBUG("process smda queue id = %d\n", mes_queue_id);
>>> - amdgpu_fence_process(queue->ring);
>>> - }
>>> - spin_unlock(&adev->mes.queue_id_lock);
>>> + if (drm_WARN_ON_ONCE(&adev->ddev,
>>> + adev->enable_mes &&
>>> + (entry->src_data[0] & AMDGPU_FENCE_MES_QUEUE_FLAG)))
>>> return 0;
>>> - }
>>> switch (entry->client_id) {
>>> case SOC15_IH_CLIENTID_SDMA0:
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
>>> b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
>>> index 51101b0aa2fa..4f78dd93939c 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
>>> @@ -1617,24 +1617,12 @@ static int sdma_v5_2_process_trap_irq(struct
>>> amdgpu_device *adev,
>>> struct amdgpu_irq_src *source,
>>> struct amdgpu_iv_entry *entry)
>>> {
>>> - uint32_t mes_queue_id = entry->src_data[0];
>>> -
>>> DRM_DEBUG("IH: SDMA trap\n");
>>> - if (adev->enable_mes && (mes_queue_id &
>>> AMDGPU_FENCE_MES_QUEUE_FLAG)) {
>>> - struct amdgpu_mes_queue *queue;
>>> -
>>> - mes_queue_id &= AMDGPU_FENCE_MES_QUEUE_ID_MASK;
>>> -
>>> - spin_lock(&adev->mes.queue_id_lock);
>>> - queue = idr_find(&adev->mes.queue_id_idr, mes_queue_id);
>>> - if (queue) {
>>> - DRM_DEBUG("process smda queue id = %d\n", mes_queue_id);
>>> - amdgpu_fence_process(queue->ring);
>>> - }
>>> - spin_unlock(&adev->mes.queue_id_lock);
>>> + if (drm_WARN_ON_ONCE(&adev->ddev,
>>> + adev->enable_mes &&
>>> + (entry->src_data[0] & AMDGPU_FENCE_MES_QUEUE_FLAG)))
>>> return 0;
>>> - }
>>> switch (entry->client_id) {
>>> case SOC15_IH_CLIENTID_SDMA0:
>>
>
