From f35c9d9c65738fdcf4a996634ac0d3f7024a6b73 Mon Sep 17 00:00:00 2001 From: Tomasz Lauda Date: Tue, 2 Oct 2018 12:27:56 +0200 Subject: [PATCH] pipeline: cache fixes for multicore processing Fixes cache issues with multicore processing of multiple pipelines: - Changes pipeline_cache method as previous implementation could not work, especially for invalidation. - Allocates dma and dai private data in uncached memory, because those resources are shared between different pipelines, so potentially also between different cores. Signed-off-by: Tomasz Lauda --- src/audio/dai.c | 8 --- src/audio/host.c | 4 -- src/audio/pipeline.c | 92 +++++++++++++------------------- src/drivers/intel/baytrail/ssp.c | 3 +- src/drivers/intel/cavs/dmic.c | 3 +- src/drivers/intel/cavs/hda-dma.c | 3 +- src/drivers/intel/cavs/ssp.c | 3 +- src/drivers/intel/dw-dma.c | 3 +- src/drivers/intel/haswell/ssp.c | 3 +- src/include/sof/dai.h | 4 +- src/include/sof/dma.h | 4 +- 11 files changed, 52 insertions(+), 78 deletions(-) diff --git a/src/audio/dai.c b/src/audio/dai.c index 56f3199bc..831538765 100644 --- a/src/audio/dai.c +++ b/src/audio/dai.c @@ -765,11 +765,7 @@ static void dai_cache(struct comp_dev *dev, int cmd) } dcache_writeback_invalidate_region(dd->dai, sizeof(*dd->dai)); - dcache_writeback_invalidate_region(dd->dai->private, - dd->dai->private_size); dcache_writeback_invalidate_region(dd->dma, sizeof(*dd->dma)); - dcache_writeback_invalidate_region(dd->dma->private, - dd->dma->private_size); dcache_writeback_invalidate_region(dd, sizeof(*dd)); dcache_writeback_invalidate_region(dev, sizeof(*dev)); break; @@ -782,11 +778,7 @@ static void dai_cache(struct comp_dev *dev, int cmd) dd = comp_get_drvdata(dev); dcache_invalidate_region(dd, sizeof(*dd)); dcache_invalidate_region(dd->dma, sizeof(*dd->dma)); - dcache_invalidate_region(dd->dma->private, - dd->dma->private_size); dcache_invalidate_region(dd->dai, sizeof(*dd->dai)); - dcache_invalidate_region(dd->dai->private, - dd->dai->private_size); list_for_item(item, &dd->config.elem_list) { dcache_invalidate_region(item, sizeof(*item)); diff --git a/src/audio/host.c b/src/audio/host.c index 31465e029..25f2e8b6a 100644 --- a/src/audio/host.c +++ b/src/audio/host.c @@ -786,8 +786,6 @@ static void host_cache(struct comp_dev *dev, int cmd) #endif dcache_writeback_invalidate_region(hd->dma, sizeof(*hd->dma)); - dcache_writeback_invalidate_region(hd->dma->private, - hd->dma->private_size); dcache_writeback_invalidate_region(hd, sizeof(*hd)); dcache_writeback_invalidate_region(dev, sizeof(*dev)); break; @@ -800,8 +798,6 @@ static void host_cache(struct comp_dev *dev, int cmd) hd = comp_get_drvdata(dev); dcache_invalidate_region(hd, sizeof(*hd)); dcache_invalidate_region(hd->dma, sizeof(*hd->dma)); - dcache_invalidate_region(hd->dma->private, - hd->dma->private_size); #if !defined CONFIG_DMA_GW list_for_item(item, &hd->local.elem_list) { diff --git a/src/audio/pipeline.c b/src/audio/pipeline.c index 6624e7aa6..2a89fef90 100644 --- a/src/audio/pipeline.c +++ b/src/audio/pipeline.c @@ -397,11 +397,8 @@ static int component_op_downstream(struct op_data *op_data, /* component should reset and free resources */ err = comp_reset(current); break; - case COMP_OPS_CACHE: - /* cache operation */ - comp_cache(current, op_data->cmd); - break; case COMP_OPS_BUFFER: /* handled by other API call */ + case COMP_OPS_CACHE: default: trace_pipe_error("eOi"); trace_error_value(op_data->op); @@ -482,11 +479,8 @@ static int component_op_upstream(struct op_data *op_data, /* component should reset and free resources */ err = comp_reset(current); break; - case COMP_OPS_CACHE: - /* cache operation */ - comp_cache(current, op_data->cmd); - break; case COMP_OPS_BUFFER: /* handled by other API call */ + case COMP_OPS_CACHE: default: trace_pipe_error("eOi"); trace_error_value(op_data->op); @@ -639,89 +633,79 @@ out: return ret; } -static void component_cache_buffers_downstream(struct comp_dev *start, - struct comp_dev *current, - struct comp_buffer *buffer, - cache_command cache_cmd) +static void component_cache_downstream(int cmd, struct comp_dev *start, + struct comp_dev *current, + struct comp_dev *previous) { + cache_command cache_cmd = comp_get_cache_command(cmd); struct list_item *clist; + struct comp_buffer *buffer; - if (current != start && buffer) { - cache_cmd(buffer, sizeof(*buffer)); + comp_cache(current, cmd); - /* stop if we reach an endpoint */ - if (current->is_endpoint) - return; - } + /* we finish walking the graph if we reach the DAI */ + if (current != start && current->is_endpoint) + return; - /* travel further */ + /* now run this operation downstream */ list_for_item(clist, ¤t->bsink_list) { buffer = container_of(clist, struct comp_buffer, source_list); - /* stop going if this component is not connected */ + if (cache_cmd) + cache_cmd(buffer, sizeof(*buffer)); + + /* don't go downstream if this component is not connected */ if (!buffer->connected) continue; - component_cache_buffers_downstream(start, buffer->sink, - buffer, cache_cmd); + component_cache_downstream(cmd, start, buffer->sink, current); } } -static void component_cache_buffers_upstream(struct comp_dev *start, - struct comp_dev *current, - struct comp_buffer *buffer, - cache_command cache_cmd) +static void component_cache_upstream(int cmd, struct comp_dev *start, + struct comp_dev *current, + struct comp_dev *previous) { + cache_command cache_cmd = comp_get_cache_command(cmd); struct list_item *clist; + struct comp_buffer *buffer; - if (current != start && buffer) { - cache_cmd(buffer, sizeof(*buffer)); + comp_cache(current, cmd); - /* stop if we reach an endpoint */ - if (current->is_endpoint) - return; - } + /* we finish walking the graph if we reach the DAI */ + if (current != start && current->is_endpoint) + return; - /* travel further */ + /* now run this operation upstream */ list_for_item(clist, ¤t->bsource_list) { buffer = container_of(clist, struct comp_buffer, sink_list); - /* stop going if this component is not connected */ + if (cache_cmd) + cache_cmd(buffer, sizeof(*buffer)); + + /* don't go upstream if this component is not connected */ if (!buffer->connected) continue; - component_cache_buffers_upstream(start, buffer->source, - buffer, cache_cmd); + component_cache_upstream(cmd, start, buffer->source, current); } } void pipeline_cache(struct pipeline *p, struct comp_dev *dev, int cmd) { cache_command cache_cmd = comp_get_cache_command(cmd); - struct op_data op_data; uint32_t flags; trace_pipe("cac"); - op_data.p = p; - op_data.op = COMP_OPS_CACHE; - op_data.cmd = cmd; - spin_lock_irq(&p->lock, flags); - if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) { - /* execute cache operation on components downstream */ - component_op_downstream(&op_data, dev, dev, NULL); - - /* execute cache operation on buffers downstream */ - component_cache_buffers_downstream(dev, dev, NULL, cache_cmd); - } else { - /* execute cache operation on components upstream */ - component_op_upstream(&op_data, dev, dev, NULL); - - /* execute cache operation on buffers upstream */ - component_cache_buffers_upstream(dev, dev, NULL, cache_cmd); - } + if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) + /* execute cache op on components and buffers downstream */ + component_cache_downstream(cmd, dev, dev, NULL); + else + /* execute cache op on components and buffers upstream */ + component_cache_upstream(cmd, dev, dev, NULL); /* execute cache operation on pipeline itself */ if (cache_cmd) diff --git a/src/drivers/intel/baytrail/ssp.c b/src/drivers/intel/baytrail/ssp.c index 6cd50eaed..cd67a47e3 100644 --- a/src/drivers/intel/baytrail/ssp.c +++ b/src/drivers/intel/baytrail/ssp.c @@ -603,7 +603,8 @@ static int ssp_probe(struct dai *dai) struct ssp_pdata *ssp; /* allocate private data */ - ssp = rzalloc(RZONE_SYS, SOF_MEM_CAPS_RAM, sizeof(*ssp)); + ssp = rzalloc(RZONE_SYS | RZONE_FLAG_UNCACHED, SOF_MEM_CAPS_RAM, + sizeof(*ssp)); dai_set_drvdata(dai, ssp); spinlock_init(&ssp->lock); diff --git a/src/drivers/intel/cavs/dmic.c b/src/drivers/intel/cavs/dmic.c index 2afa5423b..5cf88bca7 100644 --- a/src/drivers/intel/cavs/dmic.c +++ b/src/drivers/intel/cavs/dmic.c @@ -1447,7 +1447,8 @@ static int dmic_probe(struct dai *dai) trace_dmic("pro"); /* allocate private data */ - dmic = rzalloc(RZONE_SYS, SOF_MEM_CAPS_RAM, sizeof(*dmic)); + dmic = rzalloc(RZONE_SYS | RZONE_FLAG_UNCACHED, SOF_MEM_CAPS_RAM, + sizeof(*dmic)); if (!dmic) { trace_dmic_error("eap"); return -ENOMEM; diff --git a/src/drivers/intel/cavs/hda-dma.c b/src/drivers/intel/cavs/hda-dma.c index cbd02aa6e..311c70c17 100644 --- a/src/drivers/intel/cavs/hda-dma.c +++ b/src/drivers/intel/cavs/hda-dma.c @@ -613,7 +613,8 @@ static int hda_dma_probe(struct dma *dma) struct hda_chan_data *chan; /* allocate private data */ - hda_pdata = rzalloc(RZONE_SYS, SOF_MEM_CAPS_RAM, sizeof(*hda_pdata)); + hda_pdata = rzalloc(RZONE_SYS | RZONE_FLAG_UNCACHED, SOF_MEM_CAPS_RAM, + sizeof(*hda_pdata)); dma_set_drvdata(dma, hda_pdata); spinlock_init(&dma->lock); diff --git a/src/drivers/intel/cavs/ssp.c b/src/drivers/intel/cavs/ssp.c index 479c76c83..081829a9e 100644 --- a/src/drivers/intel/cavs/ssp.c +++ b/src/drivers/intel/cavs/ssp.c @@ -872,7 +872,8 @@ static int ssp_probe(struct dai *dai) struct ssp_pdata *ssp; /* allocate private data */ - ssp = rzalloc(RZONE_SYS, SOF_MEM_CAPS_RAM, sizeof(*ssp)); + ssp = rzalloc(RZONE_SYS | RZONE_FLAG_UNCACHED, SOF_MEM_CAPS_RAM, + sizeof(*ssp)); dai_set_drvdata(dai, ssp); spinlock_init(&ssp->lock); diff --git a/src/drivers/intel/dw-dma.c b/src/drivers/intel/dw-dma.c index d6c56f1b9..11e00b587 100644 --- a/src/drivers/intel/dw-dma.c +++ b/src/drivers/intel/dw-dma.c @@ -1113,7 +1113,8 @@ static int dw_dma_probe(struct dma *dma) int i; /* allocate private data */ - dw_pdata = rzalloc(RZONE_SYS, SOF_MEM_CAPS_RAM, sizeof(*dw_pdata)); + dw_pdata = rzalloc(RZONE_SYS | RZONE_FLAG_UNCACHED, SOF_MEM_CAPS_RAM, + sizeof(*dw_pdata)); dma_set_drvdata(dma, dw_pdata); spinlock_init(&dma->lock); diff --git a/src/drivers/intel/haswell/ssp.c b/src/drivers/intel/haswell/ssp.c index 3e885c7b0..2ca95effe 100644 --- a/src/drivers/intel/haswell/ssp.c +++ b/src/drivers/intel/haswell/ssp.c @@ -500,7 +500,8 @@ static int ssp_probe(struct dai *dai) struct ssp_pdata *ssp; /* allocate private data */ - ssp = rzalloc(RZONE_SYS, SOF_MEM_CAPS_RAM, sizeof(*ssp)); + ssp = rzalloc(RZONE_SYS | RZONE_FLAG_UNCACHED, SOF_MEM_CAPS_RAM, + sizeof(*ssp)); dai_set_drvdata(dai, ssp); spinlock_init(&ssp->lock); diff --git a/src/include/sof/dai.h b/src/include/sof/dai.h index ec875f876..90434188c 100644 --- a/src/include/sof/dai.h +++ b/src/include/sof/dai.h @@ -122,7 +122,6 @@ struct dai { struct dai_plat_data plat_data; const struct dai_ops *ops; void *private; - uint32_t private_size; }; /** @@ -153,8 +152,7 @@ void dai_install(struct dai_type_info *dai_type_array, size_t num_dai_types); struct dai *dai_get(uint32_t type, uint32_t index); #define dai_set_drvdata(dai, data) \ - dai->private = data; \ - dai->private_size = sizeof(*data) + dai->private = data; #define dai_get_drvdata(dai) \ dai->private; #define dai_base(dai) \ diff --git a/src/include/sof/dma.h b/src/include/sof/dma.h index d5227f5e1..aff77d331 100644 --- a/src/include/sof/dma.h +++ b/src/include/sof/dma.h @@ -164,7 +164,6 @@ struct dma { const struct dma_ops *ops; atomic_t num_channels_busy; /* number of busy channels */ void *private; - uint32_t private_size; }; /** @@ -188,8 +187,7 @@ void dma_install(struct dma *dma_array, size_t num_dmas); struct dma *dma_get(uint32_t dir, uint32_t caps, uint32_t dev, uint32_t flags); #define dma_set_drvdata(dma, data) \ - dma->private = data; \ - dma->private_size = sizeof(*data) + dma->private = data; #define dma_get_drvdata(dma) \ dma->private; #define dma_base(dma) \