diff mbox

[09/12] dma: mmp_pdma: add support for byte-aligned transfers

Message ID 1375870770-14263-10-git-send-email-zonque@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Mack Aug. 7, 2013, 10:19 a.m. UTC
The PXA DMA controller has a DALGN register which allows for
byte-aligned DMA transfers. Use it in case any of the transfer
descriptors is not aligned to a mask of ~0x7.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/dma/mmp_pdma.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

Comments

Xiang Wang Aug. 8, 2013, 9:04 a.m. UTC | #1
> Subject: [PATCH 09/12] dma: mmp_pdma: add support for byte-aligned transfers
> 
> The PXA DMA controller has a DALGN register which allows for
> byte-aligned DMA transfers. Use it in case any of the transfer
> descriptors is not aligned to a mask of ~0x7.
> 
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> ---
>  drivers/dma/mmp_pdma.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
> index 1c2c00b..7eb235b 100644
> --- a/drivers/dma/mmp_pdma.c
> +++ b/drivers/dma/mmp_pdma.c
> @@ -109,6 +109,7 @@ struct mmp_pdma_chan {
>  	struct list_head chain_pending;	/* Link descriptors queue for pending */
>  	struct list_head chain_running;	/* Link descriptors queue for running */
>  	bool idle;			/* channel statue machine */
> +	bool byte_align;
> 
>  	struct dma_pool *desc_pool;	/* Descriptors pool */
>  };
> @@ -142,7 +143,7 @@ static void set_desc(struct mmp_pdma_phy *phy, dma_addr_t addr)
> 
>  static void enable_chan(struct mmp_pdma_phy *phy)
>  {
> -	u32 reg;
> +	u32 reg, dalgn;
> 
>  	if (!phy->vchan)
>  		return;
> @@ -150,6 +151,13 @@ static void enable_chan(struct mmp_pdma_phy *phy)
>  	reg = DRCMR(phy->vchan->drcmr);
>  	writel(DRCMR_MAPVLD | phy->idx, phy->base + reg);
> 
> +	dalgn = readl(phy->base + DALGN);
> +	if (phy->vchan->byte_align)
> +		dalgn |= 1 << phy->idx;
> +	else
> +		dalgn &= ~(1 << phy->idx);
> +	writel(dalgn, phy->base + DALGN);
> +
>  	reg = (phy->idx << 2) + DCSR;
>  	writel(readl(phy->base + reg) | DCSR_RUN,
>  					phy->base + reg);
> @@ -453,6 +461,7 @@ mmp_pdma_prep_memcpy(struct dma_chan *dchan,
>  		return NULL;
> 
>  	chan = to_mmp_pdma_chan(dchan);
> +	chan->byte_align = false;
> 
>  	if (!chan->dir) {
>  		chan->dir = DMA_MEM_TO_MEM;
> @@ -469,6 +478,8 @@ mmp_pdma_prep_memcpy(struct dma_chan *dchan,
>  		}
> 
>  		copy = min_t(size_t, len, PDMA_MAX_DESC_BYTES);
> +		if (dma_src & 0x7 || dma_dst & 0x7)
> +			chan->byte_align = true;
> 
>  		new->desc.dcmd = chan->dcmd | (DCMD_LENGTH & copy);
>  		new->desc.dsadr = dma_src;
> @@ -528,12 +539,16 @@ mmp_pdma_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
>  	if ((sgl == NULL) || (sg_len == 0))
>  		return NULL;
> 
> +	chan->byte_align = false;
> +
>  	for_each_sg(sgl, sg, sg_len, i) {
>  		addr = sg_dma_address(sg);
>  		avail = sg_dma_len(sgl);
> 
>  		do {
>  			len = min_t(size_t, avail, PDMA_MAX_DESC_BYTES);
> +			if (addr & 0x7)
> +				chan->byte_align = true;
> 
>  			/* allocate and populate the descriptor */
>  			new = mmp_pdma_alloc_descriptor(chan);
> --
> 1.8.3.1
We do need to set DALGN bit in some of our drivers. But we cannot configure this via standard dma engine API.
In this patch, dma address is used to determine whether or not to set DALGN. But what if we need to use 1-byte-aligned mode when addresses are 8-byte-aligned?
Is it proper to always use 1-byte-aligned mode?
Thanks.

Regards,
Xiang
Daniel Mack Aug. 8, 2013, 9:11 a.m. UTC | #2
Hi Xiang,

On 08.08.2013 11:04, Xiang Wang wrote:
>> Subject: [PATCH 09/12] dma: mmp_pdma: add support for byte-aligned
>> transfers
>> 
>> The PXA DMA controller has a DALGN register which allows for 
>> byte-aligned DMA transfers. Use it in case any of the transfer 
>> descriptors is not aligned to a mask of ~0x7.

[...]

> We do need to set DALGN bit in some of our drivers.

Which ones, and how do you currently do that? I didn't find any code to
support this yet in mmp-pdma.

> But we cannot
> configure this via standard dma engine API. In this patch, dma
> address is used to determine whether or not to set DALGN. But what if
> we need to use 1-byte-aligned mode when addresses are
> 8-byte-aligned?

Hmm, why would you need that? What's the constraint for this driver that
they have to rely on that?

> Is it proper to always use 1-byte-aligned mode? 

As far as I understand the datasheet, this bit has performance
implications and should only be used if really needed.

I think if you have that constraint in drivers, and the dmaengine
implementation can't determine that automatically, we should introduce a
flag or something in the dma_slave_config struct.


Daniel
Xiang Wang Aug. 16, 2013, 8:05 a.m. UTC | #3
On 08/08/2013 05:11 PM, Daniel Mack wrote:
> Hi Xiang,
>
> On 08.08.2013 11:04, Xiang Wang wrote:
>>> Subject: [PATCH 09/12] dma: mmp_pdma: add support for byte-aligned
>>> transfers
>>>
>>> The PXA DMA controller has a DALGN register which allows for
>>> byte-aligned DMA transfers. Use it in case any of the transfer
>>> descriptors is not aligned to a mask of ~0x7.
>
> [...]
>
>> We do need to set DALGN bit in some of our drivers.
>
> Which ones, and how do you currently do that? I didn't find any code to
> support this yet in mmp-pdma.
Hi, Daniel
When we use DMA in UART and let DMA handle the UART trailing bytes, we 
should set the DALGN bit. Otherwise, DMA controller cannot move the 
bytes from UART FIFO to memory correctly because they are not 
8-bytes-alligned.
>
>> But we cannot
>> configure this via standard dma engine API. In this patch, dma
>> address is used to determine whether or not to set DALGN. But what if
>> we need to use 1-byte-aligned mode when addresses are
>> 8-byte-aligned?
>
> Hmm, why would you need that? What's the constraint for this driver that
> they have to rely on that?
>
>> Is it proper to always use 1-byte-aligned mode?
>
> As far as I understand the datasheet, this bit has performance
> implications and should only be used if really needed.
>
> I think if you have that constraint in drivers, and the dmaengine
> implementation can't determine that automatically, we should introduce a
> flag or something in the dma_slave_config struct.
>
>
> Daniel
>
diff mbox

Patch

diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
index 1c2c00b..7eb235b 100644
--- a/drivers/dma/mmp_pdma.c
+++ b/drivers/dma/mmp_pdma.c
@@ -109,6 +109,7 @@  struct mmp_pdma_chan {
 	struct list_head chain_pending;	/* Link descriptors queue for pending */
 	struct list_head chain_running;	/* Link descriptors queue for running */
 	bool idle;			/* channel statue machine */
+	bool byte_align;
 
 	struct dma_pool *desc_pool;	/* Descriptors pool */
 };
@@ -142,7 +143,7 @@  static void set_desc(struct mmp_pdma_phy *phy, dma_addr_t addr)
 
 static void enable_chan(struct mmp_pdma_phy *phy)
 {
-	u32 reg;
+	u32 reg, dalgn;
 
 	if (!phy->vchan)
 		return;
@@ -150,6 +151,13 @@  static void enable_chan(struct mmp_pdma_phy *phy)
 	reg = DRCMR(phy->vchan->drcmr);
 	writel(DRCMR_MAPVLD | phy->idx, phy->base + reg);
 
+	dalgn = readl(phy->base + DALGN);
+	if (phy->vchan->byte_align)
+		dalgn |= 1 << phy->idx;
+	else
+		dalgn &= ~(1 << phy->idx);
+	writel(dalgn, phy->base + DALGN);
+
 	reg = (phy->idx << 2) + DCSR;
 	writel(readl(phy->base + reg) | DCSR_RUN,
 					phy->base + reg);
@@ -453,6 +461,7 @@  mmp_pdma_prep_memcpy(struct dma_chan *dchan,
 		return NULL;
 
 	chan = to_mmp_pdma_chan(dchan);
+	chan->byte_align = false;
 
 	if (!chan->dir) {
 		chan->dir = DMA_MEM_TO_MEM;
@@ -469,6 +478,8 @@  mmp_pdma_prep_memcpy(struct dma_chan *dchan,
 		}
 
 		copy = min_t(size_t, len, PDMA_MAX_DESC_BYTES);
+		if (dma_src & 0x7 || dma_dst & 0x7)
+			chan->byte_align = true;
 
 		new->desc.dcmd = chan->dcmd | (DCMD_LENGTH & copy);
 		new->desc.dsadr = dma_src;
@@ -528,12 +539,16 @@  mmp_pdma_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
 	if ((sgl == NULL) || (sg_len == 0))
 		return NULL;
 
+	chan->byte_align = false;
+
 	for_each_sg(sgl, sg, sg_len, i) {
 		addr = sg_dma_address(sg);
 		avail = sg_dma_len(sgl);
 
 		do {
 			len = min_t(size_t, avail, PDMA_MAX_DESC_BYTES);
+			if (addr & 0x7)
+				chan->byte_align = true;
 
 			/* allocate and populate the descriptor */
 			new = mmp_pdma_alloc_descriptor(chan);