[Intel-gfx] [PATCH v4] drm/i915/lmem: debugfs for LMEM details

[Chris Wilson]

Quoting Ramalingam C (2019-12-24 08:43:00)
> From: Lukasz Fiedorowicz <lukasz.fiedorowicz at intel.com>
> 
> Debugfs i915_gem_object is extended to enable the IGTs to
> detect the LMEM's availability and the total size of LMEM.
> 
> v2: READ_ONCE is used [Chris]
> v3: %pa is used for printing the resource [Chris]
> v4: All regions' details added to debugfs [Chris]
> 
> Signed-off-by: Lukasz Fiedorowicz <lukasz.fiedorowicz at intel.com>
> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> Signed-off-by: Stuart Summers <stuart.summers at intel.com>
> Signed-off-by: Ramalingam C <ramalingam.c at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c        | 12 +++++++++++-
>  drivers/gpu/drm/i915/intel_memory_region.c | 19 ++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_memory_region.h |  4 ++++
>  3 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index d28468eaed57..35f8db7690a5 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -367,13 +367,23 @@ static void print_context_stats(struct seq_file *m,
>  static int i915_gem_object_info(struct seq_file *m, void *data)
>  {
>         struct drm_i915_private *i915 = node_to_i915(m->private);
> +       struct intel_memory_region *mem;
> +       enum intel_region_id id;
>  
>         seq_printf(m, "%u shrinkable [%u free] objects, %llu bytes\n",
>                    i915->mm.shrink_count,
>                    atomic_read(&i915->mm.free_count),
>                    i915->mm.shrink_memory);
>  
> -       seq_putc(m, '\n');

Keep the newline before the switch to context info and after the general
memory info.

> +       for (id = INTEL_REGION_SMEM; id < INTEL_REGION_UNKNOWN; id++) {

We don't have a for_each_memregion() yet?

#define for_each_memory_region(mr, i915, id) \
	for (id = 0; id < ARRAY_SIZE((i915)->mm.regions); id++) \
		for_each_if(mr = i915->mm.regions[id])

> +               mem = i915->mm.regions[id];
> +               if (!mem)
> +                       continue;
> +
> +               seq_printf(m, "%s total: %pa bytes, available %pa bytes\n",
> +                          intel_get_mem_region_name(mem), &mem->total,
> +                          &mem->avail);
> +       }
>  
>         print_context_stats(m, i915);
>  
> diff --git a/drivers/gpu/drm/i915/intel_memory_region.c b/drivers/gpu/drm/i915/intel_memory_region.c
> index e24c280e5930..824aa13e993d 100644
> --- a/drivers/gpu/drm/i915/intel_memory_region.c
> +++ b/drivers/gpu/drm/i915/intel_memory_region.c
> @@ -10,6 +10,20 @@
>  #define REGION_MAP(type, inst) \
>         BIT((type) + INTEL_MEMORY_TYPE_SHIFT) | BIT(inst)
>  
> +char *intel_get_mem_region_name(struct intel_memory_region *mem)

const char. But as this does not scale, just put a char name[8] into
intel_memory_region and populate it on init.

> +{
> +       switch (MEMORY_TYPE_FROM_REGION(mem->id)) {
> +       case INTEL_REGION_SMEM:
> +               return "SYSTEM MEM";

WHY ALL THE SHOUTING OF DIFFERING NAMES?

Just system, local, stolen would do.

> +       case INTEL_REGION_LMEM:
> +               return "LOCAL MEM";
> +       case INTEL_REGION_STOLEN:
> +               return "STOLEN MEM";
> +       default:
> +               return NULL;
> +       }
> +}