drm/amdgpu: Remove output parameter in bo list handling

Removing the output parameter from a few functions should result in more
readable code and also enables us to save some lines.

v2: fix build (Alex)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c

@@ -59,9 +59,9 @@ static int amdgpu_bo_list_entry_cmp(const void *_a, const void *_b)
 	return (int)a->priority - (int)b->priority;
 }
 
-int amdgpu_bo_list_create(struct amdgpu_device *adev, struct drm_file *filp,
-			  struct drm_amdgpu_bo_list_entry *info,
-			  size_t num_entries, struct amdgpu_bo_list **result)
+struct amdgpu_bo_list *
+amdgpu_bo_list_create(struct amdgpu_device *adev, struct drm_file *filp,
+		      struct drm_amdgpu_bo_list_entry *info, size_t num_entries)
 {
 	unsigned last_entry = 0, first_userptr = num_entries;
 	struct amdgpu_bo_list_entry *array;
@@ -72,7 +72,7 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev, struct drm_file *filp,
 
 	list = kvzalloc_flex(*list, entries, num_entries);
 	if (!list)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	kref_init(&list->refcount);
 
@@ -127,8 +127,7 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev, struct drm_file *filp,
 
 	trace_amdgpu_cs_bo_status(list->num_entries, total_size);
 
-	*result = list;
-	return 0;
+	return list;
 
 error_free:
 	for (i = 0; i < last_entry; ++i)
@@ -136,12 +135,11 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev, struct drm_file *filp,
 	for (i = first_userptr; i < num_entries; ++i)
 		amdgpu_bo_unref(&array[i].bo);
 	kvfree(list);
-	return r;
+	return ERR_PTR(r);
 
 }
 
-int amdgpu_bo_list_get(struct amdgpu_fpriv *fpriv, u32 id,
-		       struct amdgpu_bo_list **result)
+struct amdgpu_bo_list *amdgpu_bo_list_get(struct amdgpu_fpriv *fpriv, u32 id)
 {
 	struct amdgpu_bo_list *list;
 
@@ -149,11 +147,11 @@ int amdgpu_bo_list_get(struct amdgpu_fpriv *fpriv, u32 id,
 	list = xa_load(&fpriv->bo_list_handles, id);
 	if (list)
 		kref_get(&list->refcount);
+	else
+		list = ERR_PTR(-ENOENT);
 	xa_unlock(&fpriv->bo_list_handles);
 
-	*result = list;
-
-	return list ? 0 : -ENOENT;
+	return list;
 }
 
 void amdgpu_bo_list_put(struct amdgpu_bo_list *list)
@@ -162,25 +160,20 @@ void amdgpu_bo_list_put(struct amdgpu_bo_list *list)
 		kref_put(&list->refcount, amdgpu_bo_list_free);
 }
 
