From: CK Hu <ck.hu@mediatek.com> To: Yongqiang Niu <yongqiang.niu@mediatek.com>, Chun-Kuang Hu <chunkuang.hu@kernel.org> Cc: Jassi Brar <jassisinghbrar@gmail.com>, Matthias Brugger <matthias.bgg@gmail.com>, <linux-kernel@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <linux-mediatek@lists.infradead.org>, <Project_Global_Chrome_Upstream_Group@mediatek.com>, Hsin-Yi Wang <hsinyi@chromium.org>, Yongqiang Niu <yongqiang.niu@mediatek.corp-partner.google.com>, Allen-kh Cheng <allen-kh.cheng@mediatek.corp-partner.google.com> Subject: Re: [PATCH v1, 1/1] mailbox: mtk-cmdq: fix gce timeout issue Date: Mon, 15 Aug 2022 13:24:40 +0800 [thread overview] Message-ID: <d2e6147dc342867aba18f71869c9b4d4230f6ccc.camel@mediatek.com> (raw) In-Reply-To: <20220815030740.28899-2-yongqiang.niu@mediatek.com> Hi, Yongqiang: On Mon, 2022-08-15 at 11:07 +0800, Yongqiang Niu wrote: > From: Yongqiang Niu <yongqiang.niu@mediatek.corp-partner.google.com> > > 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 Why need gce ddr enable? I think gce works fine without this. Describe more about this. Regards, CK > 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 > > Signed-off-by: Yongqiang Niu < > yongqiang.niu@mediatek.corp-partner.google.com> > Signed-off-by: Allen-kh Cheng < > allen-kh.cheng@mediatek.corp-partner.google.com> > --- > drivers/mailbox/mtk-cmdq-mailbox.c | 57 ++++++++++++++++++++++++++ > ---- > 1 file changed, 51 insertions(+), 6 deletions(-) > > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c > b/drivers/mailbox/mtk-cmdq-mailbox.c > index 2578e5aaa935..64a47470f062 100644 > --- a/drivers/mailbox/mtk-cmdq-mailbox.c > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c > @@ -81,6 +81,8 @@ struct cmdq { > u8 shift_pa; > bool control_by_sw; > u32 gce_num; > + atomic_t usage; > + spinlock_t lock; > }; > > struct gce_plat { > @@ -90,6 +92,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->control_by_sw) > + 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->control_by_sw) > + 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); > @@ -271,7 +313,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); > } > } > > @@ -360,8 +403,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 > @@ -433,7 +475,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: > /* > @@ -479,7 +521,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); > @@ -490,7 +533,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; > @@ -626,6 +670,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;
WARNING: multiple messages have this Message-ID (diff)
From: CK Hu <ck.hu@mediatek.com> To: Yongqiang Niu <yongqiang.niu@mediatek.com>, Chun-Kuang Hu <chunkuang.hu@kernel.org> Cc: Jassi Brar <jassisinghbrar@gmail.com>, Matthias Brugger <matthias.bgg@gmail.com>, <linux-kernel@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <linux-mediatek@lists.infradead.org>, <Project_Global_Chrome_Upstream_Group@mediatek.com>, Hsin-Yi Wang <hsinyi@chromium.org>, Yongqiang Niu <yongqiang.niu@mediatek.corp-partner.google.com>, Allen-kh Cheng <allen-kh.cheng@mediatek.corp-partner.google.com> Subject: Re: [PATCH v1, 1/1] mailbox: mtk-cmdq: fix gce timeout issue Date: Mon, 15 Aug 2022 13:24:40 +0800 [thread overview] Message-ID: <d2e6147dc342867aba18f71869c9b4d4230f6ccc.camel@mediatek.com> (raw) In-Reply-To: <20220815030740.28899-2-yongqiang.niu@mediatek.com> Hi, Yongqiang: On Mon, 2022-08-15 at 11:07 +0800, Yongqiang Niu wrote: > From: Yongqiang Niu <yongqiang.niu@mediatek.corp-partner.google.com> > > 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 Why need gce ddr enable? I think gce works fine without this. Describe more about this. Regards, CK > 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 > > Signed-off-by: Yongqiang Niu < > yongqiang.niu@mediatek.corp-partner.google.com> > Signed-off-by: Allen-kh Cheng < > allen-kh.cheng@mediatek.corp-partner.google.com> > --- > drivers/mailbox/mtk-cmdq-mailbox.c | 57 ++++++++++++++++++++++++++ > ---- > 1 file changed, 51 insertions(+), 6 deletions(-) > > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c > b/drivers/mailbox/mtk-cmdq-mailbox.c > index 2578e5aaa935..64a47470f062 100644 > --- a/drivers/mailbox/mtk-cmdq-mailbox.c > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c > @@ -81,6 +81,8 @@ struct cmdq { > u8 shift_pa; > bool control_by_sw; > u32 gce_num; > + atomic_t usage; > + spinlock_t lock; > }; > > struct gce_plat { > @@ -90,6 +92,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->control_by_sw) > + 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->control_by_sw) > + 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); > @@ -271,7 +313,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); > } > } > > @@ -360,8 +403,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 > @@ -433,7 +475,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: > /* > @@ -479,7 +521,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); > @@ -490,7 +533,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; > @@ -626,6 +670,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; _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-08-15 5:24 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-08-15 3:07 [PATCH v1, 0/1] fix gce timeout issue Yongqiang Niu 2022-08-15 3:07 ` Yongqiang Niu 2022-08-15 3:07 ` [PATCH v1, 1/1] mailbox: mtk-cmdq: " Yongqiang Niu 2022-08-15 3:07 ` Yongqiang Niu 2022-08-15 5:24 ` CK Hu [this message] 2022-08-15 5:24 ` CK Hu 2022-08-15 5:51 ` yongqiang.niu 2022-08-15 5:51 ` yongqiang.niu 2022-08-15 6:23 ` CK Hu 2022-08-15 6:23 ` CK Hu 2022-08-15 9:51 ` yongqiang.niu 2022-08-15 9:51 ` yongqiang.niu 2022-08-16 1:49 ` CK Hu 2022-08-16 1:49 ` CK Hu 2022-08-16 2:14 ` yongqiang.niu 2022-08-16 2:14 ` yongqiang.niu 2022-08-15 3:24 ` [PATCH v1, 0/1] " Allen-KH Cheng 2022-08-15 3:24 ` Allen-KH Cheng 2022-08-15 4:30 ` Hsin-Yi Wang 2022-08-15 4:30 ` Hsin-Yi Wang
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=d2e6147dc342867aba18f71869c9b4d4230f6ccc.camel@mediatek.com \ --to=ck.hu@mediatek.com \ --cc=Project_Global_Chrome_Upstream_Group@mediatek.com \ --cc=allen-kh.cheng@mediatek.corp-partner.google.com \ --cc=chunkuang.hu@kernel.org \ --cc=hsinyi@chromium.org \ --cc=jassisinghbrar@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mediatek@lists.infradead.org \ --cc=matthias.bgg@gmail.com \ --cc=yongqiang.niu@mediatek.com \ --cc=yongqiang.niu@mediatek.corp-partner.google.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.