Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1700792 > unrolled thread

Re: [PATCH v3 2/5] dmaengine: Add STM32 DMAMUX driver

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v3 2/5] dmaengine: Add STM32 DMAMUX driver Pierre Yves MORDRET <pierre-yves.mordret@st.com> - 2017-08-01 11:40 +0200
    Re: [PATCH v3 2/5] dmaengine: Add STM32 DMAMUX driver Vinod Koul <vinod.koul@intel.com> - 2017-08-02 07:00 +0200
      Re: [PATCH v3 2/5] dmaengine: Add STM32 DMAMUX driver Peter Ujfalusi <peter.ujfalusi@ti.com> - 2017-08-02 11:30 +0200
        Re: [PATCH v3 2/5] dmaengine: Add STM32 DMAMUX driver Pierre Yves MORDRET <pierre-yves.mordret@st.com> - 2017-08-02 15:20 +0200
          Re: [PATCH v3 2/5] dmaengine: Add STM32 DMAMUX driver Peter Ujfalusi <peter.ujfalusi@ti.com> - 2017-08-02 16:20 +0200
            Re: [PATCH v3 2/5] dmaengine: Add STM32 DMAMUX driver Pierre Yves MORDRET <pierre-yves.mordret@st.com> - 2017-08-02 16:30 +0200
              Re: [PATCH v3 2/5] dmaengine: Add STM32 DMAMUX driver Peter Ujfalusi <peter.ujfalusi@ti.com> - 2017-08-03 08:50 +0200
                Re: [PATCH v3 2/5] dmaengine: Add STM32 DMAMUX driver Pierre Yves MORDRET <pierre-yves.mordret@st.com> - 2017-08-03 11:10 +0200
                  Re: [PATCH v3 2/5] dmaengine: Add STM32 DMAMUX driver Peter Ujfalusi <peter.ujfalusi@ti.com> - 2017-08-03 12:00 +0200
                    Re: [PATCH v3 2/5] dmaengine: Add STM32 DMAMUX driver Pierre Yves MORDRET <pierre-yves.mordret@st.com> - 2017-08-04 15:00 +0200
                      Re: [PATCH v3 2/5] dmaengine: Add STM32 DMAMUX driver Peter Ujfalusi <peter.ujfalusi@ti.com> - 2017-08-04 16:30 +0200

#1700792 — Re: [PATCH v3 2/5] dmaengine: Add STM32 DMAMUX driver

On 07/31/2017 02:31 PM, Vinod Koul wrote:
> On Wed, Jul 26, 2017 at 07:38:02AM +0000, Pierre Yves MORDRET wrote:
>>>>>> +
>>>>>> +#ifndef __DMA_STM32_DMAMUX_H
>>>>>> +#define __DMA_STM32_DMAMUX_H
>>>>>> +
>>>>>> +#if defined(CONFIG_STM32_DMAMUX)
>>>>>> +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id);
>>>>>
>>>>> Why do we need a custom API in this case?
>>>>>
>>>>
>>>> This API is called by DMA when a slave is requested by client. DMA can work
>>>> without DMAMUX this API has been put in place to configure DMAMUX whether client
>>>> is requesting a DMAMUX Channel instead of a DMA one.
>>>
>>> You mean the dmaengine driver right?
>>>
>>
>> Yes. The API is mainly called by "device_config" through out STM32 DMA Driver
>> when a router is in place for client.
>> Please refer to Patch 4/5 on this set.
> 
> Okay am thinking on why this can't be generic..? An optional router config
> callback?
> 

I would have liked to answer there is a callback within engine but unfortunately
I didn't figure out one when I did my router's development. I've looked once
more but again I can't find how to map chanID and request line without custom API.

[toc] | [next] | [standalone]

#1701711

