Re: [PATCH] drm/etnaviv: Add fdinfo support for memory stats

[Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxx>]

On 21/11/2024 09:24, Christian Gmeiner wrote:
> From: Christian Gmeiner <cgmeiner@xxxxxxxxxx>
>
> Use the new helper to export stats about memory usage.
>
> Signed-off-by: Christian Gmeiner <cgmeiner@xxxxxxxxxx>
> ---
>   drivers/gpu/drm/etnaviv/etnaviv_drv.c | 12 +++++++++++-
>   drivers/gpu/drm/etnaviv/etnaviv_gem.c | 12 ++++++++++++
>   2 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 6500f3999c5f..35f47dd6367f 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -488,7 +488,16 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = {
>   	ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
>   };
>   
> -DEFINE_DRM_GEM_FOPS(fops);
> +static void etnaviv_show_fdinfo(struct drm_printer *p, struct drm_file *file)
> +{
> +	drm_show_memory_stats(p, file);
> +}
> +
> +static const struct file_operations fops = {
> +	.owner = THIS_MODULE,
> +	DRM_GEM_FOPS,
> +	.show_fdinfo = drm_show_fdinfo,
> +};
>   
>   static const struct drm_driver etnaviv_drm_driver = {
>   	.driver_features    = DRIVER_GEM | DRIVER_RENDER,
> @@ -498,6 +507,7 @@ static const struct drm_driver etnaviv_drm_driver = {
>   #ifdef CONFIG_DEBUG_FS
>   	.debugfs_init       = etnaviv_debugfs_init,
>   #endif
> +	.show_fdinfo        = etnaviv_show_fdinfo,
>   	.ioctls             = etnaviv_ioctls,
>   	.num_ioctls         = DRM_ETNAVIV_NUM_IOCTLS,
>   	.fops               = &fops,
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index 5c0c9d4e3be1..e81c261b0017 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -527,6 +527,17 @@ void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj)
>   	mutex_unlock(&priv->gem_lock);
>   }
>   
> +static enum drm_gem_object_status etnaviv_gem_status(struct drm_gem_object *obj)
> +{
> +	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
> +	enum drm_gem_object_status status = 0;
> +
> +	if (etnaviv_obj->pages)
> +		status |= DRM_GEM_OBJECT_RESIDENT;
> +
> +	return status;

Or just:

return etnaviv_obj->pages ? DRM_GEM_OBJECT_RESIDENT : 0;

But it is inconsequential, or maybe even you plan to add purgeable at 
some point.

> +}
> +
>   static const struct vm_operations_struct vm_ops = {
>   	.fault = etnaviv_gem_fault,
>   	.open = drm_gem_vm_open,
> @@ -540,6 +551,7 @@ static const struct drm_gem_object_funcs etnaviv_gem_object_funcs = {
>   	.get_sg_table = etnaviv_gem_prime_get_sg_table,
>   	.vmap = etnaviv_gem_prime_vmap,
>   	.mmap = etnaviv_gem_mmap,
> +	.status = etnaviv_gem_status,
>   	.vm_ops = &vm_ops,
>   };
>  
Either way LGTM.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxx>

Regards,

Tvrtko