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
