From d612f067a9658b051d863905e1a472c9dbed0871 Mon Sep 17 00:00:00 2001 From: Jakub Dabek Date: Thu, 8 Aug 2019 15:14:56 +0200 Subject: [PATCH 01/11] memory: Add alignment option for allocation Add alignment option for memory requests. Fix alloc definitions for UT. Function definitions were using defines in their body which blocked usage of const values in said defines. Signed-off-by: Jakub Dabek --- src/arch/host/include/arch/lib/cache.h | 2 + src/include/sof/lib/alloc.h | 46 +++++- src/include/sof/lib/cache.h | 7 - src/lib/alloc.c | 138 ++++++++++++++---- .../baytrail/include/platform/lib/memory.h | 2 + .../haswell/include/platform/lib/memory.h | 6 +- .../imx8/include/platform/lib/memory.h | 3 + .../intel/cavs/include/cavs/lib/memory.h | 3 + test/cmocka/src/audio/buffer/mock.c | 8 +- test/cmocka/src/audio/component/mock.c | 2 +- test/cmocka/src/audio/kpb/kpb_mock.c | 8 +- test/cmocka/src/audio/mixer/mock.c | 8 +- 12 files changed, 180 insertions(+), 53 deletions(-) diff --git a/src/arch/host/include/arch/lib/cache.h b/src/arch/host/include/arch/lib/cache.h index a849c1c77fc8..8291675d79f3 100644 --- a/src/arch/host/include/arch/lib/cache.h +++ b/src/arch/host/include/arch/lib/cache.h @@ -12,6 +12,8 @@ #include +#define PLATFORM_DCACHE_ALIGN sizeof(void *) + static inline void dcache_writeback_region(void *addr, size_t size) {} static inline void dcache_invalidate_region(void *addr, size_t size) {} static inline void icache_invalidate_region(void *addr, size_t size) {} diff --git a/src/include/sof/lib/alloc.h b/src/include/sof/lib/alloc.h index 00a8cae11baf..dee50019fe82 100644 --- a/src/include/sof/lib/alloc.h +++ b/src/include/sof/lib/alloc.h @@ -62,6 +62,7 @@ struct mm_info { struct block_hdr { uint16_t size; /* size in blocks for continuous allocation */ uint16_t used; /* usage flags for page */ + void *unaligned_ptr; /* align ptr */ } __packed; struct block_map { @@ -105,9 +106,10 @@ struct mm { /* heap allocation and free */ void *_malloc(int zone, uint32_t caps, size_t bytes); void *_zalloc(int zone, uint32_t caps, size_t bytes); -void *_balloc(int zone, uint32_t caps, size_t bytes); +void *_balloc(int zone, uint32_t caps, size_t bytes, uint32_t alignment); void *_realloc(void *ptr, int zone, uint32_t caps, size_t bytes); -void *_brealloc(void *ptr, int zone, uint32_t caps, size_t bytes); +void *_brealloc(void *ptr, int zone, uint32_t caps, size_t bytes, + uint32_t alignment); void rfree(void *ptr); #if CONFIG_DEBUG_HEAP @@ -143,7 +145,7 @@ void rfree(void *ptr); #define rballoc(zone, caps, bytes) \ ({void *_ptr; \ do { \ - _ptr = _balloc(zone, caps, bytes); \ + _ptr = _balloc(zone, caps, bytes, PLATFORM_DCACHE_ALIGN);\ if (!_ptr) { \ trace_error(TRACE_CLASS_MEM, \ "failed to alloc 0x%x bytes caps 0x%x", \ @@ -169,7 +171,34 @@ void rfree(void *ptr); #define rbrealloc(ptr, zone, caps, bytes) \ ({void *_ptr; \ do { \ - _ptr = _brealloc(ptr, zone, caps, bytes); \ + _ptr = _brealloc(ptr, zone, caps, bytes,\ + PLATFORM_DCACHE_ALIGN);\ + if (!_ptr) { \ + trace_error(TRACE_CLASS_MEM, \ + "failed to alloc 0x%x bytes caps 0x%x", \ + bytes, caps); \ + alloc_trace_buffer_heap(zone, caps, bytes); \ + } \ + } while (0); \ + _ptr; }) + +#define rbrealloc_a(ptr, zone, caps, bytes, alignment) \ + ({void *_ptr; \ + do { \ + _ptr = _brealloc(ptr, zone, caps, bytes, alignment); \ + if (!_ptr) { \ + trace_error(TRACE_CLASS_MEM, \ + "failed to alloc 0x%x bytes caps 0x%x", \ + bytes, caps); \ + alloc_trace_buffer_heap(zone, caps, bytes); \ + } \ + } while (0); \ + _ptr; }) + +#define rballoc(zone, caps, bytes, alignment) \ + ({void *_ptr; \ + do { \ + _ptr = _balloc(zone, caps, bytes, alignment); \ if (!_ptr) { \ trace_error(TRACE_CLASS_MEM, \ "failed to alloc 0x%x bytes caps 0x%x", \ @@ -186,11 +215,16 @@ void alloc_trace_buffer_heap(int zone, uint32_t caps, size_t bytes); #define rmalloc(zone, caps, bytes) _malloc(zone, caps, bytes) #define rzalloc(zone, caps, bytes) _zalloc(zone, caps, bytes) -#define rballoc(zone, caps, bytes) _balloc(zone, caps, bytes) +#define rballoc(zone, caps, bytes) \ + _balloc(zone, caps, bytes, PLATFORM_DCACHE_ALIGN) +#define rballoc_align(zone, caps, bytes, alignment) \ + _balloc(zone, caps, bytes, alignment) #define rrealloc(ptr, zone, caps, bytes) \ _realloc(ptr, zone, caps, bytes) #define rbrealloc(ptr, zone, caps, bytes) \ - _brealloc(ptr, zone, caps, bytes) + _brealloc(ptr, zone, caps, bytes, PLATFORM_DCACHE_ALIGN) +#define rbrealloc_align(ptr, zone, caps, bytes, alignment) \ + _brealloc(ptr, zone, caps, bytes, alignment) #endif diff --git a/src/include/sof/lib/cache.h b/src/include/sof/lib/cache.h index 6144abeb7a7d..1418845fc096 100644 --- a/src/include/sof/lib/cache.h +++ b/src/include/sof/lib/cache.h @@ -22,11 +22,4 @@ /* invalidate data */ #define CACHE_INVALIDATE 1 -/* data cache line alignment */ -#if DCACHE_LINE_SIZE > 0 -#define PLATFORM_DCACHE_ALIGN DCACHE_LINE_SIZE -#else -#define PLATFORM_DCACHE_ALIGN sizeof(void *) -#endif - #endif /* __SOF_LIB_CACHE_H__ */ diff --git a/src/lib/alloc.c b/src/lib/alloc.c index 2a9d7b3db720..84d785e78077 100644 --- a/src/lib/alloc.c +++ b/src/lib/alloc.c @@ -201,9 +201,30 @@ static void *rmalloc_sys(int zone, int caps, int core, size_t bytes) return ptr; } +/* At this point the pointer we have should be unaligned + * (it was checked level higher) and be power of 2 + */ +static void *align_ptr(struct mm_heap *heap, uint32_t alignment, + void *ptr, struct block_hdr *hdr) +{ + int mod_align = 0; + + /* Save unaligned ptr to block hdr */ + hdr->unaligned_ptr = ptr; + + /* If ptr is not already aligned we calculate alignment shift */ + if (alignment && (uintptr_t)ptr % alignment) + mod_align = alignment - ((uintptr_t)ptr % alignment); + + /* Calculate aligned pointer */ + ptr = ptr + mod_align; + + return ptr; +} + /* allocate single block */ static void *alloc_block(struct mm_heap *heap, int level, - uint32_t caps) + uint32_t caps, uint32_t alignment) { struct block_map *map = &heap->map[level]; struct block_hdr *hdr; @@ -216,8 +237,11 @@ static void *alloc_block(struct mm_heap *heap, int level, map->free_count--; ptr = (void *)(map->base + map->first_free * map->block_size); + ptr = align_ptr(heap, alignment, ptr, hdr); + hdr->size = 1; hdr->used = 1; + heap->info.used += map->block_size; heap->info.free -= map->block_size; @@ -239,11 +263,11 @@ static void *alloc_block(struct mm_heap *heap, int level, /* allocates continuous blocks */ static void *alloc_cont_blocks(struct mm_heap *heap, int level, - uint32_t caps, size_t bytes) + uint32_t caps, size_t bytes, uint32_t alignment) { struct block_map *map = &heap->map[level]; struct block_hdr *hdr; - void *ptr; + void *ptr, *unaligned_ptr; unsigned int start = map->first_free; unsigned int current; unsigned int count = bytes / map->block_size; @@ -277,8 +301,13 @@ static void *alloc_cont_blocks(struct mm_heap *heap, int level, /* we found enough space, let's allocate it */ map->free_count -= count; ptr = (void *)(map->base + start * map->block_size); + unaligned_ptr = ptr; + hdr = &map->block[start]; hdr->size = count; + + ptr = align_ptr(heap, alignment, ptr, hdr); + heap->info.used += count * map->block_size; heap->info.free -= count * map->block_size; /* update first_free if needed */ @@ -296,6 +325,7 @@ static void *alloc_cont_blocks(struct mm_heap *heap, int level, for (current = start; current < start + count; current++) { hdr = &map->block[current]; hdr->used = 1; + hdr->unaligned_ptr = unaligned_ptr; } writeback_block_map(map); @@ -356,27 +386,43 @@ static struct mm_heap *get_heap_from_caps(struct mm_heap *heap, int count, } static void *get_ptr_from_heap(struct mm_heap *heap, int zone, uint32_t caps, - size_t bytes) + size_t bytes, uint32_t alignment) { struct block_map *map; - int i; + int i, temp_bytes = bytes; void *ptr = NULL; + /* Only allow alignment as a power of 2 */ + if ((alignment & (alignment - 1)) != 0) + panic(SOF_IPC_PANIC_MEM); + for (i = 0; i < heap->blocks; i++) { map = &heap->map[i]; dcache_invalidate_region(map, sizeof(*map)); + /* size of requested buffer is adjusted for alignment purposes + * we check if first free block is already aligned if not + * we need to allocate bigger size for alignment + */ + if ((map->base + (map->block_size * map->first_free)) + % alignment) + temp_bytes += alignment; + /* is block big enough */ - if (map->block_size < bytes) + if (map->block_size < temp_bytes) { + temp_bytes = bytes; continue; + } /* does block have free space */ - if (map->free_count == 0) + if (map->free_count == 0) { + temp_bytes = bytes; continue; + } /* free block space exists */ - ptr = alloc_block(heap, i, caps); + ptr = alloc_block(heap, i, caps, alignment); break; } @@ -428,14 +474,26 @@ static void free_block(void *ptr) /* calculate block header */ block = ((uint32_t)ptr - block_map->base) / block_map->block_size; - /* report an error if ptr is not aligned to block */ - if (block_map->base + block_map->block_size * block != (uint32_t)ptr) - panic(SOF_IPC_PANIC_MEM); - invalidate_blocks(block_map); hdr = &block_map->block[block]; + /* bring back original unaligned pointer position + * and calculate correct hdr for free operation (it could + * be from different block since we got user pointer here + * or null if header was not set) + */ + if (hdr->unaligned_ptr != ptr && hdr->unaligned_ptr) { + ptr = hdr->unaligned_ptr; + block = ((uint32_t)ptr - block_map->base) + / block_map->block_size; + hdr = &block_map->block[block]; + } + + /* report an error if ptr is not aligned to block */ + if (block_map->base + block_map->block_size * block != (uint32_t)ptr) + panic(SOF_IPC_PANIC_MEM); + /* free block header and continuous blocks */ used_blocks = block + hdr->size; @@ -443,6 +501,7 @@ static void free_block(void *ptr) hdr = &block_map->block[i]; hdr->size = 0; hdr->used = 0; + hdr->unaligned_ptr = NULL; block_map->free_count++; heap->info.used -= block_map->block_size; heap->info.free += block_map->block_size; @@ -547,7 +606,8 @@ static void *rmalloc_sys_runtime(int zone, int caps, int core, size_t bytes) if ((cpu_heap->caps & caps) != caps) panic(SOF_IPC_PANIC_MEM); - ptr = get_ptr_from_heap(cpu_heap, zone, caps, bytes); + ptr = get_ptr_from_heap(cpu_heap, zone, caps, bytes, + PLATFORM_DCACHE_ALIGN); /* other core should have the latest value */ if (core != cpu_get_id()) @@ -577,8 +637,8 @@ static void *rmalloc_runtime(int zone, uint32_t caps, size_t bytes) return NULL; } } - - return get_ptr_from_heap(heap, zone, caps, bytes); + return get_ptr_from_heap(heap, zone, caps, bytes, + PLATFORM_DCACHE_ALIGN); } static void *_malloc_unlocked(int zone, uint32_t caps, size_t bytes) @@ -657,26 +717,45 @@ void *rzalloc_core_sys(int core, size_t bytes) /* allocates continuous buffers - not for direct use, clients use rballoc() */ static void *alloc_heap_buffer(struct mm_heap *heap, int zone, uint32_t caps, - size_t bytes) + size_t bytes, uint32_t alignment) { struct block_map *map; - int i; + int i, temp_bytes = bytes; void *ptr = NULL; + /* Only allow alignment as a power of 2 */ + if ((alignment & (alignment - 1)) != 0) + panic(SOF_IPC_PANIC_MEM); + /* will request fit in single block */ for (i = 0; i < heap->blocks; i++) { map = &heap->map[i]; dcache_invalidate_region(map, sizeof(*map)); + /* size of requested buffer is adjusted for alignment purposes + * we check if first free block is already aligned if not + * we need to allocate bigger size for alignment + */ + if ((map->base + (map->block_size * map->first_free)) + % alignment) + temp_bytes += alignment; + /* Check if blocks are big enough and at least one is free */ - if (map->block_size >= bytes && map->free_count) { + if (map->block_size >= temp_bytes && map->free_count) { /* found: grab a block */ - ptr = alloc_block(heap, i, caps); + + ptr = alloc_block(heap, i, caps, alignment); break; } + temp_bytes = bytes; } + /* size of requested buffer is adjusted for alignment purposes + * since we span more blocks we have to assume worst case scenario + */ + bytes += alignment; + /* request spans > 1 block */ if (!ptr) { /* @@ -689,8 +768,9 @@ static void *alloc_heap_buffer(struct mm_heap *heap, int zone, uint32_t caps, dcache_invalidate_region(map, sizeof(*map)); /* allocate if block size is smaller than request */ - if (heap->size >= bytes && map->block_size < bytes) { - ptr = alloc_cont_blocks(heap, i, caps, bytes); + if (heap->size >= bytes && map->block_size < bytes) { + ptr = alloc_cont_blocks(heap, i, caps, + bytes, alignment); if (ptr) break; } @@ -702,13 +782,14 @@ static void *alloc_heap_buffer(struct mm_heap *heap, int zone, uint32_t caps, #if CONFIG_DEBUG_BLOCK_FREE if (ptr) - bzero(ptr, bytes); + bzero(ptr, temp_bytes); #endif return ptr; } -static void *_balloc_unlocked(int zone, uint32_t caps, size_t bytes) +static void *_balloc_unlocked(int zone, uint32_t caps, size_t bytes, + uint32_t alignment) { struct mm_heap *heap; unsigned int i, n; @@ -722,7 +803,7 @@ static void *_balloc_unlocked(int zone, uint32_t caps, size_t bytes) if (!heap) break; - ptr = alloc_heap_buffer(heap, zone, caps, bytes); + ptr = alloc_heap_buffer(heap, zone, caps, bytes, alignment); if (ptr) break; @@ -733,14 +814,14 @@ static void *_balloc_unlocked(int zone, uint32_t caps, size_t bytes) } /* allocates continuous buffers - not for direct use, clients use rballoc() */ -void *_balloc(int zone, uint32_t caps, size_t bytes) +void *_balloc(int zone, uint32_t caps, size_t bytes, uint32_t alignment) { void *ptr = NULL; uint32_t flags; spin_lock_irq(memmap.lock, flags); - ptr = _balloc_unlocked(zone, caps, bytes); + ptr = _balloc_unlocked(zone, caps, bytes, alignment); spin_unlock_irq(memmap.lock, flags); @@ -809,7 +890,8 @@ void *_realloc(void *ptr, int zone, uint32_t caps, size_t bytes) return new_ptr; } -void *_brealloc(void *ptr, int zone, uint32_t caps, size_t bytes) +void *_brealloc(void *ptr, int zone, uint32_t caps, size_t bytes, + uint32_t alignment) { void *new_ptr = NULL; uint32_t flags; @@ -819,7 +901,7 @@ void *_brealloc(void *ptr, int zone, uint32_t caps, size_t bytes) spin_lock_irq(memmap.lock, flags); - new_ptr = _balloc_unlocked(zone, caps, bytes); + new_ptr = _balloc_unlocked(zone, caps, bytes, alignment); if (new_ptr && ptr) memcpy_s(new_ptr, bytes, ptr, bytes); diff --git a/src/platform/baytrail/include/platform/lib/memory.h b/src/platform/baytrail/include/platform/lib/memory.h index 0492ed4ece5d..c05c4ab78ef1 100644 --- a/src/platform/baytrail/include/platform/lib/memory.h +++ b/src/platform/baytrail/include/platform/lib/memory.h @@ -16,6 +16,8 @@ void platform_init_memmap(void); #endif +#define PLATFORM_DCACHE_ALIGN sizeof(void *) + /* physical DSP addresses */ #define SHIM_BASE 0xFF340000 diff --git a/src/platform/haswell/include/platform/lib/memory.h b/src/platform/haswell/include/platform/lib/memory.h index 9b1c36f1b0b4..eb035e12bf59 100644 --- a/src/platform/haswell/include/platform/lib/memory.h +++ b/src/platform/haswell/include/platform/lib/memory.h @@ -16,8 +16,10 @@ void platform_init_memmap(void); #endif -/* physical DSP addresses */ +/* data cache line alignment */ +#define PLATFORM_DCACHE_ALIGN sizeof(void *) +/* physical DSP addresses */ #define SHIM_SIZE 0x00001000 @@ -97,7 +99,7 @@ void platform_init_memmap(void); #define HEAP_SYS_RT_COUNT1024 4 /* Heap configuration */ -#define SOF_DATA_SIZE 0xC000 +#define SOF_DATA_SIZE 0xD000 #define HEAP_SYSTEM_BASE (DRAM0_BASE + SOF_DATA_SIZE) #define HEAP_SYSTEM_SIZE 0x4000 diff --git a/src/platform/imx8/include/platform/lib/memory.h b/src/platform/imx8/include/platform/lib/memory.h index 97c12918aebc..c6b58cf9af80 100644 --- a/src/platform/imx8/include/platform/lib/memory.h +++ b/src/platform/imx8/include/platform/lib/memory.h @@ -13,6 +13,9 @@ #include #include +/* data cache line alignment */ +#define PLATFORM_DCACHE_ALIGN DCACHE_LINE_SIZE + /* physical DSP addresses */ #define IRAM_BASE 0x596f8000 diff --git a/src/platform/intel/cavs/include/cavs/lib/memory.h b/src/platform/intel/cavs/include/cavs/lib/memory.h index d24afc3805ef..97f02732e793 100644 --- a/src/platform/intel/cavs/include/cavs/lib/memory.h +++ b/src/platform/intel/cavs/include/cavs/lib/memory.h @@ -13,6 +13,9 @@ #include #include +/* data cache line alignment */ +#define PLATFORM_DCACHE_ALIGN DCACHE_LINE_SIZE + #define SRAM_BANK_SIZE (64 * 1024) #define EBB_BANKS_IN_SEGMENT 32 diff --git a/test/cmocka/src/audio/buffer/mock.c b/test/cmocka/src/audio/buffer/mock.c index 2352d0113046..050886b1e7fa 100644 --- a/test/cmocka/src/audio/buffer/mock.c +++ b/test/cmocka/src/audio/buffer/mock.c @@ -17,7 +17,7 @@ TRACE_IMPL() #if !CONFIG_LIBRARY -void *rzalloc(int zone, uint32_t caps, size_t bytes) +void *_zalloc(int zone, uint32_t caps, size_t bytes) { (void)zone; (void)caps; @@ -25,7 +25,8 @@ void *rzalloc(int zone, uint32_t caps, size_t bytes) return malloc(bytes); } -void *rballoc(int zone, uint32_t caps, size_t bytes) +void *_balloc(int zone, uint32_t caps, size_t bytes, + uint32_t alignment) { (void)zone; (void)caps; @@ -38,7 +39,8 @@ void rfree(void *ptr) free(ptr); } -void *_brealloc(void *ptr, int zone, uint32_t caps, size_t bytes) +void *_brealloc(void *ptr, int zone, uint32_t caps, size_t bytes, + uint32_t alignment) { (void)zone; (void)caps; diff --git a/test/cmocka/src/audio/component/mock.c b/test/cmocka/src/audio/component/mock.c index d75efde20eb0..1a9c5deeca0f 100644 --- a/test/cmocka/src/audio/component/mock.c +++ b/test/cmocka/src/audio/component/mock.c @@ -17,7 +17,7 @@ TRACE_IMPL() #if !CONFIG_LIBRARY -void *rzalloc(int zone, uint32_t caps, size_t bytes) +void *_zalloc(int zone, uint32_t caps, size_t bytes) { (void)zone; (void)caps; diff --git a/test/cmocka/src/audio/kpb/kpb_mock.c b/test/cmocka/src/audio/kpb/kpb_mock.c index 9590d8e0ad1e..6951625436ba 100644 --- a/test/cmocka/src/audio/kpb/kpb_mock.c +++ b/test/cmocka/src/audio/kpb/kpb_mock.c @@ -21,7 +21,8 @@ TRACE_IMPL() -void *rballoc(int zone, uint32_t caps, size_t bytes) +void *_balloc(int zone, uint32_t caps, size_t bytes, + uint32_t alignment) { (void)zone; (void)caps; @@ -29,7 +30,7 @@ void *rballoc(int zone, uint32_t caps, size_t bytes) return malloc(bytes); } -void *rzalloc(int zone, uint32_t caps, size_t bytes) +void *_zalloc(int zone, uint32_t caps, size_t bytes) { (void)zone; (void)caps; @@ -42,7 +43,8 @@ void rfree(void *ptr) free(ptr); } -void *_brealloc(void *ptr, int zone, uint32_t caps, size_t bytes) +void *_brealloc(void *ptr, int zone, uint32_t caps, size_t bytes, + uint32_t alignment) { (void)zone; (void)caps; diff --git a/test/cmocka/src/audio/mixer/mock.c b/test/cmocka/src/audio/mixer/mock.c index c470828c2a69..fed68cf29498 100644 --- a/test/cmocka/src/audio/mixer/mock.c +++ b/test/cmocka/src/audio/mixer/mock.c @@ -20,7 +20,8 @@ TRACE_IMPL() -void *rballoc(int zone, uint32_t caps, size_t bytes) +void *_balloc(int zone, uint32_t caps, size_t bytes, + uint32_t alignment) { (void)zone; (void)caps; @@ -28,7 +29,7 @@ void *rballoc(int zone, uint32_t caps, size_t bytes) return malloc(bytes); } -void *rzalloc(int zone, uint32_t caps, size_t bytes) +void *_zalloc(int zone, uint32_t caps, size_t bytes) { (void)zone; (void)caps; @@ -41,7 +42,8 @@ void rfree(void *ptr) free(ptr); } -void *_brealloc(void *ptr, int zone, uint32_t caps, size_t bytes) +void *_brealloc(void *ptr, int zone, uint32_t caps, size_t bytes, + uint32_t alignment) { (void)zone; (void)caps; From f146ee91ba45bf2fb0d37b7dd402d9b3ec451197 Mon Sep 17 00:00:00 2001 From: Tomasz Lauda Date: Tue, 15 Oct 2019 13:02:48 +0200 Subject: [PATCH 02/11] dai: fix rare case of xrun on release Adds new start_position field based on which we assess whether we should throw an xrun. It fixes rare cases of xruns on release with timer scheduled pipelines. Signed-off-by: Tomasz Lauda --- src/audio/dai.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/audio/dai.c b/src/audio/dai.c index e3784b733ef7..351381351bb7 100644 --- a/src/audio/dai.c +++ b/src/audio/dai.c @@ -66,6 +66,7 @@ struct dai_data { int xrun; /* true if we are doing xrun recovery */ uint32_t dai_pos_blks; /* position in bytes (nearest block) */ + uint64_t start_position; /* position on start */ /* host can read back this value without IPC */ uint64_t *dai_pos; @@ -485,6 +486,17 @@ static int dai_reset(struct comp_dev *dev) return 0; } +static void dai_update_start_position(struct comp_dev *dev) +{ + struct dai_data *dd = comp_get_drvdata(dev); + + /* update starting wallclock */ + platform_dai_wallclock(dev, &dd->wallclock); + + /* update start position */ + dd->start_position = dev->position; +} + /* used to pass standard and bespoke command (with data) to component */ static int dai_comp_trigger(struct comp_dev *dev, int cmd) { @@ -518,8 +530,7 @@ static int dai_comp_trigger(struct comp_dev *dev, int cmd) dd->xrun = 0; } - /* update starting wallclock */ - platform_dai_wallclock(dev, &dd->wallclock); + dai_update_start_position(dev); break; case COMP_TRIGGER_RELEASE: /* before release, we clear the buffer data to 0s, @@ -545,8 +556,7 @@ static int dai_comp_trigger(struct comp_dev *dev, int cmd) dd->xrun = 0; } - /* update starting wallclock */ - platform_dai_wallclock(dev, &dd->wallclock); + dai_update_start_position(dev); break; case COMP_TRIGGER_XRUN: trace_dai_with_ids(dev, "dai_comp_trigger(), XRUN"); @@ -576,7 +586,7 @@ static int dai_check_for_xrun(struct comp_dev *dev, uint32_t copy_bytes) return 0; /* no data yet, we're just starting */ - if (!dev->position) + if (dd->start_position == dev->position) return PPL_STATUS_PATH_STOP; /* xrun occurred */ @@ -614,7 +624,7 @@ static int dai_copy(struct comp_dev *dev) ret = dma_start(dd->chan); if (ret < 0) return ret; - platform_dai_wallclock(dev, &dd->wallclock); + dai_update_start_position(dev); return 0; } From 820a90635a2d7f69e15211ece14392f622c4c6f6 Mon Sep 17 00:00:00 2001 From: Jakub Dabek Date: Mon, 14 Oct 2019 16:18:14 +0200 Subject: [PATCH 03/11] buffer: Add alignment setting for DMA buffers. Add seeting of alignment for DMA buffer that is required by Hardware configuration. Signed-off-by: Jakub Dabek --- src/audio/host.c | 21 +++++++++++++++++++++ src/drivers/dw/dma.c | 3 +++ src/drivers/imx/edma.c | 6 +++++- src/drivers/intel/cavs/hda-dma.c | 4 ++++ src/include/sof/lib/dma.h | 5 +++-- 5 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/audio/host.c b/src/audio/host.c index 0e2e46c0a3a4..23e6022e80fe 100644 --- a/src/audio/host.c +++ b/src/audio/host.c @@ -518,6 +518,23 @@ static void host_buffer_cb(void *data, uint32_t bytes) } while (hd->split_value); } +static void realign_buffer(struct host_data *hd) +{ + uint32_t buffer_alignment; + + dma_get_attribute(hd->dma, DMA_ATTR_BUFFER_ADDRESS_ALIGNMENT, + &buffer_alignment); + + if ((uintptr_t)hd->dma_buffer->addr % buffer_alignment) { + hd->dma_buffer->addr = + rbrealloc_align(hd->dma_buffer->addr, RZONE_BUFFER, + SOF_MEM_CAPS_DMA, + hd->dma_buffer->alloc_size, + buffer_alignment); + buffer_init(hd->dma_buffer, hd->dma_buffer->alloc_size); + } +} + /* configure the DMA params and descriptors for host buffer IO */ static int host_params(struct comp_dev *dev) { @@ -548,6 +565,8 @@ static int host_params(struct comp_dev *dev) hd->dma_buffer = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); + realign_buffer(hd); + /* set callback on buffer consume */ buffer_set_cb(hd->dma_buffer, &host_buffer_cb, dev, BUFF_CB_TYPE_CONSUME); @@ -570,6 +589,8 @@ static int host_params(struct comp_dev *dev) hd->dma_buffer = list_first_item(&dev->bsource_list, struct comp_buffer, sink_list); + realign_buffer(hd); + /* set callback on buffer produce */ buffer_set_cb(hd->dma_buffer, &host_buffer_cb, dev, BUFF_CB_TYPE_PRODUCE); diff --git a/src/drivers/dw/dma.c b/src/drivers/dw/dma.c index 2d085ac1fe9a..630129b147fe 100644 --- a/src/drivers/dw/dma.c +++ b/src/drivers/dw/dma.c @@ -1078,6 +1078,9 @@ static int dw_dma_get_attribute(struct dma *dma, uint32_t type, case DMA_ATTR_COPY_ALIGNMENT: *value = DW_DMA_COPY_ALIGNMENT; break; + case DMA_ATTR_BUFFER_ADDRESS_ALIGNMENT: + *value = PLATFORM_DCACHE_ALIGN; + break; default: ret = -EINVAL; break; diff --git a/src/drivers/imx/edma.c b/src/drivers/imx/edma.c index 3acec254cdf4..9b34569903ec 100644 --- a/src/drivers/imx/edma.c +++ b/src/drivers/imx/edma.c @@ -471,10 +471,14 @@ static int edma_get_attribute(struct dma *dma, uint32_t type, uint32_t *value) * size of the elementary copy). */ *value = 4; - return 0; + break; + case DMA_ATTR_BUFFER_ADDRESS_ALIGNMENT: + *value = PLATFORM_DCACHE_ALIGN; + break; default: return -ENOENT; /* Attribute not found */ } + return 0; } static int edma_get_data_size(struct dma_chan_data *channel, diff --git a/src/drivers/intel/cavs/hda-dma.c b/src/drivers/intel/cavs/hda-dma.c index 6e4eb015e4f6..ffaf67278290 100644 --- a/src/drivers/intel/cavs/hda-dma.c +++ b/src/drivers/intel/cavs/hda-dma.c @@ -65,6 +65,7 @@ /* DGMBS align value */ #define HDA_DMA_BUFFER_ALIGNMENT 0x20 #define HDA_DMA_COPY_ALIGNMENT 0x20 +#define HDA_DMA_BUFFER_ADDRESS_ALIGNMENT 0x80 /* * DMA Pointer Trace @@ -875,6 +876,9 @@ static int hda_dma_get_attribute(struct dma *dma, uint32_t type, case DMA_ATTR_COPY_ALIGNMENT: *value = HDA_DMA_COPY_ALIGNMENT; break; + case DMA_ATTR_BUFFER_ADDRESS_ALIGNMENT: + *value = HDA_DMA_BUFFER_ADDRESS_ALIGNMENT; + break; default: ret = -EINVAL; break; diff --git a/src/include/sof/lib/dma.h b/src/include/sof/lib/dma.h index 5a21134dfc79..56b6cd9bcec6 100644 --- a/src/include/sof/lib/dma.h +++ b/src/include/sof/lib/dma.h @@ -88,8 +88,9 @@ enum dma_irq_cmd { #define DMA_CORE_INVALID 0xFFFFFFFF /* DMA attributes */ -#define DMA_ATTR_BUFFER_ALIGNMENT 0 -#define DMA_ATTR_COPY_ALIGNMENT 1 +#define DMA_ATTR_BUFFER_ALIGNMENT 0 +#define DMA_ATTR_COPY_ALIGNMENT 1 +#define DMA_ATTR_BUFFER_ADDRESS_ALIGNMENT 2 struct dma; From c01333810f15b19555aa6b82b9e3f2cacd4fb5c0 Mon Sep 17 00:00:00 2001 From: Jakub Dabek Date: Mon, 21 Oct 2019 13:23:26 +0200 Subject: [PATCH 04/11] memory: Fix DEBUG_HEAP build Unify function naming to fix DEBUG_HEAP build Signed-off-by: Jakub Dabek --- src/include/sof/lib/alloc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/include/sof/lib/alloc.h b/src/include/sof/lib/alloc.h index dee50019fe82..3f7c4d1c476b 100644 --- a/src/include/sof/lib/alloc.h +++ b/src/include/sof/lib/alloc.h @@ -182,7 +182,7 @@ void rfree(void *ptr); } while (0); \ _ptr; }) -#define rbrealloc_a(ptr, zone, caps, bytes, alignment) \ +#define rbrealloc_align(ptr, zone, caps, bytes, alignment) \ ({void *_ptr; \ do { \ _ptr = _brealloc(ptr, zone, caps, bytes, alignment); \ From db0278d2686dceeef374de2a11ba4b3727d74f0b Mon Sep 17 00:00:00 2001 From: Tomasz Lauda Date: Thu, 17 Oct 2019 14:23:01 +0200 Subject: [PATCH 05/11] buffer: add and init id, pipeline_id and caps fields Adds and initializes id, pipeline_id and caps fields in buffer struct. This is the only data needed after buffer creation. It allows us to remove the whole sof_ipc_buffer struct from the buffers. Signed-off-by: Tomasz Lauda --- src/audio/buffer.c | 39 ++++++++++++-------------------- src/audio/mux/mux.c | 4 ++-- src/audio/pipeline.c | 2 +- src/include/sof/audio/buffer.h | 12 ++++++---- src/ipc/ipc.c | 2 +- test/cmocka/src/audio/mux/util.h | 4 ++-- 6 files changed, 28 insertions(+), 35 deletions(-) diff --git a/src/audio/buffer.c b/src/audio/buffer.c index dcaa3f7148a0..478a62e5821c 100644 --- a/src/audio/buffer.c +++ b/src/audio/buffer.c @@ -7,13 +7,11 @@ #include #include -#include #include #include #include #include #include -#include #include #include #include @@ -51,10 +49,10 @@ struct comp_buffer *buffer_new(struct sof_ipc_buffer *desc) return NULL; } - assert(!memcpy_s(&buffer->ipc_buffer, sizeof(buffer->ipc_buffer), - desc, sizeof(*desc))); + buffer_init(buffer, desc->size, desc->caps); - buffer_init(buffer, desc->size); + buffer->id = desc->comp.id; + buffer->pipeline_id = desc->comp.pipeline_id; return buffer; } @@ -62,7 +60,6 @@ struct comp_buffer *buffer_new(struct sof_ipc_buffer *desc) int buffer_set_size(struct comp_buffer *buffer, uint32_t size) { void *new_ptr = NULL; - struct sof_ipc_buffer *desc = NULL; /* validate request */ if (size == 0 || size > HEAP_BUFFER_SIZE) { @@ -70,21 +67,15 @@ int buffer_set_size(struct comp_buffer *buffer, uint32_t size) return -EINVAL; } - desc = &buffer->ipc_buffer; - if (!desc) { - trace_buffer_error("resize error: invalid buffer desc"); - return -EINVAL; - } - - if (size == desc->size) + if (size == buffer->size) return 0; - new_ptr = rbrealloc(buffer->addr, RZONE_BUFFER, desc->caps, size); + new_ptr = rbrealloc(buffer->addr, RZONE_BUFFER, buffer->caps, size); /* we couldn't allocate bigger chunk */ - if (!new_ptr && size > desc->size) { + if (!new_ptr && size > buffer->size) { trace_buffer_error("resize error: can't alloc %u bytes type %u", - desc->size, desc->caps); + buffer->size, buffer->caps); return -ENOMEM; } @@ -92,9 +83,7 @@ int buffer_set_size(struct comp_buffer *buffer, uint32_t size) if (new_ptr) buffer->addr = new_ptr; - desc->size = size; - - buffer_init(buffer, desc->size); + buffer_init(buffer, size, buffer->caps); return 0; } @@ -184,10 +173,10 @@ void comp_update_buffer_produce(struct comp_buffer *buffer, uint32_t bytes) irq_local_enable(flags); tracev_buffer("comp_update_buffer_produce(), ((buffer->avail << 16) | " - "buffer->free) = %08x, ((buffer->ipc_buffer.comp.id << " - "16) | buffer->size) = %08x", + "buffer->free) = %08x, ((buffer->id << 16) | " + "buffer->size) = %08x", (buffer->avail << 16) | buffer->free, - (buffer->ipc_buffer.comp.id << 16) | buffer->size); + (buffer->id << 16) | buffer->size); tracev_buffer("comp_update_buffer_produce(), ((buffer->r_ptr - buffer" "->addr) << 16 | (buffer->w_ptr - buffer->addr)) = %08x", (buffer->r_ptr - buffer->addr) << 16 | @@ -240,11 +229,11 @@ void comp_update_buffer_consume(struct comp_buffer *buffer, uint32_t bytes) tracev_buffer("comp_update_buffer_consume(), " "(buffer->avail << 16) | buffer->free = %08x, " - "(buffer->ipc_buffer.comp.id << 16) | buffer->size = " - " %08x, (buffer->r_ptr - buffer->addr) << 16 | " + "(buffer->id << 16) | buffer->size = %08x, " + "(buffer->r_ptr - buffer->addr) << 16 | " "(buffer->w_ptr - buffer->addr)) = %08x", (buffer->avail << 16) | buffer->free, - (buffer->ipc_buffer.comp.id << 16) | buffer->size, + (buffer->id << 16) | buffer->size, (buffer->r_ptr - buffer->addr) << 16 | (buffer->w_ptr - buffer->addr)); } diff --git a/src/audio/mux/mux.c b/src/audio/mux/mux.c index 8da0b0a45659..e9d2c84e3068 100644 --- a/src/audio/mux/mux.c +++ b/src/audio/mux/mux.c @@ -228,7 +228,7 @@ static int demux_copy(struct comp_dev *dev) sink = container_of(clist, struct comp_buffer, source_list); if (sink->sink->state == dev->state) { num_sinks++; - i = get_stream_index(cd, sink->ipc_buffer.comp.pipeline_id); + i = get_stream_index(cd, sink->pipeline_id); sinks[i] = sink; } } @@ -297,7 +297,7 @@ static int mux_copy(struct comp_dev *dev) source = container_of(clist, struct comp_buffer, sink_list); if (source->source->state == dev->state) { num_sources++; - i = get_stream_index(cd, source->ipc_buffer.comp.pipeline_id); + i = get_stream_index(cd, source->pipeline_id); sources[i] = source; } } diff --git a/src/audio/pipeline.c b/src/audio/pipeline.c index 7c8e0566116f..1495ab473e25 100644 --- a/src/audio/pipeline.c +++ b/src/audio/pipeline.c @@ -72,7 +72,7 @@ int pipeline_connect(struct comp_dev *comp, struct comp_buffer *buffer, uint32_t flags; trace_pipe("pipeline: connect comp %d and buffer %d", - comp->comp.id, buffer->ipc_buffer.comp.id); + comp->comp.id, buffer->id); irq_local_disable(flags); list_item_prepend(buffer_comp_list(buffer, dir), diff --git a/src/include/sof/audio/buffer.h b/src/include/sof/audio/buffer.h index 3dd92792f9e9..21519e40efc3 100644 --- a/src/include/sof/audio/buffer.h +++ b/src/include/sof/audio/buffer.h @@ -48,8 +48,10 @@ struct comp_buffer { void *addr; /* buffer base address */ void *end_addr; /* buffer end address */ - /* IPC configuration */ - struct sof_ipc_buffer ipc_buffer; + /* configuration */ + uint32_t id; + uint32_t pipeline_id; + uint32_t caps; /* connected components */ struct comp_dev *source; /* source component */ @@ -129,7 +131,7 @@ static inline void buffer_zero(struct comp_buffer *buffer) tracev_buffer("buffer_zero()"); bzero(buffer->addr, buffer->size); - if (buffer->ipc_buffer.caps & SOF_MEM_CAPS_DMA) + if (buffer->caps & SOF_MEM_CAPS_DMA) dcache_writeback_region(buffer->addr, buffer->size); } @@ -211,10 +213,12 @@ static inline void *buffer_get_frag(struct comp_buffer *buffer, void *ptr, return current; } -static inline void buffer_init(struct comp_buffer *buffer, uint32_t size) +static inline void buffer_init(struct comp_buffer *buffer, uint32_t size, + uint32_t caps) { buffer->alloc_size = size; buffer->size = size; + buffer->caps = caps; buffer->w_ptr = buffer->addr; buffer->r_ptr = buffer->addr; buffer->end_addr = buffer->addr + size; diff --git a/src/ipc/ipc.c b/src/ipc/ipc.c index 908011daa083..335d4f54c2b7 100644 --- a/src/ipc/ipc.c +++ b/src/ipc/ipc.c @@ -51,7 +51,7 @@ struct ipc_comp_dev *ipc_get_comp(struct ipc *ipc, uint32_t id) return icd; break; case COMP_TYPE_BUFFER: - if (icd->cb->ipc_buffer.comp.id == id) + if (icd->cb->id == id) return icd; break; case COMP_TYPE_PIPELINE: diff --git a/test/cmocka/src/audio/mux/util.h b/test/cmocka/src/audio/mux/util.h index b77153ced9f4..224d6e4e31aa 100644 --- a/test/cmocka/src/audio/mux/util.h +++ b/test/cmocka/src/audio/mux/util.h @@ -29,7 +29,7 @@ static inline struct comp_buffer *create_test_sink(struct comp_dev *dev, buffer->sink->params.channels = channels; buffer->free = 0; buffer->avail = 0; - buffer->ipc_buffer.comp.pipeline_id = pipeline_id; + buffer->pipeline_id = pipeline_id; return buffer; } @@ -57,7 +57,7 @@ static inline struct comp_buffer *create_test_source(struct comp_dev *dev, buffer->source->params.channels = channels; buffer->free = 0; buffer->avail = 0; - buffer->ipc_buffer.comp.pipeline_id = pipeline_id; + buffer->pipeline_id = pipeline_id; return buffer; } From 44257a8739e63ecd8f397990c380d101d6774d7f Mon Sep 17 00:00:00 2001 From: Tomasz Lauda Date: Thu, 17 Oct 2019 14:56:00 +0200 Subject: [PATCH 06/11] buffer: extract allocation part to separate function Extracts allocation part of buffer_new to separate function. Signed-off-by: Tomasz Lauda --- src/audio/buffer.c | 42 +++++++++++++++++++++++----------- src/audio/host.c | 3 ++- src/include/sof/audio/buffer.h | 1 + 3 files changed, 32 insertions(+), 14 deletions(-) diff --git a/src/audio/buffer.c b/src/audio/buffer.c index 478a62e5821c..83bbd7179c4c 100644 --- a/src/audio/buffer.c +++ b/src/audio/buffer.c @@ -17,42 +17,58 @@ #include #include -/* create a new component in the pipeline */ -struct comp_buffer *buffer_new(struct sof_ipc_buffer *desc) +struct comp_buffer *buffer_alloc(uint32_t size, uint32_t caps, uint32_t align) { struct comp_buffer *buffer; - trace_buffer("buffer_new()"); + trace_buffer("buffer_alloc()"); /* validate request */ - if (desc->size == 0 || desc->size > HEAP_BUFFER_SIZE) { - trace_buffer_error("buffer_new() error: " - "new size = %u is invalid", desc->size); + if (size == 0 || size > HEAP_BUFFER_SIZE) { + trace_buffer_error("buffer_alloc() error: " + "new size = %u is invalid", size); return NULL; } /* allocate new buffer */ buffer = rzalloc(RZONE_RUNTIME, SOF_MEM_CAPS_RAM, sizeof(*buffer)); if (!buffer) { - trace_buffer_error("buffer_new() error: " + trace_buffer_error("buffer_alloc() error: " "could not alloc structure"); return NULL; } - buffer->addr = rballoc(RZONE_BUFFER, desc->caps, desc->size); + buffer->addr = rballoc_align(RZONE_BUFFER, caps, size, align); if (!buffer->addr) { rfree(buffer); - trace_buffer_error("buffer_new() error: " + trace_buffer_error("buffer_alloc() error: " "could not alloc size = %u " "bytes of type = %u", - desc->size, desc->caps); + size, caps); return NULL; } - buffer_init(buffer, desc->size, desc->caps); + buffer_init(buffer, size, caps); + + list_init(&buffer->source_list); + list_init(&buffer->sink_list); + + return buffer; +} + +/* create a new component in the pipeline */ +struct comp_buffer *buffer_new(struct sof_ipc_buffer *desc) +{ + struct comp_buffer *buffer; + + trace_buffer("buffer_new()"); - buffer->id = desc->comp.id; - buffer->pipeline_id = desc->comp.pipeline_id; + /* allocate buffer */ + buffer = buffer_alloc(desc->size, desc->caps, PLATFORM_DCACHE_ALIGN); + if (buffer) { + buffer->id = desc->comp.id; + buffer->pipeline_id = desc->comp.pipeline_id; + } return buffer; } diff --git a/src/audio/host.c b/src/audio/host.c index 23e6022e80fe..4a291ab7dda7 100644 --- a/src/audio/host.c +++ b/src/audio/host.c @@ -531,7 +531,8 @@ static void realign_buffer(struct host_data *hd) SOF_MEM_CAPS_DMA, hd->dma_buffer->alloc_size, buffer_alignment); - buffer_init(hd->dma_buffer, hd->dma_buffer->alloc_size); + buffer_init(hd->dma_buffer, hd->dma_buffer->alloc_size, + SOF_MEM_CAPS_DMA); } } diff --git a/src/include/sof/audio/buffer.h b/src/include/sof/audio/buffer.h index 21519e40efc3..97c98dcf7951 100644 --- a/src/include/sof/audio/buffer.h +++ b/src/include/sof/audio/buffer.h @@ -116,6 +116,7 @@ struct comp_buffer { typedef void (*cache_buff_op)(struct comp_buffer *); /* pipeline buffer creation and destruction */ +struct comp_buffer *buffer_alloc(uint32_t size, uint32_t caps, uint32_t align); struct comp_buffer *buffer_new(struct sof_ipc_buffer *desc); int buffer_set_size(struct comp_buffer *buffer, uint32_t size); void buffer_free(struct comp_buffer *buffer); From 0b3f7e2d26ffed326d381b8852ac775804d9744a Mon Sep 17 00:00:00 2001 From: Tomasz Lauda Date: Thu, 17 Oct 2019 17:22:09 +0200 Subject: [PATCH 07/11] dma: add new attribute DMA_ATTR_BUFFER_PERIOD_COUNT Adds new DMA attribute DMA_ATTR_BUFFER_PERIOD_COUNT and implements it for every DMA. This attribute says how many periods should DMA buffer consist of. Signed-off-by: Tomasz Lauda --- src/drivers/dw/dma.c | 9 +++++++++ src/drivers/imx/edma.c | 3 +++ src/drivers/intel/cavs/hda-dma.c | 6 ++++++ src/include/sof/drivers/edma.h | 2 ++ src/include/sof/lib/dma.h | 1 + 5 files changed, 21 insertions(+) diff --git a/src/drivers/dw/dma.c b/src/drivers/dw/dma.c index 630129b147fe..19d931b4ee60 100644 --- a/src/drivers/dw/dma.c +++ b/src/drivers/dw/dma.c @@ -65,6 +65,12 @@ struct dw_dma_chan_data { */ static const uint32_t burst_elems[] = {1, 2, 4, 8}; +#if CONFIG_HW_LLI +#define DW_DMA_BUFFER_PERIOD_COUNT 3 +#else +#define DW_DMA_BUFFER_PERIOD_COUNT 2 +#endif + static void dw_dma_interrupt_mask(struct dma_chan_data *channel) { /* mask block, transfer and error interrupts for channel */ @@ -1081,6 +1087,9 @@ static int dw_dma_get_attribute(struct dma *dma, uint32_t type, case DMA_ATTR_BUFFER_ADDRESS_ALIGNMENT: *value = PLATFORM_DCACHE_ALIGN; break; + case DMA_ATTR_BUFFER_PERIOD_COUNT: + *value = DW_DMA_BUFFER_PERIOD_COUNT; + break; default: ret = -EINVAL; break; diff --git a/src/drivers/imx/edma.c b/src/drivers/imx/edma.c index 9b34569903ec..2b3849d115a6 100644 --- a/src/drivers/imx/edma.c +++ b/src/drivers/imx/edma.c @@ -475,6 +475,9 @@ static int edma_get_attribute(struct dma *dma, uint32_t type, uint32_t *value) case DMA_ATTR_BUFFER_ADDRESS_ALIGNMENT: *value = PLATFORM_DCACHE_ALIGN; break; + case DMA_ATTR_BUFFER_PERIOD_COUNT: + *value = EDMA_BUFFER_PERIOD_COUNT; + break; default: return -ENOENT; /* Attribute not found */ } diff --git a/src/drivers/intel/cavs/hda-dma.c b/src/drivers/intel/cavs/hda-dma.c index ffaf67278290..6c79fc77604f 100644 --- a/src/drivers/intel/cavs/hda-dma.c +++ b/src/drivers/intel/cavs/hda-dma.c @@ -67,6 +67,9 @@ #define HDA_DMA_COPY_ALIGNMENT 0x20 #define HDA_DMA_BUFFER_ADDRESS_ALIGNMENT 0x80 +/* DMA number of buffer periods */ +#define HDA_DMA_BUFFER_PERIOD_COUNT 2 + /* * DMA Pointer Trace * @@ -879,6 +882,9 @@ static int hda_dma_get_attribute(struct dma *dma, uint32_t type, case DMA_ATTR_BUFFER_ADDRESS_ALIGNMENT: *value = HDA_DMA_BUFFER_ADDRESS_ALIGNMENT; break; + case DMA_ATTR_BUFFER_PERIOD_COUNT: + *value = HDA_DMA_BUFFER_PERIOD_COUNT; + break; default: ret = -EINVAL; break; diff --git a/src/include/sof/drivers/edma.h b/src/include/sof/drivers/edma.h index 6adce4a0e6ae..0975c079bac4 100644 --- a/src/include/sof/drivers/edma.h +++ b/src/include/sof/drivers/edma.h @@ -75,6 +75,8 @@ int edma_init(void); +#define EDMA_BUFFER_PERIOD_COUNT 2 + #define EDMA_TCD_ALIGNMENT 32 #define EDMA_HS_GET_IRQ(hs) (((hs) & MASK(8, 0)) >> 0) diff --git a/src/include/sof/lib/dma.h b/src/include/sof/lib/dma.h index 56b6cd9bcec6..43ec82e88316 100644 --- a/src/include/sof/lib/dma.h +++ b/src/include/sof/lib/dma.h @@ -91,6 +91,7 @@ enum dma_irq_cmd { #define DMA_ATTR_BUFFER_ALIGNMENT 0 #define DMA_ATTR_COPY_ALIGNMENT 1 #define DMA_ATTR_BUFFER_ADDRESS_ALIGNMENT 2 +#define DMA_ATTR_BUFFER_PERIOD_COUNT 3 struct dma; From cf7bd7546163fdf4b867ea899ffb9c70f4c42f07 Mon Sep 17 00:00:00 2001 From: Tomasz Lauda Date: Fri, 18 Oct 2019 11:12:08 +0200 Subject: [PATCH 08/11] buffer: implement buffer copy functions Implements simple buffer copy functions for 16 and 32 bit data. Signed-off-by: Tomasz Lauda --- src/include/sof/audio/buffer.h | 37 ++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/include/sof/audio/buffer.h b/src/include/sof/audio/buffer.h index 97c98dcf7951..d7892fadcd69 100644 --- a/src/include/sof/audio/buffer.h +++ b/src/include/sof/audio/buffer.h @@ -227,4 +227,41 @@ static inline void buffer_init(struct comp_buffer *buffer, uint32_t size, buffer->avail = 0; buffer_zero(buffer); } + +static inline void buffer_copy_s16(struct comp_buffer *source, + struct comp_buffer *sink, uint32_t bytes) +{ + uint32_t frames = bytes / sizeof(int16_t); + uint32_t buff_frag = 0; + int16_t *src; + int16_t *dst; + uint32_t i; + + for (i = 0; i < frames; i++) { + src = buffer_read_frag_s16(source, buff_frag); + dst = buffer_write_frag_s16(sink, buff_frag); + *dst = *src; + + buff_frag++; + } +} + +static inline void buffer_copy_s32(struct comp_buffer *source, + struct comp_buffer *sink, uint32_t bytes) +{ + uint32_t frames = bytes / sizeof(int32_t); + uint32_t buff_frag = 0; + int32_t *src; + int32_t *dst; + uint32_t i; + + for (i = 0; i < frames; i++) { + src = buffer_read_frag_s32(source, buff_frag); + dst = buffer_write_frag_s32(sink, buff_frag); + *dst = *src; + + buff_frag++; + } +} + #endif /* __SOF_AUDIO_BUFFER_H__ */ From 3bd6f8f3e7d38d744181f2a731ec39b3b33bc63a Mon Sep 17 00:00:00 2001 From: Tomasz Lauda Date: Fri, 18 Oct 2019 12:50:17 +0200 Subject: [PATCH 09/11] dma: implement dma buffer copy functions Implements copy functions from/to DMA buffer. They take care of cache operations and also can perform custom processing during copy. By default it will be usual buffer copy, but in the future it can be replaced with pcm conversions, so we don't need volume into the pipelines just for that operation. Signed-off-by: Tomasz Lauda --- src/include/sof/lib/dma.h | 12 ++++++++ src/lib/dma.c | 62 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/src/include/sof/lib/dma.h b/src/include/sof/lib/dma.h index 43ec82e88316..8d6a81feb519 100644 --- a/src/include/sof/lib/dma.h +++ b/src/include/sof/lib/dma.h @@ -26,6 +26,8 @@ #include #include +struct comp_buffer; + /** \addtogroup sof_dma_drivers DMA Drivers * DMA Drivers API specification. * @{ @@ -500,6 +502,16 @@ static inline uint32_t dma_sg_get_size(struct dma_sg_elem_array *ea) return size; } +/* copies data from DMA buffer using provided processing function */ +void dma_buffer_copy_from(struct comp_buffer *source, struct comp_buffer *sink, + void (*process)(struct comp_buffer *, struct comp_buffer *, uint32_t), + uint32_t bytes); + +/* copies data to DMA buffer using provided processing function */ +void dma_buffer_copy_to(struct comp_buffer *source, struct comp_buffer *sink, + void (*process)(struct comp_buffer *, struct comp_buffer *, uint32_t), + uint32_t bytes); + /* generic DMA DSP <-> Host copier */ struct dma_copy { diff --git a/src/lib/dma.c b/src/lib/dma.c index 510b612c55a2..193c319a3176 100644 --- a/src/lib/dma.c +++ b/src/lib/dma.c @@ -5,7 +5,9 @@ // Author: Ranjani Sridharan #include +#include #include +#include #include #include #include @@ -186,3 +188,63 @@ void dma_sg_free(struct dma_sg_elem_array *elem_array) rfree(elem_array->elems); dma_sg_init(elem_array); } + +void dma_buffer_copy_from(struct comp_buffer *source, struct comp_buffer *sink, + void (*process)(struct comp_buffer *, struct comp_buffer *, uint32_t), + uint32_t bytes) +{ + uint32_t head = bytes; + uint32_t tail = 0; + + /* source buffer contains data copied by DMA */ + if (source->r_ptr + bytes > source->end_addr) { + head = source->end_addr - source->r_ptr; + tail = bytes - head; + } + + dcache_invalidate_region(source->r_ptr, head); + if (tail) + dcache_invalidate_region(source->addr, tail); + + /* process data */ + process(source, sink, bytes); + + source->r_ptr += bytes; + + /* check for pointer wrap */ + if (source->r_ptr >= source->end_addr) + source->r_ptr = source->addr + + (source->r_ptr - source->end_addr); + + comp_update_buffer_produce(sink, bytes); +} + +void dma_buffer_copy_to(struct comp_buffer *source, struct comp_buffer *sink, + void (*process)(struct comp_buffer *, struct comp_buffer *, uint32_t), + uint32_t bytes) +{ + uint32_t head = bytes; + uint32_t tail = 0; + + /* process data */ + process(source, sink, bytes); + + /* sink buffer contains data meant to copied to DMA */ + if (sink->w_ptr + bytes > sink->end_addr) { + head = sink->end_addr - sink->w_ptr; + tail = bytes - head; + } + + dcache_writeback_region(sink->w_ptr, head); + if (tail) + dcache_writeback_region(sink->addr, tail); + + sink->w_ptr += bytes; + + /* check for pointer wrap */ + if (source->r_ptr >= sink->end_addr) + sink->w_ptr = sink->addr + + (sink->w_ptr - sink->end_addr); + + comp_update_buffer_consume(source, bytes); +} From 64b9d55f1a0c2d305d7a6c391ea88cafde5d32cf Mon Sep 17 00:00:00 2001 From: Tomasz Lauda Date: Fri, 18 Oct 2019 16:37:25 +0200 Subject: [PATCH 10/11] dw-dma: fix buffer and copy alignments DW-DMA has no specific alignments regarding buffer and copy sizes. However setting it to 32 bytes breaks stream formats with period sizes lower than 32 bytes, as linked list is being configured incorrectly. Signed-off-by: Tomasz Lauda --- src/include/sof/drivers/dw-dma.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/sof/drivers/dw-dma.h b/src/include/sof/drivers/dw-dma.h index aeb1a7aac394..8667a14679ca 100644 --- a/src/include/sof/drivers/dw-dma.h +++ b/src/include/sof/drivers/dw-dma.h @@ -134,8 +134,8 @@ #define trace_dwdma_error(__e, ...) \ trace_error(TRACE_CLASS_DMA, __e, ##__VA_ARGS__) -#define DW_DMA_BUFFER_ALIGNMENT 0x20 -#define DW_DMA_COPY_ALIGNMENT 0x20 +#define DW_DMA_BUFFER_ALIGNMENT 0x4 +#define DW_DMA_COPY_ALIGNMENT 0x4 /* TODO: add FIFO sizes */ struct dw_chan_data { From 065933a5d6a32de259008a70eea0fe82c3395f78 Mon Sep 17 00:00:00 2001 From: Tomasz Lauda Date: Fri, 18 Oct 2019 13:08:44 +0200 Subject: [PATCH 11/11] dma: alloc separate buffers for DMAs Currently DMAs are using buffers on pipelines connected to either host or dai components. This causes many problems in firmware logic e.g. we need to preload playback pipelines with data or we need to manually override buffer sizes, if the buffer isn't complaint with specific DMA requirements. Also it requires from topology creators to know some hardware specific things on each platform like that for timer based scheduling on cAVS platforms we need to have 3 periods in buffers connected to dai components. This patch allocates separate buffers for DMAs, which are made complaint by using different DMA attributes first. We no longer need to preload playback streams, because we can start DMA right away and just let it transfer zeroes before any valid data will come. Signed-off-by: Tomasz Lauda --- src/audio/dai.c | 186 ++++++++------- src/audio/host.c | 396 +++++++++++++++---------------- src/audio/kpb.c | 5 +- src/audio/pipeline.c | 21 +- src/drivers/dw/dma.c | 19 +- src/drivers/intel/cavs/hda-dma.c | 26 +- src/ipc/dma-copy.c | 3 +- src/ipc/ipc-host-ptable.c | 2 +- src/lib/dma.c | 2 +- 9 files changed, 329 insertions(+), 331 deletions(-) diff --git a/src/audio/dai.c b/src/audio/dai.c index 351381351bb7..a40d8910f693 100644 --- a/src/audio/dai.c +++ b/src/audio/dai.c @@ -65,6 +65,10 @@ struct dai_data { uint32_t frame_bytes; int xrun; /* true if we are doing xrun recovery */ + /* processing function */ + void (*process)(struct comp_buffer *source, struct comp_buffer *sink, + uint32_t bytes); + uint32_t dai_pos_blks; /* position in bytes (nearest block) */ uint64_t start_position; /* position on start */ @@ -80,6 +84,7 @@ static void dai_dma_cb(void *data, uint32_t type, struct dma_cb_data *next) struct comp_dev *dev = (struct comp_dev *)data; struct dai_data *dd = comp_get_drvdata(dev); uint32_t bytes = next->elem.size; + struct comp_buffer *local_buffer; void *buffer_ptr; tracev_dai_with_ids(dev, "dai_dma_cb()"); @@ -106,15 +111,21 @@ static void dai_dma_cb(void *data, uint32_t type, struct dma_cb_data *next) } if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) { - /* recalc available buffer space */ - comp_update_buffer_consume(dd->dma_buffer, bytes); + local_buffer = list_first_item(&dev->bsource_list, + struct comp_buffer, sink_list); + + dma_buffer_copy_to(local_buffer, dd->dma_buffer, dd->process, + bytes); - buffer_ptr = dd->dma_buffer->r_ptr; + buffer_ptr = local_buffer->r_ptr; } else { - /* recalc available buffer space */ - comp_update_buffer_produce(dd->dma_buffer, bytes); + local_buffer = list_first_item(&dev->bsink_list, + struct comp_buffer, source_list); + + dma_buffer_copy_from(dd->dma_buffer, local_buffer, dd->process, + bytes); - buffer_ptr = dd->dma_buffer->w_ptr; + buffer_ptr = local_buffer->w_ptr; } /* update host position (in bytes offset) for drivers */ @@ -210,14 +221,13 @@ static void dai_free(struct comp_dev *dev) } /* set component audio SSP and DMA configuration */ -static int dai_playback_params(struct comp_dev *dev, uint32_t period_bytes) +static int dai_playback_params(struct comp_dev *dev, uint32_t period_bytes, + uint32_t period_count) { struct dai_data *dd = comp_get_drvdata(dev); struct dma_sg_config *config = &dd->config; - struct sof_ipc_comp_config *dai_config; - int err; - uint32_t buffer_size; uint32_t fifo; + int err; /* set up DMA configuration */ config->direction = DMA_DIR_MEM_TO_DEV; @@ -235,25 +245,6 @@ static int dai_playback_params(struct comp_dev *dev, uint32_t period_bytes) config->dest_dev, dd->stream_id, config->src_width, config->dest_width); - /* set up local and host DMA elems to reset values */ - dai_config = COMP_GET_CONFIG(dev); - buffer_size = dai_config->periods_source * period_bytes; - - /* resize the buffer if space is available to align with period size */ - err = buffer_set_size(dd->dma_buffer, buffer_size); - if (err < 0) { - trace_dai_error_with_ids(dev, "dai_playback_params() error: " - "buffer_set_size() failed to resize " - "buffer. dai_config->periods_source =" - " %u; period_bytes = %u; " - "buffer_size = %u; " - "dd->dma_buffer->alloc_size = %u", - dai_config->periods_source, - period_bytes, buffer_size, - dd->dma_buffer->alloc_size); - return err; - } - if (!config->elem_array.elems) { fifo = dai_get_fifo(dd->dai, dev->params.direction, dd->stream_id); @@ -263,9 +254,9 @@ static int dai_playback_params(struct comp_dev *dev, uint32_t period_bytes) err = dma_sg_alloc(&config->elem_array, RZONE_RUNTIME, config->direction, - dai_config->periods_source, + period_count, period_bytes, - (uintptr_t)(dd->dma_buffer->r_ptr), + (uintptr_t)(dd->dma_buffer->addr), fifo); if (err < 0) { trace_dai_error_with_ids(dev, "dai_playback_params() " @@ -278,14 +269,13 @@ static int dai_playback_params(struct comp_dev *dev, uint32_t period_bytes) return 0; } -static int dai_capture_params(struct comp_dev *dev, uint32_t period_bytes) +static int dai_capture_params(struct comp_dev *dev, uint32_t period_bytes, + uint32_t period_count) { struct dai_data *dd = comp_get_drvdata(dev); struct dma_sg_config *config = &dd->config; - struct sof_ipc_comp_config *dai_config; - int err; - uint32_t buffer_size; uint32_t fifo; + int err; /* set up DMA configuration */ config->direction = DMA_DIR_DEV_TO_MEM; @@ -314,25 +304,6 @@ static int dai_capture_params(struct comp_dev *dev, uint32_t period_bytes) config->src_dev, dd->stream_id, config->src_width, config->dest_width); - /* set up local and host DMA elems to reset values */ - dai_config = COMP_GET_CONFIG(dev); - buffer_size = dai_config->periods_sink * period_bytes; - - /* resize the buffer if space is available to align with period size */ - err = buffer_set_size(dd->dma_buffer, buffer_size); - if (err < 0) { - trace_dai_error_with_ids(dev, "dai_capture_params() error: " - "buffer_set_size() failed to resize " - "buffer. dai_config->periods_sink = " - "%u; period_bytes = %u; " - "buffer_size = %u; " - "dd->dma_buffer->alloc_size = %u", - dai_config->periods_sink, - period_bytes, buffer_size, - dd->dma_buffer->alloc_size); - return err; - } - if (!config->elem_array.elems) { fifo = dai_get_fifo(dd->dai, dev->params.direction, dd->stream_id); @@ -342,9 +313,9 @@ static int dai_capture_params(struct comp_dev *dev, uint32_t period_bytes) err = dma_sg_alloc(&config->elem_array, RZONE_RUNTIME, config->direction, - dai_config->periods_sink, + period_count, period_bytes, - (uintptr_t)(dd->dma_buffer->w_ptr), + (uintptr_t)(dd->dma_buffer->addr), fifo); if (err < 0) { trace_dai_error_with_ids(dev, "dai_capture_params() " @@ -361,7 +332,10 @@ static int dai_params(struct comp_dev *dev) { struct dai_data *dd = comp_get_drvdata(dev); struct sof_ipc_comp_config *dconfig = COMP_GET_CONFIG(dev); + uint32_t period_count; uint32_t period_bytes; + uint32_t buffer_size; + uint32_t addr_align; uint32_t align; int err; @@ -384,6 +358,12 @@ static int dai_params(struct comp_dev *dev) /* for DAI, we should configure its frame_fmt from topology */ dev->params.frame_fmt = dconfig->frame_fmt; + /* set processing function */ + if (dev->params.frame_fmt == SOF_IPC_FRAME_S16_LE) + dd->process = buffer_copy_s16; + else + dd->process = buffer_copy_s32; + /* calculate period size based on config */ dd->frame_bytes = comp_frame_bytes(dev); if (!dd->frame_bytes) { @@ -392,36 +372,66 @@ static int dai_params(struct comp_dev *dev) return -EINVAL; } - err = dma_get_attribute(dd->dma, DMA_ATTR_BUFFER_ALIGNMENT, &align); + err = dma_get_attribute(dd->dma, DMA_ATTR_BUFFER_ADDRESS_ALIGNMENT, + &addr_align); if (err < 0) { - trace_dai_error("dai_params(): could not get dma buffer " - "alignment"); + trace_dai_error_with_ids(dev, "dai_params() error: could not " + "get dma buffer address alignment, " + "err = %d", err); return err; } - period_bytes = ALIGN_UP(dev->frames * dd->frame_bytes, align); - - if (!period_bytes) { - trace_dai_error_with_ids(dev, "dai_params() error: device has " - "no bytes (no frames to copy to sink)."); + err = dma_get_attribute(dd->dma, DMA_ATTR_BUFFER_ALIGNMENT, &align); + if (err < 0 || !align) { + trace_dai_error_with_ids(dev, "dai_params() error: could not " + "get valid dma buffer alignment, err = %d, " + "align = %u", err, align); return -EINVAL; } - if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) { - dd->dma_buffer = list_first_item(&dev->bsource_list, - struct comp_buffer, - sink_list); - dd->dma_buffer->r_ptr = dd->dma_buffer->addr; + err = dma_get_attribute(dd->dma, DMA_ATTR_BUFFER_PERIOD_COUNT, + &period_count); + if (err < 0 || !period_count) { + trace_dai_error_with_ids(dev, "dai_params() error: could not " + "get valid dma buffer period count, " + "err = %d, period_count = %u", err, + period_count); + return -EINVAL; + } - return dai_playback_params(dev, period_bytes); + period_bytes = dev->frames * dd->frame_bytes; + if (!period_bytes) { + trace_dai_error_with_ids(dev, "dai_params() error: invalid " + "period_bytes."); + return -EINVAL; } - dd->dma_buffer = list_first_item(&dev->bsink_list, - struct comp_buffer, source_list); - dd->dma_buffer->w_ptr = dd->dma_buffer->addr; + /* calculate DMA buffer size */ + buffer_size = ALIGN_UP(period_count * period_bytes, align); - return dai_capture_params(dev, period_bytes); + /* alloc DMA buffer or change its size if exists */ + if (dd->dma_buffer) { + err = buffer_set_size(dd->dma_buffer, buffer_size); + if (err < 0) { + trace_dai_error_with_ids(dev, "dai_params() error: " + "buffer_set_size() failed, " + "buffer_size = %u", + buffer_size); + return err; + } + } else { + dd->dma_buffer = buffer_alloc(buffer_size, SOF_MEM_CAPS_DMA, + addr_align); + if (!dd->dma_buffer) { + trace_dai_error_with_ids(dev, "dai_params() error: " + "failed to alloc dma buffer"); + return -ENOMEM; + } + } + return dev->params.direction == SOF_IPC_STREAM_PLAYBACK ? + dai_playback_params(dev, period_bytes, period_count) : + dai_capture_params(dev, period_bytes, period_count); } static int dai_prepare(struct comp_dev *dev) @@ -474,6 +484,11 @@ static int dai_reset(struct comp_dev *dev) dma_sg_free(&config->elem_array); + if (dd->dma_buffer) { + buffer_free(dd->dma_buffer); + dd->dma_buffer = NULL; + } + dd->dai_pos_blks = 0; if (dd->dai_pos) *dd->dai_pos = 0; @@ -580,6 +595,7 @@ static int dai_comp_trigger(struct comp_dev *dev, int cmd) static int dai_check_for_xrun(struct comp_dev *dev, uint32_t copy_bytes) { struct dai_data *dd = comp_get_drvdata(dev); + struct comp_buffer *local_buffer; /* data available for copy */ if (copy_bytes) @@ -594,12 +610,16 @@ static int dai_check_for_xrun(struct comp_dev *dev, uint32_t copy_bytes) trace_dai_error_with_ids(dev, "dai_check_for_xrun() " "error: underrun due to no data " "available"); - comp_underrun(dev, dd->dma_buffer, copy_bytes); + local_buffer = list_first_item(&dev->bsource_list, + struct comp_buffer, sink_list); + comp_underrun(dev, local_buffer, copy_bytes); } else { trace_dai_error_with_ids(dev, "dai_check_for_xrun() " "error: overrun due to no data " "available"); - comp_overrun(dev, dd->dma_buffer, copy_bytes); + local_buffer = list_first_item(&dev->bsink_list, + struct comp_buffer, source_list); + comp_overrun(dev, local_buffer, copy_bytes); } return -ENODATA; @@ -609,6 +629,7 @@ static int dai_check_for_xrun(struct comp_dev *dev, uint32_t copy_bytes) static int dai_copy(struct comp_dev *dev) { struct dai_data *dd = comp_get_drvdata(dev); + struct comp_buffer *local_buffer; uint32_t avail_bytes = 0; uint32_t free_bytes = 0; uint32_t copy_bytes = 0; @@ -635,9 +656,15 @@ static int dai_copy(struct comp_dev *dev) return ret; /* calculate minimum size to copy */ - copy_bytes = dev->params.direction == SOF_IPC_STREAM_PLAYBACK ? - MIN(dd->dma_buffer->avail, free_bytes) : - MIN(avail_bytes, dd->dma_buffer->free); + if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) { + local_buffer = list_first_item(&dev->bsource_list, + struct comp_buffer, sink_list); + copy_bytes = MIN(local_buffer->avail, free_bytes); + } else { + local_buffer = list_first_item(&dev->bsink_list, + struct comp_buffer, source_list); + copy_bytes = MIN(avail_bytes, local_buffer->free); + } tracev_dai_with_ids(dev, "dai_copy(), copy_bytes = 0x%x", copy_bytes); @@ -818,7 +845,6 @@ static int dai_config(struct comp_dev *dev, struct sof_ipc_dai_config *config) /* set up callback */ dma_set_cb(dd->chan, DMA_CB_TYPE_COPY, dai_dma_cb, dev); - dev->is_dma_connected = 1; } return 0; diff --git a/src/audio/host.c b/src/audio/host.c index 4a291ab7dda7..2d8cdb3e6dd6 100644 --- a/src/audio/host.c +++ b/src/audio/host.c @@ -77,7 +77,6 @@ struct host_data { struct dma_chan_data *chan; struct dma_sg_config config; struct comp_buffer *dma_buffer; - uint32_t period_bytes; /**< Size of a single period (in bytes) */ /* host position reporting related */ uint32_t host_size; /**< Host buffer size (in bytes) */ @@ -95,13 +94,14 @@ struct host_data { struct hc_buf *source; struct hc_buf *sink; - /* helper used in split transactions */ - uint32_t split_value; - uint32_t dma_copy_align; /**< Minimal chunk of data possible to be * copied by dma connected to host */ + /* processing function */ + void (*process)(struct comp_buffer *source, struct comp_buffer *sink, + uint32_t bytes); + /* stream info */ struct sof_ipc_stream_posn posn; /* TODO: update this */ }; @@ -136,11 +136,19 @@ static uint32_t host_dma_get_split(struct host_data *hd, uint32_t bytes) static void host_update_position(struct comp_dev *dev, uint32_t bytes) { struct host_data *hd = comp_get_drvdata(dev); + struct comp_buffer *local_buffer; - if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) - comp_update_buffer_produce(hd->dma_buffer, bytes); - else - comp_update_buffer_consume(hd->dma_buffer, bytes); + if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) { + local_buffer = list_first_item(&dev->bsink_list, + struct comp_buffer, source_list); + dma_buffer_copy_from(hd->dma_buffer, local_buffer, hd->process, + bytes); + } else { + local_buffer = list_first_item(&dev->bsource_list, + struct comp_buffer, sink_list); + dma_buffer_copy_to(local_buffer, hd->dma_buffer, hd->process, + bytes); + } dev->position += bytes; @@ -291,10 +299,10 @@ static int host_trigger(struct comp_dev *dev, int cmd) return ret; } - /* we should ignore any trigger commands when doing one shot, - * because transfers will start in copy and stop automatically + /* we should ignore any trigger commands besides start + * when doing one shot, because transfers will stop automatically */ - if (hd->copy_type == COMP_COPY_ONE_SHOT) + if (cmd != COMP_TRIGGER_START && hd->copy_type == COMP_COPY_ONE_SHOT) return ret; if (!hd->chan) { @@ -378,7 +386,6 @@ static struct comp_dev *host_new(struct sof_ipc_comp *comp) hd->copy_type = COMP_COPY_NORMAL; hd->posn.comp_id = comp->id; dev->state = COMP_STATE_READY; - dev->is_dma_connected = 1; return dev; } @@ -430,122 +437,15 @@ static int host_elements_reset(struct comp_dev *dev) return 0; } -static uint32_t host_buffer_get_copy_bytes(struct comp_dev *dev) -{ - struct host_data *hd = comp_get_drvdata(dev); - struct dma_sg_elem *local_elem = hd->config.elem_array.elems; - uint32_t avail_bytes = 0; - uint32_t free_bytes = 0; - uint32_t copy_bytes = 0; - int ret; - - if (hd->copy_type == COMP_COPY_ONE_SHOT) { - /* calculate minimum size to copy */ - copy_bytes = dev->params.direction == SOF_IPC_STREAM_PLAYBACK ? - hd->dma_buffer->free : hd->dma_buffer->avail; - - /* copy_bytes should be aligned to minimum possible chunk of - * data to be copied by dma. - */ - copy_bytes = ALIGN_DOWN(copy_bytes, hd->dma_copy_align); - - hd->split_value = host_dma_get_split(hd, copy_bytes); - if (hd->split_value) - copy_bytes -= hd->split_value; - - local_elem->size = copy_bytes; - } else { - /* get data sizes from DMA */ - ret = dma_get_data_size(hd->chan, &avail_bytes, - &free_bytes); - if (ret < 0) { - trace_host_error("host_buffer_cb() error: " - "dma_get_data_size() failed, ret = %u", - ret); - return 0; - } - - /* calculate minimum size to copy */ - copy_bytes = dev->params.direction == SOF_IPC_STREAM_PLAYBACK ? - MIN(avail_bytes, hd->dma_buffer->free) : - MIN(hd->dma_buffer->avail, free_bytes); - - /* copy_bytes should be aligned to minimum possible chunk of - * data to be copied by dma. - */ - copy_bytes = ALIGN_DOWN(copy_bytes, hd->dma_copy_align); - } - - return copy_bytes; -} - -static void host_buffer_cb(void *data, uint32_t bytes) -{ - struct comp_dev *dev = (struct comp_dev *)data; - struct host_data *hd = comp_get_drvdata(dev); - uint32_t copy_bytes = 0; - uint32_t flags = 0; - int ret; - - tracev_host("host_buffer_cb(), copy_bytes = 0x%x", copy_bytes); - - if (hd->copy_type == COMP_COPY_BLOCKING) - flags |= DMA_COPY_BLOCKING; - else if (hd->copy_type == COMP_COPY_ONE_SHOT) - flags |= DMA_COPY_ONE_SHOT; - - /* copy including all split transfers */ - do { - copy_bytes = host_buffer_get_copy_bytes(dev); - - /* reconfigure transfer */ - ret = dma_set_config(hd->chan, &hd->config); - if (ret < 0) { - trace_host_error("host_buffer_cb() error: " - "dma_set_config() failed, ret = %u", - ret); - hd->split_value = 0; - return; - } - - ret = dma_copy(hd->chan, copy_bytes, flags); - if (ret < 0) { - trace_host_error("host_buffer_cb() error: dma_copy() " - "failed, ret = %u", ret); - hd->split_value = 0; - return; - } - } while (hd->split_value); -} - -static void realign_buffer(struct host_data *hd) -{ - uint32_t buffer_alignment; - - dma_get_attribute(hd->dma, DMA_ATTR_BUFFER_ADDRESS_ALIGNMENT, - &buffer_alignment); - - if ((uintptr_t)hd->dma_buffer->addr % buffer_alignment) { - hd->dma_buffer->addr = - rbrealloc_align(hd->dma_buffer->addr, RZONE_BUFFER, - SOF_MEM_CAPS_DMA, - hd->dma_buffer->alloc_size, - buffer_alignment); - buffer_init(hd->dma_buffer, hd->dma_buffer->alloc_size, - SOF_MEM_CAPS_DMA); - } -} - /* configure the DMA params and descriptors for host buffer IO */ static int host_params(struct comp_dev *dev) { struct host_data *hd = comp_get_drvdata(dev); - struct sof_ipc_comp_config *cconfig = COMP_GET_CONFIG(dev); struct dma_sg_config *config = &hd->config; - uint32_t buffer_size; - uint32_t buffer_count; - uint32_t buffer_single_size; uint32_t period_count; + uint32_t period_bytes; + uint32_t buffer_size; + uint32_t addr_align; uint32_t align; int err; @@ -554,91 +454,85 @@ static int host_params(struct comp_dev *dev) /* host params always installed by pipeline IPC */ hd->host_size = dev->params.buffer.size; - err = dma_get_attribute(hd->dma, DMA_ATTR_BUFFER_ALIGNMENT, &align); + /* retrieve DMA buffer address alignment */ + err = dma_get_attribute(hd->dma, DMA_ATTR_BUFFER_ADDRESS_ALIGNMENT, + &addr_align); if (err < 0) { - trace_host_error_with_ids(dev, "could not get dma buffer " - "alignment"); + trace_host_error_with_ids(dev, "host_params() error: could not " + "get dma buffer address alignment, " + "err = %d", err); return err; } - /* determine source and sink buffer elems */ - if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) { - hd->dma_buffer = list_first_item(&dev->bsink_list, - struct comp_buffer, source_list); + /* retrieve DMA buffer size alignment */ + err = dma_get_attribute(hd->dma, DMA_ATTR_BUFFER_ALIGNMENT, &align); + if (err < 0 || !align) { + trace_host_error_with_ids(dev, "host_params() error: could not " + "get valid dma buffer alignment, err " + "= %d, align = %u", err, align); + return -EINVAL; + } - realign_buffer(hd); + /* retrieve DMA buffer period count */ + err = dma_get_attribute(hd->dma, DMA_ATTR_BUFFER_PERIOD_COUNT, + &period_count); + if (err < 0 || !period_count) { + trace_host_error_with_ids(dev, "host_params() error: could not " + "get valid dma buffer period count, " + "err = %d, period_count = %u", err, + period_count); + return -EINVAL; + } - /* set callback on buffer consume */ - buffer_set_cb(hd->dma_buffer, &host_buffer_cb, dev, - BUFF_CB_TYPE_CONSUME); + period_bytes = dev->frames * comp_frame_bytes(dev); + if (!period_bytes) { + trace_host_error_with_ids(dev, "host_params() error: invalid " + "period_bytes"); + return -EINVAL; + } + /* determine source and sink buffer elements */ + if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) { config->direction = DMA_DIR_HMEM_TO_LMEM; - - period_count = cconfig->periods_sink; - - buffer_count = hd->host.elem_array.count ? 1 : period_count; - buffer_single_size = ALIGN_UP(dev->frames * - comp_frame_bytes(dev), - align); - - if (hd->host.elem_array.count) - buffer_single_size *= period_count; - hd->source = &hd->host; hd->sink = &hd->local; } else { - hd->dma_buffer = list_first_item(&dev->bsource_list, - struct comp_buffer, sink_list); - - realign_buffer(hd); - - /* set callback on buffer produce */ - buffer_set_cb(hd->dma_buffer, &host_buffer_cb, dev, - BUFF_CB_TYPE_PRODUCE); - config->direction = DMA_DIR_LMEM_TO_HMEM; - - period_count = cconfig->periods_source; - - buffer_count = hd->host.elem_array.count ? 1 : period_count; - buffer_single_size = ALIGN_UP(dev->frames * - comp_frame_bytes(dev), - align); - - if (hd->host.elem_array.count) - buffer_single_size *= period_count; - hd->source = &hd->local; hd->sink = &hd->host; } - /* validate period count */ - if (!period_count) { - trace_host_error_with_ids(dev, "host_params() error: invalid " - "period_count"); - return -EINVAL; + /* TODO: should be taken from DMA */ + if (hd->host.elem_array.count) { + period_bytes *= period_count; + period_count = 1; } - hd->period_bytes = ALIGN_UP(dev->frames * comp_frame_bytes(dev), align); - - if (hd->period_bytes == 0) { - trace_host_error_with_ids(dev, "host_params() error: invalid " - "period_bytes"); - return -EINVAL; - } + /* calculate DMA buffer size */ + buffer_size = ALIGN_UP(period_count * period_bytes, align); - /* resize the buffer if space is available to align with period size */ - buffer_size = period_count * hd->period_bytes; - err = buffer_set_size(hd->dma_buffer, buffer_size); - if (err < 0) { - trace_host_error_with_ids(dev, "host_params() error:" - "buffer_set_size() failed, " - "buffer_size = %u", buffer_size); - return err; + /* alloc DMA buffer or change its size if exists */ + if (hd->dma_buffer) { + err = buffer_set_size(hd->dma_buffer, buffer_size); + if (err < 0) { + trace_host_error_with_ids(dev, "host_params() error: " + "buffer_set_size() failed, " + "buffer_size = %u", + buffer_size); + return err; + } + } else { + hd->dma_buffer = buffer_alloc(buffer_size, SOF_MEM_CAPS_DMA, + addr_align); + if (!hd->dma_buffer) { + trace_host_error_with_ids(dev, "host_params() error: " + "failed to alloc dma buffer"); + return -ENOMEM; + } } /* create SG DMA elems for local DMA buffer */ - err = create_local_elems(dev, buffer_count, buffer_single_size); + err = create_local_elems(dev, period_count, buffer_size / period_count); if (err < 0) return err; @@ -684,6 +578,12 @@ static int host_params(struct comp_dev *dev) /* set up callback */ dma_set_cb(hd->chan, DMA_CB_TYPE_COPY, host_dma_cb, dev); + /* set processing function */ + if (dev->params.frame_fmt == SOF_IPC_FRAME_S16_LE) + hd->process = buffer_copy_s16; + else + hd->process = buffer_copy_s32; + return 0; } @@ -703,7 +603,6 @@ static int host_prepare(struct comp_dev *dev) hd->local_pos = 0; hd->report_pos = 0; - hd->split_value = 0; dev->position = 0; return 0; @@ -746,6 +645,12 @@ static int host_reset(struct comp_dev *dev) dma_sg_free(&hd->local.elem_array); dma_sg_free(&hd->config.elem_array); + /* free DMA buffer */ + if (hd->dma_buffer) { + buffer_free(hd->dma_buffer); + hd->dma_buffer = NULL; + } + /* reset dma channel as we have put it */ hd->chan = NULL; @@ -758,10 +663,80 @@ static int host_reset(struct comp_dev *dev) return 0; } +static uint32_t host_buffer_get_copy_bytes(struct comp_dev *dev) +{ + struct host_data *hd = comp_get_drvdata(dev); + struct dma_sg_elem *local_elem = hd->config.elem_array.elems; + struct comp_buffer *local_buffer; + uint32_t avail_bytes = 0; + uint32_t free_bytes = 0; + uint32_t copy_bytes = 0; + uint32_t split_value; + int ret; + + if (hd->copy_type == COMP_COPY_ONE_SHOT) { + /* calculate minimum size to copy */ + if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) { + local_buffer = list_first_item(&dev->bsink_list, + struct comp_buffer, + source_list); + copy_bytes = local_buffer->free; + } else { + local_buffer = list_first_item(&dev->bsource_list, + struct comp_buffer, + sink_list); + copy_bytes = local_buffer->avail; + } + + /* copy_bytes should be aligned to minimum possible chunk of + * data to be copied by dma. + */ + copy_bytes = ALIGN_DOWN(copy_bytes, hd->dma_copy_align); + + split_value = host_dma_get_split(hd, copy_bytes); + if (split_value) + copy_bytes -= split_value; + + local_elem->size = copy_bytes; + } else { + /* get data sizes from DMA */ + ret = dma_get_data_size(hd->chan, &avail_bytes, + &free_bytes); + if (ret < 0) { + trace_host_error("host_buffer_cb() error: " + "dma_get_data_size() failed, ret = %u", + ret); + return 0; + } + + /* calculate minimum size to copy */ + if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) { + local_buffer = list_first_item(&dev->bsink_list, + struct comp_buffer, + source_list); + copy_bytes = MIN(avail_bytes, local_buffer->free); + } else { + local_buffer = list_first_item(&dev->bsource_list, + struct comp_buffer, + sink_list); + copy_bytes = MIN(local_buffer->avail, free_bytes); + } + + /* copy_bytes should be aligned to minimum possible chunk of + * data to be copied by dma. + */ + copy_bytes = ALIGN_DOWN(copy_bytes, hd->dma_copy_align); + } + + return copy_bytes; +} + /* copy and process stream data from source to sink buffers */ static int host_copy(struct comp_dev *dev) { struct host_data *hd = comp_get_drvdata(dev); + uint32_t copy_bytes = 0; + uint32_t flags = 0; int ret = 0; tracev_host_with_ids(dev, "host_copy()"); @@ -769,26 +744,35 @@ static int host_copy(struct comp_dev *dev) if (dev->state != COMP_STATE_ACTIVE) return 0; - /* here only do preload, further copies happen - * in host_buffer_cb() - */ - if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK && - !dev->position) { - ret = dma_copy(hd->chan, hd->dma_buffer->size, - DMA_COPY_PRELOAD); - if (ret < 0) { - if (ret == -ENODATA) { - /* preload not finished, so stop processing */ - trace_host_with_ids(dev, "host_copy(), preload" - " not yet finished"); - return PPL_STATUS_PATH_STOP; - } - - trace_host_error_with_ids(dev, "host_copy() error: " - "dma_copy() failed, " - "ret = %u", ret); - return ret; - } + if (hd->copy_type == COMP_COPY_BLOCKING) + flags |= DMA_COPY_BLOCKING; + else if (hd->copy_type == COMP_COPY_ONE_SHOT) + flags |= DMA_COPY_ONE_SHOT; + + /* update first transfer manually */ + if (!dev->position && flags & COMP_COPY_ONE_SHOT) + host_one_shot_cb(dev, hd->dma_buffer->size); + + copy_bytes = host_buffer_get_copy_bytes(dev); + if (!copy_bytes) { + trace_host_with_ids(dev, "host_copy(): no bytes to copy"); + return ret; + } + + /* reconfigure transfer */ + ret = dma_set_config(hd->chan, &hd->config); + if (ret < 0) { + trace_host_error("host_copy() error: " + "dma_set_config() failed, ret = %u", + ret); + return ret; + } + + ret = dma_copy(hd->chan, copy_bytes, flags); + if (ret < 0) { + trace_host_error("host_copy() error: dma_copy() " + "failed, ret = %u", ret); + return ret; } return ret; diff --git a/src/audio/kpb.c b/src/audio/kpb.c index 23b7115b596b..8ca6c95b4927 100644 --- a/src/audio/kpb.c +++ b/src/audio/kpb.c @@ -625,7 +625,6 @@ static int kpb_copy(struct comp_dev *dev) comp_update_buffer_produce(sink, copy_bytes); comp_update_buffer_consume(source, copy_bytes); - ret = PPL_STATUS_PATH_STOP; break; case KPB_STATE_DRAINING: /* In draining state we only buffer data in internal, @@ -1098,8 +1097,10 @@ static enum task_state kpb_draining_task(void *arg) move_buffer = false; } - if (size_to_copy) + if (size_to_copy) { comp_update_buffer_produce(sink, size_to_copy); + comp_copy(sink->sink); + } if (sync_mode_on && period_bytes >= period_bytes_limit) { current_time = platform_timer_get(platform_timer); diff --git a/src/audio/pipeline.c b/src/audio/pipeline.c index 1495ab473e25..4aa7196502bc 100644 --- a/src/audio/pipeline.c +++ b/src/audio/pipeline.c @@ -421,11 +421,7 @@ int pipeline_prepare(struct pipeline *p, struct comp_dev *dev) return ret; } - /* pipeline preload needed only for playback streams without active - * sink component (it can be active for e.g. mixer pipelines) - */ - p->preload = dev->params.direction == SOF_IPC_STREAM_PLAYBACK && - p->sink_comp->state != COMP_STATE_ACTIVE; + p->preload = false; /* initialize task if necessary */ if (!p->pipe_task) { @@ -817,21 +813,6 @@ static int pipeline_copy(struct pipeline *p) if (p->source_comp->params.direction == SOF_IPC_STREAM_PLAYBACK) { dir = PPL_DIR_UPSTREAM; start = p->sink_comp; - - /* if not pipeline preload then copy sink comp first */ - if (!p->preload) { - ret = comp_copy(start); - if (ret < 0) { - trace_pipe_error("pipeline_copy() error: " - "ret = %d", ret); - return ret; - } - - start = comp_get_previous(start, dir); - if (!start) - /* nothing else to do */ - return ret; - } } else { dir = PPL_DIR_DOWNSTREAM; start = p->source_comp; diff --git a/src/drivers/dw/dma.c b/src/drivers/dw/dma.c index 19d931b4ee60..0c51c947dc86 100644 --- a/src/drivers/dw/dma.c +++ b/src/drivers/dw/dma.c @@ -846,12 +846,21 @@ static int dw_dma_copy(struct dma_chan_data *channel, int bytes, .status = DMA_CB_STATUS_END }; - /* for preload and one shot copy just start the DMA and wait */ - if (flags & (DMA_COPY_PRELOAD | DMA_COPY_ONE_SHOT)) { + tracev_dwdma("dw_dma_copy(): dma %d channel %d copy", + channel->dma->plat_data.id, channel->index); + + if (channel->cb && channel->cb_type & DMA_CB_TYPE_COPY) + channel->cb(channel->cb_data, DMA_CB_TYPE_COPY, &next); + + if (flags & DMA_COPY_ONE_SHOT) { + /* for one shot copy start the DMA */ ret = dw_dma_start(channel); if (ret < 0) return ret; + } + if (flags & DMA_COPY_BLOCKING) { + /* wait for transfer finish */ ret = poll_for_register_delay(dma_base(channel->dma) + DW_DMA_CHAN_EN, DW_CHAN(channel->index), 0, @@ -860,12 +869,6 @@ static int dw_dma_copy(struct dma_chan_data *channel, int bytes, return ret; } - tracev_dwdma("dw_dma_copy(): dma %d channel %d copy", - channel->dma->plat_data.id, channel->index); - - if (channel->cb && channel->cb_type & DMA_CB_TYPE_COPY) - channel->cb(channel->cb_data, DMA_CB_TYPE_COPY, &next); - dw_dma_verify_transfer(channel, &next); /* increment current pointer */ diff --git a/src/drivers/intel/cavs/hda-dma.c b/src/drivers/intel/cavs/hda-dma.c index 6c79fc77604f..bdc3be00cd24 100644 --- a/src/drivers/intel/cavs/hda-dma.c +++ b/src/drivers/intel/cavs/hda-dma.c @@ -274,10 +274,22 @@ static void hda_dma_post_copy(struct dma_chan_data *chan, int bytes) if (chan->cb) chan->cb(chan->cb_data, DMA_CB_TYPE_COPY, &next); - /* Force Host DMA to exit L1 */ if (chan->direction == DMA_DIR_HMEM_TO_LMEM || - chan->direction == DMA_DIR_LMEM_TO_HMEM) + chan->direction == DMA_DIR_LMEM_TO_HMEM) { + /* set BFPI to let host gateway know we have read size, + * which will trigger next copy start. + */ + hda_dma_inc_fp(chan, bytes); + + /* Force Host DMA to exit L1 */ pm_runtime_put(PM_RUNTIME_HOST_DMA_L1, 0); + } else { + /* + * set BFPI to let link gateway know we have read size, + * which will trigger next copy start. + */ + hda_dma_inc_link_fp(chan, bytes); + } } static int hda_dma_link_copy_ch(struct dma_chan_data *chan, int bytes) @@ -295,11 +307,6 @@ static int hda_dma_link_copy_ch(struct dma_chan_data *chan, int bytes) if (dgcs & DGCS_BOR) dma_chan_reg_update_bits(chan, DGCS, DGCS_BOR, DGCS_BOR); - /* - * set BFPI to let link gateway know we have read size, - * which will trigger next copy start. - */ - hda_dma_inc_link_fp(chan, bytes); hda_dma_post_copy(chan, bytes); hda_dma_get_dbg_vals(chan, HDA_DBG_POST, HDA_DBG_LINK); @@ -367,11 +374,6 @@ static int hda_dma_host_copy(struct dma_chan_data *channel, int bytes, hda_dma_is_buffer_empty(channel); if (!ret) return -ENODATA; - } else { - /* set BFPI to let host gateway know we have read size, - * which will trigger next copy start. - */ - hda_dma_inc_fp(channel, bytes); } /* blocking mode copy */ diff --git a/src/ipc/dma-copy.c b/src/ipc/dma-copy.c index c6f058c0c2a0..619e7919fffe 100644 --- a/src/ipc/dma-copy.c +++ b/src/ipc/dma-copy.c @@ -112,7 +112,8 @@ int dma_copy_to_host_nowait(struct dma_copy *dc, struct dma_sg_config *host_sg, if (err < 0) return err; - err = dma_copy(dc->chan, local_sg_elem.size, DMA_COPY_ONE_SHOT); + err = dma_copy(dc->chan, local_sg_elem.size, + DMA_COPY_ONE_SHOT | DMA_COPY_BLOCKING); if (err < 0) return err; diff --git a/src/ipc/ipc-host-ptable.c b/src/ipc/ipc-host-ptable.c index 7b30cff8bca6..1b7c8b490c9a 100644 --- a/src/ipc/ipc-host-ptable.c +++ b/src/ipc/ipc-host-ptable.c @@ -120,7 +120,7 @@ static int ipc_get_page_descriptors(struct dma *dmac, uint8_t *page_table, } /* start the copy of page table to DSP */ - ret = dma_copy(chan, elem.size, DMA_COPY_ONE_SHOT); + ret = dma_copy(chan, elem.size, DMA_COPY_ONE_SHOT | DMA_COPY_BLOCKING); if (ret < 0) { trace_ipc_error("ipc_get_page_descriptors() error: " "dma_start() failed"); diff --git a/src/lib/dma.c b/src/lib/dma.c index 193c319a3176..a24da56a4905 100644 --- a/src/lib/dma.c +++ b/src/lib/dma.c @@ -242,7 +242,7 @@ void dma_buffer_copy_to(struct comp_buffer *source, struct comp_buffer *sink, sink->w_ptr += bytes; /* check for pointer wrap */ - if (source->r_ptr >= sink->end_addr) + if (sink->w_ptr >= sink->end_addr) sink->w_ptr = sink->addr + (sink->w_ptr - sink->end_addr);