[Mesa-dev] [PATCH] gallium/radeon: add HUD queries for GPU temperature and clocks

[Samuel Pitoiset]

On 02/15/2017 11:59 AM, Nicolai Hähnle wrote:
> On 14.02.2017 18:20, Marek Olšák wrote:
>> On Tue, Feb 14, 2017 at 9:23 AM, Nicolai Hähnle <nhaehnle at gmail.com>
>> wrote:
>>> On 13.02.2017 23:08, Samuel Pitoiset wrote:
>>>>
>>>> Only the Radeon kernel driver exposed the GPU temperature and
>>>> the shader/memory clocks, this implements the same functionality
>>>> for the AMDGPU kernel driver.
>>>>
>>>> These queries will return 0 if the DRM version is less than 3.10,
>>>> I don't explicitely check the version here because the query
>>>> codepath is already a bit messy.
>>>>
>>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>>>> ---
>>>>  src/gallium/drivers/radeon/r600_query.c       | 12 ++++++------
>>>>  src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c | 19 ++++++++++++++++++-
>>>>  2 files changed, 24 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/src/gallium/drivers/radeon/r600_query.c
>>>> b/src/gallium/drivers/radeon/r600_query.c
>>>> index 05741d33c9..4959621e11 100644
>>>> --- a/src/gallium/drivers/radeon/r600_query.c
>>>> +++ b/src/gallium/drivers/radeon/r600_query.c
>>>> @@ -1768,6 +1768,10 @@ static struct pipe_driver_query_info
>>>> r600_driver_query_list[] = {
>>>>         XG(GPIN, "GPIN_003",            GPIN_NUM_SPI,           UINT,
>>>> AVERAGE),
>>>>         XG(GPIN, "GPIN_004",            GPIN_NUM_SE,            UINT,
>>>> AVERAGE),
>>>>
>>>> +       X("temperature",                GPU_TEMPERATURE,        UINT64,
>>>> AVERAGE),
>>>> +       X("shader-clock",               CURRENT_GPU_SCLK,       HZ,
>>>> AVERAGE),
>>>> +       X("memory-clock",               CURRENT_GPU_MCLK,       HZ,
>>>> AVERAGE),
>>>> +
>>>>         /* The following queries must be at the end of the list because
>>>> their
>>>>          * availability is adjusted dynamically based on the DRM
>>>> version.
>>>> */
>>>>         X("GPU-load",                   GPU_LOAD,               UINT64,
>>>> AVERAGE),
>>>> @@ -1792,10 +1796,6 @@ static struct pipe_driver_query_info
>>>> r600_driver_query_list[] = {
>>>>         X("GPU-dma-busy",               GPU_DMA_BUSY,           UINT64,
>>>> AVERAGE),
>>>>         X("GPU-scratch-ram-busy",       GPU_SCRATCH_RAM_BUSY,   UINT64,
>>>> AVERAGE),
>>>>         X("GPU-ce-busy",                GPU_CE_BUSY,            UINT64,
>>>> AVERAGE),
>>>> -
>>>> -       X("temperature",                GPU_TEMPERATURE,        UINT64,
>>>> AVERAGE),
>>>> -       X("shader-clock",               CURRENT_GPU_SCLK,       HZ,
>>>> AVERAGE),
>>>> -       X("memory-clock",               CURRENT_GPU_MCLK,       HZ,
>>>> AVERAGE),
>>>>  };
>>>>
>>>>  #undef X
>>>> @@ -1808,9 +1808,9 @@ static unsigned r600_get_num_queries(struct
>>>> r600_common_screen *rscreen)
>>>>                 return ARRAY_SIZE(r600_driver_query_list);
>>>>         else if (rscreen->info.drm_major == 3) {
>>>>                 if (rscreen->chip_class >= VI)
>>>> -                       return ARRAY_SIZE(r600_driver_query_list) - 3;
>>>> +                       return ARRAY_SIZE(r600_driver_query_list);
>>>>                 else
>>>> -                       return ARRAY_SIZE(r600_driver_query_list) - 10;
>>>> +                       return ARRAY_SIZE(r600_driver_query_list) - 7;
>>>>         }
>>>>         else
>>>>                 return ARRAY_SIZE(r600_driver_query_list) - 25;
>>>> diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
>>>> b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
>>>> index c3dfda53f0..d99df9e941 100644
>>>> --- a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
>>>> +++ b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
>>>> @@ -63,6 +63,18 @@
>>>>  #define AMDGPU_INFO_NUM_EVICTIONS              0x18
>>>>  #endif
>>>>
>>>> +#ifndef AMDGPU_INFO_CURRENT_GPU_SCLK
>>>> +#define AMDGPU_INFO_CURRENT_GPU_SCLK   0x1c
>>>> +#endif
>>>> +
>>>> +#ifndef AMDGPU_INFO_CURRENT_GPU_MCLK
>>>> +#define AMDGPU_INFO_CURRENT_GPU_MCLK   0x1d
>>>> +#endif
>>>> +
>>>> +#ifndef AMDGPU_INFO_CURRENT_GPU_TEMP
>>>> +#define AMDGPU_INFO_CURRENT_GPU_TEMP   0x1e
>>>> +#endif
>>>> +
>>>>  static struct util_hash_table *dev_tab = NULL;
>>>>  pipe_static_mutex(dev_tab_mutex);
>>>>
>>>> @@ -459,9 +471,14 @@ static uint64_t amdgpu_query_value(struct
>>>> radeon_winsys *rws,
>>>>        amdgpu_query_heap_info(ws->dev, AMDGPU_GEM_DOMAIN_GTT, 0,
>>>> &heap);
>>>>        return heap.heap_usage;
>>>>     case RADEON_GPU_TEMPERATURE:
>>>> +      amdgpu_query_info(ws->dev, AMDGPU_INFO_CURRENT_GPU_TEMP, 4,
>>>> &retval);
>>>> +      return retval;
>>>>     case RADEON_CURRENT_SCLK:
>>>> +      amdgpu_query_info(ws->dev, AMDGPU_INFO_CURRENT_GPU_SCLK, 4,
>>>> &retval);
>>>> +      return retval;
>>>>     case RADEON_CURRENT_MCLK:
>>>> -      return 0;
>>>> +      amdgpu_query_info(ws->dev, AMDGPU_INFO_CURRENT_GPU_MCLK, 4,
>>>> &retval);
>>>> +      return retval;
>>>
>>>
>>> I think these should be guarded with a DRM (kernel) version check. Also,
>>> what Emil said.
>>
>> Let's ignore the DRM version if the query doesn't send any messages to
>> dmesg. Reporting 0 in the HUD is already self-explanatory. (at least
>> for us) We don't even check the DRM version for older queries that are
>> not universally supported.
>
> Fair enough, as long as it doesn't result in dmesg spam it's no big deal.

It doesn't.

>
> Cheers,
> Nicolai
>
>>
>> Marek
>>
>