-int amdgpu_bo_create_list_entry_array(struct drm_amdgpu_bo_list_in *in,
-				      struct drm_amdgpu_bo_list_entry **info_param)
+struct drm_amdgpu_bo_list_entry *
+amdgpu_bo_create_list_entry_array(struct drm_amdgpu_bo_list_in *in)
 {
 	const void __user *uptr = u64_to_user_ptr(in->bo_info_ptr);
 	const uint32_t bo_number = in->bo_number;
-	struct drm_amdgpu_bo_list_entry *info;
 
 	if (bo_number > AMDGPU_BO_LIST_MAX_ENTRIES)
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 
 	if (in->bo_info_size != sizeof(struct drm_amdgpu_bo_list_entry))
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 
-	info = vmemdup_array_user(uptr, bo_number, sizeof(*info));
-	if (IS_ERR(info))
-		return PTR_ERR(info);
-
-	*info_param = info;
-	return 0;
+	return vmemdup_array_user(uptr, bo_number,
+				  sizeof(struct drm_amdgpu_bo_list_entry));
 }
 
 int amdgpu_bo_list_ioctl(struct drm_device *dev, void *data,
@@ -188,27 +181,24 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void *data,
 {
 	struct amdgpu_fpriv *fpriv = filp->driver_priv;
 	struct amdgpu_device *adev = drm_to_adev(dev);
-	struct drm_amdgpu_bo_list_entry *info = NULL;
 	struct amdgpu_bo_list *list, *prev, *curr;
 	union drm_amdgpu_bo_list *args = data;
 	uint32_t handle = args->in.list_handle;
+	struct drm_amdgpu_bo_list_entry *info;
 	int r;
 
-	r = amdgpu_bo_create_list_entry_array(&args->in, &info);
-	if (r)
-		return r;
-
 	switch (args->in.operation) {
 	case AMDGPU_BO_LIST_OP_CREATE:
-		r = amdgpu_bo_list_create(adev, filp, info, args->in.bo_number,
-					  &list);
-		if (r)
-			goto error_free;
+	case AMDGPU_BO_LIST_OP_UPDATE:
+		info = amdgpu_bo_create_list_entry_array(&args->in);
+		if (IS_ERR(info))
+			return PTR_ERR(info);
 
-		r = xa_alloc(&fpriv->bo_list_handles, &handle, list,
-			     xa_limit_32b, GFP_KERNEL);
-		if (r)
-			goto error_put_list;
+		list = amdgpu_bo_list_create(adev, filp, info,
+					     args->in.bo_number);
+		kvfree(info);
+		if (IS_ERR(list))
+			return PTR_ERR(list);
 
 		break;
 
@@ -219,12 +209,20 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void *data,
 
 		break;
 
-	case AMDGPU_BO_LIST_OP_UPDATE:
-		r = amdgpu_bo_list_create(adev, filp, info, args->in.bo_number,
-					  &list);
-		if (r)
-			goto error_free;
+	default:
+		return -EINVAL;
+	};
 
+	switch (args->in.operation) {
+	case AMDGPU_BO_LIST_OP_CREATE:
+		r = xa_alloc(&fpriv->bo_list_handles, &handle, list,
+			     xa_limit_32b, GFP_KERNEL);
+		if (r)
+			goto error_put_list;
+
+		break;
+
+	case AMDGPU_BO_LIST_OP_UPDATE:
 		curr = xa_load(&fpriv->bo_list_handles, handle);
 		if (!curr) {
 			r = -ENOENT;
@@ -244,21 +242,18 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void *data,
 		amdgpu_bo_list_put(curr);
 		break;
 
+	case AMDGPU_BO_LIST_OP_DESTROY:
 	default:
-		r = -EINVAL;
-		goto error_free;
+		/* Handled above. */
+		break;
 	}
 
 	memset(args, 0, sizeof(*args));
 	args->out.list_handle = handle;
-	kvfree(info);
 
 	return 0;
 
 error_put_list:
 	amdgpu_bo_list_put(list);
-
-error_free:
-	kvfree(info);
 	return r;
 }

drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h

@@ -53,17 +53,16 @@ struct amdgpu_bo_list {
 	struct amdgpu_bo_list_entry entries[] __counted_by(num_entries);
 };
 
-int amdgpu_bo_list_get(struct amdgpu_fpriv *fpriv, u32 id,
-		       struct amdgpu_bo_list **result);
+struct amdgpu_bo_list *amdgpu_bo_list_get(struct amdgpu_fpriv *fpriv, u32 id);
 void amdgpu_bo_list_put(struct amdgpu_bo_list *list);
-int amdgpu_bo_create_list_entry_array(struct drm_amdgpu_bo_list_in *in,
-				      struct drm_amdgpu_bo_list_entry **info_param);
+struct drm_amdgpu_bo_list_entry *
+amdgpu_bo_create_list_entry_array(struct drm_amdgpu_bo_list_in *in);
 
-int amdgpu_bo_list_create(struct amdgpu_device *adev,
-				 struct drm_file *filp,
-				 struct drm_amdgpu_bo_list_entry *info,
-				 size_t num_entries,
-				 struct amdgpu_bo_list **list);
+struct amdgpu_bo_list *
+amdgpu_bo_list_create(struct amdgpu_device *adev,
+		      struct drm_file *filp,
+		      struct drm_amdgpu_bo_list_entry *info,
+		      size_t num_entries);
 
 #define amdgpu_bo_list_for_each_entry(e, list) \
 	for (e = list->entries; \

drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c

@@ -140,24 +140,19 @@ static int amdgpu_cs_p1_bo_handles(struct amdgpu_cs_parser *p,
 				   struct drm_amdgpu_bo_list_in *data)
 {
 	struct drm_amdgpu_bo_list_entry *info;
-	int r;
+	struct amdgpu_bo_list *list;
 
-	r = amdgpu_bo_create_list_entry_array(data, &info);
-	if (r)
-		return r;
-
-	r = amdgpu_bo_list_create(p->adev, p->filp, info, data->bo_number,
-				  &p->bo_list);
-	if (r)
-		goto error_free;
+	info = amdgpu_bo_create_list_entry_array(data);
+	if (IS_ERR(info))
+		return PTR_ERR(info);
 
+	list = amdgpu_bo_list_create(p->adev, p->filp, info, data->bo_number);
 	kvfree(info);
+	if (IS_ERR(list))
+		return PTR_ERR(list);
+
+	p->bo_list = list;
 	return 0;
-
-error_free:
-	kvfree(info);
-
-	return r;
 }
 
 /* Copy the data from userspace and go over it the first time */
@@ -846,6 +841,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
 {
 	struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
 	struct ttm_operation_ctx ctx = { true, false };
+	struct amdgpu_bo_list *list = NULL;
 	struct amdgpu_vm *vm = &fpriv->vm;
 	struct amdgpu_bo_list_entry *e;
 	struct drm_gem_object *obj;
@@ -857,23 +853,24 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
 		if (p->bo_list)
 			return -EINVAL;
 
-		r = amdgpu_bo_list_get(fpriv, cs->in.bo_list_handle,
-				       &p->bo_list);
-		if (r)
-			return r;
+		list = amdgpu_bo_list_get(fpriv, cs->in.bo_list_handle);
 	} else if (!p->bo_list) {
 		/* Create a empty bo_list when no handle is provided */
-		r = amdgpu_bo_list_create(p->adev, p->filp, NULL, 0,
-					  &p->bo_list);
-		if (r)
-			return r;
+		list = amdgpu_bo_list_create(p->adev, p->filp, NULL, 0);
 	}
 
+	if (IS_ERR(list))
+		return PTR_ERR(list);
+	else if (list)
+		p->bo_list = list;
+	else
+		list = p->bo_list;
+
 	/* Get userptr backing pages. If pages are updated after registered
 	 * in amdgpu_gem_userptr_ioctl(), amdgpu_cs_list_validate() will do
 	 * amdgpu_ttm_backend_bind() to flush and invalidate new pages
 	 */
-	amdgpu_bo_list_for_each_userptr_entry(e, p->bo_list) {
+	amdgpu_bo_list_for_each_userptr_entry(e, list) {
 		bool userpage_invalidated = false;
 		struct amdgpu_bo *bo = e->bo;
 
@@ -903,7 +900,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
 		if (unlikely(r))
 			goto out_free_user_pages;
 
-		amdgpu_bo_list_for_each_entry(e, p->bo_list) {
+		amdgpu_bo_list_for_each_entry(e, list) {
 			r = drm_exec_prepare_obj(&p->exec, &e->bo->tbo.base,
 						 TTM_NUM_MOVE_FENCES + p->gang_size);
 			drm_exec_retry_on_contention(&p->exec);
@@ -922,7 +919,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
 		}
 	}
 
-	amdgpu_bo_list_for_each_userptr_entry(e, p->bo_list) {
+	amdgpu_bo_list_for_each_userptr_entry(e, list) {
 		struct mm_struct *usermm;
 
 		usermm = amdgpu_ttm_tt_get_usermm(e->bo->tbo.ttm);
@@ -975,13 +972,12 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
 				     p->bytes_moved_vis);
 
 	for (i = 0; i < p->gang_size; ++i)
-		amdgpu_job_set_resources(p->jobs[i], p->bo_list->gds_obj,
-					 p->bo_list->gws_obj,
-					 p->bo_list->oa_obj);
+		amdgpu_job_set_resources(p->jobs[i], list->gds_obj,
+					 list->gws_obj, list->oa_obj);
 	return 0;
 
 out_free_user_pages:
-	amdgpu_bo_list_for_each_userptr_entry(e, p->bo_list) {
+	amdgpu_bo_list_for_each_userptr_entry(e, list) {
 		amdgpu_hmm_range_free(e->range);
 		e->range = NULL;
 	}