[PATCH v3 1/8] scsi: core: fix the dma_max_mapping_size call

[PATCH v3 1/8] scsi: core: fix the dma_max_mapping_size call

[Gerd Hoffmann]

From: Christoph Hellwig <hch@lst.de>

We should only call dma_max_mapping_size for devices that have a DMA mask
set, otherwise we can run into a NULL pointer dereference that will crash
the system.

Also we need to do right shift to get the sectors from the size in bytes,
not a left shift.

Fixes: bdd17bdef7d8 ("scsi: core: take the DMA max mapping size into account")
Reported-by: Bart Van Assche <bvanassche@acm.org>
Reported-by: Ming Lei <tom.leiming@gmail.com>
Tested-by: Guilherme G. Piccoli <gpiccoli@canonical.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
(cherry picked from commit 1b5d9a6e98350e0713b4faa1b04e8f239f63b581)
---
 drivers/scsi/scsi_lib.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 9381171c2fc0..11e64b50497f 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1784,8 +1784,10 @@ void __scsi_init_queue(struct Scsi_Host *shost, struct request_queue *q)
 		blk_queue_max_integrity_segments(q, shost->sg_prot_tablesize);
 	}
 
-	shost->max_sectors = min_t(unsigned int, shost->max_sectors,
-			dma_max_mapping_size(dev) << SECTOR_SHIFT);
+	if (dev->dma_mask) {
+		shost->max_sectors = min_t(unsigned int, shost->max_sectors,
+				dma_max_mapping_size(dev) >> SECTOR_SHIFT);
+	}
 	blk_queue_max_hw_sectors(q, shost->max_sectors);
 	if (shost->unchecked_isa_dma)
 		blk_queue_bounce_limit(q, BLK_BOUNCE_ISA);
-- 
2.18.1

[PATCH v3 2/8] ttm: turn ttm_bo_device.vma_manager into a pointer

[Gerd Hoffmann]

Rename the embedded struct vma_offset_manager, it is named _vma_manager
now.  ttm_bo_device.vma_manager is a pointer now, pointing to the
embedded ttm_bo_device._vma_manager by default.

Add ttm_bo_device_init_with_vma_manager() function which allows to
initialize ttm with a different vma manager.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/drm/ttm/ttm_bo_driver.h | 11 +++++++++--
 drivers/gpu/drm/ttm/ttm_bo.c    | 29 +++++++++++++++++++++--------
 drivers/gpu/drm/ttm/ttm_bo_vm.c |  6 +++---
 3 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 3f1935c19a66..2f84d6bcd1a7 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -441,7 +441,8 @@ extern struct ttm_bo_global {
  *
  * @driver: Pointer to a struct ttm_bo_driver struct setup by the driver.
  * @man: An array of mem_type_managers.
- * @vma_manager: Address space manager
+ * @vma_manager: Address space manager (pointer)
+ * @_vma_manager: Address space manager (enbedded)
  * lru_lock: Spinlock that protects the buffer+device lru lists and
  * ddestroy lists.
  * @dev_mapping: A pointer to the struct address_space representing the
@@ -464,7 +465,8 @@ struct ttm_bo_device {
 	/*
 	 * Protected by internal locks.
 	 */
-	struct drm_vma_offset_manager vma_manager;
+	struct drm_vma_offset_manager *vma_manager;
+	struct drm_vma_offset_manager _vma_manager;
 
 	/*
 	 * Protected by the global:lru lock.
@@ -597,6 +599,11 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
 		       struct ttm_bo_driver *driver,
 		       struct address_space *mapping,
 		       bool need_dma32);
+int ttm_bo_device_init_with_vma_manager(struct ttm_bo_device *bdev,
+					struct ttm_bo_driver *driver,
+					struct address_space *mapping,
+					struct drm_vma_offset_manager *vma_manager,
+					bool need_dma32);
 
 /**
  * ttm_bo_unmap_virtual
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 10a861a1690c..0ed1a1182962 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -672,7 +672,7 @@ static void ttm_bo_release(struct kref *kref)
 	struct ttm_bo_device *bdev = bo->bdev;
 	struct ttm_mem_type_manager *man = &bdev->man[bo->mem.mem_type];
 
-	drm_vma_offset_remove(&bdev->vma_manager, &bo->base.vma_node);
+	drm_vma_offset_remove(bdev->vma_manager, &bo->base.vma_node);
 	ttm_mem_io_lock(man, false);
 	ttm_mem_io_free_vm(bo);
 	ttm_mem_io_unlock(man);
@@ -1353,7 +1353,7 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev,
 	 */
 	if (bo->type == ttm_bo_type_device ||
 	    bo->type == ttm_bo_type_sg)
-		ret = drm_vma_offset_add(&bdev->vma_manager, &bo->base.vma_node,
+		ret = drm_vma_offset_add(bdev->vma_manager, &bo->base.vma_node,
 					 bo->mem.num_pages);
 
 	/* passed reservation objects should already be locked,
@@ -1704,7 +1704,7 @@ int ttm_bo_device_release(struct ttm_bo_device *bdev)
 			pr_debug("Swap list %d was clean\n", i);
 	spin_unlock(&glob->lru_lock);
 
-	drm_vma_offset_manager_destroy(&bdev->vma_manager);
+	drm_vma_offset_manager_destroy(&bdev->_vma_manager);
 
 	if (!ret)
 		ttm_bo_global_release();
@@ -1713,10 +1713,11 @@ int ttm_bo_device_release(struct ttm_bo_device *bdev)
 }
 EXPORT_SYMBOL(ttm_bo_device_release);
 
-int ttm_bo_device_init(struct ttm_bo_device *bdev,
-		       struct ttm_bo_driver *driver,
-		       struct address_space *mapping,
-		       bool need_dma32)
+int ttm_bo_device_init_with_vma_manager(struct ttm_bo_device *bdev,
+					struct ttm_bo_driver *driver,
+					struct address_space *mapping,
+					struct drm_vma_offset_manager *vma_manager,
+					bool need_dma32)
 {
 	struct ttm_bo_global *glob = &ttm_bo_glob;
 	int ret;
@@ -1737,7 +1738,8 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
 	if (unlikely(ret != 0))
 		goto out_no_sys;
 
-	drm_vma_offset_manager_init(&bdev->vma_manager,
+	bdev->vma_manager = vma_manager;
+	drm_vma_offset_manager_init(&bdev->_vma_manager,
 				    DRM_FILE_PAGE_OFFSET_START,
 				    DRM_FILE_PAGE_OFFSET_SIZE);
 	INIT_DELAYED_WORK(&bdev->wq, ttm_bo_delayed_workqueue);
@@ -1754,6 +1756,17 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
 	ttm_bo_global_release();
 	return ret;
 }
+EXPORT_SYMBOL(ttm_bo_device_init_with_vma_manager);
+
+int ttm_bo_device_init(struct ttm_bo_device *bdev,
+		       struct ttm_bo_driver *driver,
+		       struct address_space *mapping,
+		       bool need_dma32)
+{
+	return ttm_bo_device_init_with_vma_manager(bdev, driver, mapping,
+						   &bdev->_vma_manager,
+						   need_dma32);
+}
 EXPORT_SYMBOL(ttm_bo_device_init);
 
 /*
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 85f5bcbe0c76..d4eecde8d050 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -409,16 +409,16 @@ static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_bo_device *bdev,
 	struct drm_vma_offset_node *node;
 	struct ttm_buffer_object *bo = NULL;
 
-	drm_vma_offset_lock_lookup(&bdev->vma_manager);
+	drm_vma_offset_lock_lookup(bdev->vma_manager);
 
-	node = drm_vma_offset_lookup_locked(&bdev->vma_manager, offset, pages);
+	node = drm_vma_offset_lookup_locked(bdev->vma_manager, offset, pages);
 	if (likely(node)) {
 		bo = container_of(node, struct ttm_buffer_object,
 				  base.vma_node);
 		bo = ttm_bo_get_unless_zero(bo);
 	}
 
-	drm_vma_offset_unlock_lookup(&bdev->vma_manager);
+	drm_vma_offset_unlock_lookup(bdev->vma_manager);
 
 	if (!bo)
 		pr_err("Could not find buffer object to map\n");
-- 
2.18.1

[PATCH v3 3/8] drm/ttm: add gem_ttm_bo_device_init()

[Gerd Hoffmann]

Now with ttm_buffer_object being a subclass of drm_gem_object we can
easily lookup ttm_buffer_object for a given drm_gem_object, which in
turm allows to create common helper functions.

This patch starts off with a gem_ttm_bo_device_init() helper function
which initializes ttm with the vma offset manager used by gem, to make
sure gem and ttm have the same view on vma offsets.

With that in place gem+ttm drivers don't need their private
drm_driver.dumb_map_offset implementation any more.

v3:
 - complete rewrite

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/drm/drm_gem_ttm_helper.h     | 30 +++++++++++++++++++++++
 drivers/gpu/drm/drm_gem_ttm_helper.c | 36 ++++++++++++++++++++++++++++
 Documentation/gpu/drm-mm.rst         | 12 ++++++++++
 drivers/gpu/drm/Kconfig              |  7 ++++++
 drivers/gpu/drm/Makefile             |  3 +++
 5 files changed, 88 insertions(+)
 create mode 100644 include/drm/drm_gem_ttm_helper.h
 create mode 100644 drivers/gpu/drm/drm_gem_ttm_helper.c

diff --git a/include/drm/drm_gem_ttm_helper.h b/include/drm/drm_gem_ttm_helper.h
new file mode 100644
index 000000000000..43c9db3583cc
--- /dev/null
+++ b/include/drm/drm_gem_ttm_helper.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef DRM_GEM_TTM_HELPER_H
+#define DRM_GEM_TTM_HELPER_H
+
+#include <linux/kernel.h>
+
+#include <drm/drm_gem.h>
+#include <drm/drm_device.h>
+#include <drm/ttm/ttm_bo_api.h>
+#include <drm/ttm/ttm_bo_driver.h>
+
+/**
+ * Returns the container of type &struct ttm_buffer_object
+ * for field base.
+ * @gem:	the GEM object
+ * Returns:	The containing GEM VRAM object
+ */
+static inline struct ttm_buffer_object *drm_gem_ttm_of_gem(
+	struct drm_gem_object *gem)
+{
+	return container_of(gem, struct ttm_buffer_object, base);
+}
+
+int drm_gem_ttm_bo_device_init(struct drm_device *dev,
+			       struct ttm_bo_device *bdev,
+			       struct ttm_bo_driver *driver,
+			       bool need_dma32);
+
+#endif
diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c b/drivers/gpu/drm/drm_gem_ttm_helper.c
new file mode 100644
index 000000000000..0c57e9fd50b9
--- /dev/null
+++ b/drivers/gpu/drm/drm_gem_ttm_helper.c
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <drm/drm_gem_ttm_helper.h>
+
+/**
+ * DOC: overview
+ *
+ * This library provides helper functions for gem objects backed by
+ * ttm.
+ */
+
+/**
+ * drm_gem_ttm_bo_device_init - ttm init for devices which use gem+ttm
+ *
+ * @dev: A pointer to a struct drm_device.
+ * @bdev: A pointer to a struct ttm_bo_device to initialize.
+ * @driver: A pointer to a struct ttm_bo_driver set up by the caller.
+ * @need_dma32: Whenever the device is limited to 32bit DMA.
+ *
+ * This initializes ttm with dev->vma_offset_manager, so gem and ttm
+ * fuction are working with the same vma_offset_manager.
+ *
+ * Returns:
+ * !0: Failure.
+ */
+int drm_gem_ttm_bo_device_init(struct drm_device *dev,
+			       struct ttm_bo_device *bdev,
+			       struct ttm_bo_driver *driver,
+			       bool need_dma32)
+{
+	return ttm_bo_device_init_with_vma_manager(bdev, driver,
+						   dev->anon_inode->i_mapping,
+						   dev->vma_offset_manager,
+						   need_dma32);
+}
+EXPORT_SYMBOL(drm_gem_ttm_bo_device_init);
diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index b664f054c259..a70a1d9f30ec 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -412,6 +412,18 @@ VRAM MM Helper Functions Reference
 .. kernel-doc:: drivers/gpu/drm/drm_vram_mm_helper.c
    :export:
 
+GEM TTM Helper Functions Reference
+-----------------------------------
+
+.. kernel-doc:: drivers/gpu/drm/drm_gem_ttm_helper.c
+   :doc: overview
+
+.. kernel-doc:: include/drm/drm_gem_ttm_helper.h
+   :internal:
+
+.. kernel-doc:: drivers/gpu/drm/drm_gem_ttm_helper.c
+   :export:
+
 VMA Offset Manager
 ==================
 
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index e6f40fb54c9a..f7b25519f95c 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -172,6 +172,13 @@ config DRM_VRAM_HELPER
 	help
 	  Helpers for VRAM memory management
 
+config DRM_TTM_HELPER
+	tristate
+	depends on DRM
+	select DRM_TTM
+	help
+	  Helpers for ttm-based gem objects
+
 config DRM_GEM_CMA_HELPER
 	bool
 	depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 10f8329a8b71..545c61d6528b 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -37,6 +37,9 @@ drm_vram_helper-y := drm_gem_vram_helper.o \
 		     drm_vram_mm_helper.o
 obj-$(CONFIG_DRM_VRAM_HELPER) += drm_vram_helper.o
 
+drm_ttm_helper-y := drm_gem_ttm_helper.o
+obj-$(CONFIG_DRM_TTM_HELPER) += drm_ttm_helper.o
+
 drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_dsc.o drm_probe_helper.o \
 		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
 		drm_kms_helper_common.o drm_dp_dual_mode_helper.o \
-- 
2.18.1

[PATCH v3 4/8] drm/vram: switch vram helpers to the new gem_ttm_bo_device_init()

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/drm/drm_gem_vram_helper.h             |  6 +--
 drivers/gpu/drm/drm_gem_vram_helper.c         | 48 -------------------
 drivers/gpu/drm/drm_vram_mm_helper.c          |  6 +--
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   |  1 -
 drivers/gpu/drm/Kconfig                       |  1 +
 5 files changed, 5 insertions(+), 57 deletions(-)

diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h
index ac217d768456..701d2c46a4f4 100644
--- a/include/drm/drm_gem_vram_helper.h
+++ b/include/drm/drm_gem_vram_helper.h
@@ -4,6 +4,7 @@
 #define DRM_GEM_VRAM_HELPER_H
 
 #include <drm/drm_gem.h>
+#include <drm/drm_gem_ttm_helper.h>
 #include <drm/ttm/ttm_bo_api.h>
 #include <drm/ttm/ttm_placement.h>
 #include <linux/kernel.h> /* for container_of() */
@@ -76,7 +77,6 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
 						unsigned long pg_align,
 						bool interruptible);
 void drm_gem_vram_put(struct drm_gem_vram_object *gbo);
-u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object *gbo);
 s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo);
 int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, unsigned long pl_flag);
 int drm_gem_vram_unpin(struct drm_gem_vram_object *gbo);
