[PATCH v2, 0/5] Revert "mailbox: mediatek: remove implementation related to atomic_exec"

[PATCH v2, 0/5] Revert "mailbox: mediatek: remove implementation related to atomic_exec"

[Yongqiang Niu]

This series base linux 5.12-rc2
these patches will cause home ui flick when cursor moved,
there is no fix solution yet, revert these patches first.

change since v1:
add mtk-gce.txt and dts modification

Yongqiang Niu (5):
  Revert "drm/mediatek: Make sure previous message done or be aborted
    before send"
  Revert "mailbox: mediatek: remove implementation related to
    atomic_exec"
  Revert "dt-bindings: mailbox: mtk-gce: fix incorrect mbox-cells value"
  Revert "arm64: dts: mediatek: mt8183: fix gce incorrect mbox-cells
    value"
  arm64: dts: mediatek: mt8183: add gce information for mmsys

 .../devicetree/bindings/mailbox/mtk-gce.txt        |  2 +-
 arch/arm64/boot/dts/mediatek/mt8183.dtsi           |  5 +-
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c            |  1 -
 drivers/mailbox/mtk-cmdq-mailbox.c                 | 80 +++++++++++++++++++---
 4 files changed, 76 insertions(+), 12 deletions(-)

-- 
1.8.1.1.dirty

[PATCH v2, 1/5] Revert "drm/mediatek: Make sure previous message done or be aborted before send"

[Yongqiang Niu]

This reverts commit 839cbf0531428f3f9535077a461b8631359c1165.

Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 8b0de90..c76f446 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -463,7 +463,6 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc)
 	}
 #if IS_REACHABLE(CONFIG_MTK_CMDQ)
 	if (mtk_crtc->cmdq_client) {
-		mbox_flush(mtk_crtc->cmdq_client->chan, 2000);
 		cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE);
 		cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
 		cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false);
-- 
1.8.1.1.dirty

[PATCH v2, 2/5] Revert "mailbox: mediatek: remove implementation related to atomic_exec"

[Yongqiang Niu]

This reverts commit c9ea564f3d9dd20d88bd34f40a6ff6d31a0d7e8c.

Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
---
 drivers/mailbox/mtk-cmdq-mailbox.c | 80 +++++++++++++++++++++++++++++++++-----
 1 file changed, 71 insertions(+), 9 deletions(-)

diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
index 5665b6e..e0d9a86 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -56,6 +56,7 @@ struct cmdq_thread {
 	void __iomem		*base;
 	struct list_head	task_busy_list;
 	u32			priority;
+	bool			atomic_exec;
 };
 
 struct cmdq_task {
@@ -168,18 +169,56 @@ static void cmdq_task_insert_into_thread(struct cmdq_task *task)
 	dma_sync_single_for_cpu(dev, prev_task->pa_base,
 				prev_task->pkt->cmd_buf_size, DMA_TO_DEVICE);
 	prev_task_base[CMDQ_NUM_CMD(prev_task->pkt) - 1] =
-		(u64)CMDQ_JUMP_BY_PA << 32 | task->pa_base;
+		(u64)CMDQ_JUMP_BY_PA << 32 |
+		(task->pa_base >> task->cmdq->shift_pa);
 	dma_sync_single_for_device(dev, prev_task->pa_base,
 				   prev_task->pkt->cmd_buf_size, DMA_TO_DEVICE);
 
 	cmdq_thread_invalidate_fetched_data(thread);
 }
 
