From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751559AbeEUMTA (ORCPT ); Mon, 21 May 2018 08:19:00 -0400 Received: from mail-wr0-f196.google.com ([209.85.128.196]:40850 "EHLO mail-wr0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750970AbeEUMS5 (ORCPT ); Mon, 21 May 2018 08:18:57 -0400 X-Google-Smtp-Source: AB8JxZorns2i7W9dQQGN4lktDKxedlPz+2l1mUaj6My5gSQ3OTLUC4vA4SIKQLG4ymoAUJeyC49mbA== Subject: Re: [PATCH v2 08/29] venus: hfi_venus: fix suspend function for venus 3xx versions To: Tomasz Figa , Stanimir Varbanov Cc: Mauro Carvalho Chehab , Hans Verkuil , Linux Media Mailing List , Linux Kernel Mailing List , linux-arm-msm , vgarodia@codeaurora.org References: <20180515075859.17217-1-stanimir.varbanov@linaro.org> <20180515075859.17217-9-stanimir.varbanov@linaro.org> From: Stanimir Varbanov Message-ID: <46683c10-bd18-b0f0-9b07-acc8cadb98a3@linaro.org> Date: Mon, 21 May 2018 15:18:54 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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@linaro.org> 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 >> --- >> 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. -- regards, Stan