| Message ID | 20240925110044.3678055-6-fshao@chromium.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | MT8188 DT and binding fixes | expand |
Il 25/09/24 12:57, Fei Shao ha scritto: > There are two hardware IP blocks in MT8188 video decoder pipeline: > vdec-lat and vdec-core, which are powered by vdec0 and vdec1 power > domains respectively. > > We noticed that vdec-core needs to be powered down before vdec-lat > during suspend to prevent failures. It's unclear if it's an intended > hardware design or due to power isolation glitch. But in any case, we > observed a power-off sequence here, and it can be considered as an > indirect dependency implication between the vdec0 and vdec1 domains. > > Given that, update vdec1 as a sub-domain of vdec0 to enforce the > sequence. Also, use more specific clock names for both power domains. > As far as I know, yes, there is a sequence: - Cores (mtk-vcodec-core) gets suspended first - Then the LATs gets suspended (mtk-vcodec-lat) - Finally, the LAT SoC gets suspended (mtk-vcodec-lat-soc) ...but you checked that downstream, and your downstream misses the lat-soc HW instance, and only has the lat one. Are you sure that this is not the reason why you're getting this issue? :-) Otherwise, I feel like we must ask for some clarification from MediaTek, as I'm mostly sure that the two cores are independent from each other (but I might, of course, be wrong!). Cheers, Angelo > Signed-off-by: Fei Shao <fshao@chromium.org> > --- > > arch/arm64/boot/dts/mediatek/mt8188.dtsi | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) > > diff --git a/arch/arm64/boot/dts/mediatek/mt8188.dtsi b/arch/arm64/boot/dts/mediatek/mt8188.dtsi > index ff5c8e0597f9..a6cd08ea74eb 100644 > --- a/arch/arm64/boot/dts/mediatek/mt8188.dtsi > +++ b/arch/arm64/boot/dts/mediatek/mt8188.dtsi > @@ -1064,20 +1064,22 @@ power-domain@MT8188_POWER_DOMAIN_VPPSYS1 { > #power-domain-cells = <0>; > }; > > - power-domain@MT8188_POWER_DOMAIN_VDEC1 { > - reg = <MT8188_POWER_DOMAIN_VDEC1>; > - clocks = <&vdecsys CLK_VDEC2_LARB1>; > - clock-names = "ss-vdec"; > - mediatek,infracfg = <&infracfg_ao>; > - #power-domain-cells = <0>; > - }; > - > power-domain@MT8188_POWER_DOMAIN_VDEC0 { > reg = <MT8188_POWER_DOMAIN_VDEC0>; > clocks = <&vdecsys_soc CLK_VDEC1_SOC_LARB1>; > - clock-names = "ss-vdec"; > + clock-names = "ss-vdec1-soc-l1"; > mediatek,infracfg = <&infracfg_ao>; > - #power-domain-cells = <0>; > + #address-cells = <1>; > + #size-cells = <0>; > + #power-domain-cells = <1>; > + > + power-domain@MT8188_POWER_DOMAIN_VDEC1 { > + reg = <MT8188_POWER_DOMAIN_VDEC1>; > + clocks = <&vdecsys CLK_VDEC2_LARB1>; > + clock-names = "ss-vdec2-l1"; > + mediatek,infracfg = <&infracfg_ao>; > + #power-domain-cells = <0>; > + }; > }; > > cam_vcore: power-domain@MT8188_POWER_DOMAIN_CAM_VCORE {
On Thu, Sep 26, 2024 at 4:33 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > Il 25/09/24 12:57, Fei Shao ha scritto: > > There are two hardware IP blocks in MT8188 video decoder pipeline: > > vdec-lat and vdec-core, which are powered by vdec0 and vdec1 power > > domains respectively. > > > > We noticed that vdec-core needs to be powered down before vdec-lat > > during suspend to prevent failures. It's unclear if it's an intended > > hardware design or due to power isolation glitch. But in any case, we > > observed a power-off sequence here, and it can be considered as an > > indirect dependency implication between the vdec0 and vdec1 domains. > > > > Given that, update vdec1 as a sub-domain of vdec0 to enforce the > > sequence. Also, use more specific clock names for both power domains. > > > > As far as I know, yes, there is a sequence: > - Cores (mtk-vcodec-core) gets suspended first > - Then the LATs gets suspended (mtk-vcodec-lat) > - Finally, the LAT SoC gets suspended (mtk-vcodec-lat-soc) > > ...but you checked that downstream, and your downstream misses the lat-soc HW > instance, and only has the lat one. > > Are you sure that this is not the reason why you're getting this issue? :-) > > Otherwise, I feel like we must ask for some clarification from MediaTek, as > I'm mostly sure that the two cores are independent from each other (but I > might, of course, be wrong!). Yes I think I should... this is actually based on a downstream patch of theirs. My understanding is that LAT SoC is not always in the vdec pipeline for every MediaTek SoCs. Although the MT8188 and MT8195 have much in common, I have a vague impression that MT8188 doesn't have a LAT SoC HW, so the downstream video decoding works smoothly without describing that in DT... but still, I could be wrong, and things just happen to work. Anyway, I'll find someone on the MediaTek side for clarification. The datasheet I have doesn't seem to contain such information. Regards, Fei > > Cheers, > Angelo
On Thu, Sep 26, 2024 at 6:42 PM Fei Shao <fshao@chromium.org> wrote: > > On Thu, Sep 26, 2024 at 4:33 PM AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com> wrote: > > > > Il 25/09/24 12:57, Fei Shao ha scritto: > > > There are two hardware IP blocks in MT8188 video decoder pipeline: > > > vdec-lat and vdec-core, which are powered by vdec0 and vdec1 power > > > domains respectively. > > > > > > We noticed that vdec-core needs to be powered down before vdec-lat > > > during suspend to prevent failures. It's unclear if it's an intended > > > hardware design or due to power isolation glitch. But in any case, we > > > observed a power-off sequence here, and it can be considered as an > > > indirect dependency implication between the vdec0 and vdec1 domains. > > > > > > Given that, update vdec1 as a sub-domain of vdec0 to enforce the > > > sequence. Also, use more specific clock names for both power domains. > > > > > > > As far as I know, yes, there is a sequence: > > - Cores (mtk-vcodec-core) gets suspended first > > - Then the LATs gets suspended (mtk-vcodec-lat) > > - Finally, the LAT SoC gets suspended (mtk-vcodec-lat-soc) > > > > ...but you checked that downstream, and your downstream misses the lat-soc HW > > instance, and only has the lat one. > > > > Are you sure that this is not the reason why you're getting this issue? :-) > > > > Otherwise, I feel like we must ask for some clarification from MediaTek, as > > I'm mostly sure that the two cores are independent from each other (but I > > might, of course, be wrong!). > > Yes I think I should... this is actually based on a downstream patch of theirs. > My understanding is that LAT SoC is not always in the vdec pipeline > for every MediaTek SoCs. Although the MT8188 and MT8195 have much in > common, I have a vague impression that MT8188 doesn't have a LAT SoC > HW, so the downstream video decoding works smoothly without describing > that in DT... but still, I could be wrong, and things just happen to work. > > Anyway, I'll find someone on the MediaTek side for clarification. The > datasheet I have doesn't seem to contain such information. > MediaTek confirmed that MT8188 doesn't have a LAT SoC block. Its vdec pipeline is (mostly if not exactly) the same as MT8192 which is composed of Core + LAT only. Also, there *is* a hardware dependency between Core and LAT's power domains in both MT8192 and MT8188. And for reference, MT8192 DT described that dependency correctly. That suggests how the power domain tree should be like in MT8188 DT. I plan to send a v3 of this series to include more fixs I found last week and exclude the invalid patch, and I'll refine the commit message of this patch all together. I'll also update the bindings to document the information above so people can reference that easier in the future. Regards, Fei
diff --git a/arch/arm64/boot/dts/mediatek/mt8188.dtsi b/arch/arm64/boot/dts/mediatek/mt8188.dtsi index ff5c8e0597f9..a6cd08ea74eb 100644 --- a/arch/arm64/boot/dts/mediatek/mt8188.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8188.dtsi @@ -1064,20 +1064,22 @@ power-domain@MT8188_POWER_DOMAIN_VPPSYS1 { #power-domain-cells = <0>; }; - power-domain@MT8188_POWER_DOMAIN_VDEC1 { - reg = <MT8188_POWER_DOMAIN_VDEC1>; - clocks = <&vdecsys CLK_VDEC2_LARB1>; - clock-names = "ss-vdec"; - mediatek,infracfg = <&infracfg_ao>; - #power-domain-cells = <0>; - }; - power-domain@MT8188_POWER_DOMAIN_VDEC0 { reg = <MT8188_POWER_DOMAIN_VDEC0>; clocks = <&vdecsys_soc CLK_VDEC1_SOC_LARB1>; - clock-names = "ss-vdec"; + clock-names = "ss-vdec1-soc-l1"; mediatek,infracfg = <&infracfg_ao>; - #power-domain-cells = <0>; + #address-cells = <1>; + #size-cells = <0>; + #power-domain-cells = <1>; + + power-domain@MT8188_POWER_DOMAIN_VDEC1 { + reg = <MT8188_POWER_DOMAIN_VDEC1>; + clocks = <&vdecsys CLK_VDEC2_LARB1>; + clock-names = "ss-vdec2-l1"; + mediatek,infracfg = <&infracfg_ao>; + #power-domain-cells = <0>; + }; }; cam_vcore: power-domain@MT8188_POWER_DOMAIN_CAM_VCORE {
There are two hardware IP blocks in MT8188 video decoder pipeline: vdec-lat and vdec-core, which are powered by vdec0 and vdec1 power domains respectively. We noticed that vdec-core needs to be powered down before vdec-lat during suspend to prevent failures. It's unclear if it's an intended hardware design or due to power isolation glitch. But in any case, we observed a power-off sequence here, and it can be considered as an indirect dependency implication between the vdec0 and vdec1 domains. Given that, update vdec1 as a sub-domain of vdec0 to enforce the sequence. Also, use more specific clock names for both power domains. Signed-off-by: Fei Shao <fshao@chromium.org> --- arch/arm64/boot/dts/mediatek/mt8188.dtsi | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)