drm/i915/gem: Unify user object creation (v3)

Instead of hand-rolling the same three calls in each function, pull them
into an i915_gem_object_create_user helper.  Apart from re-ordering of
the placements array ENOMEM check, there should be no functional change.

v2 (Matthew Auld):
 - Add the call to i915_gem_flush_free_objects() from
   i915_gem_dumb_create() in a separate patch
 - Move i915_gem_object_alloc() below the simple error checks
v3 (Matthew Auld):
 - Add __ to i915_gem_object_create_user and kerneldoc which warns the
   caller that it's not validating anything.

Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210723172142.3273510-5-jason@jlekstrand.net

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c
index adcce37..23fee13 100644 (file)
--- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
@@ -11,13 +11,14 @@
 #include "i915_trace.h"
 #include "i915_user_extensions.h"
 
-static u32 object_max_page_size(struct drm_i915_gem_object *obj)
+static u32 object_max_page_size(struct intel_memory_region **placements,
+                               unsigned int n_placements)
 {
        u32 max_page_size = 0;
        int i;
 
-       for (i = 0; i < obj->mm.n_placements; i++) {
-               struct intel_memory_region *mr = obj->mm.placements[i];
+       for (i = 0; i < n_placements; i++) {
+               struct intel_memory_region *mr = placements[i];
 
                GEM_BUG_ON(!is_power_of_2(mr->min_page_size));
                max_page_size = max_t(u32, max_page_size, mr->min_page_size);
@@ -81,22 +82,46 @@ static int i915_gem_publish(struct drm_i915_gem_object *obj,
        return 0;
 }
 
-static int
-i915_gem_setup(struct drm_i915_gem_object *obj, u64 size)
+/**
+ * Creates a new object using the same path as DRM_I915_GEM_CREATE_EXT
+ * @i915: i915 private
+ * @size: size of the buffer, in bytes
+ * @placements: possible placement regions, in priority order
+ * @n_placements: number of possible placement regions
+ *
+ * This function is exposed primarily for selftests and does very little
+ * error checking.  It is assumed that the set of placement regions has
+ * already been verified to be valid.
+ */
+struct drm_i915_gem_object *
+__i915_gem_object_create_user(struct drm_i915_private *i915, u64 size,
+                             struct intel_memory_region **placements,
+                             unsigned int n_placements)
 {
-       struct intel_memory_region *mr = obj->mm.placements[0];
+       struct intel_memory_region *mr = placements[0];
+       struct drm_i915_gem_object *obj;
        unsigned int flags;
        int ret;
 
-       size = round_up(size, object_max_page_size(obj));
+       i915_gem_flush_free_objects(i915);
+
+       size = round_up(size, object_max_page_size(placements, n_placements));
        if (size == 0)
-               return -EINVAL;
+               return ERR_PTR(-EINVAL);
 
        /* For most of the ABI (e.g. mmap) we think in system pages */
        GEM_BUG_ON(!IS_ALIGNED(size, PAGE_SIZE));
 
        if (i915_gem_object_size_2big(size))
-               return -E2BIG;
+               return ERR_PTR(-E2BIG);
+
+       obj = i915_gem_object_alloc();
+       if (!obj)
+               return ERR_PTR(-ENOMEM);
+
+       ret = object_set_placements(obj, placements, n_placements);
+       if (ret)
+               goto object_free;
 
        /*
         * I915_BO_ALLOC_USER will make sure the object is cleared before
@@ -106,12 +131,18 @@ i915_gem_setup(struct drm_i915_gem_object *obj, u64 size)
 
        ret = mr->ops->init_object(mr, obj, size, 0, flags);
        if (ret)
-               return ret;
+               goto object_free;
 
        GEM_BUG_ON(size != obj->base.size);
 
        trace_i915_gem_object_create(obj);
-       return 0;
+       return obj;
+
+object_free:
+       if (obj->mm.n_placements > 1)
+               kfree(obj->mm.placements);
+       i915_gem_object_free(obj);
+       return ERR_PTR(ret);
 }
 
 int
@@ -124,7 +155,6 @@ i915_gem_dumb_create(struct drm_file *file,
        enum intel_memory_type mem_type;
        int cpp = DIV_ROUND_UP(args->bpp, 8);
        u32 format;
-       int ret;
 
        switch (cpp) {
        case 1:
@@ -151,32 +181,19 @@ i915_gem_dumb_create(struct drm_file *file,
        if (args->pitch < args->width)
                return -EINVAL;
 
-       i915_gem_flush_free_objects(i915);
-
        args->size = mul_u32_u32(args->pitch, args->height);
 
        mem_type = INTEL_MEMORY_SYSTEM;
        if (HAS_LMEM(to_i915(dev)))
                mem_type = INTEL_MEMORY_LOCAL;
 
-       obj = i915_gem_object_alloc();
-       if (!obj)
-               return -ENOMEM;
-
        mr = intel_memory_region_by_type(to_i915(dev), mem_type);
-       ret = object_set_placements(obj, &mr, 1);
-       if (ret)
-               goto object_free;
 
-       ret = i915_gem_setup(obj, args->size);
-       if (ret)
-               goto object_free;
+       obj = __i915_gem_object_create_user(to_i915(dev), args->size, &mr, 1);
+       if (IS_ERR(obj))
+               return PTR_ERR(obj);
 
        return i915_gem_publish(obj, file, &args->size, &args->handle);
-
-object_free:
-       i915_gem_object_free(obj);
-       return ret;
 }
 
 /**
@@ -193,28 +210,14 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data,
        struct drm_i915_gem_create *args = data;
        struct drm_i915_gem_object *obj;
        struct intel_memory_region *mr;
-       int ret;
-
-       i915_gem_flush_free_objects(i915);
-
-       obj = i915_gem_object_alloc();
-       if (!obj)
-               return -ENOMEM;
 
        mr = intel_memory_region_by_type(i915, INTEL_MEMORY_SYSTEM);
-       ret = object_set_placements(obj, &mr, 1);
-       if (ret)
-               goto object_free;
 
-       ret = i915_gem_setup(obj, args->size);
-       if (ret)
-               goto object_free;
+       obj = __i915_gem_object_create_user(i915, args->size, &mr, 1);
+       if (IS_ERR(obj))
+               return PTR_ERR(obj);
 
        return i915_gem_publish(obj, file, &args->size, &args->handle);
-
-object_free:
-       i915_gem_object_free(obj);
-       return ret;
 }
 
 struct create_ext {
@@ -376,38 +379,24 @@ i915_gem_create_ext_ioctl(struct drm_device *dev, void *data,
        if (args->flags)
                return -EINVAL;
 
-       i915_gem_flush_free_objects(i915);
-
-       obj = i915_gem_object_alloc();
-       if (!obj)
-               return -ENOMEM;
-
        ret = i915_user_extensions(u64_to_user_ptr(args->extensions),
                                   create_extensions,
                                   ARRAY_SIZE(create_extensions),
                                   &ext_data);
        if (ret)
-               goto object_free;
+               return ret;
 
        if (!ext_data.n_placements) {
                ext_data.placements[0] =
                        intel_memory_region_by_type(i915, INTEL_MEMORY_SYSTEM);
                ext_data.n_placements = 1;
        }
-       ret = object_set_placements(obj, ext_data.placements,
-                                   ext_data.n_placements);
-       if (ret)
-               goto object_free;
 
-       ret = i915_gem_setup(obj, args->size);
-       if (ret)
-               goto object_free;
+       obj = __i915_gem_object_create_user(i915, args->size,
+                                           ext_data.placements,
+                                           ext_data.n_placements);
+       if (IS_ERR(obj))
+               return PTR_ERR(obj);
 
        return i915_gem_publish(obj, file, &args->size, &args->handle);
-
-object_free:
-       if (obj->mm.n_placements > 1)
-               kfree(obj->mm.placements);
-       i915_gem_object_free(obj);
-       return ret;
 }

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index f3ede43..0896ac5 100644 (file)
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -61,6 +61,10 @@ i915_gem_object_create_shmem(struct drm_i915_private *i915,
 struct drm_i915_gem_object *
 i915_gem_object_create_shmem_from_data(struct drm_i915_private *i915,
                                       const void *data, resource_size_t size);
+struct drm_i915_gem_object *
+__i915_gem_object_create_user(struct drm_i915_private *i915, u64 size,
+                             struct intel_memory_region **placements,
+                             unsigned int n_placements);
 
 extern const struct drm_i915_gem_object_ops i915_gem_shmem_ops;