16:57 <lumag> sboyd, can we please try gaining your attention for a clk_disable_unused issue? https://gitlab.freedesktop.org/drm/msm/-/jobs/54990475 This happens on db410c (apq8016), if the DRM_MSM is built-in, but the adv7511, the DSI bridge, is built as a module.

16:57 <lumag> konradybcio, ^^

16:58 <konradybcio> (worth noting it's not exactly an issue with the clocks themselves being disabled :P, i like my unused cleanup..)

16:58 <lumag> konradybcio, huh?

16:59 <konradybcio> iirc there were some talks to get rid of it, i'm just preemptively defending the idea

17:01 <lumag> aj

17:01 <lumag> ah

17:15 <lumag> robclark, sboyd, konradybcio : I traced the issue to the dsi0byte_mux attempted disablement. It looks like the clock framework tries to disable the whole DSI clock tree, but then it stumbles upon the dsi driver itself, which attempts to resume to be able to shut this clock down.

17:29 <lumag> [ 13.258032] Disabling clk dsi0pllbyte

17:29 <lumag> [ 13.261973] msm-mdss 1a00000.display-subsystem: HERE: mdss_runtime_resume: 457

17:29 <lumag> [ 13.265411] msm-mdss 1a00000.display-subsystem: HERE: msm_mdss_enable: 233

17:29 <lumag> [ 13.272545] msm-mdss 1a00000.display-subsystem: HERE: msm_mdss_enable: 249

17:29 <lumag> [ 13.279625] msm-mdss 1a00000.display-subsystem: HERE: msm_mdss_enable: 256

17:29 <lumag> [ 13.288361] Disabling clk dsi0byte_mux

17:29 <lumag> [ 13.293507] msm-mdss 1a00000.display-subsystem: HERE: mdss_runtime_suspend: 448

17:29 <lumag> [ 13.296670] msm-mdss 1a00000.display-subsystem: HERE: msm_mdss_disable: 305

17:29 <lumag> [ 14.884502] wcn36xx: mac address: 02:00:c2:21:3c:38

17:30 <lumag> So, msm_mdss disabling MDSS clocks clashes with the clock subsystem trying to do its own work.

17:35 <sboyd> lumag: does it work to mark clks with the ignore unused flag?

17:38 <lumag> sboyd, which ones? low-level DSI clocks?

17:38 <lumag> or the pixel /byte output clocks?

17:38 <sboyd> lumag: yes something like that

17:39 <sboyd> disabling unused clks is entirely broken

18:02 <lumag> sboyd, guess, it doesn't help. The CLK_IGNORE_UNUSED flag is checked after clk_pm_runtime_get() returns

18:05 <sboyd> lumag: sounds like we can change that?

18:06 <lumag> sboyd, if we consider flags to be constant (which they probably are)

18:07 <sboyd> lumag: yes they all are

18:29 <lumag> sboyd, ok, it requires adding CLK_IGNORE_UNUSED to all 28nm clocks.

18:30 <lumag> This sounds like a pretty strange idea.

18:30 <lumag> Or like a hack.

19:17 <javierm> lumag: what I don't understand is why the clk is unused if the msm-mdss driver prepared/enabled it

19:18 <lumag> javierm, the DSI clocks become unused, because there is no DSI consumer (adv7511=m).

19:18 <lumag> So there is no video stream ongoing.

19:19 <javierm> lumag: but then the DSI consumer should prepare the clock on probe right ?

19:20 <lumag> javierm, only in bridge->pre_enable

19:21 <javierm> lumag: which may be OK, but why then the mdss runtime PM callbacks try to enable/disable that clock ?

19:22 <javierm> lumag: I think that the problem here is either that the dss driver is not grabbing all the clocks that needs to operate or their runtime PM count isn't correct

19:22 <javierm> I'm not familiar with the QC DSS IP block to know the answer but from a bird's eye POV that is what looks to me

19:23 <javierm> because as you said, some DSI clocks are left unused and the whole DSI sub-tree is disabled

19:23 <javierm> but then the dss runtime PM callbacks try to disable it again?

19:24 <lumag> I think the congestion is about a different clock.

19:24 <lumag> Let me debug that

19:24 <javierm> lumag: what usually happens is that the clk disable unused has some side effects because it disables the whole sub-tree as you said

19:25 <javierm> but sometimes, having a child clock (i.e: dsi), has the side effect of keeping enabled a clock that was needed by another IP block (e.g: dss)

19:25 <lumag> javierm, I think the whole clk_prepare_lock() is congested.

19:25 <javierm> but just as a by product of the child clock left enabled (and so its parent no being disabled due lock)

19:25 <javierm> lumag: oh, I see

19:25 <lumag> And that's why the lockdep doesn't prevent that

19:32 <lumag> I think, I'll abandon the CLK_IGNORE_UNUSED for now and just enforce clk_ignore_unused for the job.

19:33 <lumag> Hopefully we will get sync_state instead of clk_ignore_unused at some point.