* Re: [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2
2014-12-10 4:34 [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 Kuninori Morimoto
@ 2014-12-10 5:58 ` Vinod Koul
2014-12-10 7:05 ` Kuninori Morimoto
` (17 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Vinod Koul @ 2014-12-10 5:58 UTC (permalink / raw)
To: linux-sh
On Wed, Dec 10, 2014 at 04:34:21AM +0000, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> +static struct dma_async_tx_descriptor *
> +audmapp_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr,
> + size_t buf_len, size_t period_len,
> + enum dma_transfer_direction dir, unsigned long flags)
> +{
> + struct audmapp_chan *achan = chan_to_achan(chan);
> +
> + return &achan->async_tx;
> +}
I had commented last time as well. This doesnt look right. You are ignoring
all input parameters, so how does it work at all?
> +
> +static void audmapp_slave_config(struct audmapp_chan *achan,
> + struct dma_slave_config *cfg)
> +{
> + achan->src = cfg->src_addr;
> + achan->dst = cfg->dst_addr;
> +}
> +
> +static int audmapp_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
> + unsigned long arg)
> +{
> + struct audmapp_chan *achan = chan_to_achan(chan);
> +
> + switch (cmd) {
> + case DMA_TERMINATE_ALL:
> + audmapp_halt(achan);
> + break;
> + case DMA_SLAVE_CONFIG:
> + audmapp_slave_config(achan, (struct dma_slave_config *)arg);
> + break;
This needs to be updated to new APIs i have merged at
topic/slave_caps_device_control_fix
> + default:
> + return -ENXIO;
> + }
> +
> + return 0;
> +}
> +
> +static enum dma_status audmapp_tx_status(struct dma_chan *chan,
> + dma_cookie_t cookie,
> + struct dma_tx_state *txstate)
> +{
> + return dma_cookie_status(chan, cookie, txstate);
> +}
> +
> +static void audmapp_issue_pending(struct dma_chan *chan)
> +{
> + struct audmapp_chan *achan = chan_to_achan(chan);
> + struct audmapp_priv *priv = achan_to_priv(achan);
> + struct device *dev = priv_to_dev(priv);
> + u32 chcr = achan->chcr | PDMACHCR_DE;
> +
> + dev_dbg(dev, "src/dst/chcr = %pad/%pad/%08x\n",
> + &achan->src, &achan->dst, chcr);
> +
> + audmapp_write(achan, achan->src, PDMASAR);
> + audmapp_write(achan, achan->dst, PDMADAR);
> + audmapp_write(achan, chcr, PDMACHCR);
no locking?
I don't see any interrupt code, does that get done by some other lib code?
something seems missing here
--
~Vinod
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2
2014-12-10 4:34 [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 Kuninori Morimoto
2014-12-10 5:58 ` Vinod Koul
@ 2014-12-10 7:05 ` Kuninori Morimoto
2014-12-11 5:17 ` Vinod Koul
` (16 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kuninori Morimoto @ 2014-12-10 7:05 UTC (permalink / raw)
To: linux-sh
Hi Vinod
Thank you for youre review
Basically, this DMAC is very simple device,
it needs very few settings only.
> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > +static struct dma_async_tx_descriptor *
> > +audmapp_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr,
> > + size_t buf_len, size_t period_len,
> > + enum dma_transfer_direction dir, unsigned long flags)
> > +{
> > + struct audmapp_chan *achan = chan_to_achan(chan);
> > +
> > + return &achan->async_tx;
> > +}
> I had commented last time as well. This doesnt look right. You are ignoring
> all input parameters, so how does it work at all?
When we set simple settings to DMAC, then, it works automatically as cyclic transfer.
The usage of this DMA is very limited, so it is super simple.
In addition, this is 2nd DMA which is needed on sound.
1st DMA is controled by rcar-dmac.c
2nd DMA is this
almost all settings are set by 1st DMA, 2nd DMA is just relay.
The necessary settings of this DMAC is only "ID" and "in/out address".
We can get address from DMA_SLAVE_CONFIG,
and, ID from DT settings.
Thus, this dma_cyclic is nothing to do.
I will add this comment on v3, is that OK ?
> > +static int audmapp_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
> > + unsigned long arg)
> > +{
> > + struct audmapp_chan *achan = chan_to_achan(chan);
> > +
> > + switch (cmd) {
> > + case DMA_TERMINATE_ALL:
> > + audmapp_halt(achan);
> > + break;
> > + case DMA_SLAVE_CONFIG:
> > + audmapp_slave_config(achan, (struct dma_slave_config *)arg);
> > + break;
> This needs to be updated to new APIs i have merged at
> topic/slave_caps_device_control_fix
I see. will fix
> I don't see any interrupt code, does that get done by some other lib code?
> something seems missing here
This DMAC doesn't have interrupt.
Best regards
---
Kuninori Morimoto
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2
2014-12-10 4:34 [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 Kuninori Morimoto
2014-12-10 5:58 ` Vinod Koul
2014-12-10 7:05 ` Kuninori Morimoto
@ 2014-12-11 5:17 ` Vinod Koul
2014-12-11 5:31 ` Kuninori Morimoto
` (15 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Vinod Koul @ 2014-12-11 5:17 UTC (permalink / raw)
To: linux-sh
On Wed, Dec 10, 2014 at 07:05:27AM +0000, Kuninori Morimoto wrote:
>
> Hi Vinod
>
> Thank you for youre review
> Basically, this DMAC is very simple device,
> it needs very few settings only.
>
> > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > > +static struct dma_async_tx_descriptor *
> > > +audmapp_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr,
> > > + size_t buf_len, size_t period_len,
> > > + enum dma_transfer_direction dir, unsigned long flags)
> > > +{
> > > + struct audmapp_chan *achan = chan_to_achan(chan);
> > > +
> > > + return &achan->async_tx;
> > > +}
> > I had commented last time as well. This doesnt look right. You are ignoring
> > all input parameters, so how does it work at all?
>
> When we set simple settings to DMAC, then, it works automatically as cyclic transfer.
> The usage of this DMA is very limited, so it is super simple.
> In addition, this is 2nd DMA which is needed on sound.
> 1st DMA is controled by rcar-dmac.c
> 2nd DMA is this
> almost all settings are set by 1st DMA, 2nd DMA is just relay.
okay that needs to be called out explcitly. While reading driver it wasn't
very clear
So which one is the 1st DMA, how will the client configure these two DMAs?
--
~Vinod
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2
2014-12-10 4:34 [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 Kuninori Morimoto
` (2 preceding siblings ...)
2014-12-11 5:17 ` Vinod Koul
@ 2014-12-11 5:31 ` Kuninori Morimoto
2014-12-15 10:31 ` Kuninori Morimoto
` (14 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kuninori Morimoto @ 2014-12-11 5:31 UTC (permalink / raw)
To: linux-sh
Hi Vinod
> > When we set simple settings to DMAC, then, it works automatically as cyclic transfer.
> > The usage of this DMA is very limited, so it is super simple.
> > In addition, this is 2nd DMA which is needed on sound.
> > 1st DMA is controled by rcar-dmac.c
> > 2nd DMA is this
> > almost all settings are set by 1st DMA, 2nd DMA is just relay.
> okay that needs to be called out explicitly. While reading driver it wasn't
> very clear
> So which one is the 1st DMA, how will the client configure these two DMAs?
1st DMA is rcar-dma which was created by Laurent.
And it has been sent to you now.
${LINUX}/sound/soc/sh/rcar/core.c
is the user of these 2 dmac.
1st DMAC gets "from mem address", and "to reg address".
2nd DMAC gets "from reg address", and "to reg address".
DMAC's ID are came from DT.
Best regards
---
Kuninori Morimoto
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2
2014-12-10 4:34 [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 Kuninori Morimoto
` (3 preceding siblings ...)
2014-12-11 5:31 ` Kuninori Morimoto
@ 2014-12-15 10:31 ` Kuninori Morimoto
2014-12-15 14:54 ` Vinod Koul
` (13 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kuninori Morimoto @ 2014-12-15 10:31 UTC (permalink / raw)
To: linux-sh
Hi Vinod
> > > When we set simple settings to DMAC, then, it works automatically as cyclic transfer.
> > > The usage of this DMA is very limited, so it is super simple.
> > > In addition, this is 2nd DMA which is needed on sound.
> > > 1st DMA is controled by rcar-dmac.c
> > > 2nd DMA is this
> > > almost all settings are set by 1st DMA, 2nd DMA is just relay.
> > okay that needs to be called out explicitly. While reading driver it wasn't
> > very clear
> > So which one is the 1st DMA, how will the client configure these two DMAs?
>
> 1st DMA is rcar-dma which was created by Laurent.
> And it has been sent to you now.
>
> ${LINUX}/sound/soc/sh/rcar/core.c
> is the user of these 2 dmac.
> 1st DMAC gets "from mem address", and "to reg address".
> 2nd DMAC gets "from reg address", and "to reg address".
> DMAC's ID are came from DT.
Do you have some comments about this ?
Can I send v3 patch ?
Best regards
---
Kuninori Morimoto
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2
2014-12-10 4:34 [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 Kuninori Morimoto
` (4 preceding siblings ...)
2014-12-15 10:31 ` Kuninori Morimoto
@ 2014-12-15 14:54 ` Vinod Koul
2014-12-16 0:08 ` Kuninori Morimoto
` (12 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Vinod Koul @ 2014-12-15 14:54 UTC (permalink / raw)
To: linux-sh
On Thu, Dec 11, 2014 at 05:31:05AM +0000, Kuninori Morimoto wrote:
>
> Hi Vinod
>
> > > When we set simple settings to DMAC, then, it works automatically as cyclic transfer.
> > > The usage of this DMA is very limited, so it is super simple.
> > > In addition, this is 2nd DMA which is needed on sound.
> > > 1st DMA is controled by rcar-dmac.c
> > > 2nd DMA is this
> > > almost all settings are set by 1st DMA, 2nd DMA is just relay.
> > okay that needs to be called out explicitly. While reading driver it wasn't
> > very clear
> > So which one is the 1st DMA, how will the client configure these two DMAs?
>
> 1st DMA is rcar-dma which was created by Laurent.
> And it has been sent to you now.
>
> ${LINUX}/sound/soc/sh/rcar/core.c
> is the user of these 2 dmac.
> 1st DMAC gets "from mem address", and "to reg address".
> 2nd DMAC gets "from reg address", and "to reg address".
> DMAC's ID are came from DT.
So how is the user expectations, will they configure both the engines?
--
~Vinod
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2
2014-12-10 4:34 [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 Kuninori Morimoto
` (5 preceding siblings ...)
2014-12-15 14:54 ` Vinod Koul
@ 2014-12-16 0:08 ` Kuninori Morimoto
2014-12-16 6:58 ` Vinod Koul
` (11 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kuninori Morimoto @ 2014-12-16 0:08 UTC (permalink / raw)
To: linux-sh
Hi Vinod
> > > > When we set simple settings to DMAC, then, it works automatically as cyclic transfer.
> > > > The usage of this DMA is very limited, so it is super simple.
> > > > In addition, this is 2nd DMA which is needed on sound.
> > > > 1st DMA is controled by rcar-dmac.c
> > > > 2nd DMA is this
> > > > almost all settings are set by 1st DMA, 2nd DMA is just relay.
> > > okay that needs to be called out explicitly. While reading driver it wasn't
> > > very clear
> > > So which one is the 1st DMA, how will the client configure these two DMAs?
> >
> > 1st DMA is rcar-dma which was created by Laurent.
> > And it has been sent to you now.
> >
> > ${LINUX}/sound/soc/sh/rcar/core.c
> > is the user of these 2 dmac.
> > 1st DMAC gets "from mem address", and "to reg address".
> > 2nd DMAC gets "from reg address", and "to reg address".
> > DMAC's ID are came from DT.
> So how is the user expectations, will they configure both the engines?
This rcar-audmapp driver is very limited usage, and user is only sound device/driver.
Sound driver configures both 1st/2nd DMAC if needed (it depends on platform).
Sound driver knows all reg address / mem address which are needed for 1st/2nd DMAC settings.
1st/2nd DMAC needs general DMAEngine settings method, not special.
Now, sound driver + 1st/2nd DMAC works well on my local environment.
Best regards
---
Kuninori Morimoto
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2
2014-12-10 4:34 [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 Kuninori Morimoto
` (6 preceding siblings ...)
2014-12-16 0:08 ` Kuninori Morimoto
@ 2014-12-16 6:58 ` Vinod Koul
2014-12-16 9:03 ` Kuninori Morimoto
` (10 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Vinod Koul @ 2014-12-16 6:58 UTC (permalink / raw)
To: linux-sh
On Tue, Dec 16, 2014 at 12:08:08AM +0000, Kuninori Morimoto wrote:
>
> Hi Vinod
>
> > > > > When we set simple settings to DMAC, then, it works automatically as cyclic transfer.
> > > > > The usage of this DMA is very limited, so it is super simple.
> > > > > In addition, this is 2nd DMA which is needed on sound.
> > > > > 1st DMA is controled by rcar-dmac.c
> > > > > 2nd DMA is this
> > > > > almost all settings are set by 1st DMA, 2nd DMA is just relay.
> > > > okay that needs to be called out explicitly. While reading driver it wasn't
> > > > very clear
> > > > So which one is the 1st DMA, how will the client configure these two DMAs?
> > >
> > > 1st DMA is rcar-dma which was created by Laurent.
> > > And it has been sent to you now.
> > >
> > > ${LINUX}/sound/soc/sh/rcar/core.c
> > > is the user of these 2 dmac.
> > > 1st DMAC gets "from mem address", and "to reg address".
> > > 2nd DMAC gets "from reg address", and "to reg address".
> > > DMAC's ID are came from DT.
> > So how is the user expectations, will they configure both the engines?
>
> This rcar-audmapp driver is very limited usage, and user is only sound device/driver.
> Sound driver configures both 1st/2nd DMAC if needed (it depends on platform).
> Sound driver knows all reg address / mem address which are needed for 1st/2nd DMAC settings.
> 1st/2nd DMAC needs general DMAEngine settings method, not special.
> Now, sound driver + 1st/2nd DMAC works well on my local environment.
Are you not using the sound dmaengine library then, right?
One more question, audio data will be in system memory and then it needs to
be transfered to periphral FIFO, how will data travel thru these two DMAs?
--
~Vinod
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2
2014-12-10 4:34 [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 Kuninori Morimoto
` (7 preceding siblings ...)
2014-12-16 6:58 ` Vinod Koul
@ 2014-12-16 9:03 ` Kuninori Morimoto
2014-12-16 9:11 ` Kuninori Morimoto
` (9 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kuninori Morimoto @ 2014-12-16 9:03 UTC (permalink / raw)
To: linux-sh
Hi Vinod
> > This rcar-audmapp driver is very limited usage, and user is only sound device/driver.
> > Sound driver configures both 1st/2nd DMAC if needed (it depends on platform).
> > Sound driver knows all reg address / mem address which are needed for 1st/2nd DMAC settings.
> > 1st/2nd DMAC needs general DMAEngine settings method, not special.
> > Now, sound driver + 1st/2nd DMAC works well on my local environment.
> Are you not using the sound dmaengine library then, right?
At this point, yes.
But, I got request from Mark to use it
# current dmaengine "method" on my driver is same as library.
# but I need to fix non-dmaengine area
> One more question, audio data will be in system memory and then it needs to
> be transfered to periphral FIFO, how will data travel thru these two DMAs?
like this
1st DMA 2nd DMA
[mem] -> [periphral IP-A] -> [periphral IP-B] -> speaker
Best regards
---
Kuninori Morimoto
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2
2014-12-10 4:34 [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 Kuninori Morimoto
` (8 preceding siblings ...)
2014-12-16 9:03 ` Kuninori Morimoto
@ 2014-12-16 9:11 ` Kuninori Morimoto
2014-12-16 16:45 ` Vinod Koul
` (8 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kuninori Morimoto @ 2014-12-16 9:11 UTC (permalink / raw)
To: linux-sh
Hi Vinod again
> > This rcar-audmapp driver is very limited usage, and user is only sound device/driver.
> > Sound driver configures both 1st/2nd DMAC if needed (it depends on platform).
> > Sound driver knows all reg address / mem address which are needed for 1st/2nd DMAC settings.
> > 1st/2nd DMAC needs general DMAEngine settings method, not special.
> > Now, sound driver + 1st/2nd DMAC works well on my local environment.
> Are you not using the sound dmaengine library then, right?
Do you mean this ?
linux/sound/soc/soc-generic-dmaengine-pcm.c
Best regards
---
Kuninori Morimoto
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2
2014-12-10 4:34 [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 Kuninori Morimoto
` (9 preceding siblings ...)
2014-12-16 9:11 ` Kuninori Morimoto
@ 2014-12-16 16:45 ` Vinod Koul
2014-12-16 16:51 ` Vinod Koul
` (7 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Vinod Koul @ 2014-12-16 16:45 UTC (permalink / raw)
To: linux-sh
On Tue, Dec 16, 2014 at 09:11:22AM +0000, Kuninori Morimoto wrote:
>
> Hi Vinod again
>
> > > This rcar-audmapp driver is very limited usage, and user is only sound device/driver.
> > > Sound driver configures both 1st/2nd DMAC if needed (it depends on platform).
> > > Sound driver knows all reg address / mem address which are needed for 1st/2nd DMAC settings.
> > > 1st/2nd DMAC needs general DMAEngine settings method, not special.
> > > Now, sound driver + 1st/2nd DMAC works well on my local environment.
> > Are you not using the sound dmaengine library then, right?
>
> Do you mean this ?
>
> linux/sound/soc/soc-generic-dmaengine-pcm.c
yes but IIRC this was move to sound/core/
--
~Vinod
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2
2014-12-10 4:34 [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 Kuninori Morimoto
` (10 preceding siblings ...)
2014-12-16 16:45 ` Vinod Koul
@ 2014-12-16 16:51 ` Vinod Koul
2014-12-16 19:41 ` Laurent Pinchart
` (6 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Vinod Koul @ 2014-12-16 16:51 UTC (permalink / raw)
To: linux-sh
On Tue, Dec 16, 2014 at 09:03:38AM +0000, Kuninori Morimoto wrote:
>
> Hi Vinod
>
> > > This rcar-audmapp driver is very limited usage, and user is only sound device/driver.
> > > Sound driver configures both 1st/2nd DMAC if needed (it depends on platform).
> > > Sound driver knows all reg address / mem address which are needed for 1st/2nd DMAC settings.
> > > 1st/2nd DMAC needs general DMAEngine settings method, not special.
> > > Now, sound driver + 1st/2nd DMAC works well on my local environment.
> > Are you not using the sound dmaengine library then, right?
>
> At this point, yes.
> But, I got request from Mark to use it
Yes that is the subsytem recommendation
> # current dmaengine "method" on my driver is same as library.
> # but I need to fix non-dmaengine area
>
> > One more question, audio data will be in system memory and then it needs to
> > be transfered to periphral FIFO, how will data travel thru these two DMAs?
>
> like this
>
> 1st DMA 2nd DMA
> [mem] -> [periphral IP-A] -> [periphral IP-B] -> speaker
Okay and by current implementation you wont be able to use the generic
dmaengine sound layer. That layer assumes you will have only one dmaengine
channel to configure and will configure only one DMA and which is 1st DMA
here.
So what do we do we 2nd one here. The point that 2nd DMA is behind the 1st
DMA, should the users know about it and configure? I am not sure...
If not, then who should configure this. Another candidate is 1st DMA, but
then from the point of 1st DMA should it configure 2nd DMA as well, treat it
as its salve... sounds plausible but am not too sure... comments?
--
~Vinod
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2
2014-12-10 4:34 [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 Kuninori Morimoto
` (11 preceding siblings ...)
2014-12-16 16:51 ` Vinod Koul
@ 2014-12-16 19:41 ` Laurent Pinchart
2014-12-17 0:27 ` Kuninori Morimoto
` (5 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Laurent Pinchart @ 2014-12-16 19:41 UTC (permalink / raw)
To: linux-sh
Hi Vinod,
On Tuesday 16 December 2014 22:09:04 Vinod Koul wrote:
> On Tue, Dec 16, 2014 at 09:03:38AM +0000, Kuninori Morimoto wrote:
> > Hi Vinod
> >
> > > > This rcar-audmapp driver is very limited usage, and user is only sound
> > > > device/driver. Sound driver configures both 1st/2nd DMAC if needed
> > > > (it depends on platform). Sound driver knows all reg address / mem
> > > > address which are needed for 1st/2nd DMAC settings. 1st/2nd DMAC
> > > > needs general DMAEngine settings method, not special. Now, sound
> > > > driver + 1st/2nd DMAC works well on my local environment.> >
> > > Are you not using the sound dmaengine library then, right?
> >
> > At this point, yes.
> > But, I got request from Mark to use it
>
> Yes that is the subsytem recommendation
>
> > # current dmaengine "method" on my driver is same as library.
> > # but I need to fix non-dmaengine area
> >
> > > One more question, audio data will be in system memory and then it needs
> > > to be transfered to periphral FIFO, how will data travel thru these two
> > > DMAs?
> >
> > like this
> >
> > 1st DMA 2nd DMA
> >
> > [mem] -> [periphral IP-A] -> [periphral IP-B] -> speaker
>
> Okay and by current implementation you wont be able to use the generic
> dmaengine sound layer. That layer assumes you will have only one dmaengine
> channel to configure and will configure only one DMA and which is 1st DMA
> here.
>
> So what do we do we 2nd one here. The point that 2nd DMA is behind the 1st
> DMA, should the users know about it and configure? I am not sure...
> If not, then who should configure this. Another candidate is 1st DMA, but
> then from the point of 1st DMA should it configure 2nd DMA as well, treat it
> as its salve... sounds plausible but am not too sure... comments?
The second DMA is not a slave of the first one, given that there's a
peripheral in-between.
Not that I'm advocating for it, but given how simple the second DMA engine is,
and given that it is dedicated to a single function (transferring data between
sound-related IPs, nothing else is possible) and can't thus be used as a
general purpose DMA engine, I wouldn't mind handling it internally as part of
the sound drivers instead of exposing it through the DMA engine API.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2
2014-12-10 4:34 [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 Kuninori Morimoto
` (12 preceding siblings ...)
2014-12-16 19:41 ` Laurent Pinchart
@ 2014-12-17 0:27 ` Kuninori Morimoto
2014-12-18 20:49 ` Laurent Pinchart
` (4 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kuninori Morimoto @ 2014-12-17 0:27 UTC (permalink / raw)
To: linux-sh
Hi Vinod, Laurent
> On Tuesday 16 December 2014 22:09:04 Vinod Koul wrote:
> > On Tue, Dec 16, 2014 at 09:03:38AM +0000, Kuninori Morimoto wrote:
> > > Hi Vinod
> > >
> > > > > This rcar-audmapp driver is very limited usage, and user is only sound
> > > > > device/driver. Sound driver configures both 1st/2nd DMAC if needed
> > > > > (it depends on platform). Sound driver knows all reg address / mem
> > > > > address which are needed for 1st/2nd DMAC settings. 1st/2nd DMAC
> > > > > needs general DMAEngine settings method, not special. Now, sound
> > > > > driver + 1st/2nd DMAC works well on my local environment.> >
> > > > Are you not using the sound dmaengine library then, right?
> > >
> > > At this point, yes.
> > > But, I got request from Mark to use it
> >
> > Yes that is the subsytem recommendation
> >
> > > # current dmaengine "method" on my driver is same as library.
> > > # but I need to fix non-dmaengine area
> > >
> > > > One more question, audio data will be in system memory and then it needs
> > > > to be transfered to periphral FIFO, how will data travel thru these two
> > > > DMAs?
> > >
> > > like this
> > >
> > > 1st DMA 2nd DMA
> > >
> > > [mem] -> [periphral IP-A] -> [periphral IP-B] -> speaker
> >
> > Okay and by current implementation you wont be able to use the generic
> > dmaengine sound layer. That layer assumes you will have only one dmaengine
> > channel to configure and will configure only one DMA and which is 1st DMA
> > here.
> >
> > So what do we do we 2nd one here. The point that 2nd DMA is behind the 1st
> > DMA, should the users know about it and configure? I am not sure...
> > If not, then who should configure this. Another candidate is 1st DMA, but
> > then from the point of 1st DMA should it configure 2nd DMA as well, treat it
> > as its salve... sounds plausible but am not too sure... comments?
>
> The second DMA is not a slave of the first one, given that there's a
> peripheral in-between.
>
> Not that I'm advocating for it, but given how simple the second DMA engine is,
> and given that it is dedicated to a single function (transferring data between
> sound-related IPs, nothing else is possible) and can't thus be used as a
> general purpose DMA engine, I wouldn't mind handling it internally as part of
> the sound drivers instead of exposing it through the DMA engine API.
I need to explain more detail about data path.
Our sound has SSI/SRC/DVC, and the sound data paths are
pattern 1)
1st DMA
[mem] -> [SSI] -> speaker
pattern 2)
1st DMA 2nd DMA
[mem] -> [SRC] -> [SSI] -> speaker
pattern 3)
1st DMA 2nd DMA
[mem] -> [SRC] -> [DVC] -> [SSI] -> speaker
SRC : 9 channel
DVC : 3 channel
SSI : 9 channel
Data path is board/platform specific at this point.
(it is nice if we can select these pattern flexibility in the future)
Unfortunately, this channel size is depends on SoC,
and above is "playback" pattern only, we need to have "capture" pattern too.
(similar path, but different direction and different ID).
Now, it is controlled under sound driver, because sound knows all information
and, board/platform specific data path.
I thought that it is possible if sound driver uses 2 generic sound DMA layer,
but is it wrong ?
Best regards
---
Kuninori Morimoto
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2
2014-12-10 4:34 [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 Kuninori Morimoto
` (13 preceding siblings ...)
2014-12-17 0:27 ` Kuninori Morimoto
@ 2014-12-18 20:49 ` Laurent Pinchart
2014-12-19 0:27 ` Kuninori Morimoto
` (3 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Laurent Pinchart @ 2014-12-18 20:49 UTC (permalink / raw)
To: linux-sh
Hi Morimoto-san,
On Wednesday 17 December 2014 00:27:43 Kuninori Morimoto wrote:
> > On Tuesday 16 December 2014 22:09:04 Vinod Koul wrote:
> >> On Tue, Dec 16, 2014 at 09:03:38AM +0000, Kuninori Morimoto wrote:
> >>> Hi Vinod
> >>>
> >>>>> This rcar-audmapp driver is very limited usage, and user is only
> >>>>> sound device/driver. Sound driver configures both 1st/2nd DMAC if
> >>>>> needed (it depends on platform). Sound driver knows all reg
> >>>>> address / mem address which are needed for 1st/2nd DMAC settings.
> >>>>> 1st/2nd DMAC needs general DMAEngine settings method, not special.
> >>>>> Now, sound driver + 1st/2nd DMAC works well on my local
> >>>>> environment.
> >>>>
> >>>> Are you not using the sound dmaengine library then, right?
> >>>
> >>> At this point, yes.
> >>> But, I got request from Mark to use it
> >>
> >> Yes that is the subsytem recommendation
> >>
> >>> # current dmaengine "method" on my driver is same as library.
> >>> # but I need to fix non-dmaengine area
> >>>
> >>>> One more question, audio data will be in system memory and then it
> >>>> needs to be transfered to periphral FIFO, how will data travel thru
> >>>> these two DMAs?
> >>>
> >>> like this
> >>>
> >>> 1st DMA 2nd DMA
> >>>
> >>> [mem] -> [periphral IP-A] -> [periphral IP-B] -> speaker
> >>
> >> Okay and by current implementation you wont be able to use the generic
> >> dmaengine sound layer. That layer assumes you will have only one
> >> dmaengine channel to configure and will configure only one DMA and which
> >> is 1st DMA here.
> >>
> >> So what do we do we 2nd one here. The point that 2nd DMA is behind the
> >> 1st DMA, should the users know about it and configure? I am not sure...
> >> If not, then who should configure this. Another candidate is 1st DMA,
> >> but then from the point of 1st DMA should it configure 2nd DMA as well,
> >> treat it as its salve... sounds plausible but am not too sure...
> >> comments?
> >
> > The second DMA is not a slave of the first one, given that there's a
> > peripheral in-between.
> >
> > Not that I'm advocating for it, but given how simple the second DMA engine
> > is, and given that it is dedicated to a single function (transferring
> > data between sound-related IPs, nothing else is possible) and can't thus
> > be used as a general purpose DMA engine, I wouldn't mind handling it
> > internally as part of the sound drivers instead of exposing it through
> > the DMA engine API.
>
> I need to explain more detail about data path.
> Our sound has SSI/SRC/DVC, and the sound data paths are
>
> pattern 1)
> 1st DMA
> [mem] -> [SSI] -> speaker
>
> pattern 2)
> 1st DMA 2nd DMA
> [mem] -> [SRC] -> [SSI] -> speaker
>
> pattern 3)
> 1st DMA 2nd DMA
> [mem] -> [SRC] -> [DVC] -> [SSI] -> speaker
>
> SRC : 9 channel
> DVC : 3 channel
> SSI : 9 channel
>
> Data path is board/platform specific at this point.
> (it is nice if we can select these pattern flexibility in the future)
> Unfortunately, this channel size is depends on SoC,
> and above is "playback" pattern only, we need to have "capture" pattern too.
> (similar path, but different direction and different ID).
> Now, it is controlled under sound driver, because sound knows all
> information and, board/platform specific data path.
>
> I thought that it is possible if sound driver uses 2 generic sound DMA
> layer, but is it wrong ?
It's not wrong, but I wonder if it's the best solution.
Supporting the R-Car DMAC with a generic DMA engine driver is I think the best
solution, as the DMAC is a general-purpose DMA engine, not specific to sound.
There's no issue there.
The second DMA controller is specific to the sound subsystem. I thus wonder if
the additional complexity of supporting it through the DMA engine API (both in
terms of code complexity on the DMA driver side and the sound driver side and
in terms of DT bindings complexity) is worth it, or if it would be simpler and
cleaner to support it with a driver specific to R-Car sound. You're more
knowledgeable than I am on the subject, so I'll trust your judgment.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2
2014-12-10 4:34 [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 Kuninori Morimoto
` (14 preceding siblings ...)
2014-12-18 20:49 ` Laurent Pinchart
@ 2014-12-19 0:27 ` Kuninori Morimoto
2014-12-22 15:26 ` Vinod Koul
` (2 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kuninori Morimoto @ 2014-12-19 0:27 UTC (permalink / raw)
To: linux-sh
Hi Laurent, Vinod
> > pattern 1)
> > 1st DMA
> > [mem] -> [SSI] -> speaker
> >
> > pattern 2)
> > 1st DMA 2nd DMA
> > [mem] -> [SRC] -> [SSI] -> speaker
> >
> > pattern 3)
> > 1st DMA 2nd DMA
> > [mem] -> [SRC] -> [DVC] -> [SSI] -> speaker
> >
> > SRC : 9 channel
> > DVC : 3 channel
> > SSI : 9 channel
> >
> > Data path is board/platform specific at this point.
> > (it is nice if we can select these pattern flexibility in the future)
> > Unfortunately, this channel size is depends on SoC,
> > and above is "playback" pattern only, we need to have "capture" pattern too.
> > (similar path, but different direction and different ID).
> > Now, it is controlled under sound driver, because sound knows all
> > information and, board/platform specific data path.
> >
> > I thought that it is possible if sound driver uses 2 generic sound DMA
> > layer, but is it wrong ?
>
> It's not wrong, but I wonder if it's the best solution.
>
> Supporting the R-Car DMAC with a generic DMA engine driver is I think the best
> solution, as the DMAC is a general-purpose DMA engine, not specific to sound.
> There's no issue there.
>
> The second DMA controller is specific to the sound subsystem. I thus wonder if
> the additional complexity of supporting it through the DMA engine API (both in
> terms of code complexity on the DMA driver side and the sound driver side and
> in terms of DT bindings complexity) is worth it, or if it would be simpler and
> cleaner to support it with a driver specific to R-Car sound. You're more
> knowledgeable than I am on the subject, so I'll trust your judgment.
Yes, I agree to your opinion. R-Car DMAC should keep "general-purpose" DMA engine.
It is easy to control if 1st/2nd DMA was separated from sound driver point of view.
I guess sound HW which needs 2 DMAC is very rare case(?). I want "keep it simple"
Vinod, this means, we want to have 2 different DMA (= 1st/2nd) for sound.
1st DMA is general DMAC, 2nd DMA is sound specific DMAC.
What is you opinion ?
Best regards
---
Kuninori Morimoto
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2
2014-12-10 4:34 [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 Kuninori Morimoto
` (15 preceding siblings ...)
2014-12-19 0:27 ` Kuninori Morimoto
@ 2014-12-22 15:26 ` Vinod Koul
2014-12-24 1:39 ` Kuninori Morimoto
2014-12-24 5:35 ` Vinod Koul
18 siblings, 0 replies; 20+ messages in thread
From: Vinod Koul @ 2014-12-22 15:26 UTC (permalink / raw)
To: linux-sh
On Fri, Dec 19, 2014 at 12:27:17AM +0000, Kuninori Morimoto wrote:
>
> Hi Laurent, Vinod
>
> > > pattern 1)
> > > 1st DMA
> > > [mem] -> [SSI] -> speaker
> > >
> > > pattern 2)
> > > 1st DMA 2nd DMA
> > > [mem] -> [SRC] -> [SSI] -> speaker
> > >
> > > pattern 3)
> > > 1st DMA 2nd DMA
> > > [mem] -> [SRC] -> [DVC] -> [SSI] -> speaker
> > >
> > > SRC : 9 channel
> > > DVC : 3 channel
> > > SSI : 9 channel
> > >
> > > Data path is board/platform specific at this point.
> > > (it is nice if we can select these pattern flexibility in the future)
> > > Unfortunately, this channel size is depends on SoC,
> > > and above is "playback" pattern only, we need to have "capture" pattern too.
> > > (similar path, but different direction and different ID).
> > > Now, it is controlled under sound driver, because sound knows all
> > > information and, board/platform specific data path.
> > >
> > > I thought that it is possible if sound driver uses 2 generic sound DMA
> > > layer, but is it wrong ?
> >
> > It's not wrong, but I wonder if it's the best solution.
> >
> > Supporting the R-Car DMAC with a generic DMA engine driver is I think the best
> > solution, as the DMAC is a general-purpose DMA engine, not specific to sound.
> > There's no issue there.
> >
> > The second DMA controller is specific to the sound subsystem. I thus wonder if
> > the additional complexity of supporting it through the DMA engine API (both in
> > terms of code complexity on the DMA driver side and the sound driver side and
> > in terms of DT bindings complexity) is worth it, or if it would be simpler and
> > cleaner to support it with a driver specific to R-Car sound. You're more
> > knowledgeable than I am on the subject, so I'll trust your judgment.
>
> Yes, I agree to your opinion. R-Car DMAC should keep "general-purpose" DMA engine.
> It is easy to control if 1st/2nd DMA was separated from sound driver point of view.
> I guess sound HW which needs 2 DMAC is very rare case(?). I want "keep it simple"
>
> Vinod, this means, we want to have 2 different DMA (= 1st/2nd) for sound.
> 1st DMA is general DMAC, 2nd DMA is sound specific DMAC.
> What is you opinion ?
I think this makes sense. Going thru the driver, it was clear that we were
not really gaining anything for using dmaengine API here. So agree that lets
use dmanegine for 1st dmac thru dmaengine library and then configure this in
your sound driver..
--
~Vinod
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2
2014-12-10 4:34 [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 Kuninori Morimoto
` (16 preceding siblings ...)
2014-12-22 15:26 ` Vinod Koul
@ 2014-12-24 1:39 ` Kuninori Morimoto
2014-12-24 5:35 ` Vinod Koul
18 siblings, 0 replies; 20+ messages in thread
From: Kuninori Morimoto @ 2014-12-24 1:39 UTC (permalink / raw)
To: linux-sh
Hi Vinod, Laurent
> > > The second DMA controller is specific to the sound subsystem. I thus wonder if
> > > the additional complexity of supporting it through the DMA engine API (both in
> > > terms of code complexity on the DMA driver side and the sound driver side and
> > > in terms of DT bindings complexity) is worth it, or if it would be simpler and
> > > cleaner to support it with a driver specific to R-Car sound. You're more
> > > knowledgeable than I am on the subject, so I'll trust your judgment.
> >
> > Yes, I agree to your opinion. R-Car DMAC should keep "general-purpose" DMA engine.
> > It is easy to control if 1st/2nd DMA was separated from sound driver point of view.
> > I guess sound HW which needs 2 DMAC is very rare case(?). I want "keep it simple"
> >
> > Vinod, this means, we want to have 2 different DMA (= 1st/2nd) for sound.
> > 1st DMA is general DMAC, 2nd DMA is sound specific DMAC.
> > What is you opinion ?
> I think this makes sense. Going thru the driver, it was clear that we were
> not really gaining anything for using dmaengine API here. So agree that lets
> use dmanegine for 1st dmac thru dmaengine library and then configure this in
> your sound driver..
Thank you for your opinion.
My understanding is that we can replace rcar-audmapp driver same as before.
Current DMAEngine branch has fixup(?) branch, and we need to rebase to it.
And, this 2nd DMA needs 1st DMA which was created by Laurent.
So, I will send v3 patch if Laurent's DMA driver was accepted.
Best regards
---
Kuninori Morimoto
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2
2014-12-10 4:34 [PATCH 2/2 v2] dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 Kuninori Morimoto
` (17 preceding siblings ...)
2014-12-24 1:39 ` Kuninori Morimoto
@ 2014-12-24 5:35 ` Vinod Koul
18 siblings, 0 replies; 20+ messages in thread
From: Vinod Koul @ 2014-12-24 5:35 UTC (permalink / raw)
To: linux-sh
On Wed, Dec 24, 2014 at 01:39:05AM +0000, Kuninori Morimoto wrote:
>
> Hi Vinod, Laurent
>
> > > > The second DMA controller is specific to the sound subsystem. I thus wonder if
> > > > the additional complexity of supporting it through the DMA engine API (both in
> > > > terms of code complexity on the DMA driver side and the sound driver side and
> > > > in terms of DT bindings complexity) is worth it, or if it would be simpler and
> > > > cleaner to support it with a driver specific to R-Car sound. You're more
> > > > knowledgeable than I am on the subject, so I'll trust your judgment.
> > >
> > > Yes, I agree to your opinion. R-Car DMAC should keep "general-purpose" DMA engine.
> > > It is easy to control if 1st/2nd DMA was separated from sound driver point of view.
> > > I guess sound HW which needs 2 DMAC is very rare case(?). I want "keep it simple"
> > >
> > > Vinod, this means, we want to have 2 different DMA (= 1st/2nd) for sound.
> > > 1st DMA is general DMAC, 2nd DMA is sound specific DMAC.
> > > What is you opinion ?
> > I think this makes sense. Going thru the driver, it was clear that we were
> > not really gaining anything for using dmaengine API here. So agree that lets
> > use dmanegine for 1st dmac thru dmaengine library and then configure this in
> > your sound driver..
>
> Thank you for your opinion.
> My understanding is that we can replace rcar-audmapp driver same as before.
Use my next to rebase
> Current DMAEngine branch has fixup(?) branch, and we need to rebase to it.
> And, this 2nd DMA needs 1st DMA which was created by Laurent.
> So, I will send v3 patch if Laurent's DMA driver was accepted
I have the PULL request from Laurent, should be able to process todays
--
~Vinod
^ permalink raw reply [flat|nested] 20+ messages in thread