[CI 1/4] drm/i915: consider min_page_size when migrating

[CI 1/4] drm/i915: consider min_page_size when migrating

[Matthew Auld]

We can only force migrate an object if the existing object size is
compatible with the new destinations min_page_size for the region.
Currently we blow up with something like:

[ 2857.497462] kernel BUG at drivers/gpu/drm/i915/gt/intel_migrate.c:431!
[ 2857.497497] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
[ 2857.497502] CPU: 1 PID: 8921 Comm: i915_selftest Tainted: G     U  W         5.18.0-rc1-drm-tip+ #27
[ 2857.497513] RIP: 0010:emit_pte.cold+0x11a/0x17e [i915]
[ 2857.497646] Code: 00 48 c7 c2 f0 cd c1 a0 48 c7 c7 e9 99 bd a0 e8 d2 77 5d e0 bf 01 00 00 00 e8 08 47 5d e0 31 f6 bf 09 00 00 00 e8 3c 7b 4d e0 <0f> 0b 48 c7 c1 e0 2a c5 a0 ba 34 00 00 00 48 c7 c6 00 ce c1 a0 48
[ 2857.497654] RSP: 0018:ffffc900000f7748 EFLAGS: 00010246
[ 2857.497658] RAX: 0000000000000000 RBX: ffffc900000f77c8 RCX: 0000000000000006
[ 2857.497662] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000009
[ 2857.497665] RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000001
[ 2857.497668] R10: 0000000000022302 R11: ffff88846dea08f0 R12: 0000000000010000
[ 2857.497672] R13: 0000000001880000 R14: 000000000000081b R15: ffff888106b7c040
[ 2857.497675] FS:  00007f0d4c4e0600(0000) GS:ffff88845da80000(0000) knlGS:0000000000000000
[ 2857.497679] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 2857.497682] CR2: 00007f113966c088 CR3: 0000000211e60003 CR4: 00000000003706e0
[ 2857.497686] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 2857.497689] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 2857.497692] Call Trace:
[ 2857.497694]  <TASK>
[ 2857.497697]  intel_context_migrate_copy+0x1e5/0x4f0 [i915]

Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Nirmoy Das <nirmoy.das@linux.intel.com>
Reviewed-by: Nirmoy Das <nirmoy.das@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_object.c            | 3 +++
 drivers/gpu/drm/i915/gem/selftests/i915_gem_migrate.c | 4 +++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index 747ac65e060f..06b1b188ce5a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -606,6 +606,9 @@ bool i915_gem_object_can_migrate(struct drm_i915_gem_object *obj,
 	if (!mr)
 		return false;
 
+	if (!IS_ALIGNED(obj->base.size, mr->min_page_size))
+		return false;
+
 	if (obj->mm.region == mr)
 		return true;
 
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 6bd61b1f839c..801af51aff62 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_migrate.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_migrate.c
@@ -47,14 +47,16 @@ static int igt_create_migrate(struct intel_gt *gt, enum intel_region_id src,
 {
 	struct drm_i915_private *i915 = gt->i915;
 	struct intel_memory_region *src_mr = i915->mm.regions[src];
+	struct intel_memory_region *dst_mr = i915->mm.regions[dst];
 	struct drm_i915_gem_object *obj;
 	struct i915_gem_ww_ctx ww;
 	int err = 0;
 
 	GEM_BUG_ON(!src_mr);
+	GEM_BUG_ON(!dst_mr);
 
 	/* Switch object backing-store on create */
-	obj = i915_gem_object_create_region(src_mr, PAGE_SIZE, 0, 0);
+	obj = i915_gem_object_create_region(src_mr, dst_mr->min_page_size, 0, 0);
 	if (IS_ERR(obj))
 		return PTR_ERR(obj);
 
-- 
2.34.1

[CI 2/4] drm/i915/buddy: sanity check the size

[Matthew Auld]

Ensure we check that the size is compatible with the requested
page_size. For tiny objects that are automatically annotated with
TTM_PL_FLAG_CONTIGUOUS(since they fit within a single page), we
currently end up silently overriding the min_page_size, which ends up
hiding bugs elsewhere.

Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Nirmoy Das <nirmoy.das@linux.intel.com>
Reviewed-by: Nirmoy Das <nirmoy.das@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
index 8e4e3f72c1ef..a5109548abc0 100644
--- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
+++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
@@ -70,6 +70,7 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man,
 		min_page_size = bo->page_alignment << PAGE_SHIFT;
 
 	GEM_BUG_ON(min_page_size < mm->chunk_size);
+	GEM_BUG_ON(!IS_ALIGNED(size, min_page_size));
 
 	if (place->fpfn + bman_res->base.num_pages != place->lpfn &&
 	    place->flags & TTM_PL_FLAG_CONTIGUOUS) {
-- 
2.34.1

[CI 3/4] drm/i915/selftests: fixup min_alignment usage

[Matthew Auld]

Trying to cast the region id into the region type doesn't work too well,
since the i915_vm_min_alignment() won't give us the correct value for
the stolen-lmem case.

Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Nirmoy Das <nirmoy.das@linux.intel.com>
Cc: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index 5c9bfa409ff5..bccc49a8ab5e 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -1150,7 +1150,7 @@ static int misaligned_pin(struct i915_address_space *vm,
 		flags |= PIN_GLOBAL;
 
 	for_each_memory_region(mr, vm->i915, id) {
-		u64 min_alignment = i915_vm_min_alignment(vm, (enum intel_memory_type)id);
+		u64 min_alignment = i915_vm_min_alignment(vm, mr->type);
 		u64 size = min_alignment;
 		u64 addr = round_down(hole_start + (hole_size / 2), min_alignment);
 
-- 
2.34.1

[CI 4/4] drm/i915/selftests: tweak the misaligned_case

[Matthew Auld]

The compact-pt layout restrictions should only apply to the ppGTT. Also
make this play nice on platforms that only have the 64K GTT restriction,
and not the compact-pt thing.

Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Nirmoy Das <nirmoy.das@linux.intel.com>
Cc: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index bccc49a8ab5e..8633bec18fa7 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -1112,10 +1112,16 @@ static int misaligned_case(struct i915_address_space *vm, struct intel_memory_re
 	expected_vma_size = round_up(size, 1 << (ffs(vma->resource->page_sizes_gtt) - 1));
 	expected_node_size = expected_vma_size;
 
-	if (NEEDS_COMPACT_PT(vm->i915) && i915_gem_object_is_lmem(obj)) {
-		/* compact-pt should expand lmem node to 2MB */
+	if (HAS_64K_PAGES(vm->i915) && i915_gem_object_is_lmem(obj)) {
+		/*
+		 * The compact-pt should expand lmem node to 2MB for the ppGTT,
+		 * for all other cases we should only expect 64K.
+		 */
 		expected_vma_size = round_up(size, I915_GTT_PAGE_SIZE_64K);
-		expected_node_size = round_up(size, I915_GTT_PAGE_SIZE_2M);
+		if (NEEDS_COMPACT_PT(vm->i915) && !i915_is_ggtt(vm))
+			expected_node_size = round_up(size, I915_GTT_PAGE_SIZE_2M);
+		else
+			expected_node_size = round_up(size, I915_GTT_PAGE_SIZE_64K);
 	}
 
 	if (vma->size != expected_vma_size || vma->node.size != expected_node_size) {
-- 
2.34.1

Re: [CI 3/4] drm/i915/selftests: fixup min_alignment usage

[Ramalingam C]

On 2022-04-20 at 19:16:12 +0100, Matthew Auld wrote:
> Trying to cast the region id into the region type doesn't work too well,
> since the i915_vm_min_alignment() won't give us the correct value for
> the stolen-lmem case.
> 
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Looks good to me.

Reviewed-by: Ramalingam C <ramalingam.c@intel.com>

> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Nirmoy Das <nirmoy.das@linux.intel.com>
> Cc: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> index 5c9bfa409ff5..bccc49a8ab5e 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> @@ -1150,7 +1150,7 @@ static int misaligned_pin(struct i915_address_space *vm,
>  		flags |= PIN_GLOBAL;
>  
>  	for_each_memory_region(mr, vm->i915, id) {
> -		u64 min_alignment = i915_vm_min_alignment(vm, (enum intel_memory_type)id);
> +		u64 min_alignment = i915_vm_min_alignment(vm, mr->type);
>  		u64 size = min_alignment;
>  		u64 addr = round_down(hole_start + (hole_size / 2), min_alignment);
>  
> -- 
> 2.34.1
> 

Re: [CI 4/4] drm/i915/selftests: tweak the misaligned_case

[Ramalingam C]

On 2022-04-20 at 19:16:13 +0100, Matthew Auld wrote:
> The compact-pt layout restrictions should only apply to the ppGTT. Also
> make this play nice on platforms that only have the 64K GTT restriction,
> and not the compact-pt thing.
> 
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Nirmoy Das <nirmoy.das@linux.intel.com>
> Cc: Ramalingam C <ramalingam.c@intel.com>
Looks good to me.

Reviewed-by: Ramalingam C <ramalingam.c@intel.com>

> ---
>  drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> index bccc49a8ab5e..8633bec18fa7 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> @@ -1112,10 +1112,16 @@ static int misaligned_case(struct i915_address_space *vm, struct intel_memory_re
>  	expected_vma_size = round_up(size, 1 << (ffs(vma->resource->page_sizes_gtt) - 1));
>  	expected_node_size = expected_vma_size;
>  
> -	if (NEEDS_COMPACT_PT(vm->i915) && i915_gem_object_is_lmem(obj)) {
> -		/* compact-pt should expand lmem node to 2MB */
> +	if (HAS_64K_PAGES(vm->i915) && i915_gem_object_is_lmem(obj)) {
> +		/*
> +		 * The compact-pt should expand lmem node to 2MB for the ppGTT,
> +		 * for all other cases we should only expect 64K.
> +		 */
>  		expected_vma_size = round_up(size, I915_GTT_PAGE_SIZE_64K);
> -		expected_node_size = round_up(size, I915_GTT_PAGE_SIZE_2M);
> +		if (NEEDS_COMPACT_PT(vm->i915) && !i915_is_ggtt(vm))
> +			expected_node_size = round_up(size, I915_GTT_PAGE_SIZE_2M);
> +		else
> +			expected_node_size = round_up(size, I915_GTT_PAGE_SIZE_64K);
>  	}
>  
>  	if (vma->size != expected_vma_size || vma->node.size != expected_node_size) {
> -- 
> 2.34.1
>