From mboxrd@z Thu Jan 1 00:00:00 1970 From: dirac3000@gmail.com (dirac3000) Date: Thu, 07 Feb 2013 18:08:35 +0100 Subject: [PATCH 1/1] DMA: PL330: allow submitting 2 requests at a time In-Reply-To: References: <1360233523-20825-1-git-send-email-dirac3000@gmail.com> <5113B03D.5010206@gmail.com> Message-ID: <5113DF93.3080401@gmail.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 02/07/2013 03:12 PM, Jassi Brar wrote: > On Thu, Feb 7, 2013 at 7:16 PM, dirac3000 wrote: >> On 02/07/2013 12:31 PM, Jassi Brar wrote: >> >>> On Thu, Feb 7, 2013 at 4:08 PM, Alvaro Moran 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