| Message ID | 496565EC904933469F292DDA3F1663E60287D6286E@dlee06.ent.ti.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Thu, Mar 19, 2009 at 10:42 PM, Guzman Lugo, Fernando <x0095840@ti.com> wrote: > > > Hi Felipe, > > Â Â Â Â I am seeing with this patch because of the timeout: > > DSP device detected !! > DSPProcessor_Attach succeeded. > dspbridge: timed out waiting for mailbox > dspbridge: timed out waiting for mailbox > dspbridge: timed out waiting for mailbox > dspbridge: timed out waiting for mailbox > dspbridge: timed out waiting for mailbox > dspbridge: timed out waiting for mailbox > dspbridge: timed out waiting for mailbox > dspbridge: timed out waiting for mailbox > dspbridge: timed out waiting for mailbox > dspbridge: timed out waiting for mailbox > dspbridge: timed out waiting for mailbox > ... > > Did you see any issue when you change to 1ms? Maybe we can use a bigger timeout. Did you apply patch #1 of the B series? I didn't see any timeout on my tests.
Yes, I applied this; in fact I have applied all the patches. If I increase the timeout there are no problems. The test I run creates 4 process and each one does several a lot of calls to InputChnl and OutputChnl, so I think this test is using the mailbox a lot and would be better a bigger timeout. What do you think? Regards, Fernando. -----Original Message----- From: Felipe Contreras [mailto:felipe.contreras@gmail.com] Sent: Thursday, March 19, 2009 3:04 PM To: Guzman Lugo, Fernando Cc: linux-omap@vger.kernel.org; Kanigeri, Hari; Hiroshi DOYU; Ameya Palande; Felipe Contreras Subject: Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value On Thu, Mar 19, 2009 at 10:42 PM, Guzman Lugo, Fernando <x0095840@ti.com> wrote: > > > Hi Felipe, > > Â Â Â Â I am seeing with this patch because of the timeout: > > DSP device detected !! > DSPProcessor_Attach succeeded. > dspbridge: timed out waiting for mailbox > dspbridge: timed out waiting for mailbox > dspbridge: timed out waiting for mailbox > dspbridge: timed out waiting for mailbox > dspbridge: timed out waiting for mailbox > dspbridge: timed out waiting for mailbox > dspbridge: timed out waiting for mailbox > dspbridge: timed out waiting for mailbox > dspbridge: timed out waiting for mailbox > dspbridge: timed out waiting for mailbox > dspbridge: timed out waiting for mailbox > ... > > Did you see any issue when you change to 1ms? Maybe we can use a bigger timeout. Did you apply patch #1 of the B series? I didn't see any timeout on my tests.
On Thu, Mar 19, 2009 at 11:19 PM, Guzman Lugo, Fernando <x0095840@ti.com> wrote: > > Yes, I applied this; in fact I have applied all the patches. If I increase the timeout there are no problems. The test I run creates 4 process and each one does several a lot of calls to InputChnl and OutputChnl, so I think this test is using the mailbox a lot and would be better a bigger timeout. What do you think? How fast are these messages sent? Can you track down which functions are calling CHNLSM_InterruptDSP2 and making these timeouts happen. I think it's safe to leave the timeout at 10, but that means it's possible the code will be busy-looping up to 10 ms which will increase the CPU load. Somebody from Nokia (Siarhei?) suggested to idle-wait for the mbox empty irq, I think that's the best way to implement this, but at least for the use cases I'm interested in (video encoding/decoding) timeouts don't seem to be an issue anymore.
This is a stress test, it creates 4 processes and each process will do 1000 transfers using streams, so the trace is: STRM_Issue -> WMD_CHNL_AddIOReq -> IO_Schedule IO_Schedule schedules a call to IO_DPC using task let. IO_DPC -> IO_DispatchChnl -> InputChnl -> CHNLSM_InterruptDSP2 Also IO_DispatchChnl -> OutputChnl -> CHNLSM_InterruptDSP2. As we can call a lot CHNLSM_InterruptDSP2 in this test, there is a problem with the timeout. However running other tests, videos and mp3 there no problems. I think we should change to 10ms, only to make sure there is no problem when CHNLSM_InterruptDSP2 is called a lot. Let me know if you are agreed. Or have some comments about it. Regards, Fernando. -----Original Message----- From: Felipe Contreras [mailto:felipe.contreras@gmail.com] Sent: Thursday, March 19, 2009 3:35 PM To: Guzman Lugo, Fernando Cc: linux-omap@vger.kernel.org; Kanigeri, Hari; Hiroshi DOYU; Ameya Palande; Felipe Contreras Subject: Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value On Thu, Mar 19, 2009 at 11:19 PM, Guzman Lugo, Fernando <x0095840@ti.com> wrote: > > Yes, I applied this; in fact I have applied all the patches. If I increase the timeout there are no problems. The test I run creates 4 process and each one does several a lot of calls to InputChnl and OutputChnl, so I think this test is using the mailbox a lot and would be better a bigger timeout. What do you think? How fast are these messages sent? Can you track down which functions are calling CHNLSM_InterruptDSP2 and making these timeouts happen. I think it's safe to leave the timeout at 10, but that means it's possible the code will be busy-looping up to 10 ms which will increase the CPU load. Somebody from Nokia (Siarhei?) suggested to idle-wait for the mbox empty irq, I think that's the best way to implement this, but at least for the use cases I'm interested in (video encoding/decoding) timeouts don't seem to be an issue anymore.
On Fri, Mar 20, 2009 at 2:00 AM, Guzman Lugo, Fernando <x0095840@ti.com> wrote: > > > > This is a stress test, it creates 4 processes and each process will do 1000 transfers using streams, so the trace is: > > STRM_Issue ->  WMD_CHNL_AddIOReq  -> IO_Schedule > > IO_Schedule schedules a call to IO_DPC using task let. > > IO_DPC -> IO_DispatchChnl -> InputChnl -> CHNLSM_InterruptDSP2 > > Also    IO_DispatchChnl -> OutputChnl -> CHNLSM_InterruptDSP2. > > > As we can call a lot CHNLSM_InterruptDSP2 in this test, there is a problem with the timeout. However running other tests, videos and mp3 there no problems. I think we should change to 10ms, only to make sure there is no problem when CHNLSM_InterruptDSP2 is called a lot. > > Let me know if you are agreed. Or have some comments about it. Well again, the best way to implement the wait for a slot in the mailbox is with interrupts, not busy-looping. If we busy-loop too much that would increase the CPU usage, and that would be bad. That's why I want to use the 1ms timeout; to find issues that cause increase in CPU load. But for now I think 10ms is the safest, as it's the current value. If somebody wants to pin-point issues, then the timeout should be decreased.
On Fri, Mar 20, 2009 at 6:54 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote: > From: ext Felipe Contreras <felipe.contreras@gmail.com> > Subject: Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value > Date: Fri, 20 Mar 2009 01:06:16 +0100 > >> On Fri, Mar 20, 2009 at 2:00 AM, Guzman Lugo, Fernando <x0095840@ti.com> wrote: >> > >> > >> > >> > This is a stress test, it creates 4 processes and each process will do 1000 transfers using streams, so the trace is: >> > >> > STRM_Issue ->  WMD_CHNL_AddIOReq  -> IO_Schedule >> > >> > IO_Schedule schedules a call to IO_DPC using task let. >> > >> > IO_DPC -> IO_DispatchChnl -> InputChnl -> CHNLSM_InterruptDSP2 >> > >> > Also    IO_DispatchChnl -> OutputChnl -> CHNLSM_InterruptDSP2. >> > >> > >> > As we can call a lot CHNLSM_InterruptDSP2 in this test, there is a problem with the timeout. However running other tests, videos and mp3 there no problems. I think we should change to 10ms, only to make sure there is no problem when CHNLSM_InterruptDSP2 is called a lot. >> > >> > Let me know if you are agreed. Or have some comments about it. >> >> Well again, the best way to implement the wait for a slot in the >> mailbox is with interrupts, not busy-looping. If we busy-loop too much >> that would increase the CPU usage, and that would be bad. > > I think that s/w queuing of messages would be more efficient to allow > multiple senders to continue thier work anyway, especially in the case > of having these streamings. Indeed. But what would happen if the application is sending messages way too fast for the DSP to handle? For example, some encoding algorithm might be too heavy, and if we are in a live situation, like video call, then it's ok to drop messages, but user-space needs to be notified of these and adjust the quality-of-service. But of course some other messages, like control messages (start, stop, etc.) should not be dropped, ever, so they must be queued.
On Fri, Mar 20, 2009 at 9:36 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote: > From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com> > Subject: Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value > Date: Fri, 20 Mar 2009 06:54:04 +0200 (EET) > >> From: ext Felipe Contreras <felipe.contreras@gmail.com> >> Subject: Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value >> Date: Fri, 20 Mar 2009 01:06:16 +0100 >> >> > On Fri, Mar 20, 2009 at 2:00 AM, Guzman Lugo, Fernando <x0095840@ti.com> wrote: >> > > >> > > >> > > >> > > This is a stress test, it creates 4 processes and each process will do 1000 transfers using streams, so the trace is: >> > > >> > > STRM_Issue ->  WMD_CHNL_AddIOReq  -> IO_Schedule >> > > >> > > IO_Schedule schedules a call to IO_DPC using task let. >> > > >> > > IO_DPC -> IO_DispatchChnl -> InputChnl -> CHNLSM_InterruptDSP2 >> > > >> > > Also    IO_DispatchChnl -> OutputChnl -> CHNLSM_InterruptDSP2. >> > > >> > > >> > > As we can call a lot CHNLSM_InterruptDSP2 in this test, there is a problem with the timeout. However running other tests, videos and mp3 there no problems. I think we should change to 10ms, only to make sure there is no problem when CHNLSM_InterruptDSP2 is called a lot. >> > > >> > > Let me know if you are agreed. Or have some comments about it. >> > >> > Well again, the best way to implement the wait for a slot in the >> > mailbox is with interrupts, not busy-looping. If we busy-loop too much >> > that would increase the CPU usage, and that would be bad. >> >> I think that s/w queuing of messages would be more efficient to allow >> multiple senders to continue thier work anyway, especially in the case >> of having these streamings. > > Migrating to the existing mailbox driver("plat-omap/mailbox.c") is > also another option to support multiple OMAP architectures at the same > time. That mailbox driver has already had a message queuing feature > inside and it's buildable on OMAP3. Yeap, I have it in my to-do list, so if somebody doesn't beat me to it I play with that. Is there a way to play with the existing mailbox driver independently of dsp-bridge?
Felipe, > -----Original Message----- > From: Felipe Contreras [mailto:felipe.contreras@gmail.com] > Sent: Friday, March 20, 2009 3:48 PM > To: Hiroshi DOYU > Cc: Gupta, Ramesh; Kanigeri, Hari; Guzman Lugo, Fernando; > linux-omap@vger.kernel.org; ameya.palande@nokia.com > Subject: Re: [PATCH B 3/3] tidspbridge: decreate timeout to a > saner value > > On Fri, Mar 20, 2009 at 9:36 AM, Hiroshi DOYU > <Hiroshi.DOYU@nokia.com> wrote: > > From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com> > > Subject: Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner > > value > > Date: Fri, 20 Mar 2009 06:54:04 +0200 (EET) > > > >> From: ext Felipe Contreras <felipe.contreras@gmail.com> > >> Subject: Re: [PATCH B 3/3] tidspbridge: decreate timeout > to a saner > >> value > >> Date: Fri, 20 Mar 2009 01:06:16 +0100 > >> > >> > On Fri, Mar 20, 2009 at 2:00 AM, Guzman Lugo, Fernando > <x0095840@ti.com> wrote: > >> > > > >> > > > >> > > > >> > > This is a stress test, it creates 4 processes and each > process will do 1000 transfers using streams, so the trace is: > >> > > > >> > > STRM_Issue ->  WMD_CHNL_AddIOReq  -> IO_Schedule > >> > > > >> > > IO_Schedule schedules a call to IO_DPC using task let. > >> > > > >> > > IO_DPC -> IO_DispatchChnl -> InputChnl -> CHNLSM_InterruptDSP2 > >> > > > >> > > Also    IO_DispatchChnl -> OutputChnl -> > CHNLSM_InterruptDSP2. > >> > > > >> > > > >> > > As we can call a lot CHNLSM_InterruptDSP2 in this > test, there is a problem with the timeout. However running > other tests, videos and mp3 there no problems. I think we > should change to 10ms, only to make sure there is no problem > when CHNLSM_InterruptDSP2 is called a lot. > >> > > > >> > > Let me know if you are agreed. Or have some comments about it. > >> > > >> > Well again, the best way to implement the wait for a slot in the > >> > mailbox is with interrupts, not busy-looping. If we > busy-loop too > >> > much that would increase the CPU usage, and that would be bad. > >> > >> I think that s/w queuing of messages would be more > efficient to allow > >> multiple senders to continue thier work anyway, especially in the > >> case of having these streamings. > > > > Migrating to the existing mailbox driver("plat-omap/mailbox.c") is > > also another option to support multiple OMAP architectures > at the same > > time. That mailbox driver has already had a message queuing feature > > inside and it's buildable on OMAP3. > > Yeap, I have it in my to-do list, so if somebody doesn't beat > me to it I play with that. We already started looking into this :), made some progress, will provide more updates in a day or two. Regards Ramesh Gupta G -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Mar 20, 2009 at 12:22 PM, Gupta, Ramesh <grgupta@ti.com> wrote: > Felipe, > > >> -----Original Message----- >> From: Felipe Contreras [mailto:felipe.contreras@gmail.com] >> Sent: Friday, March 20, 2009 3:48 PM >> To: Hiroshi DOYU >> Cc: Gupta, Ramesh; Kanigeri, Hari; Guzman Lugo, Fernando; >> linux-omap@vger.kernel.org; ameya.palande@nokia.com >> Subject: Re: [PATCH B 3/3] tidspbridge: decreate timeout to a >> saner value >> >> On Fri, Mar 20, 2009 at 9:36 AM, Hiroshi DOYU >> <Hiroshi.DOYU@nokia.com> wrote: >> > From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com> >> > Migrating to the existing mailbox driver("plat-omap/mailbox.c") is >> > also another option to support multiple OMAP architectures >> at the same >> > time. That mailbox driver has already had a message queuing feature >> > inside and it's buildable on OMAP3. >> >> Yeap, I have it in my to-do list, so if somebody doesn't beat >> me to it I play with that. > > We already started looking into this :), made some progress, > will provide more updates in a day or two. Sweet! :)
* Felipe Contreras <felipe.contreras@gmail.com> [090320 03:37]: > On Fri, Mar 20, 2009 at 12:22 PM, Gupta, Ramesh <grgupta@ti.com> wrote: > > Felipe, > > > > > >> -----Original Message----- > >> From: Felipe Contreras [mailto:felipe.contreras@gmail.com] > >> Sent: Friday, March 20, 2009 3:48 PM > >> To: Hiroshi DOYU > >> Cc: Gupta, Ramesh; Kanigeri, Hari; Guzman Lugo, Fernando; > >> linux-omap@vger.kernel.org; ameya.palande@nokia.com > >> Subject: Re: [PATCH B 3/3] tidspbridge: decreate timeout to a > >> saner value > >> > >> On Fri, Mar 20, 2009 at 9:36 AM, Hiroshi DOYU > >> <Hiroshi.DOYU@nokia.com> wrote: > >> > From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com> > >> > Migrating to the existing mailbox driver("plat-omap/mailbox.c") is > >> > also another option to support multiple OMAP architectures > >> at the same > >> > time. That mailbox driver has already had a message queuing feature > >> > inside and it's buildable on OMAP3. > >> > >> Yeap, I have it in my to-do list, so if somebody doesn't beat > >> me to it I play with that. > > > > We already started looking into this :), made some progress, > > will provide more updates in a day or two. > > Sweet! :) Good to hear. For the common features, we need to have code that can be shared among all the DSP implementations. Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/dsp/bridge/wmd/tiomap_sm.c b/drivers/dsp/bridge/wmd/tiomap_sm.c index 1b31162..535dc13 100644 --- a/drivers/dsp/bridge/wmd/tiomap_sm.c +++ b/drivers/dsp/bridge/wmd/tiomap_sm.c @@ -144,7 +144,7 @@ DSP_STATUS CHNLSM_InterruptDSP2(struct WMD_DEV_CONTEXT *pDevContext, pDevContext->dwBrdState = BRD_RUNNING; } - timeout = jiffies + msecs_to_jiffies(10); + timeout = jiffies + msecs_to_jiffies(1); while (fifo_full((void __iomem *) resources.dwMboxBase, 0)) { if (time_after(jiffies, timeout)) { printk(KERN_ERR "dspbridge: timed out waiting for mailbox\n");