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: static bool venus_arm9_and_video_core_idle(struct venus_hfi_device *hdev) { // Read both registers and return true if both have the right bits set } static int venus_suspend_3xx(struct venus_core *core) { bool val; int ret; // ... ret = readx_poll_timeout(venus_arm9_and_video_core_idle, hdev, val, val, 1500, 100 * 1500); if (ret) return ret; // ... } > + if (!cnt) > + return -ETIMEDOUT; > + > + ret = venus_prepare_power_collapse(hdev, false); > + if (ret) { > + dev_err(dev, "prepare for power collapse fail (%d)\n", ret); > + return ret; > + } > + > + cnt = 100; > + while (--cnt) { > + cpu_status = venus_readl(hdev, WRAPPER_CPU_STATUS); > + ctrl_status = venus_readl(hdev, CPU_CS_SCIACMDARG0); > + > + if (cpu_status & WRAPPER_CPU_STATUS_WFI && > + ctrl_status & CPU_CS_SCIACMDARG0_PC_READY) > + break; > + > + usleep_range(1000, 1500); > + } > + > + if (!cnt) > + return -ETIMEDOUT; Same readx_poll_timeout() trick can be used here, with different helper function, e.g. venus_arm9_idle_and_pc_ready(). Best regards, Tomasz -- 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