Subject: [PATCH 1/1] DMA: PL330: allow submitting 2 requests at a time

From: [email-masked] (dirac3000)
Date: Wed, 13 Feb 2013 11:12:55 +0100

On 02/07/2013 06:08 PM, dirac3000 wrote:

> On 02/07/2013 03:12 PM, Jassi Brar wrote:
>
>> On Thu, Feb 7, 2013 at 7:16 PM, dirac3000<[email-masked]> wrote:
>>> On 02/07/2013 12:31 PM, Jassi Brar wrote:
>>>
>>>> On Thu, Feb 7, 2013 at 4:08 PM, Alvaro Moran<[email-masked]>
>>>> wrote:
>>>>>
>>>>> Due to the original driver design, only one request was processed at a
>>>>> time by the driver, even if the low-level part of the driver was
>>>>> able to
>>>>> handle 2 requests.
>>>>> With this patch we are able to create 2 microcodes per thread and to
>>>>> launch the second transfer on the interrupt handler of the first one,
>>>>> instead of having to wait for the tasklet to generate the microcode.
>>>>>
>>>> The following seems more appropriate and complete. Does it fix your
>>>> problem?
>>>>
>>>> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
>>>> index 758122f..a821d71 100644
>>>> --- a/drivers/dma/pl330.c
>>>> +++ b/drivers/dma/pl330.c
>>>> @@ -2292,13 +2292,12 @@ static inline void fill_queue(struct
>>>> dma_pl330_chan *pch)
>>>>
>>>> /* If already submitted */
>>>> if (desc->status == BUSY)
>>>> - break;
>>>> + continue;
>>>>
>>>> ret = pl330_submit_req(pch->pl330_chid,
>>>> &desc->req);
>>>> if (!ret) {
>>>> desc->status = BUSY;
>>>> - break;
>>>> } else if (ret == -EAGAIN) {
>>>> /* QFull or DMAC Dying */
>>>> break;
>>>
>>>
>>>
>>> Actually that isn't good enough. With your patch it will keep on
>>> looping on
>>> the pch->work_list entries, but it will call pl330_submit_req the
>>> first time
>>> only. I want it to call the function twice, so it will generate 2
>>> microcodes
>>> (one per available request) and it will be ready the moment we get
>>> into the
>>> interrupt handler.
>>
>> Why would it "keep on looping"? It's a for loop that will exit after
>> iterating over the list once or when the lower layer indicates QFull -
>> whichever comes first. Practically it achieves the same effect only
>> without introducing a new local variable 'busy_reqs'
>> Did you actually test the patch? If yes and it didn't work, please
>> share some log suitable log.
>> thnx.
>
>
> Oh, my fault, you are right, I didn't read it carefully!
> Now I actually tested the patch, so I am 100% sure it works and it
> increases the performance of the requests when they are correctly queued.
>
> Thanks,
>
> -Alvaro

As I told you in a previous email I tested the patch and it works fine.
Any chance of seeing it accepted in the next version?