Date:   Fri, 10 Nov 2023 11:09:29 +0000
From:   Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To:     Jagadeesh Kona <quic_jkona@...cinc.com>,
        Abel Vesa <abel.vesa@...aro.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Kevin Hilman <khilman@...nel.org>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Pavel Machek <pavel@....cz>, Len Brown <len.brown@...el.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Andy Gross <agross@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Stanimir Varbanov <stanimir.k.varbanov@...il.com>,
        Vikash Garodia <quic_vgarodia@...cinc.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>
Cc:     Taniya Das <tdas@....qualcomm.com>, linux-pm@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-clk@...r.kernel.org, linux-media@...r.kernel.org
Subject: Re: [PATCH RESEND v3 4/5] clk: qcom: Use HW_CTRL_TRIGGER flag to
 switch video GDSC to HW mode

On 10/11/2023 08:29, Jagadeesh Kona wrote:
> 
> 
> On 11/7/2023 6:35 PM, Bryan O'Donoghue wrote:
>> On 01/11/2023 09:04, Abel Vesa wrote:
>>> From: Jagadeesh Kona <quic_jkona@...cinc.com>
>>>
>>> The current HW_CTRL flag switches the video GDSC to HW control mode as
>>> part of GDSC enable itself, instead of that use HW_CTRL_TRIGGER flag to
>>> give consumer drivers more control and switch the GDSC mode as and when
>>> required.
>>>
>>> HW_CTRL_TRIGGER flag allows consumer drivers to switch the video GDSC to
>>> HW/SW control modes at runtime using dev_pm_genpd_set_hwmode API.
>>>
>>> Signed-off-by: Jagadeesh Kona <quic_jkona@...cinc.com>
>>> Signed-off-by: Abel Vesa <abel.vesa@...aro.org>
>>> ---
>>>   drivers/clk/qcom/videocc-sc7180.c | 2 +-
>>>   drivers/clk/qcom/videocc-sc7280.c | 2 +-
>>>   drivers/clk/qcom/videocc-sdm845.c | 4 ++--
>>>   drivers/clk/qcom/videocc-sm8250.c | 4 ++--
>>>   drivers/clk/qcom/videocc-sm8550.c | 4 ++--
>>>   5 files changed, 8 insertions(+), 8 deletions(-)
>>
>> So.
>>
>> I'm assuming the rest of this series works however for sc8250 at least 
>> this is a NAK, breaks venus on rb5.
>>
>> Reproduction:
>>
>> - Debian trixie rb5
>> - Linux 6.6-rc3 + this patch
>> - Attached defconfig
>> - This file:
>>    https://download.samplelib.com/mp4/sample-30s.mp4
>> - This command:
>>    ffplay -loglevel debug -code:video h264_v4l2m2m -i sample-30s.mp4
>>
>> Second play of file shows stuck gdsc as below
>>
>> I didn't try on rb3, I'd expect similar results. Does this work on 8550 ?
>>
>> [ 1601.581204] ------------[ cut here ]------------
>> [ 1601.585983] mvs0_gdsc status stuck at 'off'
>> [ 1601.586015] WARNING: CPU: 1 PID: 13372 at 
>> drivers/clk/qcom/gdsc.c:178 gdsc_toggle_logic+0x16c/0x174
>> [ 1601.599627] Modules linked in: nf_tables libcrc32c nfnetlink 
>> snd_soc_hdmi_codec q6asm_dai q6routing q6afe_dai q6asm q6adm 
>> q6afe_clocks snd_q6dsp_common q6afe q6core apr pdr_interface venus_enc 
>> venus_dec qcom_camss videobuf2_dma_contig mcp251xfd imx412 
>> videobuf2_dma_sg venus_core xhci_plat_hcd v4l2_fwnode fastrpc xhci_hcd 
>> can_dev qrtr_smd lontium_lt9611uxc msm v4l2_async v4l2_mem2mem 
>> qcom_spmi_adc_tm5 rtc_pm8xxx qcom_spmi_adc5 qcom_pon videobuf2_memops 
>> crct10dif_ce qcom_spmi_temp_alarm videobuf2_v4l2 qcom_vadc_common 
>> gpu_sched drm_dp_aux_bus videodev snd_soc_sm8250 drm_display_helper 
>> snd_soc_qcom_sdw videobuf2_common snd_soc_qcom_common qrtr 
>> i2c_qcom_cci soundwire_qcom mc i2c_qcom_geni spi_geni_qcom 
>> phy_qcom_qmp_combo qcom_q6v5_pas qcom_rng soundwire_bus qcom_pil_info 
>> snd_soc_lpass_va_macro qcom_q6v5 slimbus snd_soc_lpass_macro_common 
>> qcom_sysmon snd_soc_lpass_wsa_macro display_connector qcom_common 
>> socinfo qcom_glink_smem qmi_helpers drm_kms_helper mdt_loader qcom_wdt 
>> icc_osm_l3 qnoc_sm8250 fuse drm backlight dm_mod
>> [ 1601.599859]  ip_tables x_tables
>> [ 1601.695314] CPU: 1 PID: 13372 Comm: video_decoder Not tainted 
>> 6.6.0-rc3-00396-gdbc0d9fa7641-dirty #1
>> [ 1601.704694] Hardware name: Qualcomm Technologies, Inc. Robotics RB5 
>> (DT)
>> [ 1601.711582] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS 
>> BTYPE=--)
>> [ 1601.718740] pc : gdsc_toggle_logic+0x16c/0x174
>> [ 1601.723317] lr : gdsc_toggle_logic+0x16c/0x174
>> [ 1601.727888] sp : ffff80008adab800
>> [ 1601.731296] x29: ffff80008adab800 x28: ffffb661e8596210 x27: 
>> ffffb661e855ad88
>> [ 1601.738629] x26: 0000000000000000 x25: ffff100c0b5a0d28 x24: 
>> ffffb6620fd92118
>> [ 1601.745960] x23: ffffb6620fe1d3d8 x22: 0000000000000000 x21: 
>> 0000000000000001
>> [ 1601.753292] x20: 00000000ffffff92 x19: ffffb6620fd92118 x18: 
>> ffffffffffc0d3e8
>> [ 1601.760631] x17: 0000000000000000 x16: ffffb6620e269e14 x15: 
>> 0000000000000028
>> [ 1601.767973] x14: 0000000000000000 x13: ffff100d75c00000 x12: 
>> 0000000000000894
>> [ 1601.775304] x11: 00000000000002dc x10: ffff100d767044a0 x9 : 
>> ffff100d75c00000
>> [ 1601.782636] x8 : 00000000fffdffff x7 : ffff100d76700000 x6 : 
>> 00000000000002dc
>> [ 1601.789976] x5 : ffff100d7ef40d48 x4 : 40000000fffe02dc x3 : 
>> ffff59ab6fa21000
>> [ 1601.797306] x2 : 0000000000000000 x1 : 0000000000000000 x0 : 
>> ffff100cdacdec80
>> [ 1601.804638] Call trace:
>> [ 1601.807161]  gdsc_toggle_logic+0x16c/0x174
>> [ 1601.811383]  gdsc_enable+0x60/0x27c
>> [ 1601.814982]  _genpd_power_on+0x94/0x184
>> [ 1601.818931]  genpd_power_on+0xa8/0x16c
>> [ 1601.822791]  genpd_runtime_resume+0xd4/0x2a4
>> [ 1601.827184]  __rpm_callback+0x48/0x1dc
>> [ 1601.831045]  rpm_callback+0x6c/0x78
>> [ 1601.834638]  rpm_resume+0x45c/0x6c8
>> [ 1601.838231]  __pm_runtime_resume+0x4c/0x90
>> [ 1601.842443]  coreid_power_v4+0x378/0x58c [venus_core]
>> [ 1601.847695]  vdec_start_streaming+0xc0/0x4e8 [venus_dec]
>> [ 1601.853169]  vb2_start_streaming+0x68/0x15c [videobuf2_common]
>> [ 1601.859199]  vb2_core_streamon+0xf8/0x1bc [videobuf2_common]
>> [ 1601.865032]  vb2_streamon+0x18/0x64 [videobuf2_v4l2]
>> [ 1601.870174]  v4l2_m2m_ioctl_streamon+0x38/0x98 [v4l2_mem2mem]
>> [ 1601.876134]  v4l_streamon+0x24/0x30 [videodev]
>> [ 1601.880759]  __video_do_ioctl+0x15c/0x3c0 [videodev]
>> [ 1601.885905]  video_usercopy+0x1f0/0x658 [videodev]
>> [ 1601.890868]  video_ioctl2+0x18/0x28 [videodev]
>> [ 1601.895481]  v4l2_ioctl+0x40/0x60 [videodev]
>> [ 1601.899911]  __arm64_sys_ioctl+0xac/0xf0
>> [ 1601.903958]  invoke_syscall+0x48/0x114
>> [ 1601.907829]  el0_svc_common.constprop.0+0x40/0xe0
>> [ 1601.912672]  do_el0_svc+0x1c/0x28
>> [ 1601.916085]  el0_svc+0x40/0xe8
>> [ 1601.919243]  el0t_64_sync_handler+0x100/0x12c
>> [ 1601.923730]  el0t_64_sync+0x190/0x194
>> [ 1601.927505] ---[ end trace 0000000000000000 ]---
>> [ 1608.121533] ------------[ cut here ]------------
>>
>> And just reverting the one patch - yields
>>
>> [  157.083287] qcom-venus aa00000.video-codec: Failed to switch 
>> power-domain:1 to SW mode
>> [  162.004630] qcom-venus aa00000.video-codec: Failed to switch 
>> power-domain:1 to SW mode
>>
>> I'll leave the testing here. Please fix !
>>
> 
> Thanks Bryan for reporting this, this could be happening since GDSC 
> might be left in HW control mode during power off sequence, so the 
> subsequent GDSC enable is failing since GDSC is still in HW mode. I am 
> checking internally on this and will get back.

Great,

Please remember to check for rb3 / sdm845 too.

---
bod

Powered by blists - more mailing lists