Skip to content

pipeline: dai: dw-dma: adjust flow to more scheduler flexible #1125

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.

Sign up for GitHub

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

Merged
lgirdwood merged 2 commits into from
Mar 18, 2019
Merged

pipeline: dai: dw-dma: adjust flow to more scheduler flexible #1125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d97cd8b
pipeline: adjust flow to more scheduler flexible
tlauda Mar 14, 2019
649f00e
dai: dw-dma: adjust flow to more scheduler flexible
tlauda Mar 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/audio/dai.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,7 @@ static int dai_params(struct comp_dev *dev)
static int dai_prepare(struct comp_dev *dev)
{
struct dai_data *dd = comp_get_drvdata(dev);
struct dma_sg_config *config = &dd->config;
int ret = 0;

trace_dai_with_ids(dev, "dai_prepare()");
Expand All @@ -477,7 +478,9 @@ static int dai_prepare(struct comp_dev *dev)
/* write back buffer contents from cache */
dcache_writeback_region(dd->dma_buffer->addr, dd->dma_buffer->size);

dd->cb_type = 0;
/* set default callback type if irq disabled */
if (config->irq_disabled)
dd->cb_type = DMA_CB_TYPE_BLOCK;

/* dma reconfig not required if XRUN handling */
if (dd->xrun) {
Expand Down Expand Up @@ -615,7 +618,8 @@ static int dai_copy(struct comp_dev *dev)
dev->params.direction);
platform_dai_wallclock(dev, &dd->wallclock);

return ret;
/* let's not copy further */
return 1;

@lgirdwood lgirdwood Mar 14, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering (for a future PR), do we want to define a macro for PIPELINE_COPY_FURTHER_STOP to make it obvious for anyone reading the 1 here.

@tlauda tlauda Mar 14, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was thinking about that and it would also be helpful for things like prepare, so probably I will make a PR with something like PIPELINE_OPERATION_FURTHER_STOP.

}

/* get data sizes from DMA */
Expand Down
17 changes: 10 additions & 7 deletions src/audio/pipeline.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,8 +353,11 @@ int pipeline_prepare(struct pipeline *p, struct comp_dev *dev)
goto out;
}

/* pipeline preload needed only for playback streams */
p->preload = dev->params.direction == SOF_IPC_STREAM_PLAYBACK;
/* pipeline preload needed only for playback streams and capture
* streams scheduled with timer
*/
p->preload = dev->params.direction == SOF_IPC_STREAM_PLAYBACK ||
pipeline_is_timer_driven(p);
p->status = COMP_STATE_PREPARE;

out:
Expand Down Expand Up @@ -424,12 +427,12 @@ static void pipeline_comp_trigger_sched_comp(struct pipeline *p,
case COMP_TRIGGER_START:
p->xrun_bytes = 0;

/* playback pipelines need scheduled now, capture pipelines are
* scheduled once their initial DMA period is filled by the DAI
* or in resume process
/* playback pipelines need to be scheduled now,
* capture pipelines are scheduled only for
* timer driven scheduling
*/
if (comp->params.direction == SOF_IPC_STREAM_PLAYBACK ||
p->status == COMP_STATE_PAUSED) {
pipeline_is_timer_driven(p)) {
/* schedule initial pipeline fill when next idle */
pipeline_schedule_copy_idle(p);
}
Expand Down Expand Up @@ -596,7 +599,7 @@ static int pipeline_comp_copy(struct comp_dev *current, void *data, int dir)
/* copy to downstream immediately */
if (dir == PPL_DIR_DOWNSTREAM) {
err = comp_copy(current);
if (err < 0)
if (err < 0 || err > 0)

@lgirdwood lgirdwood Mar 14, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (err > 0 ) then it's copy stop (which is not an error). So we probably want to change this to 0 ?

@tlauda tlauda Mar 14, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For other operations like params and prepare it's not modified back to 0, so I wanted to stay aligned. Anyway maybe some higher level function will need to know this status in the future and that it was stopped intentionally.

return err;
}

Expand Down
13 changes: 13 additions & 0 deletions src/drivers/dw/dma.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,9 @@
/* number of tries to wait for reset */
#define DW_DMA_CFG_TRIES 10000

/* min number of elems for config with irq disabled */
#define DW_DMA_CFG_NO_IRQ_MIN_ELEMS 3

/* linked list item address */
#define DW_DMA_LLI_ADDRESS(lli, dir) \
(((dir) == DMA_DIR_MEM_TO_DEV) ? ((lli)->sar) : ((lli)->dar))
Expand Down Expand Up @@ -797,6 +800,16 @@ static int dw_dma_set_config(struct dma *dma, int channel,
goto out;
}

if (config->irq_disabled &&
config->elem_array.count < DW_DMA_CFG_NO_IRQ_MIN_ELEMS) {
trace_dwdma_error("dw-dma: %d channel %d not enough elems "
"for config with irq disabled",
dma->plat_data.id, channel,
config->elem_array.count);
ret = -EINVAL;
goto out;
}

/* do we need to realloc descriptors */
if (config->elem_array.count != chan->desc_count) {

Expand Down