linux-kernel - Re: [PATCH v2] mailbox: remove runtime GCE clk control

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 11 Oct 2023 03:03:06 +0000
From:   Jason-JH Lin (林睿祥) 
        <Jason-JH.Lin@...iatek.com>
To:     "jassisinghbrar@...il.com" <jassisinghbrar@...il.com>,
        "angelogioacchino.delregno@...labora.com" 
        <angelogioacchino.delregno@...labora.com>,
        "chunkuang.hu@...nel.org" <chunkuang.hu@...nel.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        Singo Chang (張興國) 
        <Singo.Chang@...iatek.com>,
        Johnson Wang (王聖鑫) 
        <Johnson.Wang@...iatek.com>,
        Jason-ch Chen (陳建豪) 
        <Jason-ch.Chen@...iatek.com>,
        Shawn Sung (宋孝謙) 
        <Shawn.Sung@...iatek.com>,
        Nancy Lin (林欣螢) <Nancy.Lin@...iatek.com>,
        Project_Global_Chrome_Upstream_Group 
        <Project_Global_Chrome_Upstream_Group@...iatek.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "matthias.bgg@...il.com" <matthias.bgg@...il.com>
Subject: Re: [PATCH v2] mailbox: remove runtime GCE clk control

On Tue, 2023-10-10 at 13:52 +0200, AngeloGioacchino Del Regno wrote:
> Il 06/10/23 11:15, Jason-JH Lin (林睿祥) ha scritto:
> > Hi Angelo,
> > 
> > Thanks for the reviews.
> > 
> > On Wed, 2023-10-04 at 11:20 +0200, AngeloGioacchino Del Regno
> > wrote:
> > > Il 04/10/23 10:54, Jason-JH.Lin ha scritto:
> > > > 1. GCE is a frequently used module, so runtime controlling
> > > > GCE clock won't save too much power and its original design
> > > > doesn't expect it to be enabled and disabled too frequently.
> > > > 
> > > > 2. Runtime controlling GCE clock will cause display HW register
> > > > configured in worng stream done event issue below:
> > > >     GCE should config HW in every vblanking duration.
> > > >     The stream done event is the start signal of vblanking.
> > > > 
> > > >     If stream done event is sent between GCE clk_disable
> > > >     and clk_enable. After GCE clk_enable the stream done event
> > > >     may not appear immediately and have about 3us delay.
> > > > 
> > > >     Normal case:
> > > >     clk_disable -> get EventA -> clk_enable -> clear EventA
> > > >     -> wait EventB -> get EventB -> config HW
> > > > 
> > > >     Abnormal case:
> > > >     clk_disable -> get EventA -> clk_enable -> EventA delay
> > > > appear
> > > >     -> clear EventA fail -> wait EventB but get EventA ->
> > > > config HW
> > > > 
> > > > So just remove the runtime GCE clock contorl.
> > > > 
> > > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@...iatek.com>
> > > 
> > > Instead of entirely removing the logic that controls the clocks
> > > and
> > > always
> > > refuse to save power, what about using autosuspend?
> > > 
> > > If the two cases that you're describing are happening always in a
> > > range of
> > > time, we could *yes* remove the "manual" bulk disable/enable
> > > calls,
> > > but then
> > > we could use runtime_suspend/runtime_resume callbacks for that.
> > > 
> > > Hint: pm_runtime_set_autosuspend_delay(dev, 1000);
> > > 
> > > Regards,
> > > Angelo
> > > 
> > 
> > These 2 issues are caused by GCE bulk_clk enable/disable too
> > frequently.
> > 
> > As I now, pm_runtime_set_autosuspend_delay() is for controlling the
> > power domain. The power domain of GCE is infrasys which can only be
> > enabled/disabled by spm during the whole system resume/suspend.
> > So I'm not sure about how can pm_runtime_set_autosuspend_delay()
> > save
> > power for GCE bulk_clk in this case.
> > 
> > Could you give more hint for me please?
> > 
> 
> I think it's faster if I send my own version of that, I'm testing
> that right now
> on multiple Chromebooks to check if this solves the issue that you're
> describing,
> which I believe it to be the "apparent random lockups" or display
> stuttering when
> in a high load situation.

Thanks for your help, I can reproduce it with your version.

> 
> I don't seem to get any more stuttering nor apparent random lockups
> on a MT8195
> Chromebook, and that's with my pm_runtime autosuspend solution, with
> a runtime
> suspend delay of 100ms, which I'm trying to decrease as much as
> possible in order
> to keep saving as much power as possible.
> 
> For this, if you could better describe how to reliably reproduce the
> issue that
> you have described, it would help me a bit in making this as good as
> possible.

After reverting this modification, you'll see 2 issues on MT8188 and
MT8195 probability:
1. A flicker shift may occur randomly when moving the cursor on the
edge of external DP monitor rapidly.
2. A premature wake caused by CMDQ during the Suspend/Resume test
randomly.

I'll send you the detail reproduce steps privately.

Regards,
Jason-JH.Lin

> 
> Thanks,
> Angelo
>