[Intel-gfx] [PATCH 0/6] Enable TTM for integrated GFX objects in sysmem

[Intel-gfx] [PATCH 0/6] Enable TTM for integrated GFX objects in sysmem

[Adrian Larumbe]

This patch series aims at enabling TTM support for system memory objects in
integrated graphics platforms. Whether one wishes to use TTM for sysmem
objects depends on a user-enabled kernel module parameter, so that the
former behaviour of using shmem objects for the system memory region is
kept by default.

The rationale for this change is the benefits in having TTM allocate BO
memory pages for us with alloc_page() rather than shmem.

The first three patches in the series, which the latter three depend on,
are being worked on in parallel by Bob, and handle TTM object cache level
and coherence for sysmem-allocated objects too.

Adrian Larumbe (4):
  drm/i915/ttm: don't overwrite cache_dirty after setting coherency
  drm/i915: Pick the right memory allocation flags for older devices
  drm/i915: Add module param for enabling TTM in sysmem region
  drm/i915: Optionally manage system memory with TTM and poolalloc

Robert Beckett (3):
  drm/i915/ttm: dont trample cache_level overrides during ttm move
  drm/i915: limit ttm to dma32 for i965G[M]
  drm/i915/ttm: only trust snooping for dgfx when deciding default
    cache_level

 drivers/gpu/drm/i915/gem/i915_gem_domain.c    |   4 +-
 drivers/gpu/drm/i915/gem/i915_gem_mman.c      |  21 +-
 drivers/gpu/drm/i915/gem/i915_gem_object.c    |   1 +
 drivers/gpu/drm/i915/gem/i915_gem_object.h    |   2 +-
 .../gpu/drm/i915/gem/i915_gem_object_types.h  |   1 +
 drivers/gpu/drm/i915/gem/i915_gem_phys.c      | 127 +++++++-----
 drivers/gpu/drm/i915/gem/i915_gem_shmem.c     |   4 +-
 drivers/gpu/drm/i915/gem/i915_gem_ttm.c       | 193 +++++++++++++++++-
 drivers/gpu/drm/i915/gem/i915_gem_ttm.h       |  14 ++
 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c  |  13 +-
 drivers/gpu/drm/i915/gem/i915_gem_userptr.c   |   2 +-
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c      |   6 +-
 drivers/gpu/drm/i915/i915_params.c            |   6 +
 drivers/gpu/drm/i915/i915_params.h            |   3 +-
 drivers/gpu/drm/i915/intel_memory_region.c    |   2 +-
 drivers/gpu/drm/i915/intel_region_ttm.c       |   7 +-
 16 files changed, 331 insertions(+), 75 deletions(-)

-- 
2.37.0

[Intel-gfx] [PATCH 1/7] drm/i915/ttm: dont trample cache_level overrides during ttm move

[Adrian Larumbe]

From: Robert Beckett <bob.beckett@collabora.com>

Various places within the driver override the default chosen cache_level.
Before ttm, these overrides were permanent until explicitly changed again
or for the lifetime of the buffer.

TTM movement code came along and decided that it could make that
decision at that time, which is usually well after object creation, so
overrode the cache_level decision and reverted it back to its default
decision.

Add logic to indicate whether the caching mode has been set by anything
other than the move logic. If so, assume that the code that overrode the
defaults knows best and keep it.

Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_object.c       | 1 +
 drivers/gpu/drm/i915/gem/i915_gem_object_types.h | 1 +
 drivers/gpu/drm/i915/gem/i915_gem_ttm.c          | 1 +
 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c     | 9 ++++++---
 4 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index ccec4055fde3..966ac2d778d5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -125,6 +125,7 @@ void i915_gem_object_set_cache_coherency(struct drm_i915_gem_object *obj,
 	struct drm_i915_private *i915 = to_i915(obj->base.dev);
 
 	obj->cache_level = cache_level;
+	obj->ttm.cache_level_override = true;
 
 	if (cache_level != I915_CACHE_NONE)
 		obj->cache_coherent = (I915_BO_CACHE_COHERENT_FOR_READ |
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index 5cf36a130061..14937cf1daaa 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -623,6 +623,7 @@ struct drm_i915_gem_object {
 		struct i915_gem_object_page_iter get_io_page;
 		struct drm_i915_gem_object *backup;
 		bool created:1;
+		bool cache_level_override:1;
 	} ttm;
 
 	/*
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 5a5cf332d8a5..0332d5214aab 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -1253,6 +1253,7 @@ int __i915_gem_ttm_object_init(struct intel_memory_region *mem,
 	i915_gem_object_init_memory_region(obj, mem);
 	i915_ttm_adjust_domains_after_move(obj);
 	i915_ttm_adjust_gem_after_move(obj);
+	obj->ttm.cache_level_override = false;
 	i915_gem_object_unlock(obj);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
index 9a7e50534b84..042c2237e287 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
@@ -129,9 +129,12 @@ void i915_ttm_adjust_gem_after_move(struct drm_i915_gem_object *obj)
 	obj->mem_flags |= i915_ttm_cpu_maps_iomem(bo->resource) ? I915_BO_FLAG_IOMEM :
 		I915_BO_FLAG_STRUCT_PAGE;
 
-	cache_level = i915_ttm_cache_level(to_i915(bo->base.dev), bo->resource,
-					   bo->ttm);
-	i915_gem_object_set_cache_coherency(obj, cache_level);
+	if (!obj->ttm.cache_level_override) {
+		cache_level = i915_ttm_cache_level(to_i915(bo->base.dev),
+						   bo->resource, bo->ttm);
+		i915_gem_object_set_cache_coherency(obj, cache_level);
+		obj->ttm.cache_level_override = false;
+	}
 }
 
 /**
-- 
2.37.0

[Intel-gfx] [PATCH 2/7] drm/i915: limit ttm to dma32 for i965G[M]

[Adrian Larumbe]

From: Robert Beckett <bob.beckett@collabora.com>

i965G[M] cannot relocate objects above 4GiB.
Ensure ttm uses dma32 on these systems.

Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_region_ttm.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_region_ttm.c b/drivers/gpu/drm/i915/intel_region_ttm.c
index 575d67bc6ffe..88b525f9bb2d 100644
--- a/drivers/gpu/drm/i915/intel_region_ttm.c
+++ b/drivers/gpu/drm/i915/intel_region_ttm.c
@@ -32,10 +32,15 @@
 int intel_region_ttm_device_init(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *drm = &dev_priv->drm;
+	bool use_dma32 = false;
+
+	/* i965g[m] cannot relocate objects above 4GiB. */
+	if (IS_I965GM(dev_priv) || IS_I965G(dev_priv))
+		use_dma32 = true;
 
 	return ttm_device_init(&dev_priv->bdev, i915_ttm_driver(),
 			       drm->dev, drm->anon_inode->i_mapping,
-			       drm->vma_offset_manager, false, false);
+			       drm->vma_offset_manager, false, use_dma32);
 }
 
 /**
-- 
2.37.0

[Intel-gfx] [PATCH 3/7] drm/i915/ttm: only trust snooping for dgfx when deciding default cache_level

[Adrian Larumbe]

From: Robert Beckett <bob.beckett@collabora.com>

By default i915_ttm_cache_level() decides I915_CACHE_LLC if HAS_SNOOP.
This is divergent from existing backends code which only considers
HAS_LLC.
Testing shows that trusting snooping on gen5- is unreliable and bsw via
ggtt mappings, so limit DGFX for now and maintain previous behaviour.

Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
index 042c2237e287..a949594237d9 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
@@ -52,7 +52,9 @@ static enum i915_cache_level
 i915_ttm_cache_level(struct drm_i915_private *i915, struct ttm_resource *res,
 		     struct ttm_tt *ttm)
 {
-	return ((HAS_LLC(i915) || HAS_SNOOP(i915)) &&
+	bool can_snoop = HAS_SNOOP(i915) && IS_DGFX(i915);
+
+	return ((HAS_LLC(i915) || can_snoop) &&
 		!i915_ttm_gtt_binds_lmem(res) &&
 		ttm->caching == ttm_cached) ? I915_CACHE_LLC :
 		I915_CACHE_NONE;
-- 
2.37.0

[Intel-gfx] [PATCH 4/7] drm/i915/ttm: don't overwrite cache_dirty after setting coherency

[Adrian Larumbe]

When i915_gem_object_set_cache_level sets the GEM object's cache_dirty to
true, in the case of TTM that will sometimes be overwritten when getting
the object's pages, more specifically for shmem-placed objects for which
its ttm structure has just been populated.

This wasn't an issue so far, even though intel_dpt_create was setting the
object's cache level to 'none', regardless of the platform and memory
placement of the framebuffer. However, commit b6f17c183a3e ("drm/i915/ttm:
dont trample cache_level overrides during ttm move") makes sure the cache
level set by older backends soon to be managed by TTM isn't altered after
their TTM bo ttm structure is populated.

However this led to the 'obj->cache_dirty = true' set in
i915_gem_object_set_cache_level and flush_write_domain to stick around
rather than being reset inside i915_ttm_adjust_gem_after_move after calling
ttm_tt_populate in __i915_ttm_get_pages, which eventually led to a warning
in DGFX platforms.

Make sure it's not set in DGFX platforms.

Signed-off-by: Adrian Larumbe <adrian.larumbe@collabora.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_domain.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 1674b0c5802b..341b60432a12 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -77,7 +77,7 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
 
 	case I915_GEM_DOMAIN_RENDER:
 		if (gpu_write_needs_clflush(obj))
-			obj->cache_dirty = true;
+			obj->cache_dirty = !IS_DGFX(to_i915(obj->base.dev));
 		break;
 	}
 
@@ -275,7 +275,7 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 	/* Always invalidate stale cachelines */
 	if (obj->cache_level != cache_level) {
 		i915_gem_object_set_cache_coherency(obj, cache_level);
-		obj->cache_dirty = true;
+		obj->cache_dirty = !IS_DGFX(to_i915(obj->base.dev));
 	}
 
 	/* The cache-level will be applied when each vma is rebound. */
