| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CA+mB=1KZJ24KXKBtPpvq+Ssjy6pJ6RbJvq+n=ZiXOQFA7B1gOQ@mail.gmail.com>
Date: Mon, 17 Feb 2014 15:22:38 +0530
From: Srikanth Thokala <sthokal@...inx.com>
To: Lars-Peter Clausen <lars@...afoo.de>
Cc: Srikanth Thokala <sthokal@...inx.com>,
Vinod Koul <vinod.koul@...el.com>, dan.j.williams@...el.com,
michal.simek@...inx.com, Grant Likely <grant.likely@...aro.org>,
robh+dt@...nel.org, Levente Kurusa <levex@...ux.com>,
andriy.shevchenko@...ux.jf.intel.com, dmaengine@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
devicetree@...r.kernel.org
Subject: Re: [PATCH v3 1/3] dma: Support multiple interleaved frames with
non-contiguous memory
On Mon, Feb 17, 2014 at 3:14 PM, Lars-Peter Clausen <lars@...afoo.de> wrote:
> On 02/17/2014 10:42 AM, Srikanth Thokala wrote:
>>
>> On Mon, Feb 17, 2014 at 3:05 PM, Lars-Peter Clausen <lars@...afoo.de>
>> wrote:
>>>
>>> On 02/17/2014 10:29 AM, Srikanth Thokala wrote:
>>>>
>>>>
>>>> On Mon, Feb 17, 2014 at 2:13 PM, Vinod Koul <vinod.koul@...el.com>
>>>> 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.
>
>
> But you didn't update them accordingly to your proposed semantics. The
> caller didn't NULL terminate the array and the drivers did blindly assume
> there will always be exactly one transfer.
Ok, got it. I will correct in v4. Thanks for pointing this.
Srikanth
>
>
>>
>> Thanks
>> Srikanth
>>
>>
>>>
>>>
>>>>
>>>>>
>>>>> --
>>>>> ~Vinod
>>>>>
>>>>>>
>>>>>> Signed-off-by: Srikanth Thokala <sthokal@...inx.com>
>>>>>> ---
>>>>>> 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@...r.kernel.org
>>>>>> 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@...r.kernel.org
>>>>> 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@...r.kernel.org
>>> 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@...r.kernel.org
> 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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists