On 16.09.25 11:46, Pierre-Eric Pelloux-Prayer wrote:
>
>
> Le 16/09/2025 à 11:25, Christian König a écrit :
>> On 16.09.25 09:08, Pierre-Eric Pelloux-Prayer wrote:
>>> amdgpu_ttm_copy_mem_to_mem has a single caller, make sure the out
>>> fence is non-NULL to simplify the code.
>>> Since none of the pointers should be NULL, we can enable
>>> __attribute__((nonnull))__.
>>>
>>> While at it make the function static since it's only used from
>>> amdgpuu_ttm.c.
>>>
>>> Signed-off-by: Pierre-Eric Pelloux-Prayer
>>> <[email protected]>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 17 ++++++++---------
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 6 ------
>>> 2 files changed, 8 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> index 27ab4e754b2a..70b817b5578d 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> @@ -284,12 +284,13 @@ static int amdgpu_ttm_map_buffer(struct
>>> ttm_buffer_object *bo,
>>> * move and different for a BO to BO copy.
>>> *
>>> */
>>> -int amdgpu_ttm_copy_mem_to_mem(struct amdgpu_device *adev,
>>> - const struct amdgpu_copy_mem *src,
>>> - const struct amdgpu_copy_mem *dst,
>>> - uint64_t size, bool tmz,
>>> - struct dma_resv *resv,
>>> - struct dma_fence **f)
>>> +__attribute__((nonnull))
>>
>> That looks fishy.
>>
>>> +static int amdgpu_ttm_copy_mem_to_mem(struct amdgpu_device *adev,
>>> + const struct amdgpu_copy_mem *src,
>>> + const struct amdgpu_copy_mem *dst,
>>> + uint64_t size, bool tmz,
>>> + struct dma_resv *resv,
>>> + struct dma_fence **f)
>>
>> I'm not an expert for those, but looking at other examples that should be
>> here and look something like:
>>
>> __attribute__((nonnull(7)))
>
> Both syntax are valid. The GCC docs says:
>
> If no arg-index is given to the nonnull attribute, all pointer arguments
> are marked as non-null
Never seen that before. Is that gcc specifc or standardized?
>
>
>>
>> But I think for this case here it is also not a must have to have that.
>
> I can remove it if you prefer, but it doesn't hurt to have the compiler
> validate usage of the functions.
Yeah it's clearly useful, but I'm worried that clang won't like it.
Christian.
>
> Pierre-Eric
>
>
>>
>> Regards,
>> Christian.
>>
>>> {
>>> struct amdgpu_ring *ring = adev->mman.buffer_funcs_ring;
>>> struct amdgpu_res_cursor src_mm, dst_mm;
>>> @@ -363,9 +364,7 @@ int amdgpu_ttm_copy_mem_to_mem(struct amdgpu_device
>>> *adev,
>>> }
>>> error:
>>> mutex_unlock(&adev->mman.gtt_window_lock);
>>> - if (f)
>>> - *f = dma_fence_get(fence);
>>> - dma_fence_put(fence);
>>> + *f = fence;
>>> return r;
>>> }
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> index bb17987f0447..07ae2853c77c 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> @@ -170,12 +170,6 @@ int amdgpu_copy_buffer(struct amdgpu_ring *ring,
>>> uint64_t src_offset,
>>> struct dma_resv *resv,
>>> struct dma_fence **fence, bool direct_submit,
>>> bool vm_needs_flush, uint32_t copy_flags);
>>> -int amdgpu_ttm_copy_mem_to_mem(struct amdgpu_device *adev,
>>> - const struct amdgpu_copy_mem *src,
>>> - const struct amdgpu_copy_mem *dst,
>>> - uint64_t size, bool tmz,
>>> - struct dma_resv *resv,
>>> - struct dma_fence **f);
>>> int amdgpu_ttm_clear_buffer(struct amdgpu_bo *bo,
>>> struct dma_resv *resv,
>>> struct dma_fence **fence);
