Thanks Vinod for quick review.
On Friday 20 April 2012 04:44 PM, Vinod Koul wrote:
On Fri, 2012-04-20 at 14:38 +0530, Laxman Dewangan wrote:
+ * dma_transfer_mode: Different dma transfer mode.
+ * DMA_MODE_ONCE: Dma transfer the configured buffer once and at the end of
+ * transfer, dma stops automatically and generates interrupt
+ * if enabled. SW need to reprogram dma for next transfer.
+ * DMA_MODE_CYCLE: Dma keeps transferring the same buffer again and again
+ * until dma stopped explicitly by SW or another buffer configured.
+ * After transfer completes, dma again starts transfer from
+ * beginning of buffer without sw intervention. If any new
+ * address/size is configured during buffer transfer then
+ * dma start transfer with new configuration otherwise it
+ * will keep transferring with old configuration. It also
+ * generates the interrupt after buffer transfer completes.
why do you need to define this? use the cyclic api to convey this
This is not the public definition, only used in dma_driver locally. The
tegra dma support the cyclic mode in two ways;
Cyclic single interrupt mode on which it generates interrupt once full
buffer transfer completes and hw keep transferring the data from start
of buffer without sw intervention.
Cyclic double interrupt mode on which it generates interrupt two times,
once after half buffer and second after full buffer. The hw keep
transferring buffer in cyclic manner.
I am using these mode based on how cyclic parameter is passed from client.
If period_len is half of buffer len the using cyclic double interrupt
mode and hence configure dma once for two interrupt.
For other cases, I am using the cyclic single interrupt mode.
+ * DMA_MODE_CYCLE_HALF_NOTIFY: In this mode dma keeps transferring the buffer
+ * into two folds. This is kind of ping-pong buffer where both
+ * buffer size should be same. Dma completes the one buffer,
+ * generates interrupt and keep transferring the next buffer
+ * whose address start just next to first buffer. At the end of
+ * second buffer transfer, dma again generates interrupt and
+ * keep transferring of the data from starting of first buffer.
+ * If sw wants to change the address/size of the buffer then
+ * it needs to change only when dma transferring the second
+ * half of buffer. In dma configuration, it only need to
+ * configure starting of first buffer and size of first buffer.
+ * Dma hw assumes that striating address of second buffer is just
+ * next to end of first buffer and size is same as the first
+ * buffer.
isnt this a specifc example of cylci and frankly why should dmaengine
care about this. This one of the configurations you are passing for a
cyclic dma operation
No special configuration is passed. Just based on buf_len and
period_len, the mode get selected.
+ */
+enum dma_transfer_mode {
+ DMA_MODE_NONE,
+ DMA_MODE_ONCE,
+ DMA_MODE_CYCLE,
+ DMA_MODE_CYCLE_HALF_NOTIFY,
+};
+
+/* List of memory allocated for that channel */
+struct tegra_dma_chan_mem_alloc {
+ struct list_head node;
+};
this seems questionable too...
When we allocate the channel, initially allocate some descriptors with
some initial count. If client has more request and if it is found out of
descriptor then we reallocate the some more descriptors. All these
allocations are dynamically and when Chanel got released, it frees all
allocations.
This structure is to maintain the list of memory pointers which are
allocated.
here I am allocating chunk of descriptor in one shot, not one by one. If
I allocate descriptor one by one then I will not need this structure.
tried to optimize the malloc call here.
+ * The client's request for data transfer can be broken into multiple
+ * sub-transfer as per requestor details and hw support.
typo ^^^^^^^^^
Will fix this.
+ * tegra_dma_desc: Tegra dma descriptors which manages the client requests.
+ * This de scripts keep track of transfer status, callbacks, transfer and
again ^^^^
Will fix this, thanks for pointing. My spell check did not found it.
+
+static dma_cookie_t tegra_dma_tx_submit(struct dma_async_tx_descriptor *tx);
+
+static int allocate_tegra_desc(struct tegra_dma_channel *tdc,
+ int ndma_desc, int nsg_req)
what does the last arg mean?
This is number of transfer request. So if client call the prep_slave or
prep_dma_cyclic with multiple segment/period len then this structure
will contain the details of sub transfer per segment/period_len.
And it allocate one main dma_descriptor which have the transfer
descriptor. So one dma_desc which is allocated one per call will contain
list of such transfer requests.
+ INIT_LIST_HEAD(&sg_req_list);
+
+ /* Calculate total require size of memory and then allocate */
+ dma_desc_size = sizeof(struct tegra_dma_desc) * ndma_desc;
+ sg_req_size = sizeof(struct tegra_dma_sg_req) * nsg_req;
+ chan_mem_size = sizeof(struct tegra_dma_chan_mem_alloc);
+ total_size = chan_mem_size + dma_desc_size + sg_req_size;
why cant you simply allocate three structs you need?
So allocation is done for number of dma desc and number of request
structure and the structure which keeps track for allocated pointers.
Here I am calculating total allocation size and then allocating once in
place of allocating them in loop.
+static int tegra_dma_slave_config(struct dma_chan *dc,
+ struct dma_slave_config *sconfig)
+{
+ struct tegra_dma_channel *tdc = to_tegra_dma_chan(dc);
+
+ if (!list_empty(&tdc->pending_sg_req)) {
+ dev_err(tdc2dev(tdc),
+ "dma requests are pending, cannot take new configuration");
+ return -EBUSY;
+ }
+
+ /* Slave specific configuration is must for channel configuration */
+ if (!dc->private) {
private is deprecated, pls dont use that
OK, I saw this in the linux-next and hence I used it. So is there any
way to send the client specific data to the dma driver.
I will remove this.
+
+ apb_seq = APB_SEQ_WRAP_WORD_1;
+
+ switch (direction) {
+ case DMA_MEM_TO_DEV:
+ apb_ptr = tdc->dma_sconfig.dst_addr;
+ apb_seq |= get_bus_width(tdc->dma_sconfig.dst_addr_width);
+ csr |= CSR_DIR;
+ break;
+
+ case DMA_DEV_TO_MEM:
+ apb_ptr = tdc->dma_sconfig.src_addr;
+ apb_seq |= get_bus_width(tdc->dma_sconfig.src_addr_width);
+ break;
you dont support DMA_MEM_TO_DEV?
Supported, first case ;-)
But MEM_TO_MEM is not supported by this dma controller.
+ if (!period_len)
+ period_len = buf_len;
i am not sure about this assignment here. Why should period length be
ZERO?
Just in case, if some client send it to ZERO then setting it to buf_len
in place of returning error.
+
+ if (buf_len % period_len) {
+ dev_err(tdc2dev(tdc),
+ "buf_len %d should be multiple of period_len %d\n",
+ buf_len, period_len);
+ return NULL;
+ }
I am assuming you are also putting this as a constraint in sound driver.
Yaah, I think sound driver make sure that the buf_len should be integer
multiple period_len. Not supporting if it is not to reduce the complexity.
+
+ half_buffer_notify = (buf_len == (2 * period_len)) ? true : false;
+ len = (half_buffer_notify) ? buf_len / 2 : period_len;
+ if ((len& 3) || (buf_addr& 3) ||
+ (len> tdc->tdma->chip_data.max_dma_count)) {
+ dev_err(tdc2dev(tdc),
+ "Dma length/memory address is not correct\n");
not supported would be apt
Fine. I will do it.
+#ifndef LINUX_TEGRA_DMA_H
+#define LINUX_TEGRA_DMA_H
+
+/*
+ * tegra_dma_burst_size: Burst size of dma.
+ * @TEGRA_DMA_AUTO: Based on transfer size, select the burst size.
+ * If it is multple of 32 bytes then burst size will be 32 bytes else
+ * If it is multiple of 16 bytes then burst size will be 16 bytes else
+ * If it is multiple of 4 bytes then burst size will be 4 bytes.
+ * @TEGRA_DMA_BURST_1: Burst size is 1 word/4 bytes.
+ * @TEGRA_DMA_BURST_4: Burst size is 4 word/16 bytes.
+ * @TEGRA_DMA_BURST_8: Burst size is 8 words/32 bytes.
+ */
+enum tegra_dma_burst_size {
+ TEGRA_DMA_AUTO,
+ TEGRA_DMA_BURST_1,
+ TEGRA_DMA_BURST_4,
+ TEGRA_DMA_BURST_8,
+};
why should this be global, clinet should pass them as defined in
dmaengine.h
The dma_slave_config on the dmaengine have the member as src_maxburst
and I understand that this is for maximum burst only.
and so passing the actual burst size, I defined new enums. Also wanted
to have the auto mode where I can select the BURST size based on the
request length.
if some encoding and understanding is maintain between client and dma
driver then I can get rid of this:
like src_maxburst = 0 is for the selection of burst size based on req
len otherwise use non-zero value of src_maxburst.
+ * @dma_dev: required DMA master client device.
+ * @dm_req_id: Peripheral dma requestor ID.
+ */
+struct tegra_dma_slave {
+ struct device *client_dev;
+ enum tegra_dma_requestor dma_req_id;
+ enum tegra_dma_burst_size burst_size;
pls remove
if above is OK then I can remove this.
+};
+
+#endif /* LINUX_TEGRA_DMA_H */
Please also update the driver to use the cookie helpers in
drivers/dma/dmaengine.h
I have already used cookie helper.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
