[Mesa-dev] [PATCH 1/2] gallium/radeon: refactor the GRBM counters path
Samuel Pitoiset
samuel.pitoiset at gmail.com
Tue Jan 24 09:57:52 UTC 2017
On 01/24/2017 10:16 AM, Nicolai Hähnle wrote:
> 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.
Sorry, next time I will wait for your Rb as well.
I will make these changes in a follow-up patch.
Thanks Nicolai.
>
> 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