On Tue, Aug 01, 2017 at 09:32:50AM +0000, Pierre Yves MORDRET wrote:
> 
> 
> On 07/31/2017 02:31 PM, Vinod Koul wrote:
> > On Wed, Jul 26, 2017 at 07:38:02AM +0000, Pierre Yves MORDRET wrote:
> >>>>>> +
> >>>>>> +#ifndef __DMA_STM32_DMAMUX_H
> >>>>>> +#define __DMA_STM32_DMAMUX_H
> >>>>>> +
> >>>>>> +#if defined(CONFIG_STM32_DMAMUX)
> >>>>>> +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id);
> >>>>>
> >>>>> Why do we need a custom API in this case?
> >>>>>
> >>>>
> >>>> This API is called by DMA when a slave is requested by client. DMA can work
> >>>> without DMAMUX this API has been put in place to configure DMAMUX whether client
> >>>> is requesting a DMAMUX Channel instead of a DMA one.
> >>>
> >>> You mean the dmaengine driver right?
> >>>
> >>
> >> Yes. The API is mainly called by "device_config" through out STM32 DMA Driver
> >> when a router is in place for client.
> >> Please refer to Patch 4/5 on this set.
> > 
> > Okay am thinking on why this can't be generic..? An optional router config
> > callback?
> > 
> 
> I would have liked to answer there is a callback within engine but unfortunately
> I didn't figure out one when I did my router's development. I've looked once
> more but again I can't find how to map chanID and request line without custom API.

Yes there is no callback for routers but we can add a generic callback
here to be used. I added Peter for his comments, isn't that something they
need too?

-- 
~Vinod

[toc] | [prev] | [next] | [standalone]

#1701947

On 2017-08-02 07:55, Vinod Koul wrote:
> On Tue, Aug 01, 2017 at 09:32:50AM +0000, Pierre Yves MORDRET wrote:
>>
>>
>> On 07/31/2017 02:31 PM, Vinod Koul wrote:
>>> On Wed, Jul 26, 2017 at 07:38:02AM +0000, Pierre Yves MORDRET wrote:
>>>>>>>> +
>>>>>>>> +#ifndef __DMA_STM32_DMAMUX_H
>>>>>>>> +#define __DMA_STM32_DMAMUX_H
>>>>>>>> +
>>>>>>>> +#if defined(CONFIG_STM32_DMAMUX)
>>>>>>>> +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id);
>>>>>>>
>>>>>>> Why do we need a custom API in this case?
>>>>>>>
>>>>>>
>>>>>> This API is called by DMA when a slave is requested by client. DMA can work
>>>>>> without DMAMUX this API has been put in place to configure DMAMUX whether client
>>>>>> is requesting a DMAMUX Channel instead of a DMA one.
>>>>>
>>>>> You mean the dmaengine driver right?
>>>>>
>>>>
>>>> Yes. The API is mainly called by "device_config" through out STM32 DMA Driver
>>>> when a router is in place for client.
>>>> Please refer to Patch 4/5 on this set.
>>>
>>> Okay am thinking on why this can't be generic..? An optional router config
>>> callback?
>>>
>>
>> I would have liked to answer there is a callback within engine but unfortunately
>> I didn't figure out one when I did my router's development. I've looked once
>> more but again I can't find how to map chanID and request line without custom API.
>
> Yes there is no callback for routers but we can add a generic callback
> here to be used. I added Peter for his comments, isn't that something they
> need too?

The event router via of_dma_router_register() should be capable of 
handling different type of muxers, like the ti-dma-crossbar.c is doing 
for two different type of event crossbars.

Basically with the of_dma_route_allocate you craft a dma_spec which can 
be understood by the dma-master pointed form the router's node.
You do the configuration of the mux in this function, craft the dma_spec 
and that's it. In DT the peripherals are using the router's node for DMA 
binding and everything is transparent for them.

Note: The use of am335x xbar in the ti-dma-crossbar is optional and only 
needed when we need to have different event than the default for a 
specific dma request line.

If you normally use the DMA like this:
dmas = <&edma 129 1>, <&ddma_xbar 128 1>;
dma-names = "tx", "rx";

