[PATCH] Add GPU_POWER sensors

[Tom St Denis]

On 11/02/17 05:56 AM, Kai Wasserbäch wrote:
> Hey Tom,
> Tom St Denis wrote on 11.02.2017 02:02:
>> 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.
>
> 1. You might want to set
>   set(CMAKE_THREAD_PREFER_PTHREAD TRUE)
> at before the find_package() for Threads
> 2. Linking usually happens through target_link_*() calls, where you would add
> Threads::Threads (special thing created by FindThreads.cmake) to the list of
> your other variables.
>
> See <https://cmake.org/cmake/help/v3.7/module/FindThreads.html> for details on
> how FindThreads.cmake works. In your case you might want to set
> THREADS_PREFER_PTHREAD_FLAG as well.

Thanks.  I've sent v2 to the list.

Cheers,
Tom