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.
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
