On 06/06/2019 14:55, Dmitry Osipenko wrote: > 06.06.2019 16:45, Dmitry Osipenko пишет: >> 06.06.2019 15:37, Jon Hunter пишет: >>> >>> On 06/06/2019 12:54, Peter Ujfalusi wrote: >>>> >>>> >>>> On 06/06/2019 13.49, Jon Hunter wrote: >>>>> >>>>> 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. >>>> >>>> Sure, hardcoding is never good ;) >>>> >>>>> 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. >>>> >>>> Looking at the drivers/dma/tegra210-adma.c for the >>>> TEGRA*_FIFO_CTRL_DEFAULT definition it is still not clear where the >>>> remote FIFO size would fit. >>>> There are fields for overflow and starvation(?) thresholds and TX/RX >>>> size (assuming word length, 3 == 32bits?). >>> >>> The TX/RX size are the FIFO size. So 3 equates to a FIFO size of 3 * 64 >>> bytes. >>> >>>> Both threshold is set to one, so I assume currently ADMA is >>>> pushing/pulling data word by word. >>> >>> That's different. That indicates thresholds when transfers start. >>> >>>> Not sure what the burst size is used for, my guess would be that it is >>>> used on the memory (DDR) side for optimized, more efficient accesses? >>> >>> That is the actual burst size. >>> >>>> My guess is that the threshold values are the counter limits, if the DMA >>>> request counter reaches it then ADMA would do a threshold limit worth of >>>> push/pull to ADMAIF. >>>> Or there is another register where the remote FIFO size can be written >>>> and ADMA is counting back from there until it reaches the threshold (and >>>> pushes/pulling again threshold amount of data) so it keeps the FIFO >>>> filled with at least threshold amount of data? >>>> >>>> I think in both cases the threshold would be the maxburst. >>>> >>>> I suppose you have the patch for adma on how to use the fifo_size >>>> parameter? That would help understand what you are trying to achieve better. >>> >>> Its quite simple, we would just use the FIFO size to set the fields >>> TEGRAXXX_ADMA_CH_FIFO_CTRL_TXSIZE/RXSIZE in the >>> TEGRAXXX_ADMA_CH_FIFO_CTRL register. That's all. >>> >>> Jon >>> >> >> Hi, >> >> If I understood everything correctly, the FIFO buffer is shared among >> all of the ADMA clients and hence it should be up to the ADMA driver to >> manage the quotas of the clients. So if there is only one client that >> uses ADMA at a time, then this client will get a whole FIFO buffer, but >> once another client starts to use ADMA, then the ADMA driver will have >> to reconfigure hardware to split the quotas. >> > > You could also simply hardcode the quotas per client in the ADMA driver > if the quotas are going to be static anyway. Essentially this is what we have done so far, but Sameer is looking for a way to make this more programmable/flexible. We can always do that if there is no other option indeed. However, seems like a good time to see if there is a better way. Jon -- nvpublic