| Message ID | 20200930091412.8020-2-peter.ujfalusi@ti.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | dmaengine/soc: k3-udma: Add support for BCDMA and PKTDMA | expand |
Hi Peter, On 30-09-20, 12:13, Peter Ujfalusi wrote: > Additional configuration for the DMA event router might be needed for a > channel which can not be done during device_alloc_chan_resources callback > since the router information is not yet present for the drivers. > > If there is a need for additional configuration for the channel if DMA > router is in use, then the driver can implement the device_router_config > callback. So what is the additional information you need, I am looking at the code below and xlate invokes device_router_config() which driver will implement.. Are you using this to configure channels based on info from DT? > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > --- > drivers/dma/of-dma.c | 10 ++++++++++ > include/linux/dmaengine.h | 2 ++ > 2 files changed, 12 insertions(+) > > diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c > index 8a4f608904b9..ec00b20ae8e4 100644 > --- a/drivers/dma/of-dma.c > +++ b/drivers/dma/of-dma.c > @@ -75,8 +75,18 @@ static struct dma_chan *of_dma_router_xlate(struct of_phandle_args *dma_spec, > ofdma->dma_router->route_free(ofdma->dma_router->dev, > route_data); > } else { > + int ret = 0; > + > chan->router = ofdma->dma_router; > chan->route_data = route_data; > + > + if (chan->device->device_router_config) > + ret = chan->device->device_router_config(chan); > + > + if (ret) { > + dma_release_channel(chan); > + chan = ERR_PTR(ret); > + } > } > > /* > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h > index dd357a747780..d6197fe875af 100644 > --- a/include/linux/dmaengine.h > +++ b/include/linux/dmaengine.h > @@ -800,6 +800,7 @@ struct dma_filter { > * by tx_status > * @device_alloc_chan_resources: allocate resources and return the > * number of allocated descriptors > + * @device_router_config: optional callback for DMA router configuration > * @device_free_chan_resources: release DMA channel's resources > * @device_prep_dma_memcpy: prepares a memcpy operation > * @device_prep_dma_xor: prepares a xor operation > @@ -874,6 +875,7 @@ struct dma_device { > enum dma_residue_granularity residue_granularity; > > int (*device_alloc_chan_resources)(struct dma_chan *chan); > + int (*device_router_config)(struct dma_chan *chan); > void (*device_free_chan_resources)(struct dma_chan *chan); > > struct dma_async_tx_descriptor *(*device_prep_dma_memcpy)( > -- > Peter > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 07/10/2020 8.44, Vinod Koul wrote: > Hi Peter, > > On 30-09-20, 12:13, Peter Ujfalusi wrote: >> Additional configuration for the DMA event router might be needed for a >> channel which can not be done during device_alloc_chan_resources callback >> since the router information is not yet present for the drivers. >> >> If there is a need for additional configuration for the channel if DMA >> router is in use, then the driver can implement the device_router_config >> callback. > > So what is the additional information you need, I am looking at the code > below and xlate invokes device_router_config() which driver will > implement.. The router driver is not yet ready due to external dependencies, it will come a bit later. > Are you using this to configure channels based on info from DT? Not really. In DT an event triggered channel can be requested via router (when this is used) for example: dmas = <&inta_l2g a b c>; a - the input number of the DMA request in l2g b - edge or level trigger to be selected c - ASEL number for the channel for coherency The l2g router driver then translate this to: <&main_bcdma 1 0 c> 1 - Global trigger 0 is used by the DMA 0 - ignored c - ASEL number. The router needs to send an event which is going to be received by the channel we have picked up, this event number can only be known when we do have the channel. So the flow in this case: router converts the dma_spec for the DMA, but it does not yet know what is the event number it has to use. The BCDMA driver will pick an available bchan and notes that the transfers will be triggered by global event 0. When we have the channel, the core saves the router information and calls the device_router_config of BCDMA. In there we call back to the router and give the event number it has to use to send the trigger for the channel. >> >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> >> --- >> drivers/dma/of-dma.c | 10 ++++++++++ >> include/linux/dmaengine.h | 2 ++ >> 2 files changed, 12 insertions(+) >> >> diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c >> index 8a4f608904b9..ec00b20ae8e4 100644 >> --- a/drivers/dma/of-dma.c >> +++ b/drivers/dma/of-dma.c >> @@ -75,8 +75,18 @@ static struct dma_chan *of_dma_router_xlate(struct of_phandle_args *dma_spec, >> ofdma->dma_router->route_free(ofdma->dma_router->dev, >> route_data); >> } else { >> + int ret = 0; >> + >> chan->router = ofdma->dma_router; >> chan->route_data = route_data; >> + >> + if (chan->device->device_router_config) >> + ret = chan->device->device_router_config(chan); >> + >> + if (ret) { >> + dma_release_channel(chan); >> + chan = ERR_PTR(ret); >> + } >> } >> >> /* >> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h >> index dd357a747780..d6197fe875af 100644 >> --- a/include/linux/dmaengine.h >> +++ b/include/linux/dmaengine.h >> @@ -800,6 +800,7 @@ struct dma_filter { >> * by tx_status >> * @device_alloc_chan_resources: allocate resources and return the >> * number of allocated descriptors >> + * @device_router_config: optional callback for DMA router configuration >> * @device_free_chan_resources: release DMA channel's resources >> * @device_prep_dma_memcpy: prepares a memcpy operation >> * @device_prep_dma_xor: prepares a xor operation >> @@ -874,6 +875,7 @@ struct dma_device { >> enum dma_residue_granularity residue_granularity; >> >> int (*device_alloc_chan_resources)(struct dma_chan *chan); >> + int (*device_router_config)(struct dma_chan *chan); >> void (*device_free_chan_resources)(struct dma_chan *chan); >> >> struct dma_async_tx_descriptor *(*device_prep_dma_memcpy)( >> -- >> Peter >> >> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. >> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 07-10-20, 11:08, Peter Ujfalusi wrote: > Not really. In DT an event triggered channel can be requested via router > (when this is used) for example: > > dmas = <&inta_l2g a b c>; > a - the input number of the DMA request in l2g > b - edge or level trigger to be selected > c - ASEL number for the channel for coherency > > The l2g router driver then translate this to: > <&main_bcdma 1 0 c> > 1 - Global trigger 0 is used by the DMA > 0 - ignored > c - ASEL number. > > The router needs to send an event which is going to be received by the > channel we have picked up, this event number can only be known when we > do have the channel. > > So the flow in this case: > router converts the dma_spec for the DMA, but it does not yet know what > is the event number it has to use. > The BCDMA driver will pick an available bchan and notes that the > transfers will be triggered by global event 0. > When we have the channel, the core saves the router information and > calls the device_router_config of BCDMA. > In there we call back to the router and give the event number it has to > use to send the trigger for the channel. Ah that is intresting, so you would call router driver foo_set_event() and would send the event number, why not call that API from alloc channel or even xlate? Why do you need new callback? Or did i miss something..
On 07/10/2020 18.55, Vinod Koul wrote: > On 07-10-20, 11:08, Peter Ujfalusi wrote: > >> Not really. In DT an event triggered channel can be requested via router >> (when this is used) for example: >> >> dmas = <&inta_l2g a b c>; >> a - the input number of the DMA request in l2g >> b - edge or level trigger to be selected >> c - ASEL number for the channel for coherency >> >> The l2g router driver then translate this to: >> <&main_bcdma 1 0 c> >> 1 - Global trigger 0 is used by the DMA >> 0 - ignored >> c - ASEL number. >> >> The router needs to send an event which is going to be received by the >> channel we have picked up, this event number can only be known when we >> do have the channel. >> >> So the flow in this case: >> router converts the dma_spec for the DMA, but it does not yet know what >> is the event number it has to use. >> The BCDMA driver will pick an available bchan and notes that the >> transfers will be triggered by global event 0. >> When we have the channel, the core saves the router information and >> calls the device_router_config of BCDMA. >> In there we call back to the router and give the event number it has to >> use to send the trigger for the channel. > > Ah that is intresting, so you would call router driver foo_set_event() > and would send the event number Yes, that's correct. > why not call that API from alloc > channel or even xlate? at alloc / xlate time the DMA driver does not have information about router. The alloc/xlate will result the channel, but in my case it will result a broken setup as the router does not know which event to send. > Why do you need new callback? When I added the DMA event router support, it was designed in a way that the DMA driver itself must not know anything about the router, it has to be transparent. One can just add a router on front of any DMA and everything will work. This is the right thing to do, and it works for existing setups. > Or did i miss something.. The BCDMA triggered channel setup is a chicken-egg setup. For this case the channel can be triggered by a global event. A channel can receive two global event, but this is not a concern atm. The event number depends on the channel we use, for simplicity let's say: bchan_id + trigger_offset = bchan_trigger_evt. of_dma_router_xlate does this: 1. calls the dma router's of_dma_route_allocate callback to allocate a route and craft a dma_spec for the DMA to configure a channel. 2. using this crafted dma_spec we request a channel via of_dma_xlate callback 3. if we got the channel, we save the router information, so it can be deallocated when the channel is disabled. I need a fourth step to do a final configuration since only at this time (after it has been allocated) the channel has information about possible router. In the new optional callback the DMA driver can figure out the event number which must be used by the router to send the event to the desired global event target of the channel. Other DMAs might need something different, but imho if there is going to be a need for such post alloc router config, then it is most likely will come from the need to feed back some sort of channel information to the router. Or take parameter from the router itself for the channel. To summarize: In of_dma_route_allocate() the router does not yet know the channel we are going to get. In of_dma_xlate() the DMA driver does not yet know if the channel will use router or not. I need to tell the router the event number it has to send, which is based on the channel number I got. - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 08-10-20, 09:41, Peter Ujfalusi wrote: > > > On 07/10/2020 18.55, Vinod Koul wrote: > > On 07-10-20, 11:08, Peter Ujfalusi wrote: > > > >> Not really. In DT an event triggered channel can be requested via router > >> (when this is used) for example: > >> > >> dmas = <&inta_l2g a b c>; > >> a - the input number of the DMA request in l2g > >> b - edge or level trigger to be selected > >> c - ASEL number for the channel for coherency > >> > >> The l2g router driver then translate this to: > >> <&main_bcdma 1 0 c> > >> 1 - Global trigger 0 is used by the DMA > >> 0 - ignored > >> c - ASEL number. > >> > >> The router needs to send an event which is going to be received by the > >> channel we have picked up, this event number can only be known when we > >> do have the channel. > >> > >> So the flow in this case: > >> router converts the dma_spec for the DMA, but it does not yet know what > >> is the event number it has to use. > >> The BCDMA driver will pick an available bchan and notes that the > >> transfers will be triggered by global event 0. > >> When we have the channel, the core saves the router information and > >> calls the device_router_config of BCDMA. > >> In there we call back to the router and give the event number it has to > >> use to send the trigger for the channel. > > > > Ah that is intresting, so you would call router driver foo_set_event() > > and would send the event number > > Yes, that's correct. > > > why not call that API from alloc > > channel or even xlate? > > at alloc / xlate time the DMA driver does not have information about > router. The alloc/xlate will result the channel, but in my case it will > result a broken setup as the router does not know which event to send. > > > Why do you need new callback? > > When I added the DMA event router support, it was designed in a way that > the DMA driver itself must not know anything about the router, it has to > be transparent. One can just add a router on front of any DMA and > everything will work. > This is the right thing to do, and it works for existing setups. > > > Or did i miss something.. > > The BCDMA triggered channel setup is a chicken-egg setup. > For this case the channel can be triggered by a global event. A channel > can receive two global event, but this is not a concern atm. > The event number depends on the channel we use, for simplicity let's > say: bchan_id + trigger_offset = bchan_trigger_evt. > > of_dma_router_xlate does this: > > 1. calls the dma router's of_dma_route_allocate callback to allocate a > route and craft a dma_spec for the DMA to configure a channel. > > 2. using this crafted dma_spec we request a channel via of_dma_xlate > callback > > 3. if we got the channel, we save the router information, so it can be > deallocated when the channel is disabled. > > I need a fourth step to do a final configuration since only at this time > (after it has been allocated) the channel has information about possible > router. > > In the new optional callback the DMA driver can figure out the event > number which must be used by the router to send the event to the desired > global event target of the channel. > > Other DMAs might need something different, but imho if there is going to > be a need for such post alloc router config, then it is most likely will > come from the need to feed back some sort of channel information to the > router. Or take parameter from the router itself for the channel. > > To summarize: > In of_dma_route_allocate() the router does not yet know the channel we > are going to get. > In of_dma_xlate() the DMA driver does not yet know if the channel will > use router or not. > I need to tell the router the event number it has to send, which is > based on the channel number I got. Sounds reasonable, btw why not pass this information in xlate. Router will have a different xlate rather than non router right, or is it same. If this information is anyway available in DT might be better to get it and use from DT Thanks
Hi Vinod, On 28/10/2020 7.55, Vinod Koul wrote: >> To summarize: >> In of_dma_route_allocate() the router does not yet know the channel we >> are going to get. >> In of_dma_xlate() the DMA driver does not yet know if the channel will >> use router or not. >> I need to tell the router the event number it has to send, which is >> based on the channel number I got. > > Sounds reasonable, btw why not pass this information in xlate. Router > will have a different xlate rather than non router right, or is it same. Yes, the router's have their separate xlate, but in that xlate we do not yet have a channel. I don't know what is the event I need to send from the router to trigger the channel. > If this information is anyway available in DT might be better to get it > and use from DT Without a channel number I can not do anything. It is close to a chicken and egg problem. - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Hey Peter, On 28-10-20, 11:56, Peter Ujfalusi wrote: > Hi Vinod, > > On 28/10/2020 7.55, Vinod Koul wrote: > > >> To summarize: > >> In of_dma_route_allocate() the router does not yet know the channel we > >> are going to get. > >> In of_dma_xlate() the DMA driver does not yet know if the channel will > >> use router or not. > >> I need to tell the router the event number it has to send, which is > >> based on the channel number I got. > > > > Sounds reasonable, btw why not pass this information in xlate. Router > > will have a different xlate rather than non router right, or is it same. > > Yes, the router's have their separate xlate, but in that xlate we do not > yet have a channel. I don't know what is the event I need to send from > the router to trigger the channel. > > > If this information is anyway available in DT might be better to get it > > and use from DT > > Without a channel number I can not do anything. > It is close to a chicken and egg problem. We get 'channel' in xlate, so wont that help? I think I am still missing something here :(
Hi Vinod, On 09/11/2020 13.45, Vinod Koul wrote: >> Without a channel number I can not do anything. >> It is close to a chicken and egg problem. > > We get 'channel' in xlate, so wont that help? I think I am still missing > something here :( Yes, we get channel in xlate, but we get the channel after ofdma->of_dma_route_allocate() of_dma_route_allocate() si the place where DMA routers create the dmaspec for the DMA controller to get a channel and they up until BCDMA did also the HW configuration to get the event routed. For a BCDMA channel we can have three triggers: Global trigger 0 for the channel Global trigger 1 for the channel Local trigger for the channel Every BCDMA channel have these triggers and for all of them they are the same (from the channel's pow). bchan0 can be triggered by global trigger 0 bchan1 can be triggered by global trigger 0 But these triggers are not the same ones, the real trigger depends on the router, which of it's input is converted to send out an event to trigger bchan0_trigger0 or to trigger bchan1_trigger0. When we got the channel with the dmaspec from the router driver then we need to tell the router driver that it needs to send a given event in order to trigger the channel that we got. We can not have traditional binding for BCDMA either where we would tell the bchan index to be used because depending on the resource allocation done within sysfw that exact channel might not be even available for us. - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
HI Peter, On 09-11-20, 14:09, Peter Ujfalusi wrote: > Hi Vinod, > > On 09/11/2020 13.45, Vinod Koul wrote: > >> Without a channel number I can not do anything. > >> It is close to a chicken and egg problem. > > > > We get 'channel' in xlate, so wont that help? I think I am still missing > > something here :( > > Yes, we get channel in xlate, but we get the channel after > ofdma->of_dma_route_allocate() That is correct, so you need this info in allocate somehow.. > of_dma_route_allocate() si the place where DMA routers create the > dmaspec for the DMA controller to get a channel and they up until BCDMA > did also the HW configuration to get the event routed. > > For a BCDMA channel we can have three triggers: > Global trigger 0 for the channel > Global trigger 1 for the channel > Local trigger for the channel > > Every BCDMA channel have these triggers and for all of them they are the > same (from the channel's pow). > bchan0 can be triggered by global trigger 0 > bchan1 can be triggered by global trigger 0 > > But these triggers are not the same ones, the real trigger depends on > the router, which of it's input is converted to send out an event to > trigger bchan0_trigger0 or to trigger bchan1_trigger0. > > When we got the channel with the dmaspec from the router driver then we > need to tell the router driver that it needs to send a given event in > order to trigger the channel that we got. > > We can not have traditional binding for BCDMA either where we would tell > the bchan index to be used because depending on the resource allocation > done within sysfw that exact channel might not be even available for us. > > - Péter > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 09/11/2020 14.23, Vinod Koul wrote: > HI Peter, > > On 09-11-20, 14:09, Peter Ujfalusi wrote: >> Hi Vinod, >> >> On 09/11/2020 13.45, Vinod Koul wrote: >>>> Without a channel number I can not do anything. >>>> It is close to a chicken and egg problem. >>> >>> We get 'channel' in xlate, so wont that help? I think I am still missing >>> something here :( >> >> Yes, we get channel in xlate, but we get the channel after >> ofdma->of_dma_route_allocate() > > That is correct, so you need this info in allocate somehow.. To know the event number the router must send to trigger the channel I need the router to 'craft' the dmaspec which can be used to request the channel. To request a bcdma channel to be triggered by global trigger 0: [A] <&main_bcdma 1 0 15> main_bcdma - phandle to BCDMA 1 - triggered by global trigger0 0 - ignored 15 - ASEL value A peripheral can not really use this binding directly as we need to configure the get the event to be sent to the given channel's trigger0. The binding for the router (l2g if INTA in this case): [B] <&inta_l2g 21 0 15> inta_l2g - phandle to therouter 21 - local event index (input event/signal) 0 - event detection mode (pulsed/rising) 15 - ASEL value The of_dma_router_xlate() receives the dmaspec for [B}, the router driver creates the dmaspec for [A]. The xlate can not request the channel first as it needs the dmaspec from the router to do so. - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c index 8a4f608904b9..ec00b20ae8e4 100644 --- a/drivers/dma/of-dma.c +++ b/drivers/dma/of-dma.c @@ -75,8 +75,18 @@ static struct dma_chan *of_dma_router_xlate(struct of_phandle_args *dma_spec, ofdma->dma_router->route_free(ofdma->dma_router->dev, route_data); } else { + int ret = 0; + chan->router = ofdma->dma_router; chan->route_data = route_data; + + if (chan->device->device_router_config) + ret = chan->device->device_router_config(chan); + + if (ret) { + dma_release_channel(chan); + chan = ERR_PTR(ret); + } } /* diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h index dd357a747780..d6197fe875af 100644 --- a/include/linux/dmaengine.h +++ b/include/linux/dmaengine.h @@ -800,6 +800,7 @@ struct dma_filter { * by tx_status * @device_alloc_chan_resources: allocate resources and return the * number of allocated descriptors + * @device_router_config: optional callback for DMA router configuration * @device_free_chan_resources: release DMA channel's resources * @device_prep_dma_memcpy: prepares a memcpy operation * @device_prep_dma_xor: prepares a xor operation @@ -874,6 +875,7 @@ struct dma_device { enum dma_residue_granularity residue_granularity; int (*device_alloc_chan_resources)(struct dma_chan *chan); + int (*device_router_config)(struct dma_chan *chan); void (*device_free_chan_resources)(struct dma_chan *chan); struct dma_async_tx_descriptor *(*device_prep_dma_memcpy)(
Additional configuration for the DMA event router might be needed for a channel which can not be done during device_alloc_chan_resources callback since the router information is not yet present for the drivers. If there is a need for additional configuration for the channel if DMA router is in use, then the driver can implement the device_router_config callback. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> --- drivers/dma/of-dma.c | 10 ++++++++++ include/linux/dmaengine.h | 2 ++ 2 files changed, 12 insertions(+)