Re: [PATCH v3 10/28] drm/amdgpu: add amdgpu_device argument to ttm functions that need it

["Christian König" <christian.koenig@amd.com>]

On 11/21/25 17:09, Pierre-Eric Pelloux-Prayer wrote:
> 
> 
> Le 21/11/2025 à 14:24, Christian König a écrit :
>> On 11/21/25 11:12, Pierre-Eric Pelloux-Prayer wrote:
>>> Instead of getting it through amdgpu_ttm_adev(bo->tbo.bdev).
>>
>> Why should that be a good idea?
> 
> IMO explicit parameters are clearer than implicit ones so if these functions depends on adev, they might as well get it as an argument instead of fishing it from one of their other arguments.

Usually defensive code does exactly the oposite. E.g. when you can figure out adev from the BO then do it.

Only when the adev of the BO and the adev which does the operation can be different then provide that as extra parameter *and* code in such a way that this actually works.

Such things can happen in an XGMI hive for example, but makes the whole stuff much more complicated (lock inversions etc...).

> But if you prefer to keep the existing code I can drop this patch.

Yes please do so and if you see other cases please make sure to use the same approach as well.

Thanks,
Christian.

> 
> Pierre-Eric
> 
> 
>>
>> Regards,
>> Christian.
>>
>>>
>>> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  5 +++--
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 11 ++++++-----
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h    |  6 ++++--
>>>   3 files changed, 13 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> index 858eb9fa061b..2ee48f76483d 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> @@ -725,7 +725,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>>>           bo->tbo.resource->mem_type == TTM_PL_VRAM) {
>>>           struct dma_fence *fence;
>>>   -        r = amdgpu_ttm_clear_buffer(bo, bo->tbo.base.resv, &fence);
>>> +        r = amdgpu_ttm_clear_buffer(adev, bo, bo->tbo.base.resv, &fence);
>>>           if (unlikely(r))
>>>               goto fail_unreserve;
>>>   @@ -1322,7 +1322,8 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object *bo)
>>>       if (r)
>>>           goto out;
>>>   -    r = amdgpu_fill_buffer(&adev->mman.clear_entity, abo, 0, &bo->base._resv,
>>> +    r = amdgpu_fill_buffer(adev,
>>> +                   &adev->mman.clear_entity, abo, 0, &bo->base._resv,
>>>                      &fence, AMDGPU_KERNEL_JOB_ID_CLEAR_ON_RELEASE);
>>>       if (WARN_ON(r))
>>>           goto out;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> index 1d3afad885da..57dff2df433b 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> @@ -414,7 +414,7 @@ static int amdgpu_move_blit(struct ttm_buffer_object *bo,
>>>           (abo->flags & AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE)) {
>>>           struct dma_fence *wipe_fence = NULL;
>>>   -        r = amdgpu_fill_buffer(&adev->mman.move_entity,
>>> +        r = amdgpu_fill_buffer(adev, &adev->mman.move_entity,
>>>                          abo, 0, NULL, &wipe_fence,
>>>                          AMDGPU_KERNEL_JOB_ID_MOVE_BLIT);
>>>           if (r) {
>>> @@ -2350,6 +2350,7 @@ static int amdgpu_ttm_fill_mem(struct amdgpu_device *adev,
>>>     /**
>>>    * amdgpu_ttm_clear_buffer - clear memory buffers
>>> + * @adev: amdgpu device object
>>>    * @bo: amdgpu buffer object
>>>    * @resv: reservation object
>>>    * @fence: dma_fence associated with the operation
>>> @@ -2359,11 +2360,11 @@ static int amdgpu_ttm_fill_mem(struct amdgpu_device *adev,
>>>    * Returns:
>>>    * 0 for success or a negative error code on failure.
>>>    */
>>> -int amdgpu_ttm_clear_buffer(struct amdgpu_bo *bo,
>>> +int amdgpu_ttm_clear_buffer(struct amdgpu_device *adev,
>>> +                struct amdgpu_bo *bo,
>>>                   struct dma_resv *resv,
>>>                   struct dma_fence **fence)
>>>   {
>>> -    struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>>>       struct amdgpu_res_cursor cursor;
>>>       u64 addr;
>>>       int r = 0;
>>> @@ -2414,14 +2415,14 @@ int amdgpu_ttm_clear_buffer(struct amdgpu_bo *bo,
>>>       return r;
>>>   }
>>>   -int amdgpu_fill_buffer(struct amdgpu_ttm_buffer_entity *entity,
>>> +int amdgpu_fill_buffer(struct amdgpu_device *adev,
>>> +               struct amdgpu_ttm_buffer_entity *entity,
>>>                  struct amdgpu_bo *bo,
>>>                  uint32_t src_data,
>>>                  struct dma_resv *resv,
>>>                  struct dma_fence **f,
>>>                  u64 k_job_id)
>>>   {
>>> -    struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>>>       struct dma_fence *fence = NULL;
>>>       struct amdgpu_res_cursor dst;
>>>       int r;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> index 9288599c9c46..d0f55a7edd30 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> @@ -174,10 +174,12 @@ int amdgpu_copy_buffer(struct amdgpu_device *adev,
>>>                  struct dma_resv *resv,
>>>                  struct dma_fence **fence,
>>>                  bool vm_needs_flush, uint32_t copy_flags);
>>> -int amdgpu_ttm_clear_buffer(struct amdgpu_bo *bo,
>>> +int amdgpu_ttm_clear_buffer(struct amdgpu_device *adev,
>>> +                struct amdgpu_bo *bo,
>>>                   struct dma_resv *resv,
>>>                   struct dma_fence **fence);
>>> -int amdgpu_fill_buffer(struct amdgpu_ttm_buffer_entity *entity,
>>> +int amdgpu_fill_buffer(struct amdgpu_device *adev,
>>> +               struct amdgpu_ttm_buffer_entity *entity,
>>>                  struct amdgpu_bo *bo,
>>>                  uint32_t src_data,
>>>                  struct dma_resv *resv,