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
