Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 367
Heap refinement Part3 -- make full use of HP SRAM banks#4521
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
7200bd51b0757e1a7578e0a63c6bab12e1dda8215a781cd4395f33b5bb7c772File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| +1 −1 | config/kbl.toml | |
| +1 −1 | config/skl.toml | |
| +1 −1 | config/sue.toml | |
| +1 −1 | config/tgl-cavs.toml |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -31,21 +31,20 @@ struct comp_buffer *buffer_alloc(uint32_t size, uint32_t caps, uint32_t align) | ||
| tr_dbg(&buffer_tr, "buffer_alloc()"); | ||
| /* validate request */ | ||
| if (size == 0 || size > HEAP_BUFFER_SIZE) { | ||
keyonjie marked this conversation as resolved.
Outdated
Uh oh!There was an error while loading. Please reload this page. | ||
| if (size == 0) { | ||
| tr_err(&buffer_tr, "buffer_alloc(): new size = %u is invalid", | ||
| size); | ||
| return NULL; | ||
| } | ||
| /* allocate new buffer */ | ||
| buffer = rzalloc(SOF_MEM_ZONE_RUNTIME_SHARED, 0, SOF_MEM_CAPS_RAM, | ||
| sizeof(*buffer)); | ||
| buffer = rballoc(SOF_MEM_FLAG_COHERENT, SOF_MEM_CAPS_RAM, sizeof(*buffer)); | ||
| if (!buffer) { | ||
| tr_err(&buffer_tr, "buffer_alloc(): could not alloc structure"); | ||
| return NULL; | ||
| } | ||
| buffer->lock = rzalloc(SOF_MEM_ZONE_RUNTIME_SHARED, 0, SOF_MEM_CAPS_RAM, | ||
| buffer->lock = rballoc(SOF_MEM_FLAG_COHERENT, SOF_MEM_CAPS_RAM, | ||
| sizeof(*buffer->lock)); | ||
| if (!buffer->lock) { | ||
| rfree(buffer); | ||
| @@ -85,7 +84,7 @@ int buffer_set_size(struct comp_buffer *buffer, uint32_t size) | ||
| void *new_ptr = NULL; | ||
| /* validate request */ | ||
| if (size == 0 || size > HEAP_BUFFER_SIZE) { | ||
| if (size == 0) { | ||
| buf_err(buffer, "resize size = %u is invalid", size); | ||
| return -EINVAL; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -173,7 +173,8 @@ static struct comp_dev *mux_new(const struct comp_driver *drv, | ||
| return NULL; | ||
| dev->ipc_config = *config; | ||
| cd = rzalloc(SOF_MEM_ZONE_RUNTIME, 0, SOF_MEM_CAPS_RAM, | ||
| /* allocate quite a big buffer, use rballoc to make sure it will succeed */ | ||
| cd = rballoc(0, SOF_MEM_CAPS_RAM, | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see why we special-case a component? something's fishy here. why not the mixer as well then?
| ||
| sizeof(*cd) + MUX_MAX_STREAMS * sizeof(struct mux_stream_data)); | ||
| if (!cd) { | ||
| rfree(dev); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -437,9 +437,6 @@ static int dw_dma_stop(struct dma_chan_data *channel) | ||
| lli->ctrl_hi &= ~DW_CTLH_DONE(1); | ||
| lli++; | ||
| } | ||
| dcache_writeback_region(dw_chan->lli, | ||
| sizeof(struct dw_lli) * channel->desc_count); | ||
| #endif | ||
| /* disable linear link position */ | ||
| @@ -557,9 +554,9 @@ static int dw_dma_set_config(struct dma_chan_data *channel, | ||
| if (dw_chan->lli) | ||
| rfree(dw_chan->lli); | ||
| dw_chan->lli = rballoc_align(0, SOF_MEM_CAPS_RAM | SOF_MEM_CAPS_DMA, | ||
| sizeof(struct dw_lli) * channel->desc_count, | ||
| PLATFORM_DCACHE_ALIGN); | ||
| dw_chan->lli = rballoc(SOF_MEM_FLAG_COHERENT, | ||
| SOF_MEM_CAPS_RAM | SOF_MEM_CAPS_DMA, | ||
| sizeof(struct dw_lli) * channel->desc_count); | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. again separate PR? This is unrelated to the allocation size, you're just changing how the LLI is handled. Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like the rebase was on the wrong branch. These are already in @keyonjie other PRs ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ah, yes, let me put them to PR #4664 Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. alignment? ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
the rballoc() internal will use default PLATFORM_DCACHE_ALIGN alignment, so no need to specify it explicitly. | ||
| if (!dw_chan->lli) { | ||
| tr_err(&dwdma_tr, "dw_dma_set_config(): dma %d channel %d lli alloc failed", | ||
| channel->dma->plat_data.id, | ||
| @@ -767,10 +764,6 @@ static int dw_dma_set_config(struct dma_chan_data *channel, | ||
| #endif | ||
| } | ||
| /* write back descriptors so DMA engine can read them directly */ | ||
| dcache_writeback_region(dw_chan->lli, | ||
| sizeof(struct dw_lli) * channel->desc_count); | ||
| channel->status = COMP_STATE_PREPARE; | ||
| dw_chan->lli_current = dw_chan->lli; | ||
| @@ -810,8 +803,9 @@ static void dw_dma_verify_transfer(struct dma_chan_data *channel, | ||
| #if defined __ZEPHYR__ | ||
| int i; | ||
| #else | ||
| struct dw_lli *lli = platform_dw_dma_lli_get(dw_chan->lli_current); | ||
| struct dw_lli *lli = dw_chan->lli_current; | ||
| #endif | ||
| switch (next->status) { | ||
| case DMA_CB_STATUS_END: | ||
| channel->status = COMP_STATE_PREPARE; | ||
| @@ -823,20 +817,14 @@ static void dw_dma_verify_transfer(struct dma_chan_data *channel, | ||
| * sure the cache is coherent between DSP and DMAC. | ||
| */ | ||
| #if defined __ZEPHYR__ | ||
| dcache_invalidate_region(dw_chan->lli, | ||
| sizeof(struct dw_lli) * channel->desc_count); | ||
| for (i = 0; i < channel->desc_count; i++) | ||
| dw_chan->lli[i].ctrl_hi &= ~DW_CTLH_DONE(1); | ||
| dcache_writeback_region(dw_chan->lli, | ||
| sizeof(struct dw_lli) * channel->desc_count); | ||
| #else | ||
| while (lli->ctrl_hi & DW_CTLH_DONE(1)) { | ||
| lli->ctrl_hi &= ~DW_CTLH_DONE(1); | ||
| dw_chan->lli_current = | ||
| (struct dw_lli *)dw_chan->lli_current->llp; | ||
| lli = platform_dw_dma_lli_get(dw_chan->lli_current); | ||
| lli = dw_chan->lli_current; | ||
| } | ||
| #endif | ||
| break; | ||
| @@ -966,9 +954,8 @@ static int dw_dma_probe(struct dma *dma) | ||
| pm_runtime_get_sync(DW_DMAC_CLK, dma->plat_data.id); | ||
| /* allocate dma channels */ | ||
| dma->chan = rzalloc(SOF_MEM_ZONE_RUNTIME_SHARED, 0, SOF_MEM_CAPS_RAM, | ||
| dma->chan = rballoc(SOF_MEM_FLAG_COHERENT, SOF_MEM_CAPS_RAM, | ||
| sizeof(struct dma_chan_data) * dma->plat_data.channels); | ||
| if (!dma->chan) { | ||
| tr_err(&dwdma_tr, "dw_dma_probe(): dma %d allocaction of channels failed", | ||
| dma->plat_data.id); | ||
| @@ -987,9 +974,7 @@ static int dw_dma_probe(struct dma *dma) | ||
| chan->index = i; | ||
| chan->core = DMA_CORE_INVALID; | ||
| dw_chan = rzalloc(SOF_MEM_ZONE_RUNTIME_SHARED, 0, SOF_MEM_CAPS_RAM, | ||
| sizeof(*dw_chan)); | ||
| dw_chan = rballoc(SOF_MEM_FLAG_COHERENT, SOF_MEM_CAPS_RAM, sizeof(*dw_chan)); | ||
| if (!dw_chan) { | ||
| tr_err(&dwdma_tr, "dw_dma_probe(): dma %d allocaction of channel %d private data failed", | ||
| dma->plat_data.id, i); | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@keyonjie this commit is quite difficult to follow. Can you split it based on the 3 things you are doing in 3 separate commits?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, thought of this, but those 3 items have dependencies with each other, e.g. if we change _CAPS_DMA of the buffer zone (the 1st above), it leads to allocation failure on many platforms, so I have to do 2nd and 3rd above to make full use of the SRAM and make rballoc() more clever to handle different asks for allocations. As @plbossart suggested, breaking functions with each single commit will break the bisection, better to do this refinement in one shot.