On 12/5/20 1:55 AM, Sit, Michael Wei Hong wrote:
-----Original Message-----
From: Lars-Peter Clausen <lars@xxxxxxxxxx>
Sent: 01 December 2020 4:22 PM
To: Shevchenko, Andriy <andriy.shevchenko@xxxxxxxxx>
Cc: Sit, Michael Wei Hong <michael.wei.hong.sit@xxxxxxxxx>;
Sia, Jee
Heng <jee.heng.sia@xxxxxxxxx>; pierre-
louis.bossart@xxxxxxxxxxxxxxx;
Rojewski, Cezary <cezary.rojewski@xxxxxxxxx>;
liam.r.girdwood@xxxxxxxxxxxxxxx; vkoul@xxxxxxxxxx;
tiwai@xxxxxxxx;
broonie@xxxxxxxxxx; alsa-devel@xxxxxxxxxxxxxxxx
Subject: Re: [RFC PATCH 2/4] ASoC: soc-generic-dmaengine-
pcm: Add
custom prepare and submit function
...
Why should we split than resplit if we may do it in one go?
Why then we have DMA capabilities returned to the
consumers.
So, I have that we need to optimize DMA SG list preparation
in a way
that consumer gets SG list cooked in accordance with DMA
limitations / requirements.
The API that the audio drivers use request a period DMA
transfer for
length N split into M segments with the callback being called
after
each segment.
How that is implemented internally in the DMA does not matter
as long
as it matches those semantics. E.g. if the segment is longer than
what
the DMA can handle it can split it into two segments internally
and
call the callback every second segment.
The way this API works there isn't even really a possibility for
the
client side to split periodic transfers.
Btw. 1024 beats per segment seems excessively small, I don't
understand how somebody would design such an DMA. Was
the assumption
that the DMA will never transfer more than PAGE_SIZE of
contiguous
memory? Or do we not understand the documentation
correctly?
[>>] The segment size is 1024 items. The item size could be 8bits,
16bits or 32bits. So, set_max_seg_size() is set to 1024*4bytes.
- Lars
Hi All,
In order to fulfil Andy request on optimizing the linked-list at the DMA client side, we proposed a few changes to the soc-generic- dmaengine and DMAENGINE API due to the AxiDMA's nature operation in number of items.
Add New DMAENGINE API:
// For DMA driver to set the max items in a segment 1. dma_set_max_seg_items(struct device *dev, unsigned int size)
// For DMA client to retrieve the max items supported in a segment 2. dma_get_max_seg_items(struct device *dev)
#1 and #2 above are the critical API needed to be exposed to the DMA Clients so that DMA Clients can use it to calculate the appropriate segment length before pass it to the DMA driver.
If #1 and #2 are No Go, then splitting linked-list in DMA client can't be achieve.
ASoC changes:
1. Adding variable to snd_pcm_hardware to store DMA limitation information.
2. soc-generic-dmaengine-pcm to register DMA limitation exposed by DMA driver using API provided above.
3. dmaengine_pcm_prepare_and_submit in pcm_dmaengine.c to check the number of items needed calculated from userspace and configure the dma accordingly if the number of items exceeds the DMA items limitation.
4. dmaengine_pcm_dma_complete in pcm_dmaengine.c to check the number of items needed calculated from userspace and update position according to DMA limitation, to trigger period_elapse appropriately.
All of the above are needed in order to facilitate storing of the DMA limitation info and using the info to configure the DMA appropriately within the DMA limits.
#3 and #4 implements a check against the number of items needed based on userspace provided information and the DMA item limit, if the limit is exceeded, the maximum limit of the DMA is used to configure the DMA transfers.
e.g.
bytes_to_samples returns a value higher than the maximum item limit of the DMA, the driver will pass the maximum usable limit of the DMA using samples_to_bytes to the DMA driver. This will make the DMA driver to use longer linked-list and would not need to do the resplitting at the DMA driver.
Below is the snapshot of the code showing how soc-generic- dmaengine make use of the new API to calculate the segment length.
---------------------------------------------------------------------------------------------------------------------------------------------
Code change in pcm.h
struct snd_pcm_hardware {
......
size_t buffer_bytes_max; /* max buffer size */
size_t period_bytes_min; /* min period size */
size_t period_bytes_max; /* max period size */
unsigned int periods_min; /* min # of periods */
unsigned int periods_max; /* max # of periods */
size_t fifo_size; /* fifo size in bytes */
--> Add variable for dma max item numbers
e.g: unsigned int dma_item_max; /* max # of dma items */
......
};
---------------------------------------------------------------------------------------------------------------------------------------------
Code change in soc-generic-dmaengine-pcm.c
static int
dmaengine_pcm_set_runtime_hwparams(struct snd_soc_component *component,
struct snd_pcm_substream *substream) {
......
memset(&hw, 0, sizeof(hw));
hw.info = SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
SNDRV_PCM_INFO_INTERLEAVED;
hw.periods_min = 2;
hw.periods_max = UINT_MAX;
hw.period_bytes_min = 256;
hw.period_bytes_max = dma_get_max_seg_size(dma_dev);
hw.buffer_bytes_max = SIZE_MAX;
hw.fifo_size = dma_data->fifo_size;
--> Add code to register MAX_DMA_Items limitation using API exposed by
--> dma
e.g: hw.dma_item_max = dma_get_max_item_number(dma_dev);
......
}
---------------------------------------------------------------------------------------------------------------------------------------------
Code change in pcm_dmaengine.c
static void dmaengine_pcm_dma_complete(void *arg) {
......
struct snd_pcm_runtime *runtime = substream->runtime;
int blocks;
-->Add code to convert period bytes to number of DMA items passed down
-->from user space
e.g : blocks = bytes_to_samples(runtime, snd_pcm_lib_period_bytes(substream));
--> Add code to check number of DMA items from user space vs DMA
--> limitation and update position accordingly
e.g:
if (blocks > hw.dma_item_max)
prtd->pos += samples_to_bytes(runtime, MAX_DMA_BLOCKS);
else
prtd->pos += snd_pcm_lib_period_bytes(substream);
......
}
static int dmaengine_pcm_prepare_and_submit(struct snd_pcm_substream *substream) {
......
struct snd_pcm_runtime *runtime = substream->runtime;
int blocks;
--> Add code to convert period bytes to number of DMA items passed down
--> from user space
e.g: blocks = bytes_to_samples(runtime, snd_pcm_lib_period_bytes(substream));
......
--> Add code to check if the number of blocks used exceed the DMA
--> limitation and prepare DMA according to limitation instead of
--> information taken from userspace
e.g:
if (blocks > hw.dma_item_max)
desc = dmaengine_prep_dma_cyclic(chan,
substream->runtime->dma_addr,
snd_pcm_lib_buffer_bytes(substream),
samples_to_bytes(runtime, MAX_DMA_BLOCKS), direction, flags);
else
desc = dmaengine_prep_dma_cyclic(chan,
substream->runtime->dma_addr,
snd_pcm_lib_buffer_bytes(substream),
snd_pcm_lib_period_bytes(substream), direction, flags);
......
}
Hi,
I don't think this approach will work. If you setup a DMA transfer with
a different configuration that what was requested your audio will glitch.
If you really want to limit your period size you need to install a range
constraint on the SNDRV_PCM_HW_PARAM_PERIOD_SIZE parameter.
But I'd highly recommend against it and just split the transfer into
multiple segments in the DMA driver. Needlessly limiting the period size
will increase the number of interrupts during audio playback/recording
and hurt the power efficiency of your system.
- Lars
