Re: [PATCH 1/4] drm/panthor: Introduce BO labeling

[Boris Brezillon]

On Sun, 16 Mar 2025 21:51:32 +0000
Adrián Larumbe <adrian.larumbe@xxxxxxxxxxxxx> wrote:

> Add a new character string Panthor BO field, and a function that allows
> setting it from within the driver.
> 
> Driver takes care of freeing the string when it's replaced or no longer
> needed at object destruction time, but allocating it is the responsibility
> of callers.
> 
> Signed-off-by: Adrián Larumbe <adrian.larumbe@xxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/panthor/panthor_gem.c | 18 ++++++++++++++++++
>  drivers/gpu/drm/panthor/panthor_gem.h | 12 ++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c
> index 8244a4e6c2a2..3c58bb2965ea 100644
> --- a/drivers/gpu/drm/panthor/panthor_gem.c
> +++ b/drivers/gpu/drm/panthor/panthor_gem.c
> @@ -18,6 +18,9 @@ static void panthor_gem_free_object(struct drm_gem_object *obj)
>  	struct panthor_gem_object *bo = to_panthor_bo(obj);
>  	struct drm_gem_object *vm_root_gem = bo->exclusive_vm_root_gem;
>  
> +	kfree(bo->label);
> +	mutex_destroy(&bo->label_lock);
> +
>  	drm_gem_free_mmap_offset(&bo->base.base);
>  	mutex_destroy(&bo->gpuva_list_lock);
>  	drm_gem_shmem_free(&bo->base);
> @@ -196,6 +199,7 @@ struct drm_gem_object *panthor_gem_create_object(struct drm_device *ddev, size_t
>  	obj->base.map_wc = !ptdev->coherent;
>  	mutex_init(&obj->gpuva_list_lock);
>  	drm_gem_gpuva_set_lock(&obj->base.base, &obj->gpuva_list_lock);
> +	mutex_init(&obj->label_lock);
>  
>  	return &obj->base.base;
>  }
> @@ -247,3 +251,17 @@ panthor_gem_create_with_handle(struct drm_file *file,
>  
>  	return ret;
>  }
> +
> +void
> +panthor_gem_label_bo(struct drm_gem_object *obj, const char *label)
> +{
> +	struct panthor_gem_object *bo = to_panthor_bo(obj);
> +	const char *old_label;
> +
> +	mutex_lock(&bo->label_lock);
> +	old_label = bo->label;
> +	bo->label = label;
> +	mutex_unlock(&bo->label_lock);
> +
> +	kfree(old_label);
> +}
> diff --git a/drivers/gpu/drm/panthor/panthor_gem.h b/drivers/gpu/drm/panthor/panthor_gem.h
> index 5749ef2ebe03..da9268d24566 100644
> --- a/drivers/gpu/drm/panthor/panthor_gem.h
> +++ b/drivers/gpu/drm/panthor/panthor_gem.h
> @@ -46,6 +46,15 @@ struct panthor_gem_object {
>  
>  	/** @flags: Combination of drm_panthor_bo_flags flags. */
>  	u32 flags;
> +
> +	/**
> +	 * @label: Pointer to NULL-terminated string, can be assigned within the
> +	 * driver itself or through a specific IOCTL.
> +	 */
> +	const char *label;
> +
> +	/** @label_lock: Lock that protects access to the @label field. */
> +	struct mutex label_lock;

Nit: can we have a label struct with the lock and string under it
instead?

	/** @label: Fields related to GEM labeling. */
	struct {
		/**
		 * @label.str: Pointer to NULL-terminated string, can be assigned within the
		 * driver itself or through a specific IOCTL.
		 */
		const char *str;

		/** @label.lock: Lock that protects access to the @label field. */
		struct mutex lock;
	} label;

>  };
>  
>  /**
> @@ -91,6 +100,9 @@ panthor_gem_create_with_handle(struct drm_file *file,
>  			       struct panthor_vm *exclusive_vm,
>  			       u64 *size, u32 flags, uint32_t *handle);
>  
> +void
> +panthor_gem_label_bo(struct drm_gem_object *obj, const char *label);
> +
>  static inline u64
>  panthor_kernel_bo_gpuva(struct panthor_kernel_bo *bo)
>  {