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

[Jassi Brar]

On Wed, Jul 3, 2024 at 11:41 AM Jason-JH Lin (林睿祥)
<Jason-JH.Lin at mediatek.com> wrote:
>
> On Thu, 2024-06-27 at 22:40 -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 26, 2024 at 4:32 AM Jason-JH Lin (林睿祥)
> > <Jason-JH.Lin at mediatek.com> wrote:
> > > >
> > > > The idea is that
> > > > 1) If the gap between transfers on a busy channel is ~10ms or
> > > > more.  And ..
> > > > 2) The silence on the mailbox channel is multiple of 100ms. And
> > ...
> > >
> > > The mailbox channel would not be always busy and the gap is not
> > > consistent as well.
> > > For example, display driver calls mbox_send_message() every ~16ms
> > while
> > > UI is updating in 60fps.
> > > If UI dose not update, display driver won't call
> > mbox_send_message(),
> > > so user may not has the gap and the silence like this.
> > >
> > From your logs, send_diff only increases from 16ms if the UI is not
> > updating. Which makes it more prudent to release the channel.
> >
> > The user needs the channel for mbox_send_message, while actual power
> > saving comes from cmdq_runtime_suspend/resume.
> > So for your target usage pattern, trace mbox_send_message() ,
> > cmdq_runtime_resume() and cmdq_runtime_suspend() and compare the
> > timestamped logs with and without this patchset.
> >
> [snip]
>
> I realized that we have 2 cmdq drivers:
> display uses 10320000.mailbox and imgsys uses 10330000.mailbox
>
You may disable one to make it easy to capture.
Make sure your kernel prints have timestamps.
Over your patchset, apply the following diff and execute your usecase.
Then share
 $ dmesg | grep Jason

diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index bafcc7b0c0b8d..90c0620c0ae63 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -284,6 +284,7 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg)
 {
  int t;

+ printk("Jason %s: %p\n", __func__, (void *)chan);
  if (!chan || !chan->cl)
  return -EINVAL;

diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c
b/drivers/mailbox/mtk-cmdq-mailbox.c
index 02cef3eee35a5..6436e1b22f353 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -317,6 +317,7 @@ static int cmdq_runtime_resume(struct device *dev)
 {
  struct cmdq *cmdq = dev_get_drvdata(dev);

+ printk("Jason %s: %p\n", __func__, (void *)dev);
  return clk_bulk_enable(cmdq->pdata->gce_num, cmdq->clocks);
 }

@@ -324,6 +325,7 @@ static int cmdq_runtime_suspend(struct device *dev)
 {
  struct cmdq *cmdq = dev_get_drvdata(dev);

+ printk("Jason %s: %p\n", __func__, (void *)dev);
  clk_bulk_disable(cmdq->pdata->gce_num, cmdq->clocks);
  return 0;
 }

Thanks