lkml.org 
[lkml]   [2018]   [May]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 08/28] venus: hfi_venus: add suspend function for 4xx version
    From
    Date
    Hi,

    On 05/09/2018 05:14 PM, Vikash Garodia wrote:
    > Hi Stanimir,
    >
    > On 2018-05-09 16:45, Stanimir Varbanov wrote:
    >> Hi Vikash,
    >>
    >> On 05/02/2018 09:07 AM, vgarodia@codeaurora.org wrote:
    >>> Hello Stanimir,
    >>>
    >>> On 2018-04-24 18:14, Stanimir Varbanov wrote:
    >>>> This adds suspend (power collapse) function with slightly
    >>>> different order of calls comparing with Venus 3xx.
    >>>>
    >>>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
    >>>> ---
    >>>>  drivers/media/platform/qcom/venus/hfi_venus.c | 52
    >>>> +++++++++++++++++++++++++++
    >>>>  1 file changed, 52 insertions(+)
    >>>>
    >>>> diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c
    >>>> b/drivers/media/platform/qcom/venus/hfi_venus.c
    >>>> index 53546174aab8..f61d34bf61b4 100644
    >>>> --- a/drivers/media/platform/qcom/venus/hfi_venus.c
    >>>> +++ b/drivers/media/platform/qcom/venus/hfi_venus.c
    >>>> @@ -1443,6 +1443,55 @@ static int venus_suspend_1xx(struct venus_core
    >>>> *core)
    >>>>      return 0;
    >>>>  }
    >>>>
    >>>> +static int venus_suspend_4xx(struct venus_core *core)
    >>>> +{
    >>>> +    struct venus_hfi_device *hdev = to_hfi_priv(core);
    >>>> +    struct device *dev = core->dev;
    >>>> +    u32 val;
    >>>> +    int ret;
    >>>> +
    >>>> +    if (!hdev->power_enabled || hdev->suspended)
    >>>> +        return 0;
    >>>> +
    >>>> +    mutex_lock(&hdev->lock);
    >>>> +    ret = venus_is_valid_state(hdev);
    >>>> +    mutex_unlock(&hdev->lock);
    >>>> +
    >>>> +    if (!ret) {
    >>>> +        dev_err(dev, "bad state, cannot suspend\n");
    >>>> +        return -EINVAL;
    >>>> +    }
    >>>> +
    >>>> +    ret = venus_prepare_power_collapse(hdev, false);
    >>>> +    if (ret) {
    >>>> +        dev_err(dev, "prepare for power collapse fail (%d)\n", ret);
    >>>> +        return ret;
    >>>> +    }
    >>>> +
    >>>> +    ret = readl_poll_timeout(core->base + CPU_CS_SCIACMDARG0, val,
    >>>> +                 val & CPU_CS_SCIACMDARG0_PC_READY,
    >>>> +                 POLL_INTERVAL_US, 100000);
    >>>> +    if (ret) {
    >>>> +        dev_err(dev, "Polling power collapse ready timed out\n");
    >>>> +        return ret;
    >>>> +    }
    >>>> +
    >>>> +    mutex_lock(&hdev->lock);
    >>>> +
    >>>> +    ret = venus_power_off(hdev);
    >>>> +    if (ret) {
    >>>> +        dev_err(dev, "venus_power_off (%d)\n", ret);
    >>>> +        mutex_unlock(&hdev->lock);
    >>>> +        return ret;
    >>>> +    }
    >>>> +
    >>>> +    hdev->suspended = true;
    >>>> +
    >>>> +    mutex_unlock(&hdev->lock);
    >>>> +
    >>>> +    return 0;
    >>>> +}
    >>>> +
    >>>>  static int venus_suspend_3xx(struct venus_core *core)
    >>>>  {
    >>>>      struct venus_hfi_device *hdev = to_hfi_priv(core);
    >>>> @@ -1507,6 +1556,9 @@ static int venus_suspend(struct venus_core *core)
    >>>>      if (core->res->hfi_version == HFI_VERSION_3XX)
    >>>>          return venus_suspend_3xx(core);
    >>>>
    >>>> +    if (core->res->hfi_version == HFI_VERSION_4XX)
    >>>> +        return venus_suspend_4xx(core);
    >>>> +
    >>>>      return venus_suspend_1xx(core);
    >>>>  }
    >>>
    >>> Let me brief on the power collapse sequence for Venus_4xx
    >>> 1. Host checks for ARM9 and Video core to be idle.
    >>>    This can be done by checking for WFI bit (bit 0) in CPU status
    >>> register for ARM9 and by checking bit 30 in Control status reg for video
    >>> core/s.
    >>> 2. Host then sends command to ARM9 to prepare for power collapse.
    >>> 3. Host then checks for WFI bit and PC_READY bit before withdrawing
    >>> going for power off.
    >>>
    >>> As per this patch, host is preparing for power collapse without checking
    >>> for #1.
    >>> Update the code to handle #3.
    >>
    >> This looks similar to suspend for Venus 3xx. Can you confirm that the
    >> sequence of checks for 4xx is the same as 3xx?
    >
    > Do you mean the driver implementation for Suspend Venus 3xx or the hardware
    > expectation for Venus 3xx ? If hardware expectation wise, the sequence is
    > same for 3xx and 4xx.
    > In the suspend implementation for 3xx, i see that the host just reads
    > the WFI
    > and idle status bits, but does not validate those bit value before
    > preparing
    > Venus for power collapse. Sequence #2 and #3 is followed for Venus 3xx
    > implementation.

    OK, than we can reuse venus_suspend_3xx function for 4xx, just need to
    fix WFI and idle bit for #1.

    --
    regards,
    Stan

    \
     
     \ /
      Last update: 2018-05-09 16:27    [W:4.269 / U:0.356 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site