@@ -110,9 +110,6 @@ extern const struct drm_vram_mm_funcs drm_gem_vram_mm_funcs;
 int drm_gem_vram_driver_dumb_create(struct drm_file *file,
 				    struct drm_device *dev,
 				    struct drm_mode_create_dumb *args);
-int drm_gem_vram_driver_dumb_mmap_offset(struct drm_file *file,
-					 struct drm_device *dev,
-					 uint32_t handle, uint64_t *offset);
 
 /**
  * define DRM_GEM_VRAM_DRIVER - default callback functions for \
@@ -123,7 +120,6 @@ int drm_gem_vram_driver_dumb_mmap_offset(struct drm_file *file,
  */
 #define DRM_GEM_VRAM_DRIVER \
 	.dumb_create		  = drm_gem_vram_driver_dumb_create, \
-	.dumb_map_offset	  = drm_gem_vram_driver_dumb_mmap_offset, \
 	.gem_prime_mmap		  = drm_gem_prime_mmap
 
 #endif
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index fd751078bae1..b78865055990 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -156,22 +156,6 @@ void drm_gem_vram_put(struct drm_gem_vram_object *gbo)
 }
 EXPORT_SYMBOL(drm_gem_vram_put);
 
-/**
- * drm_gem_vram_mmap_offset() - Returns a GEM VRAM object's mmap offset
- * @gbo:	the GEM VRAM object
- *
- * See drm_vma_node_offset_addr() for more information.
- *
- * Returns:
- * The buffer object's offset for userspace mappings on success, or
- * 0 if no offset is allocated.
- */
-u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object *gbo)
-{
-	return drm_vma_node_offset_addr(&gbo->bo.base.vma_node);
-}
-EXPORT_SYMBOL(drm_gem_vram_mmap_offset);
-
 /**
  * drm_gem_vram_offset() - \
 	Returns a GEM VRAM object's offset in video memory
@@ -511,38 +495,6 @@ int drm_gem_vram_driver_dumb_create(struct drm_file *file,
 }
 EXPORT_SYMBOL(drm_gem_vram_driver_dumb_create);
 
-/**
- * drm_gem_vram_driver_dumb_mmap_offset() - \
-	Implements &struct drm_driver.dumb_mmap_offset
- * @file:	DRM file pointer.
- * @dev:	DRM device.
- * @handle:	GEM handle
- * @offset:	Returns the mapping's memory offset on success
- *
- * Returns:
- * 0 on success, or
- * a negative errno code otherwise.
- */
-int drm_gem_vram_driver_dumb_mmap_offset(struct drm_file *file,
-					 struct drm_device *dev,
-					 uint32_t handle, uint64_t *offset)
-{
-	struct drm_gem_object *gem;
-	struct drm_gem_vram_object *gbo;
-
-	gem = drm_gem_object_lookup(file, handle);
-	if (!gem)
-		return -ENOENT;
-
-	gbo = drm_gem_vram_of_gem(gem);
-	*offset = drm_gem_vram_mmap_offset(gbo);
-
-	drm_gem_object_put_unlocked(gem);
-
-	return 0;
-}
-EXPORT_SYMBOL(drm_gem_vram_driver_dumb_mmap_offset);
-
 /*
  * PRIME helpers
  */
