Hi Tomasz, On 05/18/2018 06:14 PM, Tomasz Figa wrote: > On Tue, May 15, 2018 at 5:11 PM Stanimir Varbanov < > stanimir.varbanov@xxxxxxxxxx> wrote: > >> This fixes the suspend function for Venus 3xx versions by >> add a check for WFI (wait for interrupt) bit. This bit >> is on when the ARM9 is idle and entered in low power mode. > >> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@xxxxxxxxxx> >> --- >> drivers/media/platform/qcom/venus/hfi_venus.c | 59 > ++++++++++++++++-------- >> drivers/media/platform/qcom/venus/hfi_venus_io.h | 1 + >> 2 files changed, 41 insertions(+), 19 deletions(-) > >> diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c > b/drivers/media/platform/qcom/venus/hfi_venus.c >> index 53546174aab8..aac351f699a0 100644 >> --- a/drivers/media/platform/qcom/venus/hfi_venus.c >> +++ b/drivers/media/platform/qcom/venus/hfi_venus.c >> @@ -1447,7 +1447,7 @@ static int venus_suspend_3xx(struct venus_core > *core) >> { >> struct venus_hfi_device *hdev = to_hfi_priv(core); >> struct device *dev = core->dev; >> - u32 ctrl_status, wfi_status; >> + u32 ctrl_status, cpu_status; >> int ret; >> int cnt = 100; > >> @@ -1463,29 +1463,50 @@ static int venus_suspend_3xx(struct venus_core > *core) >> return -EINVAL; >> } > >> - ctrl_status = venus_readl(hdev, CPU_CS_SCIACMDARG0); >> - if (!(ctrl_status & CPU_CS_SCIACMDARG0_PC_READY)) { >> - wfi_status = venus_readl(hdev, WRAPPER_CPU_STATUS); >> + /* >> + * Power collapse sequence for Venus 3xx and 4xx versions: >> + * 1. Check for ARM9 and video core to be idle by checking WFI bit >> + * (bit 0) in CPU status register and by checking Idle (bit > 30) in >> + * Control status register for video core. >> + * 2. Send a command to prepare for power collapse. >> + * 3. Check for WFI and PC_READY bits. >> + */ >> + >> + while (--cnt) { >> + cpu_status = venus_readl(hdev, WRAPPER_CPU_STATUS); >> ctrl_status = venus_readl(hdev, CPU_CS_SCIACMDARG0); > >> - ret = venus_prepare_power_collapse(hdev, false); >> - if (ret) { >> - dev_err(dev, "prepare for power collapse fail > (%d)\n", >> - ret); >> - return ret; >> - } >> + if (cpu_status & WRAPPER_CPU_STATUS_WFI && >> + ctrl_status & CPU_CS_SCIACMDARG0_INIT_IDLE_MSG_MASK) >> + break; > >> - cnt = 100; >> - while (cnt--) { >> - wfi_status = venus_readl(hdev, > WRAPPER_CPU_STATUS); >> - ctrl_status = venus_readl(hdev, > CPU_CS_SCIACMDARG0); >> - if (ctrl_status & CPU_CS_SCIACMDARG0_PC_READY && >> - wfi_status & BIT(0)) >> - break; >> - usleep_range(1000, 1500); >> - } >> + usleep_range(1000, 1500); >> } > > To avoid opencoding the polling, I'd suggest doing a readx_poll_timeout() > trick: I like the idea, will try to rework that and use readx_poll_timeout. <snip> -- regards, Stan -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html