If you have DMA event router/mux, then depending on how it works you 
might have different number of parameters. In my case the DRA7 crossbar 
does not need extra parameter, so to get it in use:
dmas = <&edma_xbar 129 1>, <&edma_xbar 128 1>;
dma-names = "tx", "rx";

The router driver will rewrite the dma_spec and replace the 129/128 and 
pass something different to the dma-master (dynamic event mapping).

On am335x we have different xbar type so there:

dmas = <&edma_xbar 12 0 1>, <&edma_xbar 13 0 2>;

Out from this the router driver will create a spec equivalent to
dmas = <&edma_xbar 12 0>, <&edma_xbar 13 0>;

But it will change the xbar that DMA request 12/13 will not have the 
default event routed to.

I believe that the dma_router infra we have in dmaengine can cover most 
if not all use cases...

- Péter

[toc] | [prev] | [next] | [standalone]

#1702101

On 08/02/2017 11:19 AM, Peter Ujfalusi wrote:
> 
> 
> On 2017-08-02 07:55, Vinod Koul wrote:
>> On Tue, Aug 01, 2017 at 09:32:50AM +0000, Pierre Yves MORDRET wrote:
>>>
>>>
>>> On 07/31/2017 02:31 PM, Vinod Koul wrote:
>>>> On Wed, Jul 26, 2017 at 07:38:02AM +0000, Pierre Yves MORDRET wrote:
>>>>>>>>> +
>>>>>>>>> +#ifndef __DMA_STM32_DMAMUX_H
>>>>>>>>> +#define __DMA_STM32_DMAMUX_H
>>>>>>>>> +
>>>>>>>>> +#if defined(CONFIG_STM32_DMAMUX)
>>>>>>>>> +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id);
>>>>>>>>
>>>>>>>> Why do we need a custom API in this case?
>>>>>>>>
>>>>>>>
>>>>>>> This API is called by DMA when a slave is requested by client. DMA can work
>>>>>>> without DMAMUX this API has been put in place to configure DMAMUX whether client
>>>>>>> is requesting a DMAMUX Channel instead of a DMA one.
>>>>>>
>>>>>> You mean the dmaengine driver right?
>>>>>>
>>>>>
>>>>> Yes. The API is mainly called by "device_config" through out STM32 DMA Driver
>>>>> when a router is in place for client.
>>>>> Please refer to Patch 4/5 on this set.
>>>>
>>>> Okay am thinking on why this can't be generic..? An optional router config
>>>> callback?
>>>>
>>>
>>> I would have liked to answer there is a callback within engine but unfortunately
>>> I didn't figure out one when I did my router's development. I've looked once
>>> more but again I can't find how to map chanID and request line without custom API.
>>
>> Yes there is no callback for routers but we can add a generic callback
>> here to be used. I added Peter for his comments, isn't that something they
>> need too?
> 
> The event router via of_dma_router_register() should be capable of 
> handling different type of muxers, like the ti-dma-crossbar.c is doing 
> for two different type of event crossbars.
> 
> Basically with the of_dma_route_allocate you craft a dma_spec which can 
> be understood by the dma-master pointed form the router's node.
> You do the configuration of the mux in this function, craft the dma_spec 
> and that's it. In DT the peripherals are using the router's node for DMA 
> binding and everything is transparent for them.
> 
> Note: The use of am335x xbar in the ti-dma-crossbar is optional and only 
> needed when we need to have different event than the default for a 
> specific dma request line.
> 
> If you normally use the DMA like this:
> dmas = <&edma 129 1>, <&ddma_xbar 128 1>;
> dma-names = "tx", "rx";
> 
> If you have DMA event router/mux, then depending on how it works you 
> might have different number of parameters. In my case the DRA7 crossbar 
> does not need extra parameter, so to get it in use:
> dmas = <&edma_xbar 129 1>, <&edma_xbar 128 1>;
> dma-names = "tx", "rx";
> 
> The router driver will rewrite the dma_spec and replace the 129/128 and 
> pass something different to the dma-master (dynamic event mapping).
> 
> On am335x we have different xbar type so there:
> 
> dmas = <&edma_xbar 12 0 1>, <&edma_xbar 13 0 2>;
> 
> Out from this the router driver will create a spec equivalent to
> dmas = <&edma_xbar 12 0>, <&edma_xbar 13 0>;
> 
> But it will change the xbar that DMA request 12/13 will not have the 
> default event routed to.
> 
> I believe that the dma_router infra we have in dmaengine can cover most 
> if not all use cases...
> 
> - Péter
> 

