Am Sonntag, dem 08.09.2024 um 17:43 +0800 schrieb Sui Jingfeng:
> The etnaviv_gem_obj_add() a common operation, the 'etnaviv_drm_private::
> gem_list' is being used to record(track) all of the etnaviv GEM buffer
> object created in this driver.
>
> Once a etnaviv GEM buffer object has been allocated successfully, we
> should add it into the global etnaviv_drm_private::gem_list'. Because
> we need to free it and remove it free the list if the invoke of the
> subsequent functions fail.
>
> The only way that destroy etnaviv GEM buffer object is the implementation
> of etnaviv_gem_free_object() function. The etnaviv_gem_free_object() first
> remove the etnaviv GEM object from the list, then destroy its mapping and
> finally free its memory footprint. Therefore, in order to corresponding
> this, we add the freshly created etnaviv GEM buffer object immediately
> after it was successfully created.
>
> A benifit is that we only need to call etnaviv_gem_obj_add() once now,
> since the ernaviv_gem_new_private() has been unified. Make the
> etnaviv_gem_obj_add() static is a next nature thing.
>
Seeing this patch, I would really ask you to drop patch 11 from this
series and go the other way around: remove etnaviv_gem_obj_add() here
altogether and simply inline the few lines of code into
etnaviv_gem_new_private().
Regards,
Lucas
> Signed-off-by: Sui Jingfeng <sui.jingfeng@xxxxxxxxx>
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gem.c | 8 +++-----
> drivers/gpu/drm/etnaviv/etnaviv_gem.h | 1 -
> drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 2 --
> 3 files changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index 27e4a93c981c..ee799c02d0aa 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -584,7 +584,7 @@ void etnaviv_gem_free_object(struct drm_gem_object *obj)
> kfree(etnaviv_obj);
> }
>
> -void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj)
> +static void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj)
> {
> struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
> struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
> @@ -719,8 +719,6 @@ int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file,
> */
> mapping_set_gfp_mask(obj->filp->f_mapping, priv->shm_gfp_mask);
>
> - etnaviv_gem_obj_add(dev, obj);
> -
> ret = drm_gem_handle_create(file, obj, handle);
>
> /* drop reference from allocate - handle holds it now */
> @@ -751,6 +749,8 @@ int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags,
> drm_gem_private_object_init(dev, obj, size);
> }
>
> + etnaviv_gem_obj_add(dev, obj);
> +
> *res = to_etnaviv_bo(obj);
>
> return 0;
> @@ -849,8 +849,6 @@ int etnaviv_gem_new_userptr(struct drm_device *dev, struct drm_file *file,
> etnaviv_obj->userptr.mm = current->mm;
> etnaviv_obj->userptr.ro = !(flags & ETNA_USERPTR_WRITE);
>
> - etnaviv_gem_obj_add(dev, &etnaviv_obj->base);
> -
> ret = drm_gem_handle_create(file, &etnaviv_obj->base, handle);
>
> /* drop reference from allocate - handle holds it now */
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> index b174a9e4cc48..8d8fc5b3a541 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> @@ -121,7 +121,6 @@ int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags,
> bool shmem, const struct etnaviv_gem_ops *ops,
> struct etnaviv_gem_object **res);
>
> -void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj);
> struct page **etnaviv_gem_get_pages(struct etnaviv_gem_object *obj);
> void etnaviv_gem_put_pages(struct etnaviv_gem_object *obj);
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> index 64a858a0b0cf..04ee31461b8c 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> @@ -127,8 +127,6 @@ struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
> if (ret)
> goto fail;
>
> - etnaviv_gem_obj_add(dev, &etnaviv_obj->base);
> -
> return &etnaviv_obj->base;
>
> fail:
