Hi Vikash,
Thanks for the patch!
On 1.06.2018 23:26, Vikash Garodia wrote:
Add a new routine which abstracts the TZ call to
Actually the new routine abstracts Venus CPU state, Isn't it?
set the video hardware state.
Signed-off-by: Vikash Garodia <vgarodia@xxxxxxxxxxxxxx>
---
drivers/media/platform/qcom/venus/core.h | 5 +++++
drivers/media/platform/qcom/venus/firmware.c | 28 +++++++++++++++++++++++++++
drivers/media/platform/qcom/venus/firmware.h | 1 +
drivers/media/platform/qcom/venus/hfi_venus.c | 13 ++++---------
4 files changed, 38 insertions(+), 9 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index 85e66e2..e7bfb63 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -35,6 +35,11 @@ struct reg_val {
u32 value;
};
+enum tzbsp_video_state {
+ TZBSP_VIDEO_SUSPEND = 0,
+ TZBSP_VIDEO_RESUME
+};
please move this in firmware.c, for more see below.
+
struct venus_resources {
u64 dma_mask;
const struct freq_tbl *freq_tbl;
diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
index 7d89b5a..b4664ed 100644
--- a/drivers/media/platform/qcom/venus/firmware.c
+++ b/drivers/media/platform/qcom/venus/firmware.c
@@ -53,6 +53,34 @@ static void venus_reset_hw(struct venus_core *core)
/* Bring Arm9 out of reset */
writel_relaxed(0, reg_base + WRAPPER_A9SS_SW_RESET);
}
+
+int venus_set_hw_state(enum tzbsp_video_state state, struct venus_core *core)
can we put this function this way:
venus_set_state(struct venus_core *core, bool on)
so we set the state to On when we are power-up the venus CPU and Off
when we power-down.
by this way we really abstract the state, IMO.
+{
+ int ret;
+ struct device *dev = core->dev;
+ void __iomem *reg_base = core->base;
just 'base' should be enough.
+
+ switch (state) {
+ case TZBSP_VIDEO_SUSPEND:
+ if (qcom_scm_is_available())
You really shouldn't rely on this function (see the comment from Bjorn
on first version of this patch series).
I think we have to replace qcom_scm_is_available() with some flag which
is reflecting does the firmware subnode is exist or not. In case it is
not exist the we have to go with TZ scm calls.
+ ret = qcom_scm_set_remote_state(TZBSP_VIDEO_SUSPEND, 0);
+ else
+ writel_relaxed(1, reg_base + WRAPPER_A9SS_SW_RESET);
+ break;
+ case TZBSP_VIDEO_RESUME:
+ if (qcom_scm_is_available())
+ ret = qcom_scm_set_remote_state(TZBSP_VIDEO_RESUME, 0);
+ else
+ venus_reset_hw(core);
+ break;
+ default:
+ dev_err(dev, "invalid state\n");
+ break;
+ }
+ return ret;
+}
+EXPORT_SYMBOL_GPL(venus_set_hw_state);
+
regards,
Stan