Re: [PATCH v2 1/3] dmaengine: add TI EDMA DMA engine driver
On Tue, 2012-08-21 at 14:43 -0400, Matt Porter wrote:
> Add a DMA engine driver for the TI EDMA controller. This driver
> is implemented as a wrapper around the existing DaVinci private
> DMA implementation. This approach allows for incremental conversion
> of each peripheral driver to the DMA engine API. The EDMA driver
> supports slave transfers but does not yet support cyclic transfers.
>
> Signed-off-by: Matt Porter
mostly looks decent and in shape.
> ---
> +config TI_EDMA
> + tristate "TI EDMA support"
> + depends on ARCH_DAVINCI
> + select DMA_ENGINE
> + select DMA_VIRTUAL_CHANNELS
> + default y
default should be n for new drivers
> + help
> + Enable support for the TI EDMA controller. This DMA
> + engine is found on TI DaVinci and AM33xx parts.
> +
> config ARCH_HAS_ASYNC_TX_FIND_CHANNEL
> bool
>
> +/* Max of 16 segments per channel to conserve PaRAM slots */
> +#define MAX_NR_SG16
> +#define EDMA_MAX_SLOTS MAX_NR_SG
> +#define EDMA_DESCRIPTORS 16
> +
> +struct edma_desc {
> + struct virt_dma_descvdesc;
> + struct list_headnode;
> +
dummy space?
> + int absync;
> + int pset_nr;
> + struct edmacc_param pset[0];
> +};
> +
> +struct edma_cc;
> +
> +struct edma_chan {
> + struct virt_dma_chanvchan;
> + struct list_headnode;
> + struct edma_desc*edesc;
> + struct edma_cc *ecc;
> + int ch_num;
> + boolalloced;
> + int slot[EDMA_MAX_SLOTS];
> +
> + dma_addr_t addr;
> + int addr_width;
> + int maxburst;
> +};
> +
> +/* Dispatch a queued descriptor to the controller (caller holds lock) */
> +static void edma_execute(struct edma_chan *echan)
> +{
> + struct virt_dma_desc *vdesc = vchan_next_desc(&echan->vchan);
> + struct edma_desc *edesc;
> + int i;
> +
> + if (!vdesc) {
> + echan->edesc = NULL;
> + return;
> + }
> +
> + list_del(&vdesc->node);
> +
> + echan->edesc = edesc = to_edma_desc(&vdesc->tx);
> +
> + /* Write descriptor PaRAM set(s) */
> + for (i = 0; i < edesc->pset_nr; i++) {
> + edma_write_slot(echan->slot[i], &edesc->pset[i]);
> + dev_dbg(echan->vchan.chan.device->dev,
> + "\n pset[%d]:\n"
> + " chnum\t%d\n"
> + " slot\t%d\n"
> + " opt\t%08x\n"
> + " src\t%08x\n"
> + " dst\t%08x\n"
> + " abcnt\t%08x\n"
> + " ccnt\t%08x\n"
> + " bidx\t%08x\n"
> + " cidx\t%08x\n"
> + " lkrld\t%08x\n",
> + i, echan->ch_num, echan->slot[i],
> + edesc->pset[i].opt,
> + edesc->pset[i].src,
> + edesc->pset[i].dst,
> + edesc->pset[i].a_b_cnt,
> + edesc->pset[i].ccnt,
> + edesc->pset[i].src_dst_bidx,
> + edesc->pset[i].src_dst_cidx,
> + edesc->pset[i].link_bcntrld);
> + /* Link to the previous slot if not the last set */
> + if (i != (edesc->pset_nr - 1))
> + edma_link(echan->slot[i], echan->slot[i+1]);
> + /* Final pset links to the dummy pset */
> + else
> + edma_link(echan->slot[i], echan->ecc->dummy_slot);
> + }
> +
> + edma_start(echan->ch_num);
> +}
> +
> +static int edma_terminate_all(struct edma_chan *echan)
> +{
> + unsigned long flags;
> + LIST_HEAD(head);
> +
> + spin_lock_irqsave(&echan->vchan.lock, flags);
> +
> + /*
> + * Stop DMA activity: we assume the callback will not be called
> + * after edma_dma() returns (even if it does, it will see
> + * echan->edesc is NULL and exit.)
> + */
> + if (echan->edesc) {
> + echan->edesc = NULL;
> + edma_stop(echan->ch_num);
> + }
> +
> + vchan_get_all_descriptors(&echan->vchan, &head);
> + spin_unlock_irqrestore(&echan->vchan.lock, flags);
> + vchan_dma_desc_free_list(&echan->vchan, &head);
> +
> + return 0;
> +}
> +
> +
> +static int edma_slave_config(struct edma_chan *echan,
> + struct dma_slave_config *config)
> +{
> + if ((config->src_addr_width > DMA_SLAVE_BUSWIDTH_4_BYTES) ||
> + (config->dst_addr_width > DMA_SLAVE_BUSWIDTH_4_BYTES))
> + return -EINVAL;
the indent needs help here
> +
> + if (config->direction == DMA_MEM_TO_DEV) {
> + if (config->dst_addr)
> + echan->addr = config->dst_a
Re: [PATCH v2 3/3] spi: spi-davinci: convert to DMA engine API
On Tue, 2012-08-21 at 14:43 -0400, Matt Porter wrote:
> Removes use of the DaVinci EDMA private DMA API and replaces
> it with use of the DMA engine API.
>
> Signed-off-by: Matt Porter
> ---
> + struct dma_slave_config dma_rx_conf = {
> + .direction = DMA_DEV_TO_MEM,
> + .src_addr = (unsigned long)dspi->pbase + SPIBUF,
> + .src_addr_width = data_type,
> + .src_maxburst = 1,
what does 1 mean in this context? We define as number of units that
needs to be transfered, so are you sure that you want only one unit to
be dma'ed in a single burst. that seems like killing your dmac,
shouldn't you be using larger bursts for a better dma performance?
--
~Vinod
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 3/3] spi: spi-davinci: convert to DMA engine API
On Wed, 2012-08-22 at 12:04 -0400, Matt Porter wrote: > for querying of these types of limitations. Right now, the > mmc driver implicitly knows that EDMA needs this restriction > but it's something that should be queried before calling > prep_slave(). that's something we need to add; exporting channel capabilities. We only tell that it slave or memcpy today, but we need to tell clients what are channel supported parameter ranges. -- ~Vinod ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v3 3/3] spi: spi-davinci: convert to DMA engine API
On Thu, 2012-08-30 at 10:43 -0400, Matt Porter wrote: > On Thu, Aug 30, 2012 at 07:46:32PM +0530, Sekhar Nori wrote: > > Hi Matt, > > > > On 8/23/2012 6:39 AM, Matt Porter wrote: > > > Removes use of the DaVinci EDMA private DMA API and replaces > > > it with use of the DMA engine API. > > > > > > Signed-off-by: Matt Porter > > > > I tried testing this patch on my OMAP-L138 EVM, but SPI fails to > > initialize after applying the patch. > > > > root@arago:~# dmesg | grep -i spi > > > > spi_davinci spi_davinci.1: request RX DMA channel failed > > > > Hi Sekhar, > > Most likely CONFIG_TI_EDMA is off as it defaults to off in the v3 > series. Try enabling this and if it's the problem then this error > path can be fixed to properly fallback to PIO only or fail to > initialize as needed. I didnt see any update on this one, is it okay, if so care to send a tested-by? -- ~Vinod Koul Intel Corp. ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v3 3/3] spi: spi-davinci: convert to DMA engine API
On Fri, 2012-08-31 at 21:32 +0530, Sekhar Nori wrote: > Hi Matt, > > On 8/30/2012 8:13 PM, Matt Porter wrote: > > On Thu, Aug 30, 2012 at 07:46:32PM +0530, Sekhar Nori wrote: > >> Hi Matt, > >> > >> On 8/23/2012 6:39 AM, Matt Porter wrote: > >>> Removes use of the DaVinci EDMA private DMA API and replaces > >>> it with use of the DMA engine API. > >>> > >>> Signed-off-by: Matt Porter > >> > >> I tried testing this patch on my OMAP-L138 EVM, but SPI fails to > >> initialize after applying the patch. > >> > >> root@arago:~# dmesg | grep -i spi > >> > >> spi_davinci spi_davinci.1: request RX DMA channel failed > >> > > > > Hi Sekhar, > > > > Most likely CONFIG_TI_EDMA is off as it defaults to off in the v3 > > series. Try enabling this and if it's the problem then this error > > path can be fixed to properly fallback to PIO only or fail to > > initialize as needed. > > Yes, this was the problem. Since the SPI driver now depends on > CONFIG_TI_EDMA for basic operation may be select CONFIG_TI_EDMA in > Kconfig if SPI is enabled? That should do until the fallback to PIO is > implemented. Then this should be enabled for SPI. Care to send a patch > > With that fixed, I tested SPI on OMAP-L138 EVM and it works great. > Tested a JFFS2 mount as well as some raw chardev read/write tests. > > Tested-by: Sekhar Nori > > Thanks, > Sekhar > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- ~Vinod Koul Intel Corp. ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v3 3/3] spi: spi-davinci: convert to DMA engine API
On Fri, 2012-08-31 at 22:01 +0530, Sekhar Nori wrote: > >> Yes, this was the problem. Since the SPI driver now depends on > >> CONFIG_TI_EDMA for basic operation may be select CONFIG_TI_EDMA in > >> Kconfig if SPI is enabled? That should do until the fallback to PIO > is > >> implemented. > > Then this should be enabled for SPI. Care to send a patch > > By 'this' you mean the Kconfig select? Then there should be no need of > a > new patch for this. It can be part of this patch itself, no? Either way is okay for me. -- ~Vinod Koul Intel Corp. ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v3 3/3] spi: spi-davinci: convert to DMA engine API
On Fri, 2012-08-31 at 22:32 +0530, Vinod Koul wrote:
> On Fri, 2012-08-31 at 22:01 +0530, Sekhar Nori wrote:
> > >> Yes, this was the problem. Since the SPI driver now depends on
> > >> CONFIG_TI_EDMA for basic operation may be select CONFIG_TI_EDMA in
> > >> Kconfig if SPI is enabled? That should do until the fallback to PIO
> > is
> > >> implemented.
> > > Then this should be enabled for SPI. Care to send a patch
> >
> > By 'this' you mean the Kconfig select? Then there should be no need of
> > a
> > new patch for this. It can be part of this patch itself, no?
> Either way is okay for me.
And am in good mood :)
Applied all with this as well
>From b5f14330590118e6a0659255476c0f24ab681e05 Mon Sep 17 00:00:00 2001
From: Vinod Koul
Date: Sat, 1 Sep 2012 06:29:22 +0530
Subject: [PATCH] spi: davici - make davinci select edma
Reported-by: Sekhar Nori
Signed-off-by: Vinod Koul
---
drivers/spi/Kconfig |1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 5f84b55..b9fb9a1 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -134,6 +134,7 @@ config SPI_DAVINCI
tristate "Texas Instruments DaVinci/DA8x/OMAP-L/AM1x SoC SPI
controller"
depends on ARCH_DAVINCI
select SPI_BITBANG
+ select TI_EDMA
help
SPI master controller for DaVinci/DA8x/OMAP-L/AM1x SPI modules.
--
1.7.9.5
--
~Vinod Koul
Intel Corp.
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RFC PATCH 10/13] spi: omap2-mcspi: dma_request_slave_channel() support for DT platforms
On Fri, 2012-09-21 at 14:37 -0400, Matt Porter wrote: > On Fri, Sep 21, 2012 at 08:42:47AM -0700, Tony Lindgren wrote: > > > > Can't we come up with a version of dma_request_slave_channel that works > > both ways for now: > > > > mcspi_dma->dma_rx = > > dma_request_slave_channel_compat(mask, omap_dma_filter_fn, &sig, > > &master->dev, > > mcspi_dma->dma_rx_ch_name); > > ... > > > > Then it's just question of patching away two lines later on rather than > > having to add all this if else to all the drivers first, then patching > > it away again. > > I think that something like that is workable with the implementation > simply checking for of_node to do the right thing. Yes, I think it would be better to have common API but underneath two implementations in transitional phase. -- ~Vinod ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RFC PATCH 08/13] mmc: omap_hsmmc: limit max_segs with the EDMA DMAC
On Fri, 2012-09-21 at 19:47 +0100, Russell King - ARM Linux wrote: > On Fri, Sep 21, 2012 at 10:45:29PM +0530, S, Venkatraman wrote: > > On Thu, Sep 20, 2012 at 8:13 PM, Matt Porter wrote: > > > The EDMA DMAC has a hardware limitation that prevents supporting > > > scatter gather lists with any number of segments. Since the EDMA > > > DMA Engine driver sets the maximum segments to 16, we do the > > > same. > > > > > > Note: this can be removed once the DMA Engine API supports an > > > API to query the DMAC's segment limitations. > > > > > > > I wouldn't want to bind the properties of EDMA to omap_hsmmc as this patch > > suggests. Why don't we have a max_segs property, which when explicitly > > specified > > in DT, will override the default ? > > Why not have a generic way that DMA engine can export these kinds of > properties? We discussed this at KS. I was of opinion that DMA engine should export controller and channel capabilities as part of the channel it returns. Some folks had an opinion that they already know how to use controller so may not be very helpful, but if it is going to help (which I think), i have a patch for this :) -- ~Vinod ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RFC PATCH 08/13] mmc: omap_hsmmc: limit max_segs with the EDMA DMAC
On Mon, 2012-10-01 at 12:39 -0400, Matt Porter wrote:
> Anything you can show at this point? ;) I'd be happy to drop the
> half-hack
> for a real API. If not, I'm going to carry that to v2 atm.
This is what I had done sometime back. Feel free to update
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 9c02a45..94ae006 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -86,11 +86,11 @@ enum dma_transaction_type {
* @DMA_DEV_TO_DEV: Slave mode & From Device to Device
*/
enum dma_transfer_direction {
- DMA_MEM_TO_MEM,
- DMA_MEM_TO_DEV,
- DMA_DEV_TO_MEM,
- DMA_DEV_TO_DEV,
- DMA_TRANS_NONE,
+ DMA_MEM_TO_MEM = 0x01,
+ DMA_MEM_TO_DEV = 0x02,
+ DMA_DEV_TO_MEM = 0x04,
+ DMA_DEV_TO_DEV = 0x08,
+ DMA_TRANS_NONE = 0x10,
};
/**
@@ -371,6 +371,41 @@ struct dma_slave_config {
unsigned int slave_id;
};
+enum dmaengine_apis {
+ DMAENGINE_MEMCPY= 0x0001,
+ DMAENGINE_XOR = 0x0002,
+ DMAENGINE_XOR_VAL = 0x0004,
+ DMAENGINE_PQ= 0x0008,
+ DMAENGINE_PQ_VAL= 0x0010,
+ DMAENGINE_MEMSET= 0x0020,
+ DMAENGINE_SLAVE = 0x0040,
+ DMAENGINE_CYCLIC= 0x0080,
+ DMAENGINE_INTERLEAVED = 0x0100,
+ DMAENGINE_SG= 0x0200,
+};
+
+/* struct dmaengine_chan_caps - expose capability of a channel
+ * Note: each channel can have same or different capabilities
+ *
+ * This primarily classifies capabilities into
+ * a) APIs/ops supported
+ * b) channel physical capabilities
+ *
+ * @ops: or'ed api capability
+ * @widths: channel widths supported
+ * @dirn: channel directions supported
+ * @bursts: bitmask of burst lengths supported
+ * @mux: configurable slave id or hard wired
+ * -1 for hard wired, otherwise valid positive slave id (including zero)
+ */
+struct dmaengine_chan_caps {
+ enum dmaengine_apis ops;
+ enum dma_slave_buswidth widths;
+ enum dma_transfer_direction dirn;
+ unsigned int dma_bursts;
+ int mux;
+};
+
static inline const char *dma_chan_name(struct dma_chan *chan)
{
return dev_name(&chan->dev->device);
@@ -534,6 +569,7 @@ struct dma_tx_state {
* struct with auxiliary transfer status information, otherwise the call
* will just return a simple status code
* @device_issue_pending: push pending transactions to hardware
+ * @device_channel_caps: return the capablities of channel
*/
struct dma_device {
@@ -602,6 +638,9 @@ struct dma_device {
dma_cookie_t cookie,
struct dma_tx_state *txstate);
void (*device_issue_pending)(struct dma_chan *chan);
+
+ struct dmaengine_chan_caps *(*device_channel_caps)(
+ struct dma_chan *chan);
};
static inline int dmaengine_device_control(struct dma_chan *chan,
--
~Vinod
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RFC PATCH 1/3] dmaengine: add dma_get_channel_caps()
On Tue, 2012-10-23 at 15:54 -0700, Dan Williams wrote:
> > +struct dmaengine_chan_caps {
> > + enum dmaengine_apis ops;
> > + int seg_nr;
> > + int seg_len;
> > +};
>
> This makes sense for the potentially dynamic capability properties
> that get set after configuration, but why do we need the operation
> types here? They can be retrieved from the parent device.
I was thinking that each channel can have different capabilities.
You can assign one channel for mempcy operations exclusively and some
others for slave usage exclusively.
I believe some h/w do have such assignment so would help in doing that.
--
Vinod Koul
Intel Corp.
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RFC PATCH 1/3] dmaengine: add dma_get_channel_caps()
On Tue, 2012-10-23 at 23:49 +0100, Grant Likely wrote:
> > +enum dmaengine_apis {
> > + DMAENGINE_MEMCPY= 0x0001,
> > + DMAENGINE_XOR = 0x0002,
> > + DMAENGINE_XOR_VAL = 0x0004,
> > + DMAENGINE_PQ= 0x0008,
> > + DMAENGINE_PQ_VAL= 0x0010,
> > + DMAENGINE_MEMSET= 0x0020,
> > + DMAENGINE_SLAVE = 0x0040,
> > + DMAENGINE_CYCLIC= 0x0080,
> > + DMAENGINE_INTERLEAVED = 0x0100,
> > + DMAENGINE_SG= 0x0200,
> > +};
>
> Actually, one more comment. Why the new enum? Why can't the
> dma_transaction_type enum be used directly along with dma_cap_mask_t?
Some of the capabilities above are not there in dma_caps_t like DMA_SG.
Also DMA_INTERRUPT and DMA_PRIVATE would not make much sense here.
BUT would help to keep things simpler if have one definition which
includes all.
--
Vinod Koul
Intel Corp.
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 0/3] dmaengine: add per channel capabilities api
On Thu, Jan 10, 2013 at 02:07:03PM -0500, Matt Porter wrote: > The call is implemented as follows: > > struct dmaengine_chan_caps > *dma_get_channel_caps(struct dma_chan *chan, > enum dma_transfer_direction dir); > > The dma transfer direction parameter may appear a bit out of place > but it is necessary since the direction field in struct > dma_slave_config was deprecated. In some cases, EDMA for one, it > is necessary for the dmaengine driver to have the burst and address > width slave configuration parameters available in order to compute > the maximum segment size that can be handle. Due to this requirement, > the calling order of this api is as follows: Well you are passing direction as argument so even in EDMA it doesn't seem to help you as you seem to need burst and width!. So why do you even need the direction to compute the capablities -- ~Vinod ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 1/3] dmaengine: add dma_get_channel_caps()
On Thu, Jan 10, 2013 at 02:07:04PM -0500, Matt Porter wrote:
> +/* struct dmaengine_chan_caps - expose capability of a channel
> + * Note: each channel can have same or different capabilities
> + *
> + * This primarily classifies capabilities into
> + * a) APIs/ops supported
> + * b) channel physical capabilities
> + *
> + * @cap_mask: api/ops capability (DMA_INTERRUPT and DMA_PRIVATE
> + * are invalid api/ops and will never be set)
> + * @seg_nr: maximum number of SG segments supported on a SG/SLAVE
> + * channel (0 for no maximum or not a SG/SLAVE channel)
> + * @seg_len: maximum length of SG segments supported on a SG/SLAVE
> + *channel (0 for no maximum or not a SG/SLAVE channel)
> + */
> +struct dmaengine_chan_caps {
> + dma_cap_mask_t cap_mask;
> + int seg_nr;
> + int seg_len;
> +};
Now am really unclear why we would need direction as argument.
Also, I would add the channel physical capablities like direction, widths,
lengths etc supported.
> +/**
> + * dma_get_channel_caps - flush pending transactions to HW
flush pending... ???
> + * driver does not implement per channel capbilities then
> + * NULL is returned.
> + */
> +static inline struct dmaengine_chan_caps
> +*dma_get_channel_caps(struct dma_chan *chan, enum dma_transfer_direction dir)
you need to add this for when CONFIG_DMA_ENGINE is not defined as well.
> +{
> + if (chan->device->device_channel_caps)
> + return chan->device->device_channel_caps(chan, dir);
> + return NULL;
> +}
> +
> enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie);
> #ifdef CONFIG_DMA_ENGINE
> enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx);
--
~Vinod
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 2/3] dma: edma: add device_channel_caps() support
On Thu, Jan 10, 2013 at 02:07:05PM -0500, Matt Porter wrote:
> Implement device_channel_caps().
>
> EDMA has a finite set of PaRAM slots available for linking
> a multi-segment SG transfer. In order to prevent any one
> channel from consuming all PaRAM slots to fulfill a large SG
> transfer, the driver reports a static per-channel max number
> of SG segments it will handle.
>
> The maximum size of SG segment is limited by the slave config
> maxburst and addr_width for the channel in question. These values
> are used from the current channel config to calculate and return
> the max segment length cap.
>
> Signed-off-by: Matt Porter
> ---
> drivers/dma/edma.c | 27 +++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
> index 82c8672..fc4b9db 100644
> --- a/drivers/dma/edma.c
> +++ b/drivers/dma/edma.c
> @@ -70,6 +70,7 @@ struct edma_chan {
> boolalloced;
> int slot[EDMA_MAX_SLOTS];
> struct dma_slave_config cfg;
> + struct dmaengine_chan_caps caps;
> };
>
> struct edma_cc {
> @@ -462,6 +463,28 @@ static void edma_issue_pending(struct dma_chan *chan)
> spin_unlock_irqrestore(&echan->vchan.lock, flags);
> }
>
> +static struct dmaengine_chan_caps
> +*edma_get_channel_caps(struct dma_chan *chan, enum dma_transfer_direction
> dir)
> +{
> + struct edma_chan *echan;
> + enum dma_slave_buswidth width = 0;
> + u32 burst = 0;
> +
> + if (chan) {
I think this check is redundant.
> + echan = to_edma_chan(chan);
> + if (dir == DMA_MEM_TO_DEV) {
> + width = echan->cfg.dst_addr_width;
> + burst = echan->cfg.dst_maxburst;
Per you API example burst and width is not set yet, so this doesn't make sense
> + } else if (dir == DMA_DEV_TO_MEM) {
> + width = echan->cfg.src_addr_width;
> + burst = echan->cfg.src_maxburst;
> + }
> + echan->caps.seg_len = (SZ_64K - 1) * width * burst;
Also the defination of API is max, above computation doesn't make sense to me!
--
~Vinod
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 0/3] dmaengine: add per channel capabilities api
On Sun, Jan 20, 2013 at 11:37:35AM -0500, Matt Porter wrote: > On Sun, Jan 20, 2013 at 12:37:34PM +0000, Vinod Koul wrote: > > On Thu, Jan 10, 2013 at 02:07:03PM -0500, Matt Porter wrote: > > > The call is implemented as follows: > > > > > > struct dmaengine_chan_caps > > > *dma_get_channel_caps(struct dma_chan *chan, > > > enum dma_transfer_direction dir); > > > > > > The dma transfer direction parameter may appear a bit out of place > > > but it is necessary since the direction field in struct > > > dma_slave_config was deprecated. In some cases, EDMA for one, it > > > is necessary for the dmaengine driver to have the burst and address > > > width slave configuration parameters available in order to compute > > > the maximum segment size that can be handle. Due to this requirement, > > > the calling order of this api is as follows: > > Well you are passing direction as argument so even in EDMA it doesn't seem > > to > > help you as you seem to need burst and width!. So why do you even need the > > direction to compute the capablities > > Yes, I need burst and width, but they are dependent on direction (dst vs > src, as stored in the slave channel config). Ok, so I think I know where > this is leading...the problem is probably that I made an implicit > dependency on burst and width here. The expectation in this And also due to wrong documentation. This is what you have put up the flow as: Due to this requirement, the calling order of this api is as follows: 1. Allocate a DMA slave channel 1a. [Optionally] Get channel capabilities 2. Set slave and controller specific parameters 3. Get a descriptor for transaction 4. Submit the transaction 5. Issue pending requests and wait for callback notification Now when we query capablities, slave parameters _are_not_set_. So seems like you have thought something and written something else! Which brings me to the point on what are we trying to query: a) API capability, dont need slave parameters for that b) Sg segment length and numbers: Well these are capabilities, so it tells you what is the maximum I can do. IMO it doesn't make sense to tie it down to burst, width etc. For that configuration you are checking maximum. What this needs to return is what is the maximum length it supports and maximum number of sg list the h/w can use. Also if you return your burst and width capablity, then any client can easily find out what is the length byte value it can hold. If you feel this computaion if client specific, though looking at doesnt make me think so, you can add a callback for this computaion given the parameters. -- ~Vinod ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 2/3] dma: edma: add device_channel_caps() support
On Sun, Jan 20, 2013 at 11:51:08AM -0500, Matt Porter wrote: > The explanation in the cover letter mentions that dmaengine_slave_config() is > required to be called prior to dmaengine_get_channel_caps(). If we > switch to the alternative API, then that would go away including the > dependency on direction. Nope you got that wrong! -- ~Vinod ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 0/3] dmaengine: add per channel capabilities api
On Mon, Jan 21, 2013 at 01:19:23PM -0500, Matt Porter wrote: > > b) Sg segment length and numbers: Well these are capabilities, so it tells > > you what is the maximum I can do. IMO it doesn't make sense to tie it down > > to > > burst, width etc. For that configuration you are checking maximum. What this > > needs to return is what is the maximum length it supports and maximum > > number of > > sg list the h/w can use. Also if you return your burst and width capablity, > > then > > any client can easily find out what is the length byte value it can hold. > > Ok, so I could report the max burst size to the client, but on EDMA it's > going to be SZ_64K-1, as that's the hw limit. Max addr width would also > be the same or capped at the maximum enum the current API support of 8 > bytes. Yes IMO you should report the h/w limit and let client deduce what can be done with that h/w limit given the other parameters (burst, length etc...) > > This information is not useful to the client in my case. Only the client > knows its FIFO size, and thus the actual burst size it needs to request > as that is a value specific to the IP the client driver controls. So it > knows actual burst size and the width of that fifo transfer, because we > already support telling the dmaengine driver this info in the current > API. Your current API way is not very generic. We need to make sure it useful on other systems too. That is why it would make sense to query the DMA H/W capablities and then use it with above properties to get various parameters used for initiating a transfer, that way you dont need to set slave parameters -- ~Vinod ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v7 07/10] dmaengine: add dma_request_slave_channel_compat()
On Fri, Feb 01, 2013 at 01:22:52PM -0500, Matt Porter wrote:
> Adds a dma_request_slave_channel_compat() wrapper which accepts
> both the arguments from dma_request_channel() and
> dma_request_slave_channel(). Based on whether the driver is
> instantiated via DT, the appropriate channel request call will be
> made.
>
> This allows for a much cleaner migration of drivers to the
> dmaengine DT API as platforms continue to be mixed between those
> that boot using DT and those that do not.
>
> Suggested-by: Tony Lindgren
> Signed-off-by: Matt Porter
> Acked-by: Tony Lindgren
> Acked-by: Arnd Bergmann
Acked-by: Vinod Koul
> ---
> include/linux/dmaengine.h | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index bfcdecb..17d8ffd 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -1001,6 +1001,22 @@ void dma_run_dependencies(struct
> dma_async_tx_descriptor *tx);
> struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type);
> struct dma_chan *net_dma_find_channel(void);
> #define dma_request_channel(mask, x, y) __dma_request_channel(&(mask), x, y)
> +#define dma_request_slave_channel_compat(mask, x, y, dev, name) \
> + __dma_request_slave_channel_compat(&(mask), x, y, dev, name)
> +
> +static inline struct dma_chan
> +*__dma_request_slave_channel_compat(dma_cap_mask_t *mask, dma_filter_fn fn,
> + void *fn_param, struct device *dev,
> + char *name)
> +{
> + struct dma_chan *chan;
> +
> + chan = dma_request_slave_channel(dev, name);
> + if (chan)
> + return chan;
> +
> + return __dma_request_channel(mask, fn, fn_param);
> +}
>
> /* --- Helper iov-locking functions --- */
>
> --
> 1.7.9.5
>
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v3 1/3] dmaengine: add dma_get_slave_sg_caps()
On Mon, Feb 04, 2013 at 02:47:02PM -0500, Matt Porter wrote:
> Add a dmaengine API to retrieve slave SG transfer capabilities.
>
> The API is optionally implemented by dmaengine drivers and when
> unimplemented will return a NULL pointer. A client driver using
> this API provides the required dma channel, address width, and
> burst size of the transfer. dma_get_slave_sg_caps() returns an
> SG caps structure with the maximum number and size of SG segments
> that the given channel can handle.
Okay this sounds much better :-)
few points though:
- you added API for caps, but is actually calculating for given configuration
the max allowed range. IMHO that is not caps, perhaps renaming to get_max_sg
/some_better_name would be more apt.
- Still I like the idea of caps, but it should give H/W support capablity. If
you want to add that, pls develop on same line...
--
~Vinod
>
> Signed-off-by: Matt Porter
> ---
> include/linux/dmaengine.h | 40
> 1 file changed, 40 insertions(+)
>
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index d3201e4..5b5b220 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -371,6 +371,19 @@ struct dma_slave_config {
> unsigned int slave_id;
> };
>
> +/* struct dma_slave_sg_caps - expose SG transfer capability of a
> + * channel.
> + *
> + * @max_seg_nr: maximum number of SG segments supported on a SG/SLAVE
> + * channel (0 for no maximum or not a SG/SLAVE channel)
> + * @max_seg_len: maximum length of SG segments supported on a SG/SLAVE
> + *channel (0 for no maximum or not a SG/SLAVE channel)
> + */
> +struct dma_slave_sg_caps {
> + u32 max_seg_nr;
> + u32 max_seg_len;
> +};
> +
> static inline const char *dma_chan_name(struct dma_chan *chan)
> {
> return dev_name(&chan->dev->device);
> @@ -534,6 +547,7 @@ struct dma_tx_state {
> * struct with auxiliary transfer status information, otherwise the call
> * will just return a simple status code
> * @device_issue_pending: push pending transactions to hardware
> + * @device_slave_sg_caps: return the slave SG capabilities
> */
> struct dma_device {
>
> @@ -602,6 +616,9 @@ struct dma_device {
> dma_cookie_t cookie,
> struct dma_tx_state *txstate);
> void (*device_issue_pending)(struct dma_chan *chan);
> + struct dma_slave_sg_caps *(*device_slave_sg_caps)(
> + struct dma_chan *chan, enum dma_slave_buswidth addr_width,
> + u32 maxburst);
> };
>
> static inline int dmaengine_device_control(struct dma_chan *chan,
> @@ -969,6 +986,29 @@ dma_set_tx_state(struct dma_tx_state *st, dma_cookie_t
> last, dma_cookie_t used,
> }
> }
>
> +/**
> + * dma_get_slave_sg_caps - get DMAC SG transfer capabilities
> + * @chan: target DMA channel
> + * @addr_width: address width of the DMA transfer
> + * @maxburst: maximum DMA transfer burst size
> + *
> + * Get SG transfer capabilities for a specified channel. If the dmaengine
> + * driver does not implement SG transfer capabilities then NULL is
> + * returned.
> + */
> +static inline struct dma_slave_sg_caps
> +*dma_get_slave_sg_caps(struct dma_chan *chan,
> +enum dma_slave_buswidth addr_width,
> +u32 maxburst)
> +{
> + if (chan->device->device_slave_sg_caps)
> + return chan->device->device_slave_sg_caps(chan,
> + addr_width,
> + maxburst);
> +
> + return NULL;
> +}
> +
> enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie);
> #ifdef CONFIG_DMA_ENGINE
> enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx);
> --
> 1.7.9.5
>
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v4 1/3] dmaengine: add dma_get_slave_sg_limits()
On Wed, Mar 06, 2013 at 02:56:05PM -0500, Matt Porter wrote:
> Add a dmaengine API to retrieve slave SG transfer limits.
>
> The API is optionally implemented by dmaengine drivers and when
> unimplemented will return a NULL pointer. A client driver using
> this API provides the required dma channel, address width, and
> burst size of the transfer. dma_get_slave_sg_limits() returns an
> SG limits structure with the maximum number and size of SG segments
> that the given channel can handle.
>
> Signed-off-by: Matt Porter
> ---
> +static inline struct dma_slave_sg_limits
> +*dma_get_slave_sg_limits(struct dma_chan *chan,
> +enum dma_slave_buswidth addr_width,
> +u32 maxburst)
> +{
> + if (chan->device->device_slave_sg_limits)
> + return chan->device->device_slave_sg_limits(chan,
> + addr_width,
> + maxburst);
Hi Matt,
Sorry for delayed reply, this series was sent just before i went for vacation :)
I agree with Lars, that returning pointer maynot be good idea. So this bit needs
work. Also the readblity of above is bad by complying to 80char limit. I would
make it easier to read
--
~Vinod
> +
> + return NULL;
> +}
> +
> enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie);
> #ifdef CONFIG_DMA_ENGINE
> enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx);
>
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v4 0/3] dmaengine: add slave sg transfer limits api
On Wed, Mar 06, 2013 at 02:56:04PM -0500, Matt Porter wrote: > Changes since v3: > - Change api name to dma_get_slave_sg_limits() to avoid > confusion with h/w caps which are static. > > Changes since v2: > - Change to a separate slave sg specific api. Drop the > generic per-channel capabilities api that is not used. > > Changes since v1: > - Use the existing dma_transaction_type enums instead of > adding the mostly duplicated dmaengine_apis enums > > This series adds a new dmaengine api, dma_get_slave_sg_limits(), which > may be used by a client driver to get slave SG transfer limits for a > particular channel. At this time, these include the max number of > segments and max length of a segment that a channel can handle for a > SG transfer. Looks fine, should be ready for merge once we fix the API. Also I was under the impression that you will add another API to calculate the limits, the stuff which you were doing in caps API earlier. -- ~Vinod > > Along with the API implementation, this series implements the backend > device_slave_sg_limits() in the EDMA DMA Engine driver and converts the > davinci_mmc driver to use dma_get_slave_sg_limits() to replace hardcoded > limits. > > This is tested on the AM1808-EVM. > > Matt Porter (3): > dmaengine: add dma_get_slave_sg_limits() > dma: edma: add device_slave_sg_limits() support > mmc: davinci: get SG segment limits with dma_get_slave_sg_limits() > > drivers/dma/edma.c| 17 + > drivers/mmc/host/davinci_mmc.c| 37 --- > include/linux/dmaengine.h | 39 > + > include/linux/platform_data/mmc-davinci.h |3 --- > 4 files changed, 66 insertions(+), 30 deletions(-) > > -- > 1.7.9.5 > ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v4 0/6] dma: edma: Support scatter-lists of any length
On Thu, 2013-08-29 at 18:05 -0500, Joel Fernandes wrote: > The following series adds support to EDMA driver to enable DMA of > scatter-gather lists of arbitrary length, but still make use of only > a certain MAX number of slots at a time for a given channel. Thus > free-ing up the rest of the slots to other slaves/channels. With this > there is no need for slave drivers to query the EDMA driver about how > much is the MAX it can send at a time as done in [1]. Drivers can send > SG lists of any number of entries to DMA. Reference discussion at [2]. > > With this, all the patches for MMC and EDMA related to "sg limits" can be > dropped. > > Tested omap-aes and omap_hsmmc drivers with different MAX number of slots, > even just 1. In the case where it is 1, only 1-slot is used to DMA an > entire scatter list of arbitrary length. > Since this series touches EDMA private API code also shared with davinci-pcm, > playback of a 16-bit 44.1KHz audio file with davinci-pcm has been tested. > > Sample test run with 1 vs 16 (MAX number of slots/SG) in omap-aes driver: > MAX slots = 1: > (128 bit key, 8192 byte blocks): 1266 operations in 1 seconds (10371072 > bytes) > MAX slots = 16: > (128 bit key, 8192 byte blocks): 1601 operations in 1 seconds (13115392 > bytes) > > Note: For the above test, 8K buffer is mapped into SG list of size 2 so > only 2 slots are required. So beyond size 2, there will not be any noticeable > performance improvement. But above experiment just shows as proof of concept > that even using 1 slot is managed by just DMA'ing 1 SG entry at a time. much better series, thanks I think i am okay with this, if anyone has objections pls speak up. Also I need ack on the ARM patch 3/6 before I can carry this. -- Vinod Koul Intel Corp. ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v4 2/6] dma: edma: Write out and handle MAX_NR_SG at a given time
On Thu, Aug 29, 2013 at 06:05:41PM -0500, Joel Fernandes wrote:
> Process SG-elements in batches of MAX_NR_SG if they are greater
> than MAX_NR_SG. Due to this, at any given time only those many
> slots will be used in the given channel no matter how long the
> scatter list is. We keep track of how much has been written
> inorder to process the next batch of elements in the scatter-list
> and detect completion.
>
> For such intermediate transfer completions (one batch of MAX_NR_SG),
> make use of pause and resume functions instead of start and stop
> when such intermediate transfer is in progress or completed as we
> donot want to clear any pending events.
>
> Signed-off-by: Joel Fernandes
> ---
> drivers/dma/edma.c | 79
> --
> 1 file changed, 53 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
> index e522ad5..732829b 100644
> --- a/drivers/dma/edma.c
> +++ b/drivers/dma/edma.c
> @@ -56,6 +56,7 @@ struct edma_desc {
> struct list_headnode;
> int absync;
> int pset_nr;
> + int processed;
> struct edmacc_param pset[0];
> };
>
> @@ -104,22 +105,34 @@ static void edma_desc_free(struct virt_dma_desc *vdesc)
> /* Dispatch a queued descriptor to the controller (caller holds lock) */
> static void edma_execute(struct edma_chan *echan)
> {
> - struct virt_dma_desc *vdesc = vchan_next_desc(&echan->vchan);
> + struct virt_dma_desc *vdesc;
> struct edma_desc *edesc;
> - int i;
> -
> - if (!vdesc) {
> - echan->edesc = NULL;
> - return;
> + struct device *dev = echan->vchan.chan.device->dev;
> + int i, j, left, nslots;
> +
> + /* If either we processed all psets or we're still not started */
> + if (!echan->edesc ||
> + echan->edesc->pset_nr == echan->edesc->processed) {
> + /* Get next vdesc */
> + vdesc = vchan_next_desc(&echan->vchan);
> + if (!vdesc) {
> + echan->edesc = NULL;
> + return;
> + }
> + list_del(&vdesc->node);
> + echan->edesc = to_edma_desc(&vdesc->tx);
> }
>
> - list_del(&vdesc->node);
> + edesc = echan->edesc;
>
> - echan->edesc = edesc = to_edma_desc(&vdesc->tx);
> + /* Find out how many left */
> + left = edesc->pset_nr - edesc->processed;
> + nslots = min(MAX_NR_SG, left);
>
> /* Write descriptor PaRAM set(s) */
> - for (i = 0; i < edesc->pset_nr; i++) {
> - edma_write_slot(echan->slot[i], &edesc->pset[i]);
> + for (i = 0; i < nslots; i++) {
> + j = i + edesc->processed;
> + edma_write_slot(echan->slot[i], &edesc->pset[j]);
> dev_dbg(echan->vchan.chan.device->dev,
> "\n pset[%d]:\n"
> " chnum\t%d\n"
> @@ -132,24 +145,31 @@ static void edma_execute(struct edma_chan *echan)
> " bidx\t%08x\n"
> " cidx\t%08x\n"
> " lkrld\t%08x\n",
> - i, echan->ch_num, echan->slot[i],
> - edesc->pset[i].opt,
> - edesc->pset[i].src,
> - edesc->pset[i].dst,
> - edesc->pset[i].a_b_cnt,
> - edesc->pset[i].ccnt,
> - edesc->pset[i].src_dst_bidx,
> - edesc->pset[i].src_dst_cidx,
> - edesc->pset[i].link_bcntrld);
> + j, echan->ch_num, echan->slot[i],
> + edesc->pset[j].opt,
> + edesc->pset[j].src,
> + edesc->pset[j].dst,
> + edesc->pset[j].a_b_cnt,
> + edesc->pset[j].ccnt,
> + edesc->pset[j].src_dst_bidx,
> + edesc->pset[j].src_dst_cidx,
> + edesc->pset[j].link_bcntrld);
> /* Link to the previous slot if not the last set */
> - if (i != (edesc->pset_nr - 1))
> + if (i != (nslots - 1))
> edma_link(echan->slot[i], echan->slot[i+1]);
> /* Final pset links to the dummy pset */
> else
> edma_link(echan->slot[i], echan->ecc->dummy_slot);
> }
>
> - edma_start(echan->ch_num);
> + edesc->processed += nslots;
> +
> + edma_resume(echan->ch_num);
> +
> + if (edesc->processed <= MAX_NR_SG) {
> + dev_dbg(dev, "first transfer starting %d\n", echan->ch_num);
> + edma_start(echan->ch_num);
> + }
> }
>
> static int edma_terminate_all(struct edma_chan *echan)
> @@ -368,19 +388,26 @@ static void edma_callback(unsigned ch_num, u16
> ch_status, void *data)
> struct edma_desc *edesc;
> unsign
Re: [PATCH 2/3] dma: edma: Add support for Cyclic DMA
On Mon, Sep 23, 2013 at 06:05:14PM -0500, Joel Fernandes wrote:
> @@ -449,6 +455,138 @@ static struct dma_async_tx_descriptor
> *edma_prep_slave_sg(
> return vchan_tx_prep(&echan->vchan, &edesc->vdesc, tx_flags);
> }
>
> +static struct dma_async_tx_descriptor *edma_prep_dma_cyclic(
> + struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
> + size_t period_len, enum dma_transfer_direction direction,
> + unsigned long tx_flags, void *context)
> +{
> + struct edma_chan *echan = to_edma_chan(chan);
> + struct device *dev = chan->device->dev;
> + struct edma_desc *edesc;
> + dma_addr_t src_addr, dst_addr;
> + enum dma_slave_buswidth dev_width;
> + u32 burst;
> + int i, ret, nr_periods;
> +
> + if (unlikely(!echan || !buf_len || !period_len))
> + return NULL;
> +
> + if (direction == DMA_DEV_TO_MEM) {
> + src_addr = echan->cfg.src_addr;
> + dst_addr = buf_addr;
> + dev_width = echan->cfg.src_addr_width;
> + burst = echan->cfg.src_maxburst;
> + } else if (direction == DMA_MEM_TO_DEV) {
> + src_addr = buf_addr;
> + dst_addr = echan->cfg.dst_addr;
> + dev_width = echan->cfg.dst_addr_width;
> + burst = echan->cfg.dst_maxburst;
> + } else {
> + dev_err(dev, "%s: bad direction?\n", __func__);
> + return NULL;
> + }
> +
> + if (dev_width == DMA_SLAVE_BUSWIDTH_UNDEFINED) {
> + dev_err(dev, "Undefined slave buswidth\n");
> + return NULL;
> + }
> +
> + if (unlikely(buf_len % period_len)) {
> + dev_err(dev, "Period should be multiple of Buffer length\n");
> + return NULL;
> + }
> +
> + nr_periods = (buf_len / period_len) + 1;
?
consider the case of buf = period_len, above makes nr_period = 2.
Or buf len 100, period len 50, makes nr_period = 3
Both doesnt seem right to me?
--
~Vinod
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 1/3] dma: edma: Split out PaRAM set calculations into its own function
On Mon, Sep 23, 2013 at 06:05:13PM -0500, Joel Fernandes wrote: > PaRAM set calculation is abstracted into its own function to > enable better reuse for other DMA cases such as cyclic. We adapt > the Slave SG case to use the new function. > > This provides a much cleaner abstraction to the internals of the > PaRAM set. However, any PaRAM attributes that are not common to > all DMA types must be set separately such as setting of interrupts. > This function takes care of the most-common attributes. > > Also added comments clarifying A-sync case calculations. Applied, thanks -- ~Vinod ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 3/3] dma: edma: Increase maximum SG limit to 20
On Mon, Sep 23, 2013 at 06:05:15PM -0500, Joel Fernandes wrote: > davinci-pcm uses 16 as the no.of periods. With this, in EDMA we have to > allocate atleast 17 slots: 1 slot for channel, and 16 slots the periods. > > Due to this, the MAX_NR_SG limitation causes problems, set it to 20 to make > cyclic DMA work when davinci-pcm is converted to use DMA Engine. Also add > a comment clarifying this. Applied, thanks -- ~Vinod ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 2/3] dma: edma: Add support for Cyclic DMA
On Tue, Oct 22, 2013 at 10:30:43AM -0500, Joel Fernandes wrote: > On 10/21/2013 01:53 AM, Vinod Koul wrote: > > On Mon, Sep 23, 2013 at 06:05:14PM -0500, Joel Fernandes wrote: > >> + nr_periods = (buf_len / period_len) + 1; > > ? > > > > consider the case of buf = period_len, above makes nr_period = 2. > > > > Or buf len 100, period len 50, makes nr_period = 3 > > > > Both doesnt seem right to me? > > I guess that variable name is misleading. > > nr_periods is actually the total no.of slots needed to process the request. > Its > value is 1 greater than the total number of periods. Okay sounds good to me. I tried applying below but looks like it fails as I have already applied, 1 & 3. Can you pls rebase this resend -- ~Vinod ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 2/3] dma: edma: Add support for Cyclic DMA
On Thu, Oct 24, 2013 at 12:57:02PM -0500, Joel Fernandes wrote:
> Rebased on slave-dma/next branch and reapplied:
Looks like your MUA caused lines to get wrapped and patch is corrupt, can you
pls resend again using git-send email. I tried even the patch from
patchworks but that too failed!
>
> 8<---
> From: Joel Fernandes
> Subject: [PATCH v2] dma: edma: Add support for Cyclic DMA
>
> Using the PaRAM configuration function that we split for reuse by the
> different DMA types, we implement Cyclic DMA support.
> For the cyclic case, we pass different configuration parameters to this
> function, and handle all the Cyclic-specific functionality separately.
>
> Callbacks to the DMA users are handled using vchan_cyclic_callback in
> the virt-dma layer. Linking is handled the same way as the slave SG case
> except for the last slot where we link it back to the first one in a
> cyclic fashion.
>
> For continuity, we check for cases where no.of periods is great than the
> MAX number of slots the driver can allocate for a particular descriptor
> and error out on such cases.
>
> Signed-off-by: Joel Fernandes
> ---
> drivers/dma/edma.c | 159
> ++---
> 1 file changed, 151 insertions(+), 8 deletions(-)
>
> struct edma_chan *echan = data;
> @@ -464,24 +602,28 @@ static void edma_callback(unsigned ch_num, u16
> ch_status,
> void *data)
This seems bad
> unsigned long flags;
> struct edmacc_param p;
>
> - /* Pause the channel */
> - edma_pause(echan->ch_num);
> + edesc = echan->edesc;
> +
> + /* Pause the channel for non-cyclic */
> + if (!edesc || (edesc && !edesc->cyclic))
> + edma_pause(echan->ch_num);
>
> switch (ch_status) {
> case DMA_COMPLETE:
> spin_lock_irqsave(&echan->vchan.lock, flags);
>
> - edesc = echan->edesc;
> if (edesc) {
> - if (edesc->processed == edesc->pset_nr) {
> + if (edesc->cyclic) {
> + vchan_cyclic_callback(&edesc->vdesc);
> + } else if (edesc->processed == edesc->pset_nr) {
> dev_dbg(dev, "Transfer complete, stopping
> channel %d\n", ch_num);
> edma_stop(echan->ch_num);
> vchan_cookie_complete(&edesc->vdesc);
> + edma_execute(echan);
> } else {
> dev_dbg(dev, "Intermediate transfer complete on
> channel %d\n", ch_num);
> + edma_execute(echan);
> }
> -
> - edma_execute(echan);
> }
>
> spin_unlock_irqrestore(&echan->vchan.lock, flags);
> @@ -677,6 +819,7 @@ static void edma_dma_init(struct edma_cc *ecc, struct
> dma_device *dma,
ditto and few other which checkpatch was not happy about!
> struct device *dev)
> {
> dma->device_prep_slave_sg = edma_prep_slave_sg;
> + dma->device_prep_dma_cyclic = edma_prep_dma_cyclic;
> dma->device_alloc_chan_resources = edma_alloc_chan_resources;
> dma->device_free_chan_resources = edma_free_chan_resources;
> dma->device_issue_pending = edma_issue_pending;
> --
> 1.8.1.2
>
--
~Vinod
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH][RESEND 3/8] dma: mmp_tdma: use gen_pool_dma_alloc() to allocate descriptor
On Fri, Nov 01, 2013 at 07:48:16PM +0800, Nicolin Chen wrote: > Since gen_pool_dma_alloc() is introduced, we implement it to simplify code. Acked-by: Vinod Koul -- ~Vinod ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] dma: edma: fix incorrect SG list handling
On Mon, Mar 17, 2014 at 09:14:14AM -0400, Jon Ringle wrote: > On Mon, 17 Mar 2014, Sekhar Nori wrote: > > > Hi Jon, > > > > On Monday 17 March 2014 06:28 PM, Jon Ringle wrote: > > > > > > On Mon, 17 Mar 2014, Sekhar Nori wrote: > > > > > >> The code to handle any length SG lists calls edma_resume() > > >> even before edma_start() is called. This is incorrect > > >> because edma_resume() enables edma events on the channel > > >> after which CPU (in edma_start) cannot clear posted > > >> events by writing to ECR (per the EDMA user's guide). > > >> > > >> Because of this EDMA transfers fail to start if due > > >> to some reason there is a pending EDMA event registered > > >> even before EDMA transfers are started. This can happen if > > >> an EDMA event is a byproduct of device initialization. > > >> > > >> Fix this by calling edma_resume() only if it is not the > > >> first batch of MAX_NR_SG elements. > > >> > > >> Without this patch, MMC/SD fails to function on DA850 EVM > > >> with DMA. The behaviour is triggered by specific IP and > > >> this can explain why the issue was not reported before > > >> (example with MMC/SD on AM335x). > > >> > > >> Tested on DA850 EVM and AM335x EVM-SK using MMC/SD card. > > >> > > >> Cc: # v3.12.x+ > > >> Cc: Joel Fernandes > > >> Reported-by: Jon Ringle > > >> Signed-off-by: Sekhar Nori > > >> --- > > >> Jon, can you please confirm this fixes the issue you > > >> reported? > > > > > > The patch does not apply on linux-3.12 due to changes to the 3 context > > > lines at the start of the hunk. > > > > > > But, I manually fixed up the code and it does fix the issue on our > > > AM1808 > > > board. > > > > Thanks for the testing. The patch is meant for latest mainline but based > > on what you said, a manual backport to v3.12-stable will be required. > > > > Can you please reply with a formal Tested-by: ? > > Tested-by: Jon Ringle where is this patch, somehow am not able to find in my inbox or archives... -- ~Vinod ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 05/14] arm: common: edma: Select event queue 1 as default when booted with DT
On Fri, Apr 11, 2014 at 12:38:00PM +0300, Peter Ujfalusi wrote: > On 04/11/2014 11:56 AM, Sekhar Nori wrote: > > On Friday 11 April 2014 02:20 PM, Peter Ujfalusi wrote: > >> On 04/11/2014 11:17 AM, Sekhar Nori wrote: > >>> On Tuesday 01 April 2014 06:36 PM, Peter Ujfalusi wrote: > Use the EVENTQ_1 for default and leave the EVENTQ_0 to be used by high > priority channels, like audio. > > Signed-off-by: Peter Ujfalusi > >>> > >>> Acked-by: Sekhar Nori > >>> > --- > arch/arm/common/edma.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c > index 86a8b263278f..19520e2519d9 100644 > --- a/arch/arm/common/edma.c > +++ b/arch/arm/common/edma.c > @@ -1546,7 +1546,8 @@ static int edma_of_parse_dt(struct device *dev, > > pdata->queue_priority_mapping = queue_priority_map; > > -pdata->default_queue = 0; > +/* select queue 1 as default */ > >>> > >>> It will be nice to expand the comment with explanation of why this is > >>> being chosen as default (lower priority queue by default for typical > >>> bulk data transfer). > >> > >> Yes, extended comment is a good idea. > >> > >> For the next version I think I'm going to change the code around default > >> TC/Queue and the non default queue selection, mostly based on Joel's > >> comment: > >> > >> EVENTQ_1 as default queue. > >> Set the EVENTQ_1 priority to 7 > >> EVENTQ_0 priority is going to stay 0 and EVENTQ_2 as 2 > >> > >> Add new member to struct edma, like high_pri_queue. > >> When we set the queue priorities in edma_probe() we look for the highest > >> priority queue and save the number in high_pri_queue. > >> > >> I will rename the edma_request_non_default_queue() to > >> edma_request_high_pri_queue() and it will assign the channel to the high > >> priority queue. > >> > >> I think this way it is going to be more explicit and IMHO a bit more safer > >> in > >> a sense the we are going to get high priority when we ask for it. > > > > Sounds much better. I had posted some ideas about adding support for > > channel priority in the core code but we can leave that for Vinod and > > Dan to say if they really see a need for that. Is it part of this series? > If we do it via the dmaengine core I think it would be better to have a new > flag to be passed to dmaengine_prep_dma_*(). > We could have for example: > DMA_PREP_HIGH_PRI as flag to indicate that we need high priority DMA if it is > possible. > We can watch for this flag in the edma driver and act accordingly. > ALSA's dmaengine_pcm_prepare_and_submit() could set this flag unconditionally > since audio should be treated in this way if the DMA IP can do this. Will the priority be different for each descriptor or would be based on channel usage. If not then we can add this in dma_slave_config ? I can forsee its usage on slave controllers, so yes its useful :) -- ~Vinod > > Not sure what to do with eDMA's 8 priority level. With that we could have high > priority; low priority; low, but not the lowest priority; about in the middle > priority; etc. > > We could have also new callback in the dma_device struct with needed wrappers > to set priority level, but where to draw the range? High, Mid and Low? Range > of 0 - 10? > > -- > Péter -- ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 05/14] arm: common: edma: Select event queue 1 as default when booted with DT
On Fri, Apr 11, 2014 at 02:32:28PM +0300, Peter Ujfalusi wrote: > Hi Vinod, > > On 04/11/2014 12:42 PM, Vinod Koul wrote: > > On Fri, Apr 11, 2014 at 12:38:00PM +0300, Peter Ujfalusi wrote: > >> On 04/11/2014 11:56 AM, Sekhar Nori wrote: > >>> On Friday 11 April 2014 02:20 PM, Peter Ujfalusi wrote: > >>>> On 04/11/2014 11:17 AM, Sekhar Nori wrote: > >>>>> On Tuesday 01 April 2014 06:36 PM, Peter Ujfalusi wrote: > >>>>>> Use the EVENTQ_1 for default and leave the EVENTQ_0 to be used by high > >>>>>> priority channels, like audio. > >>>>>> > >>>>>> Signed-off-by: Peter Ujfalusi > >>>>> > >>>>> Acked-by: Sekhar Nori > >>>>> > >>>>>> --- > >>>>>> arch/arm/common/edma.c | 3 ++- > >>>>>> 1 file changed, 2 insertions(+), 1 deletion(-) > >>>>>> > >>>>>> diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c > >>>>>> index 86a8b263278f..19520e2519d9 100644 > >>>>>> --- a/arch/arm/common/edma.c > >>>>>> +++ b/arch/arm/common/edma.c > >>>>>> @@ -1546,7 +1546,8 @@ static int edma_of_parse_dt(struct device *dev, > >>>>>> > >>>>>>pdata->queue_priority_mapping = queue_priority_map; > >>>>>> > >>>>>> - pdata->default_queue = 0; > >>>>>> + /* select queue 1 as default */ > >>>>> > >>>>> It will be nice to expand the comment with explanation of why this is > >>>>> being chosen as default (lower priority queue by default for typical > >>>>> bulk data transfer). > >>>> > >>>> Yes, extended comment is a good idea. > >>>> > >>>> For the next version I think I'm going to change the code around default > >>>> TC/Queue and the non default queue selection, mostly based on Joel's > >>>> comment: > >>>> > >>>> EVENTQ_1 as default queue. > >>>> Set the EVENTQ_1 priority to 7 > >>>> EVENTQ_0 priority is going to stay 0 and EVENTQ_2 as 2 > >>>> > >>>> Add new member to struct edma, like high_pri_queue. > >>>> When we set the queue priorities in edma_probe() we look for the highest > >>>> priority queue and save the number in high_pri_queue. > >>>> > >>>> I will rename the edma_request_non_default_queue() to > >>>> edma_request_high_pri_queue() and it will assign the channel to the high > >>>> priority queue. > >>>> > >>>> I think this way it is going to be more explicit and IMHO a bit more > >>>> safer in > >>>> a sense the we are going to get high priority when we ask for it. > >>> > >>> Sounds much better. I had posted some ideas about adding support for > >>> channel priority in the core code but we can leave that for Vinod and > >>> Dan to say if they really see a need for that. > > Is it part of this series? > > No, it is not. The original series tackled the DMA queue selection within the > edma driver stack. It was selecting high priority channel for cyclic (audio) > use only, all other DMA channels were executed on a lower priority queue. > > >> If we do it via the dmaengine core I think it would be better to have a new > >> flag to be passed to dmaengine_prep_dma_*(). > >> We could have for example: > >> DMA_PREP_HIGH_PRI as flag to indicate that we need high priority DMA if it > >> is > >> possible. > >> We can watch for this flag in the edma driver and act accordingly. > >> ALSA's dmaengine_pcm_prepare_and_submit() could set this flag > >> unconditionally > >> since audio should be treated in this way if the DMA IP can do this. > > > > Will the priority be different for each descriptor or would be based on > > channel > > usage. > > I would say that it is channel based config. I don't see the reason why would > one mix different priorities on a configured channel between descriptors. > > > If not then we can add this in dma_slave_config ? > > So adding to the struct for example: > bool high_priority; In designware controller, we can have channel priorties from 0 to 7 which IIRC 7 being highest. So bool wont work. unsigned int/u8 would be good. How about your controller, is it binary? -- ~Vinod > > I'm not sure if it helps if we have let's say three priority level like, low, > normal and high. > I don't think that any client would ask for low priority instead using the > normal priority. > > > I can forsee its usage on slave controllers, so yes its useful :) > > True I'm sure it is going to be used as soon as it is available if the HW > supports priorities. > > -- > Péter -- ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 05/14] arm: common: edma: Select event queue 1 as default when booted with DT
On Fri, Apr 11, 2014 at 03:23:54PM +0300, Peter Ujfalusi wrote: > On 04/11/2014 02:31 PM, Vinod Koul wrote: > > >> I would say that it is channel based config. I don't see the reason why > >> would > >> one mix different priorities on a configured channel between descriptors. > >> > >>> If not then we can add this in dma_slave_config ? > >> > >> So adding to the struct for example: > >> bool high_priority; > > > > In designware controller, we can have channel priorties from 0 to 7 which > > IIRC 7 > > being highest. So bool wont work. unsigned int/u8 would be good. > > I see. But from a generic code what number should one pass if we want to get > the highest priority? With eDMA3 we essentially have three levels (see later) > so if we receive 7 as priority what shall we do? Just treat as highest? But if > we receive 4, which is somewhere in the middle for designware it is still > means highest for us. > > I see this too small step partitioning in common code a bit problematic when > it comes to how to interpret the 'magic numbers'. > Also how all the driver in the system will decide the priority number? I'm > sure they will pick something conveniently average for themselves and I > imagine audio would try to pick something which is bigger than the numbers > others have taken... > > > How about your controller, is it binary? > > We also have priority from 0 to 7, 0 being the highest priority. We have 3 > Transfer Controllers/Event Queues and we can set the priority for the TC/EQ > and assign the given channel to the desired TC/EQ. > So in reality we have 3 priorities to choose from in my view since we only > have 3 TC/EQ in eDMA3 (of AM335x/AM447x) on other SoCs we can have different > number of TC/EQ. I think the number shouldn't be viewed in absolute terms. If we decide that (lets say) 0-7, then any controller should map 0 to lowest and 7 to highest. For your case you can do this and then intermediate numbers would be medium priority. Such a system might work well... Also how would a client driver know which priority to use? Would it come from DT? -- ~Vinod ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 03/14] dma: edma: Add support for DMA_PAUSE/RESUME operation
On Tue, Apr 01, 2014 at 04:06:04PM +0300, Peter Ujfalusi wrote:
> Pause/Resume can be used by the audio stack when the stream is paused/resumed
> The edma platform code has support for this and the legacy audio stack used
> this.
>
> Signed-off-by: Peter Ujfalusi
> ---
> drivers/dma/edma.c | 28
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
> index 2742867fd1e6..7891378a03f0 100644
> --- a/drivers/dma/edma.c
> +++ b/drivers/dma/edma.c
> @@ -240,6 +240,26 @@ static int edma_slave_config(struct edma_chan *echan,
> return 0;
> }
>
> +static int edma_dma_pause(struct edma_chan *echan)
> +{
> + /* Pause/Resume only allowed with cyclic mode */
> + if (!echan->edesc->cyclic)
> + return -EINVAL;
why this artificial restriction? The driver can do pause even for non cyclic
cases too? Yes the usage is in cyclic context but why should we limit any future
work on this?
--
~Vinod
> +
> + edma_pause(echan->ch_num);
> + return 0;
> +}
> +
> +static int edma_dma_resume(struct edma_chan *echan)
> +{
> + /* Pause/Resume only allowed with cyclic mode */
> + if (!echan->edesc->cyclic)
> + return -EINVAL;
> +
> + edma_resume(echan->ch_num);
> + return 0;
> +}
> +
> static int edma_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
> unsigned long arg)
> {
> @@ -255,6 +275,14 @@ static int edma_control(struct dma_chan *chan, enum
> dma_ctrl_cmd cmd,
> config = (struct dma_slave_config *)arg;
> ret = edma_slave_config(echan, config);
> break;
> + case DMA_PAUSE:
> + ret = edma_dma_pause(echan);
> + break;
> +
> + case DMA_RESUME:
> + ret = edma_dma_resume(echan);
> + break;
> +
> default:
> ret = -ENOSYS;
> }
> --
> 1.9.1
>
--
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 08/14] DMA: edma: Use different eventq for cyclic channels
On Thu, Apr 10, 2014 at 11:36:30AM -0500, Joel Fernandes wrote: > On 04/01/2014 08:06 AM, Peter Ujfalusi wrote: > > To improve latency with cyclic DMA operation it is preferred to > > use different eventq/tc than the default which is used by all > > other drivers (mmc, spi, i2c, etc). > > When preparing the cyclic dma ask for non default queue for the > > channel which is going to be used with cyclic mode. > > > > Signed-off-by: Peter Ujfalusi > > --- > > drivers/dma/edma.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c > > index 1dd9e8806975..10048b40fac8 100644 > > --- a/drivers/dma/edma.c > > +++ b/drivers/dma/edma.c > > @@ -628,6 +628,9 @@ static struct dma_async_tx_descriptor > > *edma_prep_dma_cyclic( > > edesc->pset[i].opt |= TCINTEN; > > } > > > > + /* Use different eventq/tc for cyclic DMA to reduce latency */ > > + edma_request_non_default_queue(echan->ch_num); > > + > > return vchan_tx_prep(&echan->vchan, &edesc->vdesc, tx_flags); > > } > > > > > > Is there any way to guarantee that the non-default queue is of the > highest priority, or in other words default queue is of lowest priority. well as we are discussing in other thread, it would make sense to pass the required priority (i am using audio so pls give me highest one) -- ~Vinod > I know you set queue 1 as default because by default 0 is higher > priority. And then assigning non-default queue. > > When assigning default to Queue 1, it would be good to also call > assign_priority_to_queue and set QUEPRI to 7 for Queue 1. Since 0, 2 and > 4 are all non-defaults. > > Thanks, > -Joel -- ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 00/14] dma: edma: Fixes for cyclic (audio) operation
On Tue, Apr 01, 2014 at 04:06:01PM +0300, Peter Ujfalusi wrote: > Hi, > > This is basically a resend of the previous series: > https://lkml.org/lkml/2014/3/13/119 > with removed ASoC patches (most of them are applied already). > > Changes since v1: > - ASoC patches removed > - Comments from Andriy Shevchenko addressed > - patch added to fix cases when src/dst_maxburst is set to 0 > > Adding support for DMA pause/resume > Possibility to select non default event queue/TC for cyclic (audio) dma > channels: all devices using the eDMA via dmaengine was assigned to the default > EQ/TC (mmc, i2c, spi, etc, and audio). This is not optimal from system > performance point of view since sharing the same EQ/TC can cause latency > spikes > for cyclic channels (long DMA transfers for MMC for example). > > While debugging the edma to get things sorted out I noticed that the debug was > too verbose and the important information was hidden even when the we did not > asked for verbose dmaengine debug. > I have included some debug cleanups for the edma dmaengine driver also. > > Regards, > Peter > --- > Peter Ujfalusi (14): > platform_data: edma: Be precise with the paRAM struct > dma: edma: Correct the handling of src/dst_maxburst == 0 > dma: edma: Add support for DMA_PAUSE/RESUME operation > dma: edma: Set DMA_CYCLIC capability flag > arm: common: edma: Select event queue 1 as default when booted with DT > arm: common: edma: Save the number of event queues/TCs > arm: common: edma: API to request non default queue for a channel > DMA: edma: Use different eventq for cyclic channels > dma: edma: Implement device_slave_caps callback > dma: edma: Simplify direction configuration in edma_config_pset() > dma: edma: Reduce debug print verbosity for non verbose debugging > dma: edma: Prefix debug prints where the text were identical in prep > callbacks > dma: edma: Add channel number to debug prints > dma: edma: Print the direction value as well when it is not supported Okay I have noticed lots of folks are using dma: which IMO isn't right here, would prefer folks to use the right subsystem name, so dmaengine: would be right subject line :) -- ~Vinod > > arch/arm/common/edma.c | 34 +- > drivers/dma/edma.c | 96 > +- > include/linux/platform_data/edma.h | 20 > 3 files changed, 119 insertions(+), 31 deletions(-) > > -- > 1.9.1 > -- ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 1/1] dma: edma: fix incorrect SG list handling
On Mon, Apr 14, 2014 at 02:01:11PM +0530, Sekhar Nori wrote: > Vinod, > > On Wednesday 19 March 2014 11:25 AM, Sekhar Nori wrote: > > The code to handle any length SG lists calls edma_resume() > > even before edma_start() is called. This is incorrect > > because edma_resume() enables edma events on the channel > > after which CPU (in edma_start) cannot clear posted > > events by writing to ECR (per the EDMA user's guide). > > > > Because of this EDMA transfers fail to start if due > > to some reason there is a pending EDMA event registered > > even before EDMA transfers are started. This can happen if > > an EDMA event is a byproduct of device initialization. > > > > Fix this by calling edma_resume() only if it is not the > > first batch of MAX_NR_SG elements. > > > > Without this patch, MMC/SD fails to function on DA850 EVM > > with DMA. The behaviour is triggered by specific IP and > > this can explain why the issue was not reported before > > (example with MMC/SD on AM335x). > > > > Tested on DA850 EVM and AM335x EVM-SK using MMC/SD card. > > > > Cc: sta...@vger.kernel.org # v3.12.x+ > > Cc: Joel Fernandes > > Acked-by: Joel Fernandes > > Tested-by: Jon Ringle > > Tested-by: Alexander Holler > > Reported-by: Jon Ringle > > Signed-off-by: Sekhar Nori > > Looks like this patch is not in mainline still? Sorry looks like I have missed sending the email. I had applied it last week and today rebased after rc1. It would be part of rc2... -- ~Vinod > > Thanks, > Sekhar -- ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v3 00/10] dma: edma: Fixes for cyclic (audio) operation
On Mon, Apr 14, 2014 at 02:41:55PM +0300, Peter Ujfalusi wrote: > Hi, > > Changes since v2: > - Dropped patch 10 from v2 (simplify direction configuration...) > - Dropped the channel priority related patches since we are going to go via > different route for configuring the priority. > - Added ACK from Joel for the patches since they are not changed since v2 > > Changes since v1: > - ASoC patches removed > - Comments from Andriy Shevchenko addressed > - patch added to fix cases when src/dst_maxburst is set to 0 > > The series contains now only: > Support for DMA pause/resume in cyclic mode > device_slave_caps callback and DMA_CYCLIC flag correction. > While debugging the edma to get things sorted out I noticed that the debug was > too verbose and the important information was hidden even when the we did not > asked for verbose dmaengine debug. > I have included some debug cleanups for the edma dmaengine driver also. > Applied all, expect 9th one! -- ~Vinod > Regards, > Peter > --- > Peter Ujfalusi (10): > platform_data: edma: Be precise with the paRAM struct > arm: common: edma: Save the number of event queues/TCs > dmaengine: edma: Correct the handling of src/dst_maxburst == 0 > dmaengine: edma: Add support for DMA_PAUSE/RESUME operation > dmaengine: edma: Set DMA_CYCLIC capability flag > dmaengine: edma: Implement device_slave_caps callback > dmaengine: edma: Reduce debug print verbosity for non verbose > debugging > dmaengine: edma: Prefix debug prints where the text were identical in > prep callbacks > dmaengine: edma: Add channel number to debug prints > dmaengine: edma: Print the direction value as well when it is not > supported > > arch/arm/common/edma.c | 4 ++ > drivers/dma/edma.c | 87 > ++ > include/linux/platform_data/edma.h | 18 > 3 files changed, 83 insertions(+), 26 deletions(-) > > -- > 1.9.2 > -- ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v3 09/10] dmaengine: edma: Add channel number to debug prints
On Mon, Apr 14, 2014 at 02:42:04PM +0300, Peter Ujfalusi wrote: > It helps to identify issues if we have some information regarding to the > channel which the event is associated. > > Signed-off-by: Peter Ujfalusi > Acked-by: Joel Fernandes This failed to apply, cna you pls rebase this and resend... -- ~Vinod ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] dmaengine: edma: Add DMA memcpy support
On Fri, Apr 18, 2014 at 09:50:33PM -0500, Joel Fernandes wrote: > We add DMA memcpy support to EDMA driver. Successful tests performed using > dmatest kernel module. Copy alignment is set to DMA_SLAVE_BUSWIDTH_4_BYTES and > users must ensure length is aligned so that copy is performed fully. Applied, thanks -- ~Vinod ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] [FIX] dmaengine: virt-dma: Free descriptor after callback
On Fri, Apr 18, 2014 at 11:34:50AM -0500, Joel Fernandes wrote: > On 04/18/2014 03:50 AM, Russell King - ARM Linux wrote: > > On Thu, Apr 17, 2014 at 07:56:50PM -0500, Joel Fernandes wrote: > >> Free the vd (virt descriptor) after the callback is called. In EDMA driver > >> atleast which uses virt-dma, we make use of the desc during the callback > >> and if > >> its dangerously freed before the callback is called. I also noticed this in > >> omap-dma dmaengine driver. > > > > You've missed the vital bit of information: why do you make use of the > > descriptor afterwards? You shouldn't. omap-dma doesn't either. > > > > Once clients submit their request to DMA engine, they must not hold any > > kind of reference to the descriptor other than the cookie. > > > > Sorry, I confused edma/omap-dma callbacks for virt dma callbacks. > > Anyway, I think there is still a chance in edma that we refer to the > echan->edesc pointer later on after virt-dma calls the free (in > edma_execute), so I'll just NULL that out to be safe and submit a patch. Yes, that would be the right way :) While looking at this, I see it is not called out specfically in documentation, will update that as well -- ~Vinod ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] dmaengine: edma: No need save/restore interrupt flags during spin_lock in IRQ
On Thu, Apr 17, 2014 at 12:58:33AM -0500, Joel Fernandes wrote:
> The vchan lock in edma_callback is acquired in hard interrupt context. As
> interrupts are already disabled, there's no point in save/restoring interrupt
> mask bit or cpsr flags.
>
> Get rid of flags local variable and use spin_lock instead of
> spin_lock_irqsave.
Applied, thanks
--
~Vinod
>
> Signed-off-by: Joel Fernandes
> ---
> drivers/dma/edma.c |9 -
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
> index 91849aa..25a75e2 100644
> --- a/drivers/dma/edma.c
> +++ b/drivers/dma/edma.c
> @@ -638,7 +638,6 @@ static void edma_callback(unsigned ch_num, u16 ch_status,
> void *data)
> struct edma_chan *echan = data;
> struct device *dev = echan->vchan.chan.device->dev;
> struct edma_desc *edesc;
> - unsigned long flags;
> struct edmacc_param p;
>
> edesc = echan->edesc;
> @@ -649,7 +648,7 @@ static void edma_callback(unsigned ch_num, u16 ch_status,
> void *data)
>
> switch (ch_status) {
> case EDMA_DMA_COMPLETE:
> - spin_lock_irqsave(&echan->vchan.lock, flags);
> + spin_lock(&echan->vchan.lock);
>
> if (edesc) {
> if (edesc->cyclic) {
> @@ -665,11 +664,11 @@ static void edma_callback(unsigned ch_num, u16
> ch_status, void *data)
> }
> }
>
> - spin_unlock_irqrestore(&echan->vchan.lock, flags);
> + spin_unlock(&echan->vchan.lock);
>
> break;
> case EDMA_DMA_CC_ERROR:
> - spin_lock_irqsave(&echan->vchan.lock, flags);
> + spin_lock(&echan->vchan.lock);
>
> edma_read_slot(EDMA_CHAN_SLOT(echan->slot[0]), &p);
>
> @@ -700,7 +699,7 @@ static void edma_callback(unsigned ch_num, u16 ch_status,
> void *data)
> edma_trigger_channel(echan->ch_num);
> }
>
> - spin_unlock_irqrestore(&echan->vchan.lock, flags);
> + spin_unlock(&echan->vchan.lock);
>
> break;
> default:
> --
> 1.7.9.5
>
--
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RESEND] dmaengine: edma: Add channel number to debug prints
On Thu, Apr 24, 2014 at 10:29:50AM +0300, Peter Ujfalusi wrote: > It helps to identify issues if we have some information regarding to the > channel which the event is associated. > > Signed-off-by: Peter Ujfalusi > Acked-by: Joel Fernandes Applied, thanks -- ~Vinod ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 0/3] ARM/dma: edma: Serve cyclic clients via high priority queue
On Tue, Jul 08, 2014 at 01:46:35PM +0300, Peter Ujfalusi wrote: > Hi, > > It is preferred that audio is served with the highest priority queue in order > to > avoid delays in data transfer between memory and audio IP. > > The following series will add an API to arch code to assign a channel to a > given > queue. > The default queue is changed from 0 (highest priority) to lowest priority. > In the dmaengine driver we move the cyclic channel to queue0 (highest > priority) > and move it back to default queue when the channel is terminated. Applied, thanks -- ~Vinod > > Regards, > Peter > --- > Peter Ujfalusi (3): > ARM: edma: Set default queue to lowest priority > ARM: edma: Add edma_assign_channel_eventq() to move channel to a give > queue > dma: edma: Serve cyclic (audio) channels with high priority queue > > arch/arm/common/edma.c | 31 ++- > drivers/dma/edma.c | 8 > include/linux/platform_data/edma.h | 2 ++ > 3 files changed, 40 insertions(+), 1 deletion(-) > > -- > 2.0.0 > -- ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 0/2] dma: edma: Allow to disable eDMA IRQ during cyclic transfer
On Wed, Jul 16, 2014 at 03:29:19PM +0300, Peter Ujfalusi wrote: > Hi, > > After this series clients can ask to not receive notifications after each > period. > In this case we can disable the completion interrupt since the position > reporting > does not rely on it for cyclic mode. > Patchset for ASoC part has been sent which allows users space to take > adventage > of SNDRV_PCM_INFO_NO_PERIOD_WAKEUP: > [1] http://mailman.alsa-project.org/pipermail/alsa-devel/2014-July/078993.html Applied, thanks Please use right subsystem name for patches, i have fixed that while applying -- ~Vinod ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 1/2] dmaengine: edma: Do not change the error code returned from edma_alloc_slot
On Thu, Jul 31, 2014 at 01:12:37PM +0300, Peter Ujfalusi wrote: > In case of edma_alloc_slot() failure during probe we should return the error > unchanged to make debugging easier. Applied both Thanks -- ~Vinod ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] ARM: edma: Fix configuration parsing for SoCs with multiple eDMA3 CC
On Mon, Aug 04, 2014 at 03:26:56PM +0300, Peter Ujfalusi wrote:
> The edma_setup_from_hw() should know about the CC number when parsing the
> CCCFG register - when it reads the register to be precise. The base
> addresses for CCs stored in an array and we need to provide the correct id
> to edma_read() in order to read the correct register.
Just an othognal question:
I still see lot of code for edma in arm/. What is the plan to re/move that
to dma/
--
~Vinod
>
> Cc: # 3.16
> Signed-off-by: Peter Ujfalusi
> ---
> arch/arm/common/edma.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
> index 88099175fc56..d86771abbf57 100644
> --- a/arch/arm/common/edma.c
> +++ b/arch/arm/common/edma.c
> @@ -1443,14 +1443,14 @@ void edma_assign_channel_eventq(unsigned channel,
> enum dma_event_q eventq_no)
> EXPORT_SYMBOL(edma_assign_channel_eventq);
>
> static int edma_setup_from_hw(struct device *dev, struct edma_soc_info
> *pdata,
> - struct edma *edma_cc)
> + struct edma *edma_cc, int cc_id)
> {
> int i;
> u32 value, cccfg;
> s8 (*queue_priority_map)[2];
>
> /* Decode the eDMA3 configuration from CCCFG register */
> - cccfg = edma_read(0, EDMA_CCCFG);
> + cccfg = edma_read(cc_id, EDMA_CCCFG);
>
> value = GET_NUM_REGN(cccfg);
> edma_cc->num_region = BIT(value);
> @@ -1464,7 +1464,8 @@ static int edma_setup_from_hw(struct device *dev,
> struct edma_soc_info *pdata,
> value = GET_NUM_EVQUE(cccfg);
> edma_cc->num_tc = value + 1;
>
> - dev_dbg(dev, "eDMA3 HW configuration (cccfg: 0x%08x):\n", cccfg);
> + dev_dbg(dev, "eDMA3 CC%d HW configuration (cccfg: 0x%08x):\n", cc_id,
> + cccfg);
> dev_dbg(dev, "num_region: %u\n", edma_cc->num_region);
> dev_dbg(dev, "num_channel: %u\n", edma_cc->num_channels);
> dev_dbg(dev, "num_slot: %u\n", edma_cc->num_slots);
> @@ -1684,7 +1685,7 @@ static int edma_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> /* Get eDMA3 configuration from IP */
> - ret = edma_setup_from_hw(dev, info[j], edma_cc[j]);
> + ret = edma_setup_from_hw(dev, info[j], edma_cc[j], j);
> if (ret)
> return ret;
>
> --
> 2.0.2
>
--
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