Our SoC works with or without DMAMUX. Both binding is allowed. Using only DMA a
ChannelId and request line is part of the binding. Using DMAMUx now the request
line is coming from dma_spec forwards to dma-master as well explained by Peter.
However ChannelID is now given by dma_get_any_slave_channel instead of bindings.
DMAMUX driver has to be aware of this ID to route request line to out DMA
channel. This channel id information is carried on until DMAMUX through
dmaengine_slave_config with a custom API.
Hope it clarifies the need.

[toc] | [prev] | [next] | [standalone]

#1702152


Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

On 2017-08-02 16:11, Pierre Yves MORDRET wrote:
> Our SoC works with or without DMAMUX. Both binding is allowed. Using only DMA a
> ChannelId and request line is part of the binding.

In our case the am335x's eDMA can work with or without the router, we 
only use the router node if we need none default event for a given DMA 
request line.

> Using DMAMUx now the request
> line is coming from dma_spec forwards to dma-master as well explained by Peter.
> However ChannelID is now given by dma_get_any_slave_channel instead of bindings.
> DMAMUX driver has to be aware of this ID to route request line to out DMA
> channel. This channel id information is carried on until DMAMUX through
> dmaengine_slave_config with a custom API.
> Hope it clarifies the need.

I see, this is not much different then what we face with our dra7 
devices. In theory we could use direct DMA binding to the DMA controller 
itself, but some requests would not be reachable, so we always use the 
router's node for DMA on dra7 family.

Basically the router would manage the ChannelID and create 
'st,stm32-dma' compatible dma_spec (the four parameters).
Afaik you could have 3 parameters for the router and create a four 
parameter dma_spec, where the ChannelID is dynamically allocated.
But you need to convert all peripherals to use the router's node for the 
DMA.

- Péter

[toc] | [prev] | [next] | [standalone]

#1702157

On 08/02/2017 04:09 PM, Peter Ujfalusi wrote:
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 
> On 2017-08-02 16:11, Pierre Yves MORDRET wrote:
>> Our SoC works with or without DMAMUX. Both binding is allowed. Using only DMA a
>> ChannelId and request line is part of the binding.
> 
> In our case the am335x's eDMA can work with or without the router, we 
> only use the router node if we need none default event for a given DMA 
> request line.
> 
>> Using DMAMUx now the request
>> line is coming from dma_spec forwards to dma-master as well explained by Peter.
>> However ChannelID is now given by dma_get_any_slave_channel instead of bindings.
>> DMAMUX driver has to be aware of this ID to route request line to out DMA
>> channel. This channel id information is carried on until DMAMUX through
>> dmaengine_slave_config with a custom API.
>> Hope it clarifies the need.
> 
> I see, this is not much different then what we face with our dra7 
> devices. In theory we could use direct DMA binding to the DMA controller 
> itself, but some requests would not be reachable, so we always use the 
> router's node for DMA on dra7 family.
> 
> Basically the router would manage the ChannelID and create 
> 'st,stm32-dma' compatible dma_spec (the four parameters).
> Afaik you could have 3 parameters for the router and create a four 
> parameter dma_spec, where the ChannelID is dynamically allocated.

Correct router needs 3 parameters and among those 2 are forwarded though out
dma_spec. But when you say "ChannelID is dynamically allocated" you mean
dma_get_any_slave_channel ? If yes I can use the already existing bindings to
carry the channelID to DMA. No changes need to peripheral...

> But you need to convert all peripherals to use the router's node for the 
> DMA.
> 
> - Péter
> 

Py

[toc] | [prev] | [next] | [standalone]

