Gerd Hoffmann (17): drm/ttm: turn ttm_bo_device.vma_manager into a pointer drm/ttm: add gem_ttm_bo_device_init() drm/vram: switch vram helpers to the new gem_ttm_bo_device_init() drm/qxl: switch qxl to the new gem_ttm_bo_device_init() drm: add mmap() to drm_gem_object_funcs drm/shmem: switch shmem helper to drm_gem_object_funcs->mmap drm/shmem: drop DEFINE_DRM_GEM_SHMEM_FOPS drm/ttm: factor out ttm_bo_mmap_vma_setup drm/ttm: add drm_gem_ttm_mmap() drm/vram: switch vram helper to drm_gem_object_funcs->mmap codepath drm/vram: drop verify_access drm: drop DRM_VRAM_MM_FILE_OPERATIONS drm/qxl: use drm_gem_object_funcs drm/qxl: drop qxl_ttm_fault drm/qxl: switch qxl to drm_gem_object_funcs->mmap codepath drm/qxl: drop verify_access drm/qxl: use DEFINE_DRM_GEM_FOPS()
drivers/gpu/drm/qxl/qxl_drv.h | 5 +- drivers/gpu/drm/qxl/qxl_object.h | 5 -- include/drm/drm_gem.h | 9 +++ include/drm/drm_gem_shmem_helper.h | 28 +-------- include/drm/drm_gem_ttm_helper.h | 32 ++++++++++ include/drm/drm_gem_vram_helper.h | 9 +-- include/drm/drm_vram_mm_helper.h | 27 --------- include/drm/ttm/ttm_bo_api.h | 8 +++ include/drm/ttm/ttm_bo_driver.h | 11 +++- drivers/gpu/drm/ast/ast_drv.c | 5 +- drivers/gpu/drm/bochs/bochs_drv.c | 5 +- drivers/gpu/drm/cirrus/cirrus.c | 2 +- drivers/gpu/drm/drm_gem.c | 6 ++ drivers/gpu/drm/drm_gem_shmem_helper.c | 18 +++--- drivers/gpu/drm/drm_gem_ttm_helper.c | 47 +++++++++++++++ drivers/gpu/drm/drm_gem_vram_helper.c | 53 +---------------- drivers/gpu/drm/drm_vram_mm_helper.c | 44 +------------- .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 6 +- drivers/gpu/drm/mgag200/mgag200_drv.c | 5 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- drivers/gpu/drm/panfrost/panfrost_gem.c | 1 + drivers/gpu/drm/qxl/qxl_drv.c | 19 +----- drivers/gpu/drm/qxl/qxl_dumb.c | 17 ------ drivers/gpu/drm/qxl/qxl_ioctl.c | 5 +- drivers/gpu/drm/qxl/qxl_object.c | 13 +++++ drivers/gpu/drm/qxl/qxl_ttm.c | 58 ++----------------- drivers/gpu/drm/ttm/ttm_bo.c | 29 +++++++--- drivers/gpu/drm/ttm/ttm_bo_vm.c | 53 +++++++++-------- drivers/gpu/drm/v3d/v3d_bo.c | 1 + drivers/gpu/drm/v3d/v3d_drv.c | 2 +- drivers/gpu/drm/vboxvideo/vbox_drv.c | 5 +- Documentation/gpu/drm-mm.rst | 12 ++++ drivers/gpu/drm/Kconfig | 8 +++ drivers/gpu/drm/Makefile | 3 + drivers/gpu/drm/qxl/Kconfig | 1 + 35 files changed, 231 insertions(+), 323 deletions(-) create mode 100644 include/drm/drm_gem_ttm_helper.h create mode 100644 drivers/gpu/drm/drm_gem_ttm_helper.c
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");
On Thu, Aug 08, 2019 at 03:44:01PM +0200, Gerd Hoffmann wrote:
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
I think a todo to convert all other ttm drivers over and then move _vma_manager into vmwgfx would be nice. If you're not volunteering yourself for this ofc. -Daniel
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
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 \
This allows to drop drm_gem_vram_mmap_offset() and drm_gem_vram_driver_dumb_mmap_offset(), the default gem function works just fine.
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
This allows to drop qxl_mode_dumb_mmap() and qxl_bo_mmap_offset(), the default gem function works just fine.
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
drm_gem_object_funcs->vm_ops alone can't handle everything mmap() needs. Add a new callback for it.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- include/drm/drm_gem.h | 9 +++++++++ drivers/gpu/drm/drm_gem.c | 6 ++++++ 2 files changed, 15 insertions(+)
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index ae693c0666cd..ee3c4ad742c6 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -150,6 +150,15 @@ struct drm_gem_object_funcs { */ void (*vunmap)(struct drm_gem_object *obj, void *vaddr);
+ /** + * @mmap: + * + * Called by drm_gem_mmap() for additional checks/setup. + * + * This callback is optional. + */ + int (*mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma); + /** * @vm_ops: * diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index afc38cece3f5..84db8de217e1 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1105,6 +1105,8 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, vma->vm_ops = obj->funcs->vm_ops; else if (dev->driver->gem_vm_ops) vma->vm_ops = dev->driver->gem_vm_ops; + else if (obj->funcs && obj->funcs->mmap) + /* obj->funcs->mmap must set vma->vm_ops */; else return -EINVAL;
@@ -1192,6 +1194,10 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node) << PAGE_SHIFT, vma);
+ if (ret == 0) + if (obj->funcs->mmap) + ret = obj->funcs->mmap(obj, vma); + drm_gem_object_put_unlocked(obj);
return ret;
On Thu, Aug 08, 2019 at 03:44:05PM +0200, Gerd Hoffmann wrote:
drm_gem_object_funcs->vm_ops alone can't handle everything mmap() needs. Add a new callback for it.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
include/drm/drm_gem.h | 9 +++++++++ drivers/gpu/drm/drm_gem.c | 6 ++++++ 2 files changed, 15 insertions(+)
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index ae693c0666cd..ee3c4ad742c6 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -150,6 +150,15 @@ struct drm_gem_object_funcs { */ void (*vunmap)(struct drm_gem_object *obj, void *vaddr);
- /**
* @mmap:
*
* Called by drm_gem_mmap() for additional checks/setup.
*
* This callback is optional.
*/
- int (*mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma);
I think if we do an mmap callback, it should replace all the mmap handling (except the drm_gem_object_get) that drm_gem_mmap_obj does. So maybe something like the below:
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 6854f5867d51..e8b7779633dd 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1104,17 +1104,22 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, if (obj_size < vma->vm_end - vma->vm_start) return -EINVAL;
- if (obj->funcs && obj->funcs->vm_ops) - vma->vm_ops = obj->funcs->vm_ops; - else if (dev->driver->gem_vm_ops) - vma->vm_ops = dev->driver->gem_vm_ops; - else - return -EINVAL; - - vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP; vma->vm_private_data = obj; - vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); - vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot); + + if (obj->funcs && obj->funcs->mmap) + obj->funcs->mmap(obj, vma); + else { + if (obj->funcs && obj->funcs->vm_ops) + vma->vm_ops = obj->funcs->vm_ops; + else if (dev->driver->gem_vm_ops) + vma->vm_ops = dev->driver->gem_vm_ops; + else + return -EINVAL; + + vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP; + vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); + vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot); + }
/* Take a ref for this mapping of the object, so that the fault * handler can dereference the mmap offset's pointer to the object.
Since I remember quite a few discussions where the default vma flag wrangling we're doing is seriously getting in the way of things too.
I think even better would be if this new ->mmap hook could also be used directly by the dma-buf mmap code, without having to jump through hoops creating a fake file and fake vma offset and everything. I think with that we'd have a really solid case to add this ->mmap hook. -Daniel
- /**
- @vm_ops:
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index afc38cece3f5..84db8de217e1 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1105,6 +1105,8 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, vma->vm_ops = obj->funcs->vm_ops; else if (dev->driver->gem_vm_ops) vma->vm_ops = dev->driver->gem_vm_ops;
- else if (obj->funcs && obj->funcs->mmap)
else return -EINVAL;/* obj->funcs->mmap must set vma->vm_ops */;
@@ -1192,6 +1194,10 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node) << PAGE_SHIFT, vma);
if (ret == 0)
if (obj->funcs->mmap)
ret = obj->funcs->mmap(obj, vma);
drm_gem_object_put_unlocked(obj);
return ret;
-- 2.18.1
Hi,
I think if we do an mmap callback, it should replace all the mmap handling (except the drm_gem_object_get) that drm_gem_mmap_obj does. So maybe something like the below:
[ snip ]
Since I remember quite a few discussions where the default vma flag wrangling we're doing is seriously getting in the way of things too.
Yep, makes sense.
I think even better would be if this new ->mmap hook could also be used directly by the dma-buf mmap code, without having to jump through hoops creating a fake file and fake vma offset and everything. I think with that we'd have a really solid case to add this ->mmap hook.
Looks easy on a quick glance. So something like the patch below (quick sketch, not tested yet other than compiling it)?
cheers, Gerd
From c13b30d776fb593a03473fa3bc93baf470cba728 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann kraxel@redhat.com Date: Wed, 19 Jun 2019 14:26:51 +0200 Subject: [PATCH] drm: add mmap() to drm_gem_object_funcs
drm_gem_object_funcs->vm_ops alone can't handle everything which needs to be done for mmap(), tweaking vm_flags for example. So add a new mmap() callback to drm_gem_object_funcs where this code can go to.
Note that the vm_ops field is not used in case the mmap callback is presnt, it is expected that the callback sets vma->vm_ops instead.
drm_gem_mmap_obj() will use the new callback for object specific mmap setup. With this in place the need for driver-speific fops->mmap callbacks goes away, drm_gem_mmap can be hooked instead.
drm_gem_prime_mmap() will use the new callback too to just mmap gem objects directly instead of jumping though loops to make drm_gem_object_lookup() and fops->mmap work.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- include/drm/drm_gem.h | 14 ++++++++++++++ drivers/gpu/drm/drm_gem.c | 26 +++++++++++++++++--------- drivers/gpu/drm/drm_prime.c | 9 +++++++++ 3 files changed, 40 insertions(+), 9 deletions(-)
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 6aaba14f5972..e71f75a2ab57 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -150,6 +150,20 @@ struct drm_gem_object_funcs { */ void (*vunmap)(struct drm_gem_object *obj, void *vaddr);
+ /** + * @mmap: + * + * Handle mmap() of the gem object, setup vma accordingly. + * + * This callback is optional. + * + * The callback is used by by both drm_gem_mmap_obj() and + * drm_gem_prime_mmap(). When @mmap is present @vm_ops is not + * used, the @mmap callback must set vma->vm_ops instead. + * + */ + int (*mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma); + /** * @vm_ops: * diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 6854f5867d51..aabde003ac4a 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1099,22 +1099,30 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, struct vm_area_struct *vma) { struct drm_device *dev = obj->dev; + int ret;
/* Check for valid size. */ if (obj_size < vma->vm_end - vma->vm_start) return -EINVAL;
- if (obj->funcs && obj->funcs->vm_ops) - vma->vm_ops = obj->funcs->vm_ops; - else if (dev->driver->gem_vm_ops) - vma->vm_ops = dev->driver->gem_vm_ops; - else - return -EINVAL; + if (obj->funcs && obj->funcs->mmap) { + ret = obj->funcs->mmap(obj, vma); + if (ret) + return ret; + } else { + if (obj->funcs && obj->funcs->vm_ops) + vma->vm_ops = obj->funcs->vm_ops; + else if (dev->driver->gem_vm_ops) + vma->vm_ops = dev->driver->gem_vm_ops; + else + return -EINVAL; + + vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP; + vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); + vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot); + }
- vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP; vma->vm_private_data = obj; - vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); - vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
/* Take a ref for this mapping of the object, so that the fault * handler can dereference the mmap offset's pointer to the object. diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 0a2316e0e812..0814211b0f3f 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -713,6 +713,15 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) struct file *fil; int ret;
+ if (obj->funcs && obj->funcs->mmap) { + ret = obj->funcs->mmap(obj, vma); + if (ret) + return ret; + vma->vm_private_data = obj; + drm_gem_object_get(obj); + return 0; + } + priv = kzalloc(sizeof(*priv), GFP_KERNEL); fil = kzalloc(sizeof(*fil), GFP_KERNEL); if (!priv || !fil) {
On Fri, Sep 6, 2019 at 2:13 PM Gerd Hoffmann kraxel@redhat.com wrote:
Hi,
I think if we do an mmap callback, it should replace all the mmap handling (except the drm_gem_object_get) that drm_gem_mmap_obj does. So maybe something like the below:
[ snip ]
Since I remember quite a few discussions where the default vma flag wrangling we're doing is seriously getting in the way of things too.
Yep, makes sense.
I think even better would be if this new ->mmap hook could also be used directly by the dma-buf mmap code, without having to jump through hoops creating a fake file and fake vma offset and everything. I think with that we'd have a really solid case to add this ->mmap hook.
Looks easy on a quick glance. So something like the patch below (quick sketch, not tested yet other than compiling it)?
Yup, looks good, and if it works I'm happy to smash r-b and a-b tags over this.
One thought below.
cheers, Gerd
From c13b30d776fb593a03473fa3bc93baf470cba728 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann kraxel@redhat.com Date: Wed, 19 Jun 2019 14:26:51 +0200 Subject: [PATCH] drm: add mmap() to drm_gem_object_funcs
drm_gem_object_funcs->vm_ops alone can't handle everything which needs to be done for mmap(), tweaking vm_flags for example. So add a new mmap() callback to drm_gem_object_funcs where this code can go to.
Note that the vm_ops field is not used in case the mmap callback is presnt, it is expected that the callback sets vma->vm_ops instead.
drm_gem_mmap_obj() will use the new callback for object specific mmap setup. With this in place the need for driver-speific fops->mmap callbacks goes away, drm_gem_mmap can be hooked instead.
drm_gem_prime_mmap() will use the new callback too to just mmap gem objects directly instead of jumping though loops to make drm_gem_object_lookup() and fops->mmap work.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
include/drm/drm_gem.h | 14 ++++++++++++++ drivers/gpu/drm/drm_gem.c | 26 +++++++++++++++++--------- drivers/gpu/drm/drm_prime.c | 9 +++++++++ 3 files changed, 40 insertions(+), 9 deletions(-)
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 6aaba14f5972..e71f75a2ab57 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -150,6 +150,20 @@ struct drm_gem_object_funcs { */ void (*vunmap)(struct drm_gem_object *obj, void *vaddr);
/**
* @mmap:
*
* Handle mmap() of the gem object, setup vma accordingly.
*
* This callback is optional.
*
* The callback is used by by both drm_gem_mmap_obj() and
* drm_gem_prime_mmap(). When @mmap is present @vm_ops is not
* used, the @mmap callback must set vma->vm_ops instead.
*
*/
int (*mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma);
/** * @vm_ops: *
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 6854f5867d51..aabde003ac4a 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1099,22 +1099,30 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, struct vm_area_struct *vma) { struct drm_device *dev = obj->dev;
int ret; /* Check for valid size. */ if (obj_size < vma->vm_end - vma->vm_start) return -EINVAL;
if (obj->funcs && obj->funcs->vm_ops)
vma->vm_ops = obj->funcs->vm_ops;
else if (dev->driver->gem_vm_ops)
vma->vm_ops = dev->driver->gem_vm_ops;
else
return -EINVAL;
if (obj->funcs && obj->funcs->mmap) {
ret = obj->funcs->mmap(obj, vma);
if (ret)
return ret;
} else {
if (obj->funcs && obj->funcs->vm_ops)
vma->vm_ops = obj->funcs->vm_ops;
else if (dev->driver->gem_vm_ops)
vma->vm_ops = dev->driver->gem_vm_ops;
else
return -EINVAL;
vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
}
Totally unrelated discussion around HMM lead me to a bit a chase, and realizing that we probably want a
WARN_ON(!(vma->vm_flags & VM_SPECIAL));
here, to make sure drivers set at least one of the "this is a special vma, don't try to treat it like pagecache/anon memory". Just to be on the safe side. Maybe we also want to keep the entire vma->vm_flags assignment in the common code, but at least the WARN_ON would be a good check I think. Maybe also check for VM_DONTEXPAND while at it (that would also break things badly if it's not set). VM_DONTDUMP I think is leftover from when drm drivers exposed register mmaps to userspace.
Anyway, just some detail questions, I think this looks real good.
Thanks, Daniel
vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP; vma->vm_private_data = obj;
vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot); /* Take a ref for this mapping of the object, so that the fault * handler can dereference the mmap offset's pointer to the object.
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 0a2316e0e812..0814211b0f3f 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -713,6 +713,15 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) struct file *fil; int ret;
if (obj->funcs && obj->funcs->mmap) {
ret = obj->funcs->mmap(obj, vma);
if (ret)
return ret;
vma->vm_private_data = obj;
drm_gem_object_get(obj);
return 0;
}
priv = kzalloc(sizeof(*priv), GFP_KERNEL); fil = kzalloc(sizeof(*fil), GFP_KERNEL); if (!priv || !fil) {
-- 2.18.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Hi,
vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
}
Totally unrelated discussion around HMM lead me to a bit a chase, and realizing that we probably want a
WARN_ON(!(vma->vm_flags & VM_SPECIAL));
here, to make sure drivers set at least one of the "this is a special vma, don't try to treat it like pagecache/anon memory". Just to be on the safe side. Maybe we also want to keep the entire vma->vm_flags assignment in the common code, but at least the WARN_ON would be a good check I think. Maybe also check for VM_DONTEXPAND while at it
Hmm. VM_SPECIAL is this:
#define VM_SPECIAL (VM_IO | VM_DONTEXPAND | VM_PFNMAP | VM_MIXEDMAP)
Requiring VM_DONTEXPAND makes sense for sure. Dunno about the other ones. For drm_gem_vram_helper VM_IO + VM_PFNMAP are needed.
But we also have drm_gem_shmem_helper which backs objects with normal pages. In fact drm_gem_shmem_mmap does this:
/* VM_PFNMAP was set by drm_gem_mmap() */ vma->vm_flags &= ~VM_PFNMAP; vma->vm_flags |= VM_MIXEDMAP;
include/linux/mm.h isn't very helpful in explaining how VM_MIXEDMAP should be used, only saying can be both "struct page" and pfnmap, so I'm not sure why VM_MIXEDMAP is set here, it should always be "struct page" for shmem, no?
cheers, Gerd
Switch gem shmem helper from gem_driver->fops->mmap to drm_gem_object_funcs->mmap.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- include/drm/drm_gem_shmem_helper.h | 4 ++-- drivers/gpu/drm/drm_gem_shmem_helper.c | 18 +++++++----------- drivers/gpu/drm/panfrost/panfrost_gem.c | 1 + drivers/gpu/drm/v3d/v3d_bo.c | 1 + 4 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h index 038b6d313447..0f7b77cdc26b 100644 --- a/include/drm/drm_gem_shmem_helper.h +++ b/include/drm/drm_gem_shmem_helper.h @@ -108,7 +108,7 @@ struct drm_gem_shmem_object { .poll = drm_poll,\ .read = drm_read,\ .llseek = noop_llseek,\ - .mmap = drm_gem_shmem_mmap, \ + .mmap = drm_gem_mmap, \ }
struct drm_gem_shmem_object *drm_gem_shmem_create(struct drm_device *dev, size_t size); @@ -128,7 +128,7 @@ drm_gem_shmem_create_with_handle(struct drm_file *file_priv, int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev, struct drm_mode_create_dumb *args);
-int drm_gem_shmem_mmap(struct file *filp, struct vm_area_struct *vma); +int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
extern const struct vm_operations_struct drm_gem_shmem_vm_ops;
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index 2f64667ac805..2e5780520587 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -32,6 +32,7 @@ static const struct drm_gem_object_funcs drm_gem_shmem_funcs = { .get_sg_table = drm_gem_shmem_get_sg_table, .vmap = drm_gem_shmem_vmap, .vunmap = drm_gem_shmem_vunmap, + .mmap = drm_gem_shmem_mmap, .vm_ops = &drm_gem_shmem_vm_ops, };
@@ -448,30 +449,25 @@ EXPORT_SYMBOL_GPL(drm_gem_shmem_vm_ops);
/** * drm_gem_shmem_mmap - Memory-map a shmem GEM object - * @filp: File object + * @obj: gem object * @vma: VMA for the area to be mapped * * This function implements an augmented version of the GEM DRM file mmap * operation for shmem objects. Drivers which employ the shmem helpers should - * use this function as their &file_operations.mmap handler in the DRM device file's - * file_operations structure. + * use this function as their &drm_gem_object_funcs.mmap handler. * - * Instead of directly referencing this function, drivers should use the - * DEFINE_DRM_GEM_SHMEM_FOPS() macro. + * Instead of directly referencing this function, drivers can use + * drm_gem_shmem_funcs. * * Returns: * 0 on success or a negative error code on failure. */ -int drm_gem_shmem_mmap(struct file *filp, struct vm_area_struct *vma) +int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) { struct drm_gem_shmem_object *shmem; int ret;
- ret = drm_gem_mmap(filp, vma); - if (ret) - return ret; - - shmem = to_drm_gem_shmem_obj(vma->vm_private_data); + shmem = to_drm_gem_shmem_obj(obj);
ret = drm_gem_shmem_get_pages(shmem); if (ret) { diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c index 543ab1b81bd5..7c2314a88cd4 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gem.c +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c @@ -37,6 +37,7 @@ static const struct drm_gem_object_funcs panfrost_gem_funcs = { .get_sg_table = drm_gem_shmem_get_sg_table, .vmap = drm_gem_shmem_vmap, .vunmap = drm_gem_shmem_vunmap, + .mmap = drm_gem_shmem_mmap, .vm_ops = &drm_gem_shmem_vm_ops, };
diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c index a22b75a3a533..c01e2b952451 100644 --- a/drivers/gpu/drm/v3d/v3d_bo.c +++ b/drivers/gpu/drm/v3d/v3d_bo.c @@ -58,6 +58,7 @@ static const struct drm_gem_object_funcs v3d_gem_funcs = { .get_sg_table = drm_gem_shmem_get_sg_table, .vmap = drm_gem_shmem_vmap, .vunmap = drm_gem_shmem_vunmap, + .mmap = drm_gem_shmem_mmap, .vm_ops = &drm_gem_shmem_vm_ops, };
On Thu, Aug 8, 2019 at 7:44 AM Gerd Hoffmann kraxel@redhat.com wrote:
Switch gem shmem helper from gem_driver->fops->mmap to drm_gem_object_funcs->mmap.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
include/drm/drm_gem_shmem_helper.h | 4 ++-- drivers/gpu/drm/drm_gem_shmem_helper.c | 18 +++++++----------- drivers/gpu/drm/panfrost/panfrost_gem.c | 1 + drivers/gpu/drm/v3d/v3d_bo.c | 1 + 4 files changed, 11 insertions(+), 13 deletions(-)
Acked-by: Rob Herring robh@kernel.org
DEFINE_DRM_GEM_SHMEM_FOPS is identical to DEFINE_DRM_GEM_FOPS now, drop it.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- include/drm/drm_gem_shmem_helper.h | 26 ------------------------- drivers/gpu/drm/cirrus/cirrus.c | 2 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- drivers/gpu/drm/v3d/v3d_drv.c | 2 +- 4 files changed, 3 insertions(+), 29 deletions(-)
diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h index 0f7b77cdc26b..813240dcfe17 100644 --- a/include/drm/drm_gem_shmem_helper.h +++ b/include/drm/drm_gem_shmem_helper.h @@ -85,32 +85,6 @@ struct drm_gem_shmem_object { #define to_drm_gem_shmem_obj(obj) \ container_of(obj, struct drm_gem_shmem_object, base)
-/** - * DEFINE_DRM_GEM_SHMEM_FOPS() - Macro to generate file operations for shmem drivers - * @name: name for the generated structure - * - * This macro autogenerates a suitable &struct file_operations for shmem based - * drivers, which can be assigned to &drm_driver.fops. Note that this structure - * cannot be shared between drivers, because it contains a reference to the - * current module using THIS_MODULE. - * - * Note that the declaration is already marked as static - if you need a - * non-static version of this you're probably doing it wrong and will break the - * THIS_MODULE reference by accident. - */ -#define DEFINE_DRM_GEM_SHMEM_FOPS(name) \ - static const struct file_operations name = {\ - .owner = THIS_MODULE,\ - .open = drm_open,\ - .release = drm_release,\ - .unlocked_ioctl = drm_ioctl,\ - .compat_ioctl = drm_compat_ioctl,\ - .poll = drm_poll,\ - .read = drm_read,\ - .llseek = noop_llseek,\ - .mmap = drm_gem_mmap, \ - } - struct drm_gem_shmem_object *drm_gem_shmem_create(struct drm_device *dev, size_t size); void drm_gem_shmem_free_object(struct drm_gem_object *obj);
diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c index 36a69aec8a4b..9438af468331 100644 --- a/drivers/gpu/drm/cirrus/cirrus.c +++ b/drivers/gpu/drm/cirrus/cirrus.c @@ -510,7 +510,7 @@ static void cirrus_mode_config_init(struct cirrus_device *cirrus)
/* ------------------------------------------------------------------ */
-DEFINE_DRM_GEM_SHMEM_FOPS(cirrus_fops); +DEFINE_DRM_GEM_FOPS(cirrus_fops);
static struct drm_driver cirrus_driver = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC, diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index b187daa4da85..1c07e1c3386e 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -386,7 +386,7 @@ static const struct drm_ioctl_desc panfrost_drm_driver_ioctls[] = { PANFROST_IOCTL(PERFCNT_DUMP, perfcnt_dump, DRM_RENDER_ALLOW), };
-DEFINE_DRM_GEM_SHMEM_FOPS(panfrost_drm_driver_fops); +DEFINE_DRM_GEM_FOPS(panfrost_drm_driver_fops);
static struct drm_driver panfrost_drm_driver = { .driver_features = DRIVER_RENDER | DRIVER_GEM | DRIVER_SYNCOBJ, diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c index 3506ae2723ae..03e4fbe1b92b 100644 --- a/drivers/gpu/drm/v3d/v3d_drv.c +++ b/drivers/gpu/drm/v3d/v3d_drv.c @@ -169,7 +169,7 @@ v3d_postclose(struct drm_device *dev, struct drm_file *file) kfree(v3d_priv); }
-DEFINE_DRM_GEM_SHMEM_FOPS(v3d_drm_fops); +DEFINE_DRM_GEM_FOPS(v3d_drm_fops);
/* DRM_AUTH is required on SUBMIT_CL for now, while we don't have GMP * protection between clients. Note that render nodes would be be
On Thu, Aug 8, 2019 at 7:44 AM Gerd Hoffmann kraxel@redhat.com wrote:
DEFINE_DRM_GEM_SHMEM_FOPS is identical to DEFINE_DRM_GEM_FOPS now, drop it.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
include/drm/drm_gem_shmem_helper.h | 26 ------------------------- drivers/gpu/drm/cirrus/cirrus.c | 2 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- drivers/gpu/drm/v3d/v3d_drv.c | 2 +- 4 files changed, 3 insertions(+), 29 deletions(-)
Acked-by: Rob Herring robh@kernel.org
Factor out ttm vma setup to a new function.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- include/drm/ttm/ttm_bo_api.h | 8 ++++++ drivers/gpu/drm/ttm/ttm_bo_vm.c | 47 ++++++++++++++++++--------------- 2 files changed, 33 insertions(+), 22 deletions(-)
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 65ef5376de59..2c5fab0f3ed4 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -734,6 +734,14 @@ int ttm_fbdev_mmap(struct vm_area_struct *vma, struct ttm_buffer_object *bo); int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma, struct ttm_bo_device *bdev);
+/** + * ttm_bo_mmap_vma_setup - initialize vma for ttm bo mmap + * + * @bo: The buffer object. + * @vma: vma as input from the mmap method. + */ +void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct vm_area_struct *vma); + void *ttm_kmap_atomic_prot(struct page *page, pgprot_t prot);
void ttm_kunmap_atomic_prot(void *addr, pgprot_t prot); diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index d4eecde8d050..903563a7496a 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -426,6 +426,29 @@ static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_bo_device *bdev, return bo; }
+void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct vm_area_struct *vma) +{ + vma->vm_ops = &ttm_bo_vm_ops; + + /* + * Note: We're transferring the bo reference to + * vma->vm_private_data here. + */ + + vma->vm_private_data = bo; + + /* + * We'd like to use VM_PFNMAP on shared mappings, where + * (vma->vm_flags & VM_SHARED) != 0, for performance reasons, + * but for some reason VM_PFNMAP + x86 PAT + write-combine is very + * bad for performance. Until that has been sorted out, use + * VM_MIXEDMAP on all mappings. See freedesktop.org bug #75719 + */ + vma->vm_flags |= VM_MIXEDMAP; + vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP; +} +EXPORT_SYMBOL(ttm_bo_mmap_vma_setup); + int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma, struct ttm_bo_device *bdev) { @@ -449,24 +472,7 @@ int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma, if (unlikely(ret != 0)) goto out_unref;
- vma->vm_ops = &ttm_bo_vm_ops; - - /* - * Note: We're transferring the bo reference to - * vma->vm_private_data here. - */ - - vma->vm_private_data = bo; - - /* - * We'd like to use VM_PFNMAP on shared mappings, where - * (vma->vm_flags & VM_SHARED) != 0, for performance reasons, - * but for some reason VM_PFNMAP + x86 PAT + write-combine is very - * bad for performance. Until that has been sorted out, use - * VM_MIXEDMAP on all mappings. See freedesktop.org bug #75719 - */ - vma->vm_flags |= VM_MIXEDMAP; - vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP; + ttm_bo_mmap_vma_setup(bo, vma); return 0; out_unref: ttm_bo_put(bo); @@ -481,10 +487,7 @@ int ttm_fbdev_mmap(struct vm_area_struct *vma, struct ttm_buffer_object *bo)
ttm_bo_get(bo);
- vma->vm_ops = &ttm_bo_vm_ops; - vma->vm_private_data = bo; - vma->vm_flags |= VM_MIXEDMAP; - vma->vm_flags |= VM_IO | VM_DONTEXPAND; + ttm_bo_mmap_vma_setup(bo, vma); return 0; } EXPORT_SYMBOL(ttm_fbdev_mmap);
Add helper function to mmap ttm bo's via drm_gem_object_funcs->mmap().
Note that with this code path access verification is done by drm_gem_mmap() and ttm_bo_driver.verify_access() is not used.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- include/drm/drm_gem_ttm_helper.h | 2 ++ drivers/gpu/drm/drm_gem_ttm_helper.c | 11 +++++++++++ 2 files changed, 13 insertions(+)
diff --git a/include/drm/drm_gem_ttm_helper.h b/include/drm/drm_gem_ttm_helper.h index 43c9db3583cc..0de3f41a37f4 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_mmap(struct drm_gem_object *gem, + struct vm_area_struct *vma);
#endif diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c b/drivers/gpu/drm/drm_gem_ttm_helper.c index 0c57e9fd50b9..fabeced8ccf2 100644 --- a/drivers/gpu/drm/drm_gem_ttm_helper.c +++ b/drivers/gpu/drm/drm_gem_ttm_helper.c @@ -34,3 +34,14 @@ int drm_gem_ttm_bo_device_init(struct drm_device *dev, need_dma32); } EXPORT_SYMBOL(drm_gem_ttm_bo_device_init); + +int drm_gem_ttm_mmap(struct drm_gem_object *gem, + struct vm_area_struct *vma) +{ + struct ttm_buffer_object *bo = drm_gem_ttm_of_gem(gem); + + ttm_bo_get(bo); + ttm_bo_mmap_vma_setup(bo, vma); + return 0; +} +EXPORT_SYMBOL(drm_gem_ttm_mmap);
... using the new drm_gem_ttm_mmap() helper function.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- include/drm/drm_vram_mm_helper.h | 9 +-------- drivers/gpu/drm/drm_gem_vram_helper.c | 4 +++- drivers/gpu/drm/drm_vram_mm_helper.c | 27 --------------------------- 3 files changed, 4 insertions(+), 36 deletions(-)
diff --git a/include/drm/drm_vram_mm_helper.h b/include/drm/drm_vram_mm_helper.h index 2aacfb1ccfae..ff328bdaa638 100644 --- a/include/drm/drm_vram_mm_helper.h +++ b/include/drm/drm_vram_mm_helper.h @@ -77,13 +77,6 @@ struct drm_vram_mm *drm_vram_helper_alloc_mm( const struct drm_vram_mm_funcs *funcs); void drm_vram_helper_release_mm(struct drm_device *dev);
-/* - * Helpers for &struct file_operations - */ - -int drm_vram_mm_file_operations_mmap( - struct file *filp, struct vm_area_struct *vma); - /** * define DRM_VRAM_MM_FILE_OPERATIONS - default callback functions for \ &struct file_operations @@ -97,7 +90,7 @@ int drm_vram_mm_file_operations_mmap( .poll = drm_poll, \ .unlocked_ioctl = drm_ioctl, \ .compat_ioctl = drm_compat_ioctl, \ - .mmap = drm_vram_mm_file_operations_mmap, \ + .mmap = drm_gem_mmap, \ .open = drm_open, \ .release = drm_release \
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index b78865055990..ed1625f6a296 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-or-later
+#include <drm/drm_gem_ttm_helper.h> #include <drm/drm_gem_vram_helper.h> #include <drm/drm_device.h> #include <drm/drm_mode.h> @@ -585,5 +586,6 @@ static const struct drm_gem_object_funcs drm_gem_vram_object_funcs = { .pin = drm_gem_vram_object_pin, .unpin = drm_gem_vram_object_unpin, .vmap = drm_gem_vram_object_vmap, - .vunmap = drm_gem_vram_object_vunmap + .vunmap = drm_gem_vram_object_vunmap, + .mmap = drm_gem_ttm_mmap, }; diff --git a/drivers/gpu/drm/drm_vram_mm_helper.c b/drivers/gpu/drm/drm_vram_mm_helper.c index a693f9ce356c..8f6e26b3da69 100644 --- a/drivers/gpu/drm/drm_vram_mm_helper.c +++ b/drivers/gpu/drm/drm_vram_mm_helper.c @@ -268,30 +268,3 @@ void drm_vram_helper_release_mm(struct drm_device *dev) dev->vram_mm = NULL; } EXPORT_SYMBOL(drm_vram_helper_release_mm); - -/* - * Helpers for &struct file_operations - */ - -/** - * drm_vram_mm_file_operations_mmap() - \ - Implements &struct file_operations.mmap() - * @filp: the mapping's file structure - * @vma: the mapping's memory area - * - * Returns: - * 0 on success, or - * a negative error code otherwise. - */ -int drm_vram_mm_file_operations_mmap( - struct file *filp, struct vm_area_struct *vma) -{ - struct drm_file *file_priv = filp->private_data; - struct drm_device *dev = file_priv->minor->dev; - - if (WARN_ONCE(!dev->vram_mm, "VRAM MM not initialized")) - return -EINVAL; - - return drm_vram_mm_mmap(filp, vma, dev->vram_mm); -} -EXPORT_SYMBOL(drm_vram_mm_file_operations_mmap);
Not needed any more.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- include/drm/drm_gem_vram_helper.h | 3 --- include/drm/drm_vram_mm_helper.h | 3 --- drivers/gpu/drm/drm_gem_vram_helper.c | 1 - drivers/gpu/drm/drm_vram_mm_helper.c | 11 ----------- 4 files changed, 18 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/include/drm/drm_vram_mm_helper.h b/include/drm/drm_vram_mm_helper.h index ff328bdaa638..f272adc8ad37 100644 --- a/include/drm/drm_vram_mm_helper.h +++ b/include/drm/drm_vram_mm_helper.h @@ -13,8 +13,6 @@ struct drm_device; * struct drm_vram_mm_funcs - Callback functions for &struct drm_vram_mm * @evict_flags: Provides an implementation for struct \ &ttm_bo_driver.evict_flags - * @verify_access: Provides an implementation for \ - struct &ttm_bo_driver.verify_access * * These callback function integrate VRAM MM with TTM buffer objects. New * functions can be added if necessary. @@ -22,7 +20,6 @@ struct drm_device; struct drm_vram_mm_funcs { void (*evict_flags)(struct ttm_buffer_object *bo, struct ttm_placement *placement); - int (*verify_access)(struct ttm_buffer_object *bo, struct file *filp); };
/** diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index ed1625f6a296..41bb969079d8 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -445,7 +445,6 @@ 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 }; EXPORT_SYMBOL(drm_gem_vram_mm_funcs);
diff --git a/drivers/gpu/drm/drm_vram_mm_helper.c b/drivers/gpu/drm/drm_vram_mm_helper.c index 8f6e26b3da69..059a216d6e78 100644 --- a/drivers/gpu/drm/drm_vram_mm_helper.c +++ b/drivers/gpu/drm/drm_vram_mm_helper.c @@ -89,16 +89,6 @@ static void bo_driver_evict_flags(struct ttm_buffer_object *bo, vmm->funcs->evict_flags(bo, placement); }
-static int bo_driver_verify_access(struct ttm_buffer_object *bo, - struct file *filp) -{ - struct drm_vram_mm *vmm = drm_vram_mm_of_bdev(bo->bdev); - - if (!vmm->funcs || !vmm->funcs->verify_access) - return 0; - return vmm->funcs->verify_access(bo, filp); -} - static int bo_driver_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem) { @@ -140,7 +130,6 @@ static struct ttm_bo_driver bo_driver = { .init_mem_type = bo_driver_init_mem_type, .eviction_valuable = ttm_bo_eviction_valuable, .evict_flags = bo_driver_evict_flags, - .verify_access = bo_driver_verify_access, .io_mem_reserve = bo_driver_io_mem_reserve, .io_mem_free = bo_driver_io_mem_free, };
Not needed any more because we don't have vram specific functions any more. DEFINE_DRM_GEM_FOPS() can be used instead.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- include/drm/drm_vram_mm_helper.h | 17 ----------------- drivers/gpu/drm/ast/ast_drv.c | 5 +---- drivers/gpu/drm/bochs/bochs_drv.c | 5 +---- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 5 +---- drivers/gpu/drm/mgag200/mgag200_drv.c | 5 +---- drivers/gpu/drm/vboxvideo/vbox_drv.c | 5 +---- 6 files changed, 5 insertions(+), 37 deletions(-)
diff --git a/include/drm/drm_vram_mm_helper.h b/include/drm/drm_vram_mm_helper.h index f272adc8ad37..59a8a7657a5b 100644 --- a/include/drm/drm_vram_mm_helper.h +++ b/include/drm/drm_vram_mm_helper.h @@ -74,21 +74,4 @@ struct drm_vram_mm *drm_vram_helper_alloc_mm( const struct drm_vram_mm_funcs *funcs); void drm_vram_helper_release_mm(struct drm_device *dev);
-/** - * define DRM_VRAM_MM_FILE_OPERATIONS - default callback functions for \ - &struct file_operations - * - * Drivers that use VRAM MM can use this macro to initialize - * &struct file_operations with default functions. - */ -#define DRM_VRAM_MM_FILE_OPERATIONS \ - .llseek = no_llseek, \ - .read = drm_read, \ - .poll = drm_poll, \ - .unlocked_ioctl = drm_ioctl, \ - .compat_ioctl = drm_compat_ioctl, \ - .mmap = drm_gem_mmap, \ - .open = drm_open, \ - .release = drm_release \ - #endif diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c index 6ed6ff49efc0..358d2a34b4e6 100644 --- a/drivers/gpu/drm/ast/ast_drv.c +++ b/drivers/gpu/drm/ast/ast_drv.c @@ -201,10 +201,7 @@ static struct pci_driver ast_pci_driver = { .driver.pm = &ast_pm_ops, };
-static const struct file_operations ast_fops = { - .owner = THIS_MODULE, - DRM_VRAM_MM_FILE_OPERATIONS -}; +DEFINE_DRM_GEM_FOPS(ast_fops);
static struct drm_driver driver = { .driver_features = DRIVER_MODESET | DRIVER_GEM, diff --git a/drivers/gpu/drm/bochs/bochs_drv.c b/drivers/gpu/drm/bochs/bochs_drv.c index 770e1625d05e..d7e09af0832a 100644 --- a/drivers/gpu/drm/bochs/bochs_drv.c +++ b/drivers/gpu/drm/bochs/bochs_drv.c @@ -58,10 +58,7 @@ static int bochs_load(struct drm_device *dev) return ret; }
-static const struct file_operations bochs_fops = { - .owner = THIS_MODULE, - DRM_VRAM_MM_FILE_OPERATIONS -}; +DEFINE_DRM_GEM_FOPS(bochs_fops);
static struct drm_driver bochs_driver = { .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c index 6386c67e086b..b3b1275ebf51 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c @@ -27,10 +27,7 @@ #include "hibmc_drm_drv.h" #include "hibmc_drm_regs.h"
-static const struct file_operations hibmc_fops = { - .owner = THIS_MODULE, - DRM_VRAM_MM_FILE_OPERATIONS -}; +DEFINE_DRM_GEM_FOPS(hibmc_fops);
static irqreturn_t hibmc_drm_interrupt(int irq, void *arg) { diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index afd9119b6cf1..684a324990d9 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -58,10 +58,7 @@ static void mga_pci_remove(struct pci_dev *pdev) drm_put_dev(dev); }
-static const struct file_operations mgag200_driver_fops = { - .owner = THIS_MODULE, - DRM_VRAM_MM_FILE_OPERATIONS -}; +DEFINE_DRM_GEM_FOPS(mgag200_driver_fops);
static struct drm_driver driver = { .driver_features = DRIVER_GEM | DRIVER_MODESET, diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c index 6189ea89bb71..f70360081ef1 100644 --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c @@ -189,10 +189,7 @@ static struct pci_driver vbox_pci_driver = { #endif };
-static const struct file_operations vbox_fops = { - .owner = THIS_MODULE, - DRM_VRAM_MM_FILE_OPERATIONS -}; +DEFINE_DRM_GEM_FOPS(vbox_fops);
static struct drm_driver driver = { .driver_features =
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- drivers/gpu/drm/qxl/qxl_drv.c | 8 -------- drivers/gpu/drm/qxl/qxl_object.c | 12 ++++++++++++ 2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c index 12cf85a06bed..38467478c7b2 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.c +++ b/drivers/gpu/drm/qxl/qxl_drv.c @@ -257,16 +257,8 @@ static struct drm_driver qxl_driver = { #endif .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_pin = qxl_gem_prime_pin, - .gem_prime_unpin = qxl_gem_prime_unpin, - .gem_prime_get_sg_table = qxl_gem_prime_get_sg_table, .gem_prime_import_sg_table = qxl_gem_prime_import_sg_table, - .gem_prime_vmap = qxl_gem_prime_vmap, - .gem_prime_vunmap = qxl_gem_prime_vunmap, .gem_prime_mmap = qxl_gem_prime_mmap, - .gem_free_object_unlocked = qxl_gem_object_free, - .gem_open_object = qxl_gem_object_open, - .gem_close_object = qxl_gem_object_close, .fops = &qxl_fops, .ioctls = qxl_ioctls, .irq_handler = qxl_irq_handler, diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c index 548dfe6f3b26..29aab7b14513 100644 --- a/drivers/gpu/drm/qxl/qxl_object.c +++ b/drivers/gpu/drm/qxl/qxl_object.c @@ -77,6 +77,17 @@ void qxl_ttm_placement_from_domain(struct qxl_bo *qbo, u32 domain, bool pinned) } }
+static const struct drm_gem_object_funcs qxl_object_funcs = { + .free = qxl_gem_object_free, + .open = qxl_gem_object_open, + .close = qxl_gem_object_close, + .pin = qxl_gem_prime_pin, + .unpin = qxl_gem_prime_unpin, + .get_sg_table = qxl_gem_prime_get_sg_table, + .vmap = qxl_gem_prime_vmap, + .vunmap = qxl_gem_prime_vunmap, +}; + int qxl_bo_create(struct qxl_device *qdev, unsigned long size, bool kernel, bool pinned, u32 domain, struct qxl_surface *surf, @@ -100,6 +111,7 @@ int qxl_bo_create(struct qxl_device *qdev, kfree(bo); return r; } + bo->tbo.base.funcs = &qxl_object_funcs; bo->type = domain; bo->pin_count = pinned ? 1 : 0; bo->surface_id = 0;
Not sure what this hook is supposed to do. vmf->vma->vm_private_data should never be NULL, so the extra check in qxl_ttm_fault should have no effect.
Drop it.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- drivers/gpu/drm/qxl/qxl_ttm.c | 27 +-------------------------- 1 file changed, 1 insertion(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c index 3a24145dd516..41edbde0e37e 100644 --- a/drivers/gpu/drm/qxl/qxl_ttm.c +++ b/drivers/gpu/drm/qxl/qxl_ttm.c @@ -48,24 +48,8 @@ static struct qxl_device *qxl_get_qdev(struct ttm_bo_device *bdev) return qdev; }
-static struct vm_operations_struct qxl_ttm_vm_ops; -static const struct vm_operations_struct *ttm_vm_ops; - -static vm_fault_t qxl_ttm_fault(struct vm_fault *vmf) -{ - struct ttm_buffer_object *bo; - vm_fault_t ret; - - bo = (struct ttm_buffer_object *)vmf->vma->vm_private_data; - if (bo == NULL) - return VM_FAULT_NOPAGE; - ret = ttm_vm_ops->fault(vmf); - return ret; -} - int qxl_mmap(struct file *filp, struct vm_area_struct *vma) { - int r; struct drm_file *file_priv = filp->private_data; struct qxl_device *qdev = file_priv->minor->dev->dev_private;
@@ -77,16 +61,7 @@ int qxl_mmap(struct file *filp, struct vm_area_struct *vma) DRM_DEBUG_DRIVER("filp->private_data = 0x%p, vma->vm_pgoff = %lx\n", filp->private_data, vma->vm_pgoff);
- r = ttm_bo_mmap(filp, vma, &qdev->mman.bdev); - if (unlikely(r != 0)) - return r; - if (unlikely(ttm_vm_ops == NULL)) { - ttm_vm_ops = vma->vm_ops; - qxl_ttm_vm_ops = *ttm_vm_ops; - qxl_ttm_vm_ops.fault = &qxl_ttm_fault; - } - vma->vm_ops = &qxl_ttm_vm_ops; - return 0; + return ttm_bo_mmap(filp, vma, &qdev->mman.bdev); }
static int qxl_invalidate_caches(struct ttm_bo_device *bdev, uint32_t flags)
... using the use drm_gem_ttm_mmap() helper function.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- drivers/gpu/drm/qxl/qxl_drv.h | 1 - drivers/gpu/drm/qxl/qxl_drv.c | 2 +- drivers/gpu/drm/qxl/qxl_object.c | 1 + drivers/gpu/drm/qxl/qxl_ttm.c | 16 ---------------- 4 files changed, 2 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h index 82efbe76062a..dc36479a54f2 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.h +++ b/drivers/gpu/drm/qxl/qxl_drv.h @@ -352,7 +352,6 @@ int qxl_mode_dumb_create(struct drm_file *file_priv, /* qxl ttm */ int qxl_ttm_init(struct qxl_device *qdev); void qxl_ttm_fini(struct qxl_device *qdev); -int qxl_mmap(struct file *filp, struct vm_area_struct *vma);
/* qxl image */
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c index 38467478c7b2..2fb1641c817e 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.c +++ b/drivers/gpu/drm/qxl/qxl_drv.c @@ -139,7 +139,7 @@ static const struct file_operations qxl_fops = { .unlocked_ioctl = drm_ioctl, .poll = drm_poll, .read = drm_read, - .mmap = qxl_mmap, + .mmap = drm_gem_mmap, };
static int qxl_drm_freeze(struct drm_device *dev) diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c index 29aab7b14513..5c503829c580 100644 --- a/drivers/gpu/drm/qxl/qxl_object.c +++ b/drivers/gpu/drm/qxl/qxl_object.c @@ -86,6 +86,7 @@ static const struct drm_gem_object_funcs qxl_object_funcs = { .get_sg_table = qxl_gem_prime_get_sg_table, .vmap = qxl_gem_prime_vmap, .vunmap = qxl_gem_prime_vunmap, + .mmap = drm_gem_ttm_mmap, };
int qxl_bo_create(struct qxl_device *qdev, diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c index 41edbde0e37e..dbaed0e67c21 100644 --- a/drivers/gpu/drm/qxl/qxl_ttm.c +++ b/drivers/gpu/drm/qxl/qxl_ttm.c @@ -48,22 +48,6 @@ static struct qxl_device *qxl_get_qdev(struct ttm_bo_device *bdev) return qdev; }
-int qxl_mmap(struct file *filp, struct vm_area_struct *vma) -{ - struct drm_file *file_priv = filp->private_data; - struct qxl_device *qdev = file_priv->minor->dev->dev_private; - - if (qdev == NULL) { - DRM_ERROR( - "filp->private_data->minor->dev->dev_private == NULL\n"); - return -EINVAL; - } - DRM_DEBUG_DRIVER("filp->private_data = 0x%p, vma->vm_pgoff = %lx\n", - filp->private_data, vma->vm_pgoff); - - return ttm_bo_mmap(filp, vma, &qdev->mman.bdev); -} - static int qxl_invalidate_caches(struct ttm_bo_device *bdev, uint32_t flags) { return 0;
Not needed any more.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- drivers/gpu/drm/qxl/qxl_ttm.c | 9 --------- 1 file changed, 9 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c index dbaed0e67c21..d1d8fe6e1e93 100644 --- a/drivers/gpu/drm/qxl/qxl_ttm.c +++ b/drivers/gpu/drm/qxl/qxl_ttm.c @@ -110,14 +110,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) { @@ -269,7 +261,6 @@ 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, .io_mem_reserve = &qxl_ttm_io_mem_reserve, .io_mem_free = &qxl_ttm_io_mem_free, .move_notify = &qxl_bo_move_notify,
We have no qxl-specific fops any more.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- drivers/gpu/drm/qxl/qxl_drv.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c index 2fb1641c817e..4853082ba924 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.c +++ b/drivers/gpu/drm/qxl/qxl_drv.c @@ -132,15 +132,7 @@ qxl_pci_remove(struct pci_dev *pdev) drm_dev_put(dev); }
-static const struct file_operations qxl_fops = { - .owner = THIS_MODULE, - .open = drm_open, - .release = drm_release, - .unlocked_ioctl = drm_ioctl, - .poll = drm_poll, - .read = drm_read, - .mmap = drm_gem_mmap, -}; +DEFINE_DRM_GEM_FOPS(qxl_fops);
static int qxl_drm_freeze(struct drm_device *dev) {
On Thu, Aug 8, 2019 at 7:44 AM Gerd Hoffmann kraxel@redhat.com wrote:
Gerd Hoffmann (17): drm/ttm: turn ttm_bo_device.vma_manager into a pointer drm/ttm: add gem_ttm_bo_device_init() drm/vram: switch vram helpers to the new gem_ttm_bo_device_init() drm/qxl: switch qxl to the new gem_ttm_bo_device_init() drm: add mmap() to drm_gem_object_funcs drm/shmem: switch shmem helper to drm_gem_object_funcs->mmap drm/shmem: drop DEFINE_DRM_GEM_SHMEM_FOPS drm/ttm: factor out ttm_bo_mmap_vma_setup drm/ttm: add drm_gem_ttm_mmap() drm/vram: switch vram helper to drm_gem_object_funcs->mmap codepath drm/vram: drop verify_access drm: drop DRM_VRAM_MM_FILE_OPERATIONS drm/qxl: use drm_gem_object_funcs drm/qxl: drop qxl_ttm_fault drm/qxl: switch qxl to drm_gem_object_funcs->mmap codepath drm/qxl: drop verify_access drm/qxl: use DEFINE_DRM_GEM_FOPS()
Perhaps instead of changing drivers to use DEFINE_DRM_GEM_FOPS, make that the default if .fops is NULL (and perhaps also if the driver sets DRIVER_GEM). That would be in line with other recent rework making various function ptrs optional.
Rob
On Thu, Aug 08, 2019 at 08:39:11AM -0600, Rob Herring wrote:
On Thu, Aug 8, 2019 at 7:44 AM Gerd Hoffmann kraxel@redhat.com wrote:
Gerd Hoffmann (17): drm/ttm: turn ttm_bo_device.vma_manager into a pointer drm/ttm: add gem_ttm_bo_device_init() drm/vram: switch vram helpers to the new gem_ttm_bo_device_init() drm/qxl: switch qxl to the new gem_ttm_bo_device_init() drm: add mmap() to drm_gem_object_funcs drm/shmem: switch shmem helper to drm_gem_object_funcs->mmap drm/shmem: drop DEFINE_DRM_GEM_SHMEM_FOPS drm/ttm: factor out ttm_bo_mmap_vma_setup drm/ttm: add drm_gem_ttm_mmap() drm/vram: switch vram helper to drm_gem_object_funcs->mmap codepath drm/vram: drop verify_access drm: drop DRM_VRAM_MM_FILE_OPERATIONS drm/qxl: use drm_gem_object_funcs drm/qxl: drop qxl_ttm_fault drm/qxl: switch qxl to drm_gem_object_funcs->mmap codepath drm/qxl: drop verify_access drm/qxl: use DEFINE_DRM_GEM_FOPS()
Perhaps instead of changing drivers to use DEFINE_DRM_GEM_FOPS, make that the default if .fops is NULL (and perhaps also if the driver sets DRIVER_GEM). That would be in line with other recent rework making various function ptrs optional.
Not so easy for fops due to .owner = THIS_MODULE
cheers, Gerd
Hi,
I would have liked to get some context on the purpose of GEM TTM helpers. Is is just share-able code?
From my understanding VRAM helpers _are_ GEM TTM helpers. And they where re-named to VRAM helpers, so that the naming is independent from the implementation (and vice versa).
Wrt qxl, would it be possible to convert the driver over to VRAM helpers entirely? I noticed a memory region named PRIV. Could we add this to VRAM helpers?
Best regards Thomas
Am 08.08.19 um 15:44 schrieb Gerd Hoffmann:
Gerd Hoffmann (17): drm/ttm: turn ttm_bo_device.vma_manager into a pointer drm/ttm: add gem_ttm_bo_device_init() drm/vram: switch vram helpers to the new gem_ttm_bo_device_init() drm/qxl: switch qxl to the new gem_ttm_bo_device_init() drm: add mmap() to drm_gem_object_funcs drm/shmem: switch shmem helper to drm_gem_object_funcs->mmap drm/shmem: drop DEFINE_DRM_GEM_SHMEM_FOPS drm/ttm: factor out ttm_bo_mmap_vma_setup drm/ttm: add drm_gem_ttm_mmap() drm/vram: switch vram helper to drm_gem_object_funcs->mmap codepath drm/vram: drop verify_access drm: drop DRM_VRAM_MM_FILE_OPERATIONS drm/qxl: use drm_gem_object_funcs drm/qxl: drop qxl_ttm_fault drm/qxl: switch qxl to drm_gem_object_funcs->mmap codepath drm/qxl: drop verify_access drm/qxl: use DEFINE_DRM_GEM_FOPS()
drivers/gpu/drm/qxl/qxl_drv.h | 5 +- drivers/gpu/drm/qxl/qxl_object.h | 5 -- include/drm/drm_gem.h | 9 +++ include/drm/drm_gem_shmem_helper.h | 28 +-------- include/drm/drm_gem_ttm_helper.h | 32 ++++++++++ include/drm/drm_gem_vram_helper.h | 9 +-- include/drm/drm_vram_mm_helper.h | 27 --------- include/drm/ttm/ttm_bo_api.h | 8 +++ include/drm/ttm/ttm_bo_driver.h | 11 +++- drivers/gpu/drm/ast/ast_drv.c | 5 +- drivers/gpu/drm/bochs/bochs_drv.c | 5 +- drivers/gpu/drm/cirrus/cirrus.c | 2 +- drivers/gpu/drm/drm_gem.c | 6 ++ drivers/gpu/drm/drm_gem_shmem_helper.c | 18 +++--- drivers/gpu/drm/drm_gem_ttm_helper.c | 47 +++++++++++++++ drivers/gpu/drm/drm_gem_vram_helper.c | 53 +---------------- drivers/gpu/drm/drm_vram_mm_helper.c | 44 +------------- .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 6 +- drivers/gpu/drm/mgag200/mgag200_drv.c | 5 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- drivers/gpu/drm/panfrost/panfrost_gem.c | 1 + drivers/gpu/drm/qxl/qxl_drv.c | 19 +----- drivers/gpu/drm/qxl/qxl_dumb.c | 17 ------ drivers/gpu/drm/qxl/qxl_ioctl.c | 5 +- drivers/gpu/drm/qxl/qxl_object.c | 13 +++++ drivers/gpu/drm/qxl/qxl_ttm.c | 58 ++----------------- drivers/gpu/drm/ttm/ttm_bo.c | 29 +++++++--- drivers/gpu/drm/ttm/ttm_bo_vm.c | 53 +++++++++-------- drivers/gpu/drm/v3d/v3d_bo.c | 1 + drivers/gpu/drm/v3d/v3d_drv.c | 2 +- drivers/gpu/drm/vboxvideo/vbox_drv.c | 5 +- Documentation/gpu/drm-mm.rst | 12 ++++ drivers/gpu/drm/Kconfig | 8 +++ drivers/gpu/drm/Makefile | 3 + drivers/gpu/drm/qxl/Kconfig | 1 + 35 files changed, 231 insertions(+), 323 deletions(-) create mode 100644 include/drm/drm_gem_ttm_helper.h create mode 100644 drivers/gpu/drm/drm_gem_ttm_helper.c
On Mon, Aug 26, 2019 at 10:47 AM Thomas Zimmermann tzimmermann@suse.de wrote:
Hi,
I would have liked to get some context on the purpose of GEM TTM helpers. Is is just share-able code?
From my understanding VRAM helpers _are_ GEM TTM helpers. And they where re-named to VRAM helpers, so that the naming is independent from the implementation (and vice versa).
The point of the vram helpers was to have something to manage vram for dumb display-only drivers. It's prep work for Thomas Zimmermann's plan to port a pile of fbdev drivers over. So fairly intentionally limit in the use-cases it supports to keep it simple. Kinda similar to how the simple display pipe helper is designed on the kms side of things.
Wrt qxl, would it be possible to convert the driver over to VRAM helpers entirely? I noticed a memory region named PRIV. Could we add this to VRAM helpers?
For both simple display pipe and vram helpers I'd say if your use-case goes beyond simple dumb display-only driver, it's probably better to have something more flexible.
Also this patch series also adjust vram helpers, and I think it has a slightly different goal: Just aligning mmap paths a bit more between ttm and not-ttm based drivers. That's also what motivated my lockdep series, but from a locking rules instead of from a code-sharing point of view. Seems like a good goal, details might need adjustment. -Daniel
Best regards Thomas
Am 08.08.19 um 15:44 schrieb Gerd Hoffmann:
Gerd Hoffmann (17): drm/ttm: turn ttm_bo_device.vma_manager into a pointer drm/ttm: add gem_ttm_bo_device_init() drm/vram: switch vram helpers to the new gem_ttm_bo_device_init() drm/qxl: switch qxl to the new gem_ttm_bo_device_init() drm: add mmap() to drm_gem_object_funcs drm/shmem: switch shmem helper to drm_gem_object_funcs->mmap drm/shmem: drop DEFINE_DRM_GEM_SHMEM_FOPS drm/ttm: factor out ttm_bo_mmap_vma_setup drm/ttm: add drm_gem_ttm_mmap() drm/vram: switch vram helper to drm_gem_object_funcs->mmap codepath drm/vram: drop verify_access drm: drop DRM_VRAM_MM_FILE_OPERATIONS drm/qxl: use drm_gem_object_funcs drm/qxl: drop qxl_ttm_fault drm/qxl: switch qxl to drm_gem_object_funcs->mmap codepath drm/qxl: drop verify_access drm/qxl: use DEFINE_DRM_GEM_FOPS()
drivers/gpu/drm/qxl/qxl_drv.h | 5 +- drivers/gpu/drm/qxl/qxl_object.h | 5 -- include/drm/drm_gem.h | 9 +++ include/drm/drm_gem_shmem_helper.h | 28 +-------- include/drm/drm_gem_ttm_helper.h | 32 ++++++++++ include/drm/drm_gem_vram_helper.h | 9 +-- include/drm/drm_vram_mm_helper.h | 27 --------- include/drm/ttm/ttm_bo_api.h | 8 +++ include/drm/ttm/ttm_bo_driver.h | 11 +++- drivers/gpu/drm/ast/ast_drv.c | 5 +- drivers/gpu/drm/bochs/bochs_drv.c | 5 +- drivers/gpu/drm/cirrus/cirrus.c | 2 +- drivers/gpu/drm/drm_gem.c | 6 ++ drivers/gpu/drm/drm_gem_shmem_helper.c | 18 +++--- drivers/gpu/drm/drm_gem_ttm_helper.c | 47 +++++++++++++++ drivers/gpu/drm/drm_gem_vram_helper.c | 53 +---------------- drivers/gpu/drm/drm_vram_mm_helper.c | 44 +------------- .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 6 +- drivers/gpu/drm/mgag200/mgag200_drv.c | 5 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- drivers/gpu/drm/panfrost/panfrost_gem.c | 1 + drivers/gpu/drm/qxl/qxl_drv.c | 19 +----- drivers/gpu/drm/qxl/qxl_dumb.c | 17 ------ drivers/gpu/drm/qxl/qxl_ioctl.c | 5 +- drivers/gpu/drm/qxl/qxl_object.c | 13 +++++ drivers/gpu/drm/qxl/qxl_ttm.c | 58 ++----------------- drivers/gpu/drm/ttm/ttm_bo.c | 29 +++++++--- drivers/gpu/drm/ttm/ttm_bo_vm.c | 53 +++++++++-------- drivers/gpu/drm/v3d/v3d_bo.c | 1 + drivers/gpu/drm/v3d/v3d_drv.c | 2 +- drivers/gpu/drm/vboxvideo/vbox_drv.c | 5 +- Documentation/gpu/drm-mm.rst | 12 ++++ drivers/gpu/drm/Kconfig | 8 +++ drivers/gpu/drm/Makefile | 3 + drivers/gpu/drm/qxl/Kconfig | 1 + 35 files changed, 231 insertions(+), 323 deletions(-) create mode 100644 include/drm/drm_gem_ttm_helper.h create mode 100644 drivers/gpu/drm/drm_gem_ttm_helper.c
-- Thomas Zimmermann Graphics Driver Developer SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 21284 (AG Nürnberg)
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi,
Also this patch series also adjust vram helpers, and I think it has a slightly different goal: Just aligning mmap paths a bit more between ttm and not-ttm based drivers.
Not just ttm/not-ttm. gem_driver->fops->mmap is the only fops callback where we can't use a generic gem callback today. This series tries to fix that with the new drm_gem_object_funcs->mmap hook, so the mmap() code path isn't the odd one which works different than every other drm_gem_object operation.
Seems like a good goal, details might need adjustment.
Which details?
cheers, Gerd
On Tue, Aug 27, 2019 at 07:13:41AM +0200, Gerd Hoffmann wrote:
Hi,
Also this patch series also adjust vram helpers, and I think it has a slightly different goal: Just aligning mmap paths a bit more between ttm and not-ttm based drivers.
Not just ttm/not-ttm. gem_driver->fops->mmap is the only fops callback where we can't use a generic gem callback today. This series tries to fix that with the new drm_gem_object_funcs->mmap hook, so the mmap() code path isn't the odd one which works different than every other drm_gem_object operation.
Seems like a good goal, details might need adjustment.
Which details?
Just a general comment that the tricky parts are always in the details ... -Daniel
On Mon, Aug 26, 2019 at 10:47:07AM +0200, Thomas Zimmermann wrote:
Hi,
I would have liked to get some context on the purpose of GEM TTM helpers. Is is just share-able code?
Yes. Shareable code for drivers which use both ttm and gem (all except vmgfx).
From my understanding VRAM helpers _are_ GEM TTM helpers. And they where re-named to VRAM helpers, so that the naming is independent from the implementation (and vice versa).
I think vram helpers are for old/simple devices which basically support dumb gem objects stored in vram (device memory typically exposed as pci bar). Using ttm underneath is largely an implementation detail for the users of the vram helpers, they don't need to know.
Wrt qxl, would it be possible to convert the driver over to VRAM helpers entirely? I noticed a memory region named PRIV. Could we add this to VRAM helpers?
PRIV is vram too, qxl has two pci bars with memory. Historical reasons. There are rules which pci bar can store which kind of objects. I don't think it makes sense to move such device-specific things into vram helpers. Also qxl is complex enough that you can hardly hide ttm behind vram helpers.
cheers, Gerd
On Thu, Aug 08, 2019 at 03:44:00PM +0200, Gerd Hoffmann wrote:
Gerd Hoffmann (17): drm/ttm: turn ttm_bo_device.vma_manager into a pointer drm/ttm: add gem_ttm_bo_device_init() drm/vram: switch vram helpers to the new gem_ttm_bo_device_init() drm/qxl: switch qxl to the new gem_ttm_bo_device_init() drm: add mmap() to drm_gem_object_funcs drm/shmem: switch shmem helper to drm_gem_object_funcs->mmap drm/shmem: drop DEFINE_DRM_GEM_SHMEM_FOPS drm/ttm: factor out ttm_bo_mmap_vma_setup drm/ttm: add drm_gem_ttm_mmap() drm/vram: switch vram helper to drm_gem_object_funcs->mmap codepath drm/vram: drop verify_access drm: drop DRM_VRAM_MM_FILE_OPERATIONS drm/qxl: use drm_gem_object_funcs drm/qxl: drop qxl_ttm_fault drm/qxl: switch qxl to drm_gem_object_funcs->mmap codepath drm/qxl: drop verify_access drm/qxl: use DEFINE_DRM_GEM_FOPS()
I like.
Dropped 2 thoughts for a bit more polish/usefulness for all this. Unfortunately I'm kinda burried, so would be better if you find someone else for detailed review and all that. -Daniel
drivers/gpu/drm/qxl/qxl_drv.h | 5 +- drivers/gpu/drm/qxl/qxl_object.h | 5 -- include/drm/drm_gem.h | 9 +++ include/drm/drm_gem_shmem_helper.h | 28 +-------- include/drm/drm_gem_ttm_helper.h | 32 ++++++++++ include/drm/drm_gem_vram_helper.h | 9 +-- include/drm/drm_vram_mm_helper.h | 27 --------- include/drm/ttm/ttm_bo_api.h | 8 +++ include/drm/ttm/ttm_bo_driver.h | 11 +++- drivers/gpu/drm/ast/ast_drv.c | 5 +- drivers/gpu/drm/bochs/bochs_drv.c | 5 +- drivers/gpu/drm/cirrus/cirrus.c | 2 +- drivers/gpu/drm/drm_gem.c | 6 ++ drivers/gpu/drm/drm_gem_shmem_helper.c | 18 +++--- drivers/gpu/drm/drm_gem_ttm_helper.c | 47 +++++++++++++++ drivers/gpu/drm/drm_gem_vram_helper.c | 53 +---------------- drivers/gpu/drm/drm_vram_mm_helper.c | 44 +------------- .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 6 +- drivers/gpu/drm/mgag200/mgag200_drv.c | 5 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- drivers/gpu/drm/panfrost/panfrost_gem.c | 1 + drivers/gpu/drm/qxl/qxl_drv.c | 19 +----- drivers/gpu/drm/qxl/qxl_dumb.c | 17 ------ drivers/gpu/drm/qxl/qxl_ioctl.c | 5 +- drivers/gpu/drm/qxl/qxl_object.c | 13 +++++ drivers/gpu/drm/qxl/qxl_ttm.c | 58 ++----------------- drivers/gpu/drm/ttm/ttm_bo.c | 29 +++++++--- drivers/gpu/drm/ttm/ttm_bo_vm.c | 53 +++++++++-------- drivers/gpu/drm/v3d/v3d_bo.c | 1 + drivers/gpu/drm/v3d/v3d_drv.c | 2 +- drivers/gpu/drm/vboxvideo/vbox_drv.c | 5 +- Documentation/gpu/drm-mm.rst | 12 ++++ drivers/gpu/drm/Kconfig | 8 +++ drivers/gpu/drm/Makefile | 3 + drivers/gpu/drm/qxl/Kconfig | 1 + 35 files changed, 231 insertions(+), 323 deletions(-) create mode 100644 include/drm/drm_gem_ttm_helper.h create mode 100644 drivers/gpu/drm/drm_gem_ttm_helper.c
-- 2.18.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org