* [PATCH 1/1] DMA: PL330: allow submitting 2 requests at a time
@ 2013-02-07 10:38 Alvaro Moran
2013-02-07 11:31 ` Jassi Brar
0 siblings, 1 reply; 6+ messages in thread
From: Alvaro Moran @ 2013-02-07 10:38 UTC (permalink / raw)
To: linux-arm-kernel
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.
Signed-off-by: Alvaro Moran <dirac3000@gmail.com>
---
drivers/dma/pl330.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 80680ee..7814f8e 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -486,6 +486,8 @@ struct pl330_thread {
/* Index of the last submitted request or -1 if the DMA is stopped */
int req_running;
};
+/* We handle two requests at a time */
+#define NR_MAX_REQUESTS 2
enum pl330_dmac_state {
UNINIT,
@@ -2276,13 +2278,18 @@ static inline void handle_cyclic_desc_list(struct list_head *list)
static inline void fill_queue(struct dma_pl330_chan *pch)
{
struct dma_pl330_desc *desc;
+ int busy_reqs = 0;
int ret;
list_for_each_entry(desc, &pch->work_list, node) {
/* If already submitted */
- if (desc->status == BUSY)
- break;
+ if (desc->status == BUSY) {
+ busy_reqs++;
+ if (busy_reqs == NR_MAX_REQUESTS)
+ break;
+ continue;
+ }
ret = pl330_submit_req(pch->pl330_chid,
&desc->req);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 1/1] DMA: PL330: allow submitting 2 requests at a time
2013-02-07 10:38 [PATCH 1/1] DMA: PL330: allow submitting 2 requests at a time Alvaro Moran
@ 2013-02-07 11:31 ` Jassi Brar
2013-02-07 13:46 ` dirac3000
0 siblings, 1 reply; 6+ messages in thread
From: Jassi Brar @ 2013-02-07 11:31 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Feb 7, 2013 at 4:08 PM, Alvaro Moran <dirac3000@gmail.com> 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;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 1/1] DMA: PL330: allow submitting 2 requests at a time
2013-02-07 11:31 ` Jassi Brar
@ 2013-02-07 13:46 ` dirac3000
2013-02-07 14:12 ` Jassi Brar
0 siblings, 1 reply; 6+ messages in thread
From: dirac3000 @ 2013-02-07 13:46 UTC (permalink / raw)
To: linux-arm-kernel
On 02/07/2013 12:31 PM, Jassi Brar wrote:
> On Thu, Feb 7, 2013 at 4:08 PM, Alvaro Moran<dirac3000@gmail.com> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] DMA: PL330: allow submitting 2 requests at a time
2013-02-07 13:46 ` dirac3000
@ 2013-02-07 14:12 ` Jassi Brar
2013-02-07 17:08 ` dirac3000
0 siblings, 1 reply; 6+ messages in thread
From: Jassi Brar @ 2013-02-07 14:12 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Feb 7, 2013 at 7:16 PM, dirac3000 <dirac3000@gmail.com> wrote:
> On 02/07/2013 12:31 PM, Jassi Brar wrote:
>
>> On Thu, Feb 7, 2013 at 4:08 PM, Alvaro Moran<dirac3000@gmail.com> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] DMA: PL330: allow submitting 2 requests at a time
2013-02-07 14:12 ` Jassi Brar
@ 2013-02-07 17:08 ` dirac3000
2013-02-13 10:12 ` dirac3000
0 siblings, 1 reply; 6+ messages in thread
From: dirac3000 @ 2013-02-07 17:08 UTC (permalink / raw)
To: linux-arm-kernel
On 02/07/2013 03:12 PM, Jassi Brar wrote:
> On Thu, Feb 7, 2013 at 7:16 PM, dirac3000<dirac3000@gmail.com> wrote:
>> On 02/07/2013 12:31 PM, Jassi Brar wrote:
>>
>>> On Thu, Feb 7, 2013 at 4:08 PM, Alvaro Moran<dirac3000@gmail.com> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] DMA: PL330: allow submitting 2 requests at a time
2013-02-07 17:08 ` dirac3000
@ 2013-02-13 10:12 ` dirac3000
0 siblings, 0 replies; 6+ messages in thread
From: dirac3000 @ 2013-02-13 10:12 UTC (permalink / raw)
To: linux-arm-kernel
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<dirac3000@gmail.com> wrote:
>>> On 02/07/2013 12:31 PM, Jassi Brar wrote:
>>>
>>>> On Thu, Feb 7, 2013 at 4:08 PM, Alvaro Moran<dirac3000@gmail.com>
>>>> 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?
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-02-13 10:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-07 10:38 [PATCH 1/1] DMA: PL330: allow submitting 2 requests at a time Alvaro Moran
2013-02-07 11:31 ` Jassi Brar
2013-02-07 13:46 ` dirac3000
2013-02-07 14:12 ` Jassi Brar
2013-02-07 17:08 ` dirac3000
2013-02-13 10:12 ` dirac3000
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.