[v2] mailbox: mtk-cmdq: fix gce timeout issue

Commit Message

Yongqiang Niu Sept. 17, 2022, 9:09 a.m. UTC

From: Yongqiang Niu <[e-mail]>

1. enable gce ddr enable(gce reigster offset 0x48, bit 16 to 18) when gce work,
and disable gce ddr enable when gce work job done
2. split cmdq clk enable/disable api, and control gce ddr enable/disable
in clk enable/disable function to make sure it could protect when cmdq
is multiple used by display and mdp

this is only for some SOC which has flag "control_by_sw".
for this kind of gce, there is a handshake flow between gce and ddr
hardware,
if not set ddr enable flag of gce, ddr will fall into idle mode,
then gce instructions will not process done.
we need set this flag of gce to tell ddr when gce is idle or busy
controlled by software flow.

ddr problem is a special case.
when test suspend/resume case, gce sometimes will pull ddr, and ddr can
not go to suspend.
if we set gce register 0x48 to 0x7, will fix this gce pull ddr issue,
as you have referred [1] and [2] (8192 and 8195)
but for mt8186, the gce is more special, except setting of [1] and [2],
we need add more setting set gce register 0x48 to (0x7 << 16 | 0x7)
when gce working to make sure gce could process all instructions ok.
this case just need normal bootup, if we not set this, display cmdq
task will timeout, and chrome homescreen will always black screen.

and with this patch, we have done these test on mt8186:
1.suspend/resume
2.boot up to home screen
3.playback video with youtube.

suspend issue is special gce hardware issue, gce client  driver
command already process done, but gce still pull ddr.

Signed-off-by: Yongqiang Niu <[e-mail]>
Signed-off-by: Allen-kh Cheng <[e-mail]>
---
 drivers/mailbox/mtk-cmdq-mailbox.c | 67 +++++++++++++++++++++++++++---
 1 file changed, 61 insertions(+), 6 deletions(-)

Comments

kernel test robot Sept. 17, 2022, 1:06 p.m. UTC | #1

