Re: [PATCH 2/2] mailbox: mtk-cmdq: Move pm_runimte_get and put to mbox_chan_ops API

["Jason-JH Lin (林睿祥)" <Jason-JH.Lin@mediatek.com>]

On Wed, 2024-06-19 at 10:38 -0500, Jassi Brar wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On Wed, Jun 19, 2024 at 3:18 AM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
> > Il 18/06/24 17:59, Jassi Brar ha scritto:
> .....
> 
> > For example, when static content is displayed on screen, the CMDQ
> mailbox never
> > gets shut down, but no communication happens for a relatively long
> time; the
> > overhead of actually shutting down the mailbox and setting it back
> up would be
> > increasing latency in an unacceptable manner.
> >
> Hmm...  in your driver,  startup() is _empty_   and  shutdown() is
> all
> under a spin-lock with irqs disabled, so that too shouldn't be
> expensive. Right?
> Then what causes unacceptable latencies?
> 

I found that the BUG report only occurred when opening the camera APP.
Maybe something in imgsys_cmdq_sendtask() is too expensive or somewhere
else in imgsys driver.

I'm wondering why this BUG report is not occurred in display use case
which is more frequent than imgsys use case.
Does this mean sleep() is not always called in pm_runtime_get_sync()
and if we can guarantee this sleep() won't be called, then using
pm_runtime_get_sync() in atomic context is OK?

> > This is why I opted for autosuspend - it's only bringing down
> certain clocks for
> > the CMDQ HW, adding up a bit of power saving to the mix which, for
> some use cases
> > such as mobile devices with relatively small batteries, is
> definitely important.
> >
> > I'll also briefly (and only briefly) mention that 120Hz displays
> are already a
> > common thing and in this case the gap between TX and ACK is ~8.33ms
> instead, let
> > alone that displays with a framerate of more than 120Hz also do
> exist even though
> > they're less common.
> >
> I don't know how even busier channels help your point.
> 
> > All of the above describes a few of the reasons why autosuspend is
> a good choice
> > here, instead of a shutdown->startup flow.
> > And again - I can place some bets that PM would also be applicable
> to SoCs from
> > other vendors as well, with most probably different benefits (but
> still with some
> > power related benefits!) compared to MediaTek.

I agree with Angelo's point!


Regard,
Jason-JH Lin

> >
> Sure, if some platform _actually_ has a high channel
> startup()/shutdown() cost, it may need finer PM control. And I have a
> way for that, but even with that I am sure someone from MTK will
> realize they didn't need that.
> 
> Thanks.
>