* [PATCH B 0/3] tidspbridge: don't flood the mailbox @ 2009-03-18 1:26 Felipe Contreras 2009-03-18 1:26 ` [PATCH B 1/3] tidspbridge: don't flood the mailbox on MemUnMap Felipe Contreras 0 siblings, 1 reply; 21+ messages in thread From: Felipe Contreras @ 2009-03-18 1:26 UTC (permalink / raw) To: linux-omap Cc: Hari Kanigeri, Hiroshi DOYU, Ameya Palande, Fernando Guzman Lugo, Felipe Contreras This patch series is meant to be applied on top of the previous one (A) and fixes a very nasty problem that causes interrupts to be constantly dropped since the mailbox is full most of the time. The details are explained in patch 0001. Flooding the mailbox causes reliability issues (messages lost) and performance issues (busy looping waiting for a slot in the mailbox). Felipe Contreras (3): tidspbridge: don't flood the mailbox on MemUnMap tidspbridge: cleanup and remove HW_MMU_TLBFlushAll tidspbridge: decreate timeout to a saner value drivers/dsp/bridge/hw/MMUAccInt.h | 3 -- drivers/dsp/bridge/hw/MMURegAcM.h | 14 ---------- drivers/dsp/bridge/hw/hw_mmu.c | 9 ------ drivers/dsp/bridge/hw/hw_mmu.h | 2 - drivers/dsp/bridge/wmd/tiomap3430.c | 50 +++++++++++++++++++++------------- drivers/dsp/bridge/wmd/tiomap_sm.c | 2 +- 6 files changed, 32 insertions(+), 48 deletions(-) ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH B 1/3] tidspbridge: don't flood the mailbox on MemUnMap 2009-03-18 1:26 [PATCH B 0/3] tidspbridge: don't flood the mailbox Felipe Contreras @ 2009-03-18 1:26 ` Felipe Contreras 2009-03-18 1:26 ` [PATCH B 2/3] tidspbridge: cleanup and remove HW_MMU_TLBFlushAll Felipe Contreras 2009-03-18 13:06 ` [PATCH B 1/3] tidspbridge: don't flood the mailbox on MemUnMap Kanigeri, Hari 0 siblings, 2 replies; 21+ messages in thread From: Felipe Contreras @ 2009-03-18 1:26 UTC (permalink / raw) To: linux-omap Cc: Hari Kanigeri, Hiroshi DOYU, Ameya Palande, Fernando Guzman Lugo, Felipe Contreras From: Felipe Contreras <felipe.contreras@nokia.com> Impact: improves performance and reliability Check the hardware state of the DSP before sending the command to wake it up thus avoiding to flood the mailbox unnecessarily. By doing that the mailbox is free most of the time which dramatically decreases CPU usage since the check for empty slots is done in a busy loop. Also, a free mailbox means less timeouts, so now most messages are posted which improves reliability. MemMap is doing the flush properly, so this patch also refactors the code into a common flush_all function. The problem can be triggered by doing many memmap/unmaps, just like TI's OpenMAX IL implementation. Signed-off-by: Felipe Contreras <felipe.contreras@nokia.com> --- drivers/dsp/bridge/wmd/tiomap3430.c | 42 +++++++++++++++++++--------------- 1 files changed, 23 insertions(+), 19 deletions(-) diff --git a/drivers/dsp/bridge/wmd/tiomap3430.c b/drivers/dsp/bridge/wmd/tiomap3430.c index c0f4ffc..b538ef7 100644 --- a/drivers/dsp/bridge/wmd/tiomap3430.c +++ b/drivers/dsp/bridge/wmd/tiomap3430.c @@ -235,6 +235,25 @@ static struct WMD_DRV_INTERFACE drvInterfaceFxns = { WMD_MSG_SetQueueId, }; +static inline void flush_all(struct WMD_DEV_CONTEXT *pDevContext) +{ + struct CFG_HOSTRES resources; + u32 temp = 0; + + CFG_GetHostResources((struct CFG_DEVNODE *)DRV_GetFirstDevExtension(), &resources); + HW_PWRST_IVA2RegGet(resources.dwPrmBase, &temp); + + if ((temp & HW_PWR_STATE_ON) == HW_PWR_STATE_OFF) { + /* IVA domain is not in ON state*/ + DBG_Trace(DBG_LEVEL7, "temp value is 0x%x\n", temp); + CLK_Enable(SERVICESCLK_iva2_ck); + WakeDSP(pDevContext, NULL); + HW_MMU_TLBFlushAll(pDevContext->dwDSPMmuBase); + CLK_Disable(SERVICESCLK_iva2_ck); + } else + HW_MMU_TLBFlushAll(pDevContext->dwDSPMmuBase); +} + /* * ======== WMD_DRV_Entry ======== * purpose: @@ -1283,17 +1302,14 @@ static DSP_STATUS WMD_BRD_MemMap(struct WMD_DEV_CONTEXT *hDevContext, struct WMD_DEV_CONTEXT *pDevContext = hDevContext; struct HW_MMUMapAttrs_t hwAttrs; u32 numOfActualTabEntries = 0; - u32 temp = 0; - struct CFG_HOSTRES resources; u32 *pPhysAddrPageTbl = NULL; struct vm_area_struct *vma; struct mm_struct *mm = current->mm; + u32 temp = 0; DBG_Trace(DBG_ENTER, "> WMD_BRD_MemMap hDevContext %x, pa %x, va %x, " "size %x, ulMapAttr %x\n", hDevContext, ulMpuAddr, ulVirtAddr, ulNumBytes, ulMapAttr); - status = CFG_GetHostResources( - (struct CFG_DEVNODE *)DRV_GetFirstDevExtension(), &resources); if (ulNumBytes == 0) return DSP_EINVALIDARG; @@ -1421,16 +1437,7 @@ func_cont: * This is called from here instead from PteUpdate to avoid unnecessary * repetition while mapping non-contiguous physical regions of a virtual * region */ - HW_PWRST_IVA2RegGet(resources.dwPrmBase, &temp); - if ((temp & HW_PWR_STATE_ON) == HW_PWR_STATE_OFF) { - /* IVA domain is not in ON state*/ - DBG_Trace(DBG_LEVEL7, "temp value is 0x%x\n", temp); - CLK_Enable(SERVICESCLK_iva2_ck); - WakeDSP(pDevContext, NULL); - HW_MMU_TLBFlushAll(pDevContext->dwDSPMmuBase); - CLK_Disable(SERVICESCLK_iva2_ck); - } else - HW_MMU_TLBFlushAll(pDevContext->dwDSPMmuBase); + flush_all(pDevContext); DBG_Trace(DBG_ENTER, "< WMD_BRD_MemMap status %x\n", status); return status; } @@ -1571,8 +1578,7 @@ static DSP_STATUS WMD_BRD_MemUnMap(struct WMD_DEV_CONTEXT *hDevContext, /* It is better to flush the TLB here, so that any stale old entries * get flushed */ EXIT_LOOP: - CHNLSM_InterruptDSP2(pDevContext, MBX_PM_DSPWAKEUP); - HW_MMU_TLBFlushAll(pDevContext->dwDSPMmuBase); + flush_all(pDevContext); DBG_Trace(DBG_LEVEL1, "WMD_BRD_MemUnMap vaCurr %x, pteAddrL1 %x " "pteAddrL2 %x\n", vaCurr, pteAddrL1, pteAddrL2); DBG_Trace(DBG_ENTER, "< WMD_BRD_MemUnMap status %x remBytes %x, " @@ -2055,9 +2061,7 @@ func_cont: * This is called from here instead from PteUpdate to avoid unnecessary * repetition while mapping non-contiguous physical regions of a virtual * region */ - /* Waking up DSP before calling TLB Flush */ - CHNLSM_InterruptDSP2(pDevContext, MBX_PM_DSPWAKEUP); - HW_MMU_TLBFlushAll(pDevContext->dwDSPMmuBase); + flush_all(pDevContext); DBG_Trace(DBG_LEVEL7, "< WMD_BRD_MemMap at end status %x\n", status); return status; } -- 1.6.2.1.287.g9a8be ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH B 2/3] tidspbridge: cleanup and remove HW_MMU_TLBFlushAll 2009-03-18 1:26 ` [PATCH B 1/3] tidspbridge: don't flood the mailbox on MemUnMap Felipe Contreras @ 2009-03-18 1:26 ` Felipe Contreras 2009-03-18 1:26 ` [PATCH B 3/3] tidspbridge: decreate timeout to a saner value Felipe Contreras 2009-03-19 8:49 ` [PATCH B 2/3] tidspbridge: cleanup and remove HW_MMU_TLBFlushAll Felipe Contreras 2009-03-18 13:06 ` [PATCH B 1/3] tidspbridge: don't flood the mailbox on MemUnMap Kanigeri, Hari 1 sibling, 2 replies; 21+ messages in thread From: Felipe Contreras @ 2009-03-18 1:26 UTC (permalink / raw) To: linux-omap Cc: Hari Kanigeri, Hiroshi DOYU, Ameya Palande, Fernando Guzman Lugo, Felipe Contreras From: Felipe Contreras <felipe.contreras@nokia.com> It doesn't make sense to have layers and layers of constants and defines to turn one bit on. Signed-off-by: Felipe Contreras <felipe.contreras@nokia.com> --- drivers/dsp/bridge/hw/MMUAccInt.h | 3 --- drivers/dsp/bridge/hw/MMURegAcM.h | 14 -------------- drivers/dsp/bridge/hw/hw_mmu.c | 9 --------- drivers/dsp/bridge/hw/hw_mmu.h | 2 -- drivers/dsp/bridge/wmd/tiomap3430.c | 12 ++++++++++-- 5 files changed, 10 insertions(+), 30 deletions(-) diff --git a/drivers/dsp/bridge/hw/MMUAccInt.h b/drivers/dsp/bridge/hw/MMUAccInt.h index 78e1d15..6ca1573 100644 --- a/drivers/dsp/bridge/hw/MMUAccInt.h +++ b/drivers/dsp/bridge/hw/MMUAccInt.h @@ -41,7 +41,6 @@ #define EASIL1_MMUMMU_LD_TLBWriteRegister32 (MMU_BASE_EASIL1 + 214) #define EASIL1_MMUMMU_CAMWriteRegister32 (MMU_BASE_EASIL1 + 226) #define EASIL1_MMUMMU_RAMWriteRegister32 (MMU_BASE_EASIL1 + 268) -#define EASIL1_MMUMMU_GFLUSHGlobalFlushWrite32 (MMU_BASE_EASIL1 + 317) #define EASIL1_MMUMMU_FLUSH_ENTRYWriteRegister32 (MMU_BASE_EASIL1 + 322) /* Register offset address definitions */ @@ -73,7 +72,5 @@ #define MMU_MMU_LOCK_BaseValue_OFFSET 10 #define MMU_MMU_LOCK_CurrentVictim_MASK 0x3f0 #define MMU_MMU_LOCK_CurrentVictim_OFFSET 4 -#define MMU_MMU_GFLUSH_GlobalFlush_MASK 0x1 -#define MMU_MMU_GFLUSH_GlobalFlush_OFFSET 0 #endif /* _MMU_ACC_INT_H */ diff --git a/drivers/dsp/bridge/hw/MMURegAcM.h b/drivers/dsp/bridge/hw/MMURegAcM.h index a130b1a..e46fdcb 100644 --- a/drivers/dsp/bridge/hw/MMURegAcM.h +++ b/drivers/dsp/bridge/hw/MMURegAcM.h @@ -239,20 +239,6 @@ } -#define MMUMMU_GFLUSHGlobalFlushWrite32(baseAddress, value)\ -{\ - const u32 offset = MMU_MMU_GFLUSH_OFFSET;\ - register u32 data = RD_MEM_32_VOLATILE((baseAddress)+offset);\ - register u32 newValue = (value);\ - _DEBUG_LEVEL_1_EASI(EASIL1_MMUMMU_GFLUSHGlobalFlushWrite32);\ - data &= ~(MMU_MMU_GFLUSH_GlobalFlush_MASK);\ - newValue <<= MMU_MMU_GFLUSH_GlobalFlush_OFFSET;\ - newValue &= MMU_MMU_GFLUSH_GlobalFlush_MASK;\ - newValue |= data;\ - WR_MEM_32_VOLATILE(baseAddress+offset, newValue);\ -} - - #define MMUMMU_FLUSH_ENTRYWriteRegister32(baseAddress, value)\ {\ const u32 offset = MMU_MMU_FLUSH_ENTRY_OFFSET;\ diff --git a/drivers/dsp/bridge/hw/hw_mmu.c b/drivers/dsp/bridge/hw/hw_mmu.c index da7e092..ab65de0 100644 --- a/drivers/dsp/bridge/hw/hw_mmu.c +++ b/drivers/dsp/bridge/hw/hw_mmu.c @@ -212,15 +212,6 @@ HW_STATUS HW_MMU_VictimNumSet(const u32 baseAddress, return status; } -HW_STATUS HW_MMU_TLBFlushAll(const u32 baseAddress) -{ - HW_STATUS status = RET_OK; - - MMUMMU_GFLUSHGlobalFlushWrite32(baseAddress, HW_SET); - - return status; -} - HW_STATUS HW_MMU_EventAck(const u32 baseAddress, u32 irqMask) { HW_STATUS status = RET_OK; diff --git a/drivers/dsp/bridge/hw/hw_mmu.h b/drivers/dsp/bridge/hw/hw_mmu.h index 924f32b..4783276 100644 --- a/drivers/dsp/bridge/hw/hw_mmu.h +++ b/drivers/dsp/bridge/hw/hw_mmu.h @@ -91,8 +91,6 @@ extern HW_STATUS HW_MMU_TLBFlush(const u32 baseAddress, u32 virtualAddr, u32 pageSize); -extern HW_STATUS HW_MMU_TLBFlushAll(const u32 baseAddress); - extern HW_STATUS HW_MMU_TLBAdd(const u32 baseAddress, u32 physicalAddr, u32 virtualAddr, diff --git a/drivers/dsp/bridge/wmd/tiomap3430.c b/drivers/dsp/bridge/wmd/tiomap3430.c index b538ef7..30e0cb3 100644 --- a/drivers/dsp/bridge/wmd/tiomap3430.c +++ b/drivers/dsp/bridge/wmd/tiomap3430.c @@ -88,6 +88,9 @@ #define MMU_LARGE_PAGE_MASK 0xFFFF0000 #define MMU_SMALL_PAGE_MASK 0xFFFFF000 #define PAGES_II_LVL_TABLE 512 + +#define MMU_GFLUSH 0x60 + /* Forward Declarations: */ static DSP_STATUS WMD_BRD_Monitor(struct WMD_DEV_CONTEXT *pDevContext); static DSP_STATUS WMD_BRD_Read(struct WMD_DEV_CONTEXT *pDevContext, @@ -235,6 +238,11 @@ static struct WMD_DRV_INTERFACE drvInterfaceFxns = { WMD_MSG_SetQueueId, }; +static inline void tlb_flush_all(const u32 base) +{ + __raw_writeb(__raw_readb(base + MMU_GFLUSH) | 1, base + MMU_GFLUSH); +} + static inline void flush_all(struct WMD_DEV_CONTEXT *pDevContext) { struct CFG_HOSTRES resources; @@ -248,10 +256,10 @@ static inline void flush_all(struct WMD_DEV_CONTEXT *pDevContext) DBG_Trace(DBG_LEVEL7, "temp value is 0x%x\n", temp); CLK_Enable(SERVICESCLK_iva2_ck); WakeDSP(pDevContext, NULL); - HW_MMU_TLBFlushAll(pDevContext->dwDSPMmuBase); + tlb_flush_all(pDevContext->dwDSPMmuBase); CLK_Disable(SERVICESCLK_iva2_ck); } else - HW_MMU_TLBFlushAll(pDevContext->dwDSPMmuBase); + tlb_flush_all(pDevContext->dwDSPMmuBase); } /* -- 1.6.2.1.287.g9a8be ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH B 3/3] tidspbridge: decreate timeout to a saner value 2009-03-18 1:26 ` [PATCH B 2/3] tidspbridge: cleanup and remove HW_MMU_TLBFlushAll Felipe Contreras @ 2009-03-18 1:26 ` Felipe Contreras 2009-03-19 20:42 ` Guzman Lugo, Fernando 2009-03-19 8:49 ` [PATCH B 2/3] tidspbridge: cleanup and remove HW_MMU_TLBFlushAll Felipe Contreras 1 sibling, 1 reply; 21+ messages in thread From: Felipe Contreras @ 2009-03-18 1:26 UTC (permalink / raw) To: linux-omap Cc: Hari Kanigeri, Hiroshi DOYU, Ameya Palande, Fernando Guzman Lugo, Felipe Contreras From: Felipe Contreras <felipe.contreras@nokia.com> Signed-off-by: Felipe Contreras <felipe.contreras@nokia.com> --- drivers/dsp/bridge/wmd/tiomap_sm.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) 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"); -- 1.6.2.1.287.g9a8be ^ permalink raw reply related [flat|nested] 21+ messages in thread
* RE: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value 2009-03-18 1:26 ` [PATCH B 3/3] tidspbridge: decreate timeout to a saner value Felipe Contreras @ 2009-03-19 20:42 ` Guzman Lugo, Fernando 2009-03-19 21:04 ` Felipe Contreras 0 siblings, 1 reply; 21+ messages in thread From: Guzman Lugo, Fernando @ 2009-03-19 20:42 UTC (permalink / raw) To: Felipe Contreras, linux-omap Cc: Kanigeri, Hari, Hiroshi DOYU, Ameya Palande, Felipe Contreras 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. Please let me know your comments. Regards, Fernando. -----Original Message----- From: Felipe Contreras [mailto:felipe.contreras@gmail.com] Sent: Tuesday, March 17, 2009 7:27 PM To: linux-omap@vger.kernel.org Cc: Kanigeri, Hari; Hiroshi DOYU; Ameya Palande; Guzman Lugo, Fernando; Felipe Contreras Subject: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value From: Felipe Contreras <felipe.contreras@nokia.com> Signed-off-by: Felipe Contreras <felipe.contreras@nokia.com> --- drivers/dsp/bridge/wmd/tiomap_sm.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) 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"); -- 1.6.2.1.287.g9a8be ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value 2009-03-19 20:42 ` Guzman Lugo, Fernando @ 2009-03-19 21:04 ` Felipe Contreras 2009-03-19 21:19 ` Guzman Lugo, Fernando 0 siblings, 1 reply; 21+ messages in thread From: Felipe Contreras @ 2009-03-19 21:04 UTC (permalink / raw) To: Guzman Lugo, Fernando Cc: linux-omap, Kanigeri, Hari, Hiroshi DOYU, Ameya Palande, Felipe Contreras 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. -- Felipe Contreras -- 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value 2009-03-19 21:04 ` Felipe Contreras @ 2009-03-19 21:19 ` Guzman Lugo, Fernando 2009-03-19 21:35 ` Felipe Contreras 0 siblings, 1 reply; 21+ messages in thread From: Guzman Lugo, Fernando @ 2009-03-19 21:19 UTC (permalink / raw) To: Felipe Contreras Cc: linux-omap, Kanigeri, Hari, Hiroshi DOYU, Ameya Palande, Felipe Contreras 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. -- Felipe Contreras -- 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value 2009-03-19 21:19 ` Guzman Lugo, Fernando @ 2009-03-19 21:35 ` Felipe Contreras 2009-03-20 0:00 ` Guzman Lugo, Fernando 0 siblings, 1 reply; 21+ messages in thread From: Felipe Contreras @ 2009-03-19 21:35 UTC (permalink / raw) To: Guzman Lugo, Fernando Cc: linux-omap, Kanigeri, Hari, Hiroshi DOYU, Ameya Palande, Felipe Contreras 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. -- Felipe Contreras ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value 2009-03-19 21:35 ` Felipe Contreras @ 2009-03-20 0:00 ` Guzman Lugo, Fernando 2009-03-20 0:06 ` Felipe Contreras 0 siblings, 1 reply; 21+ messages in thread From: Guzman Lugo, Fernando @ 2009-03-20 0:00 UTC (permalink / raw) To: Felipe Contreras Cc: linux-omap, Kanigeri, Hari, Hiroshi DOYU, Ameya Palande, Felipe Contreras 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. -- Felipe Contreras ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value 2009-03-20 0:00 ` Guzman Lugo, Fernando @ 2009-03-20 0:06 ` Felipe Contreras 2009-03-20 4:54 ` Hiroshi DOYU 0 siblings, 1 reply; 21+ messages in thread From: Felipe Contreras @ 2009-03-20 0:06 UTC (permalink / raw) To: Guzman Lugo, Fernando Cc: linux-omap, Kanigeri, Hari, Hiroshi DOYU, Ameya Palande, Felipe Contreras 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. -- Felipe Contreras -- 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value 2009-03-20 0:06 ` Felipe Contreras @ 2009-03-20 4:54 ` Hiroshi DOYU 2009-03-20 7:36 ` Hiroshi DOYU 2009-03-20 10:15 ` Felipe Contreras 0 siblings, 2 replies; 21+ messages in thread From: Hiroshi DOYU @ 2009-03-20 4:54 UTC (permalink / raw) To: felipe.contreras Cc: x0095840, linux-omap, h-kanigeri2, ameya.palande, felipe.contreras 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. > > 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. > > -- > Felipe Contreras -- 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value 2009-03-20 4:54 ` Hiroshi DOYU @ 2009-03-20 7:36 ` Hiroshi DOYU 2009-03-20 10:17 ` Felipe Contreras 2009-03-20 10:15 ` Felipe Contreras 1 sibling, 1 reply; 21+ messages in thread From: Hiroshi DOYU @ 2009-03-20 7:36 UTC (permalink / raw) To: grgupta, h-kanigeri2, felipe.contreras Cc: x0095840, linux-omap, ameya.palande 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. -- 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value 2009-03-20 7:36 ` Hiroshi DOYU @ 2009-03-20 10:17 ` Felipe Contreras 2009-03-20 10:22 ` Gupta, Ramesh 0 siblings, 1 reply; 21+ messages in thread From: Felipe Contreras @ 2009-03-20 10:17 UTC (permalink / raw) To: Hiroshi DOYU; +Cc: grgupta, h-kanigeri2, x0095840, linux-omap, ameya.palande 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 Contreras -- 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value 2009-03-20 10:17 ` Felipe Contreras @ 2009-03-20 10:22 ` Gupta, Ramesh 2009-03-20 10:25 ` Felipe Contreras 0 siblings, 1 reply; 21+ messages in thread From: Gupta, Ramesh @ 2009-03-20 10:22 UTC (permalink / raw) To: Felipe Contreras, Hiroshi DOYU Cc: Kanigeri, Hari, Guzman Lugo, Fernando, linux-omap, ameya.palande, Varide, Nischal 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value 2009-03-20 10:22 ` Gupta, Ramesh @ 2009-03-20 10:25 ` Felipe Contreras 2009-03-20 15:28 ` Tony Lindgren 0 siblings, 1 reply; 21+ messages in thread From: Felipe Contreras @ 2009-03-20 10:25 UTC (permalink / raw) To: Gupta, Ramesh Cc: Hiroshi DOYU, Kanigeri, Hari, Guzman Lugo, Fernando, linux-omap, ameya.palande, Varide, Nischal 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value 2009-03-20 10:25 ` Felipe Contreras @ 2009-03-20 15:28 ` Tony Lindgren 0 siblings, 0 replies; 21+ messages in thread From: Tony Lindgren @ 2009-03-20 15:28 UTC (permalink / raw) To: Felipe Contreras Cc: Gupta, Ramesh, Hiroshi DOYU, Kanigeri, Hari, Guzman Lugo, Fernando, linux-omap, ameya.palande, Varide, Nischal * 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value 2009-03-20 4:54 ` Hiroshi DOYU 2009-03-20 7:36 ` Hiroshi DOYU @ 2009-03-20 10:15 ` Felipe Contreras 1 sibling, 0 replies; 21+ messages in thread From: Felipe Contreras @ 2009-03-20 10:15 UTC (permalink / raw) To: Hiroshi DOYU Cc: x0095840, linux-omap, h-kanigeri2, ameya.palande, felipe.contreras 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. -- Felipe Contreras -- 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH B 2/3] tidspbridge: cleanup and remove HW_MMU_TLBFlushAll 2009-03-18 1:26 ` [PATCH B 2/3] tidspbridge: cleanup and remove HW_MMU_TLBFlushAll Felipe Contreras 2009-03-18 1:26 ` [PATCH B 3/3] tidspbridge: decreate timeout to a saner value Felipe Contreras @ 2009-03-19 8:49 ` Felipe Contreras 1 sibling, 0 replies; 21+ messages in thread From: Felipe Contreras @ 2009-03-19 8:49 UTC (permalink / raw) To: linux-omap Cc: Hari Kanigeri, Hiroshi DOYU, Ameya Palande, Fernando Guzman Lugo, Felipe Contreras On Wed, Mar 18, 2009 at 3:26 AM, Felipe Contreras <felipe.contreras@gmail.com> wrote: > From: Felipe Contreras <felipe.contreras@nokia.com> > > It doesn't make sense to have layers and layers of constants and defines > to turn one bit on. > > Signed-off-by: Felipe Contreras <felipe.contreras@nokia.com> > --- I thought a bit more about this: > +static inline void tlb_flush_all(const u32 base) > +{ > + __raw_writeb(__raw_readb(base + MMU_GFLUSH) | 1, base + MMU_GFLUSH); > +} > + Originally it was the equivalent of __raw_writel(__raw_readl but since we are dealing only with one bit I chose 'b' instead of 'l'. Is that correct? -- Felipe Contreras -- 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH B 1/3] tidspbridge: don't flood the mailbox on MemUnMap 2009-03-18 1:26 ` [PATCH B 1/3] tidspbridge: don't flood the mailbox on MemUnMap Felipe Contreras 2009-03-18 1:26 ` [PATCH B 2/3] tidspbridge: cleanup and remove HW_MMU_TLBFlushAll Felipe Contreras @ 2009-03-18 13:06 ` Kanigeri, Hari 2009-03-18 13:29 ` Felipe Contreras 1 sibling, 1 reply; 21+ messages in thread From: Kanigeri, Hari @ 2009-03-18 13:06 UTC (permalink / raw) To: Felipe Contreras, linux-omap Cc: Hiroshi DOYU, Ameya Palande, Guzman Lugo, Fernando, Felipe Contreras Felipe, > Check the hardware state of the DSP before sending the command to wake > it up thus avoiding to flood the mailbox unnecessarily. > -- So this check was missing in Unmap part. Good catch. > - u32 temp = 0; > - struct CFG_HOSTRES resources; > u32 *pPhysAddrPageTbl = NULL; > struct vm_area_struct *vma; > struct mm_struct *mm = current->mm; > + u32 temp = 0; > -- Is temp variable needed ? Thank you, Best regards, Hari > -----Original Message----- > From: Felipe Contreras [mailto:felipe.contreras@gmail.com] > Sent: Tuesday, March 17, 2009 8:27 PM > To: linux-omap@vger.kernel.org > Cc: Kanigeri, Hari; Hiroshi DOYU; Ameya Palande; Guzman Lugo, Fernando; > Felipe Contreras > Subject: [PATCH B 1/3] tidspbridge: don't flood the mailbox on MemUnMap > > From: Felipe Contreras <felipe.contreras@nokia.com> > > Impact: improves performance and reliability > > Check the hardware state of the DSP before sending the command to wake > it up thus avoiding to flood the mailbox unnecessarily. > > By doing that the mailbox is free most of the time which dramatically > decreases CPU usage since the check for empty slots is done in a busy > loop. > > Also, a free mailbox means less timeouts, so now most messages are > posted which improves reliability. > > MemMap is doing the flush properly, so this patch also refactors the > code into a common flush_all function. > > The problem can be triggered by doing many memmap/unmaps, just like TI's > OpenMAX IL implementation. > > Signed-off-by: Felipe Contreras <felipe.contreras@nokia.com> > --- > drivers/dsp/bridge/wmd/tiomap3430.c | 42 +++++++++++++++++++----------- > ---- > 1 files changed, 23 insertions(+), 19 deletions(-) > > diff --git a/drivers/dsp/bridge/wmd/tiomap3430.c > b/drivers/dsp/bridge/wmd/tiomap3430.c > index c0f4ffc..b538ef7 100644 > --- a/drivers/dsp/bridge/wmd/tiomap3430.c > +++ b/drivers/dsp/bridge/wmd/tiomap3430.c > @@ -235,6 +235,25 @@ static struct WMD_DRV_INTERFACE drvInterfaceFxns = { > WMD_MSG_SetQueueId, > }; > > +static inline void flush_all(struct WMD_DEV_CONTEXT *pDevContext) > +{ > + struct CFG_HOSTRES resources; > + u32 temp = 0; > + > + CFG_GetHostResources((struct CFG_DEVNODE > *)DRV_GetFirstDevExtension(), &resources); > + HW_PWRST_IVA2RegGet(resources.dwPrmBase, &temp); > + > + if ((temp & HW_PWR_STATE_ON) == HW_PWR_STATE_OFF) { > + /* IVA domain is not in ON state*/ > + DBG_Trace(DBG_LEVEL7, "temp value is 0x%x\n", temp); > + CLK_Enable(SERVICESCLK_iva2_ck); > + WakeDSP(pDevContext, NULL); > + HW_MMU_TLBFlushAll(pDevContext->dwDSPMmuBase); > + CLK_Disable(SERVICESCLK_iva2_ck); > + } else > + HW_MMU_TLBFlushAll(pDevContext->dwDSPMmuBase); > +} > + > /* > * ======== WMD_DRV_Entry ======== > * purpose: > @@ -1283,17 +1302,14 @@ static DSP_STATUS WMD_BRD_MemMap(struct > WMD_DEV_CONTEXT *hDevContext, > struct WMD_DEV_CONTEXT *pDevContext = hDevContext; > struct HW_MMUMapAttrs_t hwAttrs; > u32 numOfActualTabEntries = 0; > - u32 temp = 0; > - struct CFG_HOSTRES resources; > u32 *pPhysAddrPageTbl = NULL; > struct vm_area_struct *vma; > struct mm_struct *mm = current->mm; > + u32 temp = 0; > > DBG_Trace(DBG_ENTER, "> WMD_BRD_MemMap hDevContext %x, pa %x, va %x, > " > "size %x, ulMapAttr %x\n", hDevContext, ulMpuAddr, > ulVirtAddr, > ulNumBytes, ulMapAttr); > - status = CFG_GetHostResources( > - (struct CFG_DEVNODE *)DRV_GetFirstDevExtension(), > &resources); > if (ulNumBytes == 0) > return DSP_EINVALIDARG; > > @@ -1421,16 +1437,7 @@ func_cont: > * This is called from here instead from PteUpdate to avoid > unnecessary > * repetition while mapping non-contiguous physical regions of a > virtual > * region */ > - HW_PWRST_IVA2RegGet(resources.dwPrmBase, &temp); > - if ((temp & HW_PWR_STATE_ON) == HW_PWR_STATE_OFF) { > - /* IVA domain is not in ON state*/ > - DBG_Trace(DBG_LEVEL7, "temp value is 0x%x\n", temp); > - CLK_Enable(SERVICESCLK_iva2_ck); > - WakeDSP(pDevContext, NULL); > - HW_MMU_TLBFlushAll(pDevContext->dwDSPMmuBase); > - CLK_Disable(SERVICESCLK_iva2_ck); > - } else > - HW_MMU_TLBFlushAll(pDevContext->dwDSPMmuBase); > + flush_all(pDevContext); > DBG_Trace(DBG_ENTER, "< WMD_BRD_MemMap status %x\n", status); > return status; > } > @@ -1571,8 +1578,7 @@ static DSP_STATUS WMD_BRD_MemUnMap(struct > WMD_DEV_CONTEXT *hDevContext, > /* It is better to flush the TLB here, so that any stale old > entries > * get flushed */ > EXIT_LOOP: > - CHNLSM_InterruptDSP2(pDevContext, MBX_PM_DSPWAKEUP); > - HW_MMU_TLBFlushAll(pDevContext->dwDSPMmuBase); > + flush_all(pDevContext); > DBG_Trace(DBG_LEVEL1, "WMD_BRD_MemUnMap vaCurr %x, pteAddrL1 %x " > "pteAddrL2 %x\n", vaCurr, pteAddrL1, pteAddrL2); > DBG_Trace(DBG_ENTER, "< WMD_BRD_MemUnMap status %x remBytes %x, " > @@ -2055,9 +2061,7 @@ func_cont: > * This is called from here instead from PteUpdate to avoid > unnecessary > * repetition while mapping non-contiguous physical regions of a > virtual > * region */ > - /* Waking up DSP before calling TLB Flush */ > - CHNLSM_InterruptDSP2(pDevContext, MBX_PM_DSPWAKEUP); > - HW_MMU_TLBFlushAll(pDevContext->dwDSPMmuBase); > + flush_all(pDevContext); > DBG_Trace(DBG_LEVEL7, "< WMD_BRD_MemMap at end status %x\n", > status); > return status; > } > -- > 1.6.2.1.287.g9a8be > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH B 1/3] tidspbridge: don't flood the mailbox on MemUnMap 2009-03-18 13:06 ` [PATCH B 1/3] tidspbridge: don't flood the mailbox on MemUnMap Kanigeri, Hari @ 2009-03-18 13:29 ` Felipe Contreras 2009-03-18 13:49 ` Kanigeri, Hari 0 siblings, 1 reply; 21+ messages in thread From: Felipe Contreras @ 2009-03-18 13:29 UTC (permalink / raw) To: Kanigeri, Hari Cc: linux-omap, Hiroshi DOYU, Ameya Palande, Guzman Lugo, Fernando, Felipe Contreras On Wed, Mar 18, 2009 at 3:06 PM, Kanigeri, Hari <h-kanigeri2@ti.com> wrote: > Felipe, > >> Check the hardware state of the DSP before sending the command to wake >> it up thus avoiding to flood the mailbox unnecessarily. >> > -- So this check was missing in Unmap part. Good catch. > > >> - u32 temp = 0; >> - struct CFG_HOSTRES resources; >> u32 *pPhysAddrPageTbl = NULL; >> struct vm_area_struct *vma; >> struct mm_struct *mm = current->mm; >> + u32 temp = 0; >> > > -- Is temp variable needed ? You tell me :) I just moved code around. I don't truly understand what it's doing. Maybe the read is needed for some kind of sync? I've removed the tlb flush all completely and I didn't notice any problems, is it really needed? -- Felipe Contreras -- 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH B 1/3] tidspbridge: don't flood the mailbox on MemUnMap 2009-03-18 13:29 ` Felipe Contreras @ 2009-03-18 13:49 ` Kanigeri, Hari 0 siblings, 0 replies; 21+ messages in thread From: Kanigeri, Hari @ 2009-03-18 13:49 UTC (permalink / raw) To: Felipe Contreras Cc: linux-omap, Hiroshi DOYU, Ameya Palande, Guzman Lugo, Fernando, Felipe Contreras > You tell me :) > > I just moved code around. I don't truly understand what it's doing. > Maybe the read is needed for some kind of sync? > > I've removed the tlb flush all completely and I didn't notice any > problems, is it really needed? > -- Ignore my previous comment. I think temp is used for other purpose in the same function. I thought temp was used by only the part of the code section that you stripped out. Thank you, Best regards, Hari > -----Original Message----- > From: Felipe Contreras [mailto:felipe.contreras@gmail.com] > Sent: Wednesday, March 18, 2009 8:30 AM > To: Kanigeri, Hari > Cc: linux-omap@vger.kernel.org; Hiroshi DOYU; Ameya Palande; Guzman Lugo, > Fernando; Felipe Contreras > Subject: Re: [PATCH B 1/3] tidspbridge: don't flood the mailbox on > MemUnMap > > On Wed, Mar 18, 2009 at 3:06 PM, Kanigeri, Hari <h-kanigeri2@ti.com> > wrote: > > Felipe, > > > >> Check the hardware state of the DSP before sending the command to wake > >> it up thus avoiding to flood the mailbox unnecessarily. > >> > > -- So this check was missing in Unmap part. Good catch. > > > > > >> - u32 temp = 0; > >> - struct CFG_HOSTRES resources; > >> u32 *pPhysAddrPageTbl = NULL; > >> struct vm_area_struct *vma; > >> struct mm_struct *mm = current->mm; > >> + u32 temp = 0; > >> > > > > -- Is temp variable needed ? > > You tell me :) > > I just moved code around. I don't truly understand what it's doing. > Maybe the read is needed for some kind of sync? > > I've removed the tlb flush all completely and I didn't notice any > problems, is it really needed? > > -- > Felipe Contreras -- 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2009-03-20 15:28 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-03-18 1:26 [PATCH B 0/3] tidspbridge: don't flood the mailbox Felipe Contreras 2009-03-18 1:26 ` [PATCH B 1/3] tidspbridge: don't flood the mailbox on MemUnMap Felipe Contreras 2009-03-18 1:26 ` [PATCH B 2/3] tidspbridge: cleanup and remove HW_MMU_TLBFlushAll Felipe Contreras 2009-03-18 1:26 ` [PATCH B 3/3] tidspbridge: decreate timeout to a saner value Felipe Contreras 2009-03-19 20:42 ` Guzman Lugo, Fernando 2009-03-19 21:04 ` Felipe Contreras 2009-03-19 21:19 ` Guzman Lugo, Fernando 2009-03-19 21:35 ` Felipe Contreras 2009-03-20 0:00 ` Guzman Lugo, Fernando 2009-03-20 0:06 ` Felipe Contreras 2009-03-20 4:54 ` Hiroshi DOYU 2009-03-20 7:36 ` Hiroshi DOYU 2009-03-20 10:17 ` Felipe Contreras 2009-03-20 10:22 ` Gupta, Ramesh 2009-03-20 10:25 ` Felipe Contreras 2009-03-20 15:28 ` Tony Lindgren 2009-03-20 10:15 ` Felipe Contreras 2009-03-19 8:49 ` [PATCH B 2/3] tidspbridge: cleanup and remove HW_MMU_TLBFlushAll Felipe Contreras 2009-03-18 13:06 ` [PATCH B 1/3] tidspbridge: don't flood the mailbox on MemUnMap Kanigeri, Hari 2009-03-18 13:29 ` Felipe Contreras 2009-03-18 13:49 ` Kanigeri, Hari
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).