#1702711

our mail server started to mangle outgoing mails, sorry for that, we are 
trying to resolve that...


Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

On 2017-08-02 17:28, Pierre Yves MORDRET wrote:
> Correct router needs 3 parameters and among those 2 are forwarded though out
> dma_spec. But when you say "ChannelID is dynamically allocated" you mean
> dma_get_any_slave_channel ? If yes I can use the already existing bindings to
> carry the channelID to DMA. No changes need to peripheral...

What I actually mean is that you should not need to modify the DMA 
driver at all.
According to stm32-dma.txt:
#dma-cells = <4>;
1. channelID
2. request line number
3. - 4. some parameters

I believe if you don't have the event router (directly using the DMA 
node) you always need to provide these, right?
If I'm not mistaken, when you use the event router you want to omit the 
ChannelID and get a random channel via dma_get_any_slave_channel in the 
DMA driver and feed back the channelID to the event router driver?
I believe the reason for this is that you want to keep mixed binding 
use, to have direct DMA bindings and bindings via event router.

Now what happens if you have direct binding:
device1 {
	dmas = <&dma2 1 4 0x10400 0x3>;
};

and via event router:
device2 {
	dmas =	<&dma_router 10 0x10400 0x3>,
		<&dma_router 11 0x10400 0x3>;
};

device2 probes first, will get channelID 0 and 1 via 
dma_get_any_slave_channel.

When device1 tries to probe, the channelID 1 is already taken..

You need to convert all peripherals to use the event router to avoid 
such a races. I have done the same for the dra7.
Add the event router driver,
add the event router node and convert existing users to use that
when adding new devices, use the event router node.

The event router's binding would have 3 parameters, it manages the 
available channelIDs, like the ti-dma-crossbar does for dra7. At 
allocate time it would pick an unused channelID and craft the dma-spec 
with the four parameters as documented.
The main DMA driver will not need any modification as everything will be 
taken care of by the event router.

The only gotcha is with memcpy type of transfers as they might also need 
unique channelID, but not requested via the slave binding. For that I 
have added properties to the event router to mask out certain channels 
(and I needed to do the same for the eDMA, but it is unrelated to the 
router itself).


- Péter

[toc] | [prev] | [next] | [standalone]

#1702806

On 08/03/2017 08:42 AM, Peter Ujfalusi wrote:
> our mail server started to mangle outgoing mails, sorry for that, we are 
> trying to resolve that...

No problem ;)

> 
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 
> On 2017-08-02 17:28, Pierre Yves MORDRET wrote:
>> Correct router needs 3 parameters and among those 2 are forwarded though out
>> dma_spec. But when you say "ChannelID is dynamically allocated" you mean
>> dma_get_any_slave_channel ? If yes I can use the already existing bindings to
>> carry the channelID to DMA. No changes need to peripheral...
> 
> What I actually mean is that you should not need to modify the DMA 
> driver at all.
> According to stm32-dma.txt:
> #dma-cells = <4>;
> 1. channelID
> 2. request line number
> 3. - 4. some parameters
> 
> I believe if you don't have the event router (directly using the DMA 
> node) you always need to provide these, right?

Correct

> If I'm not mistaken, when you use the event router you want to omit the 
> ChannelID and get a random channel via dma_get_any_slave_channel in the 
> DMA driver and feed back the channelID to the event router driver?

Again correct.

> I believe the reason for this is that you want to keep mixed binding 
> use, to have direct DMA bindings and bindings via event router.
> 

Well no. peripheral has to use DMAMUX and mixing up is to be avoided. This is
more for backward compatibility with SoC which doesn't have a DMAMUX.

> Now what happens if you have direct binding:
> device1 {
> 	dmas = <&dma2 1 4 0x10400 0x3>;
> };
> 
> and via event router:
> device2 {
> 	dmas =	<&dma_router 10 0x10400 0x3>,
> 		<&dma_router 11 0x10400 0x3>;
> };
> 
> device2 probes first, will get channelID 0 and 1 via 
> dma_get_any_slave_channel.
> 
> When device1 tries to probe, the channelID 1 is already taken..

