[PATCH 12/18] media: venus: firmware: Correct IS_V6() checks

Konrad Dybcio posted 18 patches 2 years, 6 months ago
There is a newer version of this series
[PATCH 12/18] media: venus: firmware: Correct IS_V6() checks
Posted by Konrad Dybcio 2 years, 6 months ago
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(-)

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;

-- 
2.39.2
Re: [PATCH 12/18] media: venus: firmware: Correct IS_V6() checks
Posted by Bryan O'Donoghue 2 years, 6 months ago
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>
Re: [PATCH 12/18] media: venus: firmware: Correct IS_V6() checks
Posted by Konrad Dybcio 2 years, 6 months ago

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>