[Lima] [PATCH 0/6] drm/lima: simplify driver by using more drm helpers

[Steven Price]

On 26/09/2019 15:10, Qiang Yu wrote:
> By using shared drm helpers:
> 1. drm_gem_objects_lookup
> 2. drm_gem_(un)lock_reservations
> 3. drm_gem_shmem_helpers
> we can simplify lima driver a lot and benifit from updates to
> these functions.
> 
> drm_gem_objects_lookup need a refine in order to be used by lima.

I'm not convinced this is actually a great idea. v3d looks like it could
already use the existing drm_gem_objects_lookup mechanism: see below
(untested) patch. So we're actually adding more code to Panfrost (and
v3d) just for the quirk in lima that the supplied array of BOs is
interleaved with flags. And the diffstat for the lima change adds lines
too! So it doesn't look like a net win to me.

I've not looked at patches 4-6, but the diffstat on those looks better!

Steve

---8<----
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 4c4b59ae2c81..dbe366d35195 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -304,48 +304,9 @@ v3d_lookup_bos(struct drm_device *dev,
 		return -EINVAL;
 	}
 
-	job->bo = kvmalloc_array(job->bo_count,
-				 sizeof(struct drm_gem_cma_object *),
-				 GFP_KERNEL | __GFP_ZERO);
-	if (!job->bo) {
-		DRM_DEBUG("Failed to allocate validated BO pointers\n");
-		return -ENOMEM;
-	}
-
-	handles = kvmalloc_array(job->bo_count, sizeof(u32), GFP_KERNEL);
-	if (!handles) {
-		ret = -ENOMEM;
-		DRM_DEBUG("Failed to allocate incoming GEM handles\n");
-		goto fail;
-	}
-
-	if (copy_from_user(handles,
-			   (void __user *)(uintptr_t)bo_handles,
-			   job->bo_count * sizeof(u32))) {
-		ret = -EFAULT;
-		DRM_DEBUG("Failed to copy in GEM handles\n");
-		goto fail;
-	}
-
-	spin_lock(&file_priv->table_lock);
-	for (i = 0; i < job->bo_count; i++) {
-		struct drm_gem_object *bo = idr_find(&file_priv->object_idr,
-						     handles[i]);
-		if (!bo) {
-			DRM_DEBUG("Failed to look up GEM BO %d: %d\n",
-				  i, handles[i]);
-			ret = -ENOENT;
-			spin_unlock(&file_priv->table_lock);
-			goto fail;
-		}
-		drm_gem_object_get(bo);
-		job->bo[i] = bo;
-	}
-	spin_unlock(&file_priv->table_lock);
-
-fail:
-	kvfree(handles);
-	return ret;
+	return drm_gem_objects_lookup(file_priv,
+				      (void __user *)(uintptr_t)bo_handles,
+				      job->bo_count, &job->bo);
 }
 
 static void