This patch series fixes an issue with discrete graphics on Intel where we allowed dma-buf import while leaving the object in local memory. This breaks down pretty badly if the import happened on a different physical device.
v7: - Drop "drm/i915/gem/ttm: Place new BOs in the requested region" - Add a new "drm/i915/gem: Call i915_gem_flush_free_objects() in i915_gem_dumb_create()" - Misc. review feedback from Matthew Auld
Jason Ekstrand (5): drm/i915/gem: Check object_can_migrate from object_migrate drm/i915/gem: Refactor placement setup for i915_gem_object_create* (v2) drm/i915/gem: Call i915_gem_flush_free_objects() in i915_gem_dumb_create() drm/i915/gem: Unify user object creation (v2) drm/i915/gem/ttm: Respect the objection region in placement_from_obj
Thomas Hellström (2): drm/i915/gem: Correct the locking and pin pattern for dma-buf (v6) drm/i915/gem: Migrate to system at dma-buf attach time (v6)
drivers/gpu/drm/i915/gem/i915_gem_create.c | 165 ++++++++-------- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 64 ++++-- drivers/gpu/drm/i915/gem/i915_gem_object.c | 13 +- drivers/gpu/drm/i915/gem/i915_gem_object.h | 4 + drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 3 +- .../drm/i915/gem/selftests/i915_gem_dmabuf.c | 184 +++++++++++++++++- .../drm/i915/gem/selftests/i915_gem_migrate.c | 15 -- 7 files changed, 318 insertions(+), 130 deletions(-)
We don't roll them together entirely because there are still a couple cases where we want a separate can_migrate check. For instance, the display code checks that you can migrate a buffer to LMEM before it accepts it in fb_create. The dma-buf import code also uses it to do an early check and return a different error code if someone tries to attach a LMEM-only dma-buf to another driver.
However, no one actually wants to call object_migrate when can_migrate has failed. The stated intention is for self-tests but none of those actually take advantage of this unsafe migration.
Signed-off-by: Jason Ekstrand jason@jlekstrand.net Cc: Daniel Vetter daniel@ffwll.ch Reviewed-by: Matthew Auld matthew.auld@intel.com --- drivers/gpu/drm/i915/gem/i915_gem_object.c | 13 ++----------- .../gpu/drm/i915/gem/selftests/i915_gem_migrate.c | 15 --------------- 2 files changed, 2 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c index 9da7b288b7ede..f2244ae09a613 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -584,12 +584,6 @@ bool i915_gem_object_can_migrate(struct drm_i915_gem_object *obj, * completed yet, and to accomplish that, i915_gem_object_wait_migration() * must be called. * - * This function is a bit more permissive than i915_gem_object_can_migrate() - * to allow for migrating objects where the caller knows exactly what is - * happening. For example within selftests. More specifically this - * function allows migrating I915_BO_ALLOC_USER objects to regions - * that are not in the list of allowable regions. - * * Note: the @ww parameter is not used yet, but included to make sure * callers put some effort into obtaining a valid ww ctx if one is * available. @@ -616,11 +610,8 @@ int i915_gem_object_migrate(struct drm_i915_gem_object *obj, if (obj->mm.region == mr) return 0;
- if (!i915_gem_object_evictable(obj)) - return -EBUSY; - - if (!obj->ops->migrate) - return -EOPNOTSUPP; + if (!i915_gem_object_can_migrate(obj, id)) + return -EINVAL;
return obj->ops->migrate(obj, mr); } diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_migrate.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_migrate.c index 0b7144d2991ca..28a700f08b49a 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_migrate.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_migrate.c @@ -61,11 +61,6 @@ static int igt_create_migrate(struct intel_gt *gt, enum intel_region_id src, if (err) continue;
- if (!i915_gem_object_can_migrate(obj, dst)) { - err = -EINVAL; - continue; - } - err = i915_gem_object_migrate(obj, &ww, dst); if (err) continue; @@ -114,11 +109,6 @@ static int lmem_pages_migrate_one(struct i915_gem_ww_ctx *ww, return err;
if (i915_gem_object_is_lmem(obj)) { - if (!i915_gem_object_can_migrate(obj, INTEL_REGION_SMEM)) { - pr_err("object can't migrate to smem.\n"); - return -EINVAL; - } - err = i915_gem_object_migrate(obj, ww, INTEL_REGION_SMEM); if (err) { pr_err("Object failed migration to smem\n"); @@ -137,11 +127,6 @@ static int lmem_pages_migrate_one(struct i915_gem_ww_ctx *ww, }
} else { - if (!i915_gem_object_can_migrate(obj, INTEL_REGION_LMEM)) { - pr_err("object can't migrate to lmem.\n"); - return -EINVAL; - } - err = i915_gem_object_migrate(obj, ww, INTEL_REGION_LMEM); if (err) { pr_err("Object failed migration to lmem\n");
Since we don't allow changing the set of regions after creation, we can make ext_set_placements() build up the region set directly in the create_ext and assign it to the object later. This is similar to what we did for contexts with the proto-context only simpler because there's no funny object shuffling. This will be used in the next patch to allow us to de-duplicate a bunch of code. Also, since we know the maximum number of regions up-front, we can use a fixed-size temporary array for the regions. This simplifies memory management a bit for this new delayed approach.
v2 (Matthew Auld): - Get rid of MAX_N_PLACEMENTS - Drop kfree(placements) from set_placements()
Signed-off-by: Jason Ekstrand jason@jlekstrand.net Cc: Matthew Auld matthew.auld@intel.com --- drivers/gpu/drm/i915/gem/i915_gem_create.c | 81 ++++++++++++---------- 1 file changed, 45 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c index 51f92e4b1a69d..5766749a449c0 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c @@ -27,10 +27,13 @@ static u32 object_max_page_size(struct drm_i915_gem_object *obj) return max_page_size; }
-static void object_set_placements(struct drm_i915_gem_object *obj, - struct intel_memory_region **placements, - unsigned int n_placements) +static int object_set_placements(struct drm_i915_gem_object *obj, + struct intel_memory_region **placements, + unsigned int n_placements) { + struct intel_memory_region **arr; + unsigned int i; + GEM_BUG_ON(!n_placements);
/* @@ -44,9 +47,20 @@ static void object_set_placements(struct drm_i915_gem_object *obj, obj->mm.placements = &i915->mm.regions[mr->id]; obj->mm.n_placements = 1; } else { - obj->mm.placements = placements; + arr = kmalloc_array(n_placements, + sizeof(struct intel_memory_region *), + GFP_KERNEL); + if (!arr) + return -ENOMEM; + + for (i = 0; i < n_placements; i++) + arr[i] = placements[i]; + + obj->mm.placements = arr; obj->mm.n_placements = n_placements; } + + return 0; }
static int i915_gem_publish(struct drm_i915_gem_object *obj, @@ -148,7 +162,9 @@ i915_gem_dumb_create(struct drm_file *file, return -ENOMEM;
mr = intel_memory_region_by_type(to_i915(dev), mem_type); - object_set_placements(obj, &mr, 1); + ret = object_set_placements(obj, &mr, 1); + if (ret) + goto object_free;
ret = i915_gem_setup(obj, args->size); if (ret) @@ -184,7 +200,9 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data, return -ENOMEM;
mr = intel_memory_region_by_type(i915, INTEL_MEMORY_SYSTEM); - object_set_placements(obj, &mr, 1); + ret = object_set_placements(obj, &mr, 1); + if (ret) + goto object_free;
ret = i915_gem_setup(obj, args->size); if (ret) @@ -199,7 +217,8 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data,
struct create_ext { struct drm_i915_private *i915; - struct drm_i915_gem_object *vanilla_object; + struct intel_memory_region *placements[INTEL_REGION_UNKNOWN]; + unsigned int n_placements; };
static void repr_placements(char *buf, size_t size, @@ -230,8 +249,7 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, struct drm_i915_private *i915 = ext_data->i915; struct drm_i915_gem_memory_class_instance __user *uregions = u64_to_user_ptr(args->regions); - struct drm_i915_gem_object *obj = ext_data->vanilla_object; - struct intel_memory_region **placements; + struct intel_memory_region *placements[INTEL_REGION_UNKNOWN]; u32 mask; int i, ret = 0;
@@ -245,6 +263,8 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, ret = -EINVAL; }
+ BUILD_BUG_ON(ARRAY_SIZE(i915->mm.regions) != ARRAY_SIZE(placements)); + BUILD_BUG_ON(ARRAY_SIZE(ext_data->placements) != ARRAY_SIZE(placements)); if (args->num_regions > ARRAY_SIZE(i915->mm.regions)) { drm_dbg(&i915->drm, "num_regions is too large\n"); ret = -EINVAL; @@ -253,21 +273,13 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, if (ret) return ret;
- placements = kmalloc_array(args->num_regions, - sizeof(struct intel_memory_region *), - GFP_KERNEL); - if (!placements) - return -ENOMEM; - mask = 0; for (i = 0; i < args->num_regions; i++) { struct drm_i915_gem_memory_class_instance region; struct intel_memory_region *mr;
- if (copy_from_user(®ion, uregions, sizeof(region))) { - ret = -EFAULT; - goto out_free; - } + if (copy_from_user(®ion, uregions, sizeof(region))) + return -EFAULT;
mr = intel_memory_region_lookup(i915, region.memory_class, @@ -293,14 +305,13 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, ++uregions; }
- if (obj->mm.placements) { + if (ext_data->n_placements) { ret = -EINVAL; goto out_dump; }
- object_set_placements(obj, placements, args->num_regions); - if (args->num_regions == 1) - kfree(placements); + for (i = 0; i < args->num_regions; i++) + ext_data->placements[i] = placements[i];
return 0;
@@ -308,11 +319,11 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, if (1) { char buf[256];
- if (obj->mm.placements) { + if (ext_data->n_placements) { repr_placements(buf, sizeof(buf), - obj->mm.placements, - obj->mm.n_placements); + ext_data->placements, + ext_data->n_placements); drm_dbg(&i915->drm, "Placements were already set in previous EXT. Existing placements: %s\n", buf); @@ -322,8 +333,6 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, drm_dbg(&i915->drm, "New placements(so far validated): %s\n", buf); }
-out_free: - kfree(placements); return ret; }
@@ -358,7 +367,6 @@ i915_gem_create_ext_ioctl(struct drm_device *dev, void *data, struct drm_i915_private *i915 = to_i915(dev); struct drm_i915_gem_create_ext *args = data; struct create_ext ext_data = { .i915 = i915 }; - struct intel_memory_region **placements_ext; struct drm_i915_gem_object *obj; int ret;
@@ -371,21 +379,22 @@ i915_gem_create_ext_ioctl(struct drm_device *dev, void *data, if (!obj) return -ENOMEM;
- ext_data.vanilla_object = obj; ret = i915_user_extensions(u64_to_user_ptr(args->extensions), create_extensions, ARRAY_SIZE(create_extensions), &ext_data); - placements_ext = obj->mm.placements; if (ret) goto object_free;
- if (!placements_ext) { - struct intel_memory_region *mr = + if (!ext_data.n_placements) { + ext_data.placements[0] = intel_memory_region_by_type(i915, INTEL_MEMORY_SYSTEM); - - object_set_placements(obj, &mr, 1); + 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) @@ -395,7 +404,7 @@ i915_gem_create_ext_ioctl(struct drm_device *dev, void *data,
object_free: if (obj->mm.n_placements > 1) - kfree(placements_ext); + kfree(obj->mm.placements); i915_gem_object_free(obj); return ret; }
On Fri, 16 Jul 2021 at 15:14, Jason Ekstrand jason@jlekstrand.net wrote:
Since we don't allow changing the set of regions after creation, we can make ext_set_placements() build up the region set directly in the create_ext and assign it to the object later. This is similar to what we did for contexts with the proto-context only simpler because there's no funny object shuffling. This will be used in the next patch to allow us to de-duplicate a bunch of code. Also, since we know the maximum number of regions up-front, we can use a fixed-size temporary array for the regions. This simplifies memory management a bit for this new delayed approach.
v2 (Matthew Auld):
- Get rid of MAX_N_PLACEMENTS
- Drop kfree(placements) from set_placements()
Signed-off-by: Jason Ekstrand jason@jlekstrand.net Cc: Matthew Auld matthew.auld@intel.com
If CI is happy, Reviewed-by: Matthew Auld matthew.auld@intel.com
On Fri, 16 Jul 2021 at 15:14, Jason Ekstrand jason@jlekstrand.net wrote:
Since we don't allow changing the set of regions after creation, we can make ext_set_placements() build up the region set directly in the create_ext and assign it to the object later. This is similar to what we did for contexts with the proto-context only simpler because there's no funny object shuffling. This will be used in the next patch to allow us to de-duplicate a bunch of code. Also, since we know the maximum number of regions up-front, we can use a fixed-size temporary array for the regions. This simplifies memory management a bit for this new delayed approach.
v2 (Matthew Auld):
- Get rid of MAX_N_PLACEMENTS
- Drop kfree(placements) from set_placements()
Signed-off-by: Jason Ekstrand jason@jlekstrand.net Cc: Matthew Auld matthew.auld@intel.com
drivers/gpu/drm/i915/gem/i915_gem_create.c | 81 ++++++++++++---------- 1 file changed, 45 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c index 51f92e4b1a69d..5766749a449c0 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c @@ -27,10 +27,13 @@ static u32 object_max_page_size(struct drm_i915_gem_object *obj) return max_page_size; }
-static void object_set_placements(struct drm_i915_gem_object *obj,
struct intel_memory_region **placements,unsigned int n_placements)+static int object_set_placements(struct drm_i915_gem_object *obj,
struct intel_memory_region **placements,unsigned int n_placements){
struct intel_memory_region **arr;unsigned int i;GEM_BUG_ON(!n_placements); /*@@ -44,9 +47,20 @@ static void object_set_placements(struct drm_i915_gem_object *obj, obj->mm.placements = &i915->mm.regions[mr->id]; obj->mm.n_placements = 1; } else {
obj->mm.placements = placements;
arr = kmalloc_array(n_placements,sizeof(struct intel_memory_region *),GFP_KERNEL);if (!arr)return -ENOMEM;for (i = 0; i < n_placements; i++)arr[i] = placements[i];obj->mm.placements = arr; obj->mm.n_placements = n_placements; }return 0;}
static int i915_gem_publish(struct drm_i915_gem_object *obj, @@ -148,7 +162,9 @@ i915_gem_dumb_create(struct drm_file *file, return -ENOMEM;
mr = intel_memory_region_by_type(to_i915(dev), mem_type);
object_set_placements(obj, &mr, 1);
ret = object_set_placements(obj, &mr, 1);if (ret)goto object_free; ret = i915_gem_setup(obj, args->size); if (ret)@@ -184,7 +200,9 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data, return -ENOMEM;
mr = intel_memory_region_by_type(i915, INTEL_MEMORY_SYSTEM);
object_set_placements(obj, &mr, 1);
ret = object_set_placements(obj, &mr, 1);if (ret)goto object_free; ret = i915_gem_setup(obj, args->size); if (ret)@@ -199,7 +217,8 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data,
struct create_ext { struct drm_i915_private *i915;
struct drm_i915_gem_object *vanilla_object;
struct intel_memory_region *placements[INTEL_REGION_UNKNOWN];unsigned int n_placements;};
static void repr_placements(char *buf, size_t size, @@ -230,8 +249,7 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, struct drm_i915_private *i915 = ext_data->i915; struct drm_i915_gem_memory_class_instance __user *uregions = u64_to_user_ptr(args->regions);
struct drm_i915_gem_object *obj = ext_data->vanilla_object;struct intel_memory_region **placements;
struct intel_memory_region *placements[INTEL_REGION_UNKNOWN]; u32 mask; int i, ret = 0;@@ -245,6 +263,8 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, ret = -EINVAL; }
BUILD_BUG_ON(ARRAY_SIZE(i915->mm.regions) != ARRAY_SIZE(placements));BUILD_BUG_ON(ARRAY_SIZE(ext_data->placements) != ARRAY_SIZE(placements)); if (args->num_regions > ARRAY_SIZE(i915->mm.regions)) { drm_dbg(&i915->drm, "num_regions is too large\n"); ret = -EINVAL;@@ -253,21 +273,13 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, if (ret) return ret;
placements = kmalloc_array(args->num_regions,sizeof(struct intel_memory_region *),GFP_KERNEL);if (!placements)return -ENOMEM;mask = 0; for (i = 0; i < args->num_regions; i++) { struct drm_i915_gem_memory_class_instance region; struct intel_memory_region *mr;if (copy_from_user(®ion, uregions, sizeof(region))) {ret = -EFAULT;goto out_free;}
if (copy_from_user(®ion, uregions, sizeof(region)))return -EFAULT; mr = intel_memory_region_lookup(i915, region.memory_class,@@ -293,14 +305,13 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, ++uregions; }
if (obj->mm.placements) {
if (ext_data->n_placements) { ret = -EINVAL; goto out_dump; }
object_set_placements(obj, placements, args->num_regions);if (args->num_regions == 1)kfree(placements);
for (i = 0; i < args->num_regions; i++)ext_data->placements[i] = placements[i];
I guess here we forget to set the ext_data->n_placements, which would explain the CI failure.
On Mon, Jul 19, 2021 at 3:18 AM Matthew Auld matthew.william.auld@gmail.com wrote:
On Fri, 16 Jul 2021 at 15:14, Jason Ekstrand jason@jlekstrand.net wrote:
Since we don't allow changing the set of regions after creation, we can make ext_set_placements() build up the region set directly in the create_ext and assign it to the object later. This is similar to what we did for contexts with the proto-context only simpler because there's no funny object shuffling. This will be used in the next patch to allow us to de-duplicate a bunch of code. Also, since we know the maximum number of regions up-front, we can use a fixed-size temporary array for the regions. This simplifies memory management a bit for this new delayed approach.
v2 (Matthew Auld):
- Get rid of MAX_N_PLACEMENTS
- Drop kfree(placements) from set_placements()
Signed-off-by: Jason Ekstrand jason@jlekstrand.net Cc: Matthew Auld matthew.auld@intel.com
drivers/gpu/drm/i915/gem/i915_gem_create.c | 81 ++++++++++++---------- 1 file changed, 45 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c index 51f92e4b1a69d..5766749a449c0 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c @@ -27,10 +27,13 @@ static u32 object_max_page_size(struct drm_i915_gem_object *obj) return max_page_size; }
-static void object_set_placements(struct drm_i915_gem_object *obj,
struct intel_memory_region **placements,unsigned int n_placements)+static int object_set_placements(struct drm_i915_gem_object *obj,
struct intel_memory_region **placements,unsigned int n_placements){
struct intel_memory_region **arr;unsigned int i;GEM_BUG_ON(!n_placements); /*@@ -44,9 +47,20 @@ static void object_set_placements(struct drm_i915_gem_object *obj, obj->mm.placements = &i915->mm.regions[mr->id]; obj->mm.n_placements = 1; } else {
obj->mm.placements = placements;
arr = kmalloc_array(n_placements,sizeof(struct intel_memory_region *),GFP_KERNEL);if (!arr)return -ENOMEM;for (i = 0; i < n_placements; i++)arr[i] = placements[i];obj->mm.placements = arr; obj->mm.n_placements = n_placements; }return 0;}
static int i915_gem_publish(struct drm_i915_gem_object *obj, @@ -148,7 +162,9 @@ i915_gem_dumb_create(struct drm_file *file, return -ENOMEM;
mr = intel_memory_region_by_type(to_i915(dev), mem_type);
object_set_placements(obj, &mr, 1);
ret = object_set_placements(obj, &mr, 1);if (ret)goto object_free; ret = i915_gem_setup(obj, args->size); if (ret)@@ -184,7 +200,9 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data, return -ENOMEM;
mr = intel_memory_region_by_type(i915, INTEL_MEMORY_SYSTEM);
object_set_placements(obj, &mr, 1);
ret = object_set_placements(obj, &mr, 1);if (ret)goto object_free; ret = i915_gem_setup(obj, args->size); if (ret)@@ -199,7 +217,8 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data,
struct create_ext { struct drm_i915_private *i915;
struct drm_i915_gem_object *vanilla_object;
struct intel_memory_region *placements[INTEL_REGION_UNKNOWN];unsigned int n_placements;};
static void repr_placements(char *buf, size_t size, @@ -230,8 +249,7 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, struct drm_i915_private *i915 = ext_data->i915; struct drm_i915_gem_memory_class_instance __user *uregions = u64_to_user_ptr(args->regions);
struct drm_i915_gem_object *obj = ext_data->vanilla_object;struct intel_memory_region **placements;
struct intel_memory_region *placements[INTEL_REGION_UNKNOWN]; u32 mask; int i, ret = 0;@@ -245,6 +263,8 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, ret = -EINVAL; }
BUILD_BUG_ON(ARRAY_SIZE(i915->mm.regions) != ARRAY_SIZE(placements));BUILD_BUG_ON(ARRAY_SIZE(ext_data->placements) != ARRAY_SIZE(placements)); if (args->num_regions > ARRAY_SIZE(i915->mm.regions)) { drm_dbg(&i915->drm, "num_regions is too large\n"); ret = -EINVAL;@@ -253,21 +273,13 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, if (ret) return ret;
placements = kmalloc_array(args->num_regions,sizeof(struct intel_memory_region *),GFP_KERNEL);if (!placements)return -ENOMEM;mask = 0; for (i = 0; i < args->num_regions; i++) { struct drm_i915_gem_memory_class_instance region; struct intel_memory_region *mr;if (copy_from_user(®ion, uregions, sizeof(region))) {ret = -EFAULT;goto out_free;}
if (copy_from_user(®ion, uregions, sizeof(region)))return -EFAULT; mr = intel_memory_region_lookup(i915, region.memory_class,@@ -293,14 +305,13 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, ++uregions; }
if (obj->mm.placements) {
if (ext_data->n_placements) { ret = -EINVAL; goto out_dump; }
object_set_placements(obj, placements, args->num_regions);if (args->num_regions == 1)kfree(placements);
for (i = 0; i < args->num_regions; i++)ext_data->placements[i] = placements[i];I guess here we forget to set the ext_data->n_placements, which would explain the CI failure.
What CI failure are you referring to?
On Tue, 20 Jul 2021 at 23:07, Jason Ekstrand jason@jlekstrand.net wrote:
On Mon, Jul 19, 2021 at 3:18 AM Matthew Auld matthew.william.auld@gmail.com wrote:
On Fri, 16 Jul 2021 at 15:14, Jason Ekstrand jason@jlekstrand.net wrote:
Since we don't allow changing the set of regions after creation, we can make ext_set_placements() build up the region set directly in the create_ext and assign it to the object later. This is similar to what we did for contexts with the proto-context only simpler because there's no funny object shuffling. This will be used in the next patch to allow us to de-duplicate a bunch of code. Also, since we know the maximum number of regions up-front, we can use a fixed-size temporary array for the regions. This simplifies memory management a bit for this new delayed approach.
v2 (Matthew Auld):
- Get rid of MAX_N_PLACEMENTS
- Drop kfree(placements) from set_placements()
Signed-off-by: Jason Ekstrand jason@jlekstrand.net Cc: Matthew Auld matthew.auld@intel.com
drivers/gpu/drm/i915/gem/i915_gem_create.c | 81 ++++++++++++---------- 1 file changed, 45 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c index 51f92e4b1a69d..5766749a449c0 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c @@ -27,10 +27,13 @@ static u32 object_max_page_size(struct drm_i915_gem_object *obj) return max_page_size; }
-static void object_set_placements(struct drm_i915_gem_object *obj,
struct intel_memory_region **placements,unsigned int n_placements)+static int object_set_placements(struct drm_i915_gem_object *obj,
struct intel_memory_region **placements,unsigned int n_placements){
struct intel_memory_region **arr;unsigned int i;GEM_BUG_ON(!n_placements); /*@@ -44,9 +47,20 @@ static void object_set_placements(struct drm_i915_gem_object *obj, obj->mm.placements = &i915->mm.regions[mr->id]; obj->mm.n_placements = 1; } else {
obj->mm.placements = placements;
arr = kmalloc_array(n_placements,sizeof(struct intel_memory_region *),GFP_KERNEL);if (!arr)return -ENOMEM;for (i = 0; i < n_placements; i++)arr[i] = placements[i];obj->mm.placements = arr; obj->mm.n_placements = n_placements; }return 0;}
static int i915_gem_publish(struct drm_i915_gem_object *obj, @@ -148,7 +162,9 @@ i915_gem_dumb_create(struct drm_file *file, return -ENOMEM;
mr = intel_memory_region_by_type(to_i915(dev), mem_type);
object_set_placements(obj, &mr, 1);
ret = object_set_placements(obj, &mr, 1);if (ret)goto object_free; ret = i915_gem_setup(obj, args->size); if (ret)@@ -184,7 +200,9 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data, return -ENOMEM;
mr = intel_memory_region_by_type(i915, INTEL_MEMORY_SYSTEM);
object_set_placements(obj, &mr, 1);
ret = object_set_placements(obj, &mr, 1);if (ret)goto object_free; ret = i915_gem_setup(obj, args->size); if (ret)@@ -199,7 +217,8 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data,
struct create_ext { struct drm_i915_private *i915;
struct drm_i915_gem_object *vanilla_object;
struct intel_memory_region *placements[INTEL_REGION_UNKNOWN];unsigned int n_placements;};
static void repr_placements(char *buf, size_t size, @@ -230,8 +249,7 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, struct drm_i915_private *i915 = ext_data->i915; struct drm_i915_gem_memory_class_instance __user *uregions = u64_to_user_ptr(args->regions);
struct drm_i915_gem_object *obj = ext_data->vanilla_object;struct intel_memory_region **placements;
struct intel_memory_region *placements[INTEL_REGION_UNKNOWN]; u32 mask; int i, ret = 0;@@ -245,6 +263,8 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, ret = -EINVAL; }
BUILD_BUG_ON(ARRAY_SIZE(i915->mm.regions) != ARRAY_SIZE(placements));BUILD_BUG_ON(ARRAY_SIZE(ext_data->placements) != ARRAY_SIZE(placements)); if (args->num_regions > ARRAY_SIZE(i915->mm.regions)) { drm_dbg(&i915->drm, "num_regions is too large\n"); ret = -EINVAL;@@ -253,21 +273,13 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, if (ret) return ret;
placements = kmalloc_array(args->num_regions,sizeof(struct intel_memory_region *),GFP_KERNEL);if (!placements)return -ENOMEM;mask = 0; for (i = 0; i < args->num_regions; i++) { struct drm_i915_gem_memory_class_instance region; struct intel_memory_region *mr;if (copy_from_user(®ion, uregions, sizeof(region))) {ret = -EFAULT;goto out_free;}
if (copy_from_user(®ion, uregions, sizeof(region)))return -EFAULT; mr = intel_memory_region_lookup(i915, region.memory_class,@@ -293,14 +305,13 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, ++uregions; }
if (obj->mm.placements) {
if (ext_data->n_placements) { ret = -EINVAL; goto out_dump; }
object_set_placements(obj, placements, args->num_regions);if (args->num_regions == 1)kfree(placements);
for (i = 0; i < args->num_regions; i++)ext_data->placements[i] = placements[i];I guess here we forget to set the ext_data->n_placements, which would explain the CI failure.
What CI failure are you referring to?
Pre-merge results for this series:
igt@gem_create@create-ext-placement-sanity-check:
shard-skl: PASS -> FAIL +1 similar issue shard-apl: NOTRUN -> FAIL shard-glk: PASS -> FAIL shard-iclb: PASS -> FAIL shard-kbl: PASS -> FAIL shard-tglb: NOTRUN -> FAIL
On Wed, Jul 21, 2021 at 3:32 AM Matthew Auld matthew.william.auld@gmail.com wrote:
On Tue, 20 Jul 2021 at 23:07, Jason Ekstrand jason@jlekstrand.net wrote:
On Mon, Jul 19, 2021 at 3:18 AM Matthew Auld matthew.william.auld@gmail.com wrote:
On Fri, 16 Jul 2021 at 15:14, Jason Ekstrand jason@jlekstrand.net wrote:
Since we don't allow changing the set of regions after creation, we can make ext_set_placements() build up the region set directly in the create_ext and assign it to the object later. This is similar to what we did for contexts with the proto-context only simpler because there's no funny object shuffling. This will be used in the next patch to allow us to de-duplicate a bunch of code. Also, since we know the maximum number of regions up-front, we can use a fixed-size temporary array for the regions. This simplifies memory management a bit for this new delayed approach.
v2 (Matthew Auld):
- Get rid of MAX_N_PLACEMENTS
- Drop kfree(placements) from set_placements()
Signed-off-by: Jason Ekstrand jason@jlekstrand.net Cc: Matthew Auld matthew.auld@intel.com
drivers/gpu/drm/i915/gem/i915_gem_create.c | 81 ++++++++++++---------- 1 file changed, 45 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c index 51f92e4b1a69d..5766749a449c0 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c @@ -27,10 +27,13 @@ static u32 object_max_page_size(struct drm_i915_gem_object *obj) return max_page_size; }
-static void object_set_placements(struct drm_i915_gem_object *obj,
struct intel_memory_region **placements,unsigned int n_placements)+static int object_set_placements(struct drm_i915_gem_object *obj,
struct intel_memory_region **placements,unsigned int n_placements){
struct intel_memory_region **arr;unsigned int i;GEM_BUG_ON(!n_placements); /*@@ -44,9 +47,20 @@ static void object_set_placements(struct drm_i915_gem_object *obj, obj->mm.placements = &i915->mm.regions[mr->id]; obj->mm.n_placements = 1; } else {
obj->mm.placements = placements;
arr = kmalloc_array(n_placements,sizeof(struct intel_memory_region *),GFP_KERNEL);if (!arr)return -ENOMEM;for (i = 0; i < n_placements; i++)arr[i] = placements[i];obj->mm.placements = arr; obj->mm.n_placements = n_placements; }return 0;}
static int i915_gem_publish(struct drm_i915_gem_object *obj, @@ -148,7 +162,9 @@ i915_gem_dumb_create(struct drm_file *file, return -ENOMEM;
mr = intel_memory_region_by_type(to_i915(dev), mem_type);
object_set_placements(obj, &mr, 1);
ret = object_set_placements(obj, &mr, 1);if (ret)goto object_free; ret = i915_gem_setup(obj, args->size); if (ret)@@ -184,7 +200,9 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data, return -ENOMEM;
mr = intel_memory_region_by_type(i915, INTEL_MEMORY_SYSTEM);
object_set_placements(obj, &mr, 1);
ret = object_set_placements(obj, &mr, 1);if (ret)goto object_free; ret = i915_gem_setup(obj, args->size); if (ret)@@ -199,7 +217,8 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data,
struct create_ext { struct drm_i915_private *i915;
struct drm_i915_gem_object *vanilla_object;
struct intel_memory_region *placements[INTEL_REGION_UNKNOWN];unsigned int n_placements;};
static void repr_placements(char *buf, size_t size, @@ -230,8 +249,7 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, struct drm_i915_private *i915 = ext_data->i915; struct drm_i915_gem_memory_class_instance __user *uregions = u64_to_user_ptr(args->regions);
struct drm_i915_gem_object *obj = ext_data->vanilla_object;struct intel_memory_region **placements;
struct intel_memory_region *placements[INTEL_REGION_UNKNOWN]; u32 mask; int i, ret = 0;@@ -245,6 +263,8 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, ret = -EINVAL; }
BUILD_BUG_ON(ARRAY_SIZE(i915->mm.regions) != ARRAY_SIZE(placements));BUILD_BUG_ON(ARRAY_SIZE(ext_data->placements) != ARRAY_SIZE(placements)); if (args->num_regions > ARRAY_SIZE(i915->mm.regions)) { drm_dbg(&i915->drm, "num_regions is too large\n"); ret = -EINVAL;@@ -253,21 +273,13 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, if (ret) return ret;
placements = kmalloc_array(args->num_regions,sizeof(struct intel_memory_region *),GFP_KERNEL);if (!placements)return -ENOMEM;mask = 0; for (i = 0; i < args->num_regions; i++) { struct drm_i915_gem_memory_class_instance region; struct intel_memory_region *mr;if (copy_from_user(®ion, uregions, sizeof(region))) {ret = -EFAULT;goto out_free;}
if (copy_from_user(®ion, uregions, sizeof(region)))return -EFAULT; mr = intel_memory_region_lookup(i915, region.memory_class,@@ -293,14 +305,13 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, ++uregions; }
if (obj->mm.placements) {
if (ext_data->n_placements) { ret = -EINVAL; goto out_dump; }
object_set_placements(obj, placements, args->num_regions);if (args->num_regions == 1)kfree(placements);
for (i = 0; i < args->num_regions; i++)ext_data->placements[i] = placements[i];I guess here we forget to set the ext_data->n_placements, which would explain the CI failure.
What CI failure are you referring to?
Pre-merge results for this series:
igt@gem_create@create-ext-placement-sanity-check:
shard-skl: PASS -> FAIL +1 similar issue shard-apl: NOTRUN -> FAIL shard-glk: PASS -> FAIL shard-iclb: PASS -> FAIL shard-kbl: PASS -> FAIL shard-tglb: NOTRUN -> FAIL
Yup. That was it. Thanks! Not sure why I didn't notice those fails....
--Jason
This doesn't really fix anything serious since the chances of a client creating and destroying a mass of dumb BOs is pretty low. However, it is called by the other two create IOCTLs to garbage collect old objects. Call it here too for consistency.
Signed-off-by: Jason Ekstrand jason@jlekstrand.net Cc: Matthew Auld matthew.auld@intel.com --- drivers/gpu/drm/i915/gem/i915_gem_create.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c index 5766749a449c0..1b370914587c0 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c @@ -151,6 +151,8 @@ 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;
On Fri, 16 Jul 2021 at 15:14, Jason Ekstrand jason@jlekstrand.net wrote:
This doesn't really fix anything serious since the chances of a client creating and destroying a mass of dumb BOs is pretty low. However, it is called by the other two create IOCTLs to garbage collect old objects. Call it here too for consistency.
Signed-off-by: Jason Ekstrand jason@jlekstrand.net Cc: Matthew Auld matthew.auld@intel.com
Reviewed-by: Matthew Auld matthew.auld@intel.com
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
Signed-off-by: Jason Ekstrand jason@jlekstrand.net Cc: Matthew Auld matthew.auld@intel.com --- drivers/gpu/drm/i915/gem/i915_gem_create.c | 108 ++++++++------------- 1 file changed, 43 insertions(+), 65 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c index 1b370914587c0..039e4f3b39c79 100644 --- 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,35 @@ 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) +static 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 +120,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 +144,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 +170,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 +199,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 { @@ -375,38 +367,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; }
On Fri, 16 Jul 2021 at 15:14, Jason Ekstrand jason@jlekstrand.net wrote:
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
Signed-off-by: Jason Ekstrand jason@jlekstrand.net Cc: Matthew Auld matthew.auld@intel.com
If CI is happy, Reviewed-by: Matthew Auld matthew.auld@intel.com
On Fri, 16 Jul 2021 at 20:21, Matthew Auld matthew.william.auld@gmail.com wrote:
On Fri, 16 Jul 2021 at 15:14, Jason Ekstrand jason@jlekstrand.net wrote:
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
Signed-off-by: Jason Ekstrand jason@jlekstrand.net Cc: Matthew Auld matthew.auld@intel.com
If CI is happy, Reviewed-by: Matthew Auld matthew.auld@intel.com
Might be good to also update the mman selftests to use this new helper.
Whenever we had a user object (n_placements > 0), we were ignoring obj->mm.region and always putting obj->placements[0] as the requested region. For LMEM+SMEM objects, this was causing them to get shoved into LMEM on every i915_ttm_get_pages() even when SMEM was requested by, say, i915_gem_object_migrate().
Signed-off-by: Jason Ekstrand jason@jlekstrand.net Cc: Thomas Hellström thomas.hellstrom@linux.intel.com Cc: Matthew Auld matthew.auld@intel.com Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com --- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index 6589411396d3f..8eeb73c7c401c 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -150,8 +150,7 @@ i915_ttm_placement_from_obj(const struct drm_i915_gem_object *obj, unsigned int i;
placement->num_placement = 1; - i915_ttm_place_from_region(num_allowed ? obj->mm.placements[0] : - obj->mm.region, requested, flags); + i915_ttm_place_from_region(obj->mm.region, requested, flags);
/* Cache this on object? */ placement->num_busy_placement = num_allowed;
On Fri, 16 Jul 2021 at 15:14, Jason Ekstrand jason@jlekstrand.net wrote:
Whenever we had a user object (n_placements > 0), we were ignoring obj->mm.region and always putting obj->placements[0] as the requested region. For LMEM+SMEM objects, this was causing them to get shoved into LMEM on every i915_ttm_get_pages() even when SMEM was requested by, say, i915_gem_object_migrate().
Signed-off-by: Jason Ekstrand jason@jlekstrand.net Cc: Thomas Hellström thomas.hellstrom@linux.intel.com Cc: Matthew Auld matthew.auld@intel.com Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com
AFAIK makes sense, just a question of properly understanding that weird migration issue first.
Assuming CI is happy, Reviewed-by: Matthew Auld matthew.auld@intel.com
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index 6589411396d3f..8eeb73c7c401c 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -150,8 +150,7 @@ i915_ttm_placement_from_obj(const struct drm_i915_gem_object *obj, unsigned int i;
placement->num_placement = 1;
i915_ttm_place_from_region(num_allowed ? obj->mm.placements[0] :obj->mm.region, requested, flags);
i915_ttm_place_from_region(obj->mm.region, requested, flags); /* Cache this on object? */ placement->num_busy_placement = num_allowed;-- 2.31.1
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
From: Thomas Hellström thomas.hellstrom@linux.intel.com
If our exported dma-bufs are imported by another instance of our driver, that instance will typically have the imported dma-bufs locked during dma_buf_map_attachment(). But the exporter also locks the same reservation object in the map_dma_buf() callback, which leads to recursive locking.
So taking the lock inside _pin_pages_unlocked() is incorrect.
Additionally, the current pinning code path is contrary to the defined way that pinning should occur.
Remove the explicit pin/unpin from the map/umap functions and move them to the attach/detach allowing correct locking to occur, and to match the static dma-buf drm_prime pattern.
Add a live selftest to exercise both dynamic and non-dynamic exports.
v2: - Extend the selftest with a fake dynamic importer. - Provide real pin and unpin callbacks to not abuse the interface. v3: (ruhl) - Remove the dynamic export support and move the pinning into the attach/detach path. v4: (ruhl) - Put pages does not need to assert on the dma-resv v5: (jason) - Lock around dma_buf_unmap_attachment() when emulating a dynamic importer in the subtests. - Use pin_pages_unlocked v6: (jason) - Use dma_buf_attach instead of dma_buf_attach_dynamic in the selftests
Reported-by: Michael J. Ruhl michael.j.ruhl@intel.com Signed-off-by: Thomas Hellström thomas.hellstrom@linux.intel.com Signed-off-by: Michael J. Ruhl michael.j.ruhl@intel.com Signed-off-by: Jason Ekstrand jason@jlekstrand.net Reviewed-by: Jason Ekstrand jason@jlekstrand.net --- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 43 ++++++-- .../drm/i915/gem/selftests/i915_gem_dmabuf.c | 103 +++++++++++++++++- 2 files changed, 132 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c index 616c3a2f1baf0..9a655f69a0671 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c @@ -12,6 +12,8 @@ #include "i915_gem_object.h" #include "i915_scatterlist.h"
+I915_SELFTEST_DECLARE(static bool force_different_devices;) + static struct drm_i915_gem_object *dma_buf_to_obj(struct dma_buf *buf) { return to_intel_bo(buf->priv); @@ -25,15 +27,11 @@ static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachme struct scatterlist *src, *dst; int ret, i;
- ret = i915_gem_object_pin_pages_unlocked(obj); - if (ret) - goto err; - /* Copy sg so that we make an independent mapping */ st = kmalloc(sizeof(struct sg_table), GFP_KERNEL); if (st == NULL) { ret = -ENOMEM; - goto err_unpin_pages; + goto err; }
ret = sg_alloc_table(st, obj->mm.pages->nents, GFP_KERNEL); @@ -58,8 +56,6 @@ static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachme sg_free_table(st); err_free: kfree(st); -err_unpin_pages: - i915_gem_object_unpin_pages(obj); err: return ERR_PTR(ret); } @@ -68,13 +64,9 @@ static void i915_gem_unmap_dma_buf(struct dma_buf_attachment *attachment, struct sg_table *sg, enum dma_data_direction dir) { - struct drm_i915_gem_object *obj = dma_buf_to_obj(attachment->dmabuf); - dma_unmap_sgtable(attachment->dev, sg, dir, DMA_ATTR_SKIP_CPU_SYNC); sg_free_table(sg); kfree(sg); - - i915_gem_object_unpin_pages(obj); }
static int i915_gem_dmabuf_vmap(struct dma_buf *dma_buf, struct dma_buf_map *map) @@ -168,7 +160,31 @@ static int i915_gem_end_cpu_access(struct dma_buf *dma_buf, enum dma_data_direct return err; }
+/** + * i915_gem_dmabuf_attach - Do any extra attach work necessary + * @dmabuf: imported dma-buf + * @attach: new attach to do work on + * + */ +static int i915_gem_dmabuf_attach(struct dma_buf *dmabuf, + struct dma_buf_attachment *attach) +{ + struct drm_i915_gem_object *obj = dma_buf_to_obj(dmabuf); + + return i915_gem_object_pin_pages_unlocked(obj); +} + +static void i915_gem_dmabuf_detach(struct dma_buf *dmabuf, + struct dma_buf_attachment *attach) +{ + struct drm_i915_gem_object *obj = dma_buf_to_obj(dmabuf); + + i915_gem_object_unpin_pages(obj); +} + static const struct dma_buf_ops i915_dmabuf_ops = { + .attach = i915_gem_dmabuf_attach, + .detach = i915_gem_dmabuf_detach, .map_dma_buf = i915_gem_map_dma_buf, .unmap_dma_buf = i915_gem_unmap_dma_buf, .release = drm_gem_dmabuf_release, @@ -204,6 +220,8 @@ static int i915_gem_object_get_pages_dmabuf(struct drm_i915_gem_object *obj) struct sg_table *pages; unsigned int sg_page_sizes;
+ assert_object_held(obj); + pages = dma_buf_map_attachment(obj->base.import_attach, DMA_BIDIRECTIONAL); if (IS_ERR(pages)) @@ -241,7 +259,8 @@ struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev, if (dma_buf->ops == &i915_dmabuf_ops) { obj = dma_buf_to_obj(dma_buf); /* is it from our device? */ - if (obj->base.dev == dev) { + if (obj->base.dev == dev && + !I915_SELFTEST_ONLY(force_different_devices)) { /* * Importing dmabuf exported from out own gem increases * refcount on gem itself instead of f_count of dmabuf. diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c index dd74bc09ec88d..4451bbb4917e4 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c @@ -35,7 +35,7 @@ static int igt_dmabuf_export(void *arg) static int igt_dmabuf_import_self(void *arg) { struct drm_i915_private *i915 = arg; - struct drm_i915_gem_object *obj; + struct drm_i915_gem_object *obj, *import_obj; struct drm_gem_object *import; struct dma_buf *dmabuf; int err; @@ -65,14 +65,112 @@ static int igt_dmabuf_import_self(void *arg) err = -EINVAL; goto out_import; } + import_obj = to_intel_bo(import); + + i915_gem_object_lock(import_obj, NULL); + err = ____i915_gem_object_get_pages(import_obj); + i915_gem_object_unlock(import_obj); + if (err) { + pr_err("Same object dma-buf get_pages failed!\n"); + goto out_import; + }
err = 0; out_import: - i915_gem_object_put(to_intel_bo(import)); + i915_gem_object_put(import_obj); +out_dmabuf: + dma_buf_put(dmabuf); +out: + i915_gem_object_put(obj); + return err; +} + +static int igt_dmabuf_import_same_driver(void *arg) +{ + struct drm_i915_private *i915 = arg; + struct drm_i915_gem_object *obj, *import_obj; + struct drm_gem_object *import; + struct dma_buf *dmabuf; + struct dma_buf_attachment *import_attach; + struct sg_table *st; + long timeout; + int err; + + force_different_devices = true; + obj = i915_gem_object_create_shmem(i915, PAGE_SIZE); + if (IS_ERR(obj)) + goto out_ret; + + dmabuf = i915_gem_prime_export(&obj->base, 0); + if (IS_ERR(dmabuf)) { + pr_err("i915_gem_prime_export failed with err=%d\n", + (int)PTR_ERR(dmabuf)); + err = PTR_ERR(dmabuf); + goto out; + } + + import = i915_gem_prime_import(&i915->drm, dmabuf); + if (IS_ERR(import)) { + pr_err("i915_gem_prime_import failed with err=%d\n", + (int)PTR_ERR(import)); + err = PTR_ERR(import); + goto out_dmabuf; + } + + if (import == &obj->base) { + pr_err("i915_gem_prime_import reused gem object!\n"); + err = -EINVAL; + goto out_import; + } + + import_obj = to_intel_bo(import); + + i915_gem_object_lock(import_obj, NULL); + err = ____i915_gem_object_get_pages(import_obj); + if (err) { + pr_err("Different objects dma-buf get_pages failed!\n"); + i915_gem_object_unlock(import_obj); + goto out_import; + } + + /* + * If the exported object is not in system memory, something + * weird is going on. TODO: When p2p is supported, this is no + * longer considered weird. + */ + if (obj->mm.region != i915->mm.regions[INTEL_REGION_SMEM]) { + pr_err("Exported dma-buf is not in system memory\n"); + err = -EINVAL; + } + + i915_gem_object_unlock(import_obj); + + /* Now try a fake an importer */ + import_attach = dma_buf_attach(dmabuf, obj->base.dev->dev); + if (IS_ERR(import_attach)) + goto out_import; + + st = dma_buf_map_attachment(import_attach, DMA_BIDIRECTIONAL); + if (IS_ERR(st)) + goto out_detach; + + timeout = dma_resv_wait_timeout(dmabuf->resv, false, true, 5 * HZ); + if (!timeout) { + pr_err("dmabuf wait for exclusive fence timed out.\n"); + timeout = -ETIME; + } + err = timeout > 0 ? 0 : timeout; + dma_buf_unmap_attachment(import_attach, st, DMA_BIDIRECTIONAL); +out_detach: + dma_buf_detach(dmabuf, import_attach); +out_import: + i915_gem_object_put(import_obj); out_dmabuf: dma_buf_put(dmabuf); out: i915_gem_object_put(obj); +out_ret: + force_different_devices = false; return err; }
@@ -286,6 +384,7 @@ int i915_gem_dmabuf_live_selftests(struct drm_i915_private *i915) { static const struct i915_subtest tests[] = { SUBTEST(igt_dmabuf_export), + SUBTEST(igt_dmabuf_import_same_driver), };
return i915_subtests(tests, i915);
On Fri, 16 Jul 2021 at 15:14, Jason Ekstrand jason@jlekstrand.net wrote:
From: Thomas Hellström thomas.hellstrom@linux.intel.com
If our exported dma-bufs are imported by another instance of our driver, that instance will typically have the imported dma-bufs locked during dma_buf_map_attachment(). But the exporter also locks the same reservation object in the map_dma_buf() callback, which leads to recursive locking.
So taking the lock inside _pin_pages_unlocked() is incorrect.
Additionally, the current pinning code path is contrary to the defined way that pinning should occur.
Remove the explicit pin/unpin from the map/umap functions and move them to the attach/detach allowing correct locking to occur, and to match the static dma-buf drm_prime pattern.
Add a live selftest to exercise both dynamic and non-dynamic exports.
v2:
- Extend the selftest with a fake dynamic importer.
- Provide real pin and unpin callbacks to not abuse the interface.
v3: (ruhl)
- Remove the dynamic export support and move the pinning into the attach/detach path.
v4: (ruhl)
- Put pages does not need to assert on the dma-resv
v5: (jason)
- Lock around dma_buf_unmap_attachment() when emulating a dynamic importer in the subtests.
- Use pin_pages_unlocked
v6: (jason)
- Use dma_buf_attach instead of dma_buf_attach_dynamic in the selftests
Reported-by: Michael J. Ruhl michael.j.ruhl@intel.com Signed-off-by: Thomas Hellström thomas.hellstrom@linux.intel.com Signed-off-by: Michael J. Ruhl michael.j.ruhl@intel.com Signed-off-by: Jason Ekstrand jason@jlekstrand.net Reviewed-by: Jason Ekstrand jason@jlekstrand.net
drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 43 ++++++-- .../drm/i915/gem/selftests/i915_gem_dmabuf.c | 103 +++++++++++++++++- 2 files changed, 132 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c index 616c3a2f1baf0..9a655f69a0671 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c @@ -12,6 +12,8 @@ #include "i915_gem_object.h" #include "i915_scatterlist.h"
+I915_SELFTEST_DECLARE(static bool force_different_devices;)
static struct drm_i915_gem_object *dma_buf_to_obj(struct dma_buf *buf) { return to_intel_bo(buf->priv); @@ -25,15 +27,11 @@ static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachme struct scatterlist *src, *dst; int ret, i;
ret = i915_gem_object_pin_pages_unlocked(obj);if (ret)goto err;/* Copy sg so that we make an independent mapping */ st = kmalloc(sizeof(struct sg_table), GFP_KERNEL); if (st == NULL) { ret = -ENOMEM;goto err_unpin_pages;
goto err; } ret = sg_alloc_table(st, obj->mm.pages->nents, GFP_KERNEL);@@ -58,8 +56,6 @@ static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachme sg_free_table(st); err_free: kfree(st); -err_unpin_pages:
i915_gem_object_unpin_pages(obj);err: return ERR_PTR(ret); } @@ -68,13 +64,9 @@ static void i915_gem_unmap_dma_buf(struct dma_buf_attachment *attachment, struct sg_table *sg, enum dma_data_direction dir) {
struct drm_i915_gem_object *obj = dma_buf_to_obj(attachment->dmabuf);dma_unmap_sgtable(attachment->dev, sg, dir, DMA_ATTR_SKIP_CPU_SYNC); sg_free_table(sg); kfree(sg);i915_gem_object_unpin_pages(obj);}
static int i915_gem_dmabuf_vmap(struct dma_buf *dma_buf, struct dma_buf_map *map) @@ -168,7 +160,31 @@ static int i915_gem_end_cpu_access(struct dma_buf *dma_buf, enum dma_data_direct return err; }
+/**
- i915_gem_dmabuf_attach - Do any extra attach work necessary
- @dmabuf: imported dma-buf
- @attach: new attach to do work on
- */
+static int i915_gem_dmabuf_attach(struct dma_buf *dmabuf,
struct dma_buf_attachment *attach)+{
struct drm_i915_gem_object *obj = dma_buf_to_obj(dmabuf);return i915_gem_object_pin_pages_unlocked(obj);+}
+static void i915_gem_dmabuf_detach(struct dma_buf *dmabuf,
struct dma_buf_attachment *attach)+{
struct drm_i915_gem_object *obj = dma_buf_to_obj(dmabuf);i915_gem_object_unpin_pages(obj);+}
We don't normally add kernel-doc for static functions? Otherwise dmabuf_detach() needs matching kernel-doc.
<snip>
+static int igt_dmabuf_import_same_driver(void *arg) +{
struct drm_i915_private *i915 = arg;struct drm_i915_gem_object *obj, *import_obj;struct drm_gem_object *import;struct dma_buf *dmabuf;struct dma_buf_attachment *import_attach;struct sg_table *st;long timeout;int err;force_different_devices = true;obj = i915_gem_object_create_shmem(i915, PAGE_SIZE);if (IS_ERR(obj))
err = PTR_ERR(obj)
<snip>
/* Now try a fake an importer */import_attach = dma_buf_attach(dmabuf, obj->base.dev->dev);if (IS_ERR(import_attach))goto out_import;st = dma_buf_map_attachment(import_attach, DMA_BIDIRECTIONAL);if (IS_ERR(st))goto out_detach;
For these two maybe missing err = ?
On Tue, Jul 20, 2021 at 4:07 AM Matthew Auld matthew.william.auld@gmail.com wrote:
On Fri, 16 Jul 2021 at 15:14, Jason Ekstrand jason@jlekstrand.net wrote:
From: Thomas Hellström thomas.hellstrom@linux.intel.com
If our exported dma-bufs are imported by another instance of our driver, that instance will typically have the imported dma-bufs locked during dma_buf_map_attachment(). But the exporter also locks the same reservation object in the map_dma_buf() callback, which leads to recursive locking.
So taking the lock inside _pin_pages_unlocked() is incorrect.
Additionally, the current pinning code path is contrary to the defined way that pinning should occur.
Remove the explicit pin/unpin from the map/umap functions and move them to the attach/detach allowing correct locking to occur, and to match the static dma-buf drm_prime pattern.
Add a live selftest to exercise both dynamic and non-dynamic exports.
v2:
- Extend the selftest with a fake dynamic importer.
- Provide real pin and unpin callbacks to not abuse the interface.
v3: (ruhl)
- Remove the dynamic export support and move the pinning into the attach/detach path.
v4: (ruhl)
- Put pages does not need to assert on the dma-resv
v5: (jason)
- Lock around dma_buf_unmap_attachment() when emulating a dynamic importer in the subtests.
- Use pin_pages_unlocked
v6: (jason)
- Use dma_buf_attach instead of dma_buf_attach_dynamic in the selftests
Reported-by: Michael J. Ruhl michael.j.ruhl@intel.com Signed-off-by: Thomas Hellström thomas.hellstrom@linux.intel.com Signed-off-by: Michael J. Ruhl michael.j.ruhl@intel.com Signed-off-by: Jason Ekstrand jason@jlekstrand.net Reviewed-by: Jason Ekstrand jason@jlekstrand.net
drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 43 ++++++-- .../drm/i915/gem/selftests/i915_gem_dmabuf.c | 103 +++++++++++++++++- 2 files changed, 132 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c index 616c3a2f1baf0..9a655f69a0671 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c @@ -12,6 +12,8 @@ #include "i915_gem_object.h" #include "i915_scatterlist.h"
+I915_SELFTEST_DECLARE(static bool force_different_devices;)
static struct drm_i915_gem_object *dma_buf_to_obj(struct dma_buf *buf) { return to_intel_bo(buf->priv); @@ -25,15 +27,11 @@ static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachme struct scatterlist *src, *dst; int ret, i;
ret = i915_gem_object_pin_pages_unlocked(obj);if (ret)goto err;/* Copy sg so that we make an independent mapping */ st = kmalloc(sizeof(struct sg_table), GFP_KERNEL); if (st == NULL) { ret = -ENOMEM;goto err_unpin_pages;
goto err; } ret = sg_alloc_table(st, obj->mm.pages->nents, GFP_KERNEL);@@ -58,8 +56,6 @@ static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachme sg_free_table(st); err_free: kfree(st); -err_unpin_pages:
i915_gem_object_unpin_pages(obj);err: return ERR_PTR(ret); } @@ -68,13 +64,9 @@ static void i915_gem_unmap_dma_buf(struct dma_buf_attachment *attachment, struct sg_table *sg, enum dma_data_direction dir) {
struct drm_i915_gem_object *obj = dma_buf_to_obj(attachment->dmabuf);dma_unmap_sgtable(attachment->dev, sg, dir, DMA_ATTR_SKIP_CPU_SYNC); sg_free_table(sg); kfree(sg);i915_gem_object_unpin_pages(obj);}
static int i915_gem_dmabuf_vmap(struct dma_buf *dma_buf, struct dma_buf_map *map) @@ -168,7 +160,31 @@ static int i915_gem_end_cpu_access(struct dma_buf *dma_buf, enum dma_data_direct return err; }
+/**
- i915_gem_dmabuf_attach - Do any extra attach work necessary
- @dmabuf: imported dma-buf
- @attach: new attach to do work on
- */
+static int i915_gem_dmabuf_attach(struct dma_buf *dmabuf,
struct dma_buf_attachment *attach)+{
struct drm_i915_gem_object *obj = dma_buf_to_obj(dmabuf);return i915_gem_object_pin_pages_unlocked(obj);+}
+static void i915_gem_dmabuf_detach(struct dma_buf *dmabuf,
struct dma_buf_attachment *attach)+{
struct drm_i915_gem_object *obj = dma_buf_to_obj(dmabuf);i915_gem_object_unpin_pages(obj);+}
We don't normally add kernel-doc for static functions? Otherwise dmabuf_detach() needs matching kernel-doc.
Dropped.
<snip>
+static int igt_dmabuf_import_same_driver(void *arg) +{
struct drm_i915_private *i915 = arg;struct drm_i915_gem_object *obj, *import_obj;struct drm_gem_object *import;struct dma_buf *dmabuf;struct dma_buf_attachment *import_attach;struct sg_table *st;long timeout;int err;force_different_devices = true;obj = i915_gem_object_create_shmem(i915, PAGE_SIZE);if (IS_ERR(obj))err = PTR_ERR(obj)
Done.
<snip>
/* Now try a fake an importer */import_attach = dma_buf_attach(dmabuf, obj->base.dev->dev);if (IS_ERR(import_attach))goto out_import;st = dma_buf_map_attachment(import_attach, DMA_BIDIRECTIONAL);if (IS_ERR(st))goto out_detach;For these two maybe missing err = ?
Yup. Fixed. I also changed the (int)PTR_ERR() in the error prints like you asked for in the next patch.
--Jason
From: Thomas Hellström thomas.hellstrom@linux.intel.com
Until we support p2p dma or as a complement to that, migrate data to system memory at dma-buf attach time if possible.
v2: - Rebase on dynamic exporter. Update the igt_dmabuf_import_same_driver selftest to migrate if we are LMEM capable. v3: - Migrate also in the pin() callback. v4: - Migrate in attach v5: (jason) - Lock around the migration v6: (jason) - Move the can_migrate check outside the lock - Rework the selftests to test more migration conditions. In particular, SMEM, LMEM, and LMEM+SMEM are all checked.
Signed-off-by: Thomas Hellström thomas.hellstrom@linux.intel.com Signed-off-by: Michael J. Ruhl michael.j.ruhl@intel.com Reported-by: kernel test robot lkp@intel.com Signed-off-by: Jason Ekstrand jason@jlekstrand.net Reviewed-by: Jason Ekstrand jason@jlekstrand.net --- drivers/gpu/drm/i915/gem/i915_gem_create.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 23 ++++- drivers/gpu/drm/i915/gem/i915_gem_object.h | 4 + .../drm/i915/gem/selftests/i915_gem_dmabuf.c | 89 ++++++++++++++++++- 4 files changed, 112 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c index 039e4f3b39c79..41c4cd3e1ea01 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c @@ -82,7 +82,7 @@ static int i915_gem_publish(struct drm_i915_gem_object *obj, return 0; }
-static struct drm_i915_gem_object * +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) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c index 9a655f69a0671..5d438b95826b9 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c @@ -170,8 +170,29 @@ static int i915_gem_dmabuf_attach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach) { struct drm_i915_gem_object *obj = dma_buf_to_obj(dmabuf); + struct i915_gem_ww_ctx ww; + int err; + + if (!i915_gem_object_can_migrate(obj, INTEL_REGION_SMEM)) + return -EOPNOTSUPP; + + for_i915_gem_ww(&ww, err, true) { + err = i915_gem_object_lock(obj, &ww); + if (err) + continue; + + err = i915_gem_object_migrate(obj, &ww, INTEL_REGION_SMEM); + if (err) + continue;
- return i915_gem_object_pin_pages_unlocked(obj); + err = i915_gem_object_wait_migration(obj, 0); + if (err) + continue; + + err = i915_gem_object_pin_pages(obj); + } + + return err; }
static void i915_gem_dmabuf_detach(struct dma_buf *dmabuf, diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h index 8be4fadeee487..fbae53bd46384 100644 --- 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;
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c index 4451bbb4917e4..7b7647e7e220a 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c @@ -85,9 +85,62 @@ static int igt_dmabuf_import_self(void *arg) return err; }
-static int igt_dmabuf_import_same_driver(void *arg) +static int igt_dmabuf_import_same_driver_lmem(void *arg) { struct drm_i915_private *i915 = arg; + struct intel_memory_region *lmem = i915->mm.regions[INTEL_REGION_LMEM]; + struct drm_i915_gem_object *obj; + struct drm_gem_object *import; + struct dma_buf *dmabuf; + int err; + + if (!i915->mm.regions[INTEL_REGION_LMEM]) + return 0; + + force_different_devices = true; + + obj = i915_gem_object_create_user(i915, PAGE_SIZE, &lmem, 1); + if (IS_ERR(obj)) { + pr_err("i915_gem_object_create_user failed with err=%d\n", + (int)PTR_ERR(dmabuf)); + err = PTR_ERR(obj); + goto out_ret; + } + + dmabuf = i915_gem_prime_export(&obj->base, 0); + if (IS_ERR(dmabuf)) { + pr_err("i915_gem_prime_export failed with err=%d\n", + (int)PTR_ERR(dmabuf)); + err = PTR_ERR(dmabuf); + goto out; + } + + /* We expect an import of an LMEM-only object to fail with + * -EOPNOTSUPP because it can't be migrated to SMEM. + */ + import = i915_gem_prime_import(&i915->drm, dmabuf); + if (!IS_ERR(import)) { + drm_gem_object_put(import); + pr_err("i915_gem_prime_import succeeded when it shouldn't have\n"); + err = -EINVAL; + } else if (PTR_ERR(import) != -EOPNOTSUPP) { + pr_err("i915_gem_prime_import failed with the wrong err=%d\n", + (int)PTR_ERR(import)); + err = PTR_ERR(import); + } + + dma_buf_put(dmabuf); +out: + i915_gem_object_put(obj); +out_ret: + force_different_devices = false; + return err; +} + +static int igt_dmabuf_import_same_driver(struct drm_i915_private *i915, + struct intel_memory_region **regions, + unsigned int num_regions) +{ struct drm_i915_gem_object *obj, *import_obj; struct drm_gem_object *import; struct dma_buf *dmabuf; @@ -97,9 +150,15 @@ static int igt_dmabuf_import_same_driver(void *arg) int err;
force_different_devices = true; - obj = i915_gem_object_create_shmem(i915, PAGE_SIZE); - if (IS_ERR(obj)) + + obj = i915_gem_object_create_user(i915, PAGE_SIZE, + regions, num_regions); + if (IS_ERR(obj)) { + pr_err("i915_gem_object_create_user failed with err=%d\n", + (int)PTR_ERR(dmabuf)); + err = PTR_ERR(obj); goto out_ret; + }
dmabuf = i915_gem_prime_export(&obj->base, 0); if (IS_ERR(dmabuf)) { @@ -174,6 +233,26 @@ static int igt_dmabuf_import_same_driver(void *arg) return err; }
+static int igt_dmabuf_import_same_driver_smem(void *arg) +{ + struct drm_i915_private *i915 = arg; + struct intel_memory_region *smem = i915->mm.regions[INTEL_REGION_SMEM]; + return igt_dmabuf_import_same_driver(i915, &smem, 1); +} + +static int igt_dmabuf_import_same_driver_lmem_smem(void *arg) +{ + struct drm_i915_private *i915 = arg; + struct intel_memory_region *regions[2]; + + if (!i915->mm.regions[INTEL_REGION_LMEM]) + return 0; + + regions[0] = i915->mm.regions[INTEL_REGION_LMEM]; + regions[1] = i915->mm.regions[INTEL_REGION_SMEM]; + return igt_dmabuf_import_same_driver(i915, regions, 2); +} + static int igt_dmabuf_import(void *arg) { struct drm_i915_private *i915 = arg; @@ -384,7 +463,9 @@ int i915_gem_dmabuf_live_selftests(struct drm_i915_private *i915) { static const struct i915_subtest tests[] = { SUBTEST(igt_dmabuf_export), - SUBTEST(igt_dmabuf_import_same_driver), + SUBTEST(igt_dmabuf_import_same_driver_lmem), + SUBTEST(igt_dmabuf_import_same_driver_smem), + SUBTEST(igt_dmabuf_import_same_driver_lmem_smem), };
return i915_subtests(tests, i915);
On Fri, 16 Jul 2021 at 15:14, Jason Ekstrand jason@jlekstrand.net wrote:
From: Thomas Hellström thomas.hellstrom@linux.intel.com
Until we support p2p dma or as a complement to that, migrate data to system memory at dma-buf attach time if possible.
v2:
- Rebase on dynamic exporter. Update the igt_dmabuf_import_same_driver selftest to migrate if we are LMEM capable.
v3:
- Migrate also in the pin() callback.
v4:
- Migrate in attach
v5: (jason)
- Lock around the migration
v6: (jason)
- Move the can_migrate check outside the lock
- Rework the selftests to test more migration conditions. In particular, SMEM, LMEM, and LMEM+SMEM are all checked.
Signed-off-by: Thomas Hellström thomas.hellstrom@linux.intel.com Signed-off-by: Michael J. Ruhl michael.j.ruhl@intel.com Reported-by: kernel test robot lkp@intel.com Signed-off-by: Jason Ekstrand jason@jlekstrand.net Reviewed-by: Jason Ekstrand jason@jlekstrand.net
drivers/gpu/drm/i915/gem/i915_gem_create.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 23 ++++- drivers/gpu/drm/i915/gem/i915_gem_object.h | 4 + .../drm/i915/gem/selftests/i915_gem_dmabuf.c | 89 ++++++++++++++++++- 4 files changed, 112 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c index 039e4f3b39c79..41c4cd3e1ea01 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c @@ -82,7 +82,7 @@ static int i915_gem_publish(struct drm_i915_gem_object *obj, return 0; }
-static struct drm_i915_gem_object * +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) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c index 9a655f69a0671..5d438b95826b9 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c @@ -170,8 +170,29 @@ static int i915_gem_dmabuf_attach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach) { struct drm_i915_gem_object *obj = dma_buf_to_obj(dmabuf);
struct i915_gem_ww_ctx ww;int err;if (!i915_gem_object_can_migrate(obj, INTEL_REGION_SMEM))return -EOPNOTSUPP;for_i915_gem_ww(&ww, err, true) {err = i915_gem_object_lock(obj, &ww);if (err)continue;err = i915_gem_object_migrate(obj, &ww, INTEL_REGION_SMEM);if (err)continue;
return i915_gem_object_pin_pages_unlocked(obj);
err = i915_gem_object_wait_migration(obj, 0);if (err)continue;err = i915_gem_object_pin_pages(obj);}return err;}
static void i915_gem_dmabuf_detach(struct dma_buf *dmabuf, diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h index 8be4fadeee487..fbae53bd46384 100644 --- 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;
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c index 4451bbb4917e4..7b7647e7e220a 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c @@ -85,9 +85,62 @@ static int igt_dmabuf_import_self(void *arg) return err; }
-static int igt_dmabuf_import_same_driver(void *arg) +static int igt_dmabuf_import_same_driver_lmem(void *arg) { struct drm_i915_private *i915 = arg;
struct intel_memory_region *lmem = i915->mm.regions[INTEL_REGION_LMEM];struct drm_i915_gem_object *obj;struct drm_gem_object *import;struct dma_buf *dmabuf;int err;if (!i915->mm.regions[INTEL_REGION_LMEM])
!lmem
return 0;force_different_devices = true;obj = i915_gem_object_create_user(i915, PAGE_SIZE, &lmem, 1);if (IS_ERR(obj)) {pr_err("i915_gem_object_create_user failed with err=%d\n",(int)PTR_ERR(dmabuf));
PTR_ERR(obj)
err = PTR_ERR(obj);goto out_ret;}dmabuf = i915_gem_prime_export(&obj->base, 0);if (IS_ERR(dmabuf)) {pr_err("i915_gem_prime_export failed with err=%d\n",(int)PTR_ERR(dmabuf));err = PTR_ERR(dmabuf);goto out;}/* We expect an import of an LMEM-only object to fail with* -EOPNOTSUPP because it can't be migrated to SMEM.*/
/* * We expect... */
import = i915_gem_prime_import(&i915->drm, dmabuf);if (!IS_ERR(import)) {drm_gem_object_put(import);pr_err("i915_gem_prime_import succeeded when it shouldn't have\n");err = -EINVAL;} else if (PTR_ERR(import) != -EOPNOTSUPP) {pr_err("i915_gem_prime_import failed with the wrong err=%d\n",(int)PTR_ERR(import));err = PTR_ERR(import);}dma_buf_put(dmabuf);+out:
i915_gem_object_put(obj);+out_ret:
force_different_devices = false;return err;+}
+static int igt_dmabuf_import_same_driver(struct drm_i915_private *i915,
struct intel_memory_region **regions,unsigned int num_regions)+{ struct drm_i915_gem_object *obj, *import_obj; struct drm_gem_object *import; struct dma_buf *dmabuf; @@ -97,9 +150,15 @@ static int igt_dmabuf_import_same_driver(void *arg) int err;
force_different_devices = true;
obj = i915_gem_object_create_shmem(i915, PAGE_SIZE);if (IS_ERR(obj))
obj = i915_gem_object_create_user(i915, PAGE_SIZE,regions, num_regions);if (IS_ERR(obj)) {pr_err("i915_gem_object_create_user failed with err=%d\n",(int)PTR_ERR(dmabuf));
PTR_ERR(obj)
err = PTR_ERR(obj); goto out_ret;} dmabuf = i915_gem_prime_export(&obj->base, 0); if (IS_ERR(dmabuf)) {@@ -174,6 +233,26 @@ static int igt_dmabuf_import_same_driver(void *arg) return err; }
+static int igt_dmabuf_import_same_driver_smem(void *arg) +{
struct drm_i915_private *i915 = arg;struct intel_memory_region *smem = i915->mm.regions[INTEL_REGION_SMEM];
Newline.
return igt_dmabuf_import_same_driver(i915, &smem, 1);+}
+static int igt_dmabuf_import_same_driver_lmem_smem(void *arg) +{
struct drm_i915_private *i915 = arg;struct intel_memory_region *regions[2];if (!i915->mm.regions[INTEL_REGION_LMEM])return 0;regions[0] = i915->mm.regions[INTEL_REGION_LMEM];regions[1] = i915->mm.regions[INTEL_REGION_SMEM];return igt_dmabuf_import_same_driver(i915, regions, 2);+}
static int igt_dmabuf_import(void *arg) { struct drm_i915_private *i915 = arg; @@ -384,7 +463,9 @@ int i915_gem_dmabuf_live_selftests(struct drm_i915_private *i915) { static const struct i915_subtest tests[] = { SUBTEST(igt_dmabuf_export),
SUBTEST(igt_dmabuf_import_same_driver),
SUBTEST(igt_dmabuf_import_same_driver_lmem),SUBTEST(igt_dmabuf_import_same_driver_smem),SUBTEST(igt_dmabuf_import_same_driver_lmem_smem), }; return i915_subtests(tests, i915);-- 2.31.1
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Fixed all the nits below locally. It'll be in the next send.
On Tue, Jul 20, 2021 at 5:53 AM Matthew Auld matthew.william.auld@gmail.com wrote:
On Fri, 16 Jul 2021 at 15:14, Jason Ekstrand jason@jlekstrand.net wrote:
From: Thomas Hellström thomas.hellstrom@linux.intel.com
Until we support p2p dma or as a complement to that, migrate data to system memory at dma-buf attach time if possible.
v2:
- Rebase on dynamic exporter. Update the igt_dmabuf_import_same_driver selftest to migrate if we are LMEM capable.
v3:
- Migrate also in the pin() callback.
v4:
- Migrate in attach
v5: (jason)
- Lock around the migration
v6: (jason)
- Move the can_migrate check outside the lock
- Rework the selftests to test more migration conditions. In particular, SMEM, LMEM, and LMEM+SMEM are all checked.
Signed-off-by: Thomas Hellström thomas.hellstrom@linux.intel.com Signed-off-by: Michael J. Ruhl michael.j.ruhl@intel.com Reported-by: kernel test robot lkp@intel.com Signed-off-by: Jason Ekstrand jason@jlekstrand.net Reviewed-by: Jason Ekstrand jason@jlekstrand.net
drivers/gpu/drm/i915/gem/i915_gem_create.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 23 ++++- drivers/gpu/drm/i915/gem/i915_gem_object.h | 4 + .../drm/i915/gem/selftests/i915_gem_dmabuf.c | 89 ++++++++++++++++++- 4 files changed, 112 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c index 039e4f3b39c79..41c4cd3e1ea01 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c @@ -82,7 +82,7 @@ static int i915_gem_publish(struct drm_i915_gem_object *obj, return 0; }
-static struct drm_i915_gem_object * +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) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c index 9a655f69a0671..5d438b95826b9 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c @@ -170,8 +170,29 @@ static int i915_gem_dmabuf_attach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach) { struct drm_i915_gem_object *obj = dma_buf_to_obj(dmabuf);
struct i915_gem_ww_ctx ww;int err;if (!i915_gem_object_can_migrate(obj, INTEL_REGION_SMEM))return -EOPNOTSUPP;for_i915_gem_ww(&ww, err, true) {err = i915_gem_object_lock(obj, &ww);if (err)continue;err = i915_gem_object_migrate(obj, &ww, INTEL_REGION_SMEM);if (err)continue;
return i915_gem_object_pin_pages_unlocked(obj);
err = i915_gem_object_wait_migration(obj, 0);if (err)continue;err = i915_gem_object_pin_pages(obj);}return err;}
static void i915_gem_dmabuf_detach(struct dma_buf *dmabuf, diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h index 8be4fadeee487..fbae53bd46384 100644 --- 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;
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c index 4451bbb4917e4..7b7647e7e220a 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c @@ -85,9 +85,62 @@ static int igt_dmabuf_import_self(void *arg) return err; }
-static int igt_dmabuf_import_same_driver(void *arg) +static int igt_dmabuf_import_same_driver_lmem(void *arg) { struct drm_i915_private *i915 = arg;
struct intel_memory_region *lmem = i915->mm.regions[INTEL_REGION_LMEM];struct drm_i915_gem_object *obj;struct drm_gem_object *import;struct dma_buf *dmabuf;int err;if (!i915->mm.regions[INTEL_REGION_LMEM])!lmem
return 0;force_different_devices = true;obj = i915_gem_object_create_user(i915, PAGE_SIZE, &lmem, 1);if (IS_ERR(obj)) {pr_err("i915_gem_object_create_user failed with err=%d\n",(int)PTR_ERR(dmabuf));PTR_ERR(obj)
err = PTR_ERR(obj);goto out_ret;}dmabuf = i915_gem_prime_export(&obj->base, 0);if (IS_ERR(dmabuf)) {pr_err("i915_gem_prime_export failed with err=%d\n",(int)PTR_ERR(dmabuf));err = PTR_ERR(dmabuf);goto out;}/* We expect an import of an LMEM-only object to fail with* -EOPNOTSUPP because it can't be migrated to SMEM.*//*
- We expect...
*/
import = i915_gem_prime_import(&i915->drm, dmabuf);if (!IS_ERR(import)) {drm_gem_object_put(import);pr_err("i915_gem_prime_import succeeded when it shouldn't have\n");err = -EINVAL;} else if (PTR_ERR(import) != -EOPNOTSUPP) {pr_err("i915_gem_prime_import failed with the wrong err=%d\n",(int)PTR_ERR(import));err = PTR_ERR(import);}dma_buf_put(dmabuf);+out:
i915_gem_object_put(obj);+out_ret:
force_different_devices = false;return err;+}
+static int igt_dmabuf_import_same_driver(struct drm_i915_private *i915,
struct intel_memory_region **regions,unsigned int num_regions)+{ struct drm_i915_gem_object *obj, *import_obj; struct drm_gem_object *import; struct dma_buf *dmabuf; @@ -97,9 +150,15 @@ static int igt_dmabuf_import_same_driver(void *arg) int err;
force_different_devices = true;
obj = i915_gem_object_create_shmem(i915, PAGE_SIZE);if (IS_ERR(obj))
obj = i915_gem_object_create_user(i915, PAGE_SIZE,regions, num_regions);if (IS_ERR(obj)) {pr_err("i915_gem_object_create_user failed with err=%d\n",(int)PTR_ERR(dmabuf));PTR_ERR(obj)
err = PTR_ERR(obj); goto out_ret;} dmabuf = i915_gem_prime_export(&obj->base, 0); if (IS_ERR(dmabuf)) {@@ -174,6 +233,26 @@ static int igt_dmabuf_import_same_driver(void *arg) return err; }
+static int igt_dmabuf_import_same_driver_smem(void *arg) +{
struct drm_i915_private *i915 = arg;struct intel_memory_region *smem = i915->mm.regions[INTEL_REGION_SMEM];Newline.
return igt_dmabuf_import_same_driver(i915, &smem, 1);+}
+static int igt_dmabuf_import_same_driver_lmem_smem(void *arg) +{
struct drm_i915_private *i915 = arg;struct intel_memory_region *regions[2];if (!i915->mm.regions[INTEL_REGION_LMEM])return 0;regions[0] = i915->mm.regions[INTEL_REGION_LMEM];regions[1] = i915->mm.regions[INTEL_REGION_SMEM];return igt_dmabuf_import_same_driver(i915, regions, 2);+}
static int igt_dmabuf_import(void *arg) { struct drm_i915_private *i915 = arg; @@ -384,7 +463,9 @@ int i915_gem_dmabuf_live_selftests(struct drm_i915_private *i915) { static const struct i915_subtest tests[] = { SUBTEST(igt_dmabuf_export),
SUBTEST(igt_dmabuf_import_same_driver),
SUBTEST(igt_dmabuf_import_same_driver_lmem),SUBTEST(igt_dmabuf_import_same_driver_smem),SUBTEST(igt_dmabuf_import_same_driver_lmem_smem), }; return i915_subtests(tests, i915);-- 2.31.1
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
dri-devel@lists.freedesktop.org
-
Jason Ekstrand -
Matthew Auld