Yes this is a flaw if we mix up bindings.

> 
> You need to convert all peripherals to use the event router to avoid 
> such a races. I have done the same for the dra7.
> Add the event router driver,
> add the event router node and convert existing users to use that
> when adding new devices, use the event router node.
> 
> The event router's binding would have 3 parameters, it manages the 
> available channelIDs, like the ti-dma-crossbar does for dra7. At 
> allocate time it would pick an unused channelID and craft the dma-spec 
> with the four parameters as documented.
> The main DMA driver will not need any modification as everything will be 
> taken care of by the event router.
> 

I look up what has been done in ti-dma-crossbar and actually this DMAMUX driver
has been well inspired from ti-dma-crossbar.
Nonetheless I understand what you meant. The channelID doesn't come from the
dmaengine and a piece a code is devised to allocate those. I could copy/paste
such code in my side but I do believe this would be better if such information
would come from dmaengine instead : this is what I did but a link/callback is
missing to craft this info until DMA. ChannelID is computed in two places in
dmaemgine and in your driver. Moreover any router is going to develop its own
channelID allocator, info which normally comes from dmaengine.

Vinod, I can update my driver to mimic what ti-dma-crossbar did to avoid the
custom API. This is s rather big change to evaluate in my side though.
However it seems to me such info should have come from dmaengine and not from
driver.
Let me know your thought about this

> The only gotcha is with memcpy type of transfers as they might also need 
> unique channelID, but not requested via the slave binding. For that I 
> have added properties to the event router to mask out certain channels 
> (and I needed to do the same for the eDMA, but it is unrelated to the 
> router itself).
> 
> 
> - Péter
> 

Py

[toc] | [prev] | [next] | [standalone]

#1702866


Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

On 2017-08-03 12:00, Pierre Yves MORDRET wrote:
>> What I actually mean is that you should not need to modify the DMA 
>> driver at all.
>> According to stm32-dma.txt:
>> #dma-cells = <4>;
>> 1. channelID
>> 2. request line number
>> 3. - 4. some parameters
>>
>> I believe if you don't have the event router (directly using the DMA 
>> node) you always need to provide these, right?
> 
> Correct
> 
>> If I'm not mistaken, when you use the event router you want to omit the 
>> ChannelID and get a random channel via dma_get_any_slave_channel in the 
>> DMA driver and feed back the channelID to the event router driver?
> 
> Again correct.
> 
>> I believe the reason for this is that you want to keep mixed binding 
>> use, to have direct DMA bindings and bindings via event router.
>>
> 
> Well no. peripheral has to use DMAMUX and mixing up is to be avoided. This is
> more for backward compatibility with SoC which doesn't have a DMAMUX.
> 
>> Now what happens if you have direct binding:
>> device1 {
>> 	dmas = <&dma2 1 4 0x10400 0x3>;
>> };
>>
>> and via event router:
>> device2 {
>> 	dmas =	<&dma_router 10 0x10400 0x3>,
>> 		<&dma_router 11 0x10400 0x3>;
>> };
>>
>> device2 probes first, will get channelID 0 and 1 via 
>> dma_get_any_slave_channel.
>>
>> When device1 tries to probe, the channelID 1 is already taken..
> 
> Yes this is a flaw if we mix up bindings.
> 
>>
>> You need to convert all peripherals to use the event router to avoid 
>> such a races. I have done the same for the dra7.
>> Add the event router driver,
>> add the event router node and convert existing users to use that
>> when adding new devices, use the event router node.
>>
>> The event router's binding would have 3 parameters, it manages the 
>> available channelIDs, like the ti-dma-crossbar does for dra7. At 
>> allocate time it would pick an unused channelID and craft the dma-spec 
>> with the four parameters as documented.
>> The main DMA driver will not need any modification as everything will be 
>> taken care of by the event router.
>>
> 
> I look up what has been done in ti-dma-crossbar and actually this DMAMUX driver
> has been well inspired from ti-dma-crossbar.
> Nonetheless I understand what you meant. The channelID doesn't come from the
> dmaengine and a piece a code is devised to allocate those. I could copy/paste
> such code in my side but I do believe this would be better if such information
> would come from dmaengine instead : this is what I did but a link/callback is
> missing to craft this info until DMA. ChannelID is computed in two places in
> dmaemgine and in your driver. Moreover any router is going to develop its own
> channelID allocator, info which normally comes from dmaengine.

