| Message ID | 20241110154152.592-3-Yunxiang.Li@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On 10/11/2024 15:41, Yunxiang Li wrote: > Make drm-active- optional just like drm-resident- and drm-purgeable-. As Jani has already commented the commit message needs some work. > 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; I think this is correct and I think I've raised that it should be like this when the code was originally added. I only don't remember what was the argument to keep it hardcoded, if there was any. Adding Rob in case he can remember. > } > > - 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; > - } Drive by cleanup, okay. > > 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; I wonder what behaviour we should have here if the driver has reported DRM_GEM_OBJECT_ACTIVE via its status vfunc. Like should it be like this: if ((s & DRM_GEM_OBJECT_ACTIVE) || !dma_resv_test_signaled(obj->resv, dma_resv_usage_rw(true))) { ... ? So if some driver starts reporting this flag _and_ is still calling drm_show_memory_stats(), it's version of activity tracking is used instead of the the dma_resv based test. > > /* 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 Good cleanup. s/whill/will/ > + * 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), > }; > > /** Regards, Tvrtko
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), }; /**
Make drm-active- optional just like drm-resident- and drm-purgeable-. 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(-)