* [PATCH 2/8] drm/radeon: use GEM references instead of TTMs
2024-07-23 12:17 [PATCH 1/8] drm/amdgpu: use GEM references instead of TTMs Christian König
@ 2024-07-23 12:17 ` Christian König
2024-07-25 12:02 ` Daniel Vetter
2024-07-23 12:17 ` [PATCH 3/8] drm/loongson: " Christian König
` (6 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Christian König @ 2024-07-23 12:17 UTC (permalink / raw)
To: matthew.brost, thomas.hellstrom; +Cc: dri-devel
Instead of a TTM reference grab a GEM reference whenever necessary.
Signed-off-by: Christian König <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: amd-gfx@lists.freedesktop.org
---
drivers/gpu/drm/radeon/radeon_gem.c | 2 +-
drivers/gpu/drm/radeon/radeon_object.c | 7 ++-----
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index 2ef201a072f1..05df49b3a5c6 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -88,7 +88,7 @@ static void radeon_gem_object_free(struct drm_gem_object *gobj)
if (robj) {
radeon_mn_unregister(robj);
- radeon_bo_unref(&robj);
+ ttm_bo_put(&robj->tbo);
}
}
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index a955f8a2f7fe..4b972c8c9516 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -256,18 +256,15 @@ struct radeon_bo *radeon_bo_ref(struct radeon_bo *bo)
if (bo == NULL)
return NULL;
- ttm_bo_get(&bo->tbo);
+ drm_gem_object_get(&bo->tbo.base);
return bo;
}
void radeon_bo_unref(struct radeon_bo **bo)
{
- struct ttm_buffer_object *tbo;
-
if ((*bo) == NULL)
return;
- tbo = &((*bo)->tbo);
- ttm_bo_put(tbo);
+ drm_gem_object_put(&(*bo)->tbo.base);
*bo = NULL;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 2/8] drm/radeon: use GEM references instead of TTMs
2024-07-23 12:17 ` [PATCH 2/8] drm/radeon: " Christian König
@ 2024-07-25 12:02 ` Daniel Vetter
0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2024-07-25 12:02 UTC (permalink / raw)
To: Christian König; +Cc: matthew.brost, thomas.hellstrom, dri-devel
On Tue, Jul 23, 2024 at 02:17:44PM +0200, Christian König wrote:
> Instead of a TTM reference grab a GEM reference whenever necessary.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/radeon/radeon_gem.c | 2 +-
> drivers/gpu/drm/radeon/radeon_object.c | 7 ++-----
> 2 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
> index 2ef201a072f1..05df49b3a5c6 100644
> --- a/drivers/gpu/drm/radeon/radeon_gem.c
> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
> @@ -88,7 +88,7 @@ static void radeon_gem_object_free(struct drm_gem_object *gobj)
>
> if (robj) {
> radeon_mn_unregister(robj);
> - radeon_bo_unref(&robj);
> + ttm_bo_put(&robj->tbo);
> }
> }
>
> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> index a955f8a2f7fe..4b972c8c9516 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.c
> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> @@ -256,18 +256,15 @@ struct radeon_bo *radeon_bo_ref(struct radeon_bo *bo)
> if (bo == NULL)
> return NULL;
>
> - ttm_bo_get(&bo->tbo);
> + drm_gem_object_get(&bo->tbo.base);
> return bo;
> }
>
> void radeon_bo_unref(struct radeon_bo **bo)
> {
> - struct ttm_buffer_object *tbo;
> -
> if ((*bo) == NULL)
> return;
> - tbo = &((*bo)->tbo);
> - ttm_bo_put(tbo);
> + drm_gem_object_put(&(*bo)->tbo.base);
> *bo = NULL;
> }
>
> --
> 2.34.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/8] drm/loongson: use GEM references instead of TTMs
2024-07-23 12:17 [PATCH 1/8] drm/amdgpu: use GEM references instead of TTMs Christian König
2024-07-23 12:17 ` [PATCH 2/8] drm/radeon: " Christian König
@ 2024-07-23 12:17 ` Christian König
2024-07-25 12:03 ` Daniel Vetter
2024-07-23 12:17 ` [PATCH 4/8] drm/vmwgfx: " Christian König
` (5 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Christian König @ 2024-07-23 12:17 UTC (permalink / raw)
To: matthew.brost, thomas.hellstrom; +Cc: dri-devel
Instead of a TTM reference grab a GEM reference whenever necessary.
Signed-off-by: Christian König <christian.koenig@amd.com>
Cc: Sui Jingfeng <suijingfeng@loongson.cn>
---
drivers/gpu/drm/loongson/lsdc_ttm.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/loongson/lsdc_ttm.c b/drivers/gpu/drm/loongson/lsdc_ttm.c
index 465f622ac05d..2e42c6970c9f 100644
--- a/drivers/gpu/drm/loongson/lsdc_ttm.c
+++ b/drivers/gpu/drm/loongson/lsdc_ttm.c
@@ -341,16 +341,12 @@ void lsdc_bo_unpin(struct lsdc_bo *lbo)
void lsdc_bo_ref(struct lsdc_bo *lbo)
{
- struct ttm_buffer_object *tbo = &lbo->tbo;
-
- ttm_bo_get(tbo);
+ drm_gem_object_get(&lbo->tbo.base);
}
void lsdc_bo_unref(struct lsdc_bo *lbo)
{
- struct ttm_buffer_object *tbo = &lbo->tbo;
-
- ttm_bo_put(tbo);
+ drm_gem_object_put(&lbo->tbo.base);
}
int lsdc_bo_kmap(struct lsdc_bo *lbo)
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 3/8] drm/loongson: use GEM references instead of TTMs
2024-07-23 12:17 ` [PATCH 3/8] drm/loongson: " Christian König
@ 2024-07-25 12:03 ` Daniel Vetter
0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2024-07-25 12:03 UTC (permalink / raw)
To: Christian König; +Cc: matthew.brost, thomas.hellstrom, dri-devel
On Tue, Jul 23, 2024 at 02:17:45PM +0200, Christian König wrote:
> Instead of a TTM reference grab a GEM reference whenever necessary.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Cc: Sui Jingfeng <suijingfeng@loongson.cn>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/loongson/lsdc_ttm.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/loongson/lsdc_ttm.c b/drivers/gpu/drm/loongson/lsdc_ttm.c
> index 465f622ac05d..2e42c6970c9f 100644
> --- a/drivers/gpu/drm/loongson/lsdc_ttm.c
> +++ b/drivers/gpu/drm/loongson/lsdc_ttm.c
> @@ -341,16 +341,12 @@ void lsdc_bo_unpin(struct lsdc_bo *lbo)
>
> void lsdc_bo_ref(struct lsdc_bo *lbo)
> {
> - struct ttm_buffer_object *tbo = &lbo->tbo;
> -
> - ttm_bo_get(tbo);
> + drm_gem_object_get(&lbo->tbo.base);
> }
>
> void lsdc_bo_unref(struct lsdc_bo *lbo)
> {
> - struct ttm_buffer_object *tbo = &lbo->tbo;
> -
> - ttm_bo_put(tbo);
> + drm_gem_object_put(&lbo->tbo.base);
> }
>
> int lsdc_bo_kmap(struct lsdc_bo *lbo)
> --
> 2.34.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/8] drm/vmwgfx: use GEM references instead of TTMs
2024-07-23 12:17 [PATCH 1/8] drm/amdgpu: use GEM references instead of TTMs Christian König
2024-07-23 12:17 ` [PATCH 2/8] drm/radeon: " Christian König
2024-07-23 12:17 ` [PATCH 3/8] drm/loongson: " Christian König
@ 2024-07-23 12:17 ` Christian König
2024-07-25 12:16 ` Daniel Vetter
2024-07-23 12:17 ` [PATCH 5/8] drm/vmwgfx: partially revert "Adapt validation code for reference-free lookups" Christian König
` (4 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Christian König @ 2024-07-23 12:17 UTC (permalink / raw)
To: matthew.brost, thomas.hellstrom; +Cc: dri-devel
Instead of a TTM reference grab a GEM reference whenever necessary.
Signed-off-by: Christian König <christian.koenig@amd.com>
Cc: Zack Rusin <zack.rusin@broadcom.com>
Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_bo.h | 19 ++-----------------
drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c | 4 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 4 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 6 +++---
drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 16 ++++++++--------
drivers/gpu/drm/vmwgfx/vmwgfx_shader.c | 4 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 2 +-
9 files changed, 23 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 00144632c600..3caaacb124ec 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -501,7 +501,7 @@ static int vmw_user_bo_synccpu_release(struct drm_file *filp,
if (!(flags & drm_vmw_synccpu_allow_cs)) {
atomic_dec(&vmw_bo->cpu_writers);
}
- vmw_user_bo_unref(&vmw_bo);
+ vmw_bo_unreference(&vmw_bo);
}
return ret;
@@ -543,7 +543,7 @@ int vmw_user_bo_synccpu_ioctl(struct drm_device *dev, void *data,
return ret;
ret = vmw_user_bo_synccpu_grab(vbo, arg->flags);
- vmw_user_bo_unref(&vbo);
+ vmw_bo_unreference(&vbo);
if (unlikely(ret != 0)) {
if (ret == -ERESTARTSYS || ret == -EBUSY)
return -EBUSY;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
index f349642e6190..42a58d57409d 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
@@ -188,30 +188,15 @@ static inline void vmw_bo_unreference(struct vmw_bo **buf)
*buf = NULL;
if (tmp_buf)
- ttm_bo_put(&tmp_buf->tbo);
+ drm_gem_object_put(&tmp_buf->tbo.base);
}
static inline struct vmw_bo *vmw_bo_reference(struct vmw_bo *buf)
{
- ttm_bo_get(&buf->tbo);
+ drm_gem_object_get(&buf->tbo.base);
return buf;
}
-static inline struct vmw_bo *vmw_user_bo_ref(struct vmw_bo *vbo)
-{
- drm_gem_object_get(&vbo->tbo.base);
- return vbo;
-}
-
-static inline void vmw_user_bo_unref(struct vmw_bo **buf)
-{
- struct vmw_bo *tmp_buf = *buf;
-
- *buf = NULL;
- if (tmp_buf)
- drm_gem_object_put(&tmp_buf->tbo.base);
-}
-
static inline struct vmw_bo *to_vmw_bo(struct drm_gem_object *gobj)
{
return container_of((gobj), struct vmw_bo, tbo.base);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
index a7c07692262b..6b700ebd71b1 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
@@ -502,7 +502,7 @@ static int vmw_cotable_resize(struct vmw_resource *res, size_t new_size)
vmw_resource_mob_attach(res);
/* Let go of the old mob. */
- vmw_user_bo_unref(&old_buf);
+ vmw_bo_unreference(&old_buf);
res->id = vcotbl->type;
ret = dma_resv_reserve_fences(bo->base.resv, 1);
@@ -521,7 +521,7 @@ static int vmw_cotable_resize(struct vmw_resource *res, size_t new_size)
out_wait:
ttm_bo_unpin(bo);
ttm_bo_unreserve(bo);
- vmw_user_bo_unref(&buf);
+ vmw_bo_unreference(&buf);
out_done:
MKS_STAT_TIME_POP(MKSSTAT_KERN_COTABLE_RESIZE);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
index 2e52d73eba48..be29b9ff8901 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
@@ -1166,7 +1166,7 @@ static int vmw_translate_mob_ptr(struct vmw_private *dev_priv,
vmw_bo_placement_set(vmw_bo, VMW_BO_DOMAIN_MOB, VMW_BO_DOMAIN_MOB);
ret = vmw_validation_add_bo(sw_context->ctx, vmw_bo);
tmp_bo = vmw_bo;
- vmw_user_bo_unref(&tmp_bo);
+ vmw_bo_unreference(&tmp_bo);
if (unlikely(ret != 0))
return ret;
@@ -1223,7 +1223,7 @@ static int vmw_translate_guest_ptr(struct vmw_private *dev_priv,
VMW_BO_DOMAIN_GMR | VMW_BO_DOMAIN_VRAM);
ret = vmw_validation_add_bo(sw_context->ctx, vmw_bo);
tmp_bo = vmw_bo;
- vmw_user_bo_unref(&tmp_bo);
+ vmw_bo_unreference(&tmp_bo);
if (unlikely(ret != 0))
return ret;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 13b2820cae51..f8158b8d2843 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1476,8 +1476,8 @@ static int vmw_create_bo_proxy(struct drm_device *dev,
/* Reserve and switch the backing mob. */
mutex_lock(&res->dev_priv->cmdbuf_mutex);
(void) vmw_resource_reserve(res, false, true);
- vmw_user_bo_unref(&res->guest_memory_bo);
- res->guest_memory_bo = vmw_user_bo_ref(bo_mob);
+ vmw_bo_unreference(&res->guest_memory_bo);
+ res->guest_memory_bo = vmw_bo_reference(bo_mob);
res->guest_memory_offset = 0;
vmw_resource_unreserve(res, false, false, false, NULL, 0);
mutex_unlock(&res->dev_priv->cmdbuf_mutex);
@@ -1670,7 +1670,7 @@ static struct drm_framebuffer *vmw_kms_fb_create(struct drm_device *dev,
err_out:
/* vmw_user_lookup_handle takes one ref so does new_fb */
if (bo)
- vmw_user_bo_unref(&bo);
+ vmw_bo_unreference(&bo);
if (surface)
vmw_surface_unreference(&surface);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c b/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
index c45b4724e414..c5860f62ec53 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
@@ -451,7 +451,7 @@ int vmw_overlay_ioctl(struct drm_device *dev, void *data,
ret = vmw_overlay_update_stream(dev_priv, buf, arg, true);
- vmw_user_bo_unref(&buf);
+ vmw_bo_unreference(&buf);
out_unlock:
mutex_unlock(&overlay->mutex);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
index 848dba09981b..cb786da99fd3 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
@@ -141,7 +141,7 @@ static void vmw_resource_release(struct kref *kref)
if (res->coherent)
vmw_bo_dirty_release(res->guest_memory_bo);
ttm_bo_unreserve(bo);
- vmw_user_bo_unref(&res->guest_memory_bo);
+ vmw_bo_unreference(&res->guest_memory_bo);
}
if (likely(res->hw_destroy != NULL)) {
@@ -457,11 +457,11 @@ void vmw_resource_unreserve(struct vmw_resource *res,
vmw_resource_mob_detach(res);
if (res->coherent)
vmw_bo_dirty_release(res->guest_memory_bo);
- vmw_user_bo_unref(&res->guest_memory_bo);
+ vmw_bo_unreference(&res->guest_memory_bo);
}
if (new_guest_memory_bo) {
- res->guest_memory_bo = vmw_user_bo_ref(new_guest_memory_bo);
+ res->guest_memory_bo = vmw_bo_reference(new_guest_memory_bo);
/*
* The validation code should already have added a
@@ -522,7 +522,7 @@ vmw_resource_check_buffer(struct ww_acquire_ctx *ticket,
}
INIT_LIST_HEAD(&val_list);
- ttm_bo_get(&res->guest_memory_bo->tbo);
+ drm_gem_object_get(&res->guest_memory_bo->tbo.base);
val_buf->bo = &res->guest_memory_bo->tbo;
val_buf->num_shared = 0;
list_add_tail(&val_buf->head, &val_list);
@@ -548,10 +548,10 @@ vmw_resource_check_buffer(struct ww_acquire_ctx *ticket,
out_no_validate:
ttm_eu_backoff_reservation(ticket, &val_list);
out_no_reserve:
- ttm_bo_put(val_buf->bo);
+ drm_gem_object_put(&val_buf->bo->base);
val_buf->bo = NULL;
if (guest_memory_dirty)
- vmw_user_bo_unref(&res->guest_memory_bo);
+ vmw_bo_unreference(&res->guest_memory_bo);
return ret;
}
@@ -610,7 +610,7 @@ vmw_resource_backoff_reservation(struct ww_acquire_ctx *ticket,
INIT_LIST_HEAD(&val_list);
list_add_tail(&val_buf->head, &val_list);
ttm_eu_backoff_reservation(ticket, &val_list);
- ttm_bo_put(val_buf->bo);
+ drm_gem_object_put(&val_buf->bo->base);
val_buf->bo = NULL;
}
@@ -727,7 +727,7 @@ int vmw_resource_validate(struct vmw_resource *res, bool intr,
goto out_no_validate;
else if (!res->func->needs_guest_memory && res->guest_memory_bo) {
WARN_ON_ONCE(vmw_resource_mob_attached(res));
- vmw_user_bo_unref(&res->guest_memory_bo);
+ vmw_bo_unreference(&res->guest_memory_bo);
}
return 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
index a01ca3226d0a..6b8e984695ed 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
@@ -180,7 +180,7 @@ static int vmw_gb_shader_init(struct vmw_private *dev_priv,
res->guest_memory_size = size;
if (byte_code) {
- res->guest_memory_bo = vmw_user_bo_ref(byte_code);
+ res->guest_memory_bo = vmw_bo_reference(byte_code);
res->guest_memory_offset = offset;
}
shader->size = size;
@@ -809,7 +809,7 @@ static int vmw_shader_define(struct drm_device *dev, struct drm_file *file_priv,
shader_type, num_input_sig,
num_output_sig, tfile, shader_handle);
out_bad_arg:
- vmw_user_bo_unref(&buffer);
+ vmw_bo_unreference(&buffer);
return ret;
}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index e7a744dfcecf..c275bb074a60 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -1505,7 +1505,7 @@ vmw_gb_surface_define_internal(struct drm_device *dev,
if (ret == 0) {
if (res->guest_memory_bo->tbo.base.size < res->guest_memory_size) {
VMW_DEBUG_USER("Surface backup buffer too small.\n");
- vmw_user_bo_unref(&res->guest_memory_bo);
+ vmw_bo_unreference(&res->guest_memory_bo);
ret = -EINVAL;
goto out_unlock;
} else {
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 4/8] drm/vmwgfx: use GEM references instead of TTMs
2024-07-23 12:17 ` [PATCH 4/8] drm/vmwgfx: " Christian König
@ 2024-07-25 12:16 ` Daniel Vetter
0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2024-07-25 12:16 UTC (permalink / raw)
To: Christian König; +Cc: matthew.brost, thomas.hellstrom, dri-devel
On Tue, Jul 23, 2024 at 02:17:46PM +0200, Christian König wrote:
> Instead of a TTM reference grab a GEM reference whenever necessary.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Cc: Zack Rusin <zack.rusin@broadcom.com>
> Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
There's a ttm_bo_put in vmwgfx_mob.c that you didn't convert, is there a
reason for that? And the mix of vmw_bo_(un)reference and
drm_gem_object_get/put in vmwgfx_resource.c is a bit confusing, I'd just
refcount the res->guest_memory_bo instead so that you can use the vmw_bo_
functions.
-Sima
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 ++--
> drivers/gpu/drm/vmwgfx/vmwgfx_bo.h | 19 ++-----------------
> drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c | 4 ++--
> drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 4 ++--
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 6 +++---
> drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c | 2 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 16 ++++++++--------
> drivers/gpu/drm/vmwgfx/vmwgfx_shader.c | 4 ++--
> drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 2 +-
> 9 files changed, 23 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> index 00144632c600..3caaacb124ec 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> @@ -501,7 +501,7 @@ static int vmw_user_bo_synccpu_release(struct drm_file *filp,
> if (!(flags & drm_vmw_synccpu_allow_cs)) {
> atomic_dec(&vmw_bo->cpu_writers);
> }
> - vmw_user_bo_unref(&vmw_bo);
> + vmw_bo_unreference(&vmw_bo);
> }
>
> return ret;
> @@ -543,7 +543,7 @@ int vmw_user_bo_synccpu_ioctl(struct drm_device *dev, void *data,
> return ret;
>
> ret = vmw_user_bo_synccpu_grab(vbo, arg->flags);
> - vmw_user_bo_unref(&vbo);
> + vmw_bo_unreference(&vbo);
> if (unlikely(ret != 0)) {
> if (ret == -ERESTARTSYS || ret == -EBUSY)
> return -EBUSY;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
> index f349642e6190..42a58d57409d 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
> @@ -188,30 +188,15 @@ static inline void vmw_bo_unreference(struct vmw_bo **buf)
>
> *buf = NULL;
> if (tmp_buf)
> - ttm_bo_put(&tmp_buf->tbo);
> + drm_gem_object_put(&tmp_buf->tbo.base);
> }
>
> static inline struct vmw_bo *vmw_bo_reference(struct vmw_bo *buf)
> {
> - ttm_bo_get(&buf->tbo);
> + drm_gem_object_get(&buf->tbo.base);
> return buf;
> }
>
> -static inline struct vmw_bo *vmw_user_bo_ref(struct vmw_bo *vbo)
> -{
> - drm_gem_object_get(&vbo->tbo.base);
> - return vbo;
> -}
> -
> -static inline void vmw_user_bo_unref(struct vmw_bo **buf)
> -{
> - struct vmw_bo *tmp_buf = *buf;
> -
> - *buf = NULL;
> - if (tmp_buf)
> - drm_gem_object_put(&tmp_buf->tbo.base);
> -}
> -
> static inline struct vmw_bo *to_vmw_bo(struct drm_gem_object *gobj)
> {
> return container_of((gobj), struct vmw_bo, tbo.base);
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
> index a7c07692262b..6b700ebd71b1 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
> @@ -502,7 +502,7 @@ static int vmw_cotable_resize(struct vmw_resource *res, size_t new_size)
>
> vmw_resource_mob_attach(res);
> /* Let go of the old mob. */
> - vmw_user_bo_unref(&old_buf);
> + vmw_bo_unreference(&old_buf);
> res->id = vcotbl->type;
>
> ret = dma_resv_reserve_fences(bo->base.resv, 1);
> @@ -521,7 +521,7 @@ static int vmw_cotable_resize(struct vmw_resource *res, size_t new_size)
> out_wait:
> ttm_bo_unpin(bo);
> ttm_bo_unreserve(bo);
> - vmw_user_bo_unref(&buf);
> + vmw_bo_unreference(&buf);
>
> out_done:
> MKS_STAT_TIME_POP(MKSSTAT_KERN_COTABLE_RESIZE);
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
> index 2e52d73eba48..be29b9ff8901 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
> @@ -1166,7 +1166,7 @@ static int vmw_translate_mob_ptr(struct vmw_private *dev_priv,
> vmw_bo_placement_set(vmw_bo, VMW_BO_DOMAIN_MOB, VMW_BO_DOMAIN_MOB);
> ret = vmw_validation_add_bo(sw_context->ctx, vmw_bo);
> tmp_bo = vmw_bo;
> - vmw_user_bo_unref(&tmp_bo);
> + vmw_bo_unreference(&tmp_bo);
> if (unlikely(ret != 0))
> return ret;
>
> @@ -1223,7 +1223,7 @@ static int vmw_translate_guest_ptr(struct vmw_private *dev_priv,
> VMW_BO_DOMAIN_GMR | VMW_BO_DOMAIN_VRAM);
> ret = vmw_validation_add_bo(sw_context->ctx, vmw_bo);
> tmp_bo = vmw_bo;
> - vmw_user_bo_unref(&tmp_bo);
> + vmw_bo_unreference(&tmp_bo);
> if (unlikely(ret != 0))
> return ret;
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 13b2820cae51..f8158b8d2843 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -1476,8 +1476,8 @@ static int vmw_create_bo_proxy(struct drm_device *dev,
> /* Reserve and switch the backing mob. */
> mutex_lock(&res->dev_priv->cmdbuf_mutex);
> (void) vmw_resource_reserve(res, false, true);
> - vmw_user_bo_unref(&res->guest_memory_bo);
> - res->guest_memory_bo = vmw_user_bo_ref(bo_mob);
> + vmw_bo_unreference(&res->guest_memory_bo);
> + res->guest_memory_bo = vmw_bo_reference(bo_mob);
> res->guest_memory_offset = 0;
> vmw_resource_unreserve(res, false, false, false, NULL, 0);
> mutex_unlock(&res->dev_priv->cmdbuf_mutex);
> @@ -1670,7 +1670,7 @@ static struct drm_framebuffer *vmw_kms_fb_create(struct drm_device *dev,
> err_out:
> /* vmw_user_lookup_handle takes one ref so does new_fb */
> if (bo)
> - vmw_user_bo_unref(&bo);
> + vmw_bo_unreference(&bo);
> if (surface)
> vmw_surface_unreference(&surface);
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c b/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
> index c45b4724e414..c5860f62ec53 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
> @@ -451,7 +451,7 @@ int vmw_overlay_ioctl(struct drm_device *dev, void *data,
>
> ret = vmw_overlay_update_stream(dev_priv, buf, arg, true);
>
> - vmw_user_bo_unref(&buf);
> + vmw_bo_unreference(&buf);
>
> out_unlock:
> mutex_unlock(&overlay->mutex);
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> index 848dba09981b..cb786da99fd3 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> @@ -141,7 +141,7 @@ static void vmw_resource_release(struct kref *kref)
> if (res->coherent)
> vmw_bo_dirty_release(res->guest_memory_bo);
> ttm_bo_unreserve(bo);
> - vmw_user_bo_unref(&res->guest_memory_bo);
> + vmw_bo_unreference(&res->guest_memory_bo);
> }
>
> if (likely(res->hw_destroy != NULL)) {
> @@ -457,11 +457,11 @@ void vmw_resource_unreserve(struct vmw_resource *res,
> vmw_resource_mob_detach(res);
> if (res->coherent)
> vmw_bo_dirty_release(res->guest_memory_bo);
> - vmw_user_bo_unref(&res->guest_memory_bo);
> + vmw_bo_unreference(&res->guest_memory_bo);
> }
>
> if (new_guest_memory_bo) {
> - res->guest_memory_bo = vmw_user_bo_ref(new_guest_memory_bo);
> + res->guest_memory_bo = vmw_bo_reference(new_guest_memory_bo);
>
> /*
> * The validation code should already have added a
> @@ -522,7 +522,7 @@ vmw_resource_check_buffer(struct ww_acquire_ctx *ticket,
> }
>
> INIT_LIST_HEAD(&val_list);
> - ttm_bo_get(&res->guest_memory_bo->tbo);
> + drm_gem_object_get(&res->guest_memory_bo->tbo.base);
> val_buf->bo = &res->guest_memory_bo->tbo;
> val_buf->num_shared = 0;
> list_add_tail(&val_buf->head, &val_list);
> @@ -548,10 +548,10 @@ vmw_resource_check_buffer(struct ww_acquire_ctx *ticket,
> out_no_validate:
> ttm_eu_backoff_reservation(ticket, &val_list);
> out_no_reserve:
> - ttm_bo_put(val_buf->bo);
> + drm_gem_object_put(&val_buf->bo->base);
> val_buf->bo = NULL;
> if (guest_memory_dirty)
> - vmw_user_bo_unref(&res->guest_memory_bo);
> + vmw_bo_unreference(&res->guest_memory_bo);
>
> return ret;
> }
> @@ -610,7 +610,7 @@ vmw_resource_backoff_reservation(struct ww_acquire_ctx *ticket,
> INIT_LIST_HEAD(&val_list);
> list_add_tail(&val_buf->head, &val_list);
> ttm_eu_backoff_reservation(ticket, &val_list);
> - ttm_bo_put(val_buf->bo);
> + drm_gem_object_put(&val_buf->bo->base);
> val_buf->bo = NULL;
> }
>
> @@ -727,7 +727,7 @@ int vmw_resource_validate(struct vmw_resource *res, bool intr,
> goto out_no_validate;
> else if (!res->func->needs_guest_memory && res->guest_memory_bo) {
> WARN_ON_ONCE(vmw_resource_mob_attached(res));
> - vmw_user_bo_unref(&res->guest_memory_bo);
> + vmw_bo_unreference(&res->guest_memory_bo);
> }
>
> return 0;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
> index a01ca3226d0a..6b8e984695ed 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
> @@ -180,7 +180,7 @@ static int vmw_gb_shader_init(struct vmw_private *dev_priv,
>
> res->guest_memory_size = size;
> if (byte_code) {
> - res->guest_memory_bo = vmw_user_bo_ref(byte_code);
> + res->guest_memory_bo = vmw_bo_reference(byte_code);
> res->guest_memory_offset = offset;
> }
> shader->size = size;
> @@ -809,7 +809,7 @@ static int vmw_shader_define(struct drm_device *dev, struct drm_file *file_priv,
> shader_type, num_input_sig,
> num_output_sig, tfile, shader_handle);
> out_bad_arg:
> - vmw_user_bo_unref(&buffer);
> + vmw_bo_unreference(&buffer);
> return ret;
> }
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> index e7a744dfcecf..c275bb074a60 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> @@ -1505,7 +1505,7 @@ vmw_gb_surface_define_internal(struct drm_device *dev,
> if (ret == 0) {
> if (res->guest_memory_bo->tbo.base.size < res->guest_memory_size) {
> VMW_DEBUG_USER("Surface backup buffer too small.\n");
> - vmw_user_bo_unref(&res->guest_memory_bo);
> + vmw_bo_unreference(&res->guest_memory_bo);
> ret = -EINVAL;
> goto out_unlock;
> } else {
> --
> 2.34.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 5/8] drm/vmwgfx: partially revert "Adapt validation code for reference-free lookups"
2024-07-23 12:17 [PATCH 1/8] drm/amdgpu: use GEM references instead of TTMs Christian König
` (2 preceding siblings ...)
2024-07-23 12:17 ` [PATCH 4/8] drm/vmwgfx: " Christian König
@ 2024-07-23 12:17 ` Christian König
2024-07-25 12:34 ` Daniel Vetter
2024-08-27 10:03 ` Thomas Hellström
2024-07-23 12:17 ` [PATCH 6/8] drm/ttm: revert "Export ttm_bo_get_unless_zero()" Christian König
` (3 subsequent siblings)
7 siblings, 2 replies; 16+ messages in thread
From: Christian König @ 2024-07-23 12:17 UTC (permalink / raw)
To: matthew.brost, thomas.hellstrom; +Cc: dri-devel
This reverts commit 64ad2abfe9a628ce79859d072704bd1ef7682044.
To me it looks like this functionality was never actually used. At least
I can't find any protection in vmw_bo_free().
Signed-off-by: Christian König <christian.koenig@amd.com>
Cc: Zack Rusin <zack.rusin@broadcom.com>
Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
---
drivers/gpu/drm/vmwgfx/vmwgfx_validation.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
index e7625b3f71e0..e11837e484aa 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
@@ -262,7 +262,8 @@ int vmw_validation_add_bo(struct vmw_validation_context *ctx,
bo_node->hash.key);
}
val_buf = &bo_node->base;
- val_buf->bo = ttm_bo_get_unless_zero(&vbo->tbo);
+ vmw_bo_reference(vbo);
+ val_buf->bo = &vbo->tbo;
if (!val_buf->bo)
return -ESRCH;
val_buf->num_shared = 0;
@@ -656,7 +657,7 @@ void vmw_validation_unref_lists(struct vmw_validation_context *ctx)
struct vmw_validation_res_node *val;
list_for_each_entry(entry, &ctx->bo_list, base.head) {
- ttm_bo_put(entry->base.bo);
+ drm_gem_object_put(&entry->base.bo->base);
entry->base.bo = NULL;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 5/8] drm/vmwgfx: partially revert "Adapt validation code for reference-free lookups"
2024-07-23 12:17 ` [PATCH 5/8] drm/vmwgfx: partially revert "Adapt validation code for reference-free lookups" Christian König
@ 2024-07-25 12:34 ` Daniel Vetter
2024-08-27 10:03 ` Thomas Hellström
1 sibling, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2024-07-25 12:34 UTC (permalink / raw)
To: Christian König; +Cc: matthew.brost, thomas.hellstrom, dri-devel
On Tue, Jul 23, 2024 at 02:17:47PM +0200, Christian König wrote:
> This reverts commit 64ad2abfe9a628ce79859d072704bd1ef7682044.
>
> To me it looks like this functionality was never actually used. At least
> I can't find any protection in vmw_bo_free().
Just to double-check I've done the audit of all callers, and they all look
like they're holding a full reference indeed. The somewhat annoying case
was vmw_sw_context->cur_query_bo because it seems to not be refcounted
itself. But that's either dev_priv->pinned_bo or
dev_priv->dummy_query_bo, both of which are refcounted, so we're good.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Cc: Zack Rusin <zack.rusin@broadcom.com>
> Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_validation.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
> index e7625b3f71e0..e11837e484aa 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
> @@ -262,7 +262,8 @@ int vmw_validation_add_bo(struct vmw_validation_context *ctx,
> bo_node->hash.key);
> }
> val_buf = &bo_node->base;
> - val_buf->bo = ttm_bo_get_unless_zero(&vbo->tbo);
> + vmw_bo_reference(vbo);
> + val_buf->bo = &vbo->tbo;
> if (!val_buf->bo)
> return -ESRCH;
> val_buf->num_shared = 0;
> @@ -656,7 +657,7 @@ void vmw_validation_unref_lists(struct vmw_validation_context *ctx)
> struct vmw_validation_res_node *val;
>
> list_for_each_entry(entry, &ctx->bo_list, base.head) {
> - ttm_bo_put(entry->base.bo);
> + drm_gem_object_put(&entry->base.bo->base);
> entry->base.bo = NULL;
> }
>
> --
> 2.34.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 5/8] drm/vmwgfx: partially revert "Adapt validation code for reference-free lookups"
2024-07-23 12:17 ` [PATCH 5/8] drm/vmwgfx: partially revert "Adapt validation code for reference-free lookups" Christian König
2024-07-25 12:34 ` Daniel Vetter
@ 2024-08-27 10:03 ` Thomas Hellström
1 sibling, 0 replies; 16+ messages in thread
From: Thomas Hellström @ 2024-08-27 10:03 UTC (permalink / raw)
To: Christian König, matthew.brost; +Cc: dri-devel
On Tue, 2024-07-23 at 14:17 +0200, Christian König wrote:
> This reverts commit 64ad2abfe9a628ce79859d072704bd1ef7682044.
>
> To me it looks like this functionality was never actually used. At
> least
> I can't find any protection in vmw_bo_free().
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Cc: Zack Rusin <zack.rusin@broadcom.com>
> Cc: Broadcom internal kernel review list
> <bcm-kernel-feedback-list@broadcom.com>
IIRC the reference-free lookups were used to avoid the extensive
referencing and unreferencing during the command stream parsing by
means of rcu protection, so when vmw_validation_add() was called the bo
pointer might have been only rcu-protected.
From a brief look this looks like it's been changed with the gem
rewrite and if so, this patch should probably be safe.
/Thomas
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_validation.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
> index e7625b3f71e0..e11837e484aa 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
> @@ -262,7 +262,8 @@ int vmw_validation_add_bo(struct
> vmw_validation_context *ctx,
> bo_node->hash.key);
> }
> val_buf = &bo_node->base;
> - val_buf->bo = ttm_bo_get_unless_zero(&vbo->tbo);
> + vmw_bo_reference(vbo);
> + val_buf->bo = &vbo->tbo;
> if (!val_buf->bo)
> return -ESRCH;
> val_buf->num_shared = 0;
> @@ -656,7 +657,7 @@ void vmw_validation_unref_lists(struct
> vmw_validation_context *ctx)
> struct vmw_validation_res_node *val;
>
> list_for_each_entry(entry, &ctx->bo_list, base.head) {
> - ttm_bo_put(entry->base.bo);
> + drm_gem_object_put(&entry->base.bo->base);
> entry->base.bo = NULL;
> }
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 6/8] drm/ttm: revert "Export ttm_bo_get_unless_zero()"
2024-07-23 12:17 [PATCH 1/8] drm/amdgpu: use GEM references instead of TTMs Christian König
` (3 preceding siblings ...)
2024-07-23 12:17 ` [PATCH 5/8] drm/vmwgfx: partially revert "Adapt validation code for reference-free lookups" Christian König
@ 2024-07-23 12:17 ` Christian König
2024-07-25 12:36 ` Daniel Vetter
2024-07-25 13:19 ` Danilo Krummrich
2024-07-23 12:17 ` [PATCH 7/8] drm/ttm: use GEM references for VM mappings Christian König
` (2 subsequent siblings)
7 siblings, 2 replies; 16+ messages in thread
From: Christian König @ 2024-07-23 12:17 UTC (permalink / raw)
To: matthew.brost, thomas.hellstrom; +Cc: dri-devel
This reverts commit 24dc64c1ba5c3ef0463d59fef6df09336754188d.
Shouldn't be needed by drivers any more.
Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/ttm/ttm_bo.c | 1 +
drivers/gpu/drm/ttm/ttm_bo_internal.h | 48 +++++++++++++++++++++++++++
drivers/gpu/drm/ttm/ttm_bo_util.c | 2 ++
drivers/gpu/drm/ttm/ttm_device.c | 1 +
include/drm/ttm/ttm_bo.h | 18 ----------
5 files changed, 52 insertions(+), 18 deletions(-)
create mode 100644 drivers/gpu/drm/ttm/ttm_bo_internal.h
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 0131ec802066..fe4638ec0976 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -45,6 +45,7 @@
#include <linux/dma-resv.h>
#include "ttm_module.h"
+#include "ttm_bo_internal.h"
static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
struct ttm_placement *placement)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_internal.h b/drivers/gpu/drm/ttm/ttm_bo_internal.h
new file mode 100644
index 000000000000..6a7305efd778
--- /dev/null
+++ b/drivers/gpu/drm/ttm/ttm_bo_internal.h
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2018 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ *
+ */
+
+#ifndef _TTM_BO_INTERNAL_H_
+#define _TTM_BO_INTERNAL_H_
+
+#include <drm/ttm/ttm_bo.h>
+
+/**
+ * ttm_bo_get_unless_zero - reference a struct ttm_buffer_object unless
+ * its refcount has already reached zero.
+ * @bo: The buffer object.
+ *
+ * Used to reference a TTM buffer object in lookups where the object is removed
+ * from the lookup structure during the destructor and for RCU lookups.
+ *
+ * Returns: @bo if the referencing was successful, NULL otherwise.
+ */
+static inline __must_check struct ttm_buffer_object *
+ttm_bo_get_unless_zero(struct ttm_buffer_object *bo)
+{
+ if (!kref_get_unless_zero(&bo->kref))
+ return NULL;
+ return bo;
+}
+
+#endif
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 3c07f4712d5c..f7143384ef1c 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -37,6 +37,8 @@
#include <drm/drm_cache.h>
+#include "ttm_bo_internal.h"
+
struct ttm_transfer_obj {
struct ttm_buffer_object base;
struct ttm_buffer_object *bo;
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index e7cc4954c1bc..2e7fa3a11dc0 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -36,6 +36,7 @@
#include <drm/ttm/ttm_placement.h>
#include "ttm_module.h"
+#include "ttm_bo_internal.h"
/*
* ttm_global_mutex - protecting the global state
diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
index d1a732d56259..31ec7fd34eeb 100644
--- a/include/drm/ttm/ttm_bo.h
+++ b/include/drm/ttm/ttm_bo.h
@@ -239,24 +239,6 @@ static inline void ttm_bo_get(struct ttm_buffer_object *bo)
kref_get(&bo->kref);
}
-/**
- * ttm_bo_get_unless_zero - reference a struct ttm_buffer_object unless
- * its refcount has already reached zero.
- * @bo: The buffer object.
- *
- * Used to reference a TTM buffer object in lookups where the object is removed
- * from the lookup structure during the destructor and for RCU lookups.
- *
- * Returns: @bo if the referencing was successful, NULL otherwise.
- */
-static inline __must_check struct ttm_buffer_object *
-ttm_bo_get_unless_zero(struct ttm_buffer_object *bo)
-{
- if (!kref_get_unless_zero(&bo->kref))
- return NULL;
- return bo;
-}
-
/**
* ttm_bo_reserve:
*
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 6/8] drm/ttm: revert "Export ttm_bo_get_unless_zero()"
2024-07-23 12:17 ` [PATCH 6/8] drm/ttm: revert "Export ttm_bo_get_unless_zero()" Christian König
@ 2024-07-25 12:36 ` Daniel Vetter
2024-07-25 13:19 ` Danilo Krummrich
1 sibling, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2024-07-25 12:36 UTC (permalink / raw)
To: Christian König; +Cc: matthew.brost, thomas.hellstrom, dri-devel
On Tue, Jul 23, 2024 at 02:17:48PM +0200, Christian König wrote:
> This reverts commit 24dc64c1ba5c3ef0463d59fef6df09336754188d.
>
> Shouldn't be needed by drivers any more.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
On the last 3 patches:
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/ttm/ttm_bo.c | 1 +
> drivers/gpu/drm/ttm/ttm_bo_internal.h | 48 +++++++++++++++++++++++++++
> drivers/gpu/drm/ttm/ttm_bo_util.c | 2 ++
> drivers/gpu/drm/ttm/ttm_device.c | 1 +
> include/drm/ttm/ttm_bo.h | 18 ----------
> 5 files changed, 52 insertions(+), 18 deletions(-)
> create mode 100644 drivers/gpu/drm/ttm/ttm_bo_internal.h
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 0131ec802066..fe4638ec0976 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -45,6 +45,7 @@
> #include <linux/dma-resv.h>
>
> #include "ttm_module.h"
> +#include "ttm_bo_internal.h"
>
> static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
> struct ttm_placement *placement)
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_internal.h b/drivers/gpu/drm/ttm/ttm_bo_internal.h
> new file mode 100644
> index 000000000000..6a7305efd778
> --- /dev/null
> +++ b/drivers/gpu/drm/ttm/ttm_bo_internal.h
> @@ -0,0 +1,48 @@
> +/*
> + * Copyright 2018 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + *
> + */
> +
> +#ifndef _TTM_BO_INTERNAL_H_
> +#define _TTM_BO_INTERNAL_H_
> +
> +#include <drm/ttm/ttm_bo.h>
> +
> +/**
> + * ttm_bo_get_unless_zero - reference a struct ttm_buffer_object unless
> + * its refcount has already reached zero.
> + * @bo: The buffer object.
> + *
> + * Used to reference a TTM buffer object in lookups where the object is removed
> + * from the lookup structure during the destructor and for RCU lookups.
> + *
> + * Returns: @bo if the referencing was successful, NULL otherwise.
> + */
> +static inline __must_check struct ttm_buffer_object *
> +ttm_bo_get_unless_zero(struct ttm_buffer_object *bo)
> +{
> + if (!kref_get_unless_zero(&bo->kref))
> + return NULL;
> + return bo;
> +}
> +
> +#endif
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 3c07f4712d5c..f7143384ef1c 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -37,6 +37,8 @@
>
> #include <drm/drm_cache.h>
>
> +#include "ttm_bo_internal.h"
> +
> struct ttm_transfer_obj {
> struct ttm_buffer_object base;
> struct ttm_buffer_object *bo;
> diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
> index e7cc4954c1bc..2e7fa3a11dc0 100644
> --- a/drivers/gpu/drm/ttm/ttm_device.c
> +++ b/drivers/gpu/drm/ttm/ttm_device.c
> @@ -36,6 +36,7 @@
> #include <drm/ttm/ttm_placement.h>
>
> #include "ttm_module.h"
> +#include "ttm_bo_internal.h"
>
> /*
> * ttm_global_mutex - protecting the global state
> diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
> index d1a732d56259..31ec7fd34eeb 100644
> --- a/include/drm/ttm/ttm_bo.h
> +++ b/include/drm/ttm/ttm_bo.h
> @@ -239,24 +239,6 @@ static inline void ttm_bo_get(struct ttm_buffer_object *bo)
> kref_get(&bo->kref);
> }
>
> -/**
> - * ttm_bo_get_unless_zero - reference a struct ttm_buffer_object unless
> - * its refcount has already reached zero.
> - * @bo: The buffer object.
> - *
> - * Used to reference a TTM buffer object in lookups where the object is removed
> - * from the lookup structure during the destructor and for RCU lookups.
> - *
> - * Returns: @bo if the referencing was successful, NULL otherwise.
> - */
> -static inline __must_check struct ttm_buffer_object *
> -ttm_bo_get_unless_zero(struct ttm_buffer_object *bo)
> -{
> - if (!kref_get_unless_zero(&bo->kref))
> - return NULL;
> - return bo;
> -}
> -
> /**
> * ttm_bo_reserve:
> *
> --
> 2.34.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 6/8] drm/ttm: revert "Export ttm_bo_get_unless_zero()"
2024-07-23 12:17 ` [PATCH 6/8] drm/ttm: revert "Export ttm_bo_get_unless_zero()" Christian König
2024-07-25 12:36 ` Daniel Vetter
@ 2024-07-25 13:19 ` Danilo Krummrich
1 sibling, 0 replies; 16+ messages in thread
From: Danilo Krummrich @ 2024-07-25 13:19 UTC (permalink / raw)
To: Christian König; +Cc: matthew.brost, thomas.hellstrom, dri-devel
On 7/23/24 2:17 PM, Christian König wrote:
> This reverts commit 24dc64c1ba5c3ef0463d59fef6df09336754188d.
>
> Shouldn't be needed by drivers any more.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
Technically, this series should be a v2, right? I also still think it would be
good to add the corresponding driver maintainers to the recipient list.
Anyway, for the TTM patches: Reviewed-by: Danilo Krummrich <dakr@kernel.org>
> ---
> drivers/gpu/drm/ttm/ttm_bo.c | 1 +
> drivers/gpu/drm/ttm/ttm_bo_internal.h | 48 +++++++++++++++++++++++++++
> drivers/gpu/drm/ttm/ttm_bo_util.c | 2 ++
> drivers/gpu/drm/ttm/ttm_device.c | 1 +
> include/drm/ttm/ttm_bo.h | 18 ----------
> 5 files changed, 52 insertions(+), 18 deletions(-)
> create mode 100644 drivers/gpu/drm/ttm/ttm_bo_internal.h
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 0131ec802066..fe4638ec0976 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -45,6 +45,7 @@
> #include <linux/dma-resv.h>
>
> #include "ttm_module.h"
> +#include "ttm_bo_internal.h"
>
> static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
> struct ttm_placement *placement)
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_internal.h b/drivers/gpu/drm/ttm/ttm_bo_internal.h
> new file mode 100644
> index 000000000000..6a7305efd778
> --- /dev/null
> +++ b/drivers/gpu/drm/ttm/ttm_bo_internal.h
> @@ -0,0 +1,48 @@
> +/*
> + * Copyright 2018 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + *
> + */
> +
> +#ifndef _TTM_BO_INTERNAL_H_
> +#define _TTM_BO_INTERNAL_H_
> +
> +#include <drm/ttm/ttm_bo.h>
> +
> +/**
> + * ttm_bo_get_unless_zero - reference a struct ttm_buffer_object unless
> + * its refcount has already reached zero.
> + * @bo: The buffer object.
> + *
> + * Used to reference a TTM buffer object in lookups where the object is removed
> + * from the lookup structure during the destructor and for RCU lookups.
> + *
> + * Returns: @bo if the referencing was successful, NULL otherwise.
> + */
> +static inline __must_check struct ttm_buffer_object *
> +ttm_bo_get_unless_zero(struct ttm_buffer_object *bo)
> +{
> + if (!kref_get_unless_zero(&bo->kref))
> + return NULL;
> + return bo;
> +}
> +
> +#endif
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 3c07f4712d5c..f7143384ef1c 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -37,6 +37,8 @@
>
> #include <drm/drm_cache.h>
>
> +#include "ttm_bo_internal.h"
> +
> struct ttm_transfer_obj {
> struct ttm_buffer_object base;
> struct ttm_buffer_object *bo;
> diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
> index e7cc4954c1bc..2e7fa3a11dc0 100644
> --- a/drivers/gpu/drm/ttm/ttm_device.c
> +++ b/drivers/gpu/drm/ttm/ttm_device.c
> @@ -36,6 +36,7 @@
> #include <drm/ttm/ttm_placement.h>
>
> #include "ttm_module.h"
> +#include "ttm_bo_internal.h"
>
> /*
> * ttm_global_mutex - protecting the global state
> diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
> index d1a732d56259..31ec7fd34eeb 100644
> --- a/include/drm/ttm/ttm_bo.h
> +++ b/include/drm/ttm/ttm_bo.h
> @@ -239,24 +239,6 @@ static inline void ttm_bo_get(struct ttm_buffer_object *bo)
> kref_get(&bo->kref);
> }
>
> -/**
> - * ttm_bo_get_unless_zero - reference a struct ttm_buffer_object unless
> - * its refcount has already reached zero.
> - * @bo: The buffer object.
> - *
> - * Used to reference a TTM buffer object in lookups where the object is removed
> - * from the lookup structure during the destructor and for RCU lookups.
> - *
> - * Returns: @bo if the referencing was successful, NULL otherwise.
> - */
> -static inline __must_check struct ttm_buffer_object *
> -ttm_bo_get_unless_zero(struct ttm_buffer_object *bo)
> -{
> - if (!kref_get_unless_zero(&bo->kref))
> - return NULL;
> - return bo;
> -}
> -
> /**
> * ttm_bo_reserve:
> *
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 7/8] drm/ttm: use GEM references for VM mappings
2024-07-23 12:17 [PATCH 1/8] drm/amdgpu: use GEM references instead of TTMs Christian König
` (4 preceding siblings ...)
2024-07-23 12:17 ` [PATCH 6/8] drm/ttm: revert "Export ttm_bo_get_unless_zero()" Christian König
@ 2024-07-23 12:17 ` Christian König
2024-07-23 12:17 ` [PATCH 8/8] drm/ttm: make ttm_bo_get internal Christian König
2024-07-25 12:01 ` [PATCH 1/8] drm/amdgpu: use GEM references instead of TTMs Daniel Vetter
7 siblings, 0 replies; 16+ messages in thread
From: Christian König @ 2024-07-23 12:17 UTC (permalink / raw)
To: matthew.brost, thomas.hellstrom; +Cc: dri-devel
Instead of a TTM reference grab a GEM reference whenever necessary for a
VM mapping.
Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/ttm/ttm_bo_vm.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 4212b8c91dd4..3f283b3433f8 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -58,13 +58,13 @@ static vm_fault_t ttm_bo_vm_fault_idle(struct ttm_buffer_object *bo,
if (vmf->flags & FAULT_FLAG_RETRY_NOWAIT)
return VM_FAULT_RETRY;
- ttm_bo_get(bo);
+ drm_gem_object_get(&bo->base);
mmap_read_unlock(vmf->vma->vm_mm);
(void)dma_resv_wait_timeout(bo->base.resv,
DMA_RESV_USAGE_KERNEL, true,
MAX_SCHEDULE_TIMEOUT);
dma_resv_unlock(bo->base.resv);
- ttm_bo_put(bo);
+ drm_gem_object_put(&bo->base);
return VM_FAULT_RETRY;
}
@@ -130,12 +130,12 @@ vm_fault_t ttm_bo_vm_reserve(struct ttm_buffer_object *bo,
*/
if (fault_flag_allow_retry_first(vmf->flags)) {
if (!(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) {
- ttm_bo_get(bo);
+ drm_gem_object_get(&bo->base);
mmap_read_unlock(vmf->vma->vm_mm);
if (!dma_resv_lock_interruptible(bo->base.resv,
NULL))
dma_resv_unlock(bo->base.resv);
- ttm_bo_put(bo);
+ drm_gem_object_put(&bo->base);
}
return VM_FAULT_RETRY;
@@ -353,7 +353,7 @@ void ttm_bo_vm_open(struct vm_area_struct *vma)
WARN_ON(bo->bdev->dev_mapping != vma->vm_file->f_mapping);
- ttm_bo_get(bo);
+ drm_gem_object_get(&bo->base);
}
EXPORT_SYMBOL(ttm_bo_vm_open);
@@ -361,7 +361,7 @@ void ttm_bo_vm_close(struct vm_area_struct *vma)
{
struct ttm_buffer_object *bo = vma->vm_private_data;
- ttm_bo_put(bo);
+ drm_gem_object_put(&bo->base);
vma->vm_private_data = NULL;
}
EXPORT_SYMBOL(ttm_bo_vm_close);
@@ -462,7 +462,7 @@ int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo)
if (is_cow_mapping(vma->vm_flags))
return -EINVAL;
- ttm_bo_get(bo);
+ drm_gem_object_get(&bo->base);
/*
* Drivers may want to override the vm_ops field. Otherwise we
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 8/8] drm/ttm: make ttm_bo_get internal
2024-07-23 12:17 [PATCH 1/8] drm/amdgpu: use GEM references instead of TTMs Christian König
` (5 preceding siblings ...)
2024-07-23 12:17 ` [PATCH 7/8] drm/ttm: use GEM references for VM mappings Christian König
@ 2024-07-23 12:17 ` Christian König
2024-07-25 12:01 ` [PATCH 1/8] drm/amdgpu: use GEM references instead of TTMs Daniel Vetter
7 siblings, 0 replies; 16+ messages in thread
From: Christian König @ 2024-07-23 12:17 UTC (permalink / raw)
To: matthew.brost, thomas.hellstrom; +Cc: dri-devel
Prevent drivers from using this directly.
Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/ttm/ttm_bo_internal.h | 10 ++++++++++
include/drm/ttm/ttm_bo.h | 10 ----------
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_internal.h b/drivers/gpu/drm/ttm/ttm_bo_internal.h
index 6a7305efd778..9d8b747a34db 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_internal.h
+++ b/drivers/gpu/drm/ttm/ttm_bo_internal.h
@@ -27,6 +27,16 @@
#include <drm/ttm/ttm_bo.h>
+/**
+ * ttm_bo_get - reference a struct ttm_buffer_object
+ *
+ * @bo: The buffer object.
+ */
+static inline void ttm_bo_get(struct ttm_buffer_object *bo)
+{
+ kref_get(&bo->kref);
+}
+
/**
* ttm_bo_get_unless_zero - reference a struct ttm_buffer_object unless
* its refcount has already reached zero.
diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
index 31ec7fd34eeb..8c1577d8793c 100644
--- a/include/drm/ttm/ttm_bo.h
+++ b/include/drm/ttm/ttm_bo.h
@@ -229,16 +229,6 @@ struct ttm_lru_walk {
s64 ttm_lru_walk_for_evict(struct ttm_lru_walk *walk, struct ttm_device *bdev,
struct ttm_resource_manager *man, s64 target);
-/**
- * ttm_bo_get - reference a struct ttm_buffer_object
- *
- * @bo: The buffer object.
- */
-static inline void ttm_bo_get(struct ttm_buffer_object *bo)
-{
- kref_get(&bo->kref);
-}
-
/**
* ttm_bo_reserve:
*
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 1/8] drm/amdgpu: use GEM references instead of TTMs
2024-07-23 12:17 [PATCH 1/8] drm/amdgpu: use GEM references instead of TTMs Christian König
` (6 preceding siblings ...)
2024-07-23 12:17 ` [PATCH 8/8] drm/ttm: make ttm_bo_get internal Christian König
@ 2024-07-25 12:01 ` Daniel Vetter
7 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2024-07-25 12:01 UTC (permalink / raw)
To: Christian König; +Cc: matthew.brost, thomas.hellstrom, dri-devel
On Tue, Jul 23, 2024 at 02:17:43PM +0200, Christian König wrote:
> Instead of a TTM reference grab a GEM reference whenever necessary.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Felix Kuehling <felix.kuehling@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
I was worried that changing the refcounting for the mmu_interval_notifier
that this patch does could result in a use-after-free, but the mmu
notifier code already does the required amount of mmgrab/mmdrop, so we're
good.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cheers, Sima
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 8 ++++----
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 7 ++-----
> 2 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 67c234bcf89f..6be3d7cd1c51 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -87,11 +87,11 @@ static const struct vm_operations_struct amdgpu_gem_vm_ops = {
>
> static void amdgpu_gem_object_free(struct drm_gem_object *gobj)
> {
> - struct amdgpu_bo *robj = gem_to_amdgpu_bo(gobj);
> + struct amdgpu_bo *aobj = gem_to_amdgpu_bo(gobj);
>
> - if (robj) {
> - amdgpu_hmm_unregister(robj);
> - amdgpu_bo_unref(&robj);
> + if (aobj) {
> + amdgpu_hmm_unregister(aobj);
> + ttm_bo_put(&aobj->tbo);
> }
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 8d8c39be6129..6c187e310034 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -853,7 +853,7 @@ struct amdgpu_bo *amdgpu_bo_ref(struct amdgpu_bo *bo)
> if (bo == NULL)
> return NULL;
>
> - ttm_bo_get(&bo->tbo);
> + drm_gem_object_get(&bo->tbo.base);
> return bo;
> }
>
> @@ -865,13 +865,10 @@ struct amdgpu_bo *amdgpu_bo_ref(struct amdgpu_bo *bo)
> */
> void amdgpu_bo_unref(struct amdgpu_bo **bo)
> {
> - struct ttm_buffer_object *tbo;
> -
> if ((*bo) == NULL)
> return;
>
> - tbo = &((*bo)->tbo);
> - ttm_bo_put(tbo);
> + drm_gem_object_get(&(*bo)->tbo.base);
> *bo = NULL;
> }
>
> --
> 2.34.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 16+ messages in thread