Hi Sricharan, On 01/10/2017 09:29 PM, Sricharan wrote: > Hi stan, > >> -----Original Message----- >> From: linux-arm-msm-owner@xxxxxxxxxxxxxxx [mailto:linux-arm-msm-owner@xxxxxxxxxxxxxxx] On Behalf Of Stanimir Varbanov >> Sent: Tuesday, January 10, 2017 10:14 PM >> To: Rajendra Nayak <rnayak@xxxxxxxxxxxxxx>; sboyd@xxxxxxxxxxxxxx; mturquette@xxxxxxxxxxxx >> Cc: linux-clk@xxxxxxxxxxxxxxx; linux-arm-msm@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; sricharan@xxxxxxxxxxxxxx >> Subject: Re: [PATCH] clk: qcom: gdsc: Fix handling of hw control enable/disable >> >> Hi Rajendra, >> >> On 01/10/2017 07:54 AM, Rajendra Nayak wrote: >>> Once a gdsc is brought in and out of HW control, there is a >>> power down and up cycle which can take upto 1us. Polling on >>> the gdsc status immediately after the hw control enable/disable >>> can mislead software/firmware to belive the gdsc is already either on >>> or off, while its yet to complete the power cycle. >>> To avoid this add a 1us delay post a enable/disable of HW control >>> mode. >>> >>> Also after the HW control mode is disabled, poll on the status to >>> check gdsc status reflects its 'on' before force disabling it >>> in software. >>> >>> Reported-by: Stanimir Varbanov <stanimir.varbanov@xxxxxxxxxx> >>> Signed-off-by: Rajendra Nayak <rnayak@xxxxxxxxxxxxxx> >>> --- >>> >>> Stan, >>> If there was a specific issue you saw with venus because of the missing >>> delay and poll, can you check if this fixes any of that. >> >> Something more about the issue. >> >> I had re-designed venus driver on three platform drivers venus-core, >> venus-dec and venus-enc in order to be able to control those three >> power-domains (VENUS_GDSC, VENUS_CORE0_GDSC and VENUS_CORE1_GDSC). >> >> After that I abstracted MMAGIC hw on a separate mmagic driver. This >> driver just controls mmagic clocks and GDSC in its runtime_suspend and >> runtime_resume methods. >> >> The DT nodes looks like: >> >> mmagic_video { >> compatible = "qcom,msm8996-mmagic"; >> clocks = <&rpmcc MSM8996_RPM_SMD_MMAXI_CLK>, >> <&mmcc MMSS_MMAGIC_AHB_CLK>, >> <&mmcc MMSS_MMAGIC_CFG_AHB_CLK>, >> <&mmcc MMAGIC_VIDEO_NOC_CFG_AHB_CLK>, >> <&mmcc MMSS_MMAGIC_MAXI_CLK>, >> <&mmcc MMAGIC_VIDEO_AXI_CLK>, >> <&mmcc SMMU_VIDEO_AHB_CLK>, >> <&mmcc SMMU_VIDEO_AXI_CLK>; >> power-domains = <&mmcc MMAGIC_VIDEO_GDSC>; >> >> video-codec { >> compatible = "qcom,msm8996-venus"; >> clocks = <&mmcc VIDEO_CORE_CLK>, >> <&mmcc VIDEO_AHB_CLK>, >> <&mmcc VIDEO_AXI_CLK>, >> <&mmcc VIDEO_MAXI_CLK>; >> power-domains = <&mmcc VENUS_GDSC>; >> ... >> >> video-decoder { >> compatible = "venus-decoder"; >> clocks = "subcore0"; >> clock-names = <&mmcc VIDEO_SUBCORE0_CLK>; >> power-domains = <&mmcc VENUS_CORE0_GDSC>; >> }; >> >> video-encoder { >> compatible = "venus-encoder"; >> clocks = "subcore1"; >> clock-names = <&mmcc VIDEO_SUBCORE1_CLK>; >> power-domains = <&mmcc VENUS_CORE1_GDSC>; >> }; >> }; >> }; >> >> Note that mmagic_video is a parent dt node for smmu_video DT node so >> that clocks and mmagic_video gdsc will be enabled once smmu driver is >> instantiated by venus-core diriver. >> > > mmagic_video is a parent DT for smmu_video ? , so there are no clocks > populated for the smmu node as such ? Yes, I completely disabled runtime pm in the arm-smmu driver. > >> Now when video-dec driver calls pm_runtime_get_sync() the sequence of >> enabling is: >> >> MMAGIC_VIDEO_GDSC -> MMAGIC clocks and SMMU clocks -> VENUS_GDSC -> >> VIDEO clocks -> VENUS_CORE0_GDSC -> VIDEO subcore0 clock >> >> When video-dec platform driver calls pm_runtime_put_sync() we should >> disabling of GDSC and clocks in the reversed oder. >> >> The issue comes when I have ran video decoder, the decoder hw finish >> stream decoding and we want to suspend venus core. The issue is that >> when I start disabling SMMU_VIDEO_AXI_CLK and/or MMAGIC_VIDEO_AXI_CLK >> the system reboots. >> >> I have added a delay (200ms) before disabling mmagic clocks and then >> everything is fine again. >> >> Any idea? >> > > Can you share me a branch, i can have a quick check with a t32 > if there is any crash logged in the TZ buffer when the system reboots. I can share a branch but you will need my initramfs too. I don't think it is tz related, most probably it is MMAGIC sequence issue or something in GDSC/MMCC. -- regards, Stan -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
