Received: by 2002:ac0:a5a6:0:0:0:0:0 with SMTP id m35-v6csp1092172imm; Wed, 19 Sep 2018 11:55:38 -0700 (PDT) X-Google-Smtp-Source: ANB0VdY0JryEZ6NVAvXZZmNQn3KLLf4k9m2gKfLqkq1ENXptizaA0TUgsT0b3cto01ByxpQxZMpU X-Received: by 2002:a65:560a:: with SMTP id l10-v6mr33880102pgs.130.1537383338704; Wed, 19 Sep 2018 11:55:38 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1537383338; cv=none; d=google.com; s=arc-20160816; b=IS/2e6wFMIBYEBmm1s973vZdUL51dKc8mtNflxR/dvx/I/PalGZWshRyTZceP3aFea lTbXlfGAqf+q4QSm2m2+/o1wMBj/eQWkRTzBNpxat5dVlb2CvcjPWrOuHl42PW/4uDBZ KZMBNVgwUX2J97Xl7Ja5P/dCvZQrKgJ4OZeUiNYcSuZ6JYcXEWb+N+nqPndrqsQ/Hris qk8DB23RGPnaC1Ury3WVw61zEtUT7aZDd5xvDRdJM8GqlGwbfYRkyvFN5RF7A9wEWYrN xrK9nmJTin4GZZLYUleduyv1nyM7zIUXG6fzZsg/NCILy1B75ugBbyyLttt7ZEqj1Aym Ae1Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:message-id:references :in-reply-to:subject:cc:to:from:date:content-transfer-encoding :mime-version:dkim-signature:dkim-signature; bh=qOFndd9zMl6HBU5sdrp0HNfsmG93WAlXcjF25bGH6j0=; b=zYKmN5dCVe8GrPsZWRoLA/veTksL7Bx0aLTSBjX5kQvXAzAmtwwFy8OY25Rf69Zdz5 pMi7bnTGNRrwB4VmOb6GEUpHRc12rMv4vIvd0uO3UgjNYTKnD403ZSnsyX49V6dmduyq Lyfo59VGhXkUa6DNmgYrwqt3WG6JsAg5O2H7L42iPWGEij50ihdZ0MuS5JN12+H9mBy/ aeUWlvLqecOKiv793/Wg9K/fPffh5F/dhgdAzrDlqLooBecq3Fy1mTQITOBg6yzlW/c3 gvH2gj0edhEKae6W8s3W5I+9Cy5CSJXV82k5TQ0GspxhFzoe0CG+cQi9hv6UJA6rxAcE wA4g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@codeaurora.org header.s=default header.b=RqiMu3Ie; dkim=pass header.i=@codeaurora.org header.s=default header.b=gc+MaZrk; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id f3-v6si21297887plo.109.2018.09.19.11.55.23; Wed, 19 Sep 2018 11:55:38 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@codeaurora.org header.s=default header.b=RqiMu3Ie; dkim=pass header.i=@codeaurora.org header.s=default header.b=gc+MaZrk; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731571AbeISXeF (ORCPT + 99 others); Wed, 19 Sep 2018 19:34:05 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:53476 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726925AbeISXeF (ORCPT ); Wed, 19 Sep 2018 19:34:05 -0400 Received: by smtp.codeaurora.org (Postfix, from userid 1000) id C961360BF5; Wed, 19 Sep 2018 17:55:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1537379702; bh=Ty5Le4td8hGpZLNS+S3EhWgPjwtNUY3w17H6HNWkkzI=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=RqiMu3Iec7/GAJ+4WZMx1sSFxpfRvN6nltOFC5QH1QOGtk023kYV+g8Z/Z7WIxzkY 4bj6AzL79ll5mXOLlMGCkWYe5rYt5N80fItrDJndYqeGsEoSTMwX+PJPbXgBAW9Niq o2OcPrfbMkCuZ1w+dzHB8vzX3ON+JI9voG5kKps0= X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on pdx-caf-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.8 required=2.0 tests=ALL_TRUSTED,BAYES_00, DKIM_SIGNED,T_DKIM_INVALID autolearn=no autolearn_force=no version=3.4.0 Received: from mail.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.codeaurora.org (Postfix) with ESMTP id BBA0E607BD; Wed, 19 Sep 2018 17:55:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1537379701; bh=Ty5Le4td8hGpZLNS+S3EhWgPjwtNUY3w17H6HNWkkzI=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=gc+MaZrkVaiu6wk5ew86TZXd7wUGgx3JW0jGl3PZGwGQxUeUs5vHLMJtjZL+mdVSM 0M7fGbA6NvK/27+Wat1XmaG+aki19KeS7ZgoZz2SiekjJz9k6pRuS8UNTMEhNOdeY8 08bmxk1oikshP2ng6OxOaQyDJMXuE2uBXL07JxqI= MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Wed, 19 Sep 2018 23:25:01 +0530 From: Vikash Garodia To: Stanimir Varbanov Cc: Alexandre Courbot , Hans Verkuil , Mauro Carvalho Chehab , robh@kernel.org, mark.rutland@arm.com, Andy Gross , Arnd Bergmann , bjorn.andersson@linaro.org, Linux Media Mailing List , LKML , linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org, devicetree@vger.kernel.org, linux-media-owner@vger.kernel.org Subject: Re: [PATCH v9 1/5] venus: firmware: add routine to reset ARM9 In-Reply-To: <97b94b9b-f028-cb8b-a3db-67626dc517ab@linaro.org> References: <1537314192-26892-1-git-send-email-vgarodia@codeaurora.org> <1537314192-26892-2-git-send-email-vgarodia@codeaurora.org> <97b94b9b-f028-cb8b-a3db-67626dc517ab@linaro.org> Message-ID: <175fcecf3be715d2a20b71746c648f1e@codeaurora.org> X-Sender: vgarodia@codeaurora.org User-Agent: Roundcube Webmail/1.2.5 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2018-09-19 20:30, Stanimir Varbanov wrote: > Hi Alex, > > On 09/19/2018 10:32 AM, Alexandre Courbot wrote: >> On Wed, Sep 19, 2018 at 8:43 AM Vikash Garodia >> wrote: >>> >>> Add routine to reset the ARM9 and brings it out of reset. Also >>> abstract the Venus CPU state handling with a new function. This >>> is in preparation to add PIL functionality in venus driver. >>> >>> Signed-off-by: Vikash Garodia >>> --- >>> drivers/media/platform/qcom/venus/core.h | 2 ++ >>> drivers/media/platform/qcom/venus/firmware.c | 33 >>> ++++++++++++++++++++++++ >>> drivers/media/platform/qcom/venus/firmware.h | 11 ++++++++ >>> drivers/media/platform/qcom/venus/hfi_venus.c | 13 +++------- >>> drivers/media/platform/qcom/venus/hfi_venus_io.h | 7 +++++ >>> 5 files changed, 57 insertions(+), 9 deletions(-) >>> >>> diff --git a/drivers/media/platform/qcom/venus/core.h >>> b/drivers/media/platform/qcom/venus/core.h >>> index 2f02365..dfd5c10 100644 >>> --- a/drivers/media/platform/qcom/venus/core.h >>> +++ b/drivers/media/platform/qcom/venus/core.h >>> @@ -98,6 +98,7 @@ struct venus_caps { >>> * @dev: convenience struct device pointer >>> * @dev_dec: convenience struct device pointer for decoder device >>> * @dev_enc: convenience struct device pointer for encoder device >>> + * @no_tz: a flag that suggests presence of trustzone >> >> Looks like it suggests the absence of trustzone? >> >> Actually I would rename it as use_tz and set it if TrustZone is used. >> This would avoid double-negative statements like what we see below. > > I find this suggestion reasonable. Initially i planned to keep it as a positive flag. The reason behind keeping it as no_tz was to keep the default value of this flag to 0 indicating tz is present by default. I can switch it to use_tz though and set it in firmware_init after the presence of firmware node is checked. >> >>> * @lock: a lock for this strucure >>> * @instances: a list_head of all instances >>> * @insts_count: num of instances >>> @@ -129,6 +130,7 @@ struct venus_core { >>> struct device *dev; >>> struct device *dev_dec; >>> struct device *dev_enc; >>> + bool no_tz; >>> struct mutex lock; >>> struct list_head instances; >>> atomic_t insts_count; >>> diff --git a/drivers/media/platform/qcom/venus/firmware.c >>> b/drivers/media/platform/qcom/venus/firmware.c >>> index c4a5778..f2ae2f0 100644 >>> --- a/drivers/media/platform/qcom/venus/firmware.c >>> +++ b/drivers/media/platform/qcom/venus/firmware.c >>> @@ -22,10 +22,43 @@ >>> #include >>> #include >>> >>> +#include "core.h" >>> #include "firmware.h" >>> +#include "hfi_venus_io.h" >>> >>> #define VENUS_PAS_ID 9 >>> #define VENUS_FW_MEM_SIZE (6 * SZ_1M) >>> +#define VENUS_FW_START_ADDR 0x0 >>> + >>> +static void venus_reset_cpu(struct venus_core *core) >>> +{ >>> + void __iomem *base = core->base; >>> + >>> + writel(0, base + WRAPPER_FW_START_ADDR); >>> + writel(VENUS_FW_MEM_SIZE, base + WRAPPER_FW_END_ADDR); >>> + writel(0, base + WRAPPER_CPA_START_ADDR); >>> + writel(VENUS_FW_MEM_SIZE, base + WRAPPER_CPA_END_ADDR); >>> + writel(VENUS_FW_MEM_SIZE, base + WRAPPER_NONPIX_START_ADDR); >>> + writel(VENUS_FW_MEM_SIZE, base + WRAPPER_NONPIX_END_ADDR); >>> + writel(0x0, base + WRAPPER_CPU_CGC_DIS); >>> + writel(0x0, base + WRAPPER_CPU_CLOCK_CONFIG); >>> + >>> + /* Bring ARM9 out of reset */ >>> + writel(0, base + WRAPPER_A9SS_SW_RESET); >>> +} >>> + >>> +int venus_set_hw_state(struct venus_core *core, bool resume) >>> +{ >>> + if (!core->no_tz) >> >> This is the kind of double negative statement I was referring do >> above: "if we do not not have TrustZone". Turning it into >> >> if (core->use_tz) >> >> would save the reader a few neurons. :) >> >>> + return qcom_scm_set_remote_state(resume, 0); >>> + >>> + if (resume) >>> + venus_reset_cpu(core); >>> + else >>> + writel(1, core->base + WRAPPER_A9SS_SW_RESET); >>> + >>> + return 0; >>> +} >>> >>> int venus_boot(struct device *dev, const char *fwname) >>> { >>> diff --git a/drivers/media/platform/qcom/venus/firmware.h >>> b/drivers/media/platform/qcom/venus/firmware.h >>> index 428efb5..397570c 100644 >>> --- a/drivers/media/platform/qcom/venus/firmware.h >>> +++ b/drivers/media/platform/qcom/venus/firmware.h >>> @@ -18,5 +18,16 @@ >>> >>> int venus_boot(struct device *dev, const char *fwname); >>> int venus_shutdown(struct device *dev); >>> +int venus_set_hw_state(struct venus_core *core, bool suspend); >>> + >>> +static inline int venus_set_hw_state_suspend(struct venus_core >>> *core) >>> +{ >>> + return venus_set_hw_state(core, false); >>> +} >>> + >>> +static inline int venus_set_hw_state_resume(struct venus_core *core) >>> +{ >>> + return venus_set_hw_state(core, true); >>> +} >> >> I think these two venus_set_hw_state_suspend() and >> venus_set_hw_state_resume() are superfluous, if you want to make the >> state explicit you can also define an enum { SUSPEND, RESUME } to use >> as argument of venus_set_hw_state() and call it directly. > > Infact this was by my request, and I wanted to avoid enum and have the > type of the action in the function name and also avoid one extra > function argument. Of course it is a matter of taste.