| Message ID | 20230824013604.466224-7-adrian.larumbe@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add fdinfo support to Panfrost | expand |
Hi Adrián,
kernel test robot noticed the following build warnings:
[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on linus/master v6.5-rc7 next-20230823]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Adri-n-Larumbe/drm-panfrost-Add-cycle-count-GPU-register-definitions/20230824-093848
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/20230824013604.466224-7-adrian.larumbe%40collabora.com
patch subject: [PATCH v2 6/6] drm/drm-file: Allow size unit selection in drm_show_memory_stats
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230824/202308241401.Hr6gvevs-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230824/202308241401.Hr6gvevs-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308241401.Hr6gvevs-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/drm_file.c:905: warning: Function parameter or member 'unit' not described in 'drm_print_memory_stats'
vim +905 drivers/gpu/drm/drm_file.c
686b21b5f6ca2f Rob Clark 2023-05-24 891
686b21b5f6ca2f Rob Clark 2023-05-24 892 /**
686b21b5f6ca2f Rob Clark 2023-05-24 893 * drm_print_memory_stats - A helper to print memory stats
686b21b5f6ca2f Rob Clark 2023-05-24 894 * @p: The printer to print output to
686b21b5f6ca2f Rob Clark 2023-05-24 895 * @stats: The collected memory stats
686b21b5f6ca2f Rob Clark 2023-05-24 896 * @supported_status: Bitmask of optional stats which are available
686b21b5f6ca2f Rob Clark 2023-05-24 897 * @region: The memory region
686b21b5f6ca2f Rob Clark 2023-05-24 898 *
686b21b5f6ca2f Rob Clark 2023-05-24 899 */
686b21b5f6ca2f Rob Clark 2023-05-24 900 void drm_print_memory_stats(struct drm_printer *p,
686b21b5f6ca2f Rob Clark 2023-05-24 901 const struct drm_memory_stats *stats,
686b21b5f6ca2f Rob Clark 2023-05-24 902 enum drm_gem_object_status supported_status,
cccad8cb432637 Adrián Larumbe 2023-08-24 903 const char *region,
cccad8cb432637 Adrián Larumbe 2023-08-24 904 unsigned int unit)
686b21b5f6ca2f Rob Clark 2023-05-24 @905 {
cccad8cb432637 Adrián Larumbe 2023-08-24 906 print_size(p, "total", region, stats->private + stats->shared, unit);
cccad8cb432637 Adrián Larumbe 2023-08-24 907 print_size(p, "shared", region, stats->shared, unit);
cccad8cb432637 Adrián Larumbe 2023-08-24 908 print_size(p, "active", region, stats->active, unit);
686b21b5f6ca2f Rob Clark 2023-05-24 909
686b21b5f6ca2f Rob Clark 2023-05-24 910 if (supported_status & DRM_GEM_OBJECT_RESIDENT)
cccad8cb432637 Adrián Larumbe 2023-08-24 911 print_size(p, "resident", region, stats->resident, unit);
686b21b5f6ca2f Rob Clark 2023-05-24 912
686b21b5f6ca2f Rob Clark 2023-05-24 913 if (supported_status & DRM_GEM_OBJECT_PURGEABLE)
cccad8cb432637 Adrián Larumbe 2023-08-24 914 print_size(p, "purgeable", region, stats->purgeable, unit);
686b21b5f6ca2f Rob Clark 2023-05-24 915 }
686b21b5f6ca2f Rob Clark 2023-05-24 916 EXPORT_SYMBOL(drm_print_memory_stats);
686b21b5f6ca2f Rob Clark 2023-05-24 917
On Wed, Aug 23, 2023 at 6:36 PM Adrián Larumbe <adrian.larumbe@collabora.com> wrote: > > The current implementation will try to pick the highest available > unit. This is rather unflexible, and allowing drivers to display BO size > statistics through fdinfo in units of their choice might be desirable. > > The new argument to drm_show_memory_stats is to be interpreted as the > integer multiplier of a 10-power of 2, so 1 would give us size in Kib and 2 > in Mib. If we want drm-file functions to pick the highest unit, then 0 > should be passed. > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> > --- > drivers/gpu/drm/drm_file.c | 22 +++++++++++++--------- > drivers/gpu/drm/msm/msm_drv.c | 2 +- > drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- > include/drm/drm_file.h | 5 +++-- > 4 files changed, 18 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c > index 762965e3d503..517e1fb8072a 100644 > --- a/drivers/gpu/drm/drm_file.c > +++ b/drivers/gpu/drm/drm_file.c > @@ -873,7 +873,7 @@ void drm_send_event(struct drm_device *dev, struct drm_pending_event *e) > EXPORT_SYMBOL(drm_send_event); > > static void print_size(struct drm_printer *p, const char *stat, > - const char *region, u64 sz) > + const char *region, u64 sz, unsigned int unit) > { > const char *units[] = {"", " KiB", " MiB"}; > unsigned u; > @@ -881,6 +881,8 @@ static void print_size(struct drm_printer *p, const char *stat, > for (u = 0; u < ARRAY_SIZE(units) - 1; u++) { > if (sz < SZ_1K) > break; > + if (unit > 0 && unit == u) > + break; > sz = div_u64(sz, SZ_1K); > } > > @@ -898,17 +900,18 @@ static void print_size(struct drm_printer *p, const char *stat, > void drm_print_memory_stats(struct drm_printer *p, > const struct drm_memory_stats *stats, > enum drm_gem_object_status supported_status, > - const char *region) > + const char *region, > + unsigned int unit) I'm not really adverse to changing what units we use.. or perhaps changing the threshold to go to higher units to be 10000x or 100000x of the previous unit. But I'm less excited about having different drivers using different units. BR, -R > { > - print_size(p, "total", region, stats->private + stats->shared); > - print_size(p, "shared", region, stats->shared); > - print_size(p, "active", region, stats->active); > + print_size(p, "total", region, stats->private + stats->shared, unit); > + print_size(p, "shared", region, stats->shared, unit); > + print_size(p, "active", region, stats->active, unit); > > if (supported_status & DRM_GEM_OBJECT_RESIDENT) > - print_size(p, "resident", region, stats->resident); > + print_size(p, "resident", region, stats->resident, unit); > > if (supported_status & DRM_GEM_OBJECT_PURGEABLE) > - print_size(p, "purgeable", region, stats->purgeable); > + print_size(p, "purgeable", region, stats->purgeable, unit); > } > EXPORT_SYMBOL(drm_print_memory_stats); > > @@ -916,11 +919,12 @@ EXPORT_SYMBOL(drm_print_memory_stats); > * drm_show_memory_stats - Helper to collect and show standard fdinfo memory stats > * @p: the printer to print output to > * @file: the DRM file > + * @unit: multipliyer of power of two exponent of desired unit > * > * Helper to iterate over GEM objects with a handle allocated in the specified > * file. > */ > -void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file) > +void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file, unsigned int unit) > { > struct drm_gem_object *obj; > struct drm_memory_stats status = {}; > @@ -967,7 +971,7 @@ void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file) > } > spin_unlock(&file->table_lock); > > - drm_print_memory_stats(p, &status, supported_status, "memory"); > + drm_print_memory_stats(p, &status, supported_status, "memory", unit); > } > EXPORT_SYMBOL(drm_show_memory_stats); > > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > index 2a0e3529598b..cd1198151744 100644 > --- a/drivers/gpu/drm/msm/msm_drv.c > +++ b/drivers/gpu/drm/msm/msm_drv.c > @@ -1067,7 +1067,7 @@ static void msm_show_fdinfo(struct drm_printer *p, struct drm_file *file) > > msm_gpu_show_fdinfo(priv->gpu, file->driver_priv, p); > > - drm_show_memory_stats(p, file); > + drm_show_memory_stats(p, file, 0); > } > > static const struct file_operations fops = { > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index 93d5f5538c0b..79c08cee3e9d 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -563,7 +563,7 @@ static void panfrost_show_fdinfo(struct drm_printer *p, struct drm_file *file) > > panfrost_gpu_show_fdinfo(pfdev, file->driver_priv, p); > > - drm_show_memory_stats(p, file); > + drm_show_memory_stats(p, file, 1); > } > > static const struct file_operations panfrost_drm_driver_fops = { > diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h > index 010239392adf..21a3b022dd63 100644 > --- a/include/drm/drm_file.h > +++ b/include/drm/drm_file.h > @@ -466,9 +466,10 @@ enum drm_gem_object_status; > void drm_print_memory_stats(struct drm_printer *p, > const struct drm_memory_stats *stats, > enum drm_gem_object_status supported_status, > - const char *region); > + const char *region, > + unsigned int unit); > > -void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file); > +void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file, unsigned int unit); > void drm_show_fdinfo(struct seq_file *m, struct file *f); > > struct file *mock_drm_getfile(struct drm_minor *minor, unsigned int flags); > -- > 2.42.0 >
>> The current implementation will try to pick the highest available >> unit. This is rather unflexible, and allowing drivers to display BO size >> statistics through fdinfo in units of their choice might be desirable. >> >> The new argument to drm_show_memory_stats is to be interpreted as the >> integer multiplier of a 10-power of 2, so 1 would give us size in Kib and 2 >> in Mib. If we want drm-file functions to pick the highest unit, then 0 >> should be passed. >> >> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> >> --- >> drivers/gpu/drm/drm_file.c | 22 +++++++++++++--------- >> drivers/gpu/drm/msm/msm_drv.c | 2 +- >> drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- >> include/drm/drm_file.h | 5 +++-- >> 4 files changed, 18 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c >> index 762965e3d503..517e1fb8072a 100644 >> --- a/drivers/gpu/drm/drm_file.c >> +++ b/drivers/gpu/drm/drm_file.c >> @@ -873,7 +873,7 @@ void drm_send_event(struct drm_device *dev, struct drm_pending_event *e) >> EXPORT_SYMBOL(drm_send_event); >> >> static void print_size(struct drm_printer *p, const char *stat, >> - const char *region, u64 sz) >> + const char *region, u64 sz, unsigned int unit) >> { >> const char *units[] = {"", " KiB", " MiB"}; >> unsigned u; >> @@ -881,6 +881,8 @@ static void print_size(struct drm_printer *p, const char *stat, >> for (u = 0; u < ARRAY_SIZE(units) - 1; u++) { >> if (sz < SZ_1K) >> break; >> + if (unit > 0 && unit == u) >> + break; >> sz = div_u64(sz, SZ_1K); >> } >> >> @@ -898,17 +900,18 @@ static void print_size(struct drm_printer *p, const char *stat, >> void drm_print_memory_stats(struct drm_printer *p, >> const struct drm_memory_stats *stats, >> enum drm_gem_object_status supported_status, >> - const char *region) >> + const char *region, >> + unsigned int unit) > >I'm not really adverse to changing what units we use.. or perhaps >changing the threshold to go to higher units to be 10000x or 100000x >of the previous unit. But I'm less excited about having different >drivers using different units. > >BR, >-R Would it be alright if I left it set to the default unit, and allow changing it at runtime with a debugfs file? >> { >> - print_size(p, "total", region, stats->private + stats->shared); >> - print_size(p, "shared", region, stats->shared); >> - print_size(p, "active", region, stats->active); >> + print_size(p, "total", region, stats->private + stats->shared, unit); >> + print_size(p, "shared", region, stats->shared, unit); >> + print_size(p, "active", region, stats->active, unit); >> >> if (supported_status & DRM_GEM_OBJECT_RESIDENT) >> - print_size(p, "resident", region, stats->resident); >> + print_size(p, "resident", region, stats->resident, unit); >> >> if (supported_status & DRM_GEM_OBJECT_PURGEABLE) >> - print_size(p, "purgeable", region, stats->purgeable); >> + print_size(p, "purgeable", region, stats->purgeable, unit); >> } >> EXPORT_SYMBOL(drm_print_memory_stats); >> >> @@ -916,11 +919,12 @@ EXPORT_SYMBOL(drm_print_memory_stats); >> * drm_show_memory_stats - Helper to collect and show standard fdinfo memory stats >> * @p: the printer to print output to >> * @file: the DRM file >> + * @unit: multipliyer of power of two exponent of desired unit >> * >> * Helper to iterate over GEM objects with a handle allocated in the specified >> * file. >> */ >> -void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file) >> +void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file, unsigned int unit) >> { >> struct drm_gem_object *obj; >> struct drm_memory_stats status = {}; >> @@ -967,7 +971,7 @@ void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file) >> } >> spin_unlock(&file->table_lock); >> >> - drm_print_memory_stats(p, &status, supported_status, "memory"); >> + drm_print_memory_stats(p, &status, supported_status, "memory", unit); >> } >> EXPORT_SYMBOL(drm_show_memory_stats); >> >> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c >> index 2a0e3529598b..cd1198151744 100644 >> --- a/drivers/gpu/drm/msm/msm_drv.c >> +++ b/drivers/gpu/drm/msm/msm_drv.c >> @@ -1067,7 +1067,7 @@ static void msm_show_fdinfo(struct drm_printer *p, struct drm_file *file) >> >> msm_gpu_show_fdinfo(priv->gpu, file->driver_priv, p); >> >> - drm_show_memory_stats(p, file); >> + drm_show_memory_stats(p, file, 0); >> } >> >> static const struct file_operations fops = { >> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c >> index 93d5f5538c0b..79c08cee3e9d 100644 >> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c >> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c >> @@ -563,7 +563,7 @@ static void panfrost_show_fdinfo(struct drm_printer *p, struct drm_file *file) >> >> panfrost_gpu_show_fdinfo(pfdev, file->driver_priv, p); >> >> - drm_show_memory_stats(p, file); >> + drm_show_memory_stats(p, file, 1); >> } >> >> static const struct file_operations panfrost_drm_driver_fops = { >> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h >> index 010239392adf..21a3b022dd63 100644 >> --- a/include/drm/drm_file.h >> +++ b/include/drm/drm_file.h >> @@ -466,9 +466,10 @@ enum drm_gem_object_status; >> void drm_print_memory_stats(struct drm_printer *p, >> const struct drm_memory_stats *stats, >> enum drm_gem_object_status supported_status, >> - const char *region); >> + const char *region, >> + unsigned int unit); >> >> -void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file); >> +void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file, unsigned int unit); >> void drm_show_fdinfo(struct seq_file *m, struct file *f); >> >> struct file *mock_drm_getfile(struct drm_minor *minor, unsigned int flags); >> -- >> 2.42.0 >>
Hi Adrián,
kernel test robot noticed the following build warnings:
[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on linus/master v6.5 next-20230831]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Adri-n-Larumbe/drm-panfrost-Add-cycle-count-GPU-register-definitions/20230824-093848
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/20230824013604.466224-7-adrian.larumbe%40collabora.com
patch subject: [PATCH v2 6/6] drm/drm-file: Allow size unit selection in drm_show_memory_stats
config: x86_64-randconfig-002-20230902 (https://download.01.org/0day-ci/archive/20230902/202309020634.fwC7KBk6-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230902/202309020634.fwC7KBk6-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309020634.fwC7KBk6-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/drm_file.c:905: warning: Function parameter or member 'unit' not described in 'drm_print_memory_stats'
vim +905 drivers/gpu/drm/drm_file.c
686b21b5f6ca2f Rob Clark 2023-05-24 891
686b21b5f6ca2f Rob Clark 2023-05-24 892 /**
686b21b5f6ca2f Rob Clark 2023-05-24 893 * drm_print_memory_stats - A helper to print memory stats
686b21b5f6ca2f Rob Clark 2023-05-24 894 * @p: The printer to print output to
686b21b5f6ca2f Rob Clark 2023-05-24 895 * @stats: The collected memory stats
686b21b5f6ca2f Rob Clark 2023-05-24 896 * @supported_status: Bitmask of optional stats which are available
686b21b5f6ca2f Rob Clark 2023-05-24 897 * @region: The memory region
686b21b5f6ca2f Rob Clark 2023-05-24 898 *
686b21b5f6ca2f Rob Clark 2023-05-24 899 */
686b21b5f6ca2f Rob Clark 2023-05-24 900 void drm_print_memory_stats(struct drm_printer *p,
686b21b5f6ca2f Rob Clark 2023-05-24 901 const struct drm_memory_stats *stats,
686b21b5f6ca2f Rob Clark 2023-05-24 902 enum drm_gem_object_status supported_status,
cccad8cb432637 Adrián Larumbe 2023-08-24 903 const char *region,
cccad8cb432637 Adrián Larumbe 2023-08-24 904 unsigned int unit)
686b21b5f6ca2f Rob Clark 2023-05-24 @905 {
cccad8cb432637 Adrián Larumbe 2023-08-24 906 print_size(p, "total", region, stats->private + stats->shared, unit);
cccad8cb432637 Adrián Larumbe 2023-08-24 907 print_size(p, "shared", region, stats->shared, unit);
cccad8cb432637 Adrián Larumbe 2023-08-24 908 print_size(p, "active", region, stats->active, unit);
686b21b5f6ca2f Rob Clark 2023-05-24 909
686b21b5f6ca2f Rob Clark 2023-05-24 910 if (supported_status & DRM_GEM_OBJECT_RESIDENT)
cccad8cb432637 Adrián Larumbe 2023-08-24 911 print_size(p, "resident", region, stats->resident, unit);
686b21b5f6ca2f Rob Clark 2023-05-24 912
686b21b5f6ca2f Rob Clark 2023-05-24 913 if (supported_status & DRM_GEM_OBJECT_PURGEABLE)
cccad8cb432637 Adrián Larumbe 2023-08-24 914 print_size(p, "purgeable", region, stats->purgeable, unit);
686b21b5f6ca2f Rob Clark 2023-05-24 915 }
686b21b5f6ca2f Rob Clark 2023-05-24 916 EXPORT_SYMBOL(drm_print_memory_stats);
686b21b5f6ca2f Rob Clark 2023-05-24 917
On Wed, Aug 30, 2023 at 8:51 AM Adrián Larumbe <adrian.larumbe@collabora.com> wrote: > > >> The current implementation will try to pick the highest available > >> unit. This is rather unflexible, and allowing drivers to display BO size > >> statistics through fdinfo in units of their choice might be desirable. > >> > >> The new argument to drm_show_memory_stats is to be interpreted as the > >> integer multiplier of a 10-power of 2, so 1 would give us size in Kib and 2 > >> in Mib. If we want drm-file functions to pick the highest unit, then 0 > >> should be passed. > >> > >> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> > >> --- > >> drivers/gpu/drm/drm_file.c | 22 +++++++++++++--------- > >> drivers/gpu/drm/msm/msm_drv.c | 2 +- > >> drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- > >> include/drm/drm_file.h | 5 +++-- > >> 4 files changed, 18 insertions(+), 13 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c > >> index 762965e3d503..517e1fb8072a 100644 > >> --- a/drivers/gpu/drm/drm_file.c > >> +++ b/drivers/gpu/drm/drm_file.c > >> @@ -873,7 +873,7 @@ void drm_send_event(struct drm_device *dev, struct drm_pending_event *e) > >> EXPORT_SYMBOL(drm_send_event); > >> > >> static void print_size(struct drm_printer *p, const char *stat, > >> - const char *region, u64 sz) > >> + const char *region, u64 sz, unsigned int unit) > >> { > >> const char *units[] = {"", " KiB", " MiB"}; > >> unsigned u; > >> @@ -881,6 +881,8 @@ static void print_size(struct drm_printer *p, const char *stat, > >> for (u = 0; u < ARRAY_SIZE(units) - 1; u++) { > >> if (sz < SZ_1K) > >> break; > >> + if (unit > 0 && unit == u) > >> + break; > >> sz = div_u64(sz, SZ_1K); > >> } > >> > >> @@ -898,17 +900,18 @@ static void print_size(struct drm_printer *p, const char *stat, > >> void drm_print_memory_stats(struct drm_printer *p, > >> const struct drm_memory_stats *stats, > >> enum drm_gem_object_status supported_status, > >> - const char *region) > >> + const char *region, > >> + unsigned int unit) > > > >I'm not really adverse to changing what units we use.. or perhaps > >changing the threshold to go to higher units to be 10000x or 100000x > >of the previous unit. But I'm less excited about having different > >drivers using different units. > > > >BR, > >-R > > Would it be alright if I left it set to the default unit, and allow changing it > at runtime with a debugfs file? I suppose we could, but it does seem a bit like overkill. OTOH I think it would make sense to increase the threshold, ie. switch to MiB after 10MiB instead of 1MiB.. at that point the fractional component is less significant.. BR, -R > >> { > >> - print_size(p, "total", region, stats->private + stats->shared); > >> - print_size(p, "shared", region, stats->shared); > >> - print_size(p, "active", region, stats->active); > >> + print_size(p, "total", region, stats->private + stats->shared, unit); > >> + print_size(p, "shared", region, stats->shared, unit); > >> + print_size(p, "active", region, stats->active, unit); > >> > >> if (supported_status & DRM_GEM_OBJECT_RESIDENT) > >> - print_size(p, "resident", region, stats->resident); > >> + print_size(p, "resident", region, stats->resident, unit); > >> > >> if (supported_status & DRM_GEM_OBJECT_PURGEABLE) > >> - print_size(p, "purgeable", region, stats->purgeable); > >> + print_size(p, "purgeable", region, stats->purgeable, unit); > >> } > >> EXPORT_SYMBOL(drm_print_memory_stats); > >> > >> @@ -916,11 +919,12 @@ EXPORT_SYMBOL(drm_print_memory_stats); > >> * drm_show_memory_stats - Helper to collect and show standard fdinfo memory stats > >> * @p: the printer to print output to > >> * @file: the DRM file > >> + * @unit: multipliyer of power of two exponent of desired unit > >> * > >> * Helper to iterate over GEM objects with a handle allocated in the specified > >> * file. > >> */ > >> -void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file) > >> +void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file, unsigned int unit) > >> { > >> struct drm_gem_object *obj; > >> struct drm_memory_stats status = {}; > >> @@ -967,7 +971,7 @@ void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file) > >> } > >> spin_unlock(&file->table_lock); > >> > >> - drm_print_memory_stats(p, &status, supported_status, "memory"); > >> + drm_print_memory_stats(p, &status, supported_status, "memory", unit); > >> } > >> EXPORT_SYMBOL(drm_show_memory_stats); > >> > >> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > >> index 2a0e3529598b..cd1198151744 100644 > >> --- a/drivers/gpu/drm/msm/msm_drv.c > >> +++ b/drivers/gpu/drm/msm/msm_drv.c > >> @@ -1067,7 +1067,7 @@ static void msm_show_fdinfo(struct drm_printer *p, struct drm_file *file) > >> > >> msm_gpu_show_fdinfo(priv->gpu, file->driver_priv, p); > >> > >> - drm_show_memory_stats(p, file); > >> + drm_show_memory_stats(p, file, 0); > >> } > >> > >> static const struct file_operations fops = { > >> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > >> index 93d5f5538c0b..79c08cee3e9d 100644 > >> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > >> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > >> @@ -563,7 +563,7 @@ static void panfrost_show_fdinfo(struct drm_printer *p, struct drm_file *file) > >> > >> panfrost_gpu_show_fdinfo(pfdev, file->driver_priv, p); > >> > >> - drm_show_memory_stats(p, file); > >> + drm_show_memory_stats(p, file, 1); > >> } > >> > >> static const struct file_operations panfrost_drm_driver_fops = { > >> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h > >> index 010239392adf..21a3b022dd63 100644 > >> --- a/include/drm/drm_file.h > >> +++ b/include/drm/drm_file.h > >> @@ -466,9 +466,10 @@ enum drm_gem_object_status; > >> void drm_print_memory_stats(struct drm_printer *p, > >> const struct drm_memory_stats *stats, > >> enum drm_gem_object_status supported_status, > >> - const char *region); > >> + const char *region, > >> + unsigned int unit); > >> > >> -void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file); > >> +void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file, unsigned int unit); > >> void drm_show_fdinfo(struct seq_file *m, struct file *f); > >> > >> struct file *mock_drm_getfile(struct drm_minor *minor, unsigned int flags); > >> -- > >> 2.42.0 > >>
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index 762965e3d503..517e1fb8072a 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -873,7 +873,7 @@ void drm_send_event(struct drm_device *dev, struct drm_pending_event *e) EXPORT_SYMBOL(drm_send_event); static void print_size(struct drm_printer *p, const char *stat, - const char *region, u64 sz) + const char *region, u64 sz, unsigned int unit) { const char *units[] = {"", " KiB", " MiB"}; unsigned u; @@ -881,6 +881,8 @@ static void print_size(struct drm_printer *p, const char *stat, for (u = 0; u < ARRAY_SIZE(units) - 1; u++) { if (sz < SZ_1K) break; + if (unit > 0 && unit == u) + break; sz = div_u64(sz, SZ_1K); } @@ -898,17 +900,18 @@ static void print_size(struct drm_printer *p, const char *stat, void drm_print_memory_stats(struct drm_printer *p, const struct drm_memory_stats *stats, enum drm_gem_object_status supported_status, - const char *region) + const char *region, + unsigned int unit) { - print_size(p, "total", region, stats->private + stats->shared); - print_size(p, "shared", region, stats->shared); - print_size(p, "active", region, stats->active); + print_size(p, "total", region, stats->private + stats->shared, unit); + print_size(p, "shared", region, stats->shared, unit); + print_size(p, "active", region, stats->active, unit); if (supported_status & DRM_GEM_OBJECT_RESIDENT) - print_size(p, "resident", region, stats->resident); + print_size(p, "resident", region, stats->resident, unit); if (supported_status & DRM_GEM_OBJECT_PURGEABLE) - print_size(p, "purgeable", region, stats->purgeable); + print_size(p, "purgeable", region, stats->purgeable, unit); } EXPORT_SYMBOL(drm_print_memory_stats); @@ -916,11 +919,12 @@ EXPORT_SYMBOL(drm_print_memory_stats); * drm_show_memory_stats - Helper to collect and show standard fdinfo memory stats * @p: the printer to print output to * @file: the DRM file + * @unit: multipliyer of power of two exponent of desired unit * * Helper to iterate over GEM objects with a handle allocated in the specified * file. */ -void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file) +void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file, unsigned int unit) { struct drm_gem_object *obj; struct drm_memory_stats status = {}; @@ -967,7 +971,7 @@ void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file) } spin_unlock(&file->table_lock); - drm_print_memory_stats(p, &status, supported_status, "memory"); + drm_print_memory_stats(p, &status, supported_status, "memory", unit); } EXPORT_SYMBOL(drm_show_memory_stats); diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 2a0e3529598b..cd1198151744 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -1067,7 +1067,7 @@ static void msm_show_fdinfo(struct drm_printer *p, struct drm_file *file) msm_gpu_show_fdinfo(priv->gpu, file->driver_priv, p); - drm_show_memory_stats(p, file); + drm_show_memory_stats(p, file, 0); } static const struct file_operations fops = { diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index 93d5f5538c0b..79c08cee3e9d 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -563,7 +563,7 @@ static void panfrost_show_fdinfo(struct drm_printer *p, struct drm_file *file) panfrost_gpu_show_fdinfo(pfdev, file->driver_priv, p); - drm_show_memory_stats(p, file); + drm_show_memory_stats(p, file, 1); } static const struct file_operations panfrost_drm_driver_fops = { diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index 010239392adf..21a3b022dd63 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -466,9 +466,10 @@ enum drm_gem_object_status; void drm_print_memory_stats(struct drm_printer *p, const struct drm_memory_stats *stats, enum drm_gem_object_status supported_status, - const char *region); + const char *region, + unsigned int unit); -void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file); +void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file, unsigned int unit); void drm_show_fdinfo(struct seq_file *m, struct file *f); struct file *mock_drm_getfile(struct drm_minor *minor, unsigned int flags);
The current implementation will try to pick the highest available unit. This is rather unflexible, and allowing drivers to display BO size statistics through fdinfo in units of their choice might be desirable. The new argument to drm_show_memory_stats is to be interpreted as the integer multiplier of a 10-power of 2, so 1 would give us size in Kib and 2 in Mib. If we want drm-file functions to pick the highest unit, then 0 should be passed. Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> --- drivers/gpu/drm/drm_file.c | 22 +++++++++++++--------- drivers/gpu/drm/msm/msm_drv.c | 2 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- include/drm/drm_file.h | 5 +++-- 4 files changed, 18 insertions(+), 13 deletions(-)