+static bool cmdq_command_is_wfe(u64 cmd)
+{
+	u64 wfe_option = CMDQ_WFE_UPDATE | CMDQ_WFE_WAIT | CMDQ_WFE_WAIT_VALUE;
+	u64 wfe_op = (u64)(CMDQ_CODE_WFE << CMDQ_OP_CODE_SHIFT) << 32;
+	u64 wfe_mask = (u64)CMDQ_OP_CODE_MASK << 32 | 0xffffffff;
+
+	return ((cmd & wfe_mask) == (wfe_op | wfe_option));
+}
+
+/* we assume tasks in the same display GCE thread are waiting the same event. */
+static void cmdq_task_remove_wfe(struct cmdq_task *task)
+{
+	struct device *dev = task->cmdq->mbox.dev;
+	u64 *base = task->pkt->va_base;
+	int i;
+
+	dma_sync_single_for_cpu(dev, task->pa_base, task->pkt->cmd_buf_size,
+				DMA_TO_DEVICE);
+	for (i = 0; i < CMDQ_NUM_CMD(task->pkt); i++)
+		if (cmdq_command_is_wfe(base[i]))
+			base[i] = (u64)CMDQ_JUMP_BY_OFFSET << 32 |
+				  (CMDQ_JUMP_PASS >> task->cmdq->shift_pa);
+	dma_sync_single_for_device(dev, task->pa_base, task->pkt->cmd_buf_size,
+				   DMA_TO_DEVICE);
+}
+
 static bool cmdq_thread_is_in_wfe(struct cmdq_thread *thread)
 {
 	return readl(thread->base + CMDQ_THR_WAIT_TOKEN) & CMDQ_THR_IS_WAITING;
 }
 