diff --git a/drivers/gpu/drm/drm_vram_mm_helper.c b/drivers/gpu/drm/drm_vram_mm_helper.c
index c911781d6728..a693f9ce356c 100644
--- a/drivers/gpu/drm/drm_vram_mm_helper.c
+++ b/drivers/gpu/drm/drm_vram_mm_helper.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 
+#include <drm/drm_gem_ttm_helper.h>
 #include <drm/drm_device.h>
 #include <drm/drm_file.h>
 #include <drm/drm_vram_mm_helper.h>
@@ -170,9 +171,8 @@ int drm_vram_mm_init(struct drm_vram_mm *vmm, struct drm_device *dev,
 	vmm->vram_size = vram_size;
 	vmm->funcs = funcs;
 
-	ret = ttm_bo_device_init(&vmm->bdev, &bo_driver,
-				 dev->anon_inode->i_mapping,
-				 true);
+	ret = drm_gem_ttm_bo_device_init(dev, &vmm->bdev, &bo_driver,
+					 true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index 2ae538835781..6386c67e086b 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -59,7 +59,6 @@ static struct drm_driver hibmc_driver = {
 	.major			= 1,
 	.minor			= 0,
 	.dumb_create            = hibmc_dumb_create,
-	.dumb_map_offset        = drm_gem_vram_driver_dumb_mmap_offset,
 	.gem_prime_mmap		= drm_gem_prime_mmap,
 	.irq_handler		= hibmc_drm_interrupt,
 };
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index f7b25519f95c..1be8ad30d8fe 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -169,6 +169,7 @@ config DRM_VRAM_HELPER
 	tristate
 	depends on DRM
 	select DRM_TTM
+	select DRM_TTM_HELPER
 	help
 	  Helpers for VRAM memory management
 
-- 
2.18.1

[PATCH v3 5/8] drm/qxl: switch qxl to the new gem_ttm_bo_device_init()

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/qxl/qxl_drv.h    |  4 +---
 drivers/gpu/drm/qxl/qxl_object.h |  5 -----
 drivers/gpu/drm/qxl/qxl_drv.c    |  1 -
 drivers/gpu/drm/qxl/qxl_dumb.c   | 17 -----------------
 drivers/gpu/drm/qxl/qxl_ioctl.c  |  5 +++--
 drivers/gpu/drm/qxl/qxl_ttm.c    |  8 ++++----
 drivers/gpu/drm/qxl/Kconfig      |  1 +
 7 files changed, 9 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 9e034c5fa87d..82efbe76062a 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -38,6 +38,7 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_encoder.h>
 #include <drm/drm_fb_helper.h>
+#include <drm/drm_gem_ttm_helper.h>
 #include <drm/drm_ioctl.h>
 #include <drm/drm_gem.h>
 #include <drm/qxl_drm.h>
@@ -347,9 +348,6 @@ int qxl_bo_kmap(struct qxl_bo *bo, void **ptr);
 int qxl_mode_dumb_create(struct drm_file *file_priv,
 			 struct drm_device *dev,
 			 struct drm_mode_create_dumb *args);
-int qxl_mode_dumb_mmap(struct drm_file *filp,
-		       struct drm_device *dev,
-		       uint32_t handle, uint64_t *offset_p);
 
 /* qxl ttm */
 int qxl_ttm_init(struct qxl_device *qdev);
diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h
index 8ae54ba7857c..1f0316ebcfd0 100644
--- a/drivers/gpu/drm/qxl/qxl_object.h
+++ b/drivers/gpu/drm/qxl/qxl_object.h
@@ -58,11 +58,6 @@ static inline unsigned long qxl_bo_size(struct qxl_bo *bo)
 	return bo->tbo.num_pages << PAGE_SHIFT;
 }
 
-static inline u64 qxl_bo_mmap_offset(struct qxl_bo *bo)
-{
-	return drm_vma_node_offset_addr(&bo->tbo.base.vma_node);
-}
-
 static inline int qxl_bo_wait(struct qxl_bo *bo, u32 *mem_type,
 			      bool no_wait)
 {
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index c1802e01d9f6..12cf85a06bed 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -252,7 +252,6 @@ static struct drm_driver qxl_driver = {
 	.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
 
 	.dumb_create = qxl_mode_dumb_create,
-	.dumb_map_offset = qxl_mode_dumb_mmap,
 #if defined(CONFIG_DEBUG_FS)
 	.debugfs_init = qxl_debugfs_init,
 #endif
diff --git a/drivers/gpu/drm/qxl/qxl_dumb.c b/drivers/gpu/drm/qxl/qxl_dumb.c
index 272d19b677d8..bd3b16a701a6 100644
--- a/drivers/gpu/drm/qxl/qxl_dumb.c
+++ b/drivers/gpu/drm/qxl/qxl_dumb.c
@@ -69,20 +69,3 @@ int qxl_mode_dumb_create(struct drm_file *file_priv,
 	args->handle = handle;
 	return 0;
 }
-
-int qxl_mode_dumb_mmap(struct drm_file *file_priv,
-		       struct drm_device *dev,
-		       uint32_t handle, uint64_t *offset_p)
-{
-	struct drm_gem_object *gobj;
-	struct qxl_bo *qobj;
-
-	BUG_ON(!offset_p);
-	gobj = drm_gem_object_lookup(file_priv, handle);
-	if (gobj == NULL)
-		return -ENOENT;
-	qobj = gem_to_qxl_bo(gobj);
-	*offset_p = qxl_bo_mmap_offset(qobj);
-	drm_gem_object_put_unlocked(gobj);
-	return 0;
-}
diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index 8117a45b3610..c8d9ea552532 100644
--- a/drivers/gpu/drm/qxl/qxl_ioctl.c
+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
@@ -67,8 +67,9 @@ static int qxl_map_ioctl(struct drm_device *dev, void *data,
 	struct qxl_device *qdev = dev->dev_private;
 	struct drm_qxl_map *qxl_map = data;
 
-	return qxl_mode_dumb_mmap(file_priv, &qdev->ddev, qxl_map->handle,
-				  &qxl_map->offset);
+	return drm_gem_dumb_map_offset(file_priv, &qdev->ddev,
+				       qxl_map->handle,
+				       &qxl_map->offset);
 }
 
 struct qxl_reloc_info {
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 9b24514c75aa..3a24145dd516 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -322,10 +322,10 @@ int qxl_ttm_init(struct qxl_device *qdev)
 	int num_io_pages; /* != rom->num_io_pages, we include surface0 */
 
 	/* No others user of address space so set it to 0 */
-	r = ttm_bo_device_init(&qdev->mman.bdev,
-			       &qxl_bo_driver,
-			       qdev->ddev.anon_inode->i_mapping,
-			       false);
+	r = drm_gem_ttm_bo_device_init(&qdev->ddev,
+				       &qdev->mman.bdev,
+				       &qxl_bo_driver,
+				       false);
 	if (r) {
 		DRM_ERROR("failed initializing buffer object driver(%d).\n", r);
 		return r;
diff --git a/drivers/gpu/drm/qxl/Kconfig b/drivers/gpu/drm/qxl/Kconfig
index d0d691b31f4a..bfe90c7d17b2 100644
--- a/drivers/gpu/drm/qxl/Kconfig
+++ b/drivers/gpu/drm/qxl/Kconfig
@@ -3,6 +3,7 @@ config DRM_QXL
 	tristate "QXL virtual GPU"
 	depends on DRM && PCI && MMU
 	select DRM_KMS_HELPER
+	select DRM_TTM_HELPER
 	select DRM_TTM
 	select CRC32
 	help
-- 
2.18.1

[PATCH v3 6/8] drm/ttm: add drm_gem_ttm_bo_driver_verify_access()

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/drm/drm_gem_ttm_helper.h     |  2 ++
 drivers/gpu/drm/drm_gem_ttm_helper.c | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/include/drm/drm_gem_ttm_helper.h b/include/drm/drm_gem_ttm_helper.h
index 43c9db3583cc..78e4d8930fec 100644
--- a/include/drm/drm_gem_ttm_helper.h
+++ b/include/drm/drm_gem_ttm_helper.h
@@ -26,5 +26,7 @@ int drm_gem_ttm_bo_device_init(struct drm_device *dev,
 			       struct ttm_bo_device *bdev,
 			       struct ttm_bo_driver *driver,
 			       bool need_dma32);
+int drm_gem_ttm_bo_driver_verify_access(struct ttm_buffer_object *bo,
+					struct file *filp);
 
 #endif
diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c b/drivers/gpu/drm/drm_gem_ttm_helper.c
index 0c57e9fd50b9..159768c7ec63 100644
--- a/drivers/gpu/drm/drm_gem_ttm_helper.c
+++ b/drivers/gpu/drm/drm_gem_ttm_helper.c
@@ -34,3 +34,25 @@ int drm_gem_ttm_bo_device_init(struct drm_device *dev,
 						   need_dma32);
 }
 EXPORT_SYMBOL(drm_gem_ttm_bo_device_init);
+
+/**
+ * drm_gem_ttm_bo_driver_verify_access() - \
+	Implements &struct ttm_bo_driver.verify_access
+ * @bo:		TTM buffer object.
+ * @filp:	File pointer.
+ *
+ * This function assumes filp->private_data refers to the
+ * corresponding &struct drm_file.
+ *
+ * Returns:
+ * 0 on success, or
+ * a negative errno code otherwise.
+ */
+int drm_gem_ttm_bo_driver_verify_access(struct ttm_buffer_object *bo,
+					struct file *filp)
+{
+	struct drm_file *ptr = filp->private_data;
+
+	return drm_vma_node_verify_access(&bo->base.vma_node, ptr);
+}
+EXPORT_SYMBOL(drm_gem_ttm_bo_driver_verify_access);
-- 
2.18.1

[PATCH v3 7/8] gem/vram: use drm_gem_ttm_bo_driver_verify_access()

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/drm/drm_gem_vram_helper.h     |  3 ---
 drivers/gpu/drm/drm_gem_vram_helper.c | 22 +---------------------
 2 files changed, 1 insertion(+), 24 deletions(-)

diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h
index 701d2c46a4f4..7723ad59a0c5 100644
--- a/include/drm/drm_gem_vram_helper.h
+++ b/include/drm/drm_gem_vram_helper.h
@@ -98,9 +98,6 @@ int drm_gem_vram_fill_create_dumb(struct drm_file *file,
 void drm_gem_vram_bo_driver_evict_flags(struct ttm_buffer_object *bo,
 					struct ttm_placement *pl);
 
-int drm_gem_vram_bo_driver_verify_access(struct ttm_buffer_object *bo,
-					 struct file *filp);
-
 extern const struct drm_vram_mm_funcs drm_gem_vram_mm_funcs;
 
 /*
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index b78865055990..02d9cf0272bc 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -415,26 +415,6 @@ void drm_gem_vram_bo_driver_evict_flags(struct ttm_buffer_object *bo,
 }
 EXPORT_SYMBOL(drm_gem_vram_bo_driver_evict_flags);
 
-/**
- * drm_gem_vram_bo_driver_verify_access() - \
-	Implements &struct ttm_bo_driver.verify_access
- * @bo:		TTM buffer object. Refers to &struct drm_gem_vram_object.bo
- * @filp:	File pointer.
- *
- * Returns:
- * 0 on success, or
- * a negative errno code otherwise.
- */
-int drm_gem_vram_bo_driver_verify_access(struct ttm_buffer_object *bo,
-					 struct file *filp)
-{
-	struct drm_gem_vram_object *gbo = drm_gem_vram_of_bo(bo);
-
-	return drm_vma_node_verify_access(&gbo->bo.base.vma_node,
-					  filp->private_data);
-}
-EXPORT_SYMBOL(drm_gem_vram_bo_driver_verify_access);
-
 /*
  * drm_gem_vram_mm_funcs - Functions for &struct drm_vram_mm
  *
@@ -444,7 +424,7 @@ EXPORT_SYMBOL(drm_gem_vram_bo_driver_verify_access);
  */
 const struct drm_vram_mm_funcs drm_gem_vram_mm_funcs = {
 	.evict_flags = drm_gem_vram_bo_driver_evict_flags,
-	.verify_access = drm_gem_vram_bo_driver_verify_access
+	.verify_access = drm_gem_ttm_bo_driver_verify_access
 };
 EXPORT_SYMBOL(drm_gem_vram_mm_funcs);
 
-- 
2.18.1

[PATCH v3 8/8] gem/qxl: use drm_gem_ttm_bo_driver_verify_access()

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/qxl/qxl_ttm.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 3a24145dd516..bcf48b062a85 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -151,14 +151,6 @@ static void qxl_evict_flags(struct ttm_buffer_object *bo,
 	*placement = qbo->placement;
 }
 
-static int qxl_verify_access(struct ttm_buffer_object *bo, struct file *filp)
-{
-	struct qxl_bo *qbo = to_qxl_bo(bo);
-
-	return drm_vma_node_verify_access(&qbo->tbo.base.vma_node,
-					  filp->private_data);
-}
-
 static int qxl_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
 				  struct ttm_mem_reg *mem)
 {
@@ -310,7 +302,7 @@ static struct ttm_bo_driver qxl_bo_driver = {
 	.eviction_valuable = ttm_bo_eviction_valuable,
 	.evict_flags = &qxl_evict_flags,
 	.move = &qxl_bo_move,
-	.verify_access = &qxl_verify_access,
+	.verify_access = drm_gem_ttm_bo_driver_verify_access,
 	.io_mem_reserve = &qxl_ttm_io_mem_reserve,
 	.io_mem_free = &qxl_ttm_io_mem_free,
 	.move_notify = &qxl_bo_move_notify,
-- 
2.18.1

Re: [PATCH v3 2/8] ttm: turn ttm_bo_device.vma_manager into a pointer

[Koenig, Christian]

Am 08.08.19 um 11:36 schrieb Gerd Hoffmann:
> Rename the embedded struct vma_offset_manager, it is named _vma_manager
> now.  ttm_bo_device.vma_manager is a pointer now, pointing to the
> embedded ttm_bo_device._vma_manager by default.
>
> Add ttm_bo_device_init_with_vma_manager() function which allows to
> initialize ttm with a different vma manager.

Can't we go down the route of completely removing the vma_manager from 
TTM? ttm_bo_mmap() would get the BO as parameter instead.

That would also make the verify_access callback completely superfluous 
and looks like a good step into the right direction of de-midlayering.

Christian.

>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   include/drm/ttm/ttm_bo_driver.h | 11 +++++++++--
>   drivers/gpu/drm/ttm/ttm_bo.c    | 29 +++++++++++++++++++++--------
>   drivers/gpu/drm/ttm/ttm_bo_vm.c |  6 +++---
>   3 files changed, 33 insertions(+), 13 deletions(-)
>
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 3f1935c19a66..2f84d6bcd1a7 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -441,7 +441,8 @@ extern struct ttm_bo_global {
>    *
>    * @driver: Pointer to a struct ttm_bo_driver struct setup by the driver.
>    * @man: An array of mem_type_managers.
> - * @vma_manager: Address space manager
> + * @vma_manager: Address space manager (pointer)
> + * @_vma_manager: Address space manager (enbedded)
>    * lru_lock: Spinlock that protects the buffer+device lru lists and
>    * ddestroy lists.
>    * @dev_mapping: A pointer to the struct address_space representing the
> @@ -464,7 +465,8 @@ struct ttm_bo_device {
>   	/*
>   	 * Protected by internal locks.
>   	 */
> -	struct drm_vma_offset_manager vma_manager;
> +	struct drm_vma_offset_manager *vma_manager;
> +	struct drm_vma_offset_manager _vma_manager;
>   
>   	/*
>   	 * Protected by the global:lru lock.
> @@ -597,6 +599,11 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
>   		       struct ttm_bo_driver *driver,
>   		       struct address_space *mapping,
>   		       bool need_dma32);
> +int ttm_bo_device_init_with_vma_manager(struct ttm_bo_device *bdev,
> +					struct ttm_bo_driver *driver,
> +					struct address_space *mapping,
> +					struct drm_vma_offset_manager *vma_manager,
> +					bool need_dma32);
>   
>   /**
>    * ttm_bo_unmap_virtual
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 10a861a1690c..0ed1a1182962 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -672,7 +672,7 @@ static void ttm_bo_release(struct kref *kref)
>   	struct ttm_bo_device *bdev = bo->bdev;
>   	struct ttm_mem_type_manager *man = &bdev->man[bo->mem.mem_type];
>   
> -	drm_vma_offset_remove(&bdev->vma_manager, &bo->base.vma_node);
> +	drm_vma_offset_remove(bdev->vma_manager, &bo->base.vma_node);
>   	ttm_mem_io_lock(man, false);
>   	ttm_mem_io_free_vm(bo);
>   	ttm_mem_io_unlock(man);
> @@ -1353,7 +1353,7 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev,
>   	 */
>   	if (bo->type == ttm_bo_type_device ||
>   	    bo->type == ttm_bo_type_sg)
> -		ret = drm_vma_offset_add(&bdev->vma_manager, &bo->base.vma_node,
> +		ret = drm_vma_offset_add(bdev->vma_manager, &bo->base.vma_node,
>   					 bo->mem.num_pages);
>   
>   	/* passed reservation objects should already be locked,
> @@ -1704,7 +1704,7 @@ int ttm_bo_device_release(struct ttm_bo_device *bdev)
>   			pr_debug("Swap list %d was clean\n", i);
>   	spin_unlock(&glob->lru_lock);
>   
> -	drm_vma_offset_manager_destroy(&bdev->vma_manager);
> +	drm_vma_offset_manager_destroy(&bdev->_vma_manager);
>   
>   	if (!ret)
>   		ttm_bo_global_release();
> @@ -1713,10 +1713,11 @@ int ttm_bo_device_release(struct ttm_bo_device *bdev)
>   }
>   EXPORT_SYMBOL(ttm_bo_device_release);
>   
> -int ttm_bo_device_init(struct ttm_bo_device *bdev,
> -		       struct ttm_bo_driver *driver,
> -		       struct address_space *mapping,
> -		       bool need_dma32)
> +int ttm_bo_device_init_with_vma_manager(struct ttm_bo_device *bdev,
> +					struct ttm_bo_driver *driver,
> +					struct address_space *mapping,
> +					struct drm_vma_offset_manager *vma_manager,
> +					bool need_dma32)
>   {
>   	struct ttm_bo_global *glob = &ttm_bo_glob;
>   	int ret;
> @@ -1737,7 +1738,8 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
>   	if (unlikely(ret != 0))
>   		goto out_no_sys;
>   
> -	drm_vma_offset_manager_init(&bdev->vma_manager,
> +	bdev->vma_manager = vma_manager;
> +	drm_vma_offset_manager_init(&bdev->_vma_manager,
>   				    DRM_FILE_PAGE_OFFSET_START,
>   				    DRM_FILE_PAGE_OFFSET_SIZE);
>   	INIT_DELAYED_WORK(&bdev->wq, ttm_bo_delayed_workqueue);
> @@ -1754,6 +1756,17 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
>   	ttm_bo_global_release();
>   	return ret;
>   }
> +EXPORT_SYMBOL(ttm_bo_device_init_with_vma_manager);
> +
> +int ttm_bo_device_init(struct ttm_bo_device *bdev,
> +		       struct ttm_bo_driver *driver,
> +		       struct address_space *mapping,
> +		       bool need_dma32)
> +{
> +	return ttm_bo_device_init_with_vma_manager(bdev, driver, mapping,
> +						   &bdev->_vma_manager,
> +						   need_dma32);
> +}
>   EXPORT_SYMBOL(ttm_bo_device_init);
>   
>   /*
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> index 85f5bcbe0c76..d4eecde8d050 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> @@ -409,16 +409,16 @@ static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_bo_device *bdev,
>   	struct drm_vma_offset_node *node;
>   	struct ttm_buffer_object *bo = NULL;
>   
> -	drm_vma_offset_lock_lookup(&bdev->vma_manager);
> +	drm_vma_offset_lock_lookup(bdev->vma_manager);
>   
> -	node = drm_vma_offset_lookup_locked(&bdev->vma_manager, offset, pages);
> +	node = drm_vma_offset_lookup_locked(bdev->vma_manager, offset, pages);
>   	if (likely(node)) {
>   		bo = container_of(node, struct ttm_buffer_object,
>   				  base.vma_node);
>   		bo = ttm_bo_get_unless_zero(bo);
>   	}
>   
> -	drm_vma_offset_unlock_lookup(&bdev->vma_manager);
> +	drm_vma_offset_unlock_lookup(bdev->vma_manager);
>   
>   	if (!bo)
>   		pr_err("Could not find buffer object to map\n");

Re: [PATCH v3 1/8] scsi: core: fix the dma_max_mapping_size call

[Gerd Hoffmann]

On Thu, Aug 08, 2019 at 11:36:55AM +0200, Gerd Hoffmann wrote:
> From: Christoph Hellwig <hch@lst.de>
> 
> We should only call dma_max_mapping_size for devices that have a DMA mask
> set, otherwise we can run into a NULL pointer dereference that will crash
> the system.
> 
> Also we need to do right shift to get the sectors from the size in bytes,
> not a left shift.

Oops, that wasn't meant to be re-sent, sorry.

drm-misc-next maintainers: any chance for a backmerge to pick up this fix,
so I don't have to carry it in my branches?

thanks,
  Gerd

Re: [PATCH v3 2/8] ttm: turn ttm_bo_device.vma_manager into a pointer

[Gerd Hoffmann]

On Thu, Aug 08, 2019 at 09:48:49AM +0000, Koenig, Christian wrote:
> Am 08.08.19 um 11:36 schrieb Gerd Hoffmann:
> > Rename the embedded struct vma_offset_manager, it is named _vma_manager
> > now.  ttm_bo_device.vma_manager is a pointer now, pointing to the
> > embedded ttm_bo_device._vma_manager by default.
> >
> > Add ttm_bo_device_init_with_vma_manager() function which allows to
> > initialize ttm with a different vma manager.
> 
> Can't we go down the route of completely removing the vma_manager from 
> TTM? ttm_bo_mmap() would get the BO as parameter instead.

It surely makes sense to target that.  This patch can be a first step
into that direction.  It allows gem and ttm to use the same
vma_offset_manager (see patch #3), which in turn makes various gem
functions work on ttm objects (see patch #4 for vram helpers).

> That would also make the verify_access callback completely superfluous 
> and looks like a good step into the right direction of de-midlayering.

Hmm, right, noticed that too while working on another patch series.
Guess I'll try to merge those two and see where I end up ...

cheers,
  Gerd

Re: [PATCH v3 2/8] ttm: turn ttm_bo_device.vma_manager into a pointer

[Daniel Vetter]

On Thu, Aug 08, 2019 at 12:35:21PM +0200, Gerd Hoffmann wrote:
> On Thu, Aug 08, 2019 at 09:48:49AM +0000, Koenig, Christian wrote:
> > Am 08.08.19 um 11:36 schrieb Gerd Hoffmann:
> > > Rename the embedded struct vma_offset_manager, it is named _vma_manager
> > > now.  ttm_bo_device.vma_manager is a pointer now, pointing to the
> > > embedded ttm_bo_device._vma_manager by default.
> > >
> > > Add ttm_bo_device_init_with_vma_manager() function which allows to
> > > initialize ttm with a different vma manager.
> > 
> > Can't we go down the route of completely removing the vma_manager from 
> > TTM? ttm_bo_mmap() would get the BO as parameter instead.
> 
> It surely makes sense to target that.  This patch can be a first step
> into that direction.  It allows gem and ttm to use the same
> vma_offset_manager (see patch #3), which in turn makes various gem
> functions work on ttm objects (see patch #4 for vram helpers).

+1 on cleaning this up for good, at least long-term ...

> > That would also make the verify_access callback completely superfluous 
> > and looks like a good step into the right direction of de-midlayering.
> 
> Hmm, right, noticed that too while working on another patch series.
> Guess I'll try to merge those two and see where I end up ...

... but if it gets too invasive I'd vote for incremental changes. Even if
we completely rip out the vma/mmap lookup stuff from ttm, we still need to
keep a copy somewhere for vmwgfx. Or would the evil plan be the vmwgfx
would use the gem mmap helpers too?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v3 2/8] ttm: turn ttm_bo_device.vma_manager into a pointer

[Thomas Hellström (VMware)]

On 8/8/19 2:02 PM, Daniel Vetter wrote:
> On Thu, Aug 08, 2019 at 12:35:21PM +0200, Gerd Hoffmann wrote:
>> On Thu, Aug 08, 2019 at 09:48:49AM +0000, Koenig, Christian wrote:
>>> Am 08.08.19 um 11:36 schrieb Gerd Hoffmann:
>>>> Rename the embedded struct vma_offset_manager, it is named _vma_manager
>>>> now.  ttm_bo_device.vma_manager is a pointer now, pointing to the
>>>> embedded ttm_bo_device._vma_manager by default.
>>>>
>>>> Add ttm_bo_device_init_with_vma_manager() function which allows to
>>>> initialize ttm with a different vma manager.
>>> Can't we go down the route of completely removing the vma_manager from
>>> TTM? ttm_bo_mmap() would get the BO as parameter instead.
>> It surely makes sense to target that.  This patch can be a first step
>> into that direction.  It allows gem and ttm to use the same
>> vma_offset_manager (see patch #3), which in turn makes various gem
>> functions work on ttm objects (see patch #4 for vram helpers).
> +1 on cleaning this up for good, at least long-term ...
>
>>> That would also make the verify_access callback completely superfluous
>>> and looks like a good step into the right direction of de-midlayering.
>> Hmm, right, noticed that too while working on another patch series.
>> Guess I'll try to merge those two and see where I end up ...
> ... but if it gets too invasive I'd vote for incremental changes. Even if
> we completely rip out the vma/mmap lookup stuff from ttm, we still need to
> keep a copy somewhere for vmwgfx. Or would the evil plan be the vmwgfx
> would use the gem mmap helpers too?

I don't think it would be too invasive. We could simply move 
ttm_bo_vm_lookup into a vmw_mmap.

/Thomas




> -Daniel

Re: [PATCH v3 2/8] ttm: turn ttm_bo_device.vma_manager into a pointer

[Koenig, Christian]

Am 08.08.19 um 14:43 schrieb Thomas Hellström (VMware):
> On 8/8/19 2:02 PM, Daniel Vetter wrote:
>> On Thu, Aug 08, 2019 at 12:35:21PM +0200, Gerd Hoffmann wrote:
>>> On Thu, Aug 08, 2019 at 09:48:49AM +0000, Koenig, Christian wrote:
>>>> Am 08.08.19 um 11:36 schrieb Gerd Hoffmann:
>>>>> Rename the embedded struct vma_offset_manager, it is named 
>>>>> _vma_manager
>>>>> now.  ttm_bo_device.vma_manager is a pointer now, pointing to the
>>>>> embedded ttm_bo_device._vma_manager by default.
>>>>>
>>>>> Add ttm_bo_device_init_with_vma_manager() function which allows to
>>>>> initialize ttm with a different vma manager.
>>>> Can't we go down the route of completely removing the vma_manager from
>>>> TTM? ttm_bo_mmap() would get the BO as parameter instead.
>>> It surely makes sense to target that.  This patch can be a first step
>>> into that direction.  It allows gem and ttm to use the same
>>> vma_offset_manager (see patch #3), which in turn makes various gem
>>> functions work on ttm objects (see patch #4 for vram helpers).
>> +1 on cleaning this up for good, at least long-term ...
>>
>>>> That would also make the verify_access callback completely superfluous
>>>> and looks like a good step into the right direction of de-midlayering.
>>> Hmm, right, noticed that too while working on another patch series.
>>> Guess I'll try to merge those two and see where I end up ...
>> ... but if it gets too invasive I'd vote for incremental changes. 
>> Even if
>> we completely rip out the vma/mmap lookup stuff from ttm, we still 
>> need to
>> keep a copy somewhere for vmwgfx. Or would the evil plan be the vmwgfx
>> would use the gem mmap helpers too?
>
> I don't think it would be too invasive. We could simply move 
> ttm_bo_vm_lookup into a vmw_mmap.

Yeah, agree. vmwgfx would just inherit what TTM is currently doing and 
everybody else would start to use the GEM helpers.

Switching the vma_manager to a pointer might be helpful in the middle of 
the patch set, but as stand a lone change that looks like a detour to me.

I suggest to start by adding the bo as parameter to ttm_bo_mmap and 
moving the lockup out of that function.

Christian.

>
> /Thomas
>
>
>
>
>> -Daniel
>
>

Re: [PATCH v3 2/8] ttm: turn ttm_bo_device.vma_manager into a pointer

[Gerd Hoffmann]

  Hi,

> > > That would also make the verify_access callback completely superfluous 
> > > and looks like a good step into the right direction of de-midlayering.
> > 
> > Hmm, right, noticed that too while working on another patch series.
> > Guess I'll try to merge those two and see where I end up ...
> 
> ... but if it gets too invasive I'd vote for incremental changes.

Yep, this is what I'm up to.  Sketching things up with vram helpers and
qxl, in a way that we can switch over drivers one by one.

Once all drivers are switched removing ttm_bo_device.vma_manager
altogether should be easy.

> Even if
> we completely rip out the vma/mmap lookup stuff from ttm, we still need to
> keep a copy somewhere for vmwgfx.

If vmwgfx is the only user we can probably just move things from ttm to
vmwgfx.

> Or would the evil plan be the vmwgfx
> would use the gem mmap helpers too?

That would work as well ;)

cheers,
  Gerd