Re: [PATCH v15 05/19] drm/etnaviv: Add contructor and destructor for etnaviv_gem_get_mapping structure

[Lucas Stach]

Am Sonntag, dem 08.09.2024 um 17:43 +0800 schrieb Sui Jingfeng:
> Because this make the code more easier to understand, When GPU access the
> VRAM, it will allocate a new mapping to use if there don't have one.
> 
> Signed-off-by: Sui Jingfeng <sui.jingfeng@xxxxxxxxx>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c | 40 +++++++++++++++++++--------
>  drivers/gpu/drm/etnaviv/etnaviv_gem.h |  6 ++++
>  2 files changed, 34 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index 85d4e7c87a6a..55004fa9fabd 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -227,6 +227,30 @@ etnaviv_gem_get_vram_mapping(struct etnaviv_gem_object *obj,
>  	return NULL;
>  }
>  
> +static struct etnaviv_vram_mapping *
> +etnaviv_gem_vram_mapping_new(struct etnaviv_gem_object *etnaviv_obj)
> +{
> +	struct etnaviv_vram_mapping *mapping;
> +
> +	mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
> +	if (!mapping)
> +		return NULL;
> +
> +	INIT_LIST_HEAD(&mapping->scan_node);
> +	mapping->object = etnaviv_obj;
> +	mapping->use = 1;
> +
> +	list_add_tail(&mapping->obj_node, &etnaviv_obj->vram_list);
> +
> +	return mapping;
> +}
> +
> +static void etnaviv_gem_vram_mapping_destroy(struct etnaviv_vram_mapping *mapping)
> +{
> +	list_del(&mapping->obj_node);
> +	kfree(mapping);
> +}
> +
>  void etnaviv_gem_mapping_unreference(struct etnaviv_vram_mapping *mapping)
>  {
>  	struct etnaviv_gem_object *etnaviv_obj = mapping->object;
> @@ -289,27 +313,20 @@ struct etnaviv_vram_mapping *etnaviv_gem_mapping_get(
>  	 */
>  	mapping = etnaviv_gem_get_vram_mapping(etnaviv_obj, NULL);
>  	if (!mapping) {
> -		mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
> +		mapping = etnaviv_gem_vram_mapping_new(etnaviv_obj);
>  		if (!mapping) {
>  			ret = -ENOMEM;
>  			goto out;
>  		}
> -
> -		INIT_LIST_HEAD(&mapping->scan_node);
> -		mapping->object = etnaviv_obj;
>  	} else {
> -		list_del(&mapping->obj_node);
> +		mapping->use = 1;
>  	}
>  
> -	mapping->use = 1;
> -
>  	ret = etnaviv_iommu_map_gem(mmu_context, etnaviv_obj,
>  				    mmu_context->global->memory_base,
>  				    mapping, va);
>  	if (ret < 0)
> -		kfree(mapping);
> -	else
> -		list_add_tail(&mapping->obj_node, &etnaviv_obj->vram_list);
> +		etnaviv_gem_vram_mapping_destroy(mapping);
>  
>  out:
>  	mutex_unlock(&etnaviv_obj->lock);
> @@ -544,8 +561,7 @@ void etnaviv_gem_free_object(struct drm_gem_object *obj)
>  		if (context)
>  			etnaviv_iommu_unmap_gem(context, mapping);
>  
> -		list_del(&mapping->obj_node);
> -		kfree(mapping);
> +		etnaviv_gem_vram_mapping_destroy(mapping);
>  	}
>  
>  	etnaviv_obj->ops->vunmap(etnaviv_obj);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> index d4965de3007c..f2ac64d8e90b 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> @@ -31,6 +31,12 @@ struct etnaviv_vram_mapping {
>  	u32 iova;
>  };
>  
> +static inline struct etnaviv_vram_mapping *
> +to_etnaviv_vram_mapping(struct drm_mm_node *vram)
> +{
> +	return container_of(vram, struct etnaviv_vram_mapping, vram_node);
> +}
> +
This hunk looks unrelated to this patch. Otherwise patch looks good.

Regards,
Lucas

>  struct etnaviv_gem_object {
>  	struct drm_gem_object base;
>  	const struct etnaviv_gem_ops *ops;