diff mbox

dspbridge: wait less and check the mailbox more

Message ID 1235171057-7859-1-git-send-email-felipe.contreras@gmail.com (mailing list archive)
State Awaiting Upstream
Headers show

Commit Message

Felipe Contreras Feb. 20, 2009, 11:04 p.m. UTC 
From: Felipe Contreras <felipe.contrers@nokia.com>

Profiling showed the __delay function is called a lot; checking the mbox
more often seems to decrease the usage by 2/3 according to OProfile.

The changes are based on 'arch/arm/plat-omap/mailbox.c'. Tested on OMAP3430.

Also, remove HW_MBOX_IsFull since it's not used by anybody.

Signed-off-by: Felipe Contreras <felipe.contreras@nokia.com>
---
 drivers/dsp/bridge/hw/hw_mbox.c    |   25 -------------------------
 drivers/dsp/bridge/hw/hw_mbox.h    |   35 -----------------------------------
 drivers/dsp/bridge/wmd/tiomap_sm.c |   17 ++++++++++-------
 3 files changed, 10 insertions(+), 67 deletions(-)

Comments

Felipe Contreras Feb. 22, 2009, 6:46 a.m. UTC | #1 
On Sun, Feb 22, 2009 at 7:21 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> Hi Felipe,
>
> From: ext Felipe Contreras <felipe.contreras@gmail.com>
> Subject: [PATCH] dspbridge: wait less and check the mailbox more
> Date: Sat, 21 Feb 2009 00:04:17 +0100
>
>> From: Felipe Contreras <felipe.contrers@nokia.com>
>>
>> Profiling showed the __delay function is called a lot; checking the mbox
>> more often seems to decrease the usage by 2/3 according to OProfile.
>
> Good finding.
>
>> The changes are based on 'arch/arm/plat-omap/mailbox.c'. Tested on OMAP3430.
>>
>> Also, remove HW_MBOX_IsFull since it's not used by anybody.
>
> Woundn't it be better to split this one into some logical parts,
> tunning up part(decreasing delay) and cleanup part(the others)?

I can do that, but my objective is to decrease CPU usage... increasing
the frequency of mbox checking means more calls to HW_MBOX_IsFull. So
the 'cleanup' part must be applied too for performance reasons.

> Home brewed MACROs themselves like UTIL_Wait() are basically evil from
> kernel source readibility perspective, but keeping consistency within
> specific module may be more important and this kind of cleaning up
> would be better to be done at onece over a module, in separate
> patches.

I can do that too.

Note that the author is wrong: contrers -> contreras
diff mbox

Patch

diff --git a/drivers/dsp/bridge/hw/hw_mbox.c b/drivers/dsp/bridge/hw/hw_mbox.c
index 2c14ade..31b890a 100644
--- a/drivers/dsp/bridge/hw/hw_mbox.c
+++ b/drivers/dsp/bridge/hw/hw_mbox.c
@@ -104,31 +104,6 @@  HW_STATUS HW_MBOX_MsgWrite(const u32 baseAddress, const HW_MBOX_Id_t mailBoxId,
 	return status;
 }
 
-/* Reads the full status register for mailbox. */
-HW_STATUS HW_MBOX_IsFull(const u32 baseAddress, const HW_MBOX_Id_t mailBoxId,
-			u32 *const pIsFull)
-{
-	HW_STATUS status = RET_OK;
-	u32 fullStatus;
-
-	/* Check input parameters */
-	CHECK_INPUT_PARAM(baseAddress, 0, RET_BAD_NULL_PARAM, RES_MBOX_BASE +
-			RES_INVALID_INPUT_PARAM);
-	CHECK_INPUT_PARAM(pIsFull,  NULL, RET_BAD_NULL_PARAM, RES_MBOX_BASE +
-			RES_INVALID_INPUT_PARAM);
-	CHECK_INPUT_RANGE_MIN0(mailBoxId, HW_MBOX_ID_MAX, RET_INVALID_ID,
-			RES_MBOX_BASE + RES_INVALID_INPUT_PARAM);
-
-	/* read the is full status parameter for Mailbox */
-	fullStatus = MLBMAILBOX_FIFOSTATUS___0_15FifoFullMBmRead32(baseAddress,
-							(u32)mailBoxId);
-
-	/* fill in return parameter */
-	*pIsFull = (fullStatus & 0xFF);
-
-	return status;
-}
-
 /* Gets number of messages in a specified mailbox. */
 HW_STATUS HW_MBOX_NumMsgGet(const u32 baseAddress, const HW_MBOX_Id_t mailBoxId,
 				u32 *const pNumMsg)