yes and no.
In my case on dr7 we have DMA request crossbar to support more events
than either eDMA or sDMA could possible handle. eDMA and sDMA works in
different ways, but the same event router driver facilitates them fine.
In sDMA any channel can service any DMA requests, while in eDMA the
channel number and the event numbers are matched (ch10 can service only
event10).

Our event router driver's task is to map the incoming event number to an
outgoing event number, which is then going to be used by the DMA driver
as incoming event. So in the crossbar we anyways need to pick an event
from the available list of unused ones. The DMA drivers (eDMA or sDMA)
would just think that the request comes via normal binding and does what
it normally does on SoC where we don't have the crossbar (OMAPs for
sDMA, daVinci, am33/am43, k2g, etc for eDMA).

I'm not sure what your DMAMUX muxes, is it the channels or the events,
but it only need to manage the needed, moving parts.

> Vinod, I can update my driver to mimic what ti-dma-crossbar did to avoid the
> custom API. This is s rather big change to evaluate in my side though.
> However it seems to me such info should have come from dmaengine and not from
> driver.
> Let me know your thought about this
> 
>> The only gotcha is with memcpy type of transfers as they might also need 
>> unique channelID, but not requested via the slave binding. For that I 
>> have added properties to the event router to mask out certain channels 
>> (and I needed to do the same for the eDMA, but it is unrelated to the 
>> router itself).
>>
>>
>> - Péter
>>
> 
> Py
> 

- Péter

[toc] | [prev] | [next] | [standalone]

#1703950

On 08/03/2017 11:48 AM, Peter Ujfalusi wrote:
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 
> On 2017-08-03 12:00, Pierre Yves MORDRET wrote:
>>> What I actually mean is that you should not need to modify the DMA 
>>> driver at all.
>>> According to stm32-dma.txt:
>>> #dma-cells = <4>;
>>> 1. channelID
>>> 2. request line number
>>> 3. - 4. some parameters
>>>
>>> I believe if you don't have the event router (directly using the DMA 
>>> node) you always need to provide these, right?
>>
>> Correct
>>
>>> If I'm not mistaken, when you use the event router you want to omit the 
>>> ChannelID and get a random channel via dma_get_any_slave_channel in the 
>>> DMA driver and feed back the channelID to the event router driver?
>>
>> Again correct.
>>
>>> I believe the reason for this is that you want to keep mixed binding 
>>> use, to have direct DMA bindings and bindings via event router.
>>>
>>
>> Well no. peripheral has to use DMAMUX and mixing up is to be avoided. This is
>> more for backward compatibility with SoC which doesn't have a DMAMUX.
>>
>>> Now what happens if you have direct binding:
>>> device1 {
>>> 	dmas = <&dma2 1 4 0x10400 0x3>;
>>> };
>>>
>>> and via event router:
>>> device2 {
>>> 	dmas =	<&dma_router 10 0x10400 0x3>,
>>> 		<&dma_router 11 0x10400 0x3>;
>>> };
>>>
>>> device2 probes first, will get channelID 0 and 1 via 
>>> dma_get_any_slave_channel.
>>>
>>> When device1 tries to probe, the channelID 1 is already taken..
>>
>> Yes this is a flaw if we mix up bindings.
>>
>>>
>>> You need to convert all peripherals to use the event router to avoid 
>>> such a races. I have done the same for the dra7.
>>> Add the event router driver,
>>> add the event router node and convert existing users to use that
>>> when adding new devices, use the event router node.
>>>
>>> The event router's binding would have 3 parameters, it manages the 
>>> available channelIDs, like the ti-dma-crossbar does for dra7. At 
>>> allocate time it would pick an unused channelID and craft the dma-spec 
>>> with the four parameters as documented.
>>> The main DMA driver will not need any modification as everything will be 
>>> taken care of by the event router.
>>>
>>
>> I look up what has been done in ti-dma-crossbar and actually this DMAMUX driver
>> has been well inspired from ti-dma-crossbar.
>> Nonetheless I understand what you meant. The channelID doesn't come from the
>> dmaengine and a piece a code is devised to allocate those. I could copy/paste
>> such code in my side but I do believe this would be better if such information
>> would come from dmaengine instead : this is what I did but a link/callback is
>> missing to craft this info until DMA. ChannelID is computed in two places in
>> dmaemgine and in your driver. Moreover any router is going to develop its own
>> channelID allocator, info which normally comes from dmaengine.
> 
> yes and no.
> In my case on dr7 we have DMA request crossbar to support more events
> than either eDMA or sDMA could possible handle. eDMA and sDMA works in
> different ways, but the same event router driver facilitates them fine.
> In sDMA any channel can service any DMA requests, while in eDMA the
> channel number and the event numbers are matched (ch10 can service only
> event10).
> 
> Our event router driver's task is to map the incoming event number to an
> outgoing event number, which is then going to be used by the DMA driver
> as incoming event. So in the crossbar we anyways need to pick an event
> from the available list of unused ones. The DMA drivers (eDMA or sDMA)
> would just think that the request comes via normal binding and does what
> it normally does on SoC where we don't have the crossbar (OMAPs for
> sDMA, daVinci, am33/am43, k2g, etc for eDMA).
> 
> I'm not sure what your DMAMUX muxes, is it the channels or the events,
> but it only need to manage the needed, moving parts.
> 

