* [PATCH] arm64: dts: mediatek: mt8195: Fix PM suspend/resume with venc clocks
@ 2023-07-06 9:58 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 8+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-06 9:58 UTC (permalink / raw)
To: matthias.bgg
Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, yong.wu, tinghan.shen,
weiyi.lu, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, kernel, AngeloGioacchino Del Regno
Before suspending the LARBs we're making sure that any operation is
done: this never happens because we are unexpectedly unclocking the
LARB20 before executing the suspend handler for the MediaTek Smart
Multimedia Interface (SMI) and the cause of this is incorrect clocks
on this LARB.
Fix this issue by changing the Local Arbiter 20 (used by the video
encoder secondary core) apb clock to CLK_VENC_CORE1_VENC;
furthermore, in order to make sure that both the PM resume and video
encoder operation is stable, add the CLK_VENC(_CORE1)_LARB clock to
the VENC (main core) and VENC_CORE1 power domains, as this IP cannot
communicate with the rest of the system (the AP) without local
arbiter clocks being operational.
Fixes: 3b5838d1d82e ("arm64: dts: mt8195: Add iommu and smi nodes")
Fixes: 2b515194bf0c ("arm64: dts: mt8195: Add power domains controller")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
arch/arm64/boot/dts/mediatek/mt8195.dtsi | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 95bd058d6083..5c670fce1e47 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -626,6 +626,8 @@ power-domain@MT8195_POWER_DOMAIN_VDEC1 {
power-domain@MT8195_POWER_DOMAIN_VENC_CORE1 {
reg = <MT8195_POWER_DOMAIN_VENC_CORE1>;
+ clocks = <&vencsys_core1 CLK_VENC_CORE1_LARB>;
+ clock-names = "venc1-larb";
mediatek,infracfg = <&infracfg_ao>;
#power-domain-cells = <0>;
};
@@ -688,6 +690,8 @@ power-domain@MT8195_POWER_DOMAIN_VDEC2 {
power-domain@MT8195_POWER_DOMAIN_VENC {
reg = <MT8195_POWER_DOMAIN_VENC>;
+ clocks = <&vencsys CLK_VENC_LARB>;
+ clock-names = "venc0-larb";
mediatek,infracfg = <&infracfg_ao>;
#power-domain-cells = <0>;
};
@@ -3094,7 +3098,7 @@ larb20: larb@1b010000 {
reg = <0 0x1b010000 0 0x1000>;
mediatek,larb-id = <20>;
mediatek,smi = <&smi_common_vpp>;
- clocks = <&vencsys_core1 CLK_VENC_CORE1_LARB>,
+ clocks = <&vencsys_core1 CLK_VENC_CORE1_VENC>,
<&vencsys_core1 CLK_VENC_CORE1_GALS>,
<&vppsys0 CLK_VPP0_GALS_VDO0_VDO1_VENCSYS_CORE1>;
clock-names = "apb", "smi", "gals";
--
2.40.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] arm64: dts: mediatek: mt8195: Fix PM suspend/resume with venc clocks
@ 2023-07-06 9:58 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 8+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-06 9:58 UTC (permalink / raw)
To: matthias.bgg
Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, yong.wu, tinghan.shen,
weiyi.lu, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, kernel, AngeloGioacchino Del Regno
Before suspending the LARBs we're making sure that any operation is
done: this never happens because we are unexpectedly unclocking the
LARB20 before executing the suspend handler for the MediaTek Smart
Multimedia Interface (SMI) and the cause of this is incorrect clocks
on this LARB.
Fix this issue by changing the Local Arbiter 20 (used by the video
encoder secondary core) apb clock to CLK_VENC_CORE1_VENC;
furthermore, in order to make sure that both the PM resume and video
encoder operation is stable, add the CLK_VENC(_CORE1)_LARB clock to
the VENC (main core) and VENC_CORE1 power domains, as this IP cannot
communicate with the rest of the system (the AP) without local
arbiter clocks being operational.
Fixes: 3b5838d1d82e ("arm64: dts: mt8195: Add iommu and smi nodes")
Fixes: 2b515194bf0c ("arm64: dts: mt8195: Add power domains controller")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
arch/arm64/boot/dts/mediatek/mt8195.dtsi | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 95bd058d6083..5c670fce1e47 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -626,6 +626,8 @@ power-domain@MT8195_POWER_DOMAIN_VDEC1 {
power-domain@MT8195_POWER_DOMAIN_VENC_CORE1 {
reg = <MT8195_POWER_DOMAIN_VENC_CORE1>;
+ clocks = <&vencsys_core1 CLK_VENC_CORE1_LARB>;
+ clock-names = "venc1-larb";
mediatek,infracfg = <&infracfg_ao>;
#power-domain-cells = <0>;
};
@@ -688,6 +690,8 @@ power-domain@MT8195_POWER_DOMAIN_VDEC2 {
power-domain@MT8195_POWER_DOMAIN_VENC {
reg = <MT8195_POWER_DOMAIN_VENC>;
+ clocks = <&vencsys CLK_VENC_LARB>;
+ clock-names = "venc0-larb";
mediatek,infracfg = <&infracfg_ao>;
#power-domain-cells = <0>;
};
@@ -3094,7 +3098,7 @@ larb20: larb@1b010000 {
reg = <0 0x1b010000 0 0x1000>;
mediatek,larb-id = <20>;
mediatek,smi = <&smi_common_vpp>;
- clocks = <&vencsys_core1 CLK_VENC_CORE1_LARB>,
+ clocks = <&vencsys_core1 CLK_VENC_CORE1_VENC>,
<&vencsys_core1 CLK_VENC_CORE1_GALS>,
<&vppsys0 CLK_VPP0_GALS_VDO0_VDO1_VENCSYS_CORE1>;
clock-names = "apb", "smi", "gals";
--
2.40.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: dts: mediatek: mt8195: Fix PM suspend/resume with venc clocks
2023-07-06 9:58 ` AngeloGioacchino Del Regno
@ 2023-07-06 12:35 ` Alexandre Mergnat
-1 siblings, 0 replies; 8+ messages in thread
From: Alexandre Mergnat @ 2023-07-06 12:35 UTC (permalink / raw)
To: AngeloGioacchino Del Regno, matthias.bgg
Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, yong.wu, tinghan.shen,
weiyi.lu, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, kernel
On 06/07/2023 11:58, AngeloGioacchino Del Regno wrote:
> Before suspending the LARBs we're making sure that any operation is
> done: this never happens because we are unexpectedly unclocking the
> LARB20 before executing the suspend handler for the MediaTek Smart
> Multimedia Interface (SMI) and the cause of this is incorrect clocks
> on this LARB.
>
> Fix this issue by changing the Local Arbiter 20 (used by the video
> encoder secondary core) apb clock to CLK_VENC_CORE1_VENC;
> furthermore, in order to make sure that both the PM resume and video
> encoder operation is stable, add the CLK_VENC(_CORE1)_LARB clock to
> the VENC (main core) and VENC_CORE1 power domains, as this IP cannot
> communicate with the rest of the system (the AP) without local
> arbiter clocks being operational.
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
--
Regards,
Alexandre
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: dts: mediatek: mt8195: Fix PM suspend/resume with venc clocks
@ 2023-07-06 12:35 ` Alexandre Mergnat
0 siblings, 0 replies; 8+ messages in thread
From: Alexandre Mergnat @ 2023-07-06 12:35 UTC (permalink / raw)
To: AngeloGioacchino Del Regno, matthias.bgg
Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, yong.wu, tinghan.shen,
weiyi.lu, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, kernel
On 06/07/2023 11:58, AngeloGioacchino Del Regno wrote:
> Before suspending the LARBs we're making sure that any operation is
> done: this never happens because we are unexpectedly unclocking the
> LARB20 before executing the suspend handler for the MediaTek Smart
> Multimedia Interface (SMI) and the cause of this is incorrect clocks
> on this LARB.
>
> Fix this issue by changing the Local Arbiter 20 (used by the video
> encoder secondary core) apb clock to CLK_VENC_CORE1_VENC;
> furthermore, in order to make sure that both the PM resume and video
> encoder operation is stable, add the CLK_VENC(_CORE1)_LARB clock to
> the VENC (main core) and VENC_CORE1 power domains, as this IP cannot
> communicate with the rest of the system (the AP) without local
> arbiter clocks being operational.
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
--
Regards,
Alexandre
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: dts: mediatek: mt8195: Fix PM suspend/resume with venc clocks
2023-07-06 9:58 ` AngeloGioacchino Del Regno
@ 2023-07-07 5:06 ` Chen-Yu Tsai
-1 siblings, 0 replies; 8+ messages in thread
From: Chen-Yu Tsai @ 2023-07-07 5:06 UTC (permalink / raw)
To: AngeloGioacchino Del Regno
Cc: matthias.bgg, robh+dt, krzysztof.kozlowski+dt, conor+dt, yong.wu,
tinghan.shen, weiyi.lu, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek, kernel
On Thu, Jul 6, 2023 at 5:59 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Before suspending the LARBs we're making sure that any operation is
> done: this never happens because we are unexpectedly unclocking the
> LARB20 before executing the suspend handler for the MediaTek Smart
> Multimedia Interface (SMI) and the cause of this is incorrect clocks
> on this LARB.
>
> Fix this issue by changing the Local Arbiter 20 (used by the video
> encoder secondary core) apb clock to CLK_VENC_CORE1_VENC;
> furthermore, in order to make sure that both the PM resume and video
> encoder operation is stable, add the CLK_VENC(_CORE1)_LARB clock to
> the VENC (main core) and VENC_CORE1 power domains, as this IP cannot
> communicate with the rest of the system (the AP) without local
> arbiter clocks being operational.
Somehow I feel like this is papering over some dependency issue in Linux.
> Fixes: 3b5838d1d82e ("arm64: dts: mt8195: Add iommu and smi nodes")
> Fixes: 2b515194bf0c ("arm64: dts: mt8195: Add power domains controller")
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
> arch/arm64/boot/dts/mediatek/mt8195.dtsi | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> index 95bd058d6083..5c670fce1e47 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> @@ -626,6 +626,8 @@ power-domain@MT8195_POWER_DOMAIN_VDEC1 {
>
> power-domain@MT8195_POWER_DOMAIN_VENC_CORE1 {
> reg = <MT8195_POWER_DOMAIN_VENC_CORE1>;
> + clocks = <&vencsys_core1 CLK_VENC_CORE1_LARB>;
> + clock-names = "venc1-larb";
> mediatek,infracfg = <&infracfg_ao>;
> #power-domain-cells = <0>;
> };
> @@ -688,6 +690,8 @@ power-domain@MT8195_POWER_DOMAIN_VDEC2 {
>
> power-domain@MT8195_POWER_DOMAIN_VENC {
> reg = <MT8195_POWER_DOMAIN_VENC>;
> + clocks = <&vencsys CLK_VENC_LARB>;
> + clock-names = "venc0-larb";
> mediatek,infracfg = <&infracfg_ao>;
> #power-domain-cells = <0>;
> };
> @@ -3094,7 +3098,7 @@ larb20: larb@1b010000 {
> reg = <0 0x1b010000 0 0x1000>;
> mediatek,larb-id = <20>;
> mediatek,smi = <&smi_common_vpp>;
> - clocks = <&vencsys_core1 CLK_VENC_CORE1_LARB>,
> + clocks = <&vencsys_core1 CLK_VENC_CORE1_VENC>,
> <&vencsys_core1 CLK_VENC_CORE1_GALS>,
> <&vppsys0 CLK_VPP0_GALS_VDO0_VDO1_VENCSYS_CORE1>;
> clock-names = "apb", "smi", "gals";
> --
> 2.40.1
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: dts: mediatek: mt8195: Fix PM suspend/resume with venc clocks
@ 2023-07-07 5:06 ` Chen-Yu Tsai
0 siblings, 0 replies; 8+ messages in thread
From: Chen-Yu Tsai @ 2023-07-07 5:06 UTC (permalink / raw)
To: AngeloGioacchino Del Regno
Cc: matthias.bgg, robh+dt, krzysztof.kozlowski+dt, conor+dt, yong.wu,
tinghan.shen, weiyi.lu, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek, kernel
On Thu, Jul 6, 2023 at 5:59 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Before suspending the LARBs we're making sure that any operation is
> done: this never happens because we are unexpectedly unclocking the
> LARB20 before executing the suspend handler for the MediaTek Smart
> Multimedia Interface (SMI) and the cause of this is incorrect clocks
> on this LARB.
>
> Fix this issue by changing the Local Arbiter 20 (used by the video
> encoder secondary core) apb clock to CLK_VENC_CORE1_VENC;
> furthermore, in order to make sure that both the PM resume and video
> encoder operation is stable, add the CLK_VENC(_CORE1)_LARB clock to
> the VENC (main core) and VENC_CORE1 power domains, as this IP cannot
> communicate with the rest of the system (the AP) without local
> arbiter clocks being operational.
Somehow I feel like this is papering over some dependency issue in Linux.
> Fixes: 3b5838d1d82e ("arm64: dts: mt8195: Add iommu and smi nodes")
> Fixes: 2b515194bf0c ("arm64: dts: mt8195: Add power domains controller")
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
> arch/arm64/boot/dts/mediatek/mt8195.dtsi | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> index 95bd058d6083..5c670fce1e47 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> @@ -626,6 +626,8 @@ power-domain@MT8195_POWER_DOMAIN_VDEC1 {
>
> power-domain@MT8195_POWER_DOMAIN_VENC_CORE1 {
> reg = <MT8195_POWER_DOMAIN_VENC_CORE1>;
> + clocks = <&vencsys_core1 CLK_VENC_CORE1_LARB>;
> + clock-names = "venc1-larb";
> mediatek,infracfg = <&infracfg_ao>;
> #power-domain-cells = <0>;
> };
> @@ -688,6 +690,8 @@ power-domain@MT8195_POWER_DOMAIN_VDEC2 {
>
> power-domain@MT8195_POWER_DOMAIN_VENC {
> reg = <MT8195_POWER_DOMAIN_VENC>;
> + clocks = <&vencsys CLK_VENC_LARB>;
> + clock-names = "venc0-larb";
> mediatek,infracfg = <&infracfg_ao>;
> #power-domain-cells = <0>;
> };
> @@ -3094,7 +3098,7 @@ larb20: larb@1b010000 {
> reg = <0 0x1b010000 0 0x1000>;
> mediatek,larb-id = <20>;
> mediatek,smi = <&smi_common_vpp>;
> - clocks = <&vencsys_core1 CLK_VENC_CORE1_LARB>,
> + clocks = <&vencsys_core1 CLK_VENC_CORE1_VENC>,
> <&vencsys_core1 CLK_VENC_CORE1_GALS>,
> <&vppsys0 CLK_VPP0_GALS_VDO0_VDO1_VENCSYS_CORE1>;
> clock-names = "apb", "smi", "gals";
> --
> 2.40.1
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: dts: mediatek: mt8195: Fix PM suspend/resume with venc clocks
2023-07-07 5:06 ` Chen-Yu Tsai
@ 2023-07-07 8:19 ` AngeloGioacchino Del Regno
-1 siblings, 0 replies; 8+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-07 8:19 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: matthias.bgg, robh+dt, krzysztof.kozlowski+dt, conor+dt, yong.wu,
tinghan.shen, weiyi.lu, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek, kernel
Il 07/07/23 07:06, Chen-Yu Tsai ha scritto:
> On Thu, Jul 6, 2023 at 5:59 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Before suspending the LARBs we're making sure that any operation is
>> done: this never happens because we are unexpectedly unclocking the
>> LARB20 before executing the suspend handler for the MediaTek Smart
>> Multimedia Interface (SMI) and the cause of this is incorrect clocks
>> on this LARB.
>>
>> Fix this issue by changing the Local Arbiter 20 (used by the video
>> encoder secondary core) apb clock to CLK_VENC_CORE1_VENC;
>> furthermore, in order to make sure that both the PM resume and video
>> encoder operation is stable, add the CLK_VENC(_CORE1)_LARB clock to
>> the VENC (main core) and VENC_CORE1 power domains, as this IP cannot
>> communicate with the rest of the system (the AP) without local
>> arbiter clocks being operational.
>
> Somehow I feel like this is papering over some dependency issue in Linux.
>
It felt the same here, but then, if you disable the video encoder driver entirely
(or even both enc/dec drivers), you'll still get issues with the LARB20 timing out
on the SLP_PROT_RDY check, as there's something in queue going through that larb,
probably from something done by the bootloader before booting Linux.
Note that I'm pointing my finger to the bootloader because - again - even disabling
the venc entirely produces the same issue, and if you disable probing the LARB20
you will anyway get sleep issues (wakes up immediately after going to sleep).
That said, there is another possible solution to that (but even then, I think that
we still need those clock assignments that I've done here), which is to implement
the SMI power domains (mtcmos...): we'd be saving *all smi registers*, resetting an
entire SMI ctx with a poweroff, powering back on and restoring all registers *but*
larb20... at least that's the only solution that I've seen downstream (android
kernels).
...and that's why I believe that this commit is correct.
Of course, if there's something that I'm underestimating here, I'd be glad to
understand.
Cheers,
Angelo
>> Fixes: 3b5838d1d82e ("arm64: dts: mt8195: Add iommu and smi nodes")
>> Fixes: 2b515194bf0c ("arm64: dts: mt8195: Add power domains controller")
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>> arch/arm64/boot/dts/mediatek/mt8195.dtsi | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>> index 95bd058d6083..5c670fce1e47 100644
>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>> @@ -626,6 +626,8 @@ power-domain@MT8195_POWER_DOMAIN_VDEC1 {
>>
>> power-domain@MT8195_POWER_DOMAIN_VENC_CORE1 {
>> reg = <MT8195_POWER_DOMAIN_VENC_CORE1>;
>> + clocks = <&vencsys_core1 CLK_VENC_CORE1_LARB>;
>> + clock-names = "venc1-larb";
>> mediatek,infracfg = <&infracfg_ao>;
>> #power-domain-cells = <0>;
>> };
>> @@ -688,6 +690,8 @@ power-domain@MT8195_POWER_DOMAIN_VDEC2 {
>>
>> power-domain@MT8195_POWER_DOMAIN_VENC {
>> reg = <MT8195_POWER_DOMAIN_VENC>;
>> + clocks = <&vencsys CLK_VENC_LARB>;
>> + clock-names = "venc0-larb";
>> mediatek,infracfg = <&infracfg_ao>;
>> #power-domain-cells = <0>;
>> };
>> @@ -3094,7 +3098,7 @@ larb20: larb@1b010000 {
>> reg = <0 0x1b010000 0 0x1000>;
>> mediatek,larb-id = <20>;
>> mediatek,smi = <&smi_common_vpp>;
>> - clocks = <&vencsys_core1 CLK_VENC_CORE1_LARB>,
>> + clocks = <&vencsys_core1 CLK_VENC_CORE1_VENC>,
>> <&vencsys_core1 CLK_VENC_CORE1_GALS>,
>> <&vppsys0 CLK_VPP0_GALS_VDO0_VDO1_VENCSYS_CORE1>;
>> clock-names = "apb", "smi", "gals";
>> --
>> 2.40.1
>>
>>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: dts: mediatek: mt8195: Fix PM suspend/resume with venc clocks
@ 2023-07-07 8:19 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 8+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-07 8:19 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: matthias.bgg, robh+dt, krzysztof.kozlowski+dt, conor+dt, yong.wu,
tinghan.shen, weiyi.lu, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek, kernel
Il 07/07/23 07:06, Chen-Yu Tsai ha scritto:
> On Thu, Jul 6, 2023 at 5:59 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Before suspending the LARBs we're making sure that any operation is
>> done: this never happens because we are unexpectedly unclocking the
>> LARB20 before executing the suspend handler for the MediaTek Smart
>> Multimedia Interface (SMI) and the cause of this is incorrect clocks
>> on this LARB.
>>
>> Fix this issue by changing the Local Arbiter 20 (used by the video
>> encoder secondary core) apb clock to CLK_VENC_CORE1_VENC;
>> furthermore, in order to make sure that both the PM resume and video
>> encoder operation is stable, add the CLK_VENC(_CORE1)_LARB clock to
>> the VENC (main core) and VENC_CORE1 power domains, as this IP cannot
>> communicate with the rest of the system (the AP) without local
>> arbiter clocks being operational.
>
> Somehow I feel like this is papering over some dependency issue in Linux.
>
It felt the same here, but then, if you disable the video encoder driver entirely
(or even both enc/dec drivers), you'll still get issues with the LARB20 timing out
on the SLP_PROT_RDY check, as there's something in queue going through that larb,
probably from something done by the bootloader before booting Linux.
Note that I'm pointing my finger to the bootloader because - again - even disabling
the venc entirely produces the same issue, and if you disable probing the LARB20
you will anyway get sleep issues (wakes up immediately after going to sleep).
That said, there is another possible solution to that (but even then, I think that
we still need those clock assignments that I've done here), which is to implement
the SMI power domains (mtcmos...): we'd be saving *all smi registers*, resetting an
entire SMI ctx with a poweroff, powering back on and restoring all registers *but*
larb20... at least that's the only solution that I've seen downstream (android
kernels).
...and that's why I believe that this commit is correct.
Of course, if there's something that I'm underestimating here, I'd be glad to
understand.
Cheers,
Angelo
>> Fixes: 3b5838d1d82e ("arm64: dts: mt8195: Add iommu and smi nodes")
>> Fixes: 2b515194bf0c ("arm64: dts: mt8195: Add power domains controller")
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>> arch/arm64/boot/dts/mediatek/mt8195.dtsi | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>> index 95bd058d6083..5c670fce1e47 100644
>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>> @@ -626,6 +626,8 @@ power-domain@MT8195_POWER_DOMAIN_VDEC1 {
>>
>> power-domain@MT8195_POWER_DOMAIN_VENC_CORE1 {
>> reg = <MT8195_POWER_DOMAIN_VENC_CORE1>;
>> + clocks = <&vencsys_core1 CLK_VENC_CORE1_LARB>;
>> + clock-names = "venc1-larb";
>> mediatek,infracfg = <&infracfg_ao>;
>> #power-domain-cells = <0>;
>> };
>> @@ -688,6 +690,8 @@ power-domain@MT8195_POWER_DOMAIN_VDEC2 {
>>
>> power-domain@MT8195_POWER_DOMAIN_VENC {
>> reg = <MT8195_POWER_DOMAIN_VENC>;
>> + clocks = <&vencsys CLK_VENC_LARB>;
>> + clock-names = "venc0-larb";
>> mediatek,infracfg = <&infracfg_ao>;
>> #power-domain-cells = <0>;
>> };
>> @@ -3094,7 +3098,7 @@ larb20: larb@1b010000 {
>> reg = <0 0x1b010000 0 0x1000>;
>> mediatek,larb-id = <20>;
>> mediatek,smi = <&smi_common_vpp>;
>> - clocks = <&vencsys_core1 CLK_VENC_CORE1_LARB>,
>> + clocks = <&vencsys_core1 CLK_VENC_CORE1_VENC>,
>> <&vencsys_core1 CLK_VENC_CORE1_GALS>,
>> <&vppsys0 CLK_VPP0_GALS_VDO0_VDO1_VENCSYS_CORE1>;
>> clock-names = "apb", "smi", "gals";
>> --
>> 2.40.1
>>
>>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-07-07 8:19 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-06 9:58 [PATCH] arm64: dts: mediatek: mt8195: Fix PM suspend/resume with venc clocks AngeloGioacchino Del Regno
2023-07-06 9:58 ` AngeloGioacchino Del Regno
2023-07-06 12:35 ` Alexandre Mergnat
2023-07-06 12:35 ` Alexandre Mergnat
2023-07-07 5:06 ` Chen-Yu Tsai
2023-07-07 5:06 ` Chen-Yu Tsai
2023-07-07 8:19 ` AngeloGioacchino Del Regno
2023-07-07 8:19 ` AngeloGioacchino Del Regno
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.