MIME-Version: 1.0
References: <20211117064158.27451-1-jason-jh.lin@mediatek.com> <20211117064158.27451-3-jason-jh.lin@mediatek.com>
In-Reply-To: <20211117064158.27451-3-jason-jh.lin@mediatek.com>
From:   Chun-Kuang Hu <chunkuang.hu@kernel.org>
Date:   Fri, 19 Nov 2021 08:01:46 +0800
Message-ID: <CAAOTY_-Ws7BmYr-87rb=MWYyGwSCCvv0AoPV10J7d+7TU+7OCA@mail.gmail.com>
Subject: Re: [PATCH 2/3] mailbox: add cmdq_mbox_flush to clear all task before suspend
To:     "jason-jh.lin" <jason-jh.lin@mediatek.com>
Cc:     Chun-Kuang Hu <chunkuang.hu@kernel.org>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        Jassi Brar <jassisinghbrar@gmail.com>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>,
        DRI Development <dri-devel@lists.freedesktop.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@lists.infradead.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Hsin-Yi Wang <hsinyi@chromium.org>,
        Fei Shao <fshao@chromium.org>, tzungbi@google.com,
        Nancy Lin <nancy.lin@mediatek.com>, singo.chang@mediatek.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi, Jason:

jason-jh.lin <jason-jh.lin@mediatek.com> =E6=96=BC 2021=E5=B9=B411=E6=9C=88=
17=E6=97=A5 =E9=80=B1=E4=B8=89 =E4=B8=8B=E5=8D=882:42=E5=AF=AB=E9=81=93=EF=
=BC=9A
>
> CMDQ driver will occupy GCE clock to execute the task in GCE thread.
>
> So call cmdq_mbox_flush to clear all task in GCE thread before
> CMDQ suspend.
>
> Signed-off-by: jason-jh.lin <jason-jh.lin@mediatek.com>
> ---
>  drivers/mailbox/mtk-cmdq-mailbox.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmd=
q-mailbox.c
> index 03f9ed4c5131..28cadfc0091b 100644
> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> @@ -484,21 +484,18 @@ static int cmdq_suspend(struct device *dev)
>         struct cmdq *cmdq =3D dev_get_drvdata(dev);
>         struct cmdq_thread *thread;
>         int i;
> -       bool task_running =3D false;
>
>         cmdq->suspended =3D true;
>
>         for (i =3D 0; i < cmdq->thread_nr; i++) {
>                 thread =3D &cmdq->thread[i];
>                 if (!list_empty(&thread->task_busy_list)) {
> -                       task_running =3D true;
> -                       break;
> +                       /* try to clear all task in this thread */
> +                       cmdq_mbox_flush(thread->chan, 2000);

I would like the normal control flow rather than error handling. So
the normal control flow is:

1. Client driver suspend: Flush command.
2. CMDQ driver suspend: There is no command to flush. If there are
command, show error message and debug the client driver.

The error handling flow:

1. Client driver suspend: Does not flush command.
2. CMDQ driver suspend: Flush command and callback to client driver.
Client driver process these callback as error handling.

The client driver may integrate multiple driver. In the suspend flow,
it may need to stop these driver in a sequence such as.

1. Stop driver 1
2. Stop driver 2
3. Stop driver 3 (cmdq)
4. Stop driver 4
5. Stop driver 5.

In the normal flow, client driver could control the stop flow. In the
error handling flow, it does not match the stop flow.

Regards,
Chun-Kuang.

> +                       dev_warn(dev, "thread[%d] exist running task(s) i=
n suspend\n", i);
>                 }
>         }
>
> -       if (task_running)
> -               dev_warn(dev, "exist running task(s) in suspend\n");
> -
>         clk_bulk_unprepare(cmdq->gce_num, cmdq->clocks);
>
>         return 0;
> --
> 2.18.0
>