+static void cmdq_thread_wait_end(struct cmdq_thread *thread,
+				 unsigned long end_pa)
+{
+	struct device *dev = thread->chan->mbox->dev;
+	unsigned long curr_pa;
+
+	if (readl_poll_timeout_atomic(thread->base + CMDQ_THR_CURR_ADDR,
+			curr_pa, curr_pa == end_pa, 1, 20))
+		dev_err(dev, "GCE thread cannot run to end.\n");
+}
+
 static void cmdq_task_exec_done(struct cmdq_task *task, enum cmdq_cb_status sta)
 {
 	struct cmdq_task_cb *cb = &task->pkt->async_cb;
@@ -371,15 +410,37 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
 			cmdq->shift_pa;
 		end_pa = readl(thread->base + CMDQ_THR_END_ADDR) <<
 			cmdq->shift_pa;
-		/* check boundary */
-		if (curr_pa == end_pa - CMDQ_INST_SIZE ||
-		    curr_pa == end_pa) {
-			/* set to this task directly */
-			writel(task->pa_base >> cmdq->shift_pa,
-			       thread->base + CMDQ_THR_CURR_ADDR);
+
+		/*
+		 * Atomic execution should remove the following wfe, i.e. only
+		 * wait event at first task, and prevent to pause when running.
+		 */
+		if (thread->atomic_exec) {
+			/* GCE is executing if command is not WFE */
+			if (!cmdq_thread_is_in_wfe(thread)) {
+				cmdq_thread_resume(thread);
+				cmdq_thread_wait_end(thread,
+						     end_pa >> cmdq->shift_pa);
+				WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
+				/* set to this task directly */
+				writel(task->pa_base >> cmdq->shift_pa,
+				       thread->base + CMDQ_THR_CURR_ADDR);
+			} else {
+				cmdq_task_insert_into_thread(task);
+				cmdq_task_remove_wfe(task);
+				smp_mb(); /* modify jump before enable thread */
+			}
 		} else {
-			cmdq_task_insert_into_thread(task);
-			smp_mb(); /* modify jump before enable thread */
+			/* check boundary */
+			if (curr_pa == end_pa - CMDQ_INST_SIZE ||
+			    curr_pa == end_pa) {
+				/* set to this task directly */
+				writel(task->pa_base >> cmdq->shift_pa,
+				       thread->base + CMDQ_THR_CURR_ADDR);
+			} else {
+				cmdq_task_insert_into_thread(task);
+				smp_mb(); /* modify jump before enable thread */
+			}
 		}
 		writel((task->pa_base + pkt->cmd_buf_size) >> cmdq->shift_pa,
 		       thread->base + CMDQ_THR_END_ADDR);
@@ -500,6 +561,7 @@ static struct mbox_chan *cmdq_xlate(struct mbox_controller *mbox,
 
 	thread = (struct cmdq_thread *)mbox->chans[ind].con_priv;
 	thread->priority = sp->args[1];
+	thread->atomic_exec = (sp->args[2] != 0);
 	thread->chan = &mbox->chans[ind];
 
 	return &mbox->chans[ind];
-- 
1.8.1.1.dirty

[PATCH v2, 3/5] Revert "dt-bindings: mailbox: mtk-gce: fix incorrect mbox-cells value"

[Yongqiang Niu]

This reverts commit f83b03fc727ab56a77e68713d6e40299698f3c9f.

Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
---
 Documentation/devicetree/bindings/mailbox/mtk-gce.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mailbox/mtk-gce.txt b/Documentation/devicetree/bindings/mailbox/mtk-gce.txt
index 7771eca..cf48cd8 100644
--- a/Documentation/devicetree/bindings/mailbox/mtk-gce.txt
+++ b/Documentation/devicetree/bindings/mailbox/mtk-gce.txt
@@ -47,7 +47,7 @@ Example:
 		interrupts = <GIC_SPI 135 IRQ_TYPE_LEVEL_LOW>;
 		clocks = <&infracfg CLK_INFRA_GCE>;
 		clock-names = "gce";
-		#mbox-cells = <2>;
+		#mbox-cells = <3>;
 	};
 
 Example for a client device:
-- 
1.8.1.1.dirty

Re: [PATCH v2, 3/5] Revert "dt-bindings: mailbox: mtk-gce: fix incorrect mbox-cells value"

[Chun-Kuang Hu]

Hi, Yongqiang:

Yongqiang Niu <yongqiang.niu@mediatek.com> 於 2021年4月12日 週一 下午7:19寫道：
>
> This reverts commit f83b03fc727ab56a77e68713d6e40299698f3c9f.
>
> Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
> ---
>  Documentation/devicetree/bindings/mailbox/mtk-gce.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mailbox/mtk-gce.txt b/Documentation/devicetree/bindings/mailbox/mtk-gce.txt
> index 7771eca..cf48cd8 100644
> --- a/Documentation/devicetree/bindings/mailbox/mtk-gce.txt
> +++ b/Documentation/devicetree/bindings/mailbox/mtk-gce.txt
> @@ -47,7 +47,7 @@ Example:
>                 interrupts = <GIC_SPI 135 IRQ_TYPE_LEVEL_LOW>;
>                 clocks = <&infracfg CLK_INFRA_GCE>;
>                 clock-names = "gce";
> -               #mbox-cells = <2>;
> +               #mbox-cells = <3>;

I think we should not change the binding just to fix software bug.
I think there are many temporary solution to fix drm bug. If drm bug
is caused by cursor plane, you could temporarily let drm not support
cursor plane to fix it (in [1], do not return DRM_PLANE_TYPE_CURSOR).
But I would like you to find out the correct solution rather than a
temporary solution because this bug is not so urgent. (For me, bug of
build fail, boot fail, black screen is urgent).

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/mediatek/mtk_drm_crtc.c?h=v5.12-rc7#n699

Regards,
Chun-Kuang.

>         };
>
>  Example for a client device:
> --
> 1.8.1.1.dirty
>

Re: [PATCH v2, 3/5] Revert "dt-bindings: mailbox: mtk-gce: fix incorrect mbox-cells value"

[Yongqiang Niu]

On Wed, 2021-04-14 at 07:38 +0800, Chun-Kuang Hu wrote:
> Hi, Yongqiang:
> 
> Yongqiang Niu <yongqiang.niu@mediatek.com> 於 2021年4月12日 週一 下午7:19寫道：
> >
> > This reverts commit f83b03fc727ab56a77e68713d6e40299698f3c9f.
> >
> > Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
> > ---
> >  Documentation/devicetree/bindings/mailbox/mtk-gce.txt | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mailbox/mtk-gce.txt b/Documentation/devicetree/bindings/mailbox/mtk-gce.txt
> > index 7771eca..cf48cd8 100644
> > --- a/Documentation/devicetree/bindings/mailbox/mtk-gce.txt
> > +++ b/Documentation/devicetree/bindings/mailbox/mtk-gce.txt
> > @@ -47,7 +47,7 @@ Example:
> >                 interrupts = <GIC_SPI 135 IRQ_TYPE_LEVEL_LOW>;
> >                 clocks = <&infracfg CLK_INFRA_GCE>;
> >                 clock-names = "gce";
> > -               #mbox-cells = <2>;
> > +               #mbox-cells = <3>;
> 
> I think we should not change the binding just to fix software bug.
> I think there are many temporary solution to fix drm bug. If drm bug
> is caused by cursor plane, you could temporarily let drm not support
> cursor plane to fix it (in [1], do not return DRM_PLANE_TYPE_CURSOR).
> But I would like you to find out the correct solution rather than a
> temporary solution because this bug is not so urgent. (For me, bug of
> build fail, boot fail, black screen is urgent).
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/mediatek/mtk_drm_crtc.c?h=v5.12-rc7#n699
> 
> Regards,
> Chun-Kuang.

this not drm bug, but caused by cmdq patch
https://patchwork.kernel.org/project/linux-mediatek/patch/20200214043325.16618-3-bibby.hsieh@mediatek.com/
before this patch, drm cursor and ui works well
after this patch, ui will flash when move cursor

this is critical issue,

> 
> >         };
> >
> >  Example for a client device:
> > --
> > 1.8.1.1.dirty
> >

