* [PATCH v3 1/7] drm/ttm: Don't override vm_ops callbacks, if set
2021-04-16 13:31 [PATCH v3 0/7] drm: Clean up mmap for TTM-based GEM drivers Thomas Zimmermann
@ 2021-04-16 13:31 ` Thomas Zimmermann
2021-04-16 13:31 ` [PATCH v3 2/7] drm/amdgpu: Implement mmap as GEM object function Thomas Zimmermann
` (6 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: Thomas Zimmermann @ 2021-04-16 13:31 UTC (permalink / raw)
To: alexander.deucher, christian.koenig, airlied, daniel, bskeggs,
ray.huang, linux-graphics-maintainer, sroland, zackr,
shashank.sharma, sam, emil.velikov, Felix.Kuehling, nirmoy.das
Cc: nouveau, amd-gfx, dri-devel, Thomas Zimmermann
Drivers may want to set their own callbacks for a VM area. Only set
TTM's callbacks if the vm_ops field is clear.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/ttm/ttm_bo_vm.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index b31b18058965..bf4a213bc66c 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -534,7 +534,12 @@ static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_device *bdev,
static void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct vm_area_struct *vma)
{
- vma->vm_ops = &ttm_bo_vm_ops;
+ /*
+ * Drivers may want to override the vm_ops field. Otherwise we
+ * use TTM's default callbacks.
+ */
+ if (!vma->vm_ops)
+ vma->vm_ops = &ttm_bo_vm_ops;
/*
* Note: We're transferring the bo reference to
--
2.31.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 2/7] drm/amdgpu: Implement mmap as GEM object function
2021-04-16 13:31 [PATCH v3 0/7] drm: Clean up mmap for TTM-based GEM drivers Thomas Zimmermann
2021-04-16 13:31 ` [PATCH v3 1/7] drm/ttm: Don't override vm_ops callbacks, if set Thomas Zimmermann
@ 2021-04-16 13:31 ` Thomas Zimmermann
2021-04-16 13:31 ` [PATCH v3 3/7] drm/radeon: " Thomas Zimmermann
` (5 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: Thomas Zimmermann @ 2021-04-16 13:31 UTC (permalink / raw)
To: alexander.deucher, christian.koenig, airlied, daniel, bskeggs,
ray.huang, linux-graphics-maintainer, sroland, zackr,
shashank.sharma, sam, emil.velikov, Felix.Kuehling, nirmoy.das
Cc: nouveau, amd-gfx, dri-devel, Thomas Zimmermann
Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.
This change resolves several inconsistencies between regular mmap and
prime-based mmap. The vm_ops field in vma is now set for all mmap'ed
areas. Previously it way only set for regular mmap calls, prime-based
mmap used TTM's default vm_ops. The function amdgpu_verify_access() is
no longer being called and therefore removed by this patch.
As a side effect, amdgpu_ttm_vm_ops and amdgpu_ttm_fault() are now
implemented in amdgpu's GEM code.
v3:
* rename mmap function to amdgpu_gem_object_mmap() (Christian)
* remove unnecessary checks from mmap (Christian)
v2:
* rename amdgpu_ttm_vm_ops and amdgpu_ttm_fault() to
amdgpu_gem_vm_ops and amdgpu_gem_fault() (Christian)
* the check for kfd_bo has meanwhile been removed
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
cleanup mmap rename gem_object_mmap
---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 46 ---------------
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h | 2 -
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 44 ++++++++++++++
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 64 ---------------------
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 -
6 files changed, 46 insertions(+), 115 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index e0c4f7c7f1b9..19c5ab08d9ec 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -42,52 +42,6 @@
#include <linux/pci-p2pdma.h>
#include <linux/pm_runtime.h>
-/**
- * amdgpu_gem_prime_mmap - &drm_driver.gem_prime_mmap implementation
- * @obj: GEM BO
- * @vma: Virtual memory area
- *
- * Sets up a userspace mapping of the BO's memory in the given
- * virtual memory area.
- *
- * Returns:
- * 0 on success or a negative error code on failure.
- */
-int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
- struct vm_area_struct *vma)
-{
- struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
- struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
- unsigned asize = amdgpu_bo_size(bo);
- int ret;
-
- if (!vma->vm_file)
- return -ENODEV;
-
- if (adev == NULL)
- return -ENODEV;
-
- /* Check for valid size. */
- if (asize < vma->vm_end - vma->vm_start)
- return -EINVAL;
-
- if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) ||
- (bo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)) {
- return -EPERM;
- }
- vma->vm_pgoff += amdgpu_bo_mmap_offset(bo) >> PAGE_SHIFT;
-
- /* prime mmap does not need to check access, so allow here */
- ret = drm_vma_node_allow(&obj->vma_node, vma->vm_file->private_data);
- if (ret)
- return ret;
-
- ret = ttm_bo_mmap(vma->vm_file, vma, &adev->mman.bdev);
- drm_vma_node_revoke(&obj->vma_node, vma->vm_file->private_data);
-
- return ret;
-}
-
static int
__dma_resv_make_exclusive(struct dma_resv *obj)
{
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
index 39b5b9616fd8..3e93b9b407a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
@@ -31,8 +31,6 @@ struct drm_gem_object *amdgpu_gem_prime_import(struct drm_device *dev,
struct dma_buf *dma_buf);
bool amdgpu_dmabuf_is_xgmi_accessible(struct amdgpu_device *adev,
struct amdgpu_bo *bo);
-int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
- struct vm_area_struct *vma);
extern const struct dma_buf_ops amdgpu_dmabuf_ops;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 80130c1c0c68..69d2485ec665 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1686,7 +1686,7 @@ static const struct file_operations amdgpu_driver_kms_fops = {
.flush = amdgpu_flush,
.release = drm_release,
.unlocked_ioctl = amdgpu_drm_ioctl,
- .mmap = amdgpu_mmap,
+ .mmap = drm_gem_mmap,
.poll = drm_poll,
.read = drm_read,
#ifdef CONFIG_COMPAT
@@ -1749,7 +1749,7 @@ static const struct drm_driver amdgpu_kms_driver = {
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_import = amdgpu_gem_prime_import,
- .gem_prime_mmap = amdgpu_gem_prime_mmap,
+ .gem_prime_mmap = drm_gem_prime_mmap,
.name = DRIVER_NAME,
.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 311bcdc59eda..df78a06ef0c8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -41,6 +41,36 @@
static const struct drm_gem_object_funcs amdgpu_gem_object_funcs;
+static vm_fault_t amdgpu_gem_fault(struct vm_fault *vmf)
+{
+ struct ttm_buffer_object *bo = vmf->vma->vm_private_data;
+ vm_fault_t ret;
+
+ ret = ttm_bo_vm_reserve(bo, vmf);
+ if (ret)
+ return ret;
+
+ ret = amdgpu_bo_fault_reserve_notify(bo);
+ if (ret)
+ goto unlock;
+
+ ret = ttm_bo_vm_fault_reserved(vmf, vmf->vma->vm_page_prot,
+ TTM_BO_VM_NUM_PREFAULT, 1);
+ if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
+ return ret;
+
+unlock:
+ dma_resv_unlock(bo->base.resv);
+ return ret;
+}
+
+static const struct vm_operations_struct amdgpu_gem_vm_ops = {
+ .fault = amdgpu_gem_fault,
+ .open = ttm_bo_vm_open,
+ .close = ttm_bo_vm_close,
+ .access = ttm_bo_vm_access
+};
+
static void amdgpu_gem_object_free(struct drm_gem_object *gobj)
{
struct amdgpu_bo *robj = gem_to_amdgpu_bo(gobj);
@@ -205,6 +235,18 @@ static void amdgpu_gem_object_close(struct drm_gem_object *obj,
ttm_eu_backoff_reservation(&ticket, &list);
}
+static int amdgpu_gem_object_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
+{
+ struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
+
+ if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm))
+ return -EPERM;
+ if (bo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)
+ return -EPERM;
+
+ return drm_gem_ttm_mmap(obj, vma);
+}
+
static const struct drm_gem_object_funcs amdgpu_gem_object_funcs = {
.free = amdgpu_gem_object_free,
.open = amdgpu_gem_object_open,
@@ -212,6 +254,8 @@ static const struct drm_gem_object_funcs amdgpu_gem_object_funcs = {
.export = amdgpu_gem_prime_export,
.vmap = drm_gem_ttm_vmap,
.vunmap = drm_gem_ttm_vunmap,
+ .mmap = amdgpu_gem_object_mmap,
+ .vm_ops = &amdgpu_gem_vm_ops,
};
/*
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 1485f33c3cc7..d4083c19402b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -152,25 +152,6 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo,
*placement = abo->placement;
}
-/**
- * amdgpu_verify_access - Verify access for a mmap call
- *
- * @bo: The buffer object to map
- * @filp: The file pointer from the process performing the mmap
- *
- * This is called by ttm_bo_mmap() to verify whether a process
- * has the right to mmap a BO to their process space.
- */
-static int amdgpu_verify_access(struct ttm_buffer_object *bo, struct file *filp)
-{
- struct amdgpu_bo *abo = ttm_to_amdgpu_bo(bo);
-
- if (amdgpu_ttm_tt_get_usermm(bo->ttm))
- return -EPERM;
- return drm_vma_node_verify_access(&abo->tbo.base.vma_node,
- filp->private_data);
-}
-
/**
* amdgpu_ttm_map_buffer - Map memory into the GART windows
* @bo: buffer object to map
@@ -1522,7 +1503,6 @@ static struct ttm_device_funcs amdgpu_bo_driver = {
.eviction_valuable = amdgpu_ttm_bo_eviction_valuable,
.evict_flags = &amdgpu_evict_flags,
.move = &amdgpu_bo_move,
- .verify_access = &amdgpu_verify_access,
.delete_mem_notify = &amdgpu_bo_delete_mem_notify,
.release_notify = &amdgpu_bo_release_notify,
.io_mem_reserve = &amdgpu_ttm_io_mem_reserve,
@@ -1897,50 +1877,6 @@ void amdgpu_ttm_set_buffer_funcs_status(struct amdgpu_device *adev, bool enable)
adev->mman.buffer_funcs_enabled = enable;
}
-static vm_fault_t amdgpu_ttm_fault(struct vm_fault *vmf)
-{
- struct ttm_buffer_object *bo = vmf->vma->vm_private_data;
- vm_fault_t ret;
-
- ret = ttm_bo_vm_reserve(bo, vmf);
- if (ret)
- return ret;
-
- ret = amdgpu_bo_fault_reserve_notify(bo);
- if (ret)
- goto unlock;
-
- ret = ttm_bo_vm_fault_reserved(vmf, vmf->vma->vm_page_prot,
- TTM_BO_VM_NUM_PREFAULT, 1);
- if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
- return ret;
-
-unlock:
- dma_resv_unlock(bo->base.resv);
- return ret;
-}
-
-static const struct vm_operations_struct amdgpu_ttm_vm_ops = {
- .fault = amdgpu_ttm_fault,
- .open = ttm_bo_vm_open,
- .close = ttm_bo_vm_close,
- .access = ttm_bo_vm_access
-};
-
-int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma)
-{
- struct drm_file *file_priv = filp->private_data;
- struct amdgpu_device *adev = drm_to_adev(file_priv->minor->dev);
- int r;
-
- r = ttm_bo_mmap(filp, vma, &adev->mman.bdev);
- if (unlikely(r != 0))
- return r;
-
- vma->vm_ops = &amdgpu_ttm_vm_ops;
- return 0;
-}
-
int amdgpu_copy_buffer(struct amdgpu_ring *ring, uint64_t src_offset,
uint64_t dst_offset, uint32_t byte_count,
struct dma_resv *resv,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index dec0db8b0b13..6e51faad7371 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -146,7 +146,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
struct dma_resv *resv,
struct dma_fence **fence);
-int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo);
int amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo);
uint64_t amdgpu_ttm_domain_start(struct amdgpu_device *adev, uint32_t type);
--
2.31.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 3/7] drm/radeon: Implement mmap as GEM object function
2021-04-16 13:31 [PATCH v3 0/7] drm: Clean up mmap for TTM-based GEM drivers Thomas Zimmermann
2021-04-16 13:31 ` [PATCH v3 1/7] drm/ttm: Don't override vm_ops callbacks, if set Thomas Zimmermann
2021-04-16 13:31 ` [PATCH v3 2/7] drm/amdgpu: Implement mmap as GEM object function Thomas Zimmermann
@ 2021-04-16 13:31 ` Thomas Zimmermann
2021-04-16 13:31 ` [PATCH v3 4/7] drm/nouveau: " Thomas Zimmermann
` (4 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: Thomas Zimmermann @ 2021-04-16 13:31 UTC (permalink / raw)
To: alexander.deucher, christian.koenig, airlied, daniel, bskeggs,
ray.huang, linux-graphics-maintainer, sroland, zackr,
shashank.sharma, sam, emil.velikov, Felix.Kuehling, nirmoy.das
Cc: nouveau, amd-gfx, dri-devel, Thomas Zimmermann
Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.
This change also allows to support prime-based mmap via DRM's helper
drm_gem_prime_mmap().
Permission checks are implemented by drm_gem_mmap(), with an additional
check for radeon_ttm_tt_has_userptr() in the GEM object function. The
function radeon_verify_access() is now unused and has thus been removed.
As a side effect, radeon_ttm_vm_ops and radeon_ttm_fault() are now
implemented in amdgpu's GEM code.
v3:
* remove unnecessary checks from mmap (Christian)
v2:
* rename radeon_ttm_vm_ops and radeon_ttm_fault() to
radeon_gem_vm_ops and radeon_gem_fault() (Christian)
* fix commit description (Alex)
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/radeon/radeon_drv.c | 3 +-
drivers/gpu/drm/radeon/radeon_gem.c | 49 ++++++++++++++++++++++
drivers/gpu/drm/radeon/radeon_ttm.c | 65 -----------------------------
drivers/gpu/drm/radeon/radeon_ttm.h | 1 -
4 files changed, 51 insertions(+), 67 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 8885e849717d..61793ffdfa1b 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -558,7 +558,7 @@ static const struct file_operations radeon_driver_kms_fops = {
.open = drm_open,
.release = drm_release,
.unlocked_ioctl = radeon_drm_ioctl,
- .mmap = radeon_mmap,
+ .mmap = drm_gem_mmap,
.poll = drm_poll,
.read = drm_read,
#ifdef CONFIG_COMPAT
@@ -633,6 +633,7 @@ static const struct drm_driver kms_driver = {
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_import_sg_table = radeon_gem_prime_import_sg_table,
+ .gem_prime_mmap = drm_gem_prime_mmap,
.name = DRIVER_NAME,
.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index 05ea2f39f626..ff8849827d61 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -44,6 +44,42 @@ void radeon_gem_prime_unpin(struct drm_gem_object *obj);
const struct drm_gem_object_funcs radeon_gem_object_funcs;
+static vm_fault_t radeon_gem_fault(struct vm_fault *vmf)
+{
+ struct ttm_buffer_object *bo = vmf->vma->vm_private_data;
+ struct radeon_device *rdev = radeon_get_rdev(bo->bdev);
+ vm_fault_t ret;
+
+ down_read(&rdev->pm.mclk_lock);
+
+ ret = ttm_bo_vm_reserve(bo, vmf);
+ if (ret)
+ goto unlock_mclk;
+
+ ret = radeon_bo_fault_reserve_notify(bo);
+ if (ret)
+ goto unlock_resv;
+
+ ret = ttm_bo_vm_fault_reserved(vmf, vmf->vma->vm_page_prot,
+ TTM_BO_VM_NUM_PREFAULT, 1);
+ if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
+ goto unlock_mclk;
+
+unlock_resv:
+ dma_resv_unlock(bo->base.resv);
+
+unlock_mclk:
+ up_read(&rdev->pm.mclk_lock);
+ return ret;
+}
+
+static const struct vm_operations_struct radeon_gem_vm_ops = {
+ .fault = radeon_gem_fault,
+ .open = ttm_bo_vm_open,
+ .close = ttm_bo_vm_close,
+ .access = ttm_bo_vm_access
+};
+
static void radeon_gem_object_free(struct drm_gem_object *gobj)
{
struct radeon_bo *robj = gem_to_radeon_bo(gobj);
@@ -226,6 +262,17 @@ static int radeon_gem_handle_lockup(struct radeon_device *rdev, int r)
return r;
}
+static int radeon_gem_object_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
+{
+ struct radeon_bo *bo = gem_to_radeon_bo(obj);
+ struct radeon_device *rdev = radeon_get_rdev(bo->tbo.bdev);
+
+ if (radeon_ttm_tt_has_userptr(rdev, bo->tbo.ttm))
+ return -EPERM;
+
+ return drm_gem_ttm_mmap(obj, vma);
+}
+
const struct drm_gem_object_funcs radeon_gem_object_funcs = {
.free = radeon_gem_object_free,
.open = radeon_gem_object_open,
@@ -236,6 +283,8 @@ const struct drm_gem_object_funcs radeon_gem_object_funcs = {
.get_sg_table = radeon_gem_prime_get_sg_table,
.vmap = drm_gem_ttm_vmap,
.vunmap = drm_gem_ttm_vunmap,
+ .mmap = radeon_gem_object_mmap,
+ .vm_ops = &radeon_gem_vm_ops,
};
/*
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 380b3007fd0b..8b6efcc4191c 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -136,17 +136,6 @@ static void radeon_evict_flags(struct ttm_buffer_object *bo,
*placement = rbo->placement;
}
-static int radeon_verify_access(struct ttm_buffer_object *bo, struct file *filp)
-{
- struct radeon_bo *rbo = container_of(bo, struct radeon_bo, tbo);
- struct radeon_device *rdev = radeon_get_rdev(bo->bdev);
-
- if (radeon_ttm_tt_has_userptr(rdev, bo->ttm))
- return -EPERM;
- return drm_vma_node_verify_access(&rbo->tbo.base.vma_node,
- filp->private_data);
-}
-
static int radeon_move_blit(struct ttm_buffer_object *bo,
bool evict,
struct ttm_resource *new_mem,
@@ -705,7 +694,6 @@ static struct ttm_device_funcs radeon_bo_driver = {
.eviction_valuable = ttm_bo_eviction_valuable,
.evict_flags = &radeon_evict_flags,
.move = &radeon_bo_move,
- .verify_access = &radeon_verify_access,
.delete_mem_notify = &radeon_bo_delete_mem_notify,
.io_mem_reserve = &radeon_ttm_io_mem_reserve,
};
@@ -802,59 +790,6 @@ void radeon_ttm_set_active_vram_size(struct radeon_device *rdev, u64 size)
man->size = size >> PAGE_SHIFT;
}
-static vm_fault_t radeon_ttm_fault(struct vm_fault *vmf)
-{
- struct ttm_buffer_object *bo = vmf->vma->vm_private_data;
- struct radeon_device *rdev = radeon_get_rdev(bo->bdev);
- vm_fault_t ret;
-
- down_read(&rdev->pm.mclk_lock);
-
- ret = ttm_bo_vm_reserve(bo, vmf);
- if (ret)
- goto unlock_mclk;
-
- ret = radeon_bo_fault_reserve_notify(bo);
- if (ret)
- goto unlock_resv;
-
- ret = ttm_bo_vm_fault_reserved(vmf, vmf->vma->vm_page_prot,
- TTM_BO_VM_NUM_PREFAULT, 1);
- if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
- goto unlock_mclk;
-
-unlock_resv:
- dma_resv_unlock(bo->base.resv);
-
-unlock_mclk:
- up_read(&rdev->pm.mclk_lock);
- return ret;
-}
-
-static const struct vm_operations_struct radeon_ttm_vm_ops = {
- .fault = radeon_ttm_fault,
- .open = ttm_bo_vm_open,
- .close = ttm_bo_vm_close,
- .access = ttm_bo_vm_access
-};
-
-int radeon_mmap(struct file *filp, struct vm_area_struct *vma)
-{
- int r;
- struct drm_file *file_priv = filp->private_data;
- struct radeon_device *rdev = file_priv->minor->dev->dev_private;
-
- if (rdev == NULL)
- return -EINVAL;
-
- r = ttm_bo_mmap(filp, vma, &rdev->mman.bdev);
- if (unlikely(r != 0))
- return r;
-
- vma->vm_ops = &radeon_ttm_vm_ops;
- return 0;
-}
-
#if defined(CONFIG_DEBUG_FS)
static int radeon_mm_vram_dump_table_show(struct seq_file *m, void *unused)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.h b/drivers/gpu/drm/radeon/radeon_ttm.h
index 4d7b90ee2774..91ea7141bc81 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.h
+++ b/drivers/gpu/drm/radeon/radeon_ttm.h
@@ -32,6 +32,5 @@ struct radeon_device;
int radeon_ttm_init(struct radeon_device *rdev);
void radeon_ttm_fini(struct radeon_device *rdev);
-int radeon_mmap(struct file *filp, struct vm_area_struct *vma);
#endif /* __RADEON_TTM_H__ */
--
2.31.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 4/7] drm/nouveau: Implement mmap as GEM object function
2021-04-16 13:31 [PATCH v3 0/7] drm: Clean up mmap for TTM-based GEM drivers Thomas Zimmermann
` (2 preceding siblings ...)
2021-04-16 13:31 ` [PATCH v3 3/7] drm/radeon: " Thomas Zimmermann
@ 2021-04-16 13:31 ` Thomas Zimmermann
2021-04-16 13:31 ` [PATCH v3 5/7] drm/vmwgfx: Inline ttm_bo_mmap() into vmwgfx driver Thomas Zimmermann
` (3 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: Thomas Zimmermann @ 2021-04-16 13:31 UTC (permalink / raw)
To: alexander.deucher, christian.koenig, airlied, daniel, bskeggs,
ray.huang, linux-graphics-maintainer, sroland, zackr,
shashank.sharma, sam, emil.velikov, Felix.Kuehling, nirmoy.das
Cc: nouveau, amd-gfx, dri-devel, Thomas Zimmermann
Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.
The GEM object function is provided by GEM TTM helpers. Nouveau's
implementation of verify_access is unused and has been removed. Access
permissions are validated by the DRM helpers.
As a side effect, nouveau_ttm_vm_ops and nouveau_ttm_fault() are now
implemented in nouveau's GEM code.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/nouveau/nouveau_bo.c | 10 ------
drivers/gpu/drm/nouveau/nouveau_drm.c | 3 +-
drivers/gpu/drm/nouveau/nouveau_gem.c | 36 ++++++++++++++++++++
drivers/gpu/drm/nouveau/nouveau_ttm.c | 49 ---------------------------
drivers/gpu/drm/nouveau/nouveau_ttm.h | 1 -
5 files changed, 38 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 3e09df0472ce..bc67cbccc83b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1051,15 +1051,6 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
return ret;
}
-static int
-nouveau_bo_verify_access(struct ttm_buffer_object *bo, struct file *filp)
-{
- struct nouveau_bo *nvbo = nouveau_bo(bo);
-
- return drm_vma_node_verify_access(&nvbo->bo.base.vma_node,
- filp->private_data);
-}
-
static void
nouveau_ttm_io_mem_free_locked(struct nouveau_drm *drm,
struct ttm_resource *reg)
@@ -1332,7 +1323,6 @@ struct ttm_device_funcs nouveau_bo_driver = {
.evict_flags = nouveau_bo_evict_flags,
.delete_mem_notify = nouveau_bo_delete_mem_notify,
.move = nouveau_bo_move,
- .verify_access = nouveau_bo_verify_access,
.io_mem_reserve = &nouveau_ttm_io_mem_reserve,
.io_mem_free = &nouveau_ttm_io_mem_free,
};
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 3204fc0a90d2..a616cf4573b8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1179,7 +1179,7 @@ nouveau_driver_fops = {
.open = drm_open,
.release = drm_release,
.unlocked_ioctl = nouveau_drm_ioctl,
- .mmap = nouveau_ttm_mmap,
+ .mmap = drm_gem_mmap,
.poll = drm_poll,
.read = drm_read,
#if defined(CONFIG_COMPAT)
@@ -1212,6 +1212,7 @@ driver_stub = {
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_import_sg_table = nouveau_gem_prime_import_sg_table,
+ .gem_prime_mmap = drm_gem_prime_mmap,
.dumb_create = nouveau_display_dumb_create,
.dumb_map_offset = drm_gem_ttm_dumb_map_offset,
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
index c88cbb85f101..71dfac820c4d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -39,6 +39,40 @@
#include <nvif/class.h>
#include <nvif/push206e.h>
+static vm_fault_t nouveau_ttm_fault(struct vm_fault *vmf)
+{
+ struct vm_area_struct *vma = vmf->vma;
+ struct ttm_buffer_object *bo = vma->vm_private_data;
+ pgprot_t prot;
+ vm_fault_t ret;
+
+ ret = ttm_bo_vm_reserve(bo, vmf);
+ if (ret)
+ return ret;
+
+ ret = nouveau_ttm_fault_reserve_notify(bo);
+ if (ret)
+ goto error_unlock;
+
+ nouveau_bo_del_io_reserve_lru(bo);
+ prot = vm_get_page_prot(vma->vm_flags);
+ ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT, 1);
+ nouveau_bo_add_io_reserve_lru(bo);
+ if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
+ return ret;
+
+error_unlock:
+ dma_resv_unlock(bo->base.resv);
+ return ret;
+}
+
+static const struct vm_operations_struct nouveau_ttm_vm_ops = {
+ .fault = nouveau_ttm_fault,
+ .open = ttm_bo_vm_open,
+ .close = ttm_bo_vm_close,
+ .access = ttm_bo_vm_access
+};
+
void
nouveau_gem_object_del(struct drm_gem_object *gem)
{
@@ -180,6 +214,8 @@ const struct drm_gem_object_funcs nouveau_gem_object_funcs = {
.get_sg_table = nouveau_gem_prime_get_sg_table,
.vmap = drm_gem_ttm_vmap,
.vunmap = drm_gem_ttm_vunmap,
+ .mmap = drm_gem_ttm_mmap,
+ .vm_ops = &nouveau_ttm_vm_ops,
};
int
diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c
index b81ae90b8449..e511a26379da 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -127,55 +127,6 @@ const struct ttm_resource_manager_func nv04_gart_manager = {
.free = nouveau_manager_del,
};
-static vm_fault_t nouveau_ttm_fault(struct vm_fault *vmf)
-{
- struct vm_area_struct *vma = vmf->vma;
- struct ttm_buffer_object *bo = vma->vm_private_data;
- pgprot_t prot;
- vm_fault_t ret;
-
- ret = ttm_bo_vm_reserve(bo, vmf);
- if (ret)
- return ret;
-
- ret = nouveau_ttm_fault_reserve_notify(bo);
- if (ret)
- goto error_unlock;
-
- nouveau_bo_del_io_reserve_lru(bo);
- prot = vm_get_page_prot(vma->vm_flags);
- ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT, 1);
- nouveau_bo_add_io_reserve_lru(bo);
- if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
- return ret;
-
-error_unlock:
- dma_resv_unlock(bo->base.resv);
- return ret;
-}
-
-static const struct vm_operations_struct nouveau_ttm_vm_ops = {
- .fault = nouveau_ttm_fault,
- .open = ttm_bo_vm_open,
- .close = ttm_bo_vm_close,
- .access = ttm_bo_vm_access
-};
-
-int
-nouveau_ttm_mmap(struct file *filp, struct vm_area_struct *vma)
-{
- struct drm_file *file_priv = filp->private_data;
- struct nouveau_drm *drm = nouveau_drm(file_priv->minor->dev);
- int ret;
-
- ret = ttm_bo_mmap(filp, vma, &drm->ttm.bdev);
- if (ret)
- return ret;
-
- vma->vm_ops = &nouveau_ttm_vm_ops;
- return 0;
-}
-
static int
nouveau_ttm_init_host(struct nouveau_drm *drm, u8 kind)
{
diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.h b/drivers/gpu/drm/nouveau/nouveau_ttm.h
index dbf6dc238efd..2f0efda7ccdb 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.h
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.h
@@ -17,7 +17,6 @@ struct ttm_tt *nouveau_sgdma_create_ttm(struct ttm_buffer_object *bo,
int nouveau_ttm_init(struct nouveau_drm *drm);
void nouveau_ttm_fini(struct nouveau_drm *drm);
-int nouveau_ttm_mmap(struct file *, struct vm_area_struct *);
int nouveau_ttm_global_init(struct nouveau_drm *);
void nouveau_ttm_global_release(struct nouveau_drm *);
--
2.31.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 5/7] drm/vmwgfx: Inline ttm_bo_mmap() into vmwgfx driver
2021-04-16 13:31 [PATCH v3 0/7] drm: Clean up mmap for TTM-based GEM drivers Thomas Zimmermann
` (3 preceding siblings ...)
2021-04-16 13:31 ` [PATCH v3 4/7] drm/nouveau: " Thomas Zimmermann
@ 2021-04-16 13:31 ` Thomas Zimmermann
2021-04-16 13:46 ` Christian König
2021-04-16 13:31 ` [PATCH v3 6/7] drm/vmwgfx: Inline vmw_verify_access() Thomas Zimmermann
` (2 subsequent siblings)
7 siblings, 1 reply; 18+ messages in thread
From: Thomas Zimmermann @ 2021-04-16 13:31 UTC (permalink / raw)
To: alexander.deucher, christian.koenig, airlied, daniel, bskeggs,
ray.huang, linux-graphics-maintainer, sroland, zackr,
shashank.sharma, sam, emil.velikov, Felix.Kuehling, nirmoy.das
Cc: nouveau, amd-gfx, dri-devel, Thomas Zimmermann
The vmwgfx driver is the only remaining user of ttm_bo_mmap(). Inline
the code. The internal helper ttm_bo_vm_lookup() is now also part of
vmwgfx as vmw_bo_vm_lookup().
v2:
* replace pr_err() with drm_err() (Zack)
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Zack Rusin <zackr@vmware.com>
---
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 56 ++++++++++++++++++++++--
1 file changed, 53 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
index cb9975889e2f..c8b6543b4e39 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
@@ -27,6 +27,32 @@
#include "vmwgfx_drv.h"
+static struct ttm_buffer_object *vmw_bo_vm_lookup(struct ttm_device *bdev,
+ unsigned long offset,
+ unsigned long pages)
+{
+ struct vmw_private *dev_priv = container_of(bdev, struct vmw_private, bdev);
+ struct drm_device *drm = &dev_priv->drm;
+ struct drm_vma_offset_node *node;
+ struct ttm_buffer_object *bo = NULL;
+
+ drm_vma_offset_lock_lookup(bdev->vma_manager);
+
+ 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);
+
+ if (!bo)
+ drm_err(drm, "Could not find buffer object to map\n");
+
+ return bo;
+}
+
int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
{
static const struct vm_operations_struct vmw_vm_ops = {
@@ -41,10 +67,28 @@ int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
};
struct drm_file *file_priv = filp->private_data;
struct vmw_private *dev_priv = vmw_priv(file_priv->minor->dev);
- int ret = ttm_bo_mmap(filp, vma, &dev_priv->bdev);
+ struct ttm_device *bdev = &dev_priv->bdev;
+ struct ttm_buffer_object *bo;
+ int ret;
+
+ if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET_START))
+ return -EINVAL;
+
+ bo = vmw_bo_vm_lookup(bdev, vma->vm_pgoff, vma_pages(vma));
+ if (unlikely(!bo))
+ return -EINVAL;
- if (ret)
- return ret;
+ if (unlikely(!bo->bdev->funcs->verify_access)) {
+ ret = -EPERM;
+ goto out_unref;
+ }
+ ret = bo->bdev->funcs->verify_access(bo, filp);
+ if (unlikely(ret != 0))
+ goto out_unref;
+
+ ret = ttm_bo_mmap_obj(vma, bo);
+ if (unlikely(ret != 0))
+ goto out_unref;
vma->vm_ops = &vmw_vm_ops;
@@ -52,7 +96,13 @@ int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
if (!is_cow_mapping(vma->vm_flags))
vma->vm_flags = (vma->vm_flags & ~VM_MIXEDMAP) | VM_PFNMAP;
+ ttm_bo_put(bo); /* release extra ref taken by ttm_bo_mmap_obj() */
+
return 0;
+
+out_unref:
+ ttm_bo_put(bo);
+ return ret;
}
/* struct vmw_validation_mem callback */
--
2.31.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3 5/7] drm/vmwgfx: Inline ttm_bo_mmap() into vmwgfx driver
2021-04-16 13:31 ` [PATCH v3 5/7] drm/vmwgfx: Inline ttm_bo_mmap() into vmwgfx driver Thomas Zimmermann
@ 2021-04-16 13:46 ` Christian König
2021-04-16 13:51 ` Christian König
2021-04-16 14:00 ` Thomas Zimmermann
0 siblings, 2 replies; 18+ messages in thread
From: Christian König @ 2021-04-16 13:46 UTC (permalink / raw)
To: Thomas Zimmermann, alexander.deucher, christian.koenig, airlied,
daniel, bskeggs, ray.huang, linux-graphics-maintainer, sroland,
zackr, shashank.sharma, sam, emil.velikov, Felix.Kuehling,
nirmoy.das
Cc: nouveau, dri-devel, amd-gfx
Am 16.04.21 um 15:31 schrieb Thomas Zimmermann:
> The vmwgfx driver is the only remaining user of ttm_bo_mmap(). Inline
> the code. The internal helper ttm_bo_vm_lookup() is now also part of
> vmwgfx as vmw_bo_vm_lookup().
>
> v2:
> * replace pr_err() with drm_err() (Zack)
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Zack Rusin <zackr@vmware.com>
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 56 ++++++++++++++++++++++--
> 1 file changed, 53 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
> index cb9975889e2f..c8b6543b4e39 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
> @@ -27,6 +27,32 @@
>
> #include "vmwgfx_drv.h"
>
> +static struct ttm_buffer_object *vmw_bo_vm_lookup(struct ttm_device *bdev,
> + unsigned long offset,
> + unsigned long pages)
> +{
> + struct vmw_private *dev_priv = container_of(bdev, struct vmw_private, bdev);
> + struct drm_device *drm = &dev_priv->drm;
> + struct drm_vma_offset_node *node;
> + struct ttm_buffer_object *bo = NULL;
> +
> + drm_vma_offset_lock_lookup(bdev->vma_manager);
> +
> + 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);
> +
> + if (!bo)
> + drm_err(drm, "Could not find buffer object to map\n");
> +
> + return bo;
> +}
> +
> int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
> {
> static const struct vm_operations_struct vmw_vm_ops = {
> @@ -41,10 +67,28 @@ int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
> };
> struct drm_file *file_priv = filp->private_data;
> struct vmw_private *dev_priv = vmw_priv(file_priv->minor->dev);
> - int ret = ttm_bo_mmap(filp, vma, &dev_priv->bdev);
> + struct ttm_device *bdev = &dev_priv->bdev;
> + struct ttm_buffer_object *bo;
> + int ret;
> +
> + if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET_START))
> + return -EINVAL;
> +
> + bo = vmw_bo_vm_lookup(bdev, vma->vm_pgoff, vma_pages(vma));
> + if (unlikely(!bo))
> + return -EINVAL;
>
> - if (ret)
> - return ret;
> + if (unlikely(!bo->bdev->funcs->verify_access)) {
> + ret = -EPERM;
> + goto out_unref;
> + }
> + ret = bo->bdev->funcs->verify_access(bo, filp);
Is there any reason we can't call vmw_verify_access() directly here?
Would allow us to completely nuke the verify_access callback as well as
far as I can see.
Regards,
Christian.
> + if (unlikely(ret != 0))
> + goto out_unref;
> +
> + ret = ttm_bo_mmap_obj(vma, bo);
> + if (unlikely(ret != 0))
> + goto out_unref;
>
> vma->vm_ops = &vmw_vm_ops;
>
> @@ -52,7 +96,13 @@ int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
> if (!is_cow_mapping(vma->vm_flags))
> vma->vm_flags = (vma->vm_flags & ~VM_MIXEDMAP) | VM_PFNMAP;
>
> + ttm_bo_put(bo); /* release extra ref taken by ttm_bo_mmap_obj() */
> +
> return 0;
> +
> +out_unref:
> + ttm_bo_put(bo);
> + return ret;
> }
>
> /* struct vmw_validation_mem callback */
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 5/7] drm/vmwgfx: Inline ttm_bo_mmap() into vmwgfx driver
2021-04-16 13:46 ` Christian König
@ 2021-04-16 13:51 ` Christian König
2021-04-20 7:51 ` Thomas Zimmermann
2021-04-16 14:00 ` Thomas Zimmermann
1 sibling, 1 reply; 18+ messages in thread
From: Christian König @ 2021-04-16 13:51 UTC (permalink / raw)
To: Christian König, Thomas Zimmermann, alexander.deucher,
airlied, daniel, bskeggs, ray.huang, linux-graphics-maintainer,
sroland, zackr, shashank.sharma, sam, emil.velikov,
Felix.Kuehling, nirmoy.das
Cc: nouveau, dri-devel, amd-gfx
Am 16.04.21 um 15:46 schrieb Christian König:
> Am 16.04.21 um 15:31 schrieb Thomas Zimmermann:
>> The vmwgfx driver is the only remaining user of ttm_bo_mmap(). Inline
>> the code. The internal helper ttm_bo_vm_lookup() is now also part of
>> vmwgfx as vmw_bo_vm_lookup().
>>
>> v2:
>> * replace pr_err() with drm_err() (Zack)
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Reviewed-by: Zack Rusin <zackr@vmware.com>
>> ---
>> drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 56 ++++++++++++++++++++++--
>> 1 file changed, 53 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
>> b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
>> index cb9975889e2f..c8b6543b4e39 100644
>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
>> @@ -27,6 +27,32 @@
>> #include "vmwgfx_drv.h"
>> +static struct ttm_buffer_object *vmw_bo_vm_lookup(struct
>> ttm_device *bdev,
>> + unsigned long offset,
>> + unsigned long pages)
>> +{
>> + struct vmw_private *dev_priv = container_of(bdev, struct
>> vmw_private, bdev);
>> + struct drm_device *drm = &dev_priv->drm;
>> + struct drm_vma_offset_node *node;
>> + struct ttm_buffer_object *bo = NULL;
>> +
>> + drm_vma_offset_lock_lookup(bdev->vma_manager);
>> +
>> + 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);
>> +
>> + if (!bo)
>> + drm_err(drm, "Could not find buffer object to map\n");
>> +
>> + return bo;
>> +}
>> +
>> int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
>> {
>> static const struct vm_operations_struct vmw_vm_ops = {
>> @@ -41,10 +67,28 @@ int vmw_mmap(struct file *filp, struct
>> vm_area_struct *vma)
>> };
>> struct drm_file *file_priv = filp->private_data;
>> struct vmw_private *dev_priv = vmw_priv(file_priv->minor->dev);
>> - int ret = ttm_bo_mmap(filp, vma, &dev_priv->bdev);
>> + struct ttm_device *bdev = &dev_priv->bdev;
>> + struct ttm_buffer_object *bo;
>> + int ret;
>> +
>> + if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET_START))
>> + return -EINVAL;
>> +
>> + bo = vmw_bo_vm_lookup(bdev, vma->vm_pgoff, vma_pages(vma));
>> + if (unlikely(!bo))
>> + return -EINVAL;
>> - if (ret)
>> - return ret;
>> + if (unlikely(!bo->bdev->funcs->verify_access)) {
>> + ret = -EPERM;
>> + goto out_unref;
>> + }
>> + ret = bo->bdev->funcs->verify_access(bo, filp);
>
> Is there any reason we can't call vmw_verify_access() directly here?
>
> Would allow us to completely nuke the verify_access callback as well
> as far as I can see.
Forget what I said, couldn't see the next patch in my mailbox at time of
writing.
Whole series is Reviewed-by: Christian König <christian.koenig@amd.com>
Thanks for the nice cleanup,
Christian.
>
> Regards,
> Christian.
>
>> + if (unlikely(ret != 0))
>> + goto out_unref;
>> +
>> + ret = ttm_bo_mmap_obj(vma, bo);
>> + if (unlikely(ret != 0))
>> + goto out_unref;
>> vma->vm_ops = &vmw_vm_ops;
>> @@ -52,7 +96,13 @@ int vmw_mmap(struct file *filp, struct
>> vm_area_struct *vma)
>> if (!is_cow_mapping(vma->vm_flags))
>> vma->vm_flags = (vma->vm_flags & ~VM_MIXEDMAP) | VM_PFNMAP;
>> + ttm_bo_put(bo); /* release extra ref taken by
>> ttm_bo_mmap_obj() */
>> +
>> return 0;
>> +
>> +out_unref:
>> + ttm_bo_put(bo);
>> + return ret;
>> }
>> /* struct vmw_validation_mem callback */
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 5/7] drm/vmwgfx: Inline ttm_bo_mmap() into vmwgfx driver
2021-04-16 13:51 ` Christian König
@ 2021-04-20 7:51 ` Thomas Zimmermann
2021-04-20 8:51 ` Daniel Vetter
0 siblings, 1 reply; 18+ messages in thread
From: Thomas Zimmermann @ 2021-04-20 7:51 UTC (permalink / raw)
To: Christian König, Christian König, alexander.deucher,
airlied, daniel, bskeggs, ray.huang, linux-graphics-maintainer,
sroland, zackr, shashank.sharma, sam, emil.velikov,
Felix.Kuehling, nirmoy.das
Cc: nouveau, amd-gfx, dri-devel
[-- Attachment #1.1.1: Type: text/plain, Size: 5098 bytes --]
Hi
Am 16.04.21 um 15:51 schrieb Christian König:
> Am 16.04.21 um 15:46 schrieb Christian König:
>> Am 16.04.21 um 15:31 schrieb Thomas Zimmermann:
>>> The vmwgfx driver is the only remaining user of ttm_bo_mmap(). Inline
>>> the code. The internal helper ttm_bo_vm_lookup() is now also part of
>>> vmwgfx as vmw_bo_vm_lookup().
>>>
>>> v2:
>>> * replace pr_err() with drm_err() (Zack)
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Reviewed-by: Zack Rusin <zackr@vmware.com>
>>> ---
>>> drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 56 ++++++++++++++++++++++--
>>> 1 file changed, 53 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
>>> b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
>>> index cb9975889e2f..c8b6543b4e39 100644
>>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
>>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
>>> @@ -27,6 +27,32 @@
>>> #include "vmwgfx_drv.h"
>>> +static struct ttm_buffer_object *vmw_bo_vm_lookup(struct
>>> ttm_device *bdev,
>>> + unsigned long offset,
>>> + unsigned long pages)
>>> +{
>>> + struct vmw_private *dev_priv = container_of(bdev, struct
>>> vmw_private, bdev);
>>> + struct drm_device *drm = &dev_priv->drm;
>>> + struct drm_vma_offset_node *node;
>>> + struct ttm_buffer_object *bo = NULL;
>>> +
>>> + drm_vma_offset_lock_lookup(bdev->vma_manager);
>>> +
>>> + 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);
>>> +
>>> + if (!bo)
>>> + drm_err(drm, "Could not find buffer object to map\n");
>>> +
>>> + return bo;
>>> +}
>>> +
>>> int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
>>> {
>>> static const struct vm_operations_struct vmw_vm_ops = {
>>> @@ -41,10 +67,28 @@ int vmw_mmap(struct file *filp, struct
>>> vm_area_struct *vma)
>>> };
>>> struct drm_file *file_priv = filp->private_data;
>>> struct vmw_private *dev_priv = vmw_priv(file_priv->minor->dev);
>>> - int ret = ttm_bo_mmap(filp, vma, &dev_priv->bdev);
>>> + struct ttm_device *bdev = &dev_priv->bdev;
>>> + struct ttm_buffer_object *bo;
>>> + int ret;
>>> +
>>> + if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET_START))
>>> + return -EINVAL;
>>> +
>>> + bo = vmw_bo_vm_lookup(bdev, vma->vm_pgoff, vma_pages(vma));
>>> + if (unlikely(!bo))
>>> + return -EINVAL;
>>> - if (ret)
>>> - return ret;
>>> + if (unlikely(!bo->bdev->funcs->verify_access)) {
>>> + ret = -EPERM;
>>> + goto out_unref;
>>> + }
>>> + ret = bo->bdev->funcs->verify_access(bo, filp);
>>
>> Is there any reason we can't call vmw_verify_access() directly here?
>>
>> Would allow us to completely nuke the verify_access callback as well
>> as far as I can see.
>
> Forget what I said, couldn't see the next patch in my mailbox at time of
> writing.
>
> Whole series is Reviewed-by: Christian König <christian.koenig@amd.com>
Thanks a lot. If I'm not mistaken, the patches at [1] need to go in
first. So it could take a a bit until this lands.
Otherwise, this series could go through the same tree as [1] if nouveau
and vmwgfx devs don't mind.
Best regards
Thomas
[1] https://patchwork.freedesktop.org/series/88822/
>
> Thanks for the nice cleanup,
> Christian.
>
>>
>> Regards,
>> Christian.
>>
>>> + if (unlikely(ret != 0))
>>> + goto out_unref;
>>> +
>>> + ret = ttm_bo_mmap_obj(vma, bo);
>>> + if (unlikely(ret != 0))
>>> + goto out_unref;
>>> vma->vm_ops = &vmw_vm_ops;
>>> @@ -52,7 +96,13 @@ int vmw_mmap(struct file *filp, struct
>>> vm_area_struct *vma)
>>> if (!is_cow_mapping(vma->vm_flags))
>>> vma->vm_flags = (vma->vm_flags & ~VM_MIXEDMAP) | VM_PFNMAP;
>>> + ttm_bo_put(bo); /* release extra ref taken
by
>>> ttm_bo_mmap_obj() */
>>> +
>>> return 0;
>>> +
>>> +out_unref:
>>> + ttm_bo_put(bo);
>>> + return ret;
>>> }
>>> /* struct vmw_validation_mem callback */
>>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 5/7] drm/vmwgfx: Inline ttm_bo_mmap() into vmwgfx driver
2021-04-20 7:51 ` Thomas Zimmermann
@ 2021-04-20 8:51 ` Daniel Vetter
2021-04-20 20:22 ` Felix Kuehling
0 siblings, 1 reply; 18+ messages in thread
From: Daniel Vetter @ 2021-04-20 8:51 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: amd-gfx, shashank.sharma, airlied, Christian König,
Felix.Kuehling, sroland, nirmoy.das, dri-devel, ray.huang,
linux-graphics-maintainer, bskeggs, nouveau, alexander.deucher,
sam, Christian König, emil.velikov
On Tue, Apr 20, 2021 at 09:51:27AM +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 16.04.21 um 15:51 schrieb Christian König:
> > Am 16.04.21 um 15:46 schrieb Christian König:
> > > Am 16.04.21 um 15:31 schrieb Thomas Zimmermann:
> > > > The vmwgfx driver is the only remaining user of ttm_bo_mmap(). Inline
> > > > the code. The internal helper ttm_bo_vm_lookup() is now also part of
> > > > vmwgfx as vmw_bo_vm_lookup().
> > > >
> > > > v2:
> > > > * replace pr_err() with drm_err() (Zack)
> > > >
> > > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > > > Reviewed-by: Zack Rusin <zackr@vmware.com>
> > > > ---
> > > > drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 56 ++++++++++++++++++++++--
> > > > 1 file changed, 53 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
> > > > b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
> > > > index cb9975889e2f..c8b6543b4e39 100644
> > > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
> > > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
> > > > @@ -27,6 +27,32 @@
> > > > #include "vmwgfx_drv.h"
> > > > +static struct ttm_buffer_object *vmw_bo_vm_lookup(struct
> > > > ttm_device *bdev,
> > > > + unsigned long offset,
> > > > + unsigned long pages)
> > > > +{
> > > > + struct vmw_private *dev_priv = container_of(bdev, struct
> > > > vmw_private, bdev);
> > > > + struct drm_device *drm = &dev_priv->drm;
> > > > + struct drm_vma_offset_node *node;
> > > > + struct ttm_buffer_object *bo = NULL;
> > > > +
> > > > + drm_vma_offset_lock_lookup(bdev->vma_manager);
> > > > +
> > > > + 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);
> > > > +
> > > > + if (!bo)
> > > > + drm_err(drm, "Could not find buffer object to map\n");
> > > > +
> > > > + return bo;
> > > > +}
> > > > +
> > > > int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
> > > > {
> > > > static const struct vm_operations_struct vmw_vm_ops = {
> > > > @@ -41,10 +67,28 @@ int vmw_mmap(struct file *filp, struct
> > > > vm_area_struct *vma)
> > > > };
> > > > struct drm_file *file_priv = filp->private_data;
> > > > struct vmw_private *dev_priv = vmw_priv(file_priv->minor->dev);
> > > > - int ret = ttm_bo_mmap(filp, vma, &dev_priv->bdev);
> > > > + struct ttm_device *bdev = &dev_priv->bdev;
> > > > + struct ttm_buffer_object *bo;
> > > > + int ret;
> > > > +
> > > > + if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET_START))
> > > > + return -EINVAL;
> > > > +
> > > > + bo = vmw_bo_vm_lookup(bdev, vma->vm_pgoff, vma_pages(vma));
> > > > + if (unlikely(!bo))
> > > > + return -EINVAL;
> > > > - if (ret)
> > > > - return ret;
> > > > + if (unlikely(!bo->bdev->funcs->verify_access)) {
> > > > + ret = -EPERM;
> > > > + goto out_unref;
> > > > + }
> > > > + ret = bo->bdev->funcs->verify_access(bo, filp);
> > >
> > > Is there any reason we can't call vmw_verify_access() directly here?
> > >
> > > Would allow us to completely nuke the verify_access callback as well
> > > as far as I can see.
> >
> > Forget what I said, couldn't see the next patch in my mailbox at time of
> > writing.
> >
> > Whole series is Reviewed-by: Christian König <christian.koenig@amd.com>
>
> Thanks a lot. If I'm not mistaken, the patches at [1] need to go in first.
> So it could take a a bit until this lands.
>
> Otherwise, this series could go through the same tree as [1] if nouveau and
> vmwgfx devs don't mind.
I would land it all through drm-misc-next. Maybe check with Alex on irc
for an ack for merging that way, but I don't think this will cause issues
against the amdgpu tree. Lots of ttm cleanup has landed this way already
past few months. Otherwise you could create a small topic branch with
these patches here and send that to Alex, and he can sort out the
interaction with Felix' series.
-Daniel
>
> Best regards
> Thomas
>
> [1] https://patchwork.freedesktop.org/series/88822/
>
> >
> > Thanks for the nice cleanup,
> > Christian.
> >
> > >
> > > Regards,
> > > Christian.
> > >
> > > > + if (unlikely(ret != 0))
> > > > + goto out_unref;
> > > > +
> > > > + ret = ttm_bo_mmap_obj(vma, bo);
> > > > + if (unlikely(ret != 0))
> > > > + goto out_unref;
> > > > vma->vm_ops = &vmw_vm_ops;
> > > > @@ -52,7 +96,13 @@ int vmw_mmap(struct file *filp, struct
> > > > vm_area_struct *vma)
> > > > if (!is_cow_mapping(vma->vm_flags))
> > > > vma->vm_flags = (vma->vm_flags & ~VM_MIXEDMAP) | VM_PFNMAP;
> > > > + ttm_bo_put(bo); /* release extra ref taken
> by
> > > > ttm_bo_mmap_obj() */
> > > > +
> > > > return 0;
> > > > +
> > > > +out_unref:
> > > > + ttm_bo_put(bo);
> > > > + return ret;
> > > > }
> > > > /* struct vmw_validation_mem callback */
> > >
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 5/7] drm/vmwgfx: Inline ttm_bo_mmap() into vmwgfx driver
2021-04-20 8:51 ` Daniel Vetter
@ 2021-04-20 20:22 ` Felix Kuehling
2021-04-20 20:53 ` Daniel Vetter
0 siblings, 1 reply; 18+ messages in thread
From: Felix Kuehling @ 2021-04-20 20:22 UTC (permalink / raw)
To: Daniel Vetter, Thomas Zimmermann
Cc: amd-gfx, shashank.sharma, airlied, Christian König, sroland,
nirmoy.das, dri-devel, ray.huang, linux-graphics-maintainer,
bskeggs, nouveau, alexander.deucher, sam, Christian König,
emil.velikov
Am 2021-04-20 um 4:51 a.m. schrieb Daniel Vetter:
>>> Whole series is Reviewed-by: Christian König <christian.koenig@amd.com>
>> Thanks a lot. If I'm not mistaken, the patches at [1] need to go in first.
>> So it could take a a bit until this lands.
>>
>> Otherwise, this series could go through the same tree as [1] if nouveau and
>> vmwgfx devs don't mind.
> I would land it all through drm-misc-next. Maybe check with Alex on irc
> for an ack for merging that way, but I don't think this will cause issues
> against the amdgpu tree. Lots of ttm cleanup has landed this way already
> past few months. Otherwise you could create a small topic branch with
> these patches here and send that to Alex, and he can sort out the
> interaction with Felix' series.
> -Daniel
My patch series involved some pretty far-reaching changes in KFD
(renaming some variables in KFD and amdgpu, changing the KFD->amdgpu
interface). We already submitted other patches on top of it that have
dependencies on it. If we decide to deliver this through a different
tree and remove it from amd-staging-drm-next, there will be conflicts to
resolve when removing it from amd-staging-drm-next, and again the next
time you merge with amd-staging-drm-next.
Regards,
Felix
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 5/7] drm/vmwgfx: Inline ttm_bo_mmap() into vmwgfx driver
2021-04-20 20:22 ` Felix Kuehling
@ 2021-04-20 20:53 ` Daniel Vetter
2021-04-21 7:01 ` Christian König
0 siblings, 1 reply; 18+ messages in thread
From: Daniel Vetter @ 2021-04-20 20:53 UTC (permalink / raw)
To: Felix Kuehling
Cc: amd-gfx list, Sharma, Shashank, Dave Airlie,
Christian König, Roland Scheidegger, Nirmoy Das, dri-devel,
Huang Rui, VMware Graphics, Ben Skeggs, Thomas Zimmermann,
Nouveau Dev, Alex Deucher, Sam Ravnborg, Christian König,
Emil Velikov
On Tue, Apr 20, 2021 at 10:23 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>
>
> Am 2021-04-20 um 4:51 a.m. schrieb Daniel Vetter:
> >>> Whole series is Reviewed-by: Christian König <christian.koenig@amd.com>
> >> Thanks a lot. If I'm not mistaken, the patches at [1] need to go in first.
> >> So it could take a a bit until this lands.
> >>
> >> Otherwise, this series could go through the same tree as [1] if nouveau and
> >> vmwgfx devs don't mind.
> > I would land it all through drm-misc-next. Maybe check with Alex on irc
> > for an ack for merging that way, but I don't think this will cause issues
> > against the amdgpu tree. Lots of ttm cleanup has landed this way already
> > past few months. Otherwise you could create a small topic branch with
> > these patches here and send that to Alex, and he can sort out the
> > interaction with Felix' series.
> > -Daniel
>
> My patch series involved some pretty far-reaching changes in KFD
> (renaming some variables in KFD and amdgpu, changing the KFD->amdgpu
> interface). We already submitted other patches on top of it that have
> dependencies on it. If we decide to deliver this through a different
> tree and remove it from amd-staging-drm-next, there will be conflicts to
> resolve when removing it from amd-staging-drm-next, and again the next
> time you merge with amd-staging-drm-next.
Ah then the usual way is for Alex to assemble a topic pull request
(stable, non-rebasing) with those select patches, which then gets
merged into drm-misc-next. Or we smash it all into amdgpu-next. Or we
just wait until -rc2 when drm-next is back open for business.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 5/7] drm/vmwgfx: Inline ttm_bo_mmap() into vmwgfx driver
2021-04-20 20:53 ` Daniel Vetter
@ 2021-04-21 7:01 ` Christian König
2021-04-21 9:12 ` Daniel Vetter
0 siblings, 1 reply; 18+ messages in thread
From: Christian König @ 2021-04-21 7:01 UTC (permalink / raw)
To: Daniel Vetter, Felix Kuehling
Cc: amd-gfx list, Sharma, Shashank, Dave Airlie,
Christian König, Roland Scheidegger, Nirmoy Das, dri-devel,
Huang Rui, VMware Graphics, Ben Skeggs, Thomas Zimmermann,
Nouveau Dev, Alex Deucher, Sam Ravnborg, Emil Velikov
Am 20.04.21 um 22:53 schrieb Daniel Vetter:
> On Tue, Apr 20, 2021 at 10:23 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>>
>> Am 2021-04-20 um 4:51 a.m. schrieb Daniel Vetter:
>>>>> Whole series is Reviewed-by: Christian König <christian.koenig@amd.com>
>>>> Thanks a lot. If I'm not mistaken, the patches at [1] need to go in first.
>>>> So it could take a a bit until this lands.
>>>>
>>>> Otherwise, this series could go through the same tree as [1] if nouveau and
>>>> vmwgfx devs don't mind.
>>> I would land it all through drm-misc-next. Maybe check with Alex on irc
>>> for an ack for merging that way, but I don't think this will cause issues
>>> against the amdgpu tree. Lots of ttm cleanup has landed this way already
>>> past few months. Otherwise you could create a small topic branch with
>>> these patches here and send that to Alex, and he can sort out the
>>> interaction with Felix' series.
>>> -Daniel
>> My patch series involved some pretty far-reaching changes in KFD
>> (renaming some variables in KFD and amdgpu, changing the KFD->amdgpu
>> interface). We already submitted other patches on top of it that have
>> dependencies on it. If we decide to deliver this through a different
>> tree and remove it from amd-staging-drm-next, there will be conflicts to
>> resolve when removing it from amd-staging-drm-next, and again the next
>> time you merge with amd-staging-drm-next.
> Ah then the usual way is for Alex to assemble a topic pull request
> (stable, non-rebasing) with those select patches, which then gets
> merged into drm-misc-next. Or we smash it all into amdgpu-next. Or we
> just wait until -rc2 when drm-next is back open for business.
I need to double check, but if I'm not totally mistaken the changes in
question should already be in drm-next.
But exactly that was the reason why I asked when the next backmerge from
drm-next into drm-misc-next is planned :)
Christian.
> -Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 5/7] drm/vmwgfx: Inline ttm_bo_mmap() into vmwgfx driver
2021-04-21 7:01 ` Christian König
@ 2021-04-21 9:12 ` Daniel Vetter
0 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2021-04-21 9:12 UTC (permalink / raw)
To: Christian König
Cc: amd-gfx list, Sharma, Shashank, Dave Airlie,
Christian König, Felix Kuehling, Roland Scheidegger,
Nirmoy Das, dri-devel, Huang Rui, VMware Graphics, Ben Skeggs,
Thomas Zimmermann, Nouveau Dev, Alex Deucher, Sam Ravnborg,
Emil Velikov
On Wed, Apr 21, 2021 at 09:01:00AM +0200, Christian König wrote:
> Am 20.04.21 um 22:53 schrieb Daniel Vetter:
> > On Tue, Apr 20, 2021 at 10:23 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
> > >
> > > Am 2021-04-20 um 4:51 a.m. schrieb Daniel Vetter:
> > > > > > Whole series is Reviewed-by: Christian König <christian.koenig@amd.com>
> > > > > Thanks a lot. If I'm not mistaken, the patches at [1] need to go in first.
> > > > > So it could take a a bit until this lands.
> > > > >
> > > > > Otherwise, this series could go through the same tree as [1] if nouveau and
> > > > > vmwgfx devs don't mind.
> > > > I would land it all through drm-misc-next. Maybe check with Alex on irc
> > > > for an ack for merging that way, but I don't think this will cause issues
> > > > against the amdgpu tree. Lots of ttm cleanup has landed this way already
> > > > past few months. Otherwise you could create a small topic branch with
> > > > these patches here and send that to Alex, and he can sort out the
> > > > interaction with Felix' series.
> > > > -Daniel
> > > My patch series involved some pretty far-reaching changes in KFD
> > > (renaming some variables in KFD and amdgpu, changing the KFD->amdgpu
> > > interface). We already submitted other patches on top of it that have
> > > dependencies on it. If we decide to deliver this through a different
> > > tree and remove it from amd-staging-drm-next, there will be conflicts to
> > > resolve when removing it from amd-staging-drm-next, and again the next
> > > time you merge with amd-staging-drm-next.
> > Ah then the usual way is for Alex to assemble a topic pull request
> > (stable, non-rebasing) with those select patches, which then gets
> > merged into drm-misc-next. Or we smash it all into amdgpu-next. Or we
> > just wait until -rc2 when drm-next is back open for business.
>
> I need to double check, but if I'm not totally mistaken the changes in
> question should already be in drm-next.
>
> But exactly that was the reason why I asked when the next backmerge from
> drm-next into drm-misc-next is planned :)
Best way to make that happen isn't to ask when it will happen, but tell
drm-misc maintainers to do it and why (ideally with references to the
commits you need). Also I tend to do those requests over irc.
Backmerges are generally on demand, not on schedule. Hence you need to
demand one :-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 5/7] drm/vmwgfx: Inline ttm_bo_mmap() into vmwgfx driver
2021-04-16 13:46 ` Christian König
2021-04-16 13:51 ` Christian König
@ 2021-04-16 14:00 ` Thomas Zimmermann
1 sibling, 0 replies; 18+ messages in thread
From: Thomas Zimmermann @ 2021-04-16 14:00 UTC (permalink / raw)
To: Christian König, alexander.deucher, christian.koenig,
airlied, daniel, bskeggs, ray.huang, linux-graphics-maintainer,
sroland, zackr, shashank.sharma, sam, emil.velikov,
Felix.Kuehling, nirmoy.das
Cc: nouveau, amd-gfx, dri-devel
[-- Attachment #1.1.1: Type: text/plain, Size: 4489 bytes --]
Hi
Am 16.04.21 um 15:46 schrieb Christian König:
> Am 16.04.21 um 15:31 schrieb Thomas Zimmermann:
>> The vmwgfx driver is the only remaining user of ttm_bo_mmap(). Inline
>> the code. The internal helper ttm_bo_vm_lookup() is now also part of
>> vmwgfx as vmw_bo_vm_lookup().
>>
>> v2:
>> * replace pr_err() with drm_err() (Zack)
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Reviewed-by: Zack Rusin <zackr@vmware.com>
>> ---
>> drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 56 ++++++++++++++++++++++--
>> 1 file changed, 53 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
>> b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
>> index cb9975889e2f..c8b6543b4e39 100644
>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
>> @@ -27,6 +27,32 @@
>> #include "vmwgfx_drv.h"
>> +static struct ttm_buffer_object *vmw_bo_vm_lookup(struct ttm_device
>> *bdev,
>> + unsigned long offset,
>> + unsigned long pages)
>> +{
>> + struct vmw_private *dev_priv = container_of(bdev, struct
>> vmw_private, bdev);
>> + struct drm_device *drm = &dev_priv->drm;
>> + struct drm_vma_offset_node *node;
>> + struct ttm_buffer_object *bo = NULL;
>> +
>> + drm_vma_offset_lock_lookup(bdev->vma_manager);
>> +
>> + 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);
>> +
>> + if (!bo)
>> + drm_err(drm, "Could not find buffer object to map\n");
>> +
>> + return bo;
>> +}
>> +
>> int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
>> {
>> static const struct vm_operations_struct vmw_vm_ops = {
>> @@ -41,10 +67,28 @@ int vmw_mmap(struct file *filp, struct
>> vm_area_struct *vma)
>> };
>> struct drm_file *file_priv = filp->private_data;
>> struct vmw_private *dev_priv = vmw_priv(file_priv->minor->dev);
>> - int ret = ttm_bo_mmap(filp, vma, &dev_priv->bdev);
>> + struct ttm_device *bdev = &dev_priv->bdev;
>> + struct ttm_buffer_object *bo;
>> + int ret;
>> +
>> + if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET_START))
>> + return -EINVAL;
>> +
>> + bo = vmw_bo_vm_lookup(bdev, vma->vm_pgoff, vma_pages(vma));
>> + if (unlikely(!bo))
>> + return -EINVAL;
>> - if (ret)
>> - return ret;
>> + if (unlikely(!bo->bdev->funcs->verify_access)) {
>> + ret = -EPERM;
>> + goto out_unref;
>> + }
>> + ret = bo->bdev->funcs->verify_access(bo, filp);
>
> Is there any reason we can't call vmw_verify_access() directly here?
>
> Would allow us to completely nuke the verify_access callback as well as
> far as I can see.
We have to spare something for patches 6 and 7. Otherwise they'd be
empty. ;)
Best regards
Thomas
>
> Regards,
> Christian.
>
>> + if (unlikely(ret != 0))
>> + goto out_unref;
>> +
>> + ret = ttm_bo_mmap_obj(vma, bo);
>> + if (unlikely(ret != 0))
>> + goto out_unref;
>> vma->vm_ops = &vmw_vm_ops;
>> @@ -52,7 +96,13 @@ int vmw_mmap(struct file *filp, struct
>> vm_area_struct *vma)
>> if (!is_cow_mapping(vma->vm_flags))
>> vma->vm_flags =
(vma->vm_flags & ~VM_MIXEDMAP) | VM_PFNMAP;
>> + ttm_bo_put(bo); /* release extra ref taken by ttm_bo_mmap_obj() */
>> +
>> return 0;
>> +
>> +out_unref:
>> + ttm_bo_put(bo);
>> + return ret;
>> }
>> /* struct vmw_validation_mem callback */
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 6/7] drm/vmwgfx: Inline vmw_verify_access()
2021-04-16 13:31 [PATCH v3 0/7] drm: Clean up mmap for TTM-based GEM drivers Thomas Zimmermann
` (4 preceding siblings ...)
2021-04-16 13:31 ` [PATCH v3 5/7] drm/vmwgfx: Inline ttm_bo_mmap() into vmwgfx driver Thomas Zimmermann
@ 2021-04-16 13:31 ` Thomas Zimmermann
2021-04-16 13:31 ` [PATCH v3 7/7] drm/ttm: Remove ttm_bo_mmap() and friends Thomas Zimmermann
2021-04-16 13:44 ` [PATCH v3 0/7] drm: Clean up mmap for TTM-based GEM drivers Christian König
7 siblings, 0 replies; 18+ messages in thread
From: Thomas Zimmermann @ 2021-04-16 13:31 UTC (permalink / raw)
To: alexander.deucher, christian.koenig, airlied, daniel, bskeggs,
ray.huang, linux-graphics-maintainer, sroland, zackr,
shashank.sharma, sam, emil.velikov, Felix.Kuehling, nirmoy.das
Cc: nouveau, amd-gfx, dri-devel, Thomas Zimmermann
Vmwgfx is the only user of the TTM's verify_access callback. Inline
the call and avoid the indirection through the function pointer.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Zack Rusin <zackr@vmware.com>
---
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 9 ---------
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 7 ++-----
2 files changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 2dc031fe4a90..a079734f9d68 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -658,14 +658,6 @@ static void vmw_evict_flags(struct ttm_buffer_object *bo,
*placement = vmw_sys_placement;
}
-static int vmw_verify_access(struct ttm_buffer_object *bo, struct file *filp)
-{
- struct ttm_object_file *tfile =
- vmw_fpriv((struct drm_file *)filp->private_data)->tfile;
-
- return vmw_user_bo_verify_access(bo, tfile);
-}
-
static int vmw_ttm_io_mem_reserve(struct ttm_device *bdev, struct ttm_resource *mem)
{
struct vmw_private *dev_priv = container_of(bdev, struct vmw_private, bdev);
@@ -768,7 +760,6 @@ struct ttm_device_funcs vmw_bo_driver = {
.eviction_valuable = ttm_bo_eviction_valuable,
.evict_flags = vmw_evict_flags,
.move = vmw_move,
- .verify_access = vmw_verify_access,
.swap_notify = vmw_swap_notify,
.io_mem_reserve = &vmw_ttm_io_mem_reserve,
};
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
index c8b6543b4e39..e6b1f98ec99f 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
@@ -67,6 +67,7 @@ int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
};
struct drm_file *file_priv = filp->private_data;
struct vmw_private *dev_priv = vmw_priv(file_priv->minor->dev);
+ struct ttm_object_file *tfile = vmw_fpriv(file_priv)->tfile;
struct ttm_device *bdev = &dev_priv->bdev;
struct ttm_buffer_object *bo;
int ret;
@@ -78,11 +79,7 @@ int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
if (unlikely(!bo))
return -EINVAL;
- if (unlikely(!bo->bdev->funcs->verify_access)) {
- ret = -EPERM;
- goto out_unref;
- }
- ret = bo->bdev->funcs->verify_access(bo, filp);
+ ret = vmw_user_bo_verify_access(bo, tfile);
if (unlikely(ret != 0))
goto out_unref;
--
2.31.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 7/7] drm/ttm: Remove ttm_bo_mmap() and friends
2021-04-16 13:31 [PATCH v3 0/7] drm: Clean up mmap for TTM-based GEM drivers Thomas Zimmermann
` (5 preceding siblings ...)
2021-04-16 13:31 ` [PATCH v3 6/7] drm/vmwgfx: Inline vmw_verify_access() Thomas Zimmermann
@ 2021-04-16 13:31 ` Thomas Zimmermann
2021-04-16 13:44 ` [PATCH v3 0/7] drm: Clean up mmap for TTM-based GEM drivers Christian König
7 siblings, 0 replies; 18+ messages in thread
From: Thomas Zimmermann @ 2021-04-16 13:31 UTC (permalink / raw)
To: alexander.deucher, christian.koenig, airlied, daniel, bskeggs,
ray.huang, linux-graphics-maintainer, sroland, zackr,
shashank.sharma, sam, emil.velikov, Felix.Kuehling, nirmoy.das
Cc: nouveau, amd-gfx, dri-devel, Thomas Zimmermann
The function ttm_bo_mmap is unused. Remove it and it's helpers; including
the verify_access callback in struct ttm_device_funcs.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/ttm/ttm_bo_vm.c | 53 ---------------------------------
include/drm/ttm/ttm_bo_api.h | 13 --------
include/drm/ttm/ttm_device.h | 15 ----------
3 files changed, 81 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index bf4a213bc66c..6cd352399941 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -508,30 +508,6 @@ static const struct vm_operations_struct ttm_bo_vm_ops = {
.access = ttm_bo_vm_access,
};
-static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_device *bdev,
- unsigned long offset,
- unsigned long pages)
-{
- struct drm_vma_offset_node *node;
- struct ttm_buffer_object *bo = NULL;
-
- drm_vma_offset_lock_lookup(bdev->vma_manager);
-
- 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);
-
- if (!bo)
- pr_err("Could not find buffer object to map\n");
-
- return bo;
-}
-
static void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct vm_area_struct *vma)
{
/*
@@ -559,35 +535,6 @@ static void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct vm_area_s
vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
}
-int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,
- struct ttm_device *bdev)
-{
- struct ttm_buffer_object *bo;
- int ret;
-
- if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET_START))
- return -EINVAL;
-
- bo = ttm_bo_vm_lookup(bdev, vma->vm_pgoff, vma_pages(vma));
- if (unlikely(!bo))
- return -EINVAL;
-
- if (unlikely(!bo->bdev->funcs->verify_access)) {
- ret = -EPERM;
- goto out_unref;
- }
- ret = bo->bdev->funcs->verify_access(bo, filp);
- if (unlikely(ret != 0))
- goto out_unref;
-
- ttm_bo_mmap_vma_setup(bo, vma);
- return 0;
-out_unref:
- ttm_bo_put(bo);
- return ret;
-}
-EXPORT_SYMBOL(ttm_bo_mmap);
-
int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo)
{
ttm_bo_get(bo);
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 2155e2e38aec..6e35680ac01b 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -522,19 +522,6 @@ void ttm_bo_vunmap(struct ttm_buffer_object *bo, struct dma_buf_map *map);
*/
int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo);
-/**
- * ttm_bo_mmap - mmap out of the ttm device address space.
- *
- * @filp: filp as input from the mmap method.
- * @vma: vma as input from the mmap method.
- * @bdev: Pointer to the ttm_device with the address space manager.
- *
- * This function is intended to be called by the device mmap method.
- * if the device address space is to be backed by the bo manager.
- */
-int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,
- struct ttm_device *bdev);
-
/**
* ttm_bo_io
*
diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
index 7c8f87bd52d3..cd592f8e941b 100644
--- a/include/drm/ttm/ttm_device.h
+++ b/include/drm/ttm/ttm_device.h
@@ -161,21 +161,6 @@ struct ttm_device_funcs {
struct ttm_resource *new_mem,
struct ttm_place *hop);
- /**
- * struct ttm_bo_driver_member verify_access
- *
- * @bo: Pointer to a buffer object.
- * @filp: Pointer to a struct file trying to access the object.
- *
- * Called from the map / write / read methods to verify that the
- * caller is permitted to access the buffer object.
- * This member may be set to NULL, which will refuse this kind of
- * access for all buffer objects.
- * This function should return 0 if access is granted, -EPERM otherwise.
- */
- int (*verify_access)(struct ttm_buffer_object *bo,
- struct file *filp);
-
/**
* Hook to notify driver about a resource delete.
*/
--
2.31.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3 0/7] drm: Clean up mmap for TTM-based GEM drivers
2021-04-16 13:31 [PATCH v3 0/7] drm: Clean up mmap for TTM-based GEM drivers Thomas Zimmermann
` (6 preceding siblings ...)
2021-04-16 13:31 ` [PATCH v3 7/7] drm/ttm: Remove ttm_bo_mmap() and friends Thomas Zimmermann
@ 2021-04-16 13:44 ` Christian König
7 siblings, 0 replies; 18+ messages in thread
From: Christian König @ 2021-04-16 13:44 UTC (permalink / raw)
To: Thomas Zimmermann, alexander.deucher, christian.koenig, airlied,
daniel, bskeggs, ray.huang, linux-graphics-maintainer, sroland,
zackr, shashank.sharma, sam, emil.velikov, Felix.Kuehling,
nirmoy.das
Cc: nouveau, dri-devel, amd-gfx
Patches #1-#4 are Reviewed-by: Christian König <christian.koenig@amd.com>
One more comment on the vmw patches.
Regards,
Christian.
Am 16.04.21 um 15:31 schrieb Thomas Zimmermann:
> Implement mmap via struct drm_gem_object_functions.mmap in amdgpu,
> radeon and nouveau. This allows for using common DRM helpers for
> the mmap-related callbacks in struct file_operations and struct
> drm_driver. The drivers have their own vm_ops, which are now set
> automatically by the DRM core functions. The code in each driver's
> verify_access becomes part of the driver's new mmap implementation.
>
> With the GEM drivers converted, vmwgfx is the only user of
> ttm_bo_mmap() and related infrastructure. So move everything into
> vmwgfx and delete the rsp code from TTM.
>
> This touches several drivers. Preferably everything would be merged
> at once via drm-misc-next.
>
> v3:
> * tidy up the new mmap functions in amdgpu and radeon (Christian)
> v2:
> * removal of amdgpu fbdev mmap already merged (Christian)
> * rebase on top of amdgpu fixes [1] (Felix)
> * replace pr_err() with drm_err() in vmwgfx patch (Zack)
> * several typos
>
> [1] https://patchwork.freedesktop.org/series/88822/
>
> Thomas Zimmermann (7):
> drm/ttm: Don't override vm_ops callbacks, if set
> drm/amdgpu: Implement mmap as GEM object function
> drm/radeon: Implement mmap as GEM object function
> drm/nouveau: Implement mmap as GEM object function
> drm/vmwgfx: Inline ttm_bo_mmap() into vmwgfx driver
> drm/vmwgfx: Inline vmw_verify_access()
> drm/ttm: Remove ttm_bo_mmap() and friends
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 46 ---------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h | 2 -
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 44 ++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 64 --------------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 -
> drivers/gpu/drm/nouveau/nouveau_bo.c | 10 ----
> drivers/gpu/drm/nouveau/nouveau_drm.c | 3 +-
> drivers/gpu/drm/nouveau/nouveau_gem.c | 36 ++++++++++++
> drivers/gpu/drm/nouveau/nouveau_ttm.c | 49 ----------------
> drivers/gpu/drm/nouveau/nouveau_ttm.h | 1 -
> drivers/gpu/drm/radeon/radeon_drv.c | 3 +-
> drivers/gpu/drm/radeon/radeon_gem.c | 49 ++++++++++++++++
> drivers/gpu/drm/radeon/radeon_ttm.c | 65 ---------------------
> drivers/gpu/drm/radeon/radeon_ttm.h | 1 -
> drivers/gpu/drm/ttm/ttm_bo_vm.c | 60 ++-----------------
> drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 9 ---
> drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 53 ++++++++++++++++-
> include/drm/ttm/ttm_bo_api.h | 13 -----
> include/drm/ttm/ttm_device.h | 15 -----
> 20 files changed, 191 insertions(+), 337 deletions(-)
>
> --
> 2.31.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread