linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Few corrections in non-tz firmware boot
@ 2022-10-05  8:37 Stanimir Varbanov
  2022-10-05  8:37 ` [PATCH 1/3] venus: firmware: Correct reset bit Stanimir Varbanov
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Stanimir Varbanov @ 2022-10-05  8:37 UTC (permalink / raw)
  To: linux-media, linux-kernel, linux-arm-msm; +Cc: Stanimir Varbanov

Here are 3 patches which corrects reset bits, non-pix memory region and
use writel/readl with memory barriers included.

Those patches in fact fixes iommu context faults seen on Chromebook
(sc7180) during testing suspend/resume.

Stanimir Varbanov (3):
  venus: firmware: Correct reset bit
  venus: firmware: Correct non-pix start and end addresses
  venus: firmware: Correct assertion of reset bit on remote processor

 drivers/media/platform/qcom/venus/firmware.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/3] venus: firmware: Correct reset bit
  2022-10-05  8:37 [PATCH 0/3] Few corrections in non-tz firmware boot Stanimir Varbanov
@ 2022-10-05  8:37 ` Stanimir Varbanov
  2022-10-05  8:37 ` [PATCH 2/3] venus: firmware: Correct non-pix start and end addresses Stanimir Varbanov
  2022-10-05  8:37 ` [PATCH 3/3] venus: firmware: Correct assertion of reset bit on remote processor Stanimir Varbanov
  2 siblings, 0 replies; 10+ messages in thread
From: Stanimir Varbanov @ 2022-10-05  8:37 UTC (permalink / raw)
  To: linux-media, linux-kernel, linux-arm-msm; +Cc: Stanimir Varbanov

The reset bit for A9SS reset register is BIT(4) and for XTSS_SW_RESET
it is BIT(0). Use the defines for those reset bits.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 drivers/media/platform/qcom/venus/firmware.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
index 14b6f1d05991..3851cedc3329 100644
--- a/drivers/media/platform/qcom/venus/firmware.c
+++ b/drivers/media/platform/qcom/venus/firmware.c
@@ -68,9 +68,11 @@ int venus_set_hw_state(struct venus_core *core, bool resume)
 		venus_reset_cpu(core);
 	} else {
 		if (IS_V6(core))
-			writel(1, core->wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
+			writel(WRAPPER_XTSS_SW_RESET_BIT,
+			       core->wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
 		else
-			writel(1, core->wrapper_base + WRAPPER_A9SS_SW_RESET);
+			writel(WRAPPER_A9SS_SW_RESET_BIT,
+			       core->wrapper_base + WRAPPER_A9SS_SW_RESET);
 	}
 
 	return 0;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3] venus: firmware: Correct non-pix start and end addresses
  2022-10-05  8:37 [PATCH 0/3] Few corrections in non-tz firmware boot Stanimir Varbanov
  2022-10-05  8:37 ` [PATCH 1/3] venus: firmware: Correct reset bit Stanimir Varbanov
@ 2022-10-05  8:37 ` Stanimir Varbanov
  2023-01-26 19:19   ` Matthias Kaehlcke
  2022-10-05  8:37 ` [PATCH 3/3] venus: firmware: Correct assertion of reset bit on remote processor Stanimir Varbanov
  2 siblings, 1 reply; 10+ messages in thread
From: Stanimir Varbanov @ 2022-10-05  8:37 UTC (permalink / raw)
  To: linux-media, linux-kernel, linux-arm-msm; +Cc: Stanimir Varbanov

The default values for those registers are zero.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 drivers/media/platform/qcom/venus/firmware.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
index 3851cedc3329..71e43611d1cf 100644
--- a/drivers/media/platform/qcom/venus/firmware.c
+++ b/drivers/media/platform/qcom/venus/firmware.c
@@ -38,8 +38,8 @@ static void venus_reset_cpu(struct venus_core *core)
 	writel(fw_size, wrapper_base + WRAPPER_FW_END_ADDR);
 	writel(0, wrapper_base + WRAPPER_CPA_START_ADDR);
 	writel(fw_size, wrapper_base + WRAPPER_CPA_END_ADDR);
-	writel(fw_size, wrapper_base + WRAPPER_NONPIX_START_ADDR);
-	writel(fw_size, wrapper_base + WRAPPER_NONPIX_END_ADDR);
+	writel(0, wrapper_base + WRAPPER_NONPIX_START_ADDR);
+	writel(0, wrapper_base + WRAPPER_NONPIX_END_ADDR);
 
 	if (IS_V6(core)) {
 		/* Bring XTSS out of reset */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/3] venus: firmware: Correct assertion of reset bit on remote processor
  2022-10-05  8:37 [PATCH 0/3] Few corrections in non-tz firmware boot Stanimir Varbanov
  2022-10-05  8:37 ` [PATCH 1/3] venus: firmware: Correct reset bit Stanimir Varbanov
  2022-10-05  8:37 ` [PATCH 2/3] venus: firmware: Correct non-pix start and end addresses Stanimir Varbanov
@ 2022-10-05  8:37 ` Stanimir Varbanov
  2 siblings, 0 replies; 10+ messages in thread
From: Stanimir Varbanov @ 2022-10-05  8:37 UTC (permalink / raw)
  To: linux-media, linux-kernel, linux-arm-msm; +Cc: Stanimir Varbanov

Currently we use read/write_relaxed in combination with mb() to assert
reset. This looks wrong because mb() after write_relaxed() will not
order correctly load-update-store sequence. Correct this by use
readl/writel which include memory barriers.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 drivers/media/platform/qcom/venus/firmware.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
index 71e43611d1cf..142d4c74017c 100644
--- a/drivers/media/platform/qcom/venus/firmware.c
+++ b/drivers/media/platform/qcom/venus/firmware.c
@@ -181,17 +181,15 @@ static int venus_shutdown_no_tz(struct venus_core *core)
 
 	if (IS_V6(core)) {
 		/* Assert the reset to XTSS */
-		reg = readl_relaxed(wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
+		reg = readl(wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
 		reg |= WRAPPER_XTSS_SW_RESET_BIT;
-		writel_relaxed(reg, wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
+		writel(reg, wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
 	} else {
 		/* Assert the reset to ARM9 */
-		reg = readl_relaxed(wrapper_base + WRAPPER_A9SS_SW_RESET);
+		reg = readl(wrapper_base + WRAPPER_A9SS_SW_RESET);
 		reg |= WRAPPER_A9SS_SW_RESET_BIT;
-		writel_relaxed(reg, wrapper_base + WRAPPER_A9SS_SW_RESET);
+		writel(reg, wrapper_base + WRAPPER_A9SS_SW_RESET);
 	}
-	/* Make sure reset is asserted before the mapping is removed */
-	mb();
 
 	iommu = core->fw.iommu_domain;
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] venus: firmware: Correct non-pix start and end addresses
  2022-10-05  8:37 ` [PATCH 2/3] venus: firmware: Correct non-pix start and end addresses Stanimir Varbanov
@ 2023-01-26 19:19   ` Matthias Kaehlcke
  2023-02-02 16:39     ` [REGRESSION][PATCH " Matthias Kaehlcke
  2023-02-05 11:00     ` [PATCH " Linux kernel regression tracking (#adding)
  0 siblings, 2 replies; 10+ messages in thread
From: Matthias Kaehlcke @ 2023-01-26 19:19 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: linux-media, linux-kernel, linux-arm-msm, Fritz Koenig, Nathan Hebert

Hi Stanimir,

On Wed, Oct 05, 2022 at 11:37:29AM +0300, Stanimir Varbanov wrote:
> The default values for those registers are zero.
> 
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> ---
>  drivers/media/platform/qcom/venus/firmware.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
> index 3851cedc3329..71e43611d1cf 100644
> --- a/drivers/media/platform/qcom/venus/firmware.c
> +++ b/drivers/media/platform/qcom/venus/firmware.c
> @@ -38,8 +38,8 @@ static void venus_reset_cpu(struct venus_core *core)
>  	writel(fw_size, wrapper_base + WRAPPER_FW_END_ADDR);
>  	writel(0, wrapper_base + WRAPPER_CPA_START_ADDR);
>  	writel(fw_size, wrapper_base + WRAPPER_CPA_END_ADDR);
> -	writel(fw_size, wrapper_base + WRAPPER_NONPIX_START_ADDR);
> -	writel(fw_size, wrapper_base + WRAPPER_NONPIX_END_ADDR);
> +	writel(0, wrapper_base + WRAPPER_NONPIX_START_ADDR);
> +	writel(0, wrapper_base + WRAPPER_NONPIX_END_ADDR);
>  
>  	if (IS_V6(core)) {
>  		/* Bring XTSS out of reset */

I found that this commit prevents the AOSS from entering sleep mode during
system suspend at least on sc7180 and sc7280. AOSS not entering sleep mode
leads to a (apparently significant) increase in S3 power consumption, on
trogdor and herobrine it prevents the system from staying suspended, because
the embedded controller detect the condition and wakes the sytem up again.

Testing is slightly involved, since unfortunately this is not the only issue
in v6.2-rcN that impacts AOSS sleep.

To reach AOSS sleep you also have to revert this commit:

3a39049f88e4 soc: qcom: rpmhpd: Use highest corner until sync_state

And apply something like the diff below (or enable the bwmon driver).

On a trogdor device you will see something like this when AOSS doesn't
enter sleep mode during system suspend:

  [   32.882869] EC detected sleep transition timeout. Total sleep transitions: 0
  [   32.882886] WARNING: CPU: 7 PID: 5682 at drivers/platform/chrome/cros_ec.c:146 cros_ec_sleep_event+0x100/0x10c
  [   32.900393] Modules linked in: uinput veth uvcvideo videobuf2_vmalloc venus_enc venus_dec videobuf2_dma_contig videobuf2_memops onboard_usb_hub cros_ec_typec typec hci_uart btqca xt_MASQUERADE venus_core v4l2_mem2mem videobuf2_v4l2 videobuf2_common qcom_q6v5_mss qcom_pil_v
  [   32.940015] CPU: 7 PID: 5682 Comm: cat Tainted: G        W          6.1.0-rc2+ #295 d14276115b3f6b03fc99220174e5d7724847cbd6
  [   32.951525] Hardware name: Google Villager (rev1+) with LTE (DT)
  [   32.957695] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
  [   32.964848] pc : cros_ec_sleep_event+0x100/0x10c
  [   32.969596] lr : cros_ec_sleep_event+0x100/0x10c

I'm also happy to help with testing if you have a candidate fix.

Thanks

Matthias

--

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 0adf13399e64..c1f6952764c5 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -3488,7 +3488,7 @@ IPCC_MPROC_SIGNAL_GLINK_QMP
                };

                pmu@9091000 {
		-                       compatible = "qcom,sc7280-llcc-bwmon";
		+                       // compatible = "qcom,sc7280-llcc-bwmon";
		                        reg = <0 0x9091000 0 0x1000>;

                        interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>;
			@@ -3528,7 +3528,7 @@ opp-7 {
			                };

                pmu@90b6400 {
		-                       compatible = "qcom,sc7280-cpu-bwmon", "qcom,msm8998-bwmon";
		+                       // compatible = "qcom,sc7280-cpu-bwmon", "qcom,msm8998-bwmon";
		                        reg = <0 0x090b6400 0 0x600>;

                        interrupts = <GIC_SPI 581 IRQ_TYPE_LEVEL_HIGH>;

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [REGRESSION][PATCH 2/3] venus: firmware: Correct non-pix start and end addresses
  2023-01-26 19:19   ` Matthias Kaehlcke
@ 2023-02-02 16:39     ` Matthias Kaehlcke
  2023-02-05 11:00     ` [PATCH " Linux kernel regression tracking (#adding)
  1 sibling, 0 replies; 10+ messages in thread
From: Matthias Kaehlcke @ 2023-02-02 16:39 UTC (permalink / raw)
  To: Stanimir Varbanov, Vikash Garodia
  Cc: linux-media, linux-kernel, linux-arm-msm, Fritz Koenig,
	Nathan Hebert, regressions

On Thu, Jan 26, 2023 at 07:19:13PM +0000, Matthias Kaehlcke wrote:
> Hi Stanimir,
> 
> On Wed, Oct 05, 2022 at 11:37:29AM +0300, Stanimir Varbanov wrote:
> > The default values for those registers are zero.
> > 
> > Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> > ---
> >  drivers/media/platform/qcom/venus/firmware.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
> > index 3851cedc3329..71e43611d1cf 100644
> > --- a/drivers/media/platform/qcom/venus/firmware.c
> > +++ b/drivers/media/platform/qcom/venus/firmware.c
> > @@ -38,8 +38,8 @@ static void venus_reset_cpu(struct venus_core *core)
> >  	writel(fw_size, wrapper_base + WRAPPER_FW_END_ADDR);
> >  	writel(0, wrapper_base + WRAPPER_CPA_START_ADDR);
> >  	writel(fw_size, wrapper_base + WRAPPER_CPA_END_ADDR);
> > -	writel(fw_size, wrapper_base + WRAPPER_NONPIX_START_ADDR);
> > -	writel(fw_size, wrapper_base + WRAPPER_NONPIX_END_ADDR);
> > +	writel(0, wrapper_base + WRAPPER_NONPIX_START_ADDR);
> > +	writel(0, wrapper_base + WRAPPER_NONPIX_END_ADDR);
> >  
> >  	if (IS_V6(core)) {
> >  		/* Bring XTSS out of reset */
> 
> I found that this commit prevents the AOSS from entering sleep mode during
> system suspend at least on sc7180 and sc7280. AOSS not entering sleep mode
> leads to a (apparently significant) increase in S3 power consumption, on
> trogdor and herobrine it prevents the system from staying suspended, because
> the embedded controller detect the condition and wakes the sytem up again.
> 
> Testing is slightly involved, since unfortunately this is not the only issue
> in v6.2-rcN that impacts AOSS sleep.
> 
> To reach AOSS sleep you also have to revert this commit:
> 
> 3a39049f88e4 soc: qcom: rpmhpd: Use highest corner until sync_state
> 
> And apply something like the diff below (or enable the bwmon driver).
> 
> On a trogdor device you will see something like this when AOSS doesn't
> enter sleep mode during system suspend:
> 
>   [   32.882869] EC detected sleep transition timeout. Total sleep transitions: 0
>   [   32.882886] WARNING: CPU: 7 PID: 5682 at drivers/platform/chrome/cros_ec.c:146 cros_ec_sleep_event+0x100/0x10c
>   [   32.900393] Modules linked in: uinput veth uvcvideo videobuf2_vmalloc venus_enc venus_dec videobuf2_dma_contig videobuf2_memops onboard_usb_hub cros_ec_typec typec hci_uart btqca xt_MASQUERADE venus_core v4l2_mem2mem videobuf2_v4l2 videobuf2_common qcom_q6v5_mss qcom_pil_v
>   [   32.940015] CPU: 7 PID: 5682 Comm: cat Tainted: G        W          6.1.0-rc2+ #295 d14276115b3f6b03fc99220174e5d7724847cbd6
>   [   32.951525] Hardware name: Google Villager (rev1+) with LTE (DT)
>   [   32.957695] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>   [   32.964848] pc : cros_ec_sleep_event+0x100/0x10c
>   [   32.969596] lr : cros_ec_sleep_event+0x100/0x10c
> 
> I'm also happy to help with testing if you have a candidate fix.

This change introduced an important regression at least for sc7180 and sc7280,
and probably other QC SoCs, I wonder if it should be reverted unless there is
an obvious better fix.

> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 0adf13399e64..c1f6952764c5 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -3488,7 +3488,7 @@ IPCC_MPROC_SIGNAL_GLINK_QMP
>                 };
> 
>                 pmu@9091000 {
> 		-                       compatible = "qcom,sc7280-llcc-bwmon";
> 		+                       // compatible = "qcom,sc7280-llcc-bwmon";
> 		                        reg = <0 0x9091000 0 0x1000>;
> 
>                         interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>;
> 			@@ -3528,7 +3528,7 @@ opp-7 {
> 			                };
> 
>                 pmu@90b6400 {
> 		-                       compatible = "qcom,sc7280-cpu-bwmon", "qcom,msm8998-bwmon";
> 		+                       // compatible = "qcom,sc7280-cpu-bwmon", "qcom,msm8998-bwmon";
> 		                        reg = <0 0x090b6400 0 0x600>;
> 
>                         interrupts = <GIC_SPI 581 IRQ_TYPE_LEVEL_HIGH>;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] venus: firmware: Correct non-pix start and end addresses
  2023-01-26 19:19   ` Matthias Kaehlcke
  2023-02-02 16:39     ` [REGRESSION][PATCH " Matthias Kaehlcke
@ 2023-02-05 11:00     ` Linux kernel regression tracking (#adding)
  2023-02-11 14:23       ` Linux regression tracking #update (Thorsten Leemhuis)
                         ` (2 more replies)
  1 sibling, 3 replies; 10+ messages in thread
From: Linux kernel regression tracking (#adding) @ 2023-02-05 11:00 UTC (permalink / raw)
  To: Matthias Kaehlcke, Stanimir Varbanov
  Cc: linux-media, linux-kernel, linux-arm-msm, Fritz Koenig, Nathan Hebert

[TLDR: I'm adding this report to the list of tracked Linux kernel
regressions; the text you find below is based on a few templates
paragraphs you might have encountered already in similar form.
See link in footer if these mails annoy you.]

On 26.01.23 20:19, Matthias Kaehlcke wrote:

> On Wed, Oct 05, 2022 at 11:37:29AM +0300, Stanimir Varbanov wrote:
>> The default values for those registers are zero.
>>
>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
>> ---
>>  drivers/media/platform/qcom/venus/firmware.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
>> index 3851cedc3329..71e43611d1cf 100644
>> --- a/drivers/media/platform/qcom/venus/firmware.c
>> +++ b/drivers/media/platform/qcom/venus/firmware.c
>> @@ -38,8 +38,8 @@ static void venus_reset_cpu(struct venus_core *core)
>>  	writel(fw_size, wrapper_base + WRAPPER_FW_END_ADDR);
>>  	writel(0, wrapper_base + WRAPPER_CPA_START_ADDR);
>>  	writel(fw_size, wrapper_base + WRAPPER_CPA_END_ADDR);
>> -	writel(fw_size, wrapper_base + WRAPPER_NONPIX_START_ADDR);
>> -	writel(fw_size, wrapper_base + WRAPPER_NONPIX_END_ADDR);
>> +	writel(0, wrapper_base + WRAPPER_NONPIX_START_ADDR);
>> +	writel(0, wrapper_base + WRAPPER_NONPIX_END_ADDR);
>>  
>>  	if (IS_V6(core)) {
>>  		/* Bring XTSS out of reset */
> 
> I found that this commit prevents the AOSS from entering sleep mode during
> system suspend at least on sc7180 and sc7280. AOSS not entering sleep mode
> leads to a (apparently significant) increase in S3 power consumption, on
> trogdor and herobrine it prevents the system from staying suspended, because
> the embedded controller detect the condition and wakes the sytem up again.

Thanks for the report. To be sure the issue doesn't fall through the
cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression
tracking bot:

#regzbot ^introduced a837e5161cfffbb32
#regzbot title meida: venus: firmware: AOSS doesn't seel anymore on at
least sc7180 and sc7280
#regzbot ignore-activity

This isn't a regression? This issue or a fix for it are already
discussed somewhere else? It was fixed already? You want to clarify when
the regression started to happen? Or point out I got the title or
something else totally wrong? Then just reply and tell me -- ideally
while also telling regzbot about it, as explained by the page listed in
the footer of this mail.

Developers: When fixing the issue, remember to add 'Link:' tags pointing
to the report (the parent of this mail). See page linked in footer for
details.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.

> Testing is slightly involved, since unfortunately this is not the only issue
> in v6.2-rcN that impacts AOSS sleep.
> 
> To reach AOSS sleep you also have to revert this commit:
> 
> 3a39049f88e4 soc: qcom: rpmhpd: Use highest corner until sync_state

FWIW, for anyone that like me is wondering about that story, it can be
found here:
https://lore.kernel.org/all/Y9v%2Fz8CYik3faHh7@google.com/

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.

> And apply something like the diff below (or enable the bwmon driver).
> 
> On a trogdor device you will see something like this when AOSS doesn't
> enter sleep mode during system suspend:
> 
>   [   32.882869] EC detected sleep transition timeout. Total sleep transitions: 0
>   [   32.882886] WARNING: CPU: 7 PID: 5682 at drivers/platform/chrome/cros_ec.c:146 cros_ec_sleep_event+0x100/0x10c
>   [   32.900393] Modules linked in: uinput veth uvcvideo videobuf2_vmalloc venus_enc venus_dec videobuf2_dma_contig videobuf2_memops onboard_usb_hub cros_ec_typec typec hci_uart btqca xt_MASQUERADE venus_core v4l2_mem2mem videobuf2_v4l2 videobuf2_common qcom_q6v5_mss qcom_pil_v
>   [   32.940015] CPU: 7 PID: 5682 Comm: cat Tainted: G        W          6.1.0-rc2+ #295 d14276115b3f6b03fc99220174e5d7724847cbd6
>   [   32.951525] Hardware name: Google Villager (rev1+) with LTE (DT)
>   [   32.957695] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>   [   32.964848] pc : cros_ec_sleep_event+0x100/0x10c
>   [   32.969596] lr : cros_ec_sleep_event+0x100/0x10c
> 
> I'm also happy to help with testing if you have a candidate fix.
>
> --
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 0adf13399e64..c1f6952764c5 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -3488,7 +3488,7 @@ IPCC_MPROC_SIGNAL_GLINK_QMP
>                 };
> 
>                 pmu@9091000 {
> 		-                       compatible = "qcom,sc7280-llcc-bwmon";
> 		+                       // compatible = "qcom,sc7280-llcc-bwmon";
> 		                        reg = <0 0x9091000 0 0x1000>;
> 
>                         interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>;
> 			@@ -3528,7 +3528,7 @@ opp-7 {
> 			                };
> 
>                 pmu@90b6400 {
> 		-                       compatible = "qcom,sc7280-cpu-bwmon", "qcom,msm8998-bwmon";
> 		+                       // compatible = "qcom,sc7280-cpu-bwmon", "qcom,msm8998-bwmon";
> 		                        reg = <0 0x090b6400 0 0x600>;
> 
>                         interrupts = <GIC_SPI 581 IRQ_TYPE_LEVEL_HIGH>;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] venus: firmware: Correct non-pix start and end addresses
  2023-02-05 11:00     ` [PATCH " Linux kernel regression tracking (#adding)
@ 2023-02-11 14:23       ` Linux regression tracking #update (Thorsten Leemhuis)
  2023-03-20 10:52       ` Linux regression tracking #update (Thorsten Leemhuis)
  2023-04-05 12:45       ` Linux regression tracking #update (Thorsten Leemhuis)
  2 siblings, 0 replies; 10+ messages in thread
From: Linux regression tracking #update (Thorsten Leemhuis) @ 2023-02-11 14:23 UTC (permalink / raw)
  To: Matthias Kaehlcke, Stanimir Varbanov
  Cc: linux-media, linux-kernel, linux-arm-msm, Fritz Koenig, Nathan Hebert

[TLDR: This mail in primarily relevant for Linux regression tracking. A
change or fix related to the regression discussed in this thread was
posted or applied, but it did not use a Link: tag to point to the
report, as Linus and the documentation call for. Things happen, no
worries -- but now the regression tracking bot needs to be told manually
about the fix. See link in footer if these mails annoy you.]

On 05.02.23 12:00, Linux kernel regression tracking (#adding) wrote:
> On 26.01.23 20:19, Matthias Kaehlcke wrote:
>> On Wed, Oct 05, 2022 at 11:37:29AM +0300, Stanimir Varbanov wrote:
>>> The default values for those registers are zero.
>>>
>>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> [...]
>> I found that this commit prevents the AOSS from entering sleep mode during
>> system suspend at least on sc7180 and sc7280. AOSS not entering sleep mode
>> leads to a (apparently significant) increase in S3 power consumption, on
>> trogdor and herobrine it prevents the system from staying suspended, because
>> the embedded controller detect the condition and wakes the sytem up again.
> 
> Thanks for the report. To be sure the issue doesn't fall through the
> cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression
> tracking bot:
> 
> #regzbot ^introduced a837e5161cfffbb32
> #regzbot title meida: venus: firmware: AOSS doesn't seel anymore on at
> least sc7180 and sc7280
> #regzbot ignore-activity

#regzbot monitor:
https://lore.kernel.org/all/20230207102254.1446461-1-javierm@redhat.com/
#regzbot ignore-activity

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] venus: firmware: Correct non-pix start and end addresses
  2023-02-05 11:00     ` [PATCH " Linux kernel regression tracking (#adding)
  2023-02-11 14:23       ` Linux regression tracking #update (Thorsten Leemhuis)
@ 2023-03-20 10:52       ` Linux regression tracking #update (Thorsten Leemhuis)
  2023-04-05 12:45       ` Linux regression tracking #update (Thorsten Leemhuis)
  2 siblings, 0 replies; 10+ messages in thread
From: Linux regression tracking #update (Thorsten Leemhuis) @ 2023-03-20 10:52 UTC (permalink / raw)
  To: Matthias Kaehlcke, Stanimir Varbanov
  Cc: linux-media, linux-kernel, linux-arm-msm, Fritz Koenig, Nathan Hebert

[TLDR: This mail in primarily relevant for Linux kernel regression
tracking. See link in footer if these mails annoy you.]

On 05.02.23 12:00, Linux kernel regression tracking (#adding) wrote:

> On 26.01.23 20:19, Matthias Kaehlcke wrote:
>> On Wed, Oct 05, 2022 at 11:37:29AM +0300, Stanimir Varbanov wrote:
>>
>> I found that this commit prevents the AOSS from entering sleep mode during
>> system suspend at least on sc7180 and sc7280. AOSS not entering sleep mode
>> leads to a (apparently significant) increase in S3 power consumption, on
>> trogdor and herobrine it prevents the system from staying suspended, because
>> the embedded controller detect the condition and wakes the sytem up again.
> 
> Thanks for the report. To be sure the issue doesn't fall through the
> cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression
> tracking bot:
> 
> #regzbot ^introduced a837e5161cfffbb32
> #regzbot title meida: venus: firmware: AOSS doesn't seel anymore on at
> least sc7180 and sc7280
> #regzbot ignore-activity

#regzbot fix: media: Revert "venus: firmware: Correct non-pix start and
end addresses"
#regzbot ignore-activity

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] venus: firmware: Correct non-pix start and end addresses
  2023-02-05 11:00     ` [PATCH " Linux kernel regression tracking (#adding)
  2023-02-11 14:23       ` Linux regression tracking #update (Thorsten Leemhuis)
  2023-03-20 10:52       ` Linux regression tracking #update (Thorsten Leemhuis)
@ 2023-04-05 12:45       ` Linux regression tracking #update (Thorsten Leemhuis)
  2 siblings, 0 replies; 10+ messages in thread
From: Linux regression tracking #update (Thorsten Leemhuis) @ 2023-04-05 12:45 UTC (permalink / raw)
  To: Matthias Kaehlcke, Stanimir Varbanov
  Cc: linux-media, linux-kernel, linux-arm-msm, Fritz Koenig, Nathan Hebert

[TLDR: This mail in primarily relevant for Linux kernel regression
tracking. See link in footer if these mails annoy you.]

On 05.02.23 12:00, Linux kernel regression tracking (#adding) wrote:

> On 26.01.23 20:19, Matthias Kaehlcke wrote:
> 
>> On Wed, Oct 05, 2022 at 11:37:29AM +0300, Stanimir Varbanov wrote:
>>> The default values for those registers are zero.
>>>
>>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
>>> ---
>>>  drivers/media/platform/qcom/venus/firmware.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
>>> index 3851cedc3329..71e43611d1cf 100644
>>> --- a/drivers/media/platform/qcom/venus/firmware.c
>>> +++ b/drivers/media/platform/qcom/venus/firmware.c
>>> @@ -38,8 +38,8 @@ static void venus_reset_cpu(struct venus_core *core)
>>>  	writel(fw_size, wrapper_base + WRAPPER_FW_END_ADDR);
>>>  	writel(0, wrapper_base + WRAPPER_CPA_START_ADDR);
>>>  	writel(fw_size, wrapper_base + WRAPPER_CPA_END_ADDR);
>>> -	writel(fw_size, wrapper_base + WRAPPER_NONPIX_START_ADDR);
>>> -	writel(fw_size, wrapper_base + WRAPPER_NONPIX_END_ADDR);
>>> +	writel(0, wrapper_base + WRAPPER_NONPIX_START_ADDR);
>>> +	writel(0, wrapper_base + WRAPPER_NONPIX_END_ADDR);
>>>  
>>>  	if (IS_V6(core)) {
>>>  		/* Bring XTSS out of reset */
>>
>> I found that this commit prevents the AOSS from entering sleep mode during
>> system suspend at least on sc7180 and sc7280. AOSS not entering sleep mode
>> leads to a (apparently significant) increase in S3 power consumption, on
>> trogdor and herobrine it prevents the system from staying suspended, because
>> the embedded controller detect the condition and wakes the sytem up again.
> 
> Thanks for the report. To be sure the issue doesn't fall through the
> cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression
> tracking bot:
> 
> #regzbot ^introduced a837e5161cfffbb32
> #regzbot title meida: venus: firmware: AOSS doesn't seel anymore on at
> least sc7180 and sc7280
> #regzbot ignore-activity
	
#regzbot fix: f95b8ea79c47c0ad
#regzbot ignore-activity

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-04-05 12:45 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-05  8:37 [PATCH 0/3] Few corrections in non-tz firmware boot Stanimir Varbanov
2022-10-05  8:37 ` [PATCH 1/3] venus: firmware: Correct reset bit Stanimir Varbanov
2022-10-05  8:37 ` [PATCH 2/3] venus: firmware: Correct non-pix start and end addresses Stanimir Varbanov
2023-01-26 19:19   ` Matthias Kaehlcke
2023-02-02 16:39     ` [REGRESSION][PATCH " Matthias Kaehlcke
2023-02-05 11:00     ` [PATCH " Linux kernel regression tracking (#adding)
2023-02-11 14:23       ` Linux regression tracking #update (Thorsten Leemhuis)
2023-03-20 10:52       ` Linux regression tracking #update (Thorsten Leemhuis)
2023-04-05 12:45       ` Linux regression tracking #update (Thorsten Leemhuis)
2022-10-05  8:37 ` [PATCH 3/3] venus: firmware: Correct assertion of reset bit on remote processor Stanimir Varbanov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).