[PATCH B 0/3] tidspbridge: don't flood the mailbox

[PATCH B 0/3] tidspbridge: don't flood the mailbox

[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(-)

[PATCH B 1/3] tidspbridge: don't flood the mailbox on MemUnMap

[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

[PATCH B 2/3] tidspbridge: cleanup and remove HW_MMU_TLBFlushAll

[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

[PATCH B 3/3] tidspbridge: decreate timeout to a saner value

[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

RE: [PATCH B 1/3] tidspbridge: don't flood the mailbox on MemUnMap

[Kanigeri, Hari]

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
> 

Re: [PATCH B 1/3] tidspbridge: don't flood the mailbox on MemUnMap

[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

RE: [PATCH B 1/3] tidspbridge: don't flood the mailbox on MemUnMap

[Kanigeri, Hari]

> 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

Re: [PATCH B 2/3] tidspbridge: cleanup and remove HW_MMU_TLBFlushAll

[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

RE: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value

[Guzman Lugo, Fernando]

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

Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value

[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

RE: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value

[Guzman Lugo, Fernando]

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

Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value

[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

RE: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value

[Guzman Lugo, Fernando]

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

Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value

[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

Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value

[Hiroshi DOYU]

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

Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value

[Hiroshi DOYU]

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

Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value

[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

Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value

[Felipe Contreras]

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

RE: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value

[Gupta, Ramesh]

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

Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value

[Felipe Contreras]

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

Re: [PATCH B 3/3] tidspbridge: decreate timeout to a saner value

[Tony Lindgren]

* 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