Re: [PATCH v7 2/4] drm: make drm-active- stats optional

[Jani Nikula <jani.nikula@linux.intel.com>]

On Sun, 10 Nov 2024, Yunxiang Li <Yunxiang.Li@amd.com> wrote:
> Make drm-active- optional just like drm-resident- and drm-purgeable-.

Why? What does it mean?

This is what the commit message should answer.

>
> Signed-off-by: Yunxiang Li <Yunxiang.Li@amd.com>
> CC: dri-devel@lists.freedesktop.org
> CC: intel-gfx@lists.freedesktop.org
> CC: amd-gfx@lists.freedesktop.org
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c |  1 +
>  drivers/gpu/drm/drm_file.c                 | 13 +++++++------
>  drivers/gpu/drm/i915/i915_drm_client.c     |  1 +
>  drivers/gpu/drm/xe/xe_drm_client.c         |  1 +
>  include/drm/drm_gem.h                      | 14 ++++++++------
>  5 files changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
> index df2cf5c339255..7717e3e4f05b5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
> @@ -97,6 +97,7 @@ void amdgpu_show_fdinfo(struct drm_printer *p, struct drm_file *file)
>  
>  		drm_print_memory_stats(p,
>  				       &stats[i].drm,
> +				       DRM_GEM_OBJECT_ACTIVE |
>  				       DRM_GEM_OBJECT_RESIDENT |
>  				       DRM_GEM_OBJECT_PURGEABLE,
>  				       pl_name[i]);
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index e285fcc28c59c..fd06671054723 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -884,7 +884,9 @@ void drm_print_memory_stats(struct drm_printer *p,
>  {
>  	print_size(p, "total", region, stats->private + stats->shared);
>  	print_size(p, "shared", region, stats->shared);
> -	print_size(p, "active", region, stats->active);
> +
> +	if (supported_status & DRM_GEM_OBJECT_ACTIVE)
> +		print_size(p, "active", region, stats->active);
>  
>  	if (supported_status & DRM_GEM_OBJECT_RESIDENT)
>  		print_size(p, "resident", region, stats->resident);
> @@ -917,15 +919,13 @@ void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file)
>  
>  		if (obj->funcs && obj->funcs->status) {
>  			s = obj->funcs->status(obj);
> -			supported_status = DRM_GEM_OBJECT_RESIDENT |
> -					DRM_GEM_OBJECT_PURGEABLE;
> +			supported_status |= s;
>  		}
>  
> -		if (drm_gem_object_is_shared_for_memory_stats(obj)) {
> +		if (drm_gem_object_is_shared_for_memory_stats(obj))
>  			status.shared += obj->size;
> -		} else {
> +		else
>  			status.private += obj->size;
> -		}
>  
>  		if (s & DRM_GEM_OBJECT_RESIDENT) {
>  			status.resident += add_size;
> @@ -938,6 +938,7 @@ void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file)
>  
>  		if (!dma_resv_test_signaled(obj->resv, dma_resv_usage_rw(true))) {
>  			status.active += add_size;
> +			supported_status |= DRM_GEM_OBJECT_ACTIVE;
>  
>  			/* If still active, don't count as purgeable: */
>  			s &= ~DRM_GEM_OBJECT_PURGEABLE;
> diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
> index f586825054918..168d7375304bc 100644
> --- a/drivers/gpu/drm/i915/i915_drm_client.c
> +++ b/drivers/gpu/drm/i915/i915_drm_client.c
> @@ -102,6 +102,7 @@ static void show_meminfo(struct drm_printer *p, struct drm_file *file)
>  	for_each_memory_region(mr, i915, id)
>  		drm_print_memory_stats(p,
>  				       &stats[id],
> +				       DRM_GEM_OBJECT_ACTIVE |
>  				       DRM_GEM_OBJECT_RESIDENT |
>  				       DRM_GEM_OBJECT_PURGEABLE,
>  				       mr->uabi_name);
> diff --git a/drivers/gpu/drm/xe/xe_drm_client.c b/drivers/gpu/drm/xe/xe_drm_client.c
> index 6a26923fa10e0..54941b4e850c4 100644
> --- a/drivers/gpu/drm/xe/xe_drm_client.c
> +++ b/drivers/gpu/drm/xe/xe_drm_client.c
> @@ -229,6 +229,7 @@ static void show_meminfo(struct drm_printer *p, struct drm_file *file)
>  		if (man) {
>  			drm_print_memory_stats(p,
>  					       &stats[mem_type],
> +					       DRM_GEM_OBJECT_ACTIVE |
>  					       DRM_GEM_OBJECT_RESIDENT |
>  					       (mem_type != XE_PL_SYSTEM ? 0 :
>  					       DRM_GEM_OBJECT_PURGEABLE),
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index bae4865b2101a..584ffdf5c2542 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -48,19 +48,21 @@ struct drm_gem_object;
>   * enum drm_gem_object_status - bitmask of object state for fdinfo reporting
>   * @DRM_GEM_OBJECT_RESIDENT: object is resident in memory (ie. not unpinned)
>   * @DRM_GEM_OBJECT_PURGEABLE: object marked as purgeable by userspace
> + * @DRM_GEM_OBJECT_ACTIVE: object is currently used by an active submission
>   *
>   * Bitmask of status used for fdinfo memory stats, see &drm_gem_object_funcs.status
> - * and drm_show_fdinfo().  Note that an object can DRM_GEM_OBJECT_PURGEABLE if
> - * it still active or not resident, in which case drm_show_fdinfo() will not
> + * and drm_show_fdinfo().  Note that an object can report DRM_GEM_OBJECT_PURGEABLE
> + * and be active or not resident, in which case drm_show_fdinfo() will not
>   * account for it as purgeable.  So drivers do not need to check if the buffer
> - * is idle and resident to return this bit.  (Ie. userspace can mark a buffer
> - * as purgeable even while it is still busy on the GPU.. it does not _actually_
> - * become puregeable until it becomes idle.  The status gem object func does
> - * not need to consider this.)
> + * is idle and resident to return this bit, i.e. userspace can mark a buffer as
> + * purgeable even while it is still busy on the GPU. It whill not get reported
> + * in the puregeable stats until it becomes idle.  The status gem object func
> + * does not need to consider this.
>   */
>  enum drm_gem_object_status {
>  	DRM_GEM_OBJECT_RESIDENT  = BIT(0),
>  	DRM_GEM_OBJECT_PURGEABLE = BIT(1),
> +	DRM_GEM_OBJECT_ACTIVE = BIT(2),
>  };
>  
>  /**

-- 
Jani Nikula, Intel