[PATCH 1/2] drm/amdgpu: Test for imported buffers with drm_gem_is_imported()

Thomas Zimmermann tzimmermann at suse.de
Tue Jun 24 13:24:40 UTC 2025


Hi

Am 24.06.25 um 15:17 schrieb Christian König:
> On 19.06.25 14:15, Thomas Zimmermann wrote:
>> Instead of testing import_attach for imported GEM buffers, invoke
>> drm_gem_is_imported() to do the test.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c |  2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c |  2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c     |  4 ++--
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  | 14 ++++++--------
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     |  2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c      |  4 ++--
>>   6 files changed, 13 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> index 35c778426a7c..9e463d3ee927 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> @@ -1317,7 +1317,7 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,
>>   	/* Handle is imported dma-buf, so cannot be migrated to VRAM for scanout */
>>   	bo = gem_to_amdgpu_bo(obj);
>>   	domains = amdgpu_display_supported_domains(drm_to_adev(dev), bo->flags);
>> -	if (obj->import_attach && !(domains & AMDGPU_GEM_DOMAIN_GTT)) {
>> +	if (drm_gem_is_imported(obj) && !(domains & AMDGPU_GEM_DOMAIN_GTT)) {
>>   		drm_dbg_kms(dev, "Cannot create framebuffer from imported dma_buf\n");
>>   		drm_gem_object_put(obj);
>>   		return ERR_PTR(-EINVAL);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
>> index 44e120f9f764..5743ebb2f1b7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
>> @@ -513,7 +513,7 @@ bool amdgpu_dmabuf_is_xgmi_accessible(struct amdgpu_device *adev,
>>   	if (!adev)
>>   		return false;
>>   
>> -	if (obj->import_attach) {
>> +	if (drm_gem_is_imported(obj)) {
>>   		struct dma_buf *dma_buf = obj->import_attach->dmabuf;
>>   
>>   		if (dma_buf->ops != &amdgpu_dmabuf_ops)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> index e5e33a68d935..d1ccbfcf21fa 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> @@ -317,7 +317,7 @@ static int amdgpu_gem_object_open(struct drm_gem_object *obj,
>>   	 */
>>   	if (!vm->is_compute_context || !vm->process_info)
>>   		return 0;
>> -	if (!obj->import_attach ||
>> +	if (!drm_gem_is_imported(obj) ||
>>   	    !dma_buf_is_dynamic(obj->import_attach->dmabuf))
>>   		return 0;
>>   	mutex_lock_nested(&vm->process_info->lock, 1);
>> @@ -1024,7 +1024,7 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
>>   		break;
>>   	}
>>   	case AMDGPU_GEM_OP_SET_PLACEMENT:
>> -		if (robj->tbo.base.import_attach &&
>> +		if (drm_gem_is_imported(&robj->tbo.base) &&
>>   		    args->value & AMDGPU_GEM_DOMAIN_VRAM) {
>>   			r = -EINVAL;
>>   			amdgpu_bo_unreserve(robj);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index 73403744331a..989181e5f8e6 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -62,7 +62,7 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
>>   
>>   	amdgpu_bo_kunmap(bo);
>>   
>> -	if (bo->tbo.base.import_attach)
>> +	if (drm_gem_is_imported(&bo->tbo.base))
>>   		drm_prime_gem_destroy(&bo->tbo.base, bo->tbo.sg);
>>   	drm_gem_object_release(&bo->tbo.base);
>>   	amdgpu_bo_unref(&bo->parent);
>> @@ -939,7 +939,7 @@ int amdgpu_bo_pin(struct amdgpu_bo *bo, u32 domain)
>>   		domain = bo->preferred_domains & domain;
>>   
>>   	/* A shared bo cannot be migrated to VRAM */
>> -	if (bo->tbo.base.import_attach) {
>> +	if (drm_gem_is_imported(&bo->tbo.base)) {
>>   		if (domain & AMDGPU_GEM_DOMAIN_GTT)
>>   			domain = AMDGPU_GEM_DOMAIN_GTT;
>>   		else
>> @@ -967,7 +967,7 @@ int amdgpu_bo_pin(struct amdgpu_bo *bo, u32 domain)
>>   	 */
>>   	domain = amdgpu_bo_get_preferred_domain(adev, domain);
>>   
>> -	if (bo->tbo.base.import_attach)
>> +	if (drm_gem_is_imported(&bo->tbo.base))
>>   		dma_buf_pin(bo->tbo.base.import_attach);
>>   
>>   	/* force to pin into visible video ram */
>> @@ -1018,7 +1018,7 @@ void amdgpu_bo_unpin(struct amdgpu_bo *bo)
>>   	if (bo->tbo.pin_count)
>>   		return;
>>   
>> -	if (bo->tbo.base.import_attach)
>> +	if (drm_gem_is_imported(&bo->tbo.base))
>>   		dma_buf_unpin(bo->tbo.base.import_attach);
>>   
>>   	if (bo->tbo.resource->mem_type == TTM_PL_VRAM) {
>> @@ -1263,7 +1263,7 @@ void amdgpu_bo_move_notify(struct ttm_buffer_object *bo,
>>   
>>   	amdgpu_bo_kunmap(abo);
>>   
>> -	if (abo->tbo.base.dma_buf && !abo->tbo.base.import_attach &&
>> +	if (abo->tbo.base.dma_buf && !drm_gem_is_imported(&abo->tbo.base) &&
>>   	    old_mem && old_mem->mem_type != TTM_PL_SYSTEM)
>>   		dma_buf_move_notify(abo->tbo.base.dma_buf);
>>   
>> @@ -1576,7 +1576,6 @@ uint32_t amdgpu_bo_get_preferred_domain(struct amdgpu_device *adev,
>>   u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo, struct seq_file *m)
>>   {
>>   	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>> -	struct dma_buf_attachment *attachment;
>>   	struct dma_buf *dma_buf;
>>   	const char *placement;
>>   	unsigned int pin_count;
>> @@ -1631,9 +1630,8 @@ u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo, struct seq_file *m)
>>   		seq_printf(m, " pin count %d", pin_count);
>>   
>>   	dma_buf = READ_ONCE(bo->tbo.base.dma_buf);
>> -	attachment = READ_ONCE(bo->tbo.base.import_attach);
>>   
>> -	if (attachment)
>> +	if (drm_gem_is_imported(&bo->tbo.base))
> Mhm we did that here for printing BO information without taking a lock.
>
> That isn't 100% clean since the backing DMA-buf could be destroyed in just that moment, but since it is only used for debugfs hasn't bothered anybody so far.
>
> Maybe putting this in an RCU critical section instead of the READ_ONCE would help? The underlying file is RCU protected.

Together with the READ_ONCE on the dma_buf field, it looks like this is 
fairly special anyway. I'll drop this from the patch.

Best regards
Thomas

>
> Apart from that looks good to me.
>
> Regards,
> Christian.
>
>>   		seq_printf(m, " imported from ino:%lu", file_inode(dma_buf->file)->i_ino);
>>   	else if (dma_buf)
>>   		seq_printf(m, " exported as ino:%lu", file_inode(dma_buf->file)->i_ino);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index 9c5df35f05b7..6ce45278d69b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -1060,7 +1060,7 @@ static void amdgpu_ttm_backend_unbind(struct ttm_device *bdev,
>>   	/* if the pages have userptr pinning then clear that first */
>>   	if (gtt->userptr) {
>>   		amdgpu_ttm_tt_unpin_userptr(bdev, ttm);
>> -	} else if (ttm->sg && gtt->gobj->import_attach) {
>> +	} else if (ttm->sg && drm_gem_is_imported(gtt->gobj)) {
>>   		struct dma_buf_attachment *attach;
>>   
>>   		attach = gtt->gobj->import_attach;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 0ff95a56c2ce..04100d4dea03 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1271,7 +1271,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
>>   	} else {
>>   		struct drm_gem_object *obj = &bo->tbo.base;
>>   
>> -		if (obj->import_attach && bo_va->is_xgmi) {
>> +		if (drm_gem_is_imported(obj) && bo_va->is_xgmi) {
>>   			struct dma_buf *dma_buf = obj->import_attach->dmabuf;
>>   			struct drm_gem_object *gobj = dma_buf->priv;
>>   			struct amdgpu_bo *abo = gem_to_amdgpu_bo(gobj);
>> @@ -1631,7 +1631,7 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
>>   		 * validation
>>   		 */
>>   		if (vm->is_compute_context &&
>> -		    bo_va->base.bo->tbo.base.import_attach &&
>> +		    drm_gem_is_imported(&bo_va->base.bo->tbo.base) &&
>>   		    (!bo_va->base.bo->tbo.resource ||
>>   		     bo_va->base.bo->tbo.resource->mem_type == TTM_PL_SYSTEM))
>>   			amdgpu_vm_bo_evicted_user(&bo_va->base);

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)



More information about the dri-devel mailing list