On 1/27/2010 6:46 PM, Menon, Nishanth wrote:
Omar Ramirez Luna had written, on 01/26/2010 04:23 PM, the following:
Hi,
On 1/23/2010 4:19 AM, Felipe Contreras wrote:
From: Jouni Hogander<jouni.hogander@xxxxxxxxx>
Signed-off-by: Jouni Hogander<jouni.hogander@xxxxxxxxx>
Signed-off-by: Hiroshi DOYU<Hiroshi.DOYU@xxxxxxxxx>
This patch is missing to export enable_off_mode otherwise it will fail
while linking.
FTR, I don't like to be exporting symbols of other subsystems, guess the
same applies to:
http://marc.info/?l=linux-omap&m=126450677527042&w=2
looking at the patch itself few questions arise:
---
drivers/dsp/bridge/rmgr/drv_interface.c | 5 -----
drivers/dsp/bridge/wmd/tiomap3430_pwr.c | 9 ++++-----
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/dsp/bridge/rmgr/drv_interface.c b/drivers/dsp/bridge/rmgr/drv_interface.c
index a02a32a..b20f77e 100644
--- a/drivers/dsp/bridge/rmgr/drv_interface.c
+++ b/drivers/dsp/bridge/rmgr/drv_interface.c
@@ -78,8 +78,6 @@ s32 dsp_debug;
struct platform_device *omap_dspbridge_dev;
-/* This is a test variable used by Bridge to test different sleep states */
-s32 dsp_test_sleepstate;
struct bridge_dev {
struct cdev cdev;
};
@@ -129,9 +127,6 @@ module_param(dsp_debug, int, 0);
MODULE_PARM_DESC(dsp_debug, "Wait after loading DSP image. default = false");
#endif
-module_param(dsp_test_sleepstate, int, 0);
-MODULE_PARM_DESC(dsp_test_sleepstate, "DSP Sleep state = 0");
-
module_param(base_img, charp, 0);
MODULE_PARM_DESC(base_img, "DSP base image, default = NULL");
diff --git a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
index 084f406..c0e0994 100644
--- a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
+++ b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
@@ -54,10 +54,9 @@
#include<mach-omap2/cm-regbits-34xx.h>
#ifdef CONFIG_PM
-extern s32 dsp_test_sleepstate;
+extern unsigned short enable_off_mode;
#endif
extern struct MAILBOX_CONTEXT mboxsetting;
-
/*
* ======== handle_constraints_set ========
* Sets new DSP constraint
@@ -196,7 +195,7 @@ DSP_STATUS SleepDSP(struct WMD_DEV_CONTEXT *pDevContext, IN u32 dwCmd,
switch (pDevContext->dwBrdState) {
case BRD_RUNNING:
status = HW_MBOX_saveSettings(resources.dwMboxBase);
- if (dsp_test_sleepstate == HW_PWR_STATE_OFF) {
+ if (enable_off_mode) {
CHNLSM_InterruptDSP2(pDevContext,
MBX_PM_DSPHIBERNATE);
DBG_Trace(DBG_LEVEL7,
@@ -211,7 +210,7 @@ DSP_STATUS SleepDSP(struct WMD_DEV_CONTEXT *pDevContext, IN u32 dwCmd,
break;
case BRD_RETENTION:
status = HW_MBOX_saveSettings(resources.dwMboxBase);
- if (dsp_test_sleepstate == HW_PWR_STATE_OFF) {
+ if (enable_off_mode) {
CHNLSM_InterruptDSP2(pDevContext,
MBX_PM_DSPHIBERNATE);
targetPwrState = HW_PWR_STATE_OFF;
@@ -261,7 +260,7 @@ DSP_STATUS SleepDSP(struct WMD_DEV_CONTEXT *pDevContext, IN u32 dwCmd,
pwrState);
/* Update the Bridger Driver state */
- if (dsp_test_sleepstate == HW_PWR_STATE_OFF)
+ if (enable_off_mode)
pDevContext->dwBrdState = BRD_HIBERNATION;
else
pDevContext->dwBrdState = BRD_RETENTION;
IMHO it changes behavior:
before:
IF dsp_test_sleepstate was set to HW_PWR_STATE_OFF DSP was told to go to
hibernate every time SleepDSP is called else the mbox event
default behavior is to ask BIOS for retention.
dsp_test_sleepstate is initialized always to 0
with the patch:
If sysfs entry(meant for MPU) has enabled off mode, THEN DSP will be
send to off mode every time we get message to SleepDSP. In other words
MPU and DSP retention/OFF are tied together.
(-->) means mbx bridge to dsp
(...) means do not disturb dsp while sleeping
Suspend coming for this scenarios:
if DSP = ON, enable_off_mode = 1 --> OFF (HIB)
if DSP = ON, enable_off_mode = 0 --> RET
if DSP = OFF, enable_off_mode = 1 ...
if DSP = OFF, enable_off_mode = 0 ...
The questions then are:
a) why was this done so?
always target the next lower state (according to the specified), but DO
NOT disturb the dsp if it already went to sleep
b) is the intention of test_sleepstate param meant only as a test?
should be clear enough:
/* This is a test variable used by Bridge to test different sleep states */
c) was it intended that SleepDSP should put IVA to sleep no matter what
MPU decisions are?
if the DSP is already OFF and MPU wants to be in RET why MPU can be
smart enough to tell ok this guy went down, print a message like "this
might be wrong" but it is acceptable.
the other tendency claims that the dsp should be woken and then told to
go to next-power-state (if OFF not enabled).
bottom line is that we listen to what mpu wants if the dsp is awake but
ignore if the dsp has gone to off.
- omar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
