From 787831388d81136e2875b3a9ebb77e92b5d15acd Mon Sep 17 00:00:00 2001 From: Vedashree Vidwans Date: Mon, 16 May 2022 17:14:35 -0700 Subject: [PATCH] tegra: hwpm: fix bugs on ToT - With latest changes, disable PMA triggers contained a bug related to timeout checks. Reading register value in while loop was removed accidentally. Replace HWPM_TIMEOUT macro with do-while loop to check condition. - Correct func enum used to release router. - Correct MSS MCF IP and instance address stride. Jira THWPM-41 Change-Id: I28b4b223c33992599332de39471fd80215e0c98b Signed-off-by: Vedashree Vidwans Reviewed-on: https://git-master.nvidia.com/r/c/linux-nvidia/+/2713365 Reviewed-by: Vasuki Shankar Reviewed-by: svc-mobile-coverity Reviewed-by: svc-mobile-cert Reviewed-by: Seema Khowala Reviewed-by: svc_kernel_abi GVS: Gerrit_Virtual_Submit --- common/tegra_hwpm_mem_buf_utils.c | 30 ++++--- common/tegra_hwpm_resource_utils.c | 2 +- .../ip/mss_channel/t234_hwpm_ip_mss_channel.c | 2 + .../t234_hwpm_ip_mss_iso_niso_hubs.c | 2 + hal/t234/ip/mss_mcf/t234_hwpm_ip_mss_mcf.c | 6 +- hal/t234/ip/pcie/t234_hwpm_ip_pcie.c | 1 + hal/t234/ip/vi/t234_hwpm_ip_vi.c | 2 + hal/t234/t234_hwpm_aperture_utils.c | 81 ++++++++++++------- include/tegra_hwpm.h | 40 +++++---- 9 files changed, 98 insertions(+), 68 deletions(-) diff --git a/common/tegra_hwpm_mem_buf_utils.c b/common/tegra_hwpm_mem_buf_utils.c index 90f5d45..4e7b2cc 100644 --- a/common/tegra_hwpm_mem_buf_utils.c +++ b/common/tegra_hwpm_mem_buf_utils.c @@ -209,21 +209,27 @@ int tegra_hwpm_clear_mem_pipeline(struct tegra_soc_hwpm *hwpm) /* Stream MEM_BYTES to clear pipeline */ if (hwpm->mem_bytes_kernel) { - bool timeout = false; + s32 timeout_msecs = 1000; + u32 sleep_msecs = 100; u32 *mem_bytes_kernel_u32 = (u32 *)(hwpm->mem_bytes_kernel); - ret = hwpm->active_chip->stream_mem_bytes(hwpm); - if (ret != 0) { + do { + ret = hwpm->active_chip->stream_mem_bytes(hwpm); + if (ret != 0) { + tegra_hwpm_err(hwpm, + "Trigger mem_bytes streaming failed"); + goto fail; + } + msleep(sleep_msecs); + timeout_msecs -= sleep_msecs; + } while ((*mem_bytes_kernel_u32 == + TEGRA_SOC_HWPM_MEM_BYTES_INVALID) && + (timeout_msecs > 0)); + + if (timeout_msecs <= 0) { tegra_hwpm_err(hwpm, - "Failed to trigger mem_bytes streaming"); - goto fail; - } - timeout = HWPM_TIMEOUT(*mem_bytes_kernel_u32 != - TEGRA_SOC_HWPM_MEM_BYTES_INVALID, - "MEM_BYTES streaming"); - if (timeout) { - ret = -EIO; - goto fail; + "Timeout expired for MEM_BYTES streaming"); + return -ETIMEDOUT; } } diff --git a/common/tegra_hwpm_resource_utils.c b/common/tegra_hwpm_resource_utils.c index 57c2023..dc581e0 100644 --- a/common/tegra_hwpm_resource_utils.c +++ b/common/tegra_hwpm_resource_utils.c @@ -46,7 +46,7 @@ int tegra_hwpm_release_rtr(struct tegra_soc_hwpm *hwpm) tegra_hwpm_fn(hwpm, " "); err = tegra_hwpm_func_single_ip(hwpm, NULL, - TEGRA_HWPM_RELEASE_RESOURCES, + TEGRA_HWPM_RELEASE_ROUTER, active_chip->get_rtr_int_idx(hwpm)); if (err != 0) { tegra_hwpm_err(hwpm, "failed to release IP %d", diff --git a/hal/t234/ip/mss_channel/t234_hwpm_ip_mss_channel.c b/hal/t234/ip/mss_channel/t234_hwpm_ip_mss_channel.c index d8fcacb..eeadaf1 100644 --- a/hal/t234/ip/mss_channel/t234_hwpm_ip_mss_channel.c +++ b/hal/t234/ip/mss_channel/t234_hwpm_ip_mss_channel.c @@ -538,6 +538,7 @@ struct hwpm_ip_inst t234_mss_channel_inst_static_array[ T234_HWPM_IP_MSS_CHANNEL_NUM_PERFMUX_PER_INST, .element_static_array = t234_mss_channel_inst0_perfmux_element_static_array, + /* NOTE: range should be in ascending order */ .range_start = addr_map_mc8_base_r(), .range_end = addr_map_mc3_limit_r(), .element_stride = addr_map_mc0_limit_r() - @@ -600,6 +601,7 @@ struct hwpm_ip t234_hwpm_ip_mss_channel = { * TEGRA_HWPM_APERTURE_TYPE_PERFMUX */ { + /* NOTE: range should be in ascending order */ .range_start = addr_map_mc8_base_r(), .range_end = addr_map_mc3_limit_r(), .inst_stride = addr_map_mc3_limit_r() - diff --git a/hal/t234/ip/mss_iso_niso_hubs/t234_hwpm_ip_mss_iso_niso_hubs.c b/hal/t234/ip/mss_iso_niso_hubs/t234_hwpm_ip_mss_iso_niso_hubs.c index a7ea955..7c7b7cc 100644 --- a/hal/t234/ip/mss_iso_niso_hubs/t234_hwpm_ip_mss_iso_niso_hubs.c +++ b/hal/t234/ip/mss_iso_niso_hubs/t234_hwpm_ip_mss_iso_niso_hubs.c @@ -232,6 +232,7 @@ struct hwpm_ip_inst t234_mss_iso_niso_hub_inst_static_array[ T234_HWPM_IP_MSS_ISO_NISO_HUBS_NUM_PERFMUX_PER_INST, .element_static_array = t234_mss_iso_niso_hub_inst0_perfmux_element_static_array, + /* NOTE: range should be in ascending order */ .range_start = addr_map_mc8_base_r(), .range_end = addr_map_mc3_limit_r(), .element_stride = addr_map_mc8_limit_r() - @@ -294,6 +295,7 @@ struct hwpm_ip t234_hwpm_ip_mss_iso_niso_hubs = { * TEGRA_HWPM_APERTURE_TYPE_PERFMUX */ { + /* NOTE: range should be in ascending order */ .range_start = addr_map_mc8_base_r(), .range_end = addr_map_mc3_limit_r(), .inst_stride = addr_map_mc3_limit_r() - diff --git a/hal/t234/ip/mss_mcf/t234_hwpm_ip_mss_mcf.c b/hal/t234/ip/mss_mcf/t234_hwpm_ip_mss_mcf.c index ad30363..6276251 100644 --- a/hal/t234/ip/mss_mcf/t234_hwpm_ip_mss_mcf.c +++ b/hal/t234/ip/mss_mcf/t234_hwpm_ip_mss_mcf.c @@ -224,6 +224,7 @@ struct hwpm_ip_inst t234_mss_mcf_inst_static_array[ T234_HWPM_IP_MSS_MCF_NUM_PERFMUX_PER_INST, .element_static_array = t234_mss_mcf_inst0_perfmux_element_static_array, + /* NOTE: range should be in ascending order */ .range_start = addr_map_mc4_base_r(), .range_end = addr_map_mc3_limit_r(), .element_stride = addr_map_mc4_limit_r() - @@ -286,9 +287,10 @@ struct hwpm_ip t234_hwpm_ip_mss_mcf = { * TEGRA_HWPM_APERTURE_TYPE_PERFMUX */ { + /* NOTE: range should be in ascending order */ .range_start = addr_map_mc4_base_r(), .range_end = addr_map_mc3_limit_r(), - .inst_stride = addr_map_mc4_limit_r() - + .inst_stride = addr_map_mc3_limit_r() - addr_map_mc4_base_r() + 1ULL, .inst_slots = 0U, .inst_arr = NULL, @@ -312,7 +314,7 @@ struct hwpm_ip t234_hwpm_ip_mss_mcf = { { .range_start = addr_map_rpg_pm_mcf0_base_r(), .range_end = addr_map_rpg_pm_mcf2_limit_r(), - .inst_stride = addr_map_rpg_pm_mcf0_limit_r() - + .inst_stride = addr_map_rpg_pm_mcf2_limit_r() - addr_map_rpg_pm_mcf0_base_r() + 1ULL, .inst_slots = 0U, .inst_arr = NULL, diff --git a/hal/t234/ip/pcie/t234_hwpm_ip_pcie.c b/hal/t234/ip/pcie/t234_hwpm_ip_pcie.c index dd981af..8e28775 100644 --- a/hal/t234/ip/pcie/t234_hwpm_ip_pcie.c +++ b/hal/t234/ip/pcie/t234_hwpm_ip_pcie.c @@ -1100,6 +1100,7 @@ struct hwpm_ip t234_hwpm_ip_pcie = { * TEGRA_HWPM_APERTURE_TYPE_PERFMUX */ { + /* NOTE: range should be in ascending order */ .range_start = addr_map_pcie_c8_ctl_base_r(), .range_end = addr_map_pcie_c7_ctl_limit_r(), .inst_stride = addr_map_pcie_c8_ctl_limit_r() - diff --git a/hal/t234/ip/vi/t234_hwpm_ip_vi.c b/hal/t234/ip/vi/t234_hwpm_ip_vi.c index 1fb5ef8..7c9d29c 100644 --- a/hal/t234/ip/vi/t234_hwpm_ip_vi.c +++ b/hal/t234/ip/vi/t234_hwpm_ip_vi.c @@ -108,6 +108,7 @@ struct hwpm_ip_inst t234_vi_inst_static_array[ T234_HWPM_IP_VI_NUM_PERFMUX_PER_INST, .element_static_array = t234_vi_inst0_perfmux_element_static_array, + /* NOTE: range should be in ascending order */ .range_start = addr_map_vi_thi_base_r(), .range_end = addr_map_vi_thi_limit_r(), .element_stride = addr_map_vi_thi_limit_r() - @@ -227,6 +228,7 @@ struct hwpm_ip t234_hwpm_ip_vi = { * TEGRA_HWPM_APERTURE_TYPE_PERFMUX */ { + /* NOTE: range should be in ascending order */ .range_start = addr_map_vi2_thi_base_r(), .range_end = addr_map_vi_thi_limit_r(), .inst_stride = addr_map_vi2_thi_limit_r() - diff --git a/hal/t234/t234_hwpm_aperture_utils.c b/hal/t234/t234_hwpm_aperture_utils.c index a540010..b5141d8 100644 --- a/hal/t234/t234_hwpm_aperture_utils.c +++ b/hal/t234/t234_hwpm_aperture_utils.c @@ -27,10 +27,11 @@ int t234_hwpm_disable_triggers(struct tegra_soc_hwpm *hwpm) { int err = 0; - bool timeout = false; u32 reg_val = 0U; u32 field_mask = 0U; u32 field_val = 0U; + s32 timeout_msecs = 1000; + u32 sleep_msecs = 100; struct tegra_soc_hwpm_chip *active_chip = hwpm->active_chip; struct hwpm_ip *chip_ip = active_chip->chip_ips[ active_chip->get_rtr_int_idx(hwpm)]; @@ -89,46 +90,64 @@ int t234_hwpm_disable_triggers(struct tegra_soc_hwpm *hwpm) } /* Wait for PERFMONs, ROUTER, and PMA to idle */ - err = tegra_hwpm_readl(hwpm, rtr_perfmux, - pmmsys_sys0router_perfmonstatus_r(), ®_val); - if (err != 0) { - tegra_hwpm_err(hwpm, "hwpm read failed"); - return err; - } - timeout = HWPM_TIMEOUT( - pmmsys_sys0router_perfmonstatus_merged_v(reg_val) == 0U, - "NV_PERF_PMMSYS_SYS0ROUTER_PERFMONSTATUS_MERGED_EMPTY"); - if (timeout) { + do { + err = tegra_hwpm_readl(hwpm, rtr_perfmux, + pmmsys_sys0router_perfmonstatus_r(), ®_val); + if (err != 0) { + tegra_hwpm_err(hwpm, "hwpm read failed"); + return err; + } + msleep(sleep_msecs); + timeout_msecs -= sleep_msecs; + } while ((pmmsys_sys0router_perfmonstatus_merged_v(reg_val) != 0U) && + (timeout_msecs > 0)); + + if (timeout_msecs <= 0) { + tegra_hwpm_err(hwpm, "Timeout expired for " + "NV_PERF_PMMSYS_SYS0ROUTER_PERFMONSTATUS_MERGED_EMPTY"); return -ETIMEDOUT; } - err = tegra_hwpm_readl(hwpm, rtr_perfmux, - pmmsys_sys0router_enginestatus_r(), ®_val); - if (err != 0) { - tegra_hwpm_err(hwpm, "hwpm read failed"); - return err; - } - timeout = HWPM_TIMEOUT( - pmmsys_sys0router_enginestatus_status_v(reg_val) == - pmmsys_sys0router_enginestatus_status_empty_v(), - "NV_PERF_PMMSYS_SYS0ROUTER_ENGINESTATUS_STATUS_EMPTY"); - if (timeout) { + timeout_msecs = 1000; + + do { + err = tegra_hwpm_readl(hwpm, rtr_perfmux, + pmmsys_sys0router_enginestatus_r(), ®_val); + if (err != 0) { + tegra_hwpm_err(hwpm, "hwpm read failed"); + return err; + } + msleep(sleep_msecs); + timeout_msecs -= sleep_msecs; + } while ((pmmsys_sys0router_enginestatus_status_v(reg_val) != + pmmsys_sys0router_enginestatus_status_empty_v()) && + (timeout_msecs > 0)); + if (timeout_msecs <= 0) { + tegra_hwpm_err(hwpm, "Timeout expired for " + "NV_PERF_PMMSYS_SYS0ROUTER_ENGINESTATUS_STATUS_EMPTY"); return -ETIMEDOUT; } + timeout_msecs = 1000; + field_mask = pmasys_enginestatus_status_m() | pmasys_enginestatus_rbufempty_m(); field_val = pmasys_enginestatus_status_empty_f() | pmasys_enginestatus_rbufempty_empty_f(); - err = tegra_hwpm_readl(hwpm, pma_perfmux, - pmasys_enginestatus_r(), ®_val); - if (err != 0) { - tegra_hwpm_err(hwpm, "hwpm read failed"); - return err; - } - timeout = HWPM_TIMEOUT((reg_val & field_mask) == field_val, - "NV_PERF_PMASYS_ENGINESTATUS"); - if (timeout) { + do { + err = tegra_hwpm_readl(hwpm, pma_perfmux, + pmasys_enginestatus_r(), ®_val); + if (err != 0) { + tegra_hwpm_err(hwpm, "hwpm read failed"); + return err; + } + msleep(sleep_msecs); + timeout_msecs -= sleep_msecs; + } while (((reg_val & field_mask) != field_val) && (timeout_msecs > 0)); + + if (timeout_msecs <= 0) { + tegra_hwpm_err(hwpm, "Timeout expired for " + "NV_PERF_PMASYS_ENGINESTATUS"); return -ETIMEDOUT; } diff --git a/include/tegra_hwpm.h b/include/tegra_hwpm.h index bced552..02e4a28 100644 --- a/include/tegra_hwpm.h +++ b/include/tegra_hwpm.h @@ -30,24 +30,6 @@ #define TEGRA_SOC_HWPM_IP_INACTIVE ~(0U) -/* FIXME: Default timeout is 1 sec. Is this sufficient for pre-si? */ -#define HWPM_TIMEOUT(timeout_check, expiry_msg) ({ \ - bool timeout_expired = false; \ - s32 timeout_msecs = 1000; \ - u32 sleep_msecs = 100; \ - while (!(timeout_check)) { \ - msleep(sleep_msecs); \ - timeout_msecs -= sleep_msecs; \ - if (timeout_msecs <= 0) { \ - tegra_hwpm_err(NULL, "Timeout expired for %s!", \ - expiry_msg); \ - timeout_expired = true; \ - break; \ - } \ - } \ - timeout_expired; \ -}) - struct hwpm_ip_register_list { struct tegra_soc_hwpm_ip_ops ip_ops; struct hwpm_ip_register_list *next; @@ -190,7 +172,7 @@ struct hwpm_ip_aperture { }; struct hwpm_ip_element_info { - /* Number of apertures per instance */ + /* Number of elements per instance */ u32 num_element_per_inst; /* @@ -199,7 +181,14 @@ struct hwpm_ip_element_info { */ struct hwpm_ip_aperture *element_static_array; - /* Instance address range corresponding to aperture */ + /* + * Ascending instance address range corresponding to elements + * NOTE: It is possible that address range of elements in the instance + * are not in same sequential order as their indexes. + * For example, 0th element of an instance can have start address higher + * than element 1. In this case, range_start and range_end should be + * initialized in increasing order. + */ u64 range_start; u64 range_end; @@ -213,7 +202,7 @@ struct hwpm_ip_element_info { u32 element_slots; /* - * Dynamic elements array corresponding to this aperture + * Dynamic elements array corresponding to this element * Array size: element_slots pointers */ struct hwpm_ip_aperture **element_arr; @@ -246,7 +235,14 @@ struct hwpm_ip_inst { }; struct hwpm_ip_inst_per_aperture_info { - /* IP address range corresponding to aperture */ + /* + * Ascending IP address range corresponding to instances + * NOTE: It is possible that address range of IP instances + * are not in same sequential order as their indexes. + * For example, 0th instance of an IP can have start address higher + * than instance 1. In this case, range_start and range_end should be + * initialized in increasing order. + */ u64 range_start; u64 range_end;