On Mon, Feb 17, 2014 at 3:05 PM, Lars-Peter Clausen <lars@xxxxxxxxxx> wrote:
> On 02/17/2014 10:29 AM, Srikanth Thokala wrote:
>>
>> On Mon, Feb 17, 2014 at 2:13 PM, Vinod Koul <vinod.koul@xxxxxxxxx> wrote:
>>>
>>> On Sat, Feb 15, 2014 at 05:30:17PM +0530, Srikanth Thokala wrote:
>>>>
>>>> The current implementation of interleaved DMA API support multiple
>>>> frames only when the memory is contiguous by incrementing src_start/
>>>> dst_start members of interleaved template.
>>>>
>>>> But, when the memory is non-contiguous it will restrict slave device
>>>> to not submit multiple frames in a batch. This patch handles this
>>>> issue by allowing the slave device to send array of interleaved dma
>>>> templates each having a different memory location.
>>>
>>> This seems to be missing the numbers of templates you are sending,
>>> wouldnt this
>>> require sending ARRAY_SiZE too?
>>>
>>> And why send double pointer?
>>
>>
>> Array size is not required, when we pass the double pointer. The last
>> element would be
>> pointed to NULL and we could get the number of templates from this
>> condition.
>> Here is an example snippet,
>>
>> In slave device driver,
>>
>> struct dma_interleaved_template **xts;
>>
>> xts = kcalloc(frm_cnt+1, sizeof(struct
>> dma_interleaved_template *), GFP_KERNEL);
>> /* Error check for xts */
>> for (i = 0; i < frm_cnt; i++) {
>> xts[i] = kmalloc(sizeof(struct
>> dma_interleaved_template), GFP_KERNEL);
>> /* Error check for xts[i] */
>> }
>> xts[i] = NULL;
>>
>> In DMA engine driver, we could get the number of frames by,
>>
>> for (; xts[frmno] != NULL; frmno++);
>>
>> I felt this way is simpler than adding an extra argument to the API.
>> Please let me know
>> your opinion and suggest me a better way.
>
>
> I think Vinod's suggestion of passing in an array of interleaved_templates
> and the size of the array is better than what you are currently doing.
Ok, Lars. I will update with this in my v4. Thanks.
>
> Btw. you also need to update the current implementations and users of the
> API accordingly.
Yes, I have updated them in this patch.
Thanks
Srikanth
>
>
>>
>>>
>>> --
>>> ~Vinod
>>>
>>>>
>>>> Signed-off-by: Srikanth Thokala <sthokal@xxxxxxxxxx>
>>>> ---
>>>> Documentation/dmaengine.txt | 2 +-
>>>> drivers/dma/imx-dma.c | 3 ++-
>>>> drivers/dma/sirf-dma.c | 3 ++-
>>>> drivers/media/platform/m2m-deinterlace.c | 2 +-
>>>> include/linux/dmaengine.h | 6 +++---
>>>> 5 files changed, 9 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/Documentation/dmaengine.txt b/Documentation/dmaengine.txt
>>>> index 879b6e3..c642614 100644
>>>> --- a/Documentation/dmaengine.txt
>>>> +++ b/Documentation/dmaengine.txt
>>>> @@ -94,7 +94,7 @@ The slave DMA usage consists of following steps:
>>>> size_t period_len, enum dma_data_direction direction);
>>>>
>>>> struct dma_async_tx_descriptor *(*device_prep_interleaved_dma)(
>>>> - struct dma_chan *chan, struct dma_interleaved_template
>>>> *xt,
>>>> + struct dma_chan *chan, struct dma_interleaved_template
>>>> **xts,
>>>> unsigned long flags);
>>>>
>>>> The peripheral driver is expected to have mapped the scatterlist
>>>> for
>>>> diff --git a/drivers/dma/imx-dma.c b/drivers/dma/imx-dma.c
>>>> index 6f9ac20..e2c52ce 100644
>>>> --- a/drivers/dma/imx-dma.c
>>>> +++ b/drivers/dma/imx-dma.c
>>>> @@ -954,12 +954,13 @@ static struct dma_async_tx_descriptor
>>>> *imxdma_prep_dma_memcpy(
>>>> }
>>>>
>>>> static struct dma_async_tx_descriptor *imxdma_prep_dma_interleaved(
>>>> - struct dma_chan *chan, struct dma_interleaved_template *xt,
>>>> + struct dma_chan *chan, struct dma_interleaved_template **xts,
>>>> unsigned long flags)
>>>> {
>>>> struct imxdma_channel *imxdmac = to_imxdma_chan(chan);
>>>> struct imxdma_engine *imxdma = imxdmac->imxdma;
>>>> struct imxdma_desc *desc;
>>>> + struct dma_interleaved_template *xt = *xts;
>>>>
>>>> dev_dbg(imxdma->dev, "%s channel: %d src_start=0x%llx
>>>> dst_start=0x%llx\n"
>>>> " src_sgl=%s dst_sgl=%s numf=%zu frame_size=%zu\n",
>>>> __func__,
>>>> diff --git a/drivers/dma/sirf-dma.c b/drivers/dma/sirf-dma.c
>>>> index d4d3a31..b6a150b 100644
>>>> --- a/drivers/dma/sirf-dma.c
>>>> +++ b/drivers/dma/sirf-dma.c
>>>> @@ -509,12 +509,13 @@ sirfsoc_dma_tx_status(struct dma_chan *chan,
>>>> dma_cookie_t cookie,
>>>> }
>>>>
>>>> static struct dma_async_tx_descriptor *sirfsoc_dma_prep_interleaved(
>>>> - struct dma_chan *chan, struct dma_interleaved_template *xt,
>>>> + struct dma_chan *chan, struct dma_interleaved_template **xts,
>>>> unsigned long flags)
>>>> {
>>>> struct sirfsoc_dma *sdma = dma_chan_to_sirfsoc_dma(chan);
>>>> struct sirfsoc_dma_chan *schan =
>>>> dma_chan_to_sirfsoc_dma_chan(chan);
>>>> struct sirfsoc_dma_desc *sdesc = NULL;
>>>> + struct dma_interleaved_template *xt = *xts;
>>>> unsigned long iflags;
>>>> int ret;
>>>>
>>>> diff --git a/drivers/media/platform/m2m-deinterlace.c
>>>> b/drivers/media/platform/m2m-deinterlace.c
>>>> index 6bb86b5..468110a 100644
>>>> --- a/drivers/media/platform/m2m-deinterlace.c
>>>> +++ b/drivers/media/platform/m2m-deinterlace.c
>>>> @@ -343,7 +343,7 @@ static void deinterlace_issue_dma(struct
>>>> deinterlace_ctx *ctx, int op,
>>>> ctx->xt->dst_sgl = true;
>>>> flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
>>>>
>>>> - tx = dmadev->device_prep_interleaved_dma(chan, ctx->xt, flags);
>>>> + tx = dmadev->device_prep_interleaved_dma(chan, &ctx->xt, flags);
>>>> if (tx == NULL) {
>>>> v4l2_warn(&pcdev->v4l2_dev, "DMA interleaved prep
>>>> error\n");
>>>> return;
>>>> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
>>>> index c5c92d5..2f77a9a 100644
>>>> --- a/include/linux/dmaengine.h
>>>> +++ b/include/linux/dmaengine.h
>>>> @@ -675,7 +675,7 @@ struct dma_device {
>>>> size_t period_len, enum dma_transfer_direction direction,
>>>> unsigned long flags, void *context);
>>>> struct dma_async_tx_descriptor *(*device_prep_interleaved_dma)(
>>>> - struct dma_chan *chan, struct dma_interleaved_template
>>>> *xt,
>>>> + struct dma_chan *chan, struct dma_interleaved_template
>>>> **xts,
>>>> unsigned long flags);
>>>> int (*device_control)(struct dma_chan *chan, enum dma_ctrl_cmd
>>>> cmd,
>>>> unsigned long arg);
>>>> @@ -752,10 +752,10 @@ static inline struct dma_async_tx_descriptor
>>>> *dmaengine_prep_dma_cyclic(
>>>> }
>>>>
>>>> static inline struct dma_async_tx_descriptor
>>>> *dmaengine_prep_interleaved_dma(
>>>> - struct dma_chan *chan, struct dma_interleaved_template
>>>> *xt,
>>>> + struct dma_chan *chan, struct dma_interleaved_template
>>>> **xts,
>>>> unsigned long flags)
>>>> {
>>>> - return chan->device->device_prep_interleaved_dma(chan, xt, flags);
>>>> + return chan->device->device_prep_interleaved_dma(chan, xts,
>>>> flags);
>>>> }
>>>>
>>>> static inline int dma_get_slave_caps(struct dma_chan *chan, struct
>>>> dma_slave_caps *caps)
>>>> --
>>>> 1.7.9.5
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
>>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>>
>>> --
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel"
>>> in
>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at http://www.tux.org/lkml/
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
