Message ID | 87vblknmw3.wl%kuninori.morimoto.gx@renesas.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
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
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 -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Vinod These are v3 of rcar-audmapp rework patches. rcar-audmapp is very simple DMAC. - usage is very limited - no interrupt rcar-audmapp works with rcar-dma, and almost all settings are done under rcar-dma side. These are based on topic/slave_caps_device_control_fix Kuninori Morimoto (2): dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v1 dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 drivers/dma/sh/Kconfig | 3 +- drivers/dma/sh/rcar-audmapp.c | 416 +++++++++++------------- include/linux/platform_data/dma-rcar-audmapp.h | 34 -- 3 files changed, 196 insertions(+), 257 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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?
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 -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Vinod > These are v3 of rcar-audmapp rework patches. > rcar-audmapp is very simple DMAC. > - usage is very limited > - no interrupt > rcar-audmapp works with rcar-dma, and almost all settings > are done under rcar-dma side. > These are based on topic/slave_caps_device_control_fix > > Kuninori Morimoto (2): > dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v1 > dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 > > drivers/dma/sh/Kconfig | 3 +- > drivers/dma/sh/rcar-audmapp.c | 416 +++++++++++------------- > include/linux/platform_data/dma-rcar-audmapp.h | 34 -- > 3 files changed, 196 insertions(+), 257 deletions(-) I had sent this v3 patch before finishing discussion. And, I noticed small bug on this v3. Please just ignore v3. I will re-send v4 after discussion. (and it needs Laurent's rcar-dma driver anyway) Best regards --- Kuninori Morimoto -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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?
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 -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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?
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 -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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/
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?
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.
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 -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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..
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 -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
Hi Vinod > > 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 Thank you for your help. These are Audio DMAC peri peri v3 patches. 1) removes old drivers, and 2) replaces it. These are based on vinod/topic/rcar Kuninori Morimoto (2): 1) dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v1 2) dmaengine: rcar-audmapp: independent from SH_DMAE_BASE v2 drivers/dma/sh/Kconfig | 13 +- drivers/dma/sh/Makefile | 2 +- drivers/dma/sh/rcar-audmapp.c | 416 +++++++++++------------- include/linux/platform_data/dma-rcar-audmapp.h | 34 -- 4 files changed, 202 insertions(+), 263 deletions(-) Best regards --- Kuninori Morimoto -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/dma/sh/Kconfig b/drivers/dma/sh/Kconfig index 0c43210..68310cf 100644 --- a/drivers/dma/sh/Kconfig +++ b/drivers/dma/sh/Kconfig @@ -46,3 +46,10 @@ config RCAR_HPB_DMAE depends on SH_DMAE_BASE help Enable support for the Renesas R-Car series DMA controllers. + +config RCAR_AUDMAC_PP + tristate "Renesas R-Car Audio DMAC Peripheral Peripheral support" + depends on ARCH_SHMOBILE || COMPILE_TEST + select RENESAS_DMA + help + Enable support for the Renesas R-Car Audio DMAC Peripheral Peripheral controllers. diff --git a/drivers/dma/sh/Makefile b/drivers/dma/sh/Makefile index aede7db..0a5cfdb 100644 --- a/drivers/dma/sh/Makefile +++ b/drivers/dma/sh/Makefile @@ -15,3 +15,4 @@ obj-$(CONFIG_SH_DMAE) += shdma.o obj-$(CONFIG_SUDMAC) += sudmac.o obj-$(CONFIG_RCAR_HPB_DMAE) += rcar-hpbdma.o +obj-$(CONFIG_RCAR_AUDMAC_PP) += rcar-audmapp.o diff --git a/drivers/dma/sh/rcar-audmapp.c b/drivers/dma/sh/rcar-audmapp.c new file mode 100644 index 0000000..13fd4de --- /dev/null +++ b/drivers/dma/sh/rcar-audmapp.c @@ -0,0 +1,338 @@ +/* + * This is for Renesas R-Car Audio-DMAC-peri-peri. + * + * Copyright (C) 2014 Renesas Electronics Corporation + * Copyright (C) 2014 Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> + * + * based on the drivers/dma/sh/rcar-dmac.c + * + * Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com> + * + * This is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + */ +#include <linux/delay.h> +#include <linux/module.h> +#include <linux/of_dma.h> +#include <linux/platform_device.h> +#include "../dmaengine.h" + +/* + * DMA register + */ +#define PDMASAR 0x00 +#define PDMADAR 0x04 +#define PDMACHCR 0x0c + +/* PDMACHCR */ +#define PDMACHCR_DE (1 << 0) + +#define AUDMAPP_MAX_CHANNELS 29 + +/* Default MEMCPY transfer size = 2^2 = 4 bytes */ +#define LOG2_DEFAULT_XFER_SIZE 2 + +struct audmapp_priv; +struct audmapp_chan { + struct dma_chan chan; + struct dma_async_tx_descriptor async_tx; + + dma_addr_t src; + dma_addr_t dst; + u32 chcr; + + int id; +}; + +struct audmapp_priv { + struct dma_device dma; + void __iomem *achan_reg; + + struct audmapp_chan achan[AUDMAPP_MAX_CHANNELS]; +}; + +#define chan_to_achan(chan) container_of(chan, struct audmapp_chan, chan) +#define achan_to_priv(achan) container_of(achan - achan->id, \ + struct audmapp_priv, achan[0]) + +#define priv_to_dev(priv) ((priv)->dma.dev) +#define priv_to_dma(priv) (&(priv)->dma) + +#define audmapp_reg(achan, _reg) (achan_to_priv(achan)->achan_reg + \ + 0x20 + (0x10 * achan->id) + _reg) + +#define audmapp_for_each_achan(achan, priv, i) \ + for (i = 0; \ + (i < AUDMAPP_MAX_CHANNELS && ((achan) = priv->achan + i)); \ + i++) + +static void audmapp_write(struct audmapp_chan *achan, u32 data, u32 _reg) +{ + struct audmapp_priv *priv = achan_to_priv(achan); + struct device *dev = priv_to_dev(priv); + void __iomem *reg = audmapp_reg(achan, _reg); + + dev_dbg(dev, "w %p : %08x\n", reg, data); + + iowrite32(data, reg); +} + +static u32 audmapp_read(struct audmapp_chan *achan, u32 _reg) +{ + return ioread32(audmapp_reg(achan, _reg)); +} + +static void audmapp_halt(struct audmapp_chan *achan) +{ + int i; + + audmapp_write(achan, 0, PDMACHCR); + + for (i = 0; i < 1024; i++) { + if (0 == audmapp_read(achan, PDMACHCR)) + return; + udelay(1); + } +} + +static int audmapp_alloc_chan_resources(struct dma_chan *chan) +{ + if (chan->private) + return -ENODEV; + + chan->private = chan_to_achan(chan); + + return 0; +} + +static void audmapp_free_chan_resources(struct dma_chan *chan) +{ + chan->private = NULL; +} + +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; +} + +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; + 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); +} + +static bool audmapp_chan_filter(struct dma_chan *chan, void *arg) +{ + struct dma_device *dma = chan->device; + struct of_phandle_args *dma_spec = arg; + + /* + * FIXME: Using a filter on OF platforms is a nonsense. The OF xlate + * function knows from which device it wants to allocate a channel from, + * and would be perfectly capable of selecting the channel it wants. + * Forcing it to call dma_request_channel() and iterate through all + * channels from all controllers is just pointless. + */ + if (dma->device_control != audmapp_control || + dma_spec->np != dma->dev->of_node) + return false; + + /* + * see + * audmapp_alloc_chan_resources() + * audmapp_free_chan_resources() + */ + return !chan->private; +} + +static struct dma_chan *audmapp_of_xlate(struct of_phandle_args *dma_spec, + struct of_dma *ofdma) +{ + struct audmapp_chan *achan; + struct dma_chan *chan; + dma_cap_mask_t mask; + + if (dma_spec->args_count != 1) + return NULL; + + dma_cap_zero(mask); + dma_cap_set(DMA_SLAVE, mask); + + chan = dma_request_channel(mask, audmapp_chan_filter, dma_spec); + if (!chan) + return NULL; + + achan = chan_to_achan(chan); + achan->chcr = dma_spec->args[0] << 16; + + return chan; +} + +static dma_cookie_t audmapp_tx_submit(struct dma_async_tx_descriptor *tx) +{ + return dma_cookie_assign(tx); +} + +static int audmapp_chan_desc_probe(struct platform_device *pdev, + struct audmapp_priv *priv) + +{ + struct dma_device *dma = priv_to_dma(priv); + struct device *dev = priv_to_dev(priv); + struct audmapp_chan *achan; + struct dma_chan *chan; + int i; + + audmapp_for_each_achan(achan, priv, i) { + chan = &achan->chan; + + achan->id = i; + + /* + * Initialize the DMA engine channel and add it to the DMA + * engine channels list. + */ + chan->private = NULL; + chan->device = dma; + dma_cookie_init(chan); + list_add_tail(&chan->device_node, &dma->channels); + + achan->async_tx.tx_submit = audmapp_tx_submit; + dma_async_tx_descriptor_init(&achan->async_tx, chan); + + dev_dbg(dev, "%02d : %p\n", i, audmapp_reg(achan, 0)); + } + + return 0; +} + +static int audmapp_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct audmapp_priv *priv; + struct dma_device *dma; + struct resource *res; + int ret; + + of_dma_controller_register(dev->of_node, audmapp_of_xlate, pdev); + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->achan_reg = devm_ioremap_resource(dev, res); + if (IS_ERR(priv->achan_reg)) + return PTR_ERR(priv->achan_reg); + + dev_dbg(dev, "%llx => %p\n", (u64)res->start, priv->achan_reg); + + dma = priv_to_dma(priv); + dma->copy_align = LOG2_DEFAULT_XFER_SIZE; + INIT_LIST_HEAD(&dma->channels); + dma_cap_set(DMA_SLAVE, dma->cap_mask); + + dma->device_alloc_chan_resources = audmapp_alloc_chan_resources; + dma->device_free_chan_resources = audmapp_free_chan_resources; + dma->device_prep_dma_cyclic = audmapp_prep_dma_cyclic; + dma->device_control = audmapp_control; + dma->device_tx_status = audmapp_tx_status; + dma->device_issue_pending = audmapp_issue_pending; + dma->dev = dev; + + platform_set_drvdata(pdev, priv); + + ret = audmapp_chan_desc_probe(pdev, priv); + if (ret) + return ret; + + ret = dma_async_device_register(dma); + if (ret) + return ret; + + dev_info(dev, "probed\n"); + + return ret; +} + +static int audmapp_remove(struct platform_device *pdev) +{ + struct audmapp_priv *priv = platform_get_drvdata(pdev); + struct dma_device *dma = priv_to_dma(priv); + + dma_async_device_unregister(dma); + + of_dma_controller_free(pdev->dev.of_node); + + return 0; +} + +static const struct of_device_id audmapp_of_match[] = { + { .compatible = "renesas,rcar-audmapp", }, + {}, +}; + +static struct platform_driver audmapp_driver = { + .probe = audmapp_probe, + .remove = audmapp_remove, + .driver = { + .name = "rcar-audmapp-engine", + .of_match_table = audmapp_of_match, + }, +}; +module_platform_driver(audmapp_driver); + +MODULE_AUTHOR("Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>"); +MODULE_DESCRIPTION("Renesas R-Car Audio DMAC peri-peri driver"); +MODULE_LICENSE("GPL");