| 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
| ||
|
Date: Wed, 12 Mar 2014 00:07:14 +0530
From: Srikanth Thokala <sthokal@...inx.com>
To: Srikanth Thokala <sthokal@...inx.com>
Cc: Jassi Brar <jaswinder.singh@...aro.org>,
Jassi Brar <jassisinghbrar@...il.com>,
Dan Williams <dan.j.williams@...el.com>,
Vinod Koul <vinod.koul@...el.com>,
Michal Simek <michal.simek@...inx.com>,
Grant Likely <grant.likely@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Levente Kurusa <levex@...ux.com>,
Lars-Peter Clausen <lars@...afoo.de>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
dmaengine@...r.kernel.org,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v4 2/2] dma: Add Xilinx AXI Video Direct Memory Access
Engine driver support
On Wed, Mar 12, 2014 at 12:02 AM, Srikanth Thokala <sthokal@...inx.com> wrote:
> On Tue, Mar 11, 2014 at 7:14 PM, Jassi Brar <jaswinder.singh@...aro.org> wrote:
>> On 11 March 2014 00:00, Srikanth Thokala <sthokal@...inx.com> wrote:
>>> On Mon, Mar 10, 2014 at 9:30 PM, Jassi Brar <jassisinghbrar@...il.com> wrote:
>>>> On Thu, Mar 6, 2014 at 7:18 PM, Srikanth Thokala <sthokal@...inx.com> wrote:
>>>>
>>>>> +static struct dma_async_tx_descriptor *
>>>>> +xilinx_vdma_dma_prep_interleaved(struct dma_chan *dchan,
>>>>> + struct dma_interleaved_template *xt,
>>>>> + unsigned long flags)
>>>>> +{
>>>>> + struct xilinx_vdma_chan *chan = to_xilinx_chan(dchan);
>>>>> + struct xilinx_vdma_tx_descriptor *desc;
>>>>> + struct xilinx_vdma_tx_segment *segment;
>>>>> + struct xilinx_vdma_tx_segment *prev = NULL;
>>>>> + int i;
>>>>> +
>>>>> + if ((xt->dir != DMA_MEM_TO_DEV) && (xt->dir != DMA_DEV_TO_MEM))
>>>>> + return NULL;
>>>>> +
>>>>> + if (!xt->numf || !xt->sgl[0].size)
>>>>> + return NULL;
>>>>> +
>>>>> + /* Allocate a transaction descriptor. */
>>>>> + desc = xilinx_vdma_alloc_tx_descriptor(chan);
>>>>> + if (!desc)
>>>>> + return NULL;
>>>>> +
>>>>> + dma_async_tx_descriptor_init(&desc->async_tx, &chan->common);
>>>>> + desc->async_tx.tx_submit = xilinx_vdma_tx_submit;
>>>>> + desc->async_tx.cookie = 0;
>>>>> + async_tx_ack(&desc->async_tx);
>>>>> +
>>>>> + /* Build the list of transaction segments. */
>>>>> + for (i = 0; i < xt->frame_size; i++) {
>>>>> + struct xilinx_vdma_desc_hw *hw;
>>>>> +
>>>>> + /* Allocate the link descriptor from DMA pool */
>>>>> + segment = xilinx_vdma_alloc_tx_segment(chan);
>>>>> + if (!segment)
>>>>> + goto error;
>>>>> +
>>>>> + /* Fill in the hardware descriptor */
>>>>> + hw = &segment->hw;
>>>>> + hw->vsize = xt->numf;
>>>>> + hw->hsize = xt->sgl[0].size;
>>>>> + hw->stride = xt->sgl[0].icg <<
>>>>> + XILINX_VDMA_FRMDLY_STRIDE_STRIDE_SHIFT;
>>>>>
>>>> It seems the xt->frame_size is (should be?) always going to be 1?
>>>> If yes, the for-loop isn't needed.
>>>> If no, you should probably use 'i' as the index to sgl[], and not always 0.
>>>
>>> It can be either
>>> '1': User can queue each segment and submit all the segments at once as a
>>> single async tx descriptor. I am using this for scatter-gather mode of
>>> operation where the src_start/dst_start is different for each segment.
>>> 'Number of frames': In this case the driver gets contiguous frame buffer memory
>>> and it prepares segments with addresses as multiples of
>>> src_start/dst_start.
>>>
>> Quoting you from earlier thread ....
>> " In video frame context,
>> chunk.size -> hsize
>> chunk.icg -> stride
>> numf -> vsize
>> and frame_size is always 1 as it will have only one chunk in a line. "
>> ... which makes sense.
>>
>> I hope you realize that, 'frame_size' denotes the number of chunks in
>> a frame and 'numf' denotes the number of frames in the transfer
>> request. It seems wrong when you assume vsize:=numf but
>> frame_size>1 ... i.e, each horizontal video line has two or more
>> dis-contiguous chunks? This might work for client drivers written
>> specifically for Xilinx AXI Video DMAC but will fail over any other
>> controller driver.
>>
>> IMO, xilinx_vdma.c should expect only 1 video frame per
>> device_prep_interleaved_dma() call (i.e, vsize=numf and frame_size==1)
>> Let the client driver submit 1/2/.../16 requests before kick
>> starting the DMA. Upon dma_async_tx_descriptor.callback(), the client
>> 'refills' the frame and re-queues it back. This should work because
>> the video-frames' attributes and locations are not going to change
>> within a session.
>>
>>> Please let me know if you see any issues in this implementation.
>>>
>> I am ok if you want the driver upstream now and want to 'fix' it when
>> you actually start seeing problems :)
>
> I agree with you and I have no issues in fixing this now. I will fix
> it in my v6
Apologies, it is v5.
> with the driver accepting only one frame/interleave_dma call.
>
> But, I feel this information has to be documented to the description of
> interleaved_dma API as it is leading to interpret the members of this
> structure differently wrt video frame terminology.
>
> Thanks
> Srikanth
>
>> --
>> 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