[Mesa-dev] [PATCH 1/2] gallium/radeon: refactor the GRBM counters path
Nicolai Hähnle
nhaehnle at gmail.com
Tue Jan 24 09:16:26 UTC 2017
On 20.01.2017 20:19, Samuel Pitoiset wrote:
> This will allow to expose more queries in order to know which
> blocks are busy/idle.
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
> src/gallium/drivers/radeon/r600_gpu_load.c | 55 ++++++++++++++-------------
> src/gallium/drivers/radeon/r600_pipe_common.h | 18 +++++----
> src/gallium/drivers/radeon/r600_query.c | 14 +++----
> 3 files changed, 44 insertions(+), 43 deletions(-)
>
> diff --git a/src/gallium/drivers/radeon/r600_gpu_load.c b/src/gallium/drivers/radeon/r600_gpu_load.c
> index e3488b3ac7..38ddda1652 100644
> --- a/src/gallium/drivers/radeon/r600_gpu_load.c
> +++ b/src/gallium/drivers/radeon/r600_gpu_load.c
> @@ -35,6 +35,7 @@
> */
>
> #include "r600_pipe_common.h"
> +#include "r600_query.h"
> #include "os/os_time.h"
>
> /* For good accuracy at 1000 fps or lower. This will be inaccurate for higher
> @@ -45,6 +46,12 @@
> #define SPI_BUSY(x) (((x) >> 22) & 0x1)
> #define GUI_ACTIVE(x) (((x) >> 31) & 0x1)
>
> +#define UPDATE_COUNTER(field, mask) \
> + if (mask(value)) \
> + p_atomic_inc(&counters->named.field.busy); \
> + else \
> + p_atomic_inc(&counters->named.field.idle);
> +
Since this macro is very specific for this particular function, please
add an #undef UPDATE_COUNTER below.
General macro hygiene would suggest to wrap this in do { ... } while
(false). I suspect this is technically not really needed here, but it's
a good common practice.
Now I see you've already pushed this, oh well. If you make these changes
in a follow-up patch, consider them to have my R-b.
Cheers,
Nicolai
> static void r600_update_grbm_counters(struct r600_common_screen *rscreen,
> union r600_grbm_counters *counters)
> {
> @@ -52,15 +59,8 @@ static void r600_update_grbm_counters(struct r600_common_screen *rscreen,
>
> rscreen->ws->read_registers(rscreen->ws, GRBM_STATUS, 1, &value);
>
> - if (SPI_BUSY(value))
> - p_atomic_inc(&counters->named.spi_busy);
> - else
> - p_atomic_inc(&counters->named.spi_idle);
> -
> - if (GUI_ACTIVE(value))
> - p_atomic_inc(&counters->named.gui_busy);
> - else
> - p_atomic_inc(&counters->named.gui_idle);
> + UPDATE_COUNTER(spi, SPI_BUSY);
> + UPDATE_COUNTER(gui, GUI_ACTIVE);
> }
>
> static PIPE_THREAD_ROUTINE(r600_gpu_load_thread, param)
> @@ -104,8 +104,8 @@ void r600_gpu_load_kill_thread(struct r600_common_screen *rscreen)
> rscreen->gpu_load_thread = 0;
> }
>
> -static uint64_t r600_read_counter(struct r600_common_screen *rscreen,
> - unsigned busy_index)
> +static uint64_t r600_read_grbm_counter(struct r600_common_screen *rscreen,
> + unsigned busy_index)
> {
> /* Start the thread if needed. */
> if (!rscreen->gpu_load_thread) {
> @@ -123,10 +123,10 @@ static uint64_t r600_read_counter(struct r600_common_screen *rscreen,
> return busy | ((uint64_t)idle << 32);
> }
>
> -static unsigned r600_end_counter(struct r600_common_screen *rscreen,
> - uint64_t begin, unsigned busy_index)
> +static unsigned r600_end_grbm_counter(struct r600_common_screen *rscreen,
> + uint64_t begin, unsigned busy_index)
> {
> - uint64_t end = r600_read_counter(rscreen, busy_index);
> + uint64_t end = r600_read_grbm_counter(rscreen, busy_index);
> unsigned busy = (end & 0xffffffff) - (begin & 0xffffffff);
> unsigned idle = (end >> 32) - (begin >> 32);
>
> @@ -147,25 +147,28 @@ static unsigned r600_end_counter(struct r600_common_screen *rscreen,
> }
> }
>
> -#define BUSY_INDEX(rscreen, field) (&rscreen->grbm_counters.named.field##_busy - \
> +#define BUSY_INDEX(rscreen, field) (&rscreen->grbm_counters.named.field.busy - \
> rscreen->grbm_counters.array)
>
> -uint64_t r600_begin_counter_spi(struct r600_common_screen *rscreen)
> -{
> - return r600_read_counter(rscreen, BUSY_INDEX(rscreen, spi));
> -}
> -
> -unsigned r600_end_counter_spi(struct r600_common_screen *rscreen, uint64_t begin)
> +static unsigned busy_index_from_type(struct r600_common_screen *rscreen,
> + unsigned type)
> {
> - return r600_end_counter(rscreen, begin, BUSY_INDEX(rscreen, spi));
> + switch (type) {
> + case R600_QUERY_GPU_LOAD: return BUSY_INDEX(rscreen, gui);
> + case R600_QUERY_GPU_SHADERS_BUSY: return BUSY_INDEX(rscreen, spi);
> + default: unreachable("query type does not correspond to grbm id");
> + }
> }
>
> -uint64_t r600_begin_counter_gui(struct r600_common_screen *rscreen)
> +uint64_t r600_begin_counter(struct r600_common_screen *rscreen, unsigned type)
> {
> - return r600_read_counter(rscreen, BUSY_INDEX(rscreen, gui));
> + unsigned busy_index = busy_index_from_type(rscreen, type);
> + return r600_read_grbm_counter(rscreen, busy_index);
> }
>
> -unsigned r600_end_counter_gui(struct r600_common_screen *rscreen, uint64_t begin)
> +unsigned r600_end_counter(struct r600_common_screen *rscreen, unsigned type,
> + uint64_t begin)
> {
> - return r600_end_counter(rscreen, begin, BUSY_INDEX(rscreen, gui));
> + unsigned busy_index = busy_index_from_type(rscreen, type);
> + return r600_end_grbm_counter(rscreen, begin, busy_index);
> }
> diff --git a/src/gallium/drivers/radeon/r600_pipe_common.h b/src/gallium/drivers/radeon/r600_pipe_common.h
> index 97e944186e..e340e6f7f1 100644
> --- a/src/gallium/drivers/radeon/r600_pipe_common.h
> +++ b/src/gallium/drivers/radeon/r600_pipe_common.h
> @@ -352,12 +352,15 @@ struct r600_surface {
> unsigned db_preload_control; /* EG and later */
> };
>
> +struct r600_grbm_counter {
> + unsigned busy;
> + unsigned idle;
> +};
> +
> union r600_grbm_counters {
> struct {
> - unsigned spi_busy;
> - unsigned spi_idle;
> - unsigned gui_busy;
> - unsigned gui_idle;
> + struct r600_grbm_counter spi;
> + struct r600_grbm_counter gui;
> } named;
> unsigned array[0];
> };
> @@ -748,10 +751,9 @@ bool r600_check_device_reset(struct r600_common_context *rctx);
>
> /* r600_gpu_load.c */
> void r600_gpu_load_kill_thread(struct r600_common_screen *rscreen);
> -uint64_t r600_begin_counter_spi(struct r600_common_screen *rscreen);
> -unsigned r600_end_counter_spi(struct r600_common_screen *rscreen, uint64_t begin);
> -uint64_t r600_begin_counter_gui(struct r600_common_screen *rscreen);
> -unsigned r600_end_counter_gui(struct r600_common_screen *rscreen, uint64_t begin);
> +uint64_t r600_begin_counter(struct r600_common_screen *rscreen, unsigned type);
> +unsigned r600_end_counter(struct r600_common_screen *rscreen, unsigned type,
> + uint64_t begin);
>
> /* r600_perfcounters.c */
> void r600_perfcounters_destroy(struct r600_common_screen *rscreen);
> diff --git a/src/gallium/drivers/radeon/r600_query.c b/src/gallium/drivers/radeon/r600_query.c
> index 5712cbe63f..1f9f191049 100644
> --- a/src/gallium/drivers/radeon/r600_query.c
> +++ b/src/gallium/drivers/radeon/r600_query.c
> @@ -145,10 +145,9 @@ static bool r600_query_sw_begin(struct r600_common_context *rctx,
> break;
> }
> case R600_QUERY_GPU_LOAD:
> - query->begin_result = r600_begin_counter_gui(rctx->screen);
> - break;
> case R600_QUERY_GPU_SHADERS_BUSY:
> - query->begin_result = r600_begin_counter_spi(rctx->screen);
> + query->begin_result = r600_begin_counter(rctx->screen,
> + query->b.type);
> break;
> case R600_QUERY_NUM_COMPILATIONS:
> query->begin_result = p_atomic_read(&rctx->screen->num_compilations);
> @@ -239,13 +238,10 @@ static bool r600_query_sw_end(struct r600_common_context *rctx,
> break;
> }
> case R600_QUERY_GPU_LOAD:
> - query->end_result = r600_end_counter_gui(rctx->screen,
> - query->begin_result);
> - query->begin_result = 0;
> - break;
> case R600_QUERY_GPU_SHADERS_BUSY:
> - query->end_result = r600_end_counter_spi(rctx->screen,
> - query->begin_result);
> + query->end_result = r600_end_counter(rctx->screen,
> + query->b.type,
> + query->begin_result);
> query->begin_result = 0;
> break;
> case R600_QUERY_NUM_COMPILATIONS:
>
More information about the mesa-dev
mailing list