From faadbc42eedcfa941a048311187986ce299812ca Mon Sep 17 00:00:00 2001 From: Jaroslaw Stelter Date: Fri, 2 Jun 2023 15:08:46 +0200 Subject: [PATCH] mm_drv: tlb: Fix mapped page in bank calculation The initial implementation was broken during improvements. There was incorrect assumption that all pages are unmapped at initials state. In reality at the beginning whole memory is powered on, so we should mark all pages as mapped. Later in initialization code unused pages are unmapped and if after this some banks become empty (all pages unmapped), the power is switched off. Signed-off-by: Jaroslaw Stelter --- drivers/mm/mm_drv_bank.c | 14 ++++++-------- drivers/mm/mm_drv_intel_adsp_mtl_tlb.c | 4 ++-- include/zephyr/drivers/mm/mm_drv_bank.h | 6 +++++- tests/drivers/mm/sys_mm_drv_bank/src/main.c | 7 ++++++- 4 files changed, 19 insertions(+), 12 deletions(-) diff --git a/drivers/mm/mm_drv_bank.c b/drivers/mm/mm_drv_bank.c index 14b4b40613a..af3ced8171d 100644 --- a/drivers/mm/mm_drv_bank.c +++ b/drivers/mm/mm_drv_bank.c @@ -15,24 +15,22 @@ */ #include - +#include #include #include void sys_mm_drv_bank_init(struct mem_drv_bank *bank, uint32_t bank_pages) { - bank->unmapped_pages = bank_pages; - bank->mapped_pages = 0; - bank->max_mapped_pages = 0; + bank->unmapped_pages = 0; + bank->mapped_pages = bank_pages; + bank->max_mapped_pages = bank_pages; } uint32_t sys_mm_drv_bank_page_mapped(struct mem_drv_bank *bank) { bank->unmapped_pages--; bank->mapped_pages++; - if (bank->mapped_pages > bank->max_mapped_pages) { - bank->max_mapped_pages = bank->mapped_pages; - } + __ASSERT_NO_MSG(bank->mapped_pages <= bank->max_mapped_pages); return bank->mapped_pages; } @@ -40,7 +38,7 @@ uint32_t sys_mm_drv_bank_page_unmapped(struct mem_drv_bank *bank) { bank->unmapped_pages++; bank->mapped_pages--; - + __ASSERT_NO_MSG(bank->unmapped_pages <= bank->max_mapped_pages); return bank->unmapped_pages; } diff --git a/drivers/mm/mm_drv_intel_adsp_mtl_tlb.c b/drivers/mm/mm_drv_intel_adsp_mtl_tlb.c index 504b51e6e5a..c75c3d9f0fd 100644 --- a/drivers/mm/mm_drv_intel_adsp_mtl_tlb.c +++ b/drivers/mm/mm_drv_intel_adsp_mtl_tlb.c @@ -378,7 +378,7 @@ int sys_mm_drv_unmap_page(void *virt) sys_mm_drv_report_page_usage(); #endif - if (sys_mm_drv_bank_page_unmapped(&hpsram_bank[bank_idx]) == 0) { + if (sys_mm_drv_bank_page_unmapped(&hpsram_bank[bank_idx]) == SRAM_BANK_PAGE_NUM) { sys_mm_drv_hpsram_pwr(bank_idx, false, false); } } @@ -649,7 +649,7 @@ static int sys_mm_drv_mm_init(const struct device *dev) */ for (int i = 0; i < L2_SRAM_BANK_NUM; i++) { sys_mm_drv_bank_init(&hpsram_bank[i], - SRAM_BANK_SIZE / CONFIG_MM_DRV_PAGE_SIZE); + SRAM_BANK_PAGE_NUM); } #ifdef CONFIG_SOC_INTEL_COMM_WIDGET used_pages = L2_SRAM_BANK_NUM * SRAM_BANK_SIZE / CONFIG_MM_DRV_PAGE_SIZE; diff --git a/include/zephyr/drivers/mm/mm_drv_bank.h b/include/zephyr/drivers/mm/mm_drv_bank.h index 1b53e9c2131..9e1a51ebcbf 100644 --- a/include/zephyr/drivers/mm/mm_drv_bank.h +++ b/include/zephyr/drivers/mm/mm_drv_bank.h @@ -21,6 +21,8 @@ #include #include +#define SRAM_BANK_PAGE_NUM (SRAM_BANK_SIZE / CONFIG_MM_DRV_PAGE_SIZE) + struct mem_drv_bank { uint32_t unmapped_pages; uint32_t mapped_pages; @@ -32,7 +34,9 @@ struct mem_drv_bank { * * The driver may wish to track various information about the memory banks * that it uses. This routine will initialize a generic structure containing - * that information. + * that information. Since at the power on all memory banks are switched on + * it will start with all pages mapped. In next phase of driver initialization + * unused pages will be unmapped. * * @param bank Pointer to the memory bank structure used for tracking * @param bank_pages Number of pages in the memory bank diff --git a/tests/drivers/mm/sys_mm_drv_bank/src/main.c b/tests/drivers/mm/sys_mm_drv_bank/src/main.c index 786c24f662d..9c834c5efa5 100644 --- a/tests/drivers/mm/sys_mm_drv_bank/src/main.c +++ b/tests/drivers/mm/sys_mm_drv_bank/src/main.c @@ -34,10 +34,15 @@ ZTEST(sys_mm_bank_api, test_sys_mm_drv_bank) struct sys_memory_stats stats; struct sys_memory_stats expected; uint32_t count; + uint32_t index; /* Verify that the initialization routine works as expected. */ - + /* It set mapped state for all pages */ sys_mm_drv_bank_init(&bank_data, BANK_PAGES); + /* Now unmap all pages */ + for (index = 0; index < BANK_PAGES; index++) { + sys_mm_drv_bank_page_unmapped(&bank_data); + } expected.free_bytes = EXPECTED(BANK_PAGES); expected.allocated_bytes = EXPECTED(0);