Date:   Thu, 6 Jun 2019 11:49:16 +0100
From:   Jon Hunter <jonathanh@...dia.com>
To:     Peter Ujfalusi <peter.ujfalusi@...com>,
        Sameer Pujar <spujar@...dia.com>, Vinod Koul <vkoul@...nel.org>
CC:     <dan.j.williams@...el.com>, <tiwai@...e.com>,
        <dmaengine@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <sharadg@...dia.com>, <rlokhande@...dia.com>, <dramesh@...dia.com>,
        <mkumard@...dia.com>, linux-tegra <linux-tegra@...r.kernel.org>
Subject: Re: [PATCH] [RFC] dmaengine: add fifo_size member


On 06/06/2019 11:22, Peter Ujfalusi wrote:

...

>>>> It does sounds like that FIFO_SIZE == src/dst_maxburst in your case as
>>>> well.
>>> Not exactly equal.
>>> ADMA burst_size can range from 1(WORD) to 16(WORDS)
>>> FIFO_SIZE can be adjusted from 16(WORDS) to 1024(WORDS) [can vary in
>>> multiples of 16]
>>
>> So I think that the key thing to highlight here, is that the as Sameer
>> highlighted above for the Tegra ADMA there are two values that need to
>> be programmed; the DMA client FIFO size and the max burst size. The ADMA
>> has register fields for both of these.
> 
> How does the ADMA uses the 'client FIFO size' and 'max burst size'
> values and what is the relation of these values to the peripheral side
> (ADMAIF)?

Per Sameer's previous comment, the FIFO size is used by the ADMA to
determine how much space is available in the FIFO. I assume the burst
size just limits how much data is transferred per transaction.

>> As you can see from the above the FIFO size can be much greater than the
>> burst size and so ideally both of these values would be passed to the DMA.
>>
>> We could get by with just passing the FIFO size (as the max burst size)
>> and then have the DMA driver set the max burst size depending on this,
>> but this does feel quite correct for this DMA. Hence, ideally, we would
>> like to pass both.
>>
>> We are also open to other ideas.
> 
> I can not find public documentation (I think they are walled off by
> registration), but correct me if I'm wrong:

No unfortunately, you are not wrong here :-(

> ADMAIF - peripheral side
>  - kind of a small DMA for audio preipheral(s)?

Yes this is the interface to the APE (audio processing engine) and data
sent to the ADMAIF is then sent across a crossbar to one of many
devices/interfaces (I2S, DMIC, etc). Basically a large mux that is user
configurable depending on the use-case.

>  - Variable FIFO size

Yes.

>  - sends DMA request to ADMA per words

>From Sameer's notes it says the ADMAIF send a signal to the ADMA per
word, yes.

> ADMA - system DMA
>  - receives the DMA requests from ADMAIF
>  - counts the requests
>  - based on some threshold of the counter it will send/read from ADMAIF?
>   - maxburst number of words probably?

Sounds about right to me.

> ADMA needs to know the ADMAIF's FIFO size because, it is the one who is
> managing that FIFO from the outside, making sure that it does not over
> or underrun?

Yes.

> And it is the one who sets the pace (in effect the DMA burst size - how
> many bytes the DMA jumps between refills) of refills to the ADMAIF's FIFO?

Yes.

So currently, if you look at the ADMA driver
(drivers/dma/tegra210-adma.c) you will see we use the src/dst_maxburst
for the burst, but the FIFO size is hard-coded (see the
TEGRA210_FIFO_CTRL_DEFAULT and TEGRA186_FIFO_CTRL_DEFAULT definitions).
Ideally, we should not hard-code this but pass it.

Given that there are no current users of the ADMA upstream, we could
change the usage of the src/dst_maxburst, but being able to set the FIFO
size as well would be ideal.

Cheers
Jon

-- 
nvpublic

Powered by blists - more mailing lists