On 02/11/2017 08:29 AM, Kai Wasserbäch wrote:
Hey Tom, Tom St Denis wrote on 11.02.2017 12:26: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 <[email protected]> (v2) Cleaned up CMake around pthreads --- CMakeLists.txt | 4 +++ 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(+), 18 deletions(-) create mode 100644 src/lib/read_sensor.c diff --git a/CMakeLists.txt b/CMakeLists.txt index ef78c97ad763..8d89445c39e3 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}) +set(CMAKE_THREAD_PREFER_PTHREAD TRUE) +find_package(Threads REQUIRED) + find_package(Curses REQUIRED) include_directories(${CURSES_INCLUDE_DIRS}) @@ -31,6 +34,7 @@ include_directories(${LIBDRM_INCLUDE_DIR}) set(REQUIRED_EXTERNAL_LIBS ${CURSES_LIBRARIES} ${PCIACCESS_LIBRARIES} + Threads::Threads ) # Global setting: build everything position independent diff --git a/README b/README index 13cdac663d20..8a8de8485ac7 100644 --- a/README +++ b/README @@ -28,9 +28,9 @@ mailing list at: Building --------- -To build umr you will need pciaccess, ncurses, and libdrm headers and -libraries. Which are available in both Fedora and Ubuntu (as well as -other distributions). To build umr: +To build umr you will need pciaccess, ncurses, libdrm, and pthread +headers and libraries. Which are available in both Fedora and Ubuntu +(as well as other distributions). To build umr:maybe just write "most distributions"? Since you're not giving package names I don't really see a point in naming two distributions, especially where one is just a derivative.
I can update that. To be honest I do 99% of my testing with Fedora and Ubuntu is used quite a bit amongst AMDers.
$ mkdir build && cd build/ && cmake ../ $ make diff --git a/src/app/top.c b/src/app/top.c index b081515a5b40..60f629d247f3 100644 --- a/src/app/top.c +++ b/src/app/top.c @@ -54,6 +54,7 @@ enum sensor_maps { SENSOR_IDENTITY=0, // x = x SENSOR_D1000, // x = x/1000 SENSOR_D100, // x = x/100 + SENSOR_WATT, }; enum sensor_print { @@ -61,6 +62,7 @@ enum sensor_print { SENSOR_MHZ, SENSOR_PERCENT, SENSOR_TEMP, + SENSOR_POWER, }; 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 }, };I would agree with Edward and rather have these in a separate clean-up patch.
It's interesting that you note this.
Could you put the assignment and break on their own lines? Would increase readability IMHO. Also maybe add spaces between variables, operators and constants? I'm imagining something like
Because of this.I'm not disagreeing with your formatting suggestion it just seems at this point we're building a deluxe bike shed. I already own a bike shed (which my daughter calls a mini house) and don't need another. :-)
If I revert the hunk about the removed comment lines I cannot logically include your formatting changes since they're not material to the functional changes introduced by this commit.
A reasonable compromise is to split this into two patches one which cleans up formatting first (and removes the commented lines) and the 2nd which introduces the new sensors.
Tom _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
