Re: [PATCH 3/3] dmaengine: tegra-adma: Add support for Tegra210 ADMA

[Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]

On 09/29/2015 02:13 AM, Jon Hunter wrote:
>
> On 28/09/15 17:36, Stephen Warren wrote:
>> On 09/28/2015 08:57 AM, Jon Hunter wrote:
>>>
>>> On 25/09/15 16:47, Arnd Bergmann wrote:
>>>> On Friday 25 September 2015 16:38:55 Jon Hunter wrote:
>>>>> On 25/09/15 16:17, Jon Hunter wrote:
>>>>>>
>>>>>> On 25/09/15 16:03, Arnd Bergmann wrote:
>>>>>>> On Friday 25 September 2015 15:56:40 Jon Hunter wrote:
>>>>>>>> +       case DMA_MEM_TO_DEV:
>>>>>>>> +               burst_size = fls(tdc->config.dst_maxburst);
>>>>>>>> +               ch_regs->config = ADMA_CH_CONFIG_SRC_BUF(num_bufs
>>>>>>>> - 1);
>>>>>>>> +               ch_regs->ctrl =
>>>>>>>> ADMA_CH_CTRL_XFER_DIR(ADMA_MEM_TO_AHUB) |
>>>>>>>> +
>>>>>>>> ADMA_CH_CTRL_TX_REQ(tdc->config.slave_id);
>>>>>>>> +               ch_regs->src_addr = buf_addr;
>>>>>>>> +               break;
>>>>>>>> +
>>>>>>>> +       case DMA_DEV_TO_MEM:
>>>>>>>> +               burst_size = fls(tdc->config.src_maxburst);
>>>>>>>> +               ch_regs->config = ADMA_CH_CONFIG_TRG_BUF(num_bufs
>>>>>>>> - 1);
>>>>>>>> +               ch_regs->ctrl =
>>>>>>>> ADMA_CH_CTRL_XFER_DIR(ADMA_AHUB_TO_MEM) |
>>>>>>>> +
>>>>>>>> ADMA_CH_CTRL_RX_REQ(tdc->config.slave_id);
>>>>>>>> +               ch_regs->trg_addr = buf_addr;
>>>>>>>> +               break;
>>>>>>>
>>>>>>> Do not use the 'slave_id' field here to identify the slave device,
>>>>>>> that
>>>>>>> concept is broken. Instead, put the slave identification into the
>>>>>>> dma specifier in DT and read it out in your xlate function.
>>>>>>
>>>>>> Why is it broken?
>>>>>>
>>>>>> What happens if I don't know the slave-id? In other words, the
>>>>>> slave-id
>>>>>> can be dynamically allocated and associated with a given slave.
>>>>>
>>>>> I guess thinking about it some more, the driver could assign an id
>>>>> itself to a given channel and I could avoid using slave_id here. There
>>>>> are 22 channels and 10 tx and 10 rx requests.
>>>>
>>>> This sounds roughly right. So you could pick the ch_regs->ctrl value
>>>> when you allocate the tegra_adma_chan structure, and then map it to
>>>> the slave in the xlate() function.
>>>
>>> Actually, what I mentioned about would not work as it is not the DMA
>>> that should assign the requests to the channel.
>>>
>>> I think that I have poorly described the hardware and how it works, so
>>> let me try and explain a bit more.
>>>
>>>   From a hardware perspective it looks like the following ...
>>>
>>> memory <-> adma <-> adma-if <-> xbar <-> clients
>>>
>>> where:
>>> - memory is the system memory
>>> - adma is the dma controller
>>> - adma-if is the dma interface to a crossbar
>>> - xbar is the crossbar
>>> - clients are various audio interfaces, such as i2s, etc
>>>
>>> The adma-if is essentially a mux with 10 tx and 10 rx ports. Any of the
>>> 22 adma channels can be mapped to any of the 10 tx or rx ports. The
>>> request signal used by the adma is determined by which port it is
>>> configured to use. However, what makes this even more configurable is
>>> the xbar is also a mux that can route adma-if ports to the various
>>> clients.
>>>
>>> So potentially, you could use any adma channel and any port to route
>>> audio to any of the clients. However, the adma-if needs to know which
>>> adma channel is mapped to which port
>>
>> It does? I'm pretty sure it didn't in earlier chips; what changed?
>
> I *believe* that T210 is the first one to have the ADMA controller where
> as previous chips used the APB-DMA controller.

Yes, I believe that's true.

> Looking at the APB-DMA on
> T124 I can see that there is a fixed REQ_SEL value for each of the APBIF
> (equivalent of the ADMA-IF on T210).

I believe that is still true on T210. I don't see any difference in the 
way request select values are used/configured/... The only difference is 
that we're using a different DMA engine, but that DMA engine is 
configured in the same way.

>> For earlier chips, I believe all that's required is:
>>
>> When programming the DMA engine, you need to know which ADMA-IF is in
>> use, so the correct DMA request selector can be programmed into the DMA
>> engine for flow-control.
>>
>> ADMA-IF simply receives the data from DMA, and forwards it to the XBAR
>> tagged with the ADMA-IF's own ID.
>>
>> The XBAR programming selects which data source (ADMA-IF TX, I2S RX, ...)
>> each sink (ADMARX, I2S TX, ...) receives.
>
> Yes, exactly, this part sounds the same. However, just the ADMA itself
> allows for even more configuration.
>
>> Ideally, when an I2S controller needs to start transmitting data, it
>> should dynamically allocate an ADMA-IF, query it for its DMA slave
>> request ID, and then forward this information to the ASoC code that sets
>> up the DMA transfer.
>
> Agree.
>
>> In practice, this means that since any I2S module could use any ADMA-IF,
>> you probably need to list all DMA request selectors possible in the
>> I2S's DMA-related properties, so it can choose which one to use.
>
> Possibly, but really I think that the i2s only cares about the ADMA-IF
> and the hardware request used by the ADMA can be abstracted by the
> ADMA-IF. In other words, if you allocate an ADMA channel to work with a
> specific ADMA-IF, then let the ADMA-IF select the hardware request
> because as long as one is available, you don't care which.

Each ADMAIF (FIFO) has a single request select value statically assigned 
to it as far as I can tell, just like in previous chips.

>> Or perhaps the XBAR binding should list all the DMA requestors so that
>> each I2S node doesn't have to.
>
> Yes, however, I think that the ADMA-IF would make sense as it really
> cares about the mapping of hardware request to the ADMA-IF port.
>
> Cheers
> Jon

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html