Our DMAMUX can manage up to 255 request lines (only 128 is eventually assigned
though) onto 16 events: 8 events mapped on 1 DMA and the 8 others onto the
second DMA. Request line numbering is fixed (a peripheral DMA request is
assigned to one MUX input) and but can be routed randomly onto the any 16
channels. We use chanID to mux input on event.
chanID given by dma_get_any_slave_channe is enough in our case.

Py

>> Vinod, I can update my driver to mimic what ti-dma-crossbar did to avoid the
>> custom API. This is s rather big change to evaluate in my side though.
>> However it seems to me such info should have come from dmaengine and not from
>> driver.
>> Let me know your thought about this
>>
>>> The only gotcha is with memcpy type of transfers as they might also need 
>>> unique channelID, but not requested via the slave binding. For that I 
>>> have added properties to the event router to mask out certain channels 
>>> (and I needed to do the same for the eDMA, but it is unrelated to the 
>>> router itself).
>>>
>>>
>>> - Péter
>>>
>>
>> Py
>>
> 
> - Péter
> 

[toc] | [prev] | [next] | [standalone]

#1704019

On 08/04/2017 03:50 PM, Pierre Yves MORDRET wrote:
> Our DMAMUX can manage up to 255 request lines (only 128 is eventually assigned
> though) onto 16 events: 8 events mapped on 1 DMA and the 8 others onto the
> second DMA. Request line numbering is fixed (a peripheral DMA request is
> assigned to one MUX input) and but can be routed randomly onto the any 16
> channels. We use chanID to mux input on event.
> chanID given by dma_get_any_slave_channe is enough in our case.

I would think that if you have in the router node:
dma-masters = <&dma1>, <&dma2>;

and request a DMA via the router:
dmas = <&dma_router req_in param1 param2>;

then the router driver would decide which dma-master it is going to assign the
given request line and craft the dma-spec based on this decision. This
requires no callback to the router from the DMA master driver at all.

The idea of the dma event router is to be transparent for the DMA clients
(peripherals needing DMA channel) and for the DMA drivers as well. Neither
should know that the events are muxed as it does not really matter for them.

-- 
Péter

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web