[Mesa-dev] [PATCH 3/4] winsys/radeon: Keep bo statistics

[Marek Olšák]

On Mon, Jan 6, 2014 at 12:17 PM, Lauri Kasanen <cand at gmx.com> wrote:
> These will be used later on for optimizing the VRAM placement.
>
> No measurable overhead (glxgears).

I recommend testing torcs (the Forza track) next time. glxgears is not
useful here.

>
> Signed-off-by: Lauri Kasanen <cand at gmx.com>
> ---
>  src/gallium/winsys/radeon/drm/radeon_drm_bo.c |  3 +++
>  src/gallium/winsys/radeon/drm/radeon_drm_bo.h | 16 ++++++++++++++++
>  src/gallium/winsys/radeon/drm/radeon_drm_cs.c |  8 ++++++++
>  3 files changed, 27 insertions(+)
>
> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
> index 7543840..9aa1a0f 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
> @@ -529,6 +529,9 @@ static void *radeon_bo_map(struct radeon_winsys_cs_handle *buf,
>          fprintf(ws->bo_stats_file, "%p cpu mapped @%llu\n", bo, stats_time_get(ws));
>      }
>
> +    bo->stats.num_cpu_ops++;
> +    bo->stats.last_cpu_time = stats_time_get(ws);
> +
>      return radeon_bo_do_map(bo);
>  }
>
> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.h b/src/gallium/winsys/radeon/drm/radeon_drm_bo.h
> index 5536bc1..651694b 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.h
> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.h
> @@ -44,6 +44,20 @@ struct radeon_bo_desc {
>      unsigned initial_domains;
>  };
>
> +struct radeon_bo_stats {
> +    uint64_t num_reads;
> +    uint64_t num_writes;
> +    uint64_t num_cpu_ops;
> +
> +    /* Milliseconds in a monotonic clock */
> +    uint64_t last_read_time;
> +    uint64_t last_write_time;
> +    uint64_t last_cpu_time;
> +
> +    /* Depth, MSAA, etc. */
> +    bool high_prio;
> +};
> +
>  struct radeon_bo {
>      struct pb_buffer base;
>
> @@ -67,6 +81,8 @@ struct radeon_bo {
>
>      boolean flinked;
>      uint32_t flink;
> +
> +    struct radeon_bo_stats stats;
>  };
>
>  struct pb_manager *radeon_bomgr_create(struct radeon_drm_winsys *rws);
> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_cs.c b/src/gallium/winsys/radeon/drm/radeon_drm_cs.c
> index 4d46e85..f78b6cc 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_drm_cs.c
> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_cs.c
> @@ -361,6 +361,14 @@ static unsigned radeon_drm_cs_add_reloc(struct radeon_winsys_cs *rcs,
>          }
>      }
>
> +    if (usage & RADEON_USAGE_WRITE) {
> +        bo->stats.num_writes++;
> +        bo->stats.last_write_time = stats_time_get(ws);
> +    } else {
> +        bo->stats.num_reads++;
> +        bo->stats.last_read_time = stats_time_get(ws);
> +    }

You do know that this is mostly useless, don't you? You sure won't
know which resources are used for reading only, because
glBufferSubData or glTexSubImage may generate a write, and an app
which uploads data every frame or every couple of draw calls can cause
the resource to appear as being used for writing all the time, while
in reality it's only used for reading during rendering. You also won't
know how many draw calls the resource is used in, because the function
is called only once per state change and there can be many draw calls
per state change. In my opinion, the only useful information would be
"last_use_time" and you can record it just once in radeon_cs_flush (or
radeon_drm_cs_emit_ioctl_oneshot if you want to do it in the CS
thread) instead of doing it in add_reloc, which may be called many
times for the same buffer.

Marek