[PATCH v2, 4/5] Revert "arm64: dts: mediatek: mt8183: fix gce incorrect mbox-cells value"

[Yongqiang Niu]

This reverts commit e55c56df43dd11de4a6c08e3ea52ca45b51c8800.

Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 80519a1..bc89283 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -541,7 +541,7 @@
 			compatible = "mediatek,mt8183-gce";
 			reg = <0 0x10238000 0 0x4000>;
 			interrupts = <GIC_SPI 162 IRQ_TYPE_LEVEL_LOW>;
-			#mbox-cells = <2>;
+			#mbox-cells = <3>;
 			clocks = <&infracfg CLK_INFRA_GCE>;
 			clock-names = "gce";
 		};
-- 
1.8.1.1.dirty

[PATCH v2, 5/5] arm64: dts: mediatek: mt8183: add gce information for mmsys

[Yongqiang Niu]

add gce information for mmsys

Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index bc89283..e3a8b10 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -983,6 +983,9 @@
 			compatible = "mediatek,mt8183-mmsys", "syscon";
 			reg = <0 0x14000000 0 0x1000>;
 			#clock-cells = <1>;
+			mboxes = <&gce 0 CMDQ_THR_PRIO_HIGHEST 1>,
+				 <&gce 1 CMDQ_THR_PRIO_HIGHEST 1>;
+			mediatek,gce-client-reg = <&gce SUBSYS_1400XXXX 0 0x1000>;
 		};
 
 		ovl0: ovl@14008000 {
-- 
1.8.1.1.dirty

Re: [PATCH v2, 0/5] Revert "mailbox: mediatek: remove implementation related to atomic_exec"

[Jassi Brar]

On Mon, Apr 12, 2021 at 6:18 AM Yongqiang Niu
<yongqiang.niu@mediatek.com> wrote:
>
> This series base linux 5.12-rc2
> these patches will cause home ui flick when cursor moved,
> there is no fix solution yet, revert these patches first.
>
> change since v1:
> add mtk-gce.txt and dts modification
>
> Yongqiang Niu (5):
>   Revert "drm/mediatek: Make sure previous message done or be aborted
>     before send"
>   Revert "mailbox: mediatek: remove implementation related to
>     atomic_exec"
>   Revert "dt-bindings: mailbox: mtk-gce: fix incorrect mbox-cells value"
>   Revert "arm64: dts: mediatek: mt8183: fix gce incorrect mbox-cells
>     value"
>   arm64: dts: mediatek: mt8183: add gce information for mmsys
>
>  .../devicetree/bindings/mailbox/mtk-gce.txt        |  2 +-
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi           |  5 +-
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c            |  1 -
>  drivers/mailbox/mtk-cmdq-mailbox.c                 | 80 +++++++++++++++++++---
>  4 files changed, 76 insertions(+), 12 deletions(-)
>
Please break the patchsets (this and the other 3) into mailbox only
and platform specific ones.
Also, it would help if there are some acked/reviewed by some mtk folks
especially when reverting patches.

thanks

Re: [PATCH v2, 0/5] Revert "mailbox: mediatek: remove implementation related to atomic_exec"

[Matthias Brugger]

On 12/04/2021 17:29, Jassi Brar wrote:
> On Mon, Apr 12, 2021 at 6:18 AM Yongqiang Niu
> <yongqiang.niu@mediatek.com> wrote:
>>
>> This series base linux 5.12-rc2
>> these patches will cause home ui flick when cursor moved,
>> there is no fix solution yet, revert these patches first.
>>
>> change since v1:
>> add mtk-gce.txt and dts modification
>>
>> Yongqiang Niu (5):
>>   Revert "drm/mediatek: Make sure previous message done or be aborted
>>     before send"
>>   Revert "mailbox: mediatek: remove implementation related to
>>     atomic_exec"
>>   Revert "dt-bindings: mailbox: mtk-gce: fix incorrect mbox-cells value"
>>   Revert "arm64: dts: mediatek: mt8183: fix gce incorrect mbox-cells
>>     value"
>>   arm64: dts: mediatek: mt8183: add gce information for mmsys
>>
>>  .../devicetree/bindings/mailbox/mtk-gce.txt        |  2 +-
>>  arch/arm64/boot/dts/mediatek/mt8183.dtsi           |  5 +-
>>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c            |  1 -
>>  drivers/mailbox/mtk-cmdq-mailbox.c                 | 80 +++++++++++++++++++---
>>  4 files changed, 76 insertions(+), 12 deletions(-)
>>
> Please break the patchsets (this and the other 3) into mailbox only
> and platform specific ones.
> Also, it would help if there are some acked/reviewed by some mtk folks
> especially when reverting patches.
> 

I'd prefer to have DT and mailbox patches together as otherwise the burden on me
to find out which patches in the driver are needed, and to check if these got
accepted, gets higher.

Regards,
Matthias

Re: [PATCH v2, 0/5] Revert "mailbox: mediatek: remove implementation related to atomic_exec"

[Jassi Brar]

On Wed, Apr 14, 2021 at 3:51 AM Matthias Brugger <matthias.bgg@gmail.com> wrote:
>
>
>
> On 12/04/2021 17:29, Jassi Brar wrote:
> > On Mon, Apr 12, 2021 at 6:18 AM Yongqiang Niu
> > <yongqiang.niu@mediatek.com> wrote:
> >>
> >> This series base linux 5.12-rc2
> >> these patches will cause home ui flick when cursor moved,
> >> there is no fix solution yet, revert these patches first.
> >>
> >> change since v1:
> >> add mtk-gce.txt and dts modification
> >>
> >> Yongqiang Niu (5):
> >>   Revert "drm/mediatek: Make sure previous message done or be aborted
> >>     before send"
> >>   Revert "mailbox: mediatek: remove implementation related to
> >>     atomic_exec"
> >>   Revert "dt-bindings: mailbox: mtk-gce: fix incorrect mbox-cells value"
> >>   Revert "arm64: dts: mediatek: mt8183: fix gce incorrect mbox-cells
> >>     value"
> >>   arm64: dts: mediatek: mt8183: add gce information for mmsys
> >>
> >>  .../devicetree/bindings/mailbox/mtk-gce.txt        |  2 +-
> >>  arch/arm64/boot/dts/mediatek/mt8183.dtsi           |  5 +-
> >>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c            |  1 -
> >>  drivers/mailbox/mtk-cmdq-mailbox.c                 | 80 +++++++++++++++++++---
> >>  4 files changed, 76 insertions(+), 12 deletions(-)
> >>
> > Please break the patchsets (this and the other 3) into mailbox only
> > and platform specific ones.
> > Also, it would help if there are some acked/reviewed by some mtk folks
> > especially when reverting patches.
> >
>
> I'd prefer to have DT and mailbox patches together as otherwise the burden on me
> to find out which patches in the driver are needed, and to check if these got
> accepted, gets higher.
>
I meant the patches that need to go via mailbox tree (controller) and the rest.

thanks.