diff mbox series

[v4,2/3] dmaengine: add peripheral configuration

Message ID 20201008123151.764238-3-vkoul@kernel.org (mailing list archive)
State Changes Requested
Headers show
Series dmaengine: Add support for QCOM GSI dma controller | expand

Commit Message

Vinod Koul Oct. 8, 2020, 12:31 p.m. UTC
Some complex dmaengine controllers have capability to program the
peripheral device, so pass on the peripheral configuration as part of
dma_slave_config

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 include/linux/dmaengine.h | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Peter Ujfalusi Oct. 9, 2020, 9:04 a.m. UTC | #1
On 08/10/2020 15.31, Vinod Koul wrote:
> Some complex dmaengine controllers have capability to program the
> peripheral device, so pass on the peripheral configuration as part of
> dma_slave_config
>
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>  include/linux/dmaengine.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 6fbd5c99e30c..a15dc2960f6d 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -418,6 +418,9 @@ enum dma_slave_buswidth {
>   * @slave_id: Slave requester id. Only valid for slave channels. The dma
>   * slave peripheral will have unique id as dma requester which need to be
>   * pass as slave config.
> + * @peripheral_config: peripheral configuration for programming peripheral
> + * for dmaengine transfer
> + * @peripheral_size: peripheral configuration buffer size
>   *
>   * This struct is passed in as configuration data to a DMA engine
>   * in order to set up a certain channel for DMA transport at runtime.
> @@ -443,6 +446,8 @@ struct dma_slave_config {
>  	u32 dst_port_window_size;
>  	bool device_fc;
>  	unsigned int slave_id;
> +	void *peripheral_config;
> +	size_t peripheral_size;

Do you foresee a need of src/dst pair of these?
If we do DEV_TO_DEV with different type of peripherals it is going to
cause issues.

AASRC to McASP and McASP to AASRC for example.

>  };
>  
>  /**
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Vinod Koul Oct. 9, 2020, 10:30 a.m. UTC | #2
Hi Peter,

On 09-10-20, 12:04, Peter Ujfalusi wrote:
> On 08/10/2020 15.31, Vinod Koul wrote:
> > Some complex dmaengine controllers have capability to program the
> > peripheral device, so pass on the peripheral configuration as part of
> > dma_slave_config
> >
> > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > ---
> >  include/linux/dmaengine.h | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> > index 6fbd5c99e30c..a15dc2960f6d 100644
> > --- a/include/linux/dmaengine.h
> > +++ b/include/linux/dmaengine.h
> > @@ -418,6 +418,9 @@ enum dma_slave_buswidth {
> >   * @slave_id: Slave requester id. Only valid for slave channels. The dma
> >   * slave peripheral will have unique id as dma requester which need to be
> >   * pass as slave config.
> > + * @peripheral_config: peripheral configuration for programming peripheral
> > + * for dmaengine transfer
> > + * @peripheral_size: peripheral configuration buffer size
> >   *
> >   * This struct is passed in as configuration data to a DMA engine
> >   * in order to set up a certain channel for DMA transport at runtime.
> > @@ -443,6 +446,8 @@ struct dma_slave_config {
> >  	u32 dst_port_window_size;
> >  	bool device_fc;
> >  	unsigned int slave_id;
> > +	void *peripheral_config;
> > +	size_t peripheral_size;
> 
> Do you foresee a need of src/dst pair of these?
> If we do DEV_TO_DEV with different type of peripherals it is going to
> cause issues.

Not really as the channel already has direction and this is per channel.
If for any any reason subsequent txn is for different direction, I would
expect that parameters are set again before prep_ calls
Peter Ujfalusi Oct. 9, 2020, 10:45 a.m. UTC | #3
Hi Vinod,

On 09/10/2020 13.30, Vinod Koul wrote:
> Hi Peter,
> 
> On 09-10-20, 12:04, Peter Ujfalusi wrote:
>> On 08/10/2020 15.31, Vinod Koul wrote:
>>> Some complex dmaengine controllers have capability to program the
>>> peripheral device, so pass on the peripheral configuration as part of
>>> dma_slave_config
>>>
>>> Signed-off-by: Vinod Koul <vkoul@kernel.org>
>>> ---
>>>  include/linux/dmaengine.h | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
>>> index 6fbd5c99e30c..a15dc2960f6d 100644
>>> --- a/include/linux/dmaengine.h
>>> +++ b/include/linux/dmaengine.h
>>> @@ -418,6 +418,9 @@ enum dma_slave_buswidth {
>>>   * @slave_id: Slave requester id. Only valid for slave channels. The dma
>>>   * slave peripheral will have unique id as dma requester which need to be
>>>   * pass as slave config.
>>> + * @peripheral_config: peripheral configuration for programming peripheral
>>> + * for dmaengine transfer
>>> + * @peripheral_size: peripheral configuration buffer size
>>>   *
>>>   * This struct is passed in as configuration data to a DMA engine
>>>   * in order to set up a certain channel for DMA transport at runtime.
>>> @@ -443,6 +446,8 @@ struct dma_slave_config {
>>>  	u32 dst_port_window_size;
>>>  	bool device_fc;
>>>  	unsigned int slave_id;
>>> +	void *peripheral_config;
>>> +	size_t peripheral_size;
>>
>> Do you foresee a need of src/dst pair of these?
>> If we do DEV_TO_DEV with different type of peripherals it is going to
>> cause issues.
> 
> Not really as the channel already has direction and this is per channel.

Yes, in case of DEV_TO_MEM or MEM_TO_DEV.

> If for any any reason subsequent txn is for different direction, I would
> expect that parameters are set again before prep_ calls

But in DEV_TO_DEV?
If we have two peripherals, both needs config:
p1_config and p2_config

What and how would one use the single peripheral_config?

If only one of them needs config, then sure, the driver can pin-point
which one the single config might apply to.

Or you chain the same type of peripheral and you would need different
config for tx and rx?

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Vinod Koul Oct. 9, 2020, 11:15 a.m. UTC | #4
On 09-10-20, 13:45, Peter Ujfalusi wrote:
> Hi Vinod,
> 
> On 09/10/2020 13.30, Vinod Koul wrote:
> > Hi Peter,
> > 
> > On 09-10-20, 12:04, Peter Ujfalusi wrote:
> >> On 08/10/2020 15.31, Vinod Koul wrote:
> >>> Some complex dmaengine controllers have capability to program the
> >>> peripheral device, so pass on the peripheral configuration as part of
> >>> dma_slave_config
> >>>
> >>> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> >>> ---
> >>>  include/linux/dmaengine.h | 5 +++++
> >>>  1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> >>> index 6fbd5c99e30c..a15dc2960f6d 100644
> >>> --- a/include/linux/dmaengine.h
> >>> +++ b/include/linux/dmaengine.h
> >>> @@ -418,6 +418,9 @@ enum dma_slave_buswidth {
> >>>   * @slave_id: Slave requester id. Only valid for slave channels. The dma
> >>>   * slave peripheral will have unique id as dma requester which need to be
> >>>   * pass as slave config.
> >>> + * @peripheral_config: peripheral configuration for programming peripheral
> >>> + * for dmaengine transfer
> >>> + * @peripheral_size: peripheral configuration buffer size
> >>>   *
> >>>   * This struct is passed in as configuration data to a DMA engine
> >>>   * in order to set up a certain channel for DMA transport at runtime.
> >>> @@ -443,6 +446,8 @@ struct dma_slave_config {
> >>>  	u32 dst_port_window_size;
> >>>  	bool device_fc;
> >>>  	unsigned int slave_id;
> >>> +	void *peripheral_config;
> >>> +	size_t peripheral_size;
> >>
> >> Do you foresee a need of src/dst pair of these?
> >> If we do DEV_TO_DEV with different type of peripherals it is going to
> >> cause issues.
> > 
> > Not really as the channel already has direction and this is per channel.
> 
> Yes, in case of DEV_TO_MEM or MEM_TO_DEV.
> 
> > If for any any reason subsequent txn is for different direction, I would
> > expect that parameters are set again before prep_ calls
> 
> But in DEV_TO_DEV?

Do we support that :D

> If we have two peripherals, both needs config:
> p1_config and p2_config
> 
> What and how would one use the single peripheral_config?

Since the config is implementation specific, I do not think it limits.
You may create

struct peter_config {
        struct p1_config;
        struct p2_config;
};

> 
> If only one of them needs config, then sure, the driver can pin-point
> which one the single config might apply to.
> 
> Or you chain the same type of peripheral and you would need different
> config for tx and rx?
> 
> - Péter
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Peter Ujfalusi Oct. 9, 2020, 11:29 a.m. UTC | #5
On 09/10/2020 14.15, Vinod Koul wrote:
>>> If for any any reason subsequent txn is for different direction, I would
>>> expect that parameters are set again before prep_ calls
>>
>> But in DEV_TO_DEV?
> 
> Do we support that :D
> 
>> If we have two peripherals, both needs config:
>> p1_config and p2_config
>>
>> What and how would one use the single peripheral_config?
> 
> Since the config is implementation specific, I do not think it limits.
> You may create
> 
> struct peter_config {
>         struct p1_config;
>         struct p2_config;
> };

The use case is:
MEM -DMA-> P1 -DMA-> P2
or
P2 -DMA-> P1 -DMA-> MEM
or
MEM -DMA-> P2
or
P2 -DMA-> MEM
or
MEM -DMA-> P1 -DMA-> MEM

How would the DMA guess what it should do? How would the independent P1
and P2 would know how to set up the config?

>>
>> If only one of them needs config, then sure, the driver can pin-point
>> which one the single config might apply to.
>>
>> Or you chain the same type of peripheral and you would need different
>> config for tx and rx?
>>
>> - Péter
>>
>> 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
Vinod Koul Oct. 12, 2020, 6:09 a.m. UTC | #6
On 09-10-20, 14:29, Peter Ujfalusi wrote:
> 
> 
> On 09/10/2020 14.15, Vinod Koul wrote:
> >>> If for any any reason subsequent txn is for different direction, I would
> >>> expect that parameters are set again before prep_ calls
> >>
> >> But in DEV_TO_DEV?
> > 
> > Do we support that :D
> > 
> >> If we have two peripherals, both needs config:
> >> p1_config and p2_config
> >>
> >> What and how would one use the single peripheral_config?
> > 
> > Since the config is implementation specific, I do not think it limits.
> > You may create
> > 
> > struct peter_config {
> >         struct p1_config;
> >         struct p2_config;
> > };
> 
> The use case is:
> MEM -DMA-> P1 -DMA-> P2
> or
> P2 -DMA-> P1 -DMA-> MEM
> or
> MEM -DMA-> P2
> or
> P2 -DMA-> MEM
> or
> MEM -DMA-> P1 -DMA-> MEM
> 
> How would the DMA guess what it should do? How would the independent P1
> and P2 would know how to set up the config?

As I said, we do not support DEV_TO_DEV yet :)

Question is how would p1<-->p2 look, will p1 initiate a DMA txn or p2..?
who will configure these..

Do you have a real world example in horizon...
Peter Ujfalusi Oct. 12, 2020, 12:05 p.m. UTC | #7
On 12/10/2020 9.09, Vinod Koul wrote:
> On 09-10-20, 14:29, Peter Ujfalusi wrote:
>>
>>
>> On 09/10/2020 14.15, Vinod Koul wrote:
>>>>> If for any any reason subsequent txn is for different direction, I would
>>>>> expect that parameters are set again before prep_ calls
>>>>
>>>> But in DEV_TO_DEV?
>>>
>>> Do we support that :D
>>>
>>>> If we have two peripherals, both needs config:
>>>> p1_config and p2_config
>>>>
>>>> What and how would one use the single peripheral_config?
>>>
>>> Since the config is implementation specific, I do not think it limits.
>>> You may create
>>>
>>> struct peter_config {
>>>         struct p1_config;
>>>         struct p2_config;
>>> };
>>
>> The use case is:
>> MEM -DMA-> P1 -DMA-> P2
>> or
>> P2 -DMA-> P1 -DMA-> MEM
>> or
>> MEM -DMA-> P2
>> or
>> P2 -DMA-> MEM
>> or
>> MEM -DMA-> P1 -DMA-> MEM
>>
>> How would the DMA guess what it should do? How would the independent P1
>> and P2 would know how to set up the config?
> 
> As I said, we do not support DEV_TO_DEV yet :)
> 
> Question is how would p1<-->p2 look, will p1 initiate a DMA txn or p2..?
> who will configure these..

That's a good question, I have not really thought about that.
If we have MEM in the picture, then it is a bit cleaner, but I would guess.

> Do you have a real world example in horizon...

In j721e we have AASRC module which needs special PDMA configuration to
match with it's setup, AASRC can be chained with McASP, which in turn
have different type of PDMA.

- Péter

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

Patch

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 6fbd5c99e30c..a15dc2960f6d 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -418,6 +418,9 @@  enum dma_slave_buswidth {
  * @slave_id: Slave requester id. Only valid for slave channels. The dma
  * slave peripheral will have unique id as dma requester which need to be
  * pass as slave config.
+ * @peripheral_config: peripheral configuration for programming peripheral
+ * for dmaengine transfer
+ * @peripheral_size: peripheral configuration buffer size
  *
  * This struct is passed in as configuration data to a DMA engine
  * in order to set up a certain channel for DMA transport at runtime.
@@ -443,6 +446,8 @@  struct dma_slave_config {
 	u32 dst_port_window_size;
 	bool device_fc;
 	unsigned int slave_id;
+	void *peripheral_config;
+	size_t peripheral_size;
 };
 
 /**