diff --git a/projects/amdsmi/CHANGELOG.md b/projects/amdsmi/CHANGELOG.md index 8cbaf2b9461e..ddaf7becf5bc 100644 --- a/projects/amdsmi/CHANGELOG.md +++ b/projects/amdsmi/CHANGELOG.md @@ -135,6 +135,9 @@ GPU: 0 ### Resolved Issues +- **Fixed `amd-smi metric --clock` reporting FCLK `max_clk` as 0 MHz**. + - `amdsmi_get_clock_info()` derived the FCLK (data-fabric) range from `pp_od_clk_voltage`, which on some GPUs (e.g. MI45x) lists only `OD_SCLK`/`OD_MCLK` and no FCLK section. The parsed maximum stayed at 0 and the code never fell back to `pp_dpm_fclk`, so `amd-smi metric --clock` showed `FCLK_0 MAX_CLK: 0 MHz`. The range now falls back to `pp_dpm_fclk` (and the analogous `pp_dpm_sclk`/`pp_dpm_mclk`) whenever the overdrive file omits that domain. + - **Fixed `amd-smi ras --cper --json` emitting nothing when there are no CPER entries**. - The common no-entries case printed empty output, so consumers feeding stdout to `json.loads` failed with `Expecting value: line 1 column 1 (char 0)`. The command now always emits exactly one valid JSON document: `[]` when there are no entries, or a single aggregated array across all GPUs when there are. `--follow` mode stays silent until entries appear. The human-readable primary-partition warning is also suppressed in JSON mode so it no longer corrupts the output. diff --git a/projects/amdsmi/include/amd_smi/impl/amd_smi_clk_testing.h b/projects/amdsmi/include/amd_smi/impl/amd_smi_clk_testing.h new file mode 100644 index 000000000000..a43fab10a472 --- /dev/null +++ b/projects/amdsmi/include/amd_smi/impl/amd_smi_clk_testing.h @@ -0,0 +1,39 @@ +/* + * Copyright (c) Advanced Micro Devices, Inc. All rights reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#pragma once + +#include + +#include "amd_smi/amdsmi.h" + +// Library-local test seam for the pp_od_clk_voltage parser behind amd-smi's +// per-domain min/max clock. Not amdsmi_-prefixed, so the linker version script +// keeps it out of libamd_smi.so; tests reach it through the static archive. +// Shared by the definition (src/amd_smi/amd_smi_utils.cc) and the unit tests so +// the signature stays in sync. +// +// Returns true and writes *max_freq/*min_freq when the domain's overdrive +// section has a nonzero max; false when the section is absent (MI45x has no +// OD_FCLK) or all levels read zero, so the caller falls back to pp_dpm_*. +bool smi_amdgpu_parse_od_clk_range(std::istream& od_stream, amdsmi_clk_type_t domain, + unsigned int* max_freq, unsigned int* min_freq); diff --git a/projects/amdsmi/include/amd_smi/impl/amd_smi_utils.h b/projects/amdsmi/include/amd_smi/impl/amd_smi_utils.h index 4a275c485211..4727e542a0fd 100644 --- a/projects/amdsmi/include/amd_smi/impl/amd_smi_utils.h +++ b/projects/amdsmi/include/amd_smi/impl/amd_smi_utils.h @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include diff --git a/projects/amdsmi/src/amd_smi/amd_smi_utils.cc b/projects/amdsmi/src/amd_smi/amd_smi_utils.cc index 820ba98eac02..1010818b6750 100644 --- a/projects/amdsmi/src/amd_smi/amd_smi_utils.cc +++ b/projects/amdsmi/src/amd_smi/amd_smi_utils.cc @@ -51,6 +51,7 @@ #include #include +#include "amd_smi/impl/amd_smi_clk_testing.h" #include "amd_smi/impl/amd_smi_common.h" #include "amd_smi/impl/amd_smi_gpu_mutex.h" #include "amd_smi/impl/amd_smi_system.h" @@ -296,6 +297,62 @@ amdsmi_status_t smi_amdgpu_get_power_cap(amd::smi::AMDSmiGPUDevice* device, uint return AMDSMI_STATUS_SUCCESS; } +bool smi_amdgpu_parse_od_clk_range(std::istream& od_stream, amdsmi_clk_type_t domain, + unsigned int* max_freq, unsigned int* min_freq) { + // Section header (and its GFXCLK/MCLK/FCLK alias) whose levels feed this + // domain's user-defined range. + const char* od_header = nullptr; + const char* alias_header = nullptr; + switch (domain) { + case AMDSMI_CLK_TYPE_GFX: + od_header = "OD_SCLK:"; + alias_header = "GFXCLK:"; + break; + case AMDSMI_CLK_TYPE_MEM: + od_header = "OD_MCLK:"; + alias_header = "MCLK:"; + break; + case AMDSMI_CLK_TYPE_DF: + od_header = "OD_FCLK:"; + alias_header = "FCLK:"; + break; + default: + return false; + } + + unsigned int max = 0; + unsigned int min = UINT_MAX; + bool in_domain = false; + bool found = false; + char str[10]; + unsigned int dpm_level, freq; + for (std::string line; getline(od_stream, line);) { + // Section headers end with ':'. This domain's header (or its alias) starts + // capture; any other header ends it, so an adjacent section such as + // OD_VDDC_CURVE is not folded into the range. + if (!line.empty() && line.back() == ':') { + in_domain = line.compare(od_header) == 0 || line.compare(alias_header) == 0; + continue; + } + if (!in_domain) { + continue; + } + if (sscanf(line.c_str(), "%u: %u%9s", &dpm_level, &freq, str) <= 2) { + continue; // skip lines that don't conform to the format + } + found = true; + if (freq > max) max = freq; + if (freq < min) min = freq; + } + + if (!found || max == 0) { + return false; + } + *max_freq = max; + *min_freq = min; + return true; +} + amdsmi_status_t smi_amdgpu_get_ranges(amd::smi::AMDSmiGPUDevice* device, amdsmi_clk_type_t domain, int* max_freq, int* min_freq, int* num_dpm, int* sleep_state_freq) { @@ -304,19 +361,17 @@ amdsmi_status_t smi_amdgpu_get_ranges(amd::smi::AMDSmiGPUDevice* device, amdsmi_ std::string smclk_min_max_fullpath = ""; - bool sclk = false; - bool mclk = false; - bool fclk = false; + bool use_od_range = false; switch (domain) { case AMDSMI_CLK_TYPE_GFX: smclk_min_max_fullpath = fullpath + "/pp_od_clk_voltage"; fullpath += "/pp_dpm_sclk"; - sclk = true; + use_od_range = true; break; case AMDSMI_CLK_TYPE_MEM: smclk_min_max_fullpath = fullpath + "/pp_od_clk_voltage"; fullpath += "/pp_dpm_mclk"; - mclk = true; + use_od_range = true; break; case AMDSMI_CLK_TYPE_VCLK0: fullpath += "/pp_dpm_vclk"; @@ -336,7 +391,7 @@ amdsmi_status_t smi_amdgpu_get_ranges(amd::smi::AMDSmiGPUDevice* device, amdsmi_ case AMDSMI_CLK_TYPE_DF: smclk_min_max_fullpath = fullpath + "/pp_od_clk_voltage"; fullpath += "/pp_dpm_fclk"; - fclk = true; + use_od_range = true; break; default: return AMDSMI_STATUS_INVAL; @@ -355,65 +410,13 @@ amdsmi_status_t smi_amdgpu_get_ranges(amd::smi::AMDSmiGPUDevice* device, amdsmi_ dpm = 0; sleep_freq = UINT_MAX; current_freq = 0; - // if getting sclk, mclk or fclk info, read pp_od_clk_voltage for min and max info - if (sclk || mclk || fclk) { + // GFX/MEM/DF expose a user-defined range in pp_od_clk_voltage; when it omits + // this domain's section (e.g. no OD_FCLK on MI45x) fall back to the pp_dpm_* + // levels below. + if (use_od_range) { std::ifstream smclk_ranges(smclk_min_max_fullpath.c_str()); - unsigned int smax = 0; - unsigned int mmax = 0; - unsigned int fmax = 0; - unsigned int smin = UINT_MAX; - unsigned int mmin = UINT_MAX; - unsigned int fmin = UINT_MAX; - - // if pp_od_clk_voltage is not found, then go back to using the original pp_dpm files - if (!smclk_ranges.is_open()) { - sclk = false; - mclk = false; - fclk = false; - } else { - // using enum to switch between recording for sclk, mclk, or fclk - enum ClkType { PARSING_SCLK = 0, PARSING_MCLK = 1, PARSING_FCLK = 2 }; - ClkType current_clk_type = PARSING_SCLK; - unsigned int dpm_level, freq; - for (std::string line; getline(smclk_ranges, line);) { - if (line.compare("GFXCLK:") == 0 || line.compare("OD_SCLK:") == 0) { - current_clk_type = PARSING_SCLK; - continue; - } else if (line.compare("MCLK:") == 0 || line.compare("OD_MCLK:") == 0) { - current_clk_type = PARSING_MCLK; - continue; - } else if (line.compare("FCLK:") == 0 || line.compare("OD_FCLK:") == 0) { - current_clk_type = PARSING_FCLK; - continue; - } - if (sscanf(line.c_str(), "%u: %d%9s", &dpm_level, &freq, str) <= 2) { - // skip lines that don't conform to the format - continue; - } - if (current_clk_type == PARSING_SCLK) { - if (freq > smax) smax = freq; - if (freq < smin) smin = freq; - } else if (current_clk_type == PARSING_MCLK) { - if (freq > mmax) mmax = freq; - if (freq < mmin) mmin = freq; - } else if (current_clk_type == PARSING_FCLK) { - if (freq > fmax) fmax = freq; - if (freq < fmin) fmin = freq; - } - } - - if (sclk) { - max = smax; - min = smin; - } else if (mclk) { - max = mmax; - min = mmin; - } else if (fclk) { - max = fmax; - min = fmin; - } - - smclk_ranges.close(); + if (!smi_amdgpu_parse_od_clk_range(smclk_ranges, domain, &max, &min)) { + use_od_range = false; } } // obtain rest of info from regular pp_dpm_* files. @@ -422,7 +425,7 @@ amdsmi_status_t smi_amdgpu_get_ranges(amd::smi::AMDSmiGPUDevice* device, amdsmi_ char firstChar = line[0]; if (firstChar == 'S') { - if (sscanf(line.c_str(), "%c: %d%9s", &single_char, &sleep_freq, str) <= 2) { + if (sscanf(line.c_str(), "%c: %u%9s", &single_char, &sleep_freq, str) <= 2) { ranges.close(); return AMDSMI_STATUS_NO_DATA; } @@ -436,7 +439,7 @@ amdsmi_status_t smi_amdgpu_get_ranges(amd::smi::AMDSmiGPUDevice* device, amdsmi_ * incorrect min value. */ - if (sscanf(line.c_str(), "%u: %d%c", &dpm_level, &freq, str) <= 2) { + if (sscanf(line.c_str(), "%u: %u%c", &dpm_level, &freq, str) <= 2) { ranges.close(); return AMDSMI_STATUS_IO; } @@ -446,9 +449,8 @@ amdsmi_status_t smi_amdgpu_get_ranges(amd::smi::AMDSmiGPUDevice* device, amdsmi_ current_freq = freq; } - // not * was detected so check for the min max if not sclk, mclk, or fclk, which are user - // defined - if (!sclk && !mclk && !fclk) { + // Domains without an OD range derive min/max from the dpm levels here. + if (!use_od_range) { max = freq > max ? freq : max; min = freq < min ? freq : min; } diff --git a/projects/amdsmi/tests/amd_smi_test/unit/gpu/clock_range_test.cc b/projects/amdsmi/tests/amd_smi_test/unit/gpu/clock_range_test.cc new file mode 100644 index 000000000000..64a6bada1bcb --- /dev/null +++ b/projects/amdsmi/tests/amd_smi_test/unit/gpu/clock_range_test.cc @@ -0,0 +1,250 @@ +/* + * Copyright (c) Advanced Micro Devices, Inc. All rights reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +// Unit tests for smi_amdgpu_parse_od_clk_range(), the pp_od_clk_voltage parser +// behind amd-smi's per-domain min/max clock. Driven over in-memory streams -- +// no GPU required. Guards the MI45x case where pp_od_clk_voltage carries no +// OD_FCLK section: the parser must report the domain absent so the caller falls +// back to pp_dpm_fclk instead of reporting a max of 0 MHz. + +#include + +#include +#include + +#include "amd_smi/amdsmi.h" +#include "amd_smi/impl/amd_smi_clk_testing.h" + +namespace { + +// pp_od_clk_voltage as seen on MI45x: SCLK and MCLK sections plus an OD_RANGE +// footer, but no OD_FCLK section. +constexpr char kOdNoFclk[] = + "OD_SCLK:\n" + "0: 500Mhz\n" + "1: 2100Mhz\n" + "OD_MCLK:\n" + "0: 900Mhz\n" + "1: 1200Mhz\n" + "OD_RANGE:\n" + "SCLK: 500Mhz 2100Mhz\n" + "MCLK: 900Mhz 1200Mhz\n"; + +// Same layout with an explicit OD_FCLK section present. +constexpr char kOdWithFclk[] = + "OD_SCLK:\n" + "0: 500Mhz\n" + "1: 2100Mhz\n" + "OD_FCLK:\n" + "0: 1000Mhz\n" + "1: 1100Mhz\n"; + +// Bare GFXCLK/MCLK/FCLK aliases in place of the OD_* headers, as some GPUs emit. +constexpr char kOdAliasHeaders[] = + "GFXCLK:\n" + "0: 500Mhz\n" + "1: 2100Mhz\n" + "MCLK:\n" + "0: 900Mhz\n" + "1: 1200Mhz\n" + "FCLK:\n" + "0: 1000Mhz\n" + "1: 1100Mhz\n"; + +// OD_FCLK present but every level reads 0 -- no usable range. +constexpr char kOdFclkAllZero[] = + "OD_FCLK:\n" + "0: 0Mhz\n" + "1: 0Mhz\n"; + +// OD_FCLK with a non-conforming line between two valid levels. +constexpr char kOdFclkGarbageLine[] = + "OD_FCLK:\n" + "0: 1000Mhz\n" + "not-a-level\n" + "1: 1100Mhz\n"; + +// "Old Format" layout: OD_FCLK is immediately followed by OD_VDDC_CURVE, whose +// "idx: freq volt" lines resemble level lines and must not fold into the range. +constexpr char kOdFclkThenCurve[] = + "OD_SCLK:\n" + "0: 500Mhz\n" + "1: 2100Mhz\n" + "OD_FCLK:\n" + "0: 1000Mhz\n" + "1: 1100Mhz\n" + "OD_VDDC_CURVE:\n" + "0: 500Mhz 700mV\n" + "1: 1354Mhz 860mV\n" + "2: 2000Mhz 1150mV\n"; + +// Two OD_FCLK sections: levels from every occurrence merge into one range. +constexpr char kOdFclkDuplicate[] = + "OD_FCLK:\n" + "0: 1000Mhz\n" + "1: 1100Mhz\n" + "OD_SCLK:\n" + "0: 500Mhz\n" + "OD_FCLK:\n" + "0: 900Mhz\n" + "1: 2000Mhz\n"; + +// A single-level (locked-clock) section: min == max. +constexpr char kOdFclkSingleLevel[] = + "OD_FCLK:\n" + "0: 1500Mhz\n"; + +TEST(GpuUnit, OdClkRangeReadsSclkSection) { + std::istringstream od(kOdNoFclk); + unsigned int max = 0; + unsigned int min = UINT_MAX; + EXPECT_TRUE(smi_amdgpu_parse_od_clk_range(od, AMDSMI_CLK_TYPE_GFX, &max, &min)); + EXPECT_EQ(max, 2100u); + EXPECT_EQ(min, 500u); +} + +TEST(GpuUnit, OdClkRangeReadsMclkSection) { + std::istringstream od(kOdNoFclk); + unsigned int max = 0; + unsigned int min = UINT_MAX; + EXPECT_TRUE(smi_amdgpu_parse_od_clk_range(od, AMDSMI_CLK_TYPE_MEM, &max, &min)); + EXPECT_EQ(max, 1200u); + EXPECT_EQ(min, 900u); +} + +// The regression: with no OD_FCLK section the parser reports "absent" (false) +// and leaves the out-params untouched, so the caller derives FCLK min/max from +// pp_dpm_fclk rather than reporting 0. +TEST(GpuUnit, OdClkRangeFallsBackWhenFclkSectionMissing) { + std::istringstream od(kOdNoFclk); + unsigned int max = 4242; + unsigned int min = 4242; + EXPECT_FALSE(smi_amdgpu_parse_od_clk_range(od, AMDSMI_CLK_TYPE_DF, &max, &min)); + EXPECT_EQ(max, 4242u); + EXPECT_EQ(min, 4242u); +} + +// With OD_FCLK present the range is read directly, no fallback. +TEST(GpuUnit, OdClkRangeReadsFclkSectionWhenPresent) { + std::istringstream od(kOdWithFclk); + unsigned int max = 0; + unsigned int min = UINT_MAX; + EXPECT_TRUE(smi_amdgpu_parse_od_clk_range(od, AMDSMI_CLK_TYPE_DF, &max, &min)); + EXPECT_EQ(max, 1100u); + EXPECT_EQ(min, 1000u); +} + +// A missing/empty pp_od_clk_voltage yields no section -> fall back. +TEST(GpuUnit, OdClkRangeFallsBackOnEmptyStream) { + std::istringstream od(""); + unsigned int max = 7; + unsigned int min = 7; + EXPECT_FALSE(smi_amdgpu_parse_od_clk_range(od, AMDSMI_CLK_TYPE_GFX, &max, &min)); +} + +// Domains that have no overdrive section (e.g. SOC) are never OD-backed. +TEST(GpuUnit, OdClkRangeRejectsNonOdDomain) { + std::istringstream od(kOdWithFclk); + unsigned int max = 7; + unsigned int min = 7; + EXPECT_FALSE(smi_amdgpu_parse_od_clk_range(od, AMDSMI_CLK_TYPE_SOC, &max, &min)); +} + +// The bare GFXCLK/MCLK/FCLK aliases are accepted just like the OD_* headers. +TEST(GpuUnit, OdClkRangeReadsAliasHeaders) { + unsigned int max = 0; + unsigned int min = UINT_MAX; + std::istringstream gfx(kOdAliasHeaders); + EXPECT_TRUE(smi_amdgpu_parse_od_clk_range(gfx, AMDSMI_CLK_TYPE_GFX, &max, &min)); + EXPECT_EQ(max, 2100u); + EXPECT_EQ(min, 500u); + + max = 0; + min = UINT_MAX; + std::istringstream mem(kOdAliasHeaders); + EXPECT_TRUE(smi_amdgpu_parse_od_clk_range(mem, AMDSMI_CLK_TYPE_MEM, &max, &min)); + EXPECT_EQ(max, 1200u); + EXPECT_EQ(min, 900u); + + max = 0; + min = UINT_MAX; + std::istringstream df(kOdAliasHeaders); + EXPECT_TRUE(smi_amdgpu_parse_od_clk_range(df, AMDSMI_CLK_TYPE_DF, &max, &min)); + EXPECT_EQ(max, 1100u); + EXPECT_EQ(min, 1000u); +} + +// Section present but every level parses to 0 -> reported absent so the caller +// still falls back to pp_dpm_*. +TEST(GpuUnit, OdClkRangeFallsBackWhenAllLevelsZero) { + std::istringstream od(kOdFclkAllZero); + unsigned int max = 555; + unsigned int min = 555; + EXPECT_FALSE(smi_amdgpu_parse_od_clk_range(od, AMDSMI_CLK_TYPE_DF, &max, &min)); + EXPECT_EQ(max, 555u); + EXPECT_EQ(min, 555u); +} + +// A non-conforming line inside the section is skipped, not treated as its end, +// so levels on both sides still contribute to the range. +TEST(GpuUnit, OdClkRangeSkipsMalformedLineWithinSection) { + std::istringstream od(kOdFclkGarbageLine); + unsigned int max = 0; + unsigned int min = UINT_MAX; + EXPECT_TRUE(smi_amdgpu_parse_od_clk_range(od, AMDSMI_CLK_TYPE_DF, &max, &min)); + EXPECT_EQ(max, 1100u); + EXPECT_EQ(min, 1000u); +} + +// An unrecognized section header (OD_VDDC_CURVE) ends FCLK parsing, so its curve +// lines are not mistaken for FCLK levels and do not inflate the max. +TEST(GpuUnit, OdClkRangeStopsAtUnrecognizedSectionHeader) { + std::istringstream od(kOdFclkThenCurve); + unsigned int max = 0; + unsigned int min = UINT_MAX; + EXPECT_TRUE(smi_amdgpu_parse_od_clk_range(od, AMDSMI_CLK_TYPE_DF, &max, &min)); + EXPECT_EQ(max, 1100u); + EXPECT_EQ(min, 1000u); +} + +// Repeated section headers merge: levels from every OD_FCLK occurrence feed one +// range. No live sysfs file repeats a header; this pins the contract. +TEST(GpuUnit, OdClkRangeMergesRepeatedSectionHeaders) { + std::istringstream od(kOdFclkDuplicate); + unsigned int max = 0; + unsigned int min = UINT_MAX; + EXPECT_TRUE(smi_amdgpu_parse_od_clk_range(od, AMDSMI_CLK_TYPE_DF, &max, &min)); + EXPECT_EQ(max, 2000u); + EXPECT_EQ(min, 900u); +} + +// A single-level (locked-clock) section reports min == max. +TEST(GpuUnit, OdClkRangeReadsSingleLevelSection) { + std::istringstream od(kOdFclkSingleLevel); + unsigned int max = 0; + unsigned int min = UINT_MAX; + EXPECT_TRUE(smi_amdgpu_parse_od_clk_range(od, AMDSMI_CLK_TYPE_DF, &max, &min)); + EXPECT_EQ(max, 1500u); + EXPECT_EQ(min, 1500u); +} + +} // namespace