[PATCH] Add GPU_POWER sensors

[Tom St Denis]

On 02/10/2017 07:25 PM, Edward O'Callaghan wrote:
> Hey Tom,
>
> On 02/11/2017 05:10 AM, Tom St Denis wrote:
>> Add the ability to sample GPU_POWER sensors.  Because
>> the sensors have a high latency we read them from a background
>> thread which means we've added the requirement for pthreads.
>>
>> Signed-off-by: Tom St Denis <tom.stdenis at amd.com>
>> ---
>>  CMakeLists.txt         |  5 ++-
>>  README                 |  6 ++--
>>  src/app/top.c          | 88 +++++++++++++++++++++++++++++++++++++++++---------
>>  src/lib/CMakeLists.txt |  1 +
>>  src/lib/read_sensor.c  | 37 +++++++++++++++++++++
>>  src/umr.h              |  5 +++
>>  6 files changed, 123 insertions(+), 19 deletions(-)
>>  create mode 100644 src/lib/read_sensor.c
>>
>> diff --git a/CMakeLists.txt b/CMakeLists.txt
>> index ef78c97ad763..7b771d01919b 100644
>> --- a/CMakeLists.txt
>> +++ b/CMakeLists.txt
>> @@ -19,6 +19,9 @@ add_definitions(-DUMR_BUILD_REV=\"${GIT_REV}\")
>>  # Add local repository for FindXXX.cmake modules.
>>  SET(CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake_modules/" ${CMAKE_MODULE_PATH})
>>
>> +find_package(Threads REQUIRED)
>> +include_directories(${THREADS_INCLUDE_DIRS})
> Do you need this include_directories() line?
>
>> +
>>  find_package(Curses REQUIRED)
>>  include_directories(${CURSES_INCLUDE_DIRS})
>>
>> @@ -37,7 +40,7 @@ set(REQUIRED_EXTERNAL_LIBS
>>  set(CMAKE_POSITION_INDEPENDENT_CODE ON)
>>
>>  # CFLAGS += -Wall -W -O2 -g3 -Isrc/ -DPIC -fPIC
>> -set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -W -O2 -g3")
>> +set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -pthread -Wall -W -O2 -g3")
> You don't really want to have your linkage flags here, I think your
> looking for ${CMAKE_THREAD_LIBS_INIT} to go into the
> REQUIRED_EXTERNAL_LIBS list.

How does it "go into"?  Simple '+='?

Can you just send a quick patch I can squash into this?  The less I know 
about cmake the more room I have in my head for useful things :-) hehehe.

>>  enum drm_print {
>> @@ -171,19 +173,14 @@ static struct umr_bitfield stat_uvd_pgfsm7_bits[] = {
>>  static struct umr_bitfield stat_mc_hub_bits[] = {
>>  	 { "OUTSTANDING_READ", 255, 255, &umr_bitfield_default },
>>  	 { "OUTSTANDING_WRITE", 255, 255, &umr_bitfield_default },
>> -//	 { "OUTSTANDING_ATOMIC", 255, 255, &umr_bitfield_default },
>>  	 { "OUTSTANDING_HUB_RDREQ", 255, 255, &umr_bitfield_default },
>>  	 { "OUTSTANDING_HUB_RDRET", 255, 255, &umr_bitfield_default },
>>  	 { "OUTSTANDING_HUB_WRREQ", 255, 255, &umr_bitfield_default },
>>  	 { "OUTSTANDING_HUB_WRRET", 255, 255, &umr_bitfield_default },
>> -//	 { "OUTSTANDING_HUB_ATOMIC_REQ", 255, 255, &umr_bitfield_default },
>> -//	 { "OUTSTANDING_HUB_ATOMIC_RET", 255, 255, &umr_bitfield_default },
>>  	 { "OUTSTANDING_RPB_READ", 255, 255, &umr_bitfield_default },
>>  	 { "OUTSTANDING_RPB_WRITE", 255, 255, &umr_bitfield_default },
>> -//	 { "OUTSTANDING_RPB_ATOMIC", 255, 255, &umr_bitfield_default },
>>  	 { "OUTSTANDING_MCD_READ", 255, 255, &umr_bitfield_default },
>>  	 { "OUTSTANDING_MCD_WRITE", 255, 255, &umr_bitfield_default },
>> -//	 { "OUTSTANDING_MCD_ATOMIC", 255, 255, &umr_bitfield_default },
>>  	 { NULL, 0, 0, NULL },
>>  };
> this hulk seems unrelated to this patch?

It is, but since there's no functional change I figured I'd just squash 
that in as house keeping.

Cheers,
tom