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

Message

Qiang Yu Sept. 26, 2019, 2:10 p.m. UTC

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.

Note:
1. changes to panfrost and v3d are just compile tested.
2. patch series is based on drm-misc-next branch

Qiang Yu (6):
  drm/gem: refine drm_gem_objects_lookup
  drm/v3d: use drm_gem_objects_lookup
  drm/lima: use drm_gem_objects_lookup
  drm/lima: use drm_gem_shmem_helpers
  drm/lima: use drm_gem_(un)lock_reservations
  drm/lima: add __GFP_NOWARN flag to all dma_alloc_wc

 drivers/gpu/drm/drm_gem.c               |  28 +--
 drivers/gpu/drm/lima/Kconfig            |   1 +
 drivers/gpu/drm/lima/Makefile           |   4 +-
 drivers/gpu/drm/lima/lima_device.c      |   2 +-
 drivers/gpu/drm/lima/lima_drv.c         |  27 +--
 drivers/gpu/drm/lima/lima_gem.c         | 254 ++++++++++--------------
 drivers/gpu/drm/lima/lima_gem.h         |  32 ++-
 drivers/gpu/drm/lima/lima_gem_prime.c   |  46 -----
 drivers/gpu/drm/lima/lima_gem_prime.h   |  13 --
 drivers/gpu/drm/lima/lima_mmu.c         |   1 -
 drivers/gpu/drm/lima/lima_object.c      | 119 -----------
 drivers/gpu/drm/lima/lima_object.h      |  35 ----
 drivers/gpu/drm/lima/lima_sched.c       |   6 +-
 drivers/gpu/drm/lima/lima_vm.c          |  87 ++++----
 drivers/gpu/drm/panfrost/panfrost_drv.c |  23 ++-
 drivers/gpu/drm/v3d/v3d_gem.c           |  35 +---
 include/drm/drm_gem.h                   |   2 +-
 17 files changed, 222 insertions(+), 493 deletions(-)
 delete mode 100644 drivers/gpu/drm/lima/lima_gem_prime.c
 delete mode 100644 drivers/gpu/drm/lima/lima_gem_prime.h
 delete mode 100644 drivers/gpu/drm/lima/lima_object.c
 delete mode 100644 drivers/gpu/drm/lima/lima_object.h

Comments

Steven Price Sept. 26, 2019, 3 p.m. UTC | #1

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