| Message ID | ff4e2e34-a677-4c39-8c29-83655c5512ae@freebox.fr |
|---|---|
| State | Accepted |
| Commit | e20ae5ae9f0c843aded4f06f3d1cab7384789e92 |
| Headers | show |
| Series | [v3] clk: qcom: mmcc-msm8998: fix venus clock issue | expand |
On 29/04/2024 14:45, Marc Gonzalez wrote: > On 27/04/2024 21:34, Bjorn Andersson wrote: > >> On Thu, 25 Apr 2024 17:07:07 +0200, Marc Gonzalez wrote: >> >>> Right now, msm8998 video decoder (venus) is non-functional: >>> >>> $ time mpv --hwdec=v4l2m2m-copy --vd-lavc-software-fallback=no --vo=null --no-audio --untimed --length=30 --quiet demo-480.webm >>> (+) Video --vid=1 (*) (vp9 854x480 29.970fps) >>> Audio --aid=1 --alang=eng (*) (opus 2ch 48000Hz) >>> [ffmpeg/video] vp9_v4l2m2m: output VIDIOC_REQBUFS failed: Connection timed out >>> [ffmpeg/video] vp9_v4l2m2m: no v4l2 output context's buffers >>> [ffmpeg/video] vp9_v4l2m2m: can't configure decoder >>> Could not open codec. >>> Software decoding fallback is disabled. >>> Exiting... (Quit) >>> >>> [...] >> >> Applied, thanks! >> >> [1/1] clk: qcom: mmcc-msm8998: fix venus clock issue >> commit: e20ae5ae9f0c843aded4f06f3d1cab7384789e92 > > Yes! > > Going on a tangent. > > During my tests, I saw an unrelated error in the boot log: > > [ 10.404521] clk: Disabling unused clocks > [ 10.412141] ------------[ cut here ]------------ > [ 10.415538] vmem_ahb_clk status stuck at 'on' > [ 10.415570] WARNING: CPU: 0 PID: 1 at drivers/clk/qcom/clk-branch.c:87 clk_branch_toggle+0x160/0x178 > [ 10.424420] Modules linked in: > [ 10.433586] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.9.0-rc1-00027-g483ea571c987 #70 > [ 10.436478] Hardware name: Freebox Delta (DT) > [ 10.444356] pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--) > [ 10.448884] pc : clk_branch_toggle+0x160/0x178 > [ 10.455642] lr : clk_branch_toggle+0x160/0x178 > [ 10.460154] sp : ffff80008005bc40 > [ 10.464574] x29: ffff80008005bc40 x28: 0000000000000000 x27: ffff800082df9070 > [ 10.467982] x26: ffff800082d100b0 x25: ffff800082c57cb0 x24: ffff800082b23958 > [ 10.475100] x23: 0000000000000000 x22: 0000000000000000 x21: ffff8000833b6208 > [ 10.482218] x20: ffff80008072bbec x19: 0000000000000000 x18: ffffffffff00d218 > [ 10.489337] x17: ffff800083476aa8 x16: ffff800083476a38 x15: 0000000000000030 > [ 10.496454] x14: 0000000000000000 x13: ffff0000f5348000 x12: 000000000000086d > [ 10.503572] x11: 00000000000002cf x10: ffff0000f7f4c368 x9 : ffff0000f5348000 > [ 10.510692] x8 : 00000000fff7ffff x7 : ffff0000f7f48000 x6 : 00000000000002cf > [ 10.517809] x5 : 00000000005ffff4 x4 : 40000000fff802cf x3 : 0000000000000000 > [ 10.524926] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000080118000 > [ 10.532046] Call trace: > [ 10.539137] clk_branch_toggle+0x160/0x178 > [ 10.541402] clk_branch2_disable+0x1c/0x28 > [ 10.545569] clk_disable_unused_subtree+0xfc/0x138 > [ 10.549652] clk_disable_unused_subtree+0x2c/0x138 > [ 10.554427] clk_disable_unused_subtree+0x2c/0x138 > [ 10.559201] clk_disable_unused_subtree+0x2c/0x138 > [ 10.563975] clk_disable_unused_subtree+0x2c/0x138 > [ 10.568749] clk_disable_unused_subtree+0x2c/0x138 > [ 10.573525] clk_disable_unused_subtree+0x2c/0x138 > [ 10.578298] clk_disable_unused+0x50/0x138 > [ 10.583070] do_one_initcall+0x6c/0x1b0 > [ 10.587147] kernel_init_freeable+0x1d4/0x2cc > [ 10.590883] kernel_init+0x20/0x1d8 > [ 10.595391] ret_from_fork+0x10/0x20 > [ 10.598693] ---[ end trace 0000000000000000 ]--- Taking sm8250 as an example the vidoecc ahb clk is a candidate to be always-on. drivers/clk/qcom/videocc-sm8250.c qcom_branch_set_clk_en(regmap, 0xe58); /* VIDEO_CC_AHB_CLK */ --- bod
On 29/04/2024 15:52, Konrad Dybcio wrote: >>> >>> [ 10.404521] clk: Disabling unused clocks >>> [ 10.412141] ------------[ cut here ]------------ >>> [ 10.415538] vmem_ahb_clk status stuck at 'on' > > We currently don't support VMEM (which is a small SRAM dedicated to venus) > upstream, but venus functions without it (albeit I'd expect it to be > slower or not fully capable without it) Ah vmem right, indeed. Just try switching it off Marc, you probably don't need this clock. --- bod
On 29.04.2024 5:04 PM, Bryan O'Donoghue wrote: > On 29/04/2024 15:52, Konrad Dybcio wrote: >>>> >>>> [ 10.404521] clk: Disabling unused clocks >>>> [ 10.412141] ------------[ cut here ]------------ >>>> [ 10.415538] vmem_ahb_clk status stuck at 'on' >> >> We currently don't support VMEM (which is a small SRAM dedicated to venus) >> upstream, but venus functions without it (albeit I'd expect it to be >> slower or not fully capable without it) > > Ah vmem right, indeed. > > Just try switching it off Marc, you probably don't need this clock. ?????????? That's what the kernel is trying to do as part of the cleanup.. Either this clock is always-on or held through some hardware vote. Or the status bit is wrong. Just leave it be. Konrad
On 30/04/2024 11:00, Konrad Dybcio wrote: > ?????????? > > That's what the kernel is trying to do as part of the cleanup.. Either > this clock is always-on or held through some hardware vote. Or the status > bit is wrong. > > Just leave it be. > > Konrad That's not different to what I just said but, thanks for restating it. --- bod
diff --git a/drivers/clk/qcom/mmcc-msm8998.c b/drivers/clk/qcom/mmcc-msm8998.c index 1180e48c687ac..275fb3b71ede4 100644 --- a/drivers/clk/qcom/mmcc-msm8998.c +++ b/drivers/clk/qcom/mmcc-msm8998.c @@ -2535,6 +2535,8 @@ static struct clk_branch vmem_ahb_clk = { static struct gdsc video_top_gdsc = { .gdscr = 0x1024, + .cxcs = (unsigned int []){ 0x1028, 0x1034, 0x1038 }, + .cxc_count = 3, .pd = { .name = "video_top", }, @@ -2543,20 +2545,26 @@ static struct gdsc video_top_gdsc = { static struct gdsc video_subcore0_gdsc = { .gdscr = 0x1040, + .cxcs = (unsigned int []){ 0x1048 }, + .cxc_count = 1, .pd = { .name = "video_subcore0", }, .parent = &video_top_gdsc.pd, .pwrsts = PWRSTS_OFF_ON, + .flags = HW_CTRL, }; static struct gdsc video_subcore1_gdsc = { .gdscr = 0x1044, + .cxcs = (unsigned int []){ 0x104c }, + .cxc_count = 1, .pd = { .name = "video_subcore1", }, .parent = &video_top_gdsc.pd, .pwrsts = PWRSTS_OFF_ON, + .flags = HW_CTRL, }; static struct gdsc mdss_gdsc = {
Right now, msm8998 video decoder (venus) is non-functional: $ time mpv --hwdec=v4l2m2m-copy --vd-lavc-software-fallback=no --vo=null --no-audio --untimed --length=30 --quiet demo-480.webm (+) Video --vid=1 (*) (vp9 854x480 29.970fps) Audio --aid=1 --alang=eng (*) (opus 2ch 48000Hz) [ffmpeg/video] vp9_v4l2m2m: output VIDIOC_REQBUFS failed: Connection timed out [ffmpeg/video] vp9_v4l2m2m: no v4l2 output context's buffers [ffmpeg/video] vp9_v4l2m2m: can't configure decoder Could not open codec. Software decoding fallback is disabled. Exiting... (Quit) Bryan O'Donoghue suggested the proper fix: - Set required register offsets in venus GDSC structs. - Set HW_CTRL flag. $ time mpv --hwdec=v4l2m2m-copy --vd-lavc-software-fallback=no --vo=null --no-audio --untimed --length=30 --quiet demo-480.webm (+) Video --vid=1 (*) (vp9 854x480 29.970fps) Audio --aid=1 --alang=eng (*) (opus 2ch 48000Hz) [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl ... Using hardware decoding (v4l2m2m-copy). VO: [null] 854x480 nv12 Exiting... (End of file) real 0m3.315s user 0m1.277s sys 0m0.453s NOTES: GDSC = Globally Distributed Switch Controller Use same code as mmcc-msm8996 with: s/venus_gdsc/video_top_gdsc/ s/venus_core0_gdsc/video_subcore0_gdsc/ s/venus_core1_gdsc/video_subcore1_gdsc/ https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/include/dt-bindings/clock/msm-clocks-hwio-8996.h https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/include/dt-bindings/clock/msm-clocks-hwio-8998.h 0x1024 = MMSS_VIDEO GDSCR (undocumented) 0x1028 = MMSS_VIDEO_CORE_CBCR 0x1030 = MMSS_VIDEO_AHB_CBCR 0x1034 = MMSS_VIDEO_AXI_CBCR 0x1038 = MMSS_VIDEO_MAXI_CBCR 0x1040 = MMSS_VIDEO_SUBCORE0 GDSCR (undocumented) 0x1044 = MMSS_VIDEO_SUBCORE1 GDSCR (undocumented) 0x1048 = MMSS_VIDEO_SUBCORE0_CBCR 0x104c = MMSS_VIDEO_SUBCORE1_CBCR Fixes: d14b15b5931c2b ("clk: qcom: Add MSM8998 Multimedia Clock Controller (MMCC) driver") Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> --- Changes from v2 to v3: add mpv results and Fixes tag. --- drivers/clk/qcom/mmcc-msm8998.c | 8 ++++++++ 1 file changed, 8 insertions(+)