Hi Yongqiang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on fujitsu-integration/mailbox-for-next]
[also build test ERROR on soc/for-next linus/master v6.0-rc5 next-20220916]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Yongqiang-Niu/mailbox-mtk-cmdq-fix-gce-timeout-issue/20220917-171148
base:   https://git.linaro.org/landing-teams/working/fujitsu/integration.git mailbox-for-next
config: openrisc-randconfig-r023-20220916 (https://download.01.org/0day-ci/archive/20220917/[e-mail]/config)
compiler: or1k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/044a12235901d40a3442fa5ab0c4b3233f370e22
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Yongqiang-Niu/mailbox-mtk-cmdq-fix-gce-timeout-issue/20220917-171148
        git checkout 044a12235901d40a3442fa5ab0c4b3233f370e22
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=openrisc SHELL=/bin/bash drivers/mailbox/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[e-mail]>

All errors (new ones prefixed by >>):

>> drivers/mailbox/mtk-cmdq-mailbox.c:713:10: error: 'const struct gce_plat' has no member named 'sw_ddr_en'
     713 |         .sw_ddr_en = true,
         |          ^~~~~~~~~
   drivers/mailbox/mtk-cmdq-mailbox.c:714:20: warning: initialized field overwritten [-Woverride-init]
     714 |         .gce_num = 1
         |                    ^
   drivers/mailbox/mtk-cmdq-mailbox.c:714:20: note: (near initialization for 'gce_plat_v7.gce_num')


vim +713 drivers/mailbox/mtk-cmdq-mailbox.c

   708	
   709	static const struct gce_plat gce_plat_v7 = {
   710		.thread_nr = 24,
   711		.shift = 3,
   712		.control_by_sw = true,
 > 713		.sw_ddr_en = true,
   714		.gce_num = 1
   715	};
   716

kernel test robot Sept. 17, 2022, 1:37 p.m. UTC | #2

Hi Yongqiang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on fujitsu-integration/mailbox-for-next]
[also build test ERROR on soc/for-next linus/master v6.0-rc5 next-20220916]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Yongqiang-Niu/mailbox-mtk-cmdq-fix-gce-timeout-issue/20220917-171148
base:   https://git.linaro.org/landing-teams/working/fujitsu/integration.git mailbox-for-next
config: arm-randconfig-r033-20220916 (https://download.01.org/0day-ci/archive/20220917/[e-mail]/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 791a7ae1ba3efd6bca96338e10ffde557ba83920)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/intel-lab-lkp/linux/commit/044a12235901d40a3442fa5ab0c4b3233f370e22
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Yongqiang-Niu/mailbox-mtk-cmdq-fix-gce-timeout-issue/20220917-171148
        git checkout 044a12235901d40a3442fa5ab0c4b3233f370e22
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[e-mail]>

All errors (new ones prefixed by >>):

>> drivers/mailbox/mtk-cmdq-mailbox.c:713:3: error: field designator 'sw_ddr_en' does not refer to any field in type 'const struct gce_plat'
           .sw_ddr_en = true,
            ^
   1 error generated.


vim +713 drivers/mailbox/mtk-cmdq-mailbox.c

   708	
   709	static const struct gce_plat gce_plat_v7 = {
   710		.thread_nr = 24,
   711		.shift = 3,
   712		.control_by_sw = true,
 > 713		.sw_ddr_en = true,
   714		.gce_num = 1
   715	};
   716

Patch

diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
index 9465f9081515..adb6fce1c90f 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -80,7 +80,10 @@  struct cmdq {
 	bool			suspended;
 	u8			shift_pa;
 	bool			control_by_sw;
+	bool			sw_ddr_en;
 	u32			gce_num;
+	atomic_t		usage;
+	spinlock_t		lock;
 };
 
 struct gce_plat {
@@ -90,6 +93,46 @@  struct gce_plat {
 	u32 gce_num;
 };
 
+static s32 cmdq_clk_enable(struct cmdq *cmdq)
+{
+	s32 usage, ret;
+	unsigned long flags;
+
+	spin_lock_irqsave(&cmdq->lock, flags);
+
+	usage = atomic_inc_return(&cmdq->usage);
+
+	ret = clk_bulk_enable(cmdq->gce_num, cmdq->clocks);
+	if (usage <=0 || ret < 0) {
+		dev_err(cmdq->mbox.dev, "ref count %d ret %d suspend %d\n",
+			usage, ret, cmdq->suspended);
+	} else if (usage == 1) {
+		if (cmdq->sw_ddr_en)
+			writel((0x7 << 16) + 0x7, cmdq->base + GCE_GCTL_VALUE);
+	}
+
+	spin_unlock_irqrestore(&cmdq->lock, flags);
+
+	return ret;
+}
+
+static void cmdq_clk_disable(struct cmdq *cmdq)
+{
+	s32 usage;
+
+	usage = atomic_dec_return(&cmdq->usage);
+
+	if (usage < 0) {
+		dev_err(cmdq->mbox.dev, "ref count %d suspend %d\n",
+			usage, cmdq->suspended);
+	} else if (usage == 0) {
+		if (cmdq->sw_ddr_en)
+			writel(0x7, cmdq->base + GCE_GCTL_VALUE);
+	}
+
+	clk_bulk_disable(cmdq->gce_num, cmdq->clocks);
+}
+
 u8 cmdq_get_shift_pa(struct mbox_chan *chan)
 {
 	struct cmdq *cmdq = container_of(chan->mbox, struct cmdq, mbox);
@@ -266,7 +309,8 @@  static void cmdq_thread_irq_handler(struct cmdq *cmdq,
 
 	if (list_empty(&thread->task_busy_list)) {
 		cmdq_thread_disable(cmdq, thread);
-		clk_bulk_disable(cmdq->gce_num, cmdq->clocks);
+
+		cmdq_clk_disable(cmdq);
 	}
 }
 
@@ -355,8 +399,7 @@  static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
 	task->pkt = pkt;
 
 	if (list_empty(&thread->task_busy_list)) {
-		WARN_ON(clk_bulk_enable(cmdq->gce_num, cmdq->clocks));
-
+		WARN_ON(cmdq_clk_enable(cmdq) < 0);
 		/*
 		 * The thread reset will clear thread related register to 0,
 		 * including pc, end, priority, irq, suspend and enable. Thus
@@ -428,7 +471,7 @@  static void cmdq_mbox_shutdown(struct mbox_chan *chan)
 	}
 
 	cmdq_thread_disable(cmdq, thread);
-	clk_bulk_disable(cmdq->gce_num, cmdq->clocks);
+	cmdq_clk_disable(cmdq);
 
 done:
 	/*
@@ -468,7 +511,8 @@  static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout)
 
 	cmdq_thread_resume(thread);
 	cmdq_thread_disable(cmdq, thread);
-	clk_bulk_disable(cmdq->gce_num, cmdq->clocks);
+
+	cmdq_clk_disable(cmdq);
 
 out:
 	spin_unlock_irqrestore(&thread->chan->lock, flags);
@@ -479,7 +523,8 @@  static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout)
 	spin_unlock_irqrestore(&thread->chan->lock, flags);
 	if (readl_poll_timeout_atomic(thread->base + CMDQ_THR_ENABLE_TASK,
 				      enable, enable == 0, 1, timeout)) {
-		dev_err(cmdq->mbox.dev, "Fail to wait GCE thread 0x%x done\n",
+		dev_err(cmdq->mbox.dev,
+			"Fail to wait GCE thread 0x%x done\n",
 			(u32)(thread->base - cmdq->base));
 
 		return -EFAULT;
@@ -615,6 +660,7 @@  static int cmdq_probe(struct platform_device *pdev)
 
 	WARN_ON(clk_bulk_prepare(cmdq->gce_num, cmdq->clocks));
 
+	spin_lock_init(&cmdq->lock);
 	cmdq_init(cmdq);
 
 	return 0;
@@ -660,9 +706,18 @@  static const struct gce_plat gce_plat_v6 = {
 	.gce_num = 2
 };
 
+static const struct gce_plat gce_plat_v7 = {
+	.thread_nr = 24,
+	.shift = 3,
+	.control_by_sw = true,
+	.sw_ddr_en = true,
+	.gce_num = 1
+};
+
 static const struct of_device_id cmdq_of_ids[] = {
 	{.compatible = "mediatek,mt8173-gce", .data = (void *)&gce_plat_v2},
 	{.compatible = "mediatek,mt8183-gce", .data = (void *)&gce_plat_v3},
+	{.compatible = "mediatek,mt8186-gce", .data = (void *)&gce_plat_v7},
 	{.compatible = "mediatek,mt6779-gce", .data = (void *)&gce_plat_v4},
 	{.compatible = "mediatek,mt8192-gce", .data = (void *)&gce_plat_v5},
 	{.compatible = "mediatek,mt8195-gce", .data = (void *)&gce_plat_v6},