On Thu, Jul 19, 2018 at 07:53:25PM +0100, Liam Girdwood wrote:
The Sound Open Firmware driver core is a generic architecture independent layer that allows SOF to be used on many different different architectures and platforms. It abstracts DSP operations and IO methods
+++ b/include/sound/soc.h @@ -1133,6 +1133,7 @@ struct snd_soc_pcm_runtime { /* runtime devices */ struct snd_pcm *pcm; struct snd_compr *compr;
- struct snd_sof_pcm *sof; struct snd_soc_dai *codec_dai; struct snd_soc_dai *cpu_dai;
Can we rename this somehow to be less specific to SoF or move it to be somewhere other than the runtime structure? I can see why you've done this but I can also see every DSP vendor turning up and trying to add their own field in here.
+/*
- This file is provided under a dual BSD/GPLv2 license. When using or
- redistributing this file, you may do so under either license.
- Copyright(c) 2017 Intel Corporation. All rights reserved.
2017?
index 000000000000..29458909182a --- /dev/null +++ b/sound/soc/sof/core.c @@ -0,0 +1,373 @@ +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) +/*
- This file is provided under a dual BSD/GPLv2 license. When using or
Please make the entire comment a C++ one, it makes it look more intentional.
+static inline unsigned int sof_get_pages(size_t size) +{
- return (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
+}
This feels like there should be a generic MM function for it?
+/*
- Generic buffer page table creation.
- */
+int snd_sof_create_page_table(struct snd_sof_dev *sdev,
struct snd_dma_buffer *dmab,
unsigned char *page_table, size_t size)
+{
- int i, pages;
- pages = snd_sgbuf_aligned_pages(size);
- dev_dbg(sdev->dev, "generating page table for %p size 0x%zx pages %d\n",
dmab->area, size, pages);
- for (i = 0; i < pages; i++) {
u32 idx = (((i << 2) + i)) >> 1;
u32 pfn = snd_sgbuf_get_addr(dmab, i * PAGE_SIZE) >> PAGE_SHIFT;
u32 *pg_table;
dev_dbg(sdev->dev, "pfn i %i idx %d pfn %x\n", i, idx, pfn);
pg_table = (u32 *)(page_table + idx);
if (i & 1)
*pg_table |= (pfn << 4);
else
*pg_table |= pfn;
- }
I'm not 100% clear what this is intended to do - lots of magic numbers and dependencies on the host memory configuration.