-- 
2.37.0

[Intel-gfx] [PATCH 5/7] drm/i915: Pick the right memory allocation flags for older devices

[Adrian Larumbe]

i965gm devices cannot relocate objects above 4GiB. This situation was
already being handled in the older shmem GEM object backend, but not in TTM
for BO's that are allocated in system memory.

Borrow the code from shmem so that TTM handles them in the same way.

Signed-off-by: Adrian Larumbe <adrian.larumbe@collabora.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 0332d5214aab..b232aed4927e 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -207,6 +207,11 @@ static int i915_ttm_tt_shmem_populate(struct ttm_device *bdev,
 			return PTR_ERR(filp);
 
 		mask = GFP_HIGHUSER | __GFP_RECLAIMABLE;
+		if (IS_I965GM(i915) || IS_I965G(i915)) {
+			/* 965gm cannot relocate objects above 4GiB. */
+			mask &= ~__GFP_HIGHMEM;
+			mask |= __GFP_DMA32;
+		}
 
 		mapping = filp->f_mapping;
 		mapping_set_gfp_mask(mapping, mask);
-- 
2.37.0

[Intel-gfx] [PATCH 6/7] drm/i915: Add module param for enabling TTM in sysmem region

[Adrian Larumbe]

Introduces a new module parameter, 'use_pool_alloc', which defaults to
'false'. Its goal is to make the driver fall back on TTM for setting up
the system memory region, so that object allocation will be done through
the TTM subsystem rather than shmem objects.

This commit only brings in the new kernel module param, which will be
used by successive commits.

Signed-off-by: Adrian Larumbe <adrian.larumbe@collabora.com>
---
 drivers/gpu/drm/i915/i915_params.c | 6 ++++++
 drivers/gpu/drm/i915/i915_params.h | 3 ++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 6fc475a5db61..1af11f030ab1 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -207,6 +207,12 @@ i915_param_named_unsafe(lmem_size, uint, 0400,
 i915_param_named_unsafe(lmem_bar_size, uint, 0400,
 			"Set the lmem bar size(in MiB).");
 
+i915_param_named_unsafe(use_pool_alloc, bool, 0600,
+	"Force the driver to use TTM's pool allocator API for smem objects. "
+	"This will cause TTM to take over BO allocation even in integrated platforms. "
+	"(default: false)");
+
+
 static __always_inline void _print_param(struct drm_printer *p,
 					 const char *name,
 					 const char *type,
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 2733cb6cfe09..992ee2a4947d 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -84,7 +84,8 @@ struct drm_printer;
 	param(bool, verbose_state_checks, true, 0) \
 	param(bool, nuclear_pageflip, false, 0400) \
 	param(bool, enable_dp_mst, true, 0600) \
-	param(bool, enable_gvt, false, IS_ENABLED(CONFIG_DRM_I915_GVT) ? 0400 : 0)
+	param(bool, enable_gvt, false, IS_ENABLED(CONFIG_DRM_I915_GVT) ? 0400 : 0) \
+	param(bool, use_pool_alloc, false, 0600)
 
 #define MEMBER(T, member, ...) T member;
 struct i915_params {
-- 
2.37.0

[Intel-gfx] [PATCH 7/7] drm/i915: Optionally manage system memory with TTM and poolalloc

[Adrian Larumbe]

Allow system memory to be managed by TTM on integrated graphics platforms.
We replace using the shmem objects with similar use of TTM objects and can
then benefit from using alloc_page() pages instead of shmem pages.

This commit has no effect on DGFX hardware.

Because it manages objects allocated on system memory, support for the
legacy mmap ioctl was introduced. The change also affects how the original
object is accessed when constructing a phys gem object, which means it must
be mapped and accessed in its entirety rather than traversing the list of
shmem pages.

A new set of integrated TTM GEM object operations are needed, which do not
hold pointers to memory mapping functions, because mmap'ing of TTM-managed
BO's in system memory should still be handled by the GEM subsystem. This
new set of operations also handles pwrite and pread, which are forbidden on
platforms having LMEM.

Signed-off-by: Adrian Larumbe <adrian.larumbe@collabora.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c    |  21 ++-
 drivers/gpu/drm/i915/gem/i915_gem_object.h  |   2 +-
 drivers/gpu/drm/i915/gem/i915_gem_phys.c    | 127 ++++++++-----
 drivers/gpu/drm/i915/gem/i915_gem_shmem.c   |   4 +-
 drivers/gpu/drm/i915/gem/i915_gem_ttm.c     | 187 ++++++++++++++++++--
 drivers/gpu/drm/i915/gem/i915_gem_ttm.h     |  14 ++
 drivers/gpu/drm/i915/gem/i915_gem_userptr.c |   2 +-
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c    |   6 +-
 drivers/gpu/drm/i915/intel_memory_region.c  |   2 +-
 9 files changed, 298 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 0c5c43852e24..112de5c0a23f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -83,6 +83,22 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
 	if (!obj)
 		return -ENOENT;
 
+	if (range_overflows(args->offset, args->size, (u64)obj->base.size)) {
+		addr = -EINVAL;
+		goto err;
+	}
+
+	if (i915_gem_object_is_ttm(obj)) {
+		GEM_WARN_ON(!i915->params.use_pool_alloc);
+
+		addr = i915_gem_ttm_mmap_ioctl(obj, args);
+		if (IS_ERR_VALUE(addr))
+			goto err;
+
+		args->addr_ptr = (u64)addr;
+		return 0;
+	}
+
 	/* prime objects have no backing filp to GEM mmap
 	 * pages from.
 	 */
@@ -91,11 +107,6 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
 		goto err;
 	}
 
-	if (range_overflows(args->offset, args->size, (u64)obj->base.size)) {
-		addr = -EINVAL;
-		goto err;
-	}
-
 	addr = vm_mmap(obj->base.filp, 0, args->size,
 		       PROT_READ | PROT_WRITE, MAP_SHARED,
 		       args->offset);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 6f0a3ce35567..c130db4d757f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -74,7 +74,7 @@ __i915_gem_object_create_user(struct drm_i915_private *i915, u64 size,
 
 extern const struct drm_i915_gem_object_ops i915_gem_shmem_ops;
 
-void __i915_gem_object_release_shmem(struct drm_i915_gem_object *obj,
+void __i915_gem_object_release_smem(struct drm_i915_gem_object *obj,
 				     struct sg_table *pages,
 				     bool needs_clflush);
 
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_phys.c b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
index 0d0e46dae559..4efedd3fdf7c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
@@ -16,16 +16,15 @@
 #include "i915_gem_region.h"
 #include "i915_gem_tiling.h"
 #include "i915_scatterlist.h"
+#include "i915_gem_ttm.h"
 
 static int i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
 {
-	struct address_space *mapping = obj->base.filp->f_mapping;
 	struct drm_i915_private *i915 = to_i915(obj->base.dev);
 	struct scatterlist *sg;
 	struct sg_table *st;
 	dma_addr_t dma;
 	void *vaddr;
-	void *dst;
 	int i;
 
 	if (GEM_WARN_ON(i915_gem_object_needs_bit17_swizzle(obj)))
@@ -57,22 +56,40 @@ static int i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
 	sg_dma_address(sg) = dma;
 	sg_dma_len(sg) = obj->base.size;
 
-	dst = vaddr;
-	for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
-		struct page *page;
-		void *src;
+	if (i915_gem_object_is_ttm(obj)) {
+		void *objaddr;
 
-		page = shmem_read_mapping_page(mapping, i);
-		if (IS_ERR(page))
-			goto err_st;
+		objaddr = i915_gem_object_pin_map(obj, I915_MAP_WB);
+		if (IS_ERR(objaddr))
+			return PTR_ERR(objaddr);
 
-		src = kmap_atomic(page);
-		memcpy(dst, src, PAGE_SIZE);
-		drm_clflush_virt_range(dst, PAGE_SIZE);
-		kunmap_atomic(src);
+		drm_clflush_virt_range(objaddr, obj->base.size);
+		memcpy(vaddr, objaddr, obj->base.size);
 
-		put_page(page);
-		dst += PAGE_SIZE;
+		i915_gem_object_unpin_map(obj);
+
+		drm_clflush_virt_range(vaddr, obj->base.size);
+
+	} else {
+		struct address_space *mapping = obj->base.filp->f_mapping;
+		void *dst = vaddr;
+
+		for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
+			struct page *page;
+			void *src;
+
+			page = shmem_read_mapping_page(mapping, i);
+			if (IS_ERR(page))
+				goto err_st;
+
+			src = kmap_atomic(page);
+			memcpy(dst, src, PAGE_SIZE);
+			drm_clflush_virt_range(dst, PAGE_SIZE);
+			kunmap_atomic(src);
+
+			put_page(page);
+			dst += PAGE_SIZE;
+		}
 	}
 
 	intel_gt_chipset_flush(to_gt(i915));
@@ -99,32 +116,48 @@ i915_gem_object_put_pages_phys(struct drm_i915_gem_object *obj,
 	dma_addr_t dma = sg_dma_address(pages->sgl);
 	void *vaddr = sg_page(pages->sgl);
 
-	__i915_gem_object_release_shmem(obj, pages, false);
+	__i915_gem_object_release_smem(obj, pages, false);
 
 	if (obj->mm.dirty) {
-		struct address_space *mapping = obj->base.filp->f_mapping;
-		void *src = vaddr;
-		int i;
-
-		for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
-			struct page *page;
-			char *dst;
-
-			page = shmem_read_mapping_page(mapping, i);
-			if (IS_ERR(page))
-				continue;
-
-			dst = kmap_atomic(page);
-			drm_clflush_virt_range(src, PAGE_SIZE);
-			memcpy(dst, src, PAGE_SIZE);
-			kunmap_atomic(dst);
-
-			set_page_dirty(page);
-			if (obj->mm.madv == I915_MADV_WILLNEED)
-				mark_page_accessed(page);
-			put_page(page);
-
-			src += PAGE_SIZE;
+		if (!i915_gem_object_is_ttm(obj)) {
+			struct address_space *mapping = obj->base.filp->f_mapping;
+			void *src = vaddr;
+			int i;
+
+			for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
+				struct page *page;
+				char *dst;
+
+				page = shmem_read_mapping_page(mapping, i);
+				if (IS_ERR(page))
+					continue;
+
+				dst = kmap_atomic(page);
+				drm_clflush_virt_range(src, PAGE_SIZE);
+				memcpy(dst, src, PAGE_SIZE);
+				kunmap_atomic(dst);
+
+				set_page_dirty(page);
+				if (obj->mm.madv == I915_MADV_WILLNEED)
+					mark_page_accessed(page);
+				put_page(page);
+
+				src += PAGE_SIZE;
+			}
+		} else {
+			void *objaddr;
+
+			objaddr = i915_gem_object_pin_map(obj, I915_MAP_WB);
+			if (IS_ERR(objaddr)) {
+				drm_dbg(obj->base.dev,
+					"i915_gem_object_pin_map_unlocked failed\n");
+				return;
+			}
+
+			drm_clflush_virt_range(vaddr, PAGE_SIZE);
+			memcpy(objaddr, vaddr, obj->base.size);
+			drm_clflush_virt_range(objaddr, obj->base.size);
+			i915_gem_object_unpin_map(obj);
 		}
 		obj->mm.dirty = false;
 	}
@@ -188,7 +221,7 @@ int i915_gem_object_pread_phys(struct drm_i915_gem_object *obj,
 	return 0;
 }
 
-static int i915_gem_object_shmem_to_phys(struct drm_i915_gem_object *obj)
+static int i915_gem_object_smem_to_phys(struct drm_i915_gem_object *obj)
 {
 	struct sg_table *pages;
 	int err;
@@ -201,10 +234,12 @@ static int i915_gem_object_shmem_to_phys(struct drm_i915_gem_object *obj)
 
 	/* Perma-pin (until release) the physical set of pages */
 	__i915_gem_object_pin_pages(obj);
-
-	if (!IS_ERR_OR_NULL(pages))
-		i915_gem_object_put_pages_shmem(obj, pages);
-
+	if (!IS_ERR_OR_NULL(pages)) {
+		if (!i915_gem_object_is_ttm(obj))
+			i915_gem_object_put_pages_shmem(obj, pages);
+		else
+			i915_gem_object_put_pages_ttm(obj, pages);
+	}
 	i915_gem_object_release_memory_region(obj);
 	return 0;
 
@@ -226,7 +261,7 @@ int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj, int align)
 	if (align > obj->base.size)
 		return -EINVAL;
 
-	if (!i915_gem_object_is_shmem(obj))
+	if (!i915_gem_object_is_smem(obj))
 		return -EINVAL;
 
 	if (!i915_gem_object_has_struct_page(obj))
@@ -251,7 +286,7 @@ int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj, int align)
 		return -EFAULT;
 	}
 
-	return i915_gem_object_shmem_to_phys(obj);
+	return i915_gem_object_smem_to_phys(obj);
 }
 
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index 4eed3dd90ba8..ef1192a6485c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -351,7 +351,7 @@ static int shmem_shrink(struct drm_i915_gem_object *obj, unsigned int flags)
 }
 
 void
-__i915_gem_object_release_shmem(struct drm_i915_gem_object *obj,
+__i915_gem_object_release_smem(struct drm_i915_gem_object *obj,
 				struct sg_table *pages,
 				bool needs_clflush)
 {
@@ -382,7 +382,7 @@ __i915_gem_object_release_shmem(struct drm_i915_gem_object *obj,
 
 void i915_gem_object_put_pages_shmem(struct drm_i915_gem_object *obj, struct sg_table *pages)
 {
-	__i915_gem_object_release_shmem(obj, pages, true);
+	__i915_gem_object_release_smem(obj, pages, true);
 
 	i915_gem_gtt_finish_pages(obj, pages);
 
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index b232aed4927e..170b30e5c1a2 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -4,9 +4,11 @@
  */
 
 #include <linux/shmem_fs.h>
+#include <linux/mman.h>
 
 #include <drm/ttm/ttm_bo_driver.h>
 #include <drm/ttm/ttm_placement.h>
+#include <drm/ttm/ttm_bo_api.h>
 #include <drm/drm_buddy.h>
 
 #include "i915_drv.h"
@@ -20,6 +22,8 @@
 #include "gem/i915_gem_ttm.h"
 #include "gem/i915_gem_ttm_move.h"
 #include "gem/i915_gem_ttm_pm.h"
+#include "gem/i915_gem_clflush.h"
+#include "gem/i915_gem_tiling.h"
 #include "gt/intel_gpu_commands.h"
 
 #define I915_TTM_PRIO_PURGE     0
@@ -291,7 +295,7 @@ static struct ttm_tt *i915_ttm_tt_create(struct ttm_buffer_object *bo,
 	if (!i915_tt)
 		return NULL;
 
-	if (obj->flags & I915_BO_ALLOC_CPU_CLEAR &&
+	if (obj->flags & (I915_BO_ALLOC_CPU_CLEAR | I915_BO_ALLOC_USER) &&
 	    man->use_tt)
 		page_flags |= TTM_TT_FLAG_ZERO_ALLOC;
 
@@ -299,7 +303,8 @@ static struct ttm_tt *i915_ttm_tt_create(struct ttm_buffer_object *bo,
 	if (i915_gem_object_is_shrinkable(obj) && caching == ttm_cached) {
 		page_flags |= TTM_TT_FLAG_EXTERNAL |
 			      TTM_TT_FLAG_EXTERNAL_MAPPABLE;
-		i915_tt->is_shmem = true;
+
+		i915_tt->is_shmem = i915->params.use_pool_alloc ? false : true;
 	}
 
 	if (HAS_FLAT_CCS(i915) && i915_gem_object_needs_ccs_pages(obj))
@@ -518,9 +523,7 @@ static int i915_ttm_shrink(struct drm_i915_gem_object *obj, unsigned int flags)
 	if (!bo->ttm || bo->resource->mem_type != TTM_PL_SYSTEM)
 		return 0;
 
-	GEM_BUG_ON(!i915_tt->is_shmem);
-
-	if (!i915_tt->filp)
+	if (!ttm_tt_is_populated(bo->ttm))
 		return 0;
 
 	ret = ttm_bo_wait_ctx(bo, &ctx);
@@ -797,6 +800,16 @@ static int __i915_ttm_get_pages(struct drm_i915_gem_object *obj,
 	}
 
 	if (bo->ttm && !ttm_tt_is_populated(bo->ttm)) {
+		struct drm_i915_private *i915 = to_i915(obj->base.dev);
+		const size_t size = (size_t)bo->ttm->num_pages << PAGE_SHIFT;
+		struct intel_memory_region *mr = i915->mm.regions[INTEL_MEMORY_SYSTEM];
+		/*
+		 * If there's no chance of allocating enough pages for the whole
+		 * object, bail early.
+		 */
+		if (size > resource_size(&mr->region))
+			return -ENOMEM;
+
 		ret = ttm_tt_populate(bo->bdev, bo->ttm, &ctx);
 		if (ret)
 			return ret;
@@ -812,6 +825,14 @@ static int __i915_ttm_get_pages(struct drm_i915_gem_object *obj,
 		if (IS_ERR(rsgt))
 			return PTR_ERR(rsgt);
 
+		if (!HAS_LMEM(to_i915(obj->base.dev)) && bo->ttm) {
+			if (i915_gem_object_needs_bit17_swizzle(obj))
+				i915_gem_object_do_bit_17_swizzle(obj, &rsgt->table);
+
+			if (i915_gem_object_can_bypass_llc(obj))
+				obj->cache_dirty = true;
+		}
+
 		GEM_BUG_ON(obj->mm.rsgt);
 		obj->mm.rsgt = rsgt;
 		__i915_gem_object_set_pages(obj, &rsgt->table,
@@ -888,8 +909,8 @@ static int i915_ttm_migrate(struct drm_i915_gem_object *obj,
 	return __i915_ttm_migrate(obj, mr, obj->flags);
 }
 
-static void i915_ttm_put_pages(struct drm_i915_gem_object *obj,
-			       struct sg_table *st)
+void i915_gem_object_put_pages_ttm(struct drm_i915_gem_object *obj,
+				   struct sg_table *st)
 {
 	/*
 	 * We're currently not called from a shrinker, so put_pages()
@@ -899,10 +920,23 @@ static void i915_ttm_put_pages(struct drm_i915_gem_object *obj,
 	 * and shrinkers will move it out if needed.
 	 */
 
+	if (!HAS_LMEM(to_i915(obj->base.dev)) &&
+	    i915_gem_object_needs_bit17_swizzle(obj))
+		i915_gem_object_save_bit_17_swizzle(obj, st);
+
 	if (obj->mm.rsgt)
 		i915_refct_sgt_put(fetch_and_zero(&obj->mm.rsgt));
 }
 
+static void i915_ttm_put_pages(struct drm_i915_gem_object *obj,
+			       struct sg_table *st)
+{
+	if (likely(i915_gem_object_has_struct_page(obj)))
+		i915_gem_object_put_pages_ttm(obj, st);
+	else
+		i915_gem_object_put_pages_phys(obj, st);
+}
+
 /**
  * i915_ttm_adjust_lru - Adjust an object's position on relevant LRU lists.
  * @obj: The object
@@ -1138,7 +1172,46 @@ static void i915_ttm_unmap_virtual(struct drm_i915_gem_object *obj)
 	ttm_bo_unmap_virtual(i915_gem_to_ttm(obj));
 }
 
-static const struct drm_i915_gem_object_ops i915_gem_ttm_obj_ops = {
+static int
+ttm_pwrite(struct drm_i915_gem_object *obj,
+	     const struct drm_i915_gem_pwrite *arg)
+{
+
+	if (!i915_gem_object_has_struct_page(obj))
+		return i915_gem_object_pwrite_phys(obj, arg);
+
+	return -ENODEV;
+}
+
+static int
+ttm_pread(struct drm_i915_gem_object *obj,
+	    const struct drm_i915_gem_pread *arg)
+{
+	if (!i915_gem_object_has_struct_page(obj))
+		return i915_gem_object_pread_phys(obj, arg);
+
+	return -ENODEV;
+}
+
+static const struct drm_i915_gem_object_ops i915_gem_ttm_integrated_obj_ops = {
+	.name = "i915_gem_object_ttm",
+	.flags = I915_GEM_OBJECT_IS_SHRINKABLE |
+		 I915_GEM_OBJECT_SELF_MANAGED_SHRINK_LIST,
+
+	.get_pages = i915_ttm_get_pages,
+	.put_pages = i915_ttm_put_pages,
+	.truncate = i915_ttm_truncate,
+	.shrink = i915_ttm_shrink,
+
+	.pwrite = ttm_pwrite,
+	.pread = ttm_pread,
+
+	.adjust_lru = i915_ttm_adjust_lru,
+	.delayed_free = i915_ttm_delayed_free,
+	.migrate = i915_ttm_migrate,
+};
+
+static const struct drm_i915_gem_object_ops i915_gem_ttm_discrete_obj_ops = {
 	.name = "i915_gem_object_ttm",
 	.flags = I915_GEM_OBJECT_IS_SHRINKABLE |
 		 I915_GEM_OBJECT_SELF_MANAGED_SHRINK_LIST,
@@ -1160,8 +1233,15 @@ static const struct drm_i915_gem_object_ops i915_gem_ttm_obj_ops = {
 void i915_ttm_bo_destroy(struct ttm_buffer_object *bo)
 {
 	struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
+	struct drm_i915_private *i915 = to_i915(obj->base.dev);
+
+	/*
+	 * This is for the case that an shmem object was turned into a
+	 * phys one and early released its memory region
+	 */
+	if (likely(IS_DGFX(i915) || i915_gem_object_has_struct_page(obj)))
+		i915_gem_object_release_memory_region(obj);
 
-	i915_gem_object_release_memory_region(obj);
 	mutex_destroy(&obj->ttm.get_io_page.lock);
 
 	if (obj->ttm.created) {
@@ -1214,7 +1294,6 @@ int __i915_gem_ttm_object_init(struct intel_memory_region *mem,
 	int ret;
 
 	drm_gem_private_object_init(&i915->drm, &obj->base, size);
-	i915_gem_object_init(obj, &i915_gem_ttm_obj_ops, &lock_class, flags);
 
 	obj->bo_offset = offset;
 
@@ -1227,6 +1306,16 @@ int __i915_gem_ttm_object_init(struct intel_memory_region *mem,
 	bo_type = (obj->flags & I915_BO_ALLOC_USER) ? ttm_bo_type_device :
 		ttm_bo_type_kernel;
 
+	if (!HAS_LMEM(i915) && i915->params.use_pool_alloc) {
+		GEM_WARN_ON(mem->type != INTEL_MEMORY_SYSTEM);
+		bo_type = ttm_bo_type_kernel;
+	}
+
+	i915_gem_object_init(obj, (bo_type == ttm_bo_type_kernel) ?
+			     &i915_gem_ttm_integrated_obj_ops :
+			     &i915_gem_ttm_discrete_obj_ops,
+			     &lock_class, flags);
+
 	obj->base.vma_node.driver_private = i915_gem_to_ttm(obj);
 
 	/* Forcing the page size is kernel internal only */
@@ -1286,3 +1375,81 @@ i915_gem_ttm_system_setup(struct drm_i915_private *i915,
 	intel_memory_region_set_name(mr, "system-ttm");
 	return mr;
 }
+
+bool i915_gem_object_is_ttm(const struct drm_i915_gem_object *obj)
+{
+	return obj->ops == &i915_gem_ttm_discrete_obj_ops ||
+		obj->ops == &i915_gem_ttm_integrated_obj_ops;
+}
+
+struct drm_i915_gem_object *
+i915_gem_object_create_ttm_from_data(struct drm_i915_private *dev_priv,
+				       const void *data, resource_size_t size)
+{
+	struct drm_i915_gem_object *obj;
+	void *vaddr;
+
+	obj = i915_gem_object_create_shmem(dev_priv, round_up(size, PAGE_SIZE));
+	if (IS_ERR(obj))
+		return obj;
+
+	vaddr = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WC);
+	if (IS_ERR(vaddr)) {
+		i915_gem_object_put(obj);
+		return vaddr;
+	}
+
+	memcpy(vaddr, data, size);
+
+	i915_gem_object_unpin_map(obj);
+
+	return obj;
+}
+
+unsigned long i915_gem_ttm_mmap_ioctl(struct drm_i915_gem_object *obj,
+				      struct drm_i915_gem_mmap *args)
+{
+	struct ttm_buffer_object *bo = i915_gem_to_ttm(obj);
+	struct mm_struct *mm = current->mm;
+	struct vm_area_struct *vma;
+	unsigned long addr;
+
+	addr = vm_mmap(NULL, 0, args->size,
+		       PROT_READ | PROT_WRITE, MAP_SHARED,
+		       args->offset);
+	if (IS_ERR_VALUE(addr))
+		return addr;
+
+	if (mmap_write_lock_killable(mm))
+		return -EINTR;
+	vma = find_vma(current->mm, addr);
+	if (IS_ERR_OR_NULL(vma)) {
+		mmap_write_unlock(mm);
+		return -ENOMEM;
+	}
+
+	vma->vm_flags |= VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP | VM_IO;
+	if (args->flags & I915_MMAP_WC)
+		vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+	else
+		vma->vm_page_prot = pgprot_decrypted(vm_get_page_prot(vma->vm_flags));
+	vma->vm_ops = i915_gem_ttm_discrete_obj_ops.mmap_ops;
+	vma->vm_private_data = bo;
+
+	mmap_write_unlock(mm);
+
+	return addr;
+}
+
+bool i915_gem_object_is_smem(struct drm_i915_gem_object *obj)
+{
+	struct intel_memory_region *mr = READ_ONCE(obj->mm.region);
+
+#ifdef CONFIG_LOCKDEP
+	if (i915_gem_object_migratable(obj) &&
+	    i915_gem_object_evictable(obj))
+		assert_object_held(obj);
+#endif
+	return mr && (mr->type == INTEL_MEMORY_SYSTEM ||
+		      mr->type == INTEL_MEMORY_STOLEN_SYSTEM);
+}
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.h b/drivers/gpu/drm/i915/gem/i915_gem_ttm.h
index e4842b4296fc..f8f3fbb2b532 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.h
@@ -95,4 +95,18 @@ static inline bool i915_ttm_cpu_maps_iomem(struct ttm_resource *mem)
 
 bool i915_ttm_resource_mappable(struct ttm_resource *res);
 
+bool i915_gem_object_is_ttm(const struct drm_i915_gem_object *obj);
+
+struct drm_i915_gem_object *
+i915_gem_object_create_ttm_from_data(struct drm_i915_private *dev_priv,
+				     const void *data, resource_size_t size);
+
+unsigned long i915_gem_ttm_mmap_ioctl(struct drm_i915_gem_object *obj,
+				      struct drm_i915_gem_mmap *args);
+
+bool i915_gem_object_is_smem(struct drm_i915_gem_object *obj);
+
+void i915_gem_object_put_pages_ttm(struct drm_i915_gem_object *obj,
+				   struct sg_table *st);
+
 #endif
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
index 094f06b4ce33..30446770a4de 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
@@ -192,7 +192,7 @@ i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj,
 	if (!pages)
 		return;
 
-	__i915_gem_object_release_shmem(obj, pages, true);
+	__i915_gem_object_release_smem(obj, pages, true);
 	i915_gem_gtt_finish_pages(obj, pages);
 
 	/*
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index d5fca1f68eff..17a81bfccec8 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -11,6 +11,7 @@
 #include <drm/drm_print.h>
 
 #include "gem/i915_gem_lmem.h"
+#include "gem/i915_gem_ttm.h"
 #include "intel_uc_fw.h"
 #include "intel_uc_fw_abi.h"
 #include "i915_drv.h"
@@ -482,7 +483,10 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
 		if (!IS_ERR(obj))
 			obj->flags |= I915_BO_ALLOC_PM_EARLY;
 	} else {
-		obj = i915_gem_object_create_shmem_from_data(i915, fw->data, fw->size);
+		if (unlikely(i915->params.use_pool_alloc))
+			obj = i915_gem_object_create_ttm_from_data(i915, fw->data, fw->size);
+		else
+			obj = i915_gem_object_create_shmem_from_data(i915, fw->data, fw->size);
 	}
 
 	if (IS_ERR(obj)) {
diff --git a/drivers/gpu/drm/i915/intel_memory_region.c b/drivers/gpu/drm/i915/intel_memory_region.c
index 9a4a7fb55582..442687285ce6 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/intel_memory_region.c
@@ -321,7 +321,7 @@ int intel_memory_regions_hw_probe(struct drm_i915_private *i915)
 		instance = intel_region_map[i].instance;
 		switch (type) {
 		case INTEL_MEMORY_SYSTEM:
-			if (IS_DGFX(i915))
+			if (IS_DGFX(i915) || i915->params.use_pool_alloc)
 				mem = i915_gem_ttm_system_setup(i915, type,
 								instance);
 			else
-- 
2.37.0

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Enable TTM for integrated GFX objects in sysmem

[Patchwork]

== Series Details ==

Series: Enable TTM for integrated GFX objects in sysmem
URL   : https://patchwork.freedesktop.org/series/106913/
State : warning

== Summary ==

Error: dim checkpatch failed
30b2640e4185 drm/i915/ttm: dont trample cache_level overrides during ttm move
b283b8a7d71e drm/i915: limit ttm to dma32 for i965G[M]
ae22ccdeda30 drm/i915/ttm: only trust snooping for dgfx when deciding default cache_level
8399f78186f7 drm/i915/ttm: don't overwrite cache_dirty after setting coherency
-:14: WARNING:UNKNOWN_COMMIT_ID: Unknown commit id 'b6f17c183a3e', maybe rebased or not pulled?
#14: 
placement of the framebuffer. However, commit b6f17c183a3e ("drm/i915/ttm:

total: 0 errors, 1 warnings, 0 checks, 16 lines checked
b08c8f9acefb drm/i915: Pick the right memory allocation flags for older devices
68bee3ccdca5 drm/i915: Add module param for enabling TTM in sysmem region
-:25: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#25: FILE: drivers/gpu/drm/i915/i915_params.c:211:
+i915_param_named_unsafe(use_pool_alloc, bool, 0600,
+	"Force the driver to use TTM's pool allocator API for smem objects. "

-:29: CHECK:LINE_SPACING: Please don't use multiple blank lines
#29: FILE: drivers/gpu/drm/i915/i915_params.c:215:
+
+

total: 0 errors, 0 warnings, 2 checks, 21 lines checked
085d8ed8fa28 drm/i915: Optionally manage system memory with TTM and poolalloc
-:76: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#76: FILE: drivers/gpu/drm/i915/gem/i915_gem_object.h:78:
+void __i915_gem_object_release_smem(struct drm_i915_gem_object *obj,
 				     struct sg_table *pages,

-:282: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#282: FILE: drivers/gpu/drm/i915/gem/i915_gem_shmem.c:355:
+__i915_gem_object_release_smem(struct drm_i915_gem_object *obj,
 				struct sg_table *pages,

-:423: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#423: FILE: drivers/gpu/drm/i915/gem/i915_gem_ttm.c:1177:
+ttm_pwrite(struct drm_i915_gem_object *obj,
+	     const struct drm_i915_gem_pwrite *arg)

-:425: CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
#425: FILE: drivers/gpu/drm/i915/gem/i915_gem_ttm.c:1179:
+{
+

-:434: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#434: FILE: drivers/gpu/drm/i915/gem/i915_gem_ttm.c:1188:
+ttm_pread(struct drm_i915_gem_object *obj,
+	    const struct drm_i915_gem_pread *arg)

-:519: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#519: FILE: drivers/gpu/drm/i915/gem/i915_gem_ttm.c:1387:
+i915_gem_object_create_ttm_from_data(struct drm_i915_private *dev_priv,
+				       const void *data, resource_size_t size)

total: 0 errors, 0 warnings, 6 checks, 567 lines checked

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Enable TTM for integrated GFX objects in sysmem

[Patchwork]

== Series Details ==

Series: Enable TTM for integrated GFX objects in sysmem
URL   : https://patchwork.freedesktop.org/series/106913/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

[Intel-gfx] ✗ Fi.CI.BAT: failure for Enable TTM for integrated GFX objects in sysmem

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 6683 bytes --]

== Series Details ==

Series: Enable TTM for integrated GFX objects in sysmem
URL   : https://patchwork.freedesktop.org/series/106913/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11963 -> Patchwork_106913v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_106913v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_106913v1, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106913v1/index.html

Participating hosts (47 -> 44)
------------------------------

  Missing    (3): fi-ctg-p8600 fi-icl-u2 fi-hsw-4200u 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_106913v1:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@load:
    - bat-dg1-5:          [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11963/bat-dg1-5/igt@i915_module_load@load.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106913v1/bat-dg1-5/igt@i915_module_load@load.html
    - bat-dg1-6:          [PASS][3] -> [INCOMPLETE][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11963/bat-dg1-6/igt@i915_module_load@load.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106913v1/bat-dg1-6/igt@i915_module_load@load.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_module_load@load:
    - {bat-dg2-9}:        [PASS][5] -> [INCOMPLETE][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11963/bat-dg2-9/igt@i915_module_load@load.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106913v1/bat-dg2-9/igt@i915_module_load@load.html
    - {bat-dg2-8}:        [PASS][7] -> [INCOMPLETE][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11963/bat-dg2-8/igt@i915_module_load@load.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106913v1/bat-dg2-8/igt@i915_module_load@load.html

  
Known issues
------------

  Here are the changes found in Patchwork_106913v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_suspend@basic-s3-without-i915:
    - fi-rkl-11600:       [PASS][9] -> [INCOMPLETE][10] ([i915#5982])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11963/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106913v1/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-bdw-5557u:       NOTRUN -> [SKIP][11] ([fdo#109271] / [fdo#111827])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106913v1/fi-bdw-5557u/igt@kms_chamelium@common-hpd-after-suspend.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3@smem:
    - {bat-adlm-1}:       [DMESG-WARN][12] ([i915#2867]) -> [PASS][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11963/bat-adlm-1/igt@gem_exec_suspend@basic-s3@smem.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106913v1/bat-adlm-1/igt@gem_exec_suspend@basic-s3@smem.html

  * igt@i915_selftest@live@hugepages:
    - fi-rkl-guc:         [DMESG-WARN][14] -> [PASS][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11963/fi-rkl-guc/igt@i915_selftest@live@hugepages.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106913v1/fi-rkl-guc/igt@i915_selftest@live@hugepages.html

  * igt@i915_suspend@basic-s3-without-i915:
    - fi-bdw-5557u:       [INCOMPLETE][16] ([i915#146]) -> [PASS][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11963/fi-bdw-5557u/igt@i915_suspend@basic-s3-without-i915.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106913v1/fi-bdw-5557u/igt@i915_suspend@basic-s3-without-i915.html

  
#### Warnings ####

  * igt@runner@aborted:
    - bat-dg1-5:          [FAIL][18] ([i915#4312] / [i915#5257]) -> [FAIL][19] ([i915#4312])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11963/bat-dg1-5/igt@runner@aborted.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106913v1/bat-dg1-5/igt@runner@aborted.html
    - bat-dg1-6:          [FAIL][20] ([i915#4312] / [i915#5257]) -> [FAIL][21] ([i915#4312])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11963/bat-dg1-6/igt@runner@aborted.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106913v1/bat-dg1-6/igt@runner@aborted.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#146]: https://gitlab.freedesktop.org/drm/intel/issues/146
  [i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888
  [i915#2867]: https://gitlab.freedesktop.org/drm/intel/issues/2867
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#5153]: https://gitlab.freedesktop.org/drm/intel/issues/5153
  [i915#5257]: https://gitlab.freedesktop.org/drm/intel/issues/5257
  [i915#5982]: https://gitlab.freedesktop.org/drm/intel/issues/5982
  [i915#6106]: https://gitlab.freedesktop.org/drm/intel/issues/6106


Build changes
-------------

  * Linux: CI_DRM_11963 -> Patchwork_106913v1

  CI-20190529: 20190529
  CI_DRM_11963: d0b86a71849272bc47e5434cd0b0c428c1c6b2f5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6609: 541d11665dc829f60c84061422adce6b44fa2aef @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_106913v1: d0b86a71849272bc47e5434cd0b0c428c1c6b2f5 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

e6aa9016e60f drm/i915: Optionally manage system memory with TTM and poolalloc
61f3ff9efb91 drm/i915: Add module param for enabling TTM in sysmem region
be0cbdc5eb0f drm/i915: Pick the right memory allocation flags for older devices
a69894e8dad5 drm/i915/ttm: don't overwrite cache_dirty after setting coherency
65b6fd9f01fa drm/i915/ttm: only trust snooping for dgfx when deciding default cache_level
6217f3edfa96 drm/i915: limit ttm to dma32 for i965G[M]
f9dc545abb0d drm/i915/ttm: dont trample cache_level overrides during ttm move

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106913v1/index.html

[-- Attachment #2: Type: text/html, Size: 7693 bytes --]

Re: [Intel-gfx] [PATCH 6/7] drm/i915: Add module param for enabling TTM in sysmem region

[Jani Nikula]

On Tue, 02 Aug 2022, Adrian Larumbe <adrian.larumbe@collabora.com> wrote:
> Introduces a new module parameter, 'use_pool_alloc', which defaults to
> 'false'. Its goal is to make the driver fall back on TTM for setting up
> the system memory region, so that object allocation will be done through
> the TTM subsystem rather than shmem objects.
>
> This commit only brings in the new kernel module param, which will be
> used by successive commits.
>
> Signed-off-by: Adrian Larumbe <adrian.larumbe@collabora.com>
> ---
>  drivers/gpu/drm/i915/i915_params.c | 6 ++++++
>  drivers/gpu/drm/i915/i915_params.h | 3 ++-
>  2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 6fc475a5db61..1af11f030ab1 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -207,6 +207,12 @@ i915_param_named_unsafe(lmem_size, uint, 0400,
>  i915_param_named_unsafe(lmem_bar_size, uint, 0400,
>  			"Set the lmem bar size(in MiB).");
>  
> +i915_param_named_unsafe(use_pool_alloc, bool, 0600,

Do you expect to be able to change this runtime? Or the device specific
debugfs parameter knob?

> +	"Force the driver to use TTM's pool allocator API for smem objects. "
> +	"This will cause TTM to take over BO allocation even in integrated platforms. "
> +	"(default: false)");
> +
> +

Superfluous newline.

>  static __always_inline void _print_param(struct drm_printer *p,
>  					 const char *name,
>  					 const char *type,
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index 2733cb6cfe09..992ee2a4947d 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -84,7 +84,8 @@ struct drm_printer;
>  	param(bool, verbose_state_checks, true, 0) \
>  	param(bool, nuclear_pageflip, false, 0400) \
>  	param(bool, enable_dp_mst, true, 0600) \
> -	param(bool, enable_gvt, false, IS_ENABLED(CONFIG_DRM_I915_GVT) ? 0400 : 0)
> +	param(bool, enable_gvt, false, IS_ENABLED(CONFIG_DRM_I915_GVT) ? 0400 : 0) \
> +	param(bool, use_pool_alloc, false, 0600)
>  
>  #define MEMBER(T, member, ...) T member;
>  struct i915_params {

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 6/7] drm/i915: Add module param for enabling TTM in sysmem region

[Adrian Larumbe]

On 03.08.2022 17:22, Jani Nikula wrote:
>On Tue, 02 Aug 2022, Adrian Larumbe <adrian.larumbe@collabora.com> wrote:
>> Introduces a new module parameter, 'use_pool_alloc', which defaults to
>> 'false'. Its goal is to make the driver fall back on TTM for setting up
>> the system memory region, so that object allocation will be done through
>> the TTM subsystem rather than shmem objects.
>>
>> This commit only brings in the new kernel module param, which will be
>> used by successive commits.
>>
>> Signed-off-by: Adrian Larumbe <adrian.larumbe@collabora.com>
>> ---
>>  drivers/gpu/drm/i915/i915_params.c | 6 ++++++
>>  drivers/gpu/drm/i915/i915_params.h | 3 ++-
>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
>> index 6fc475a5db61..1af11f030ab1 100644
>> --- a/drivers/gpu/drm/i915/i915_params.c
>> +++ b/drivers/gpu/drm/i915/i915_params.c
>> @@ -207,6 +207,12 @@ i915_param_named_unsafe(lmem_size, uint, 0400,
>>  i915_param_named_unsafe(lmem_bar_size, uint, 0400,
>>  			"Set the lmem bar size(in MiB).");
>>  
>> +i915_param_named_unsafe(use_pool_alloc, bool, 0600,
>
>Do you expect to be able to change this runtime? Or the device specific
>debugfs parameter knob?

No, it's a driver load-time setting and doesn't change unless you unload the
driver module and load it back without setting the parameter.

Does that mean I would have to use a different param declaration macro?

>> +	"Force the driver to use TTM's pool allocator API for smem objects. "
>> +	"This will cause TTM to take over BO allocation even in integrated platforms. "
>> +	"(default: false)");
>> +
>> +
>
>Superfluous newline.
>
>>  static __always_inline void _print_param(struct drm_printer *p,
>>  					 const char *name,
>>  					 const char *type,
>> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
>> index 2733cb6cfe09..992ee2a4947d 100644
>> --- a/drivers/gpu/drm/i915/i915_params.h
>> +++ b/drivers/gpu/drm/i915/i915_params.h
>> @@ -84,7 +84,8 @@ struct drm_printer;
>>  	param(bool, verbose_state_checks, true, 0) \
>>  	param(bool, nuclear_pageflip, false, 0400) \
>>  	param(bool, enable_dp_mst, true, 0600) \
>> -	param(bool, enable_gvt, false, IS_ENABLED(CONFIG_DRM_I915_GVT) ? 0400 : 0)
>> +	param(bool, enable_gvt, false, IS_ENABLED(CONFIG_DRM_I915_GVT) ? 0400 : 0) \
>> +	param(bool, use_pool_alloc, false, 0600)
>>  
>>  #define MEMBER(T, member, ...) T member;
>>  struct i915_params {
>
>-- 
>Jani Nikula, Intel Open Source Graphics Center

Adrian Larumbe

Re: [Intel-gfx] [PATCH 6/7] drm/i915: Add module param for enabling TTM in sysmem region

[Jani Nikula]

On Tue, 09 Aug 2022, Adrian Larumbe <adrian.larumbe@collabora.com> wrote:
> On 03.08.2022 17:22, Jani Nikula wrote:
>>On Tue, 02 Aug 2022, Adrian Larumbe <adrian.larumbe@collabora.com> wrote:
>>> Introduces a new module parameter, 'use_pool_alloc', which defaults to
>>> 'false'. Its goal is to make the driver fall back on TTM for setting up
>>> the system memory region, so that object allocation will be done through
>>> the TTM subsystem rather than shmem objects.
>>>
>>> This commit only brings in the new kernel module param, which will be
>>> used by successive commits.
>>>
>>> Signed-off-by: Adrian Larumbe <adrian.larumbe@collabora.com>
>>> ---
>>>  drivers/gpu/drm/i915/i915_params.c | 6 ++++++
>>>  drivers/gpu/drm/i915/i915_params.h | 3 ++-
>>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
>>> index 6fc475a5db61..1af11f030ab1 100644
>>> --- a/drivers/gpu/drm/i915/i915_params.c
>>> +++ b/drivers/gpu/drm/i915/i915_params.c
>>> @@ -207,6 +207,12 @@ i915_param_named_unsafe(lmem_size, uint, 0400,
>>>  i915_param_named_unsafe(lmem_bar_size, uint, 0400,
>>>  			"Set the lmem bar size(in MiB).");
>>>  
>>> +i915_param_named_unsafe(use_pool_alloc, bool, 0600,
>>
>>Do you expect to be able to change this runtime? Or the device specific
>>debugfs parameter knob?
>
> No, it's a driver load-time setting and doesn't change unless you unload the
> driver module and load it back without setting the parameter.
>
> Does that mean I would have to use a different param declaration macro?

No, just different mode, 0400 instead of 0600.

>
>>> +	"Force the driver to use TTM's pool allocator API for smem objects. "
>>> +	"This will cause TTM to take over BO allocation even in integrated platforms. "
>>> +	"(default: false)");
>>> +
>>> +
>>
>>Superfluous newline.
>>
>>>  static __always_inline void _print_param(struct drm_printer *p,
>>>  					 const char *name,
>>>  					 const char *type,
>>> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
>>> index 2733cb6cfe09..992ee2a4947d 100644
>>> --- a/drivers/gpu/drm/i915/i915_params.h
>>> +++ b/drivers/gpu/drm/i915/i915_params.h
>>> @@ -84,7 +84,8 @@ struct drm_printer;
>>>  	param(bool, verbose_state_checks, true, 0) \
>>>  	param(bool, nuclear_pageflip, false, 0400) \
>>>  	param(bool, enable_dp_mst, true, 0600) \
>>> -	param(bool, enable_gvt, false, IS_ENABLED(CONFIG_DRM_I915_GVT) ? 0400 : 0)
>>> +	param(bool, enable_gvt, false, IS_ENABLED(CONFIG_DRM_I915_GVT) ? 0400 : 0) \
>>> +	param(bool, use_pool_alloc, false, 0600)
>>>  
>>>  #define MEMBER(T, member, ...) T member;
>>>  struct i915_params {
>>
>>-- 
>>Jani Nikula, Intel Open Source Graphics Center
>
> Adrian Larumbe

-- 
Jani Nikula, Intel Open Source Graphics Center