Re: [PATCH] drm/amdgpu: Initialise drm_gem_object_funcs for imported BOs

Am 08.12.20 um 18:10 schrieb Andrey Grodzovsky:

For BOs imported from outside of amdgpu, setting of amdgpu_gem_object_funcs was missing in amdgpu_dma_buf_create_obj. Fix by refactoring BO creation and amdgpu_gem_object_funcs setting into single function called from both code paths.

Can you outline why we can't use amdgpu_gem_object_create() directly?

I mean we have a bit of extra error handling in there and we need to grab the resv lock and set the domains after creation, but that shouldn't matter and I don't see why that should not work.

Thanks, Christian.

This fixes null ptr regression casued by commit d693def drm: Remove obsolete GEM and PRIME callbacks from struct drm_driver

Signed-off-by: Andrey Grodzovsky andrey.grodzovsky@amd.com

drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 13 ++++++------- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 22 +++++++++++++++++----- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h | 5 +++++ 3 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c index e5919ef..da4d0ab 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c @@ -405,6 +405,7 @@ struct dma_buf *amdgpu_gem_prime_export(struct drm_gem_object *gobj, return buf; }

• /**
  • amdgpu_dma_buf_create_obj - create BO for DMA-buf import

@@ -424,7 +425,7 @@ amdgpu_dma_buf_create_obj(struct drm_device *dev, struct dma_buf *dma_buf) struct amdgpu_device *adev = drm_to_adev(dev); struct amdgpu_bo *bo; struct amdgpu_bo_param bp;

• int ret;

• struct drm_gem_object *obj;

  memset(&bp, 0, sizeof(bp)); bp.size = dma_buf->size;

@@ -434,21 +435,19 @@ amdgpu_dma_buf_create_obj(struct drm_device *dev, struct dma_buf *dma_buf) bp.type = ttm_bo_type_sg; bp.resv = resv; dma_resv_lock(resv, NULL);

• ret = amdgpu_bo_create(adev, &bp, &bo);
• if (ret)

• obj = amdgpu_gem_object_create_raw(adev, &bp);

• if (IS_ERR(obj)) goto error;

• bo = gem_to_amdgpu_bo(obj); bo->allowed_domains = AMDGPU_GEM_DOMAIN_GTT; bo->preferred_domains = AMDGPU_GEM_DOMAIN_GTT; if (dma_buf->ops != &amdgpu_dmabuf_ops) bo->prime_shared_count = 1;

• dma_resv_unlock(resv);
• return &bo->tbo.base;
• error: dma_resv_unlock(resv);
• return ERR_PTR(ret);

• return obj; }

  /**

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index c9f94fb..5f22ce6 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -52,13 +52,26 @@ static void amdgpu_gem_object_free(struct drm_gem_object *gobj) } }

+struct drm_gem_object *amdgpu_gem_object_create_raw(struct amdgpu_device *adev,

• 				    struct amdgpu_bo_param *bp)
  

+{

• struct amdgpu_bo *bo;
• int r;
• r = amdgpu_bo_create(adev, bp, &bo);
• if (r)

• return ERR_PTR(r);
  

• bo->tbo.base.funcs = &amdgpu_gem_object_funcs;
• return &bo->tbo.base;

+}

• int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size, int alignment, u32 initial_domain, u64 flags, enum ttm_bo_type type, struct dma_resv *resv, struct drm_gem_object **obj) {

• struct amdgpu_bo *bo; struct amdgpu_bo_param bp; int r;

@@ -73,8 +86,9 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size, retry: bp.flags = flags; bp.domain = initial_domain;

• r = amdgpu_bo_create(adev, &bp, &bo);
• if (r) {

• *obj = amdgpu_gem_object_create_raw(adev, &bp);
• if (IS_ERR(*obj)) {

• r = PTR_ERR(*obj);
  

  if (r != -ERESTARTSYS) { if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) { flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;

@@ -90,8 +104,6 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size, } return r; }

• *obj = &bo->tbo.base;

• (*obj)->funcs = &amdgpu_gem_object_funcs;

  return 0; }

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h index 637bf51..a6b90d3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h @@ -38,12 +38,17 @@ unsigned long amdgpu_gem_timeout(uint64_t timeout_ns); /*

• GEM objects.

*/

+struct amdgpu_bo_param;

• void amdgpu_gem_force_release(struct amdgpu_device *adev); int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size, int alignment, u32 initial_domain, u64 flags, enum ttm_bo_type type, struct dma_resv *resv, struct drm_gem_object **obj);

+struct drm_gem_object *amdgpu_gem_object_create_raw(struct amdgpu_device *adev,

• 				    struct amdgpu_bo_param *bp);
  
  

  int amdgpu_mode_dumb_create(struct drm_file *file_priv, struct drm_device *dev,