diff --git a/drivers/dsp/bridge/hw/hw_mbox.h b/drivers/dsp/bridge/hw/hw_mbox.h
index 225fb40..d2981d3 100644
--- a/drivers/dsp/bridge/hw/hw_mbox.h
+++ b/drivers/dsp/bridge/hw/hw_mbox.h
@@ -130,41 +130,6 @@  extern HW_STATUS HW_MBOX_MsgWrite(
 		  );
 
 /*
-* FUNCTION      : HW_MBOX_IsFull
-*
-* INPUTS:
-*
-*   Identifier  : baseAddress
-*   Type	: const u32
-*   Description : Base Address of instance of Mailbox module
-*
-*   Identifier  : mailBoxId
-*   Type	: const HW_MBOX_Id_t
-*   Description : Mail Box Sub module Id to check
-*
-* OUTPUTS:
-*
-*   Identifier  : pIsFull
-*   Type	: u32 *const
-*   Description : false means mail box not Full
-*		 true means mailbox full.
-*
-* RETURNS:
-*
-*   Type	: ReturnCode_t
-*   Description : RET_OK	      No errors occured
-*		 RET_BAD_NULL_PARAM  Address/pointer Paramater was set to 0/NULL
-*		 RET_INVALID_ID      Invalid Id used
-*
-* PURPOSE:      : this function reads the full status register for mailbox.
-*/
-extern HW_STATUS HW_MBOX_IsFull(
-		      const u32	 baseAddress,
-		      const HW_MBOX_Id_t   mailBoxId,
-		      u32 *const	pIsFull
-		  );
-
-/*
 * FUNCTION      : HW_MBOX_NumMsgGet
 *
 * INPUTS:
diff --git a/drivers/dsp/bridge/wmd/tiomap_sm.c b/drivers/dsp/bridge/wmd/tiomap_sm.c
index 9bc5b54..0653f40 100644
--- a/drivers/dsp/bridge/wmd/tiomap_sm.c
+++ b/drivers/dsp/bridge/wmd/tiomap_sm.c
@@ -156,6 +156,13 @@  DSP_STATUS CHNLSM_DisableInterrupt(struct WMD_DEV_CONTEXT *hDevContext)
 	return status;
 }
 
+#define MAILBOX_FIFOSTATUS(m) (0x80 + 4 * (m))
+
+static inline unsigned int fifo_full(void __iomem *mbox_base, int mbox_id)
+{
+	return __raw_readl(mbox_base + MAILBOX_FIFOSTATUS(mbox_id)) & 0x1;
+}
+
 /*
  *  ======== CHNLSM_InterruptDSP ========
  *      Send an interrupt to the DSP processor(s).
@@ -171,9 +178,8 @@  DSP_STATUS CHNLSM_InterruptDSP(struct WMD_DEV_CONTEXT *hDevContext)
 	u32 opplevel = 0;
 #endif
 	HW_STATUS hwStatus;
-	u32 mbxFull;
 	struct CFG_HOSTRES resources;
-	u16 cnt = 10;
+	u16 cnt = 1000;
 	u32 temp;
 	/* We are waiting indefinitely here. This needs to be fixed in the
 	 * second phase */
@@ -214,12 +220,9 @@  DSP_STATUS CHNLSM_InterruptDSP(struct WMD_DEV_CONTEXT *hDevContext)
 		pDevContext->dwBrdState = BRD_RUNNING;
 	}
 	while (--cnt) {
-		hwStatus = HW_MBOX_IsFull(resources.dwMboxBase,
-					   MBOX_ARM2DSP, &mbxFull);
-		if (mbxFull)
-			UTIL_Wait(1000);	/* wait for 1 ms)      */
-		else
+		if (!fifo_full((void __iomem *) resources.dwMboxBase, 0))
 			break;
+		udelay(1);
 	}
 	if (!cnt) {
 		DBG_Trace(DBG_LEVEL7, "Timed out waiting for DSP mailbox \n");