| Message ID | 20230228-topic-venus-v1-12-58c2c88384e9@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Venus QoL / maintainability fixes | expand |
On 28/02/2023 15:24, Konrad Dybcio wrote: > - if (IS_V6(core)) > + /* > + * This may sound counter-intuitive, but when there's no TZ, we gotta > + * do things that it would otherwise do for us, such as initializing > + * the hardware at a very basic level. > + * */ Suggest "When there is no TZ we have got to initialize hardware in-lieu of TZ" as an example. Either way please drop that "gotta" - I ain't gonna ACK such a butchering of the language. Then Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
On 28.02.2023 16:48, Bryan O'Donoghue wrote: > On 28/02/2023 15:24, Konrad Dybcio wrote: >> - if (IS_V6(core)) >> + /* >> + * This may sound counter-intuitive, but when there's no TZ, we gotta >> + * do things that it would otherwise do for us, such as initializing >> + * the hardware at a very basic level. >> + * */ > > Suggest "When there is no TZ we have got to initialize hardware in-lieu of TZ" as an example. > > Either way please drop that "gotta" - I ain't gonna ACK such a butchering of the language. Gotta do what you gotta do.. :P I can reword it. Konrad > > Then > > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c index 1bb6406af564..10d3805dc2cb 100644 --- a/drivers/media/platform/qcom/venus/firmware.c +++ b/drivers/media/platform/qcom/venus/firmware.c @@ -29,7 +29,12 @@ static void venus_reset_cpu(struct venus_core *core) u32 fw_size = core->fw.mapped_mem_size; void __iomem *wrapper_base; - if (IS_V6(core)) + /* + * This may sound counter-intuitive, but when there's no TZ, we gotta + * do things that it would otherwise do for us, such as initializing + * the hardware at a very basic level. + * */ + if (!core->use_tz) wrapper_base = core->wrapper_tz_base; else wrapper_base = core->wrapper_base; @@ -41,7 +46,7 @@ static void venus_reset_cpu(struct venus_core *core) writel(0, wrapper_base + WRAPPER_NONPIX_START_ADDR); writel(0, wrapper_base + WRAPPER_NONPIX_END_ADDR); - if (IS_V6(core)) { + if (!core->use_tz) { /* Bring XTSS out of reset */ writel(0, wrapper_base + WRAPPER_TZ_XTSS_SW_RESET); } else { @@ -67,7 +72,7 @@ int venus_set_hw_state(struct venus_core *core, bool resume) if (resume) { venus_reset_cpu(core); } else { - if (IS_V6(core)) + if (!core->use_tz) writel(WRAPPER_XTSS_SW_RESET_BIT, core->wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET); else @@ -179,7 +184,7 @@ static int venus_shutdown_no_tz(struct venus_core *core) void __iomem *wrapper_base = core->wrapper_base; void __iomem *wrapper_tz_base = core->wrapper_tz_base; - if (IS_V6(core)) { + if (IS_IRIS2_1(core)) { /* Assert the reset to XTSS */ reg = readl(wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET); reg |= WRAPPER_XTSS_SW_RESET_BIT;
Most of these checks should have checked for TZ presence (or well, absence), as we shouldn't really be doing things that the black box does for us on non-CrOS platforms. The IS_V6() check in venus_shutdown_no_tz() should have checked whether the core version is IRIS2_1 (so, SC7280). Fix that. Fixes: afeae6ef0780 ("media: venus: firmware: enable no tz fw loading for sc7280") Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> --- drivers/media/platform/qcom/venus/firmware.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)