[PATCH v1, 1/1] mailbox: mtk-cmdq: fix gce timeout issue
CK Hu
ck.hu at mediatek.com
Sun Aug 14 23:23:54 PDT 2022
Hi, Yongqiang:
On Mon, 2022-08-15 at 13:51 +0800, yongqiang.niu wrote:
> On Mon, 2022-08-15 at 13:24 +0800, CK Hu wrote:
> > Hi, Yongqiang:
> >
> >
> > On Mon, 2022-08-15 at 11:07 +0800, Yongqiang Niu wrote:
> > > From: Yongqiang Niu <
> > > yongqiang.niu at 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
>
> 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.
You [1] and Jason [2] has already enable control_by_sw and I believe
you two have test your patch in general. Is the ddr problem a special
case? How to operate to trigger this bug? Describe more in commit
message to let us know why need this patch.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mailbox/mtk-cmdq-mailbox.c?h=v6.0-rc1&id=84fd4201b78b96f8d31f6a2624be27ad6306a9bc
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mailbox/mtk-cmdq-mailbox.c?h=v6.0-rc1&id=9388501fbb99a1b6a23f28634d125567a3b45a3d
>
> >
> > > 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 at mediatek.corp-partner.google.com>
> > > Signed-off-by: Allen-kh Cheng <
> > > allen-kh.cheng at 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;
> >
> >
>
>
More information about the linux-arm-kernel
mailing list