From: "Martin Krastev (VMware)" <martinkrastev768@gmail.com>
To: Zack Rusin <zackr@vmware.com>, dri-devel@lists.freedesktop.org
Cc: krastevm@vmware.com, mombasawalam@vmware.com, banackm@vmware.com
Subject: Re: [PATCH 2/7] drm/vmwgfx: Remove the duplicate bo_free function
Date: Fri, 27 Jan 2023 19:00:15 +0200 [thread overview]
Message-ID: <e62178a8-e75a-366f-ced3-263d8dfd2b58@gmail.com> (raw)
In-Reply-To: <20230126173813.602748-3-zack@kde.org>
From: Martin Krastev <krastevm@vmware.com>
LGTM!
Reviewed-by: Martin Krastev <krastevm@vmware.com>
Regards,
Martin
On 26.01.23 г. 19:38 ч., Zack Rusin wrote:
> From: Zack Rusin <zackr@vmware.com>
>
> Remove the explicit bo_free parameter which was switching between
> vmw_bo_bo_free and vmw_gem_destroy which had exactly the same
> implementation.
>
> It makes no sense to keep parameter which is always the same, remove it
> and all code referencing it. Instead use the vmw_bo_bo_free directly.
>
> Signed-off-by: Zack Rusin <zackr@vmware.com>
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 49 ++++++++++--------------
> drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c | 2 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 3 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 6 +--
> drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 18 +--------
> drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 3 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_shader.c | 2 +-
> 8 files changed, 27 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> index aa1cd5126a32..8aaeeecd2016 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> @@ -46,6 +46,22 @@ vmw_buffer_object(struct ttm_buffer_object *bo)
> return container_of(bo, struct vmw_buffer_object, base);
> }
>
> +/**
> + * vmw_bo_bo_free - vmw buffer object destructor
> + *
> + * @bo: Pointer to the embedded struct ttm_buffer_object
> + */
> +static void vmw_bo_bo_free(struct ttm_buffer_object *bo)
> +{
> + struct vmw_buffer_object *vmw_bo = vmw_buffer_object(bo);
> +
> + WARN_ON(vmw_bo->dirty);
> + WARN_ON(!RB_EMPTY_ROOT(&vmw_bo->res_tree));
> + vmw_bo_unmap(vmw_bo);
> + drm_gem_object_release(&bo->base);
> + kfree(vmw_bo);
> +}
> +
> /**
> * bo_is_vmw - check if the buffer object is a &vmw_buffer_object
> * @bo: ttm buffer object to be checked
> @@ -58,8 +74,7 @@ vmw_buffer_object(struct ttm_buffer_object *bo)
> */
> static bool bo_is_vmw(struct ttm_buffer_object *bo)
> {
> - return bo->destroy == &vmw_bo_bo_free ||
> - bo->destroy == &vmw_gem_destroy;
> + return bo->destroy == &vmw_bo_bo_free;
> }
>
> /**
> @@ -376,23 +391,6 @@ void vmw_bo_unmap(struct vmw_buffer_object *vbo)
> ttm_bo_kunmap(&vbo->map);
> }
>
> -
> -/**
> - * vmw_bo_bo_free - vmw buffer object destructor
> - *
> - * @bo: Pointer to the embedded struct ttm_buffer_object
> - */
> -void vmw_bo_bo_free(struct ttm_buffer_object *bo)
> -{
> - struct vmw_buffer_object *vmw_bo = vmw_buffer_object(bo);
> -
> - WARN_ON(vmw_bo->dirty);
> - WARN_ON(!RB_EMPTY_ROOT(&vmw_bo->res_tree));
> - vmw_bo_unmap(vmw_bo);
> - drm_gem_object_release(&bo->base);
> - kfree(vmw_bo);
> -}
> -
> /* default destructor */
> static void vmw_bo_default_destroy(struct ttm_buffer_object *bo)
> {
> @@ -449,13 +447,10 @@ int vmw_bo_create_kernel(struct vmw_private *dev_priv, unsigned long size,
> int vmw_bo_create(struct vmw_private *vmw,
> size_t size, struct ttm_placement *placement,
> bool interruptible, bool pin,
> - void (*bo_free)(struct ttm_buffer_object *bo),
> struct vmw_buffer_object **p_bo)
> {
> int ret;
>
> - BUG_ON(!bo_free);
> -
> *p_bo = kmalloc(sizeof(**p_bo), GFP_KERNEL);
> if (unlikely(!*p_bo)) {
> DRM_ERROR("Failed to allocate a buffer.\n");
> @@ -463,8 +458,7 @@ int vmw_bo_create(struct vmw_private *vmw,
> }
>
> ret = vmw_bo_init(vmw, *p_bo, size,
> - placement, interruptible, pin,
> - bo_free);
> + placement, interruptible, pin);
> if (unlikely(ret != 0))
> goto out_error;
>
> @@ -484,7 +478,6 @@ int vmw_bo_create(struct vmw_private *vmw,
> * @placement: Initial placement.
> * @interruptible: Whether waits should be performed interruptible.
> * @pin: If the BO should be created pinned at a fixed location.
> - * @bo_free: The buffer object destructor.
> * Returns: Zero on success, negative error code on error.
> *
> * Note that on error, the code will free the buffer object.
> @@ -492,8 +485,7 @@ int vmw_bo_create(struct vmw_private *vmw,
> int vmw_bo_init(struct vmw_private *dev_priv,
> struct vmw_buffer_object *vmw_bo,
> size_t size, struct ttm_placement *placement,
> - bool interruptible, bool pin,
> - void (*bo_free)(struct ttm_buffer_object *bo))
> + bool interruptible, bool pin)
> {
> struct ttm_operation_ctx ctx = {
> .interruptible = interruptible,
> @@ -503,7 +495,6 @@ int vmw_bo_init(struct vmw_private *dev_priv,
> struct drm_device *vdev = &dev_priv->drm;
> int ret;
>
> - WARN_ON_ONCE(!bo_free);
> memset(vmw_bo, 0, sizeof(*vmw_bo));
> BUILD_BUG_ON(TTM_MAX_BO_PRIORITY <= 3);
> vmw_bo->base.priority = 3;
> @@ -513,7 +504,7 @@ int vmw_bo_init(struct vmw_private *dev_priv,
> drm_gem_private_object_init(vdev, &vmw_bo->base.base, size);
>
> ret = ttm_bo_init_reserved(bdev, &vmw_bo->base, ttm_bo_type_device,
> - placement, 0, &ctx, NULL, NULL, bo_free);
> + placement, 0, &ctx, NULL, NULL, vmw_bo_bo_free);
> if (unlikely(ret)) {
> return ret;
> }
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
> index b78a10312fad..87455446a6f9 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
> @@ -424,7 +424,7 @@ static int vmw_cotable_resize(struct vmw_resource *res, size_t new_size)
> * we can use tryreserve without failure.
> */
> ret = vmw_bo_create(dev_priv, new_size, &vmw_mob_placement,
> - true, true, vmw_bo_bo_free, &buf);
> + true, true, &buf);
> if (ret) {
> DRM_ERROR("Failed initializing new cotable MOB.\n");
> goto out_done;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index e0c2e3748015..7272aff7855d 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -398,8 +398,7 @@ static int vmw_dummy_query_bo_create(struct vmw_private *dev_priv)
> * user of the bo currently.
> */
> ret = vmw_bo_create(dev_priv, PAGE_SIZE,
> - &vmw_sys_placement, false, true,
> - &vmw_bo_bo_free, &vbo);
> + &vmw_sys_placement, false, true, &vbo);
> if (unlikely(ret != 0))
> return ret;
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> index 4dfa5044a9e7..3e8ab2ce5b94 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> @@ -891,7 +891,6 @@ extern int vmw_bo_unpin(struct vmw_private *vmw_priv,
> extern void vmw_bo_get_guest_ptr(const struct ttm_buffer_object *buf,
> SVGAGuestPtr *ptr);
> extern void vmw_bo_pin_reserved(struct vmw_buffer_object *bo, bool pin);
> -extern void vmw_bo_bo_free(struct ttm_buffer_object *bo);
> extern int vmw_bo_create_kernel(struct vmw_private *dev_priv,
> unsigned long size,
> struct ttm_placement *placement,
> @@ -899,13 +898,11 @@ extern int vmw_bo_create_kernel(struct vmw_private *dev_priv,
> extern int vmw_bo_create(struct vmw_private *dev_priv,
> size_t size, struct ttm_placement *placement,
> bool interruptible, bool pin,
> - void (*bo_free)(struct ttm_buffer_object *bo),
> struct vmw_buffer_object **p_bo);
> extern int vmw_bo_init(struct vmw_private *dev_priv,
> struct vmw_buffer_object *vmw_bo,
> size_t size, struct ttm_placement *placement,
> - bool interruptible, bool pin,
> - void (*bo_free)(struct ttm_buffer_object *bo));
> + bool interruptible, bool pin);
> extern int vmw_bo_unref_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
> extern int vmw_user_bo_synccpu_ioctl(struct drm_device *dev, void *data,
> @@ -980,7 +977,6 @@ extern int vmw_gem_object_create_with_handle(struct vmw_private *dev_priv,
> struct vmw_buffer_object **p_vbo);
> extern int vmw_gem_object_create_ioctl(struct drm_device *dev, void *data,
> struct drm_file *filp);
> -extern void vmw_gem_destroy(struct ttm_buffer_object *bo);
> extern void vmw_debugfs_gem_init(struct vmw_private *vdev);
>
> /**
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
> index ba4ddd9f7a7e..ae39029fec4a 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
> @@ -125,22 +125,6 @@ static const struct drm_gem_object_funcs vmw_gem_object_funcs = {
> .vm_ops = &vmw_vm_ops,
> };
>
> -/**
> - * vmw_gem_destroy - vmw buffer object destructor
> - *
> - * @bo: Pointer to the embedded struct ttm_buffer_object
> - */
> -void vmw_gem_destroy(struct ttm_buffer_object *bo)
> -{
> - struct vmw_buffer_object *vbo = vmw_buffer_object(bo);
> -
> - WARN_ON(vbo->dirty);
> - WARN_ON(!RB_EMPTY_ROOT(&vbo->res_tree));
> - vmw_bo_unmap(vbo);
> - drm_gem_object_release(&vbo->base.base);
> - kfree(vbo);
> -}
> -
> int vmw_gem_object_create_with_handle(struct vmw_private *dev_priv,
> struct drm_file *filp,
> uint32_t size,
> @@ -153,7 +137,7 @@ int vmw_gem_object_create_with_handle(struct vmw_private *dev_priv,
> (dev_priv->has_mob) ?
> &vmw_sys_placement :
> &vmw_vram_sys_placement,
> - true, false, &vmw_gem_destroy, p_vbo);
> + true, false, p_vbo);
>
> (*p_vbo)->base.base.funcs = &vmw_gem_object_funcs;
> if (ret != 0)
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> index c7d645e5ec7b..5879e8b9950a 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> @@ -332,8 +332,7 @@ static int vmw_resource_buf_alloc(struct vmw_resource *res,
>
> ret = vmw_bo_create(res->dev_priv, res->backup_size,
> res->func->backup_placement,
> - interruptible, false,
> - &vmw_bo_bo_free, &backup);
> + interruptible, false, &backup);
> if (unlikely(ret != 0))
> goto out_no_bo;
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> index e1f36a09c59c..e51a63c05943 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> @@ -445,7 +445,7 @@ vmw_sou_primary_plane_prepare_fb(struct drm_plane *plane,
> vmw_overlay_pause_all(dev_priv);
> ret = vmw_bo_create(dev_priv, size,
> &vmw_vram_placement,
> - false, true, &vmw_bo_bo_free, &vps->bo);
> + false, true, &vps->bo);
> vmw_overlay_resume_all(dev_priv);
> if (ret) {
> vps->bo = NULL; /* vmw_bo_init frees on error */
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
> index 108a496b5d18..93b1400aed4a 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
> @@ -893,7 +893,7 @@ int vmw_compat_shader_add(struct vmw_private *dev_priv,
> return -EINVAL;
>
> ret = vmw_bo_create(dev_priv, size, &vmw_sys_placement,
> - true, true, vmw_bo_bo_free, &buf);
> + true, true, &buf);
> if (unlikely(ret != 0))
> goto out;
>
next prev parent reply other threads:[~2023-01-27 17:00 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-26 17:38 [PATCH 0/7] drm/vmwgfx: Refactor the buffer object code Zack Rusin
2023-01-26 17:38 ` [PATCH 1/7] drm/vmwgfx: Use the common gem mmap instead of the custom code Zack Rusin
2023-01-27 13:46 ` Thomas Zimmermann
2023-01-27 16:23 ` Martin Krastev (VMware)
2023-01-26 17:38 ` [PATCH 2/7] drm/vmwgfx: Remove the duplicate bo_free function Zack Rusin
2023-01-27 17:00 ` Martin Krastev (VMware) [this message]
2023-01-26 17:38 ` [PATCH 3/7] drm/vmwgfx: Rename vmw_buffer_object to vmw_bo Zack Rusin
2023-01-27 13:51 ` Thomas Zimmermann
2023-01-27 17:06 ` Martin Krastev (VMware)
2023-01-26 17:38 ` [PATCH 4/7] drm/vmwgfx: Simplify fb pinning Zack Rusin
2023-01-27 18:53 ` Martin Krastev (VMware)
2023-01-26 17:38 ` [PATCH 5/7] drm/vmwgfx: Cleanup the vmw bo usage in the cursor paths Zack Rusin
2023-01-27 13:12 ` Thomas Zimmermann
2023-01-27 18:57 ` Martin Krastev (VMware)
2023-01-26 17:38 ` [PATCH 6/7] drm/vmwgfx: Abstract placement selection Zack Rusin
2023-01-27 13:42 ` Thomas Zimmermann
2023-01-28 15:09 ` kernel test robot
2023-01-28 15:09 ` kernel test robot
2023-01-26 17:38 ` [PATCH 7/7] drm/vmwgfx: Stop using raw ttm_buffer_object's Zack Rusin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e62178a8-e75a-366f-ced3-263d8dfd2b58@gmail.com \
--to=martinkrastev768@gmail.com \
--cc=banackm@vmware.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=krastevm@vmware.com \
--cc=mombasawalam@vmware.com \
--cc=zackr@vmware.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.