On Fri, 2014-02-14 at 18:06 +0100, Takashi Iwai wrote:
At Fri, 14 Feb 2014 16:11:15 +0000, Liam Girdwood wrote:
On Fri, 2014-02-14 at 09:38 +0000, Liam Girdwood wrote:
On Fri, 2014-02-14 at 09:55 +0100, Takashi Iwai wrote:
At Thu, 13 Feb 2014 19:15:28 +0000, Liam Girdwood wrote:
+/* allocate contiguous free DSP blocks - callers hold locks */ +static int block_alloc_contiguous(struct sst_module *module,
- struct sst_module_data *data, u32 next_offset, int size)
+{
- struct sst_dsp *dsp = module->dsp;
- struct sst_mem_block *block, *tmp;
- int ret;
- /* find first free blocks that can hold module */
- list_for_each_entry_safe(block, tmp, &dsp->free_block_list, list) {
/* ignore blocks that dont match type */if (block->type != data->type)continue;/* is block next after parent ? */if (next_offset == block->offset) {/* do we need more blocks */if (size > block->size) {ret = block_alloc_contiguous(module,data, block->offset + block->size,size - block->size);if (ret < 0)return ret;How many contiguous blocks can be?
In theory, the whole DSP memory can be allocated as a contiguous block, but in practice it's only really a few blocks contiguous per module atm.
The recursive call for each one block doesn't look scaling.
I've double checked the logic here and added more comments and debug. I may be missing something though, but it does work and is designed to deal with unordered blocks (on address) in the list.
The recursion basically checks the whole list each time for the next block in address order. It then checks if the required size > current block size. We decrement size on each by block size and increment address by block size on every call. If the required size < current block size we allocate the block and come back up the stack allocating the other blocks in the sequence.
From the debug we get :-
[ 6.733916] haswell-pcm-audio haswell-pcm-audio: module 0 added block 0:4 at offset 0xc0000 [ 6.733925] haswell-pcm-audio haswell-pcm-audio: module 0 added block 0:3 at offset 0xb8000 [ 6.733930] haswell-pcm-audio haswell-pcm-audio: module 0 added block 0:2 at offset 0xb0000 [ 6.733936] haswell-pcm-audio haswell-pcm-audio: module 0 added block 0:1 at offset 0xa8000 [ 6.810179] haswell-pcm-audio haswell-pcm-audio: module 0 added block 1:1 at offset 0x88000 [ 6.810189] haswell-pcm-audio haswell-pcm-audio: module 0 added block 1:0 at offset 0x80000 [ 6.857452] haswell-pcm-audio haswell-pcm-audio: scratch buffer required is 56320 bytes [ 6.857461] haswell-pcm-audio haswell-pcm-audio: allocating scratch blocks [ 6.857469] haswell-pcm-audio haswell-pcm-audio: module 0 added block 1:6 at offset 0x70000 [ 6.860268] haswell-pcm-audio haswell-pcm-audio: FW loaded: type 172 - version: 0.0 build 1
Well, the code is a bit confusing because of the comment:
I agree, and I had updated the comments for V2.
static int block_alloc_contiguous(struct sst_module *module, struct sst_module_data *data, u32 next_offset, int size) { ..... /* find first free blocks that can hold module */ list_for_each_entry_safe(block, tmp, &dsp->free_block_list, list) {
Actually, this loop doesn't look for the first free blocks but rather looks for the block that matches with the given type and the offset.
/* ignore blocks that dont match type */ if (block->type != data->type) continue; /* is block next after parent ? */ if (next_offset == block->offset) {Then, it tries allocating the next block if the size of the found block isn't big enough:
/* do we need more blocks */ if (size > block->size) { ret = block_alloc_contiguous(module, data, block->offset + block->size, size - block->size); if (ret < 0) return ret;So, here is the recursion, and it means that at maximum (size / block_size) level recursions may happen.
My point is that this doesn't have to be recursion. It can be implemented via a flat loop as simply as well. For example, a code like below would work, too.
struct block *find_block(int type, int offset) { struct block *block; list_for_each_entry(block, &free_blocks, list) { if (block->type == type && block->offset == offset) return block; } return NULL; }
int alloc_contiguous(int type, int offset, int size) { struct list_head tmp = LIST_HEAD_INIT(tmp); struct block *block;
while (size > 0) { block = find_block(type, offset); if (!block) { list_splice(&tmp, &free_list); return -ENOMEM; } list_move_tail(&block->list, &tmp); offset += block->size; size -= block->size; }
list_for_each_entry(block, &tmp, list) { block->data_type = xxx; .... } list_splice(&tmp, &used_list); return 0; }
BTW, this is still suboptimal; we can remember the last failed point and rescan from there at next.
Or, if you manage the sorted list (tree) instead of a plain list, things will be also easier, I suppose.
Agree it would be faster if ordered, but this code will be infrequently used so the emphasis was more on ease of development/testing. I'll remove the recursion for V2.
Thanks
Liam