Mediatek mailbox controller implement its own data queue rather than using
mailbox framework's queue. This series let the framework provide abort
function and Mediatek mailbox controller implement the abort-function, so
it could use framework's queue.
The reason that Mediatek mailbox controller has to implement its own queue:
One client of Mediatek mailbox controller is display driver. When a cursor
is moving, display continuously update the register related to the cursor.
Display hardware has a limitation that register should be updated in the
vblank period which is a small interval. In tradition, display hardware
would trigger an interrupt when vblank start, and driver could update
register in this irq handler. But the interrupt handler has the risk that
it could be delayed by some reason so the handler may be postponed out of
this vblank interval. In order to reduce the risk, display driver use GCE
hardware to write register. If a cursor move 3 times before vblank,
display driver would send 3 messages sequentially to mailbox controller.
If the controller use framework's queue, controller just receive the
first message and the others is queued in framework. The first message
could be executed exactly in vblank interval but the other messages are
sent to controller when controller notify framework tx_done in interrupt
handler. The interrupt may be postponed by some reason this is what we
worried. So Mediatek mailbox controller has to implement its own queue to
make sure that all message execute in vblank interval.
The reason that abort-function could let Mediatek mailbox controller use
framework's queue:
The primary concept is to let display driver send at most one message to
mailbox controller. When it need to send the second message before the
first message is done, it should abort the first message and then send the
second message which should merge the first message. For other client
driver, it could still send multiple messages into framework's queue.
CK Hu (3):
mailbox: Add ability for clients to abort data in channel
mailbox: mediatek: Implement abort_data function.
mailbox: mediatek: Remove busylist
drivers/mailbox/mailbox.c | 23 +++
drivers/mailbox/mtk-cmdq-mailbox.c | 281 +++++++----------------------
include/linux/mailbox_client.h | 1 +
include/linux/mailbox_controller.h | 4 +
4 files changed, 90 insertions(+), 219 deletions(-)
--
2.18.0
After implement abort_data, controller need not to implement its own
queue. Remove busylist because it's useless.
Signed-off-by: CK Hu <[email protected]>
---
drivers/mailbox/mtk-cmdq-mailbox.c | 255 ++++-------------------------
1 file changed, 29 insertions(+), 226 deletions(-)
diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
index f2219f263ef6..45c59f677ecb 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -16,9 +16,7 @@
#include <linux/mailbox/mtk-cmdq-mailbox.h>
#include <linux/of_device.h>
-#define CMDQ_OP_CODE_MASK (0xff << CMDQ_OP_CODE_SHIFT)
#define CMDQ_IRQ_MASK 0xffff
-#define CMDQ_NUM_CMD(t) (t->cmd_buf_size / CMDQ_INST_SIZE)
#define CMDQ_CURR_IRQ_STATUS 0x10
#define CMDQ_THR_SLOT_CYCLES 0x30
@@ -47,22 +45,10 @@
#define CMDQ_THR_IRQ_EN (CMDQ_THR_IRQ_ERROR | CMDQ_THR_IRQ_DONE)
#define CMDQ_THR_IS_WAITING BIT(31)
-#define CMDQ_JUMP_BY_OFFSET 0x10000000
-#define CMDQ_JUMP_BY_PA 0x10000001
-
struct cmdq_thread {
struct mbox_chan *chan;
void __iomem *base;
- struct list_head task_busy_list;
u32 priority;
- bool atomic_exec;
-};
-
-struct cmdq_task {
- struct cmdq *cmdq;
- struct list_head list_entry;
- dma_addr_t pa_base;
- struct cmdq_thread *thread;
struct cmdq_pkt *pkt; /* the packet sent from mailbox client */
};
@@ -130,171 +116,47 @@ static void cmdq_thread_disable(struct cmdq *cmdq, struct cmdq_thread *thread)
writel(CMDQ_THR_DISABLED, thread->base + CMDQ_THR_ENABLE_TASK);
}
-/* notify GCE to re-fetch commands by setting GCE thread PC */
-static void cmdq_thread_invalidate_fetched_data(struct cmdq_thread *thread)
-{
- writel(readl(thread->base + CMDQ_THR_CURR_ADDR),
- thread->base + CMDQ_THR_CURR_ADDR);
-}
-
-static void cmdq_task_insert_into_thread(struct cmdq_task *task)
-{
- struct device *dev = task->cmdq->mbox.dev;
- struct cmdq_thread *thread = task->thread;
- struct cmdq_task *prev_task = list_last_entry(
- &thread->task_busy_list, typeof(*task), list_entry);
- u64 *prev_task_base = prev_task->pkt->va_base;
-
- /* let previous task jump to this 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;
- 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;
- 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;
- struct cmdq_cb_data data;
-
- WARN_ON(cb->cb == (cmdq_async_flush_cb)NULL);
- data.sta = sta;
- data.data = cb->data;
- cb->cb(data);
-
- list_del(&task->list_entry);
-}
-
-static void cmdq_task_handle_error(struct cmdq_task *task)
-{
- struct cmdq_thread *thread = task->thread;
- struct cmdq_task *next_task;
-
- dev_err(task->cmdq->mbox.dev, "task 0x%p error\n", task);
- WARN_ON(cmdq_thread_suspend(task->cmdq, thread) < 0);
- next_task = list_first_entry_or_null(&thread->task_busy_list,
- struct cmdq_task, list_entry);
- if (next_task)
- writel(next_task->pa_base, thread->base + CMDQ_THR_CURR_ADDR);
- cmdq_thread_resume(thread);
-}
-
static void cmdq_thread_irq_handler(struct cmdq *cmdq,
struct cmdq_thread *thread)
{
- struct cmdq_task *task, *tmp, *curr_task = NULL;
- u32 curr_pa, irq_flag, task_end_pa;
- bool err;
+ unsigned long flags;
+ u32 curr_pa, irq_flag, end_pa;
+ int ret = 0;
+ spin_lock_irqsave(&thread->chan->lock, flags);
irq_flag = readl(thread->base + CMDQ_THR_IRQ_STATUS);
writel(~irq_flag, thread->base + CMDQ_THR_IRQ_STATUS);
- /*
- * When ISR call this function, another CPU core could run
- * "release task" right before we acquire the spin lock, and thus
- * reset / disable this GCE thread, so we need to check the enable
- * bit of this GCE thread.
- */
- if (!(readl(thread->base + CMDQ_THR_ENABLE_TASK) & CMDQ_THR_ENABLED))
- return;
-
- if (irq_flag & CMDQ_THR_IRQ_ERROR)
- err = true;
- else if (irq_flag & CMDQ_THR_IRQ_DONE)
- err = false;
- else
- return;
-
curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR);
+ end_pa = readl(thread->base + CMDQ_THR_END_ADDR);
- list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
- list_entry) {
- task_end_pa = task->pa_base + task->pkt->cmd_buf_size;
- if (curr_pa >= task->pa_base && curr_pa < task_end_pa)
- curr_task = task;
-
- if (!curr_task || curr_pa == task_end_pa - CMDQ_INST_SIZE) {
- cmdq_task_exec_done(task, CMDQ_CB_NORMAL);
- kfree(task);
- } else if (err) {
- cmdq_task_exec_done(task, CMDQ_CB_ERROR);
- cmdq_task_handle_error(curr_task);
- kfree(task);
- }
-
- if (curr_task)
- break;
- }
+ if (curr_pa != end_pa || irq_flag & CMDQ_THR_IRQ_ERROR)
+ ret = -EFAULT;
- if (list_empty(&thread->task_busy_list)) {
- cmdq_thread_disable(cmdq, thread);
- clk_disable(cmdq->clock);
- }
+ thread->pkt = NULL;
+ cmdq_thread_disable(cmdq, thread);
+ clk_disable(cmdq->clock);
+ spin_unlock_irqrestore(&thread->chan->lock, flags);
+ mbox_chan_txdone(thread->chan, ret);
}
static irqreturn_t cmdq_irq_handler(int irq, void *dev)
{
struct cmdq *cmdq = dev;
- unsigned long irq_status, flags = 0L;
+ unsigned long irq_status;
int bit;
irq_status = readl(cmdq->base + CMDQ_CURR_IRQ_STATUS) & CMDQ_IRQ_MASK;
if (!(irq_status ^ CMDQ_IRQ_MASK))
return IRQ_NONE;
- for_each_clear_bit(bit, &irq_status, fls(CMDQ_IRQ_MASK)) {
- struct cmdq_thread *thread = &cmdq->thread[bit];
-
- spin_lock_irqsave(&thread->chan->lock, flags);
- cmdq_thread_irq_handler(cmdq, thread);
- spin_unlock_irqrestore(&thread->chan->lock, flags);
- }
+ for_each_clear_bit(bit, &irq_status, fls(CMDQ_IRQ_MASK))
+ cmdq_thread_irq_handler(cmdq, &cmdq->thread[bit]);
return IRQ_HANDLED;
}
@@ -310,7 +172,7 @@ static int cmdq_suspend(struct device *dev)
for (i = 0; i < cmdq->thread_nr; i++) {
thread = &cmdq->thread[i];
- if (!list_empty(&thread->task_busy_list)) {
+ if (thread->pkt) {
task_running = true;
break;
}
@@ -347,72 +209,21 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
struct cmdq_pkt *pkt = (struct cmdq_pkt *)data;
struct cmdq_thread *thread = (struct cmdq_thread *)chan->con_priv;
struct cmdq *cmdq = dev_get_drvdata(chan->mbox->dev);
- struct cmdq_task *task;
- unsigned long curr_pa, end_pa;
/* Client should not flush new tasks if suspended. */
WARN_ON(cmdq->suspended);
- task = kzalloc(sizeof(*task), GFP_ATOMIC);
- if (!task)
- return -ENOMEM;
+ thread->pkt = pkt;
- task->cmdq = cmdq;
- INIT_LIST_HEAD(&task->list_entry);
- task->pa_base = pkt->pa_base;
- task->thread = thread;
- task->pkt = pkt;
-
- if (list_empty(&thread->task_busy_list)) {
- WARN_ON(clk_enable(cmdq->clock) < 0);
- WARN_ON(cmdq_thread_reset(cmdq, thread) < 0);
-
- writel(task->pa_base, thread->base + CMDQ_THR_CURR_ADDR);
- writel(task->pa_base + pkt->cmd_buf_size,
- thread->base + CMDQ_THR_END_ADDR);
- writel(thread->priority, thread->base + CMDQ_THR_PRIORITY);
- writel(CMDQ_THR_IRQ_EN, thread->base + CMDQ_THR_IRQ_ENABLE);
- writel(CMDQ_THR_ENABLED, thread->base + CMDQ_THR_ENABLE_TASK);
- } else {
- WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
- curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR);
- end_pa = readl(thread->base + CMDQ_THR_END_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);
- WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
- /* set to this task directly */
- writel(task->pa_base,
- 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 {
- /* check boundary */
- if (curr_pa == end_pa - CMDQ_INST_SIZE ||
- curr_pa == end_pa) {
- /* set to this task directly */
- writel(task->pa_base,
- 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,
- thread->base + CMDQ_THR_END_ADDR);
- cmdq_thread_resume(thread);
- }
- list_move_tail(&task->list_entry, &thread->task_busy_list);
+ WARN_ON(clk_enable(cmdq->clock) < 0);
+ WARN_ON(cmdq_thread_reset(cmdq, thread) < 0);
+
+ writel(thread->pkt->pa_base, thread->base + CMDQ_THR_CURR_ADDR);
+ writel(thread->pkt->pa_base + pkt->cmd_buf_size,
+ thread->base + CMDQ_THR_END_ADDR);
+ writel(thread->priority, thread->base + CMDQ_THR_PRIORITY);
+ writel(CMDQ_THR_IRQ_EN, thread->base + CMDQ_THR_IRQ_ENABLE);
+ writel(CMDQ_THR_ENABLED, thread->base + CMDQ_THR_ENABLE_TASK);
return 0;
}
@@ -421,23 +232,18 @@ static void cmdq_mbox_abort_data(struct mbox_chan *chan)
{
struct cmdq_thread *thread = (struct cmdq_thread *)chan->con_priv;
struct cmdq *cmdq = dev_get_drvdata(chan->mbox->dev);
- struct cmdq_task *task, *tmp;
unsigned long flags;
u32 enable;
spin_lock_irqsave(&thread->chan->lock, flags);
- if (list_empty(&thread->task_busy_list))
+ if (!thread->pkt)
goto out;
WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
if (!cmdq_thread_is_in_wfe(thread))
goto wait;
- list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
- list_entry) {
- list_del(&task->list_entry);
- kfree(task);
- }
+ thread->pkt = NULL;
cmdq_thread_resume(thread);
cmdq_thread_disable(cmdq, thread);
@@ -483,7 +289,6 @@ 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];
@@ -539,8 +344,7 @@ static int cmdq_probe(struct platform_device *pdev)
cmdq->mbox.ops = &cmdq_mbox_chan_ops;
cmdq->mbox.of_xlate = cmdq_xlate;
- /* make use of TXDONE_BY_ACK */
- cmdq->mbox.txdone_irq = false;
+ cmdq->mbox.txdone_irq = true;
cmdq->mbox.txdone_poll = false;
cmdq->thread = devm_kcalloc(dev, cmdq->thread_nr,
@@ -551,7 +355,6 @@ static int cmdq_probe(struct platform_device *pdev)
for (i = 0; i < cmdq->thread_nr; i++) {
cmdq->thread[i].base = cmdq->base + CMDQ_THR_BASE +
CMDQ_THR_SIZE * i;
- INIT_LIST_HEAD(&cmdq->thread[i].task_busy_list);
cmdq->mbox.chans[i].con_priv = (void *)&cmdq->thread[i];
}
--
2.18.1
This patch supplies a new framework API, mbox_abort_channel(), and
a new controller interface, abort_data().
For some client's application, it need to clean up the data in channel
but keep the channel so it could send data to channel later.
Signed-off-by: CK Hu <[email protected]>
---
drivers/mailbox/mailbox.c | 23 +++++++++++++++++++++++
include/linux/mailbox_client.h | 1 +
include/linux/mailbox_controller.h | 4 ++++
3 files changed, 28 insertions(+)
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index c6a7d4582dc6..281647162c76 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -428,6 +428,29 @@ struct mbox_chan *mbox_request_channel_byname(struct mbox_client *cl,
}
EXPORT_SYMBOL_GPL(mbox_request_channel_byname);
+/**
+ * mbox_abort_channel - The client abort all data in a mailbox
+ * channel by this call.
+ * @chan: The mailbox channel to be aborted.
+ */
+void mbox_abort_channel(struct mbox_chan *chan)
+{
+ unsigned long flags;
+
+ if (!chan || !chan->cl)
+ return;
+
+ if (chan->mbox->ops->abort_data)
+ chan->mbox->ops->abort_data(chan);
+
+ /* The queued TX requests are simply aborted, no callbacks are made */
+ spin_lock_irqsave(&chan->lock, flags);
+ chan->cl = NULL;
+ chan->active_req = NULL;
+ spin_unlock_irqrestore(&chan->lock, flags);
+}
+EXPORT_SYMBOL_GPL(mbox_abort_channel);
+
/**
* mbox_free_channel - The client relinquishes control of a mailbox
* channel by this call.
diff --git a/include/linux/mailbox_client.h b/include/linux/mailbox_client.h
index faa7da3c9c8b..209d1d458029 100644
--- a/include/linux/mailbox_client.h
+++ b/include/linux/mailbox_client.h
@@ -47,6 +47,7 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg);
int mbox_flush(struct mbox_chan *chan, unsigned long timeout);
void mbox_client_txdone(struct mbox_chan *chan, int r); /* atomic */
bool mbox_client_peek_data(struct mbox_chan *chan); /* atomic */
+void mbox_abort_channel(struct mbox_chan *chan); /* may sleep */
void mbox_free_channel(struct mbox_chan *chan); /* may sleep */
#endif /* __MAILBOX_CLIENT_H */
diff --git a/include/linux/mailbox_controller.h b/include/linux/mailbox_controller.h
index 4994a438444c..518aa4ca2fe4 100644
--- a/include/linux/mailbox_controller.h
+++ b/include/linux/mailbox_controller.h
@@ -27,6 +27,9 @@ struct mbox_chan;
* @flush: Called when a client requests transmissions to be blocking but
* the context doesn't allow sleeping. Typically the controller
* will implement a busy loop waiting for the data to flush out.
+ * @abort_data: Called when a client relinquishes control of a chan.
+ * This call may block too. The controller may do stuff
+ * that need to sleep.
* @startup: Called when a client requests the chan. The controller
* could ask clients for additional parameters of communication
* to be provided via client's chan_data. This call may
@@ -50,6 +53,7 @@ struct mbox_chan;
struct mbox_chan_ops {
int (*send_data)(struct mbox_chan *chan, void *data);
int (*flush)(struct mbox_chan *chan, unsigned long timeout);
+ void (*abort_data)(struct mbox_chan *chan);
int (*startup)(struct mbox_chan *chan);
void (*shutdown)(struct mbox_chan *chan);
bool (*last_tx_done)(struct mbox_chan *chan);
--
2.18.1
For client driver which need to reorganize the command buffer, it could
use this function to abort the sent but not executed command buffer.
Signed-off-by: CK Hu <[email protected]>
---
drivers/mailbox/mtk-cmdq-mailbox.c | 40 ++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
index 22811784dc7d..f2219f263ef6 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -417,6 +417,45 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
return 0;
}
+static void cmdq_mbox_abort_data(struct mbox_chan *chan)
+{
+ struct cmdq_thread *thread = (struct cmdq_thread *)chan->con_priv;
+ struct cmdq *cmdq = dev_get_drvdata(chan->mbox->dev);
+ struct cmdq_task *task, *tmp;
+ unsigned long flags;
+ u32 enable;
+
+ spin_lock_irqsave(&thread->chan->lock, flags);
+ if (list_empty(&thread->task_busy_list))
+ goto out;
+
+ WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
+ if (!cmdq_thread_is_in_wfe(thread))
+ goto wait;
+
+ list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
+ list_entry) {
+ list_del(&task->list_entry);
+ kfree(task);
+ }
+
+ cmdq_thread_resume(thread);
+ cmdq_thread_disable(cmdq, thread);
+ clk_disable(cmdq->clock);
+
+out:
+ spin_unlock_irqrestore(&thread->chan->lock, flags);
+ return;
+
+wait:
+ cmdq_thread_resume(thread);
+ spin_unlock_irqrestore(&thread->chan->lock, flags);
+ if (readl_poll_timeout_atomic(thread->base + CMDQ_THR_ENABLE_TASK,
+ enable, !enable, 1, 20))
+ dev_err(cmdq->mbox.dev, "Fail to wait GCE thread 0x%x done\n",
+ (u32)(thread->base - cmdq->base));
+}
+
static int cmdq_mbox_startup(struct mbox_chan *chan)
{
return 0;
@@ -427,6 +466,7 @@ static void cmdq_mbox_shutdown(struct mbox_chan *chan)
}
static const struct mbox_chan_ops cmdq_mbox_chan_ops = {
+ .abort_data = cmdq_mbox_abort_data,
.send_data = cmdq_mbox_send_data,
.startup = cmdq_mbox_startup,
.shutdown = cmdq_mbox_shutdown,
--
2.18.1
On Tue, Jan 15, 2019 at 11:07 PM CK Hu <[email protected]> wrote:
>
> This patch supplies a new framework API, mbox_abort_channel(), and
> a new controller interface, abort_data().
>
> For some client's application, it need to clean up the data in channel
> but keep the channel so it could send data to channel later.
>
> Signed-off-by: CK Hu <[email protected]>
> ---
> drivers/mailbox/mailbox.c | 23 +++++++++++++++++++++++
> include/linux/mailbox_client.h | 1 +
> include/linux/mailbox_controller.h | 4 ++++
> 3 files changed, 28 insertions(+)
>
> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> index c6a7d4582dc6..281647162c76 100644
> --- a/drivers/mailbox/mailbox.c
> +++ b/drivers/mailbox/mailbox.c
> @@ -428,6 +428,29 @@ struct mbox_chan *mbox_request_channel_byname(struct mbox_client *cl,
> }
> EXPORT_SYMBOL_GPL(mbox_request_channel_byname);
>
> +/**
> + * mbox_abort_channel - The client abort all data in a mailbox
> + * channel by this call.
> + * @chan: The mailbox channel to be aborted.
> + */
> +void mbox_abort_channel(struct mbox_chan *chan)
> +{
> + unsigned long flags;
> +
> + if (!chan || !chan->cl)
> + return;
> +
> + if (chan->mbox->ops->abort_data)
> + chan->mbox->ops->abort_data(chan);
> +
> + /* The queued TX requests are simply aborted, no callbacks are made */
> + spin_lock_irqsave(&chan->lock, flags);
> + chan->cl = NULL;
> + chan->active_req = NULL;
> + spin_unlock_irqrestore(&chan->lock, flags);
> +}
>
Why not just release and then request channel again ?
mbox_abort_channel() is just a copy of mbox_free_channel() and if the
abort can sleep, that is more reason to just do that.
Hi, Jassi:
On Wed, 2019-01-16 at 10:22 -0600, Jassi Brar wrote:
> On Tue, Jan 15, 2019 at 11:07 PM CK Hu <[email protected]> wrote:
> >
> > This patch supplies a new framework API, mbox_abort_channel(), and
> > a new controller interface, abort_data().
> >
> > For some client's application, it need to clean up the data in channel
> > but keep the channel so it could send data to channel later.
> >
> > Signed-off-by: CK Hu <[email protected]>
> > ---
> > drivers/mailbox/mailbox.c | 23 +++++++++++++++++++++++
> > include/linux/mailbox_client.h | 1 +
> > include/linux/mailbox_controller.h | 4 ++++
> > 3 files changed, 28 insertions(+)
> >
> > diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> > index c6a7d4582dc6..281647162c76 100644
> > --- a/drivers/mailbox/mailbox.c
> > +++ b/drivers/mailbox/mailbox.c
> > @@ -428,6 +428,29 @@ struct mbox_chan *mbox_request_channel_byname(struct mbox_client *cl,
> > }
> > EXPORT_SYMBOL_GPL(mbox_request_channel_byname);
> >
> > +/**
> > + * mbox_abort_channel - The client abort all data in a mailbox
> > + * channel by this call.
> > + * @chan: The mailbox channel to be aborted.
> > + */
> > +void mbox_abort_channel(struct mbox_chan *chan)
> > +{
> > + unsigned long flags;
> > +
> > + if (!chan || !chan->cl)
> > + return;
> > +
> > + if (chan->mbox->ops->abort_data)
> > + chan->mbox->ops->abort_data(chan);
> > +
> > + /* The queued TX requests are simply aborted, no callbacks are made */
> > + spin_lock_irqsave(&chan->lock, flags);
> > + chan->cl = NULL;
> > + chan->active_req = NULL;
> > + spin_unlock_irqrestore(&chan->lock, flags);
> > +}
> >
> Why not just release and then request channel again ?
> mbox_abort_channel() is just a copy of mbox_free_channel() and if the
> abort can sleep, that is more reason to just do that.
The cursor may change position 300 times in one second, so I still
concern the processing time. Request and free channel looks too heavy to
do 300 times per second. Conceptually, I just want to clean up the
channel rather than free and request it, so they are somewhat different.
I say it may sleep because mbox_abort_channel() is a copy of
mbox_free_channel(). I could change it to 'must not sleep' because
Mediatek controller does not sleep in abort callback.
Regards,
CK
On Thu, Jan 17, 2019 at 2:00 AM CK Hu <[email protected]> wrote:
>
> Hi, Jassi:
>
> On Wed, 2019-01-16 at 10:22 -0600, Jassi Brar wrote:
> > On Tue, Jan 15, 2019 at 11:07 PM CK Hu <[email protected]> wrote:
> > >
> > > This patch supplies a new framework API, mbox_abort_channel(), and
> > > a new controller interface, abort_data().
> > >
> > > For some client's application, it need to clean up the data in channel
> > > but keep the channel so it could send data to channel later.
> > >
> > > Signed-off-by: CK Hu <[email protected]>
> > > ---
> > > drivers/mailbox/mailbox.c | 23 +++++++++++++++++++++++
> > > include/linux/mailbox_client.h | 1 +
> > > include/linux/mailbox_controller.h | 4 ++++
> > > 3 files changed, 28 insertions(+)
> > >
> > > diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> > > index c6a7d4582dc6..281647162c76 100644
> > > --- a/drivers/mailbox/mailbox.c
> > > +++ b/drivers/mailbox/mailbox.c
> > > @@ -428,6 +428,29 @@ struct mbox_chan *mbox_request_channel_byname(struct mbox_client *cl,
> > > }
> > > EXPORT_SYMBOL_GPL(mbox_request_channel_byname);
> > >
> > > +/**
> > > + * mbox_abort_channel - The client abort all data in a mailbox
> > > + * channel by this call.
> > > + * @chan: The mailbox channel to be aborted.
> > > + */
> > > +void mbox_abort_channel(struct mbox_chan *chan)
> > > +{
> > > + unsigned long flags;
> > > +
> > > + if (!chan || !chan->cl)
> > > + return;
> > > +
> > > + if (chan->mbox->ops->abort_data)
> > > + chan->mbox->ops->abort_data(chan);
> > > +
> > > + /* The queued TX requests are simply aborted, no callbacks are made */
> > > + spin_lock_irqsave(&chan->lock, flags);
> > > + chan->cl = NULL;
> > > + chan->active_req = NULL;
> > > + spin_unlock_irqrestore(&chan->lock, flags);
> > > +}
> > >
> > Why not just release and then request channel again ?
> > mbox_abort_channel() is just a copy of mbox_free_channel() and if the
> > abort can sleep, that is more reason to just do that.
>
> The cursor may change position 300 times in one second, so I still
> concern the processing time. Request and free channel looks too heavy to
> do 300 times per second.
You send data 300Hz, not abort ... which is supposed to be a rare event.
And then your shutdown/startup is currently empty, while abort isn't
quite lightweight.
> Conceptually, I just want to clean up the
> channel rather than free and request it, so they are somewhat different.
> I say it may sleep because mbox_abort_channel() is a copy of
> mbox_free_channel(). I could change it to 'must not sleep' because
> Mediatek controller does not sleep in abort callback.
>
No please. Just use the shutdown/startup.
Thanks.
Hi CK,
On Tue, 2019-01-29 at 17:20 +0800, Houlong Wei (魏厚龙) wrote:
> -----Original Message-----
> From: CK Hu [mailto:[email protected]]
> Sent: Wednesday, January 16, 2019 1:05 PM
> To: Jassi Brar <[email protected]>; Matthias Brugger <[email protected]>; Houlong Wei (魏厚龙) <[email protected]>
> Cc: [email protected]; [email protected]; [email protected]; srv_heupstream <[email protected]>; CK Hu (胡俊光) <[email protected]>
> Subject: [PATCH 3/3] mailbox: mediatek: Remove busylist
>
> After implement abort_data, controller need not to implement its own queue. Remove busylist because it's useless.
Remove busy list in controller makes client driver have no way to queue
pkt in gce hardware thread, which may hurt display and multimedia
performance since each pkt waits IRQ delay before previous pkt. Suggest
keep busy list design.
Regards,
Dennis
>
> Signed-off-by: CK Hu <[email protected]>
> ---
> drivers/mailbox/mtk-cmdq-mailbox.c | 255 ++++-------------------------
> 1 file changed, 29 insertions(+), 226 deletions(-)
>
> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> index f2219f263ef6..45c59f677ecb 100644
> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> @@ -16,9 +16,7 @@
> #include <linux/mailbox/mtk-cmdq-mailbox.h>
> #include <linux/of_device.h>
>
> -#define CMDQ_OP_CODE_MASK (0xff << CMDQ_OP_CODE_SHIFT)
> #define CMDQ_IRQ_MASK 0xffff
> -#define CMDQ_NUM_CMD(t) (t->cmd_buf_size / CMDQ_INST_SIZE)
>
> #define CMDQ_CURR_IRQ_STATUS 0x10
> #define CMDQ_THR_SLOT_CYCLES 0x30
> @@ -47,22 +45,10 @@
> #define CMDQ_THR_IRQ_EN (CMDQ_THR_IRQ_ERROR | CMDQ_THR_IRQ_DONE)
> #define CMDQ_THR_IS_WAITING BIT(31)
>
> -#define CMDQ_JUMP_BY_OFFSET 0x10000000
> -#define CMDQ_JUMP_BY_PA 0x10000001
> -
> struct cmdq_thread {
> struct mbox_chan *chan;
> void __iomem *base;
> - struct list_head task_busy_list;
> u32 priority;
> - bool atomic_exec;
> -};
> -
> -struct cmdq_task {
> - struct cmdq *cmdq;
> - struct list_head list_entry;
> - dma_addr_t pa_base;
> - struct cmdq_thread *thread;
> struct cmdq_pkt *pkt; /* the packet sent from mailbox client */
> };
>
> @@ -130,171 +116,47 @@ static void cmdq_thread_disable(struct cmdq *cmdq, struct cmdq_thread *thread)
> writel(CMDQ_THR_DISABLED, thread->base + CMDQ_THR_ENABLE_TASK); }
>
> -/* notify GCE to re-fetch commands by setting GCE thread PC */ -static void cmdq_thread_invalidate_fetched_data(struct cmdq_thread *thread) -{
> - writel(readl(thread->base + CMDQ_THR_CURR_ADDR),
> - thread->base + CMDQ_THR_CURR_ADDR);
> -}
> -
> -static void cmdq_task_insert_into_thread(struct cmdq_task *task) -{
> - struct device *dev = task->cmdq->mbox.dev;
> - struct cmdq_thread *thread = task->thread;
> - struct cmdq_task *prev_task = list_last_entry(
> - &thread->task_busy_list, typeof(*task), list_entry);
> - u64 *prev_task_base = prev_task->pkt->va_base;
> -
> - /* let previous task jump to this 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;
> - 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;
> - 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;
> - struct cmdq_cb_data data;
> -
> - WARN_ON(cb->cb == (cmdq_async_flush_cb)NULL);
> - data.sta = sta;
> - data.data = cb->data;
> - cb->cb(data);
> -
> - list_del(&task->list_entry);
> -}
> -
> -static void cmdq_task_handle_error(struct cmdq_task *task) -{
> - struct cmdq_thread *thread = task->thread;
> - struct cmdq_task *next_task;
> -
> - dev_err(task->cmdq->mbox.dev, "task 0x%p error\n", task);
> - WARN_ON(cmdq_thread_suspend(task->cmdq, thread) < 0);
> - next_task = list_first_entry_or_null(&thread->task_busy_list,
> - struct cmdq_task, list_entry);
> - if (next_task)
> - writel(next_task->pa_base, thread->base + CMDQ_THR_CURR_ADDR);
> - cmdq_thread_resume(thread);
> -}
> -
> static void cmdq_thread_irq_handler(struct cmdq *cmdq,
> struct cmdq_thread *thread)
> {
> - struct cmdq_task *task, *tmp, *curr_task = NULL;
> - u32 curr_pa, irq_flag, task_end_pa;
> - bool err;
> + unsigned long flags;
> + u32 curr_pa, irq_flag, end_pa;
> + int ret = 0;
>
> + spin_lock_irqsave(&thread->chan->lock, flags);
> irq_flag = readl(thread->base + CMDQ_THR_IRQ_STATUS);
> writel(~irq_flag, thread->base + CMDQ_THR_IRQ_STATUS);
>
> - /*
> - * When ISR call this function, another CPU core could run
> - * "release task" right before we acquire the spin lock, and thus
> - * reset / disable this GCE thread, so we need to check the enable
> - * bit of this GCE thread.
> - */
> - if (!(readl(thread->base + CMDQ_THR_ENABLE_TASK) & CMDQ_THR_ENABLED))
> - return;
> -
> - if (irq_flag & CMDQ_THR_IRQ_ERROR)
> - err = true;
> - else if (irq_flag & CMDQ_THR_IRQ_DONE)
> - err = false;
> - else
> - return;
> -
> curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR);
> + end_pa = readl(thread->base + CMDQ_THR_END_ADDR);
>
> - list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> - list_entry) {
> - task_end_pa = task->pa_base + task->pkt->cmd_buf_size;
> - if (curr_pa >= task->pa_base && curr_pa < task_end_pa)
> - curr_task = task;
> -
> - if (!curr_task || curr_pa == task_end_pa - CMDQ_INST_SIZE) {
> - cmdq_task_exec_done(task, CMDQ_CB_NORMAL);
> - kfree(task);
> - } else if (err) {
> - cmdq_task_exec_done(task, CMDQ_CB_ERROR);
> - cmdq_task_handle_error(curr_task);
> - kfree(task);
> - }
> -
> - if (curr_task)
> - break;
> - }
> + if (curr_pa != end_pa || irq_flag & CMDQ_THR_IRQ_ERROR)
> + ret = -EFAULT;
>
> - if (list_empty(&thread->task_busy_list)) {
> - cmdq_thread_disable(cmdq, thread);
> - clk_disable(cmdq->clock);
> - }
> + thread->pkt = NULL;
> + cmdq_thread_disable(cmdq, thread);
> + clk_disable(cmdq->clock);
> + spin_unlock_irqrestore(&thread->chan->lock, flags);
> + mbox_chan_txdone(thread->chan, ret);
> }
>
> static irqreturn_t cmdq_irq_handler(int irq, void *dev) {
> struct cmdq *cmdq = dev;
> - unsigned long irq_status, flags = 0L;
> + unsigned long irq_status;
> int bit;
>
> irq_status = readl(cmdq->base + CMDQ_CURR_IRQ_STATUS) & CMDQ_IRQ_MASK;
> if (!(irq_status ^ CMDQ_IRQ_MASK))
> return IRQ_NONE;
>
> - for_each_clear_bit(bit, &irq_status, fls(CMDQ_IRQ_MASK)) {
> - struct cmdq_thread *thread = &cmdq->thread[bit];
> -
> - spin_lock_irqsave(&thread->chan->lock, flags);
> - cmdq_thread_irq_handler(cmdq, thread);
> - spin_unlock_irqrestore(&thread->chan->lock, flags);
> - }
> + for_each_clear_bit(bit, &irq_status, fls(CMDQ_IRQ_MASK))
> + cmdq_thread_irq_handler(cmdq, &cmdq->thread[bit]);
>
> return IRQ_HANDLED;
> }
> @@ -310,7 +172,7 @@ static int cmdq_suspend(struct device *dev)
>
> for (i = 0; i < cmdq->thread_nr; i++) {
> thread = &cmdq->thread[i];
> - if (!list_empty(&thread->task_busy_list)) {
> + if (thread->pkt) {
> task_running = true;
> break;
> }
> @@ -347,72 +209,21 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
> struct cmdq_pkt *pkt = (struct cmdq_pkt *)data;
> struct cmdq_thread *thread = (struct cmdq_thread *)chan->con_priv;
> struct cmdq *cmdq = dev_get_drvdata(chan->mbox->dev);
> - struct cmdq_task *task;
> - unsigned long curr_pa, end_pa;
>
> /* Client should not flush new tasks if suspended. */
> WARN_ON(cmdq->suspended);
>
> - task = kzalloc(sizeof(*task), GFP_ATOMIC);
> - if (!task)
> - return -ENOMEM;
> + thread->pkt = pkt;
>
> - task->cmdq = cmdq;
> - INIT_LIST_HEAD(&task->list_entry);
> - task->pa_base = pkt->pa_base;
> - task->thread = thread;
> - task->pkt = pkt;
> -
> - if (list_empty(&thread->task_busy_list)) {
> - WARN_ON(clk_enable(cmdq->clock) < 0);
> - WARN_ON(cmdq_thread_reset(cmdq, thread) < 0);
> -
> - writel(task->pa_base, thread->base + CMDQ_THR_CURR_ADDR);
> - writel(task->pa_base + pkt->cmd_buf_size,
> - thread->base + CMDQ_THR_END_ADDR);
> - writel(thread->priority, thread->base + CMDQ_THR_PRIORITY);
> - writel(CMDQ_THR_IRQ_EN, thread->base + CMDQ_THR_IRQ_ENABLE);
> - writel(CMDQ_THR_ENABLED, thread->base + CMDQ_THR_ENABLE_TASK);
> - } else {
> - WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
> - curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR);
> - end_pa = readl(thread->base + CMDQ_THR_END_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);
> - WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
> - /* set to this task directly */
> - writel(task->pa_base,
> - 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 {
> - /* check boundary */
> - if (curr_pa == end_pa - CMDQ_INST_SIZE ||
> - curr_pa == end_pa) {
> - /* set to this task directly */
> - writel(task->pa_base,
> - 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,
> - thread->base + CMDQ_THR_END_ADDR);
> - cmdq_thread_resume(thread);
> - }
> - list_move_tail(&task->list_entry, &thread->task_busy_list);
> + WARN_ON(clk_enable(cmdq->clock) < 0);
> + WARN_ON(cmdq_thread_reset(cmdq, thread) < 0);
> +
> + writel(thread->pkt->pa_base, thread->base + CMDQ_THR_CURR_ADDR);
> + writel(thread->pkt->pa_base + pkt->cmd_buf_size,
> + thread->base + CMDQ_THR_END_ADDR);
> + writel(thread->priority, thread->base + CMDQ_THR_PRIORITY);
> + writel(CMDQ_THR_IRQ_EN, thread->base + CMDQ_THR_IRQ_ENABLE);
> + writel(CMDQ_THR_ENABLED, thread->base + CMDQ_THR_ENABLE_TASK);
>
> return 0;
> }
> @@ -421,23 +232,18 @@ static void cmdq_mbox_abort_data(struct mbox_chan *chan) {
> struct cmdq_thread *thread = (struct cmdq_thread *)chan->con_priv;
> struct cmdq *cmdq = dev_get_drvdata(chan->mbox->dev);
> - struct cmdq_task *task, *tmp;
> unsigned long flags;
> u32 enable;
>
> spin_lock_irqsave(&thread->chan->lock, flags);
> - if (list_empty(&thread->task_busy_list))
> + if (!thread->pkt)
> goto out;
>
> WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
> if (!cmdq_thread_is_in_wfe(thread))
> goto wait;
>
> - list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> - list_entry) {
> - list_del(&task->list_entry);
> - kfree(task);
> - }
> + thread->pkt = NULL;
>
> cmdq_thread_resume(thread);
> cmdq_thread_disable(cmdq, thread);
> @@ -483,7 +289,6 @@ 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];
> @@ -539,8 +344,7 @@ static int cmdq_probe(struct platform_device *pdev)
> cmdq->mbox.ops = &cmdq_mbox_chan_ops;
> cmdq->mbox.of_xlate = cmdq_xlate;
>
> - /* make use of TXDONE_BY_ACK */
> - cmdq->mbox.txdone_irq = false;
> + cmdq->mbox.txdone_irq = true;
> cmdq->mbox.txdone_poll = false;
>
> cmdq->thread = devm_kcalloc(dev, cmdq->thread_nr, @@ -551,7 +355,6 @@ static int cmdq_probe(struct platform_device *pdev)
> for (i = 0; i < cmdq->thread_nr; i++) {
> cmdq->thread[i].base = cmdq->base + CMDQ_THR_BASE +
> CMDQ_THR_SIZE * i;
> - INIT_LIST_HEAD(&cmdq->thread[i].task_busy_list);
> cmdq->mbox.chans[i].con_priv = (void *)&cmdq->thread[i];
> }
>
> --
> 2.18.1
>
Hi, Dennis:
On Tue, 2019-02-12 at 10:18 +0800, Dennis-YC Hsieh wrote:
> Hi CK,
>
> On Tue, 2019-01-29 at 17:20 +0800, Houlong Wei (魏厚龙) wrote:
> > -----Original Message-----
> > From: CK Hu [mailto:[email protected]]
> > Sent: Wednesday, January 16, 2019 1:05 PM
> > To: Jassi Brar <[email protected]>; Matthias Brugger <[email protected]>; Houlong Wei (魏厚龙) <[email protected]>
> > Cc: [email protected]; [email protected]; [email protected]; srv_heupstream <[email protected]>; CK Hu (胡俊光) <[email protected]>
> > Subject: [PATCH 3/3] mailbox: mediatek: Remove busylist
> >
> > After implement abort_data, controller need not to implement its own queue. Remove busylist because it's useless.
>
> Remove busy list in controller makes client driver have no way to queue
> pkt in gce hardware thread, which may hurt display and multimedia
> performance since each pkt waits IRQ delay before previous pkt. Suggest
> keep busy list design.
If some client driver need gce to cascade pkt to prevent irq delay, I
should keep busylist. For drm driver, I still want to apply the first
two patches of this series and remove atomic feature because drm could
keep just one pkt and reuse it to prevent frequently allocate/free pkt
and the total commands executed in vblank period would be
well-controlled. Do you have any concern about removing atomic feature?
Regards,
CK
>
>
> Regards,
> Dennis
>
> >
> > Signed-off-by: CK Hu <[email protected]>
> > ---
> > drivers/mailbox/mtk-cmdq-mailbox.c | 255 ++++-------------------------
> > 1 file changed, 29 insertions(+), 226 deletions(-)
> >
> > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> > index f2219f263ef6..45c59f677ecb 100644
> > --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> > @@ -16,9 +16,7 @@
> > #include <linux/mailbox/mtk-cmdq-mailbox.h>
> > #include <linux/of_device.h>
> >
> > -#define CMDQ_OP_CODE_MASK (0xff << CMDQ_OP_CODE_SHIFT)
> > #define CMDQ_IRQ_MASK 0xffff
> > -#define CMDQ_NUM_CMD(t) (t->cmd_buf_size / CMDQ_INST_SIZE)
> >
> > #define CMDQ_CURR_IRQ_STATUS 0x10
> > #define CMDQ_THR_SLOT_CYCLES 0x30
> > @@ -47,22 +45,10 @@
> > #define CMDQ_THR_IRQ_EN (CMDQ_THR_IRQ_ERROR | CMDQ_THR_IRQ_DONE)
> > #define CMDQ_THR_IS_WAITING BIT(31)
> >
> > -#define CMDQ_JUMP_BY_OFFSET 0x10000000
> > -#define CMDQ_JUMP_BY_PA 0x10000001
> > -
> > struct cmdq_thread {
> > struct mbox_chan *chan;
> > void __iomem *base;
> > - struct list_head task_busy_list;
> > u32 priority;
> > - bool atomic_exec;
> > -};
> > -
> > -struct cmdq_task {
> > - struct cmdq *cmdq;
> > - struct list_head list_entry;
> > - dma_addr_t pa_base;
> > - struct cmdq_thread *thread;
> > struct cmdq_pkt *pkt; /* the packet sent from mailbox client */
> > };
> >
> > @@ -130,171 +116,47 @@ static void cmdq_thread_disable(struct cmdq *cmdq, struct cmdq_thread *thread)
> > writel(CMDQ_THR_DISABLED, thread->base + CMDQ_THR_ENABLE_TASK); }
> >
> > -/* notify GCE to re-fetch commands by setting GCE thread PC */ -static void cmdq_thread_invalidate_fetched_data(struct cmdq_thread *thread) -{
> > - writel(readl(thread->base + CMDQ_THR_CURR_ADDR),
> > - thread->base + CMDQ_THR_CURR_ADDR);
> > -}
> > -
> > -static void cmdq_task_insert_into_thread(struct cmdq_task *task) -{
> > - struct device *dev = task->cmdq->mbox.dev;
> > - struct cmdq_thread *thread = task->thread;
> > - struct cmdq_task *prev_task = list_last_entry(
> > - &thread->task_busy_list, typeof(*task), list_entry);
> > - u64 *prev_task_base = prev_task->pkt->va_base;
> > -
> > - /* let previous task jump to this 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;
> > - 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;
> > - 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;
> > - struct cmdq_cb_data data;
> > -
> > - WARN_ON(cb->cb == (cmdq_async_flush_cb)NULL);
> > - data.sta = sta;
> > - data.data = cb->data;
> > - cb->cb(data);
> > -
> > - list_del(&task->list_entry);
> > -}
> > -
> > -static void cmdq_task_handle_error(struct cmdq_task *task) -{
> > - struct cmdq_thread *thread = task->thread;
> > - struct cmdq_task *next_task;
> > -
> > - dev_err(task->cmdq->mbox.dev, "task 0x%p error\n", task);
> > - WARN_ON(cmdq_thread_suspend(task->cmdq, thread) < 0);
> > - next_task = list_first_entry_or_null(&thread->task_busy_list,
> > - struct cmdq_task, list_entry);
> > - if (next_task)
> > - writel(next_task->pa_base, thread->base + CMDQ_THR_CURR_ADDR);
> > - cmdq_thread_resume(thread);
> > -}
> > -
> > static void cmdq_thread_irq_handler(struct cmdq *cmdq,
> > struct cmdq_thread *thread)
> > {
> > - struct cmdq_task *task, *tmp, *curr_task = NULL;
> > - u32 curr_pa, irq_flag, task_end_pa;
> > - bool err;
> > + unsigned long flags;
> > + u32 curr_pa, irq_flag, end_pa;
> > + int ret = 0;
> >
> > + spin_lock_irqsave(&thread->chan->lock, flags);
> > irq_flag = readl(thread->base + CMDQ_THR_IRQ_STATUS);
> > writel(~irq_flag, thread->base + CMDQ_THR_IRQ_STATUS);
> >
> > - /*
> > - * When ISR call this function, another CPU core could run
> > - * "release task" right before we acquire the spin lock, and thus
> > - * reset / disable this GCE thread, so we need to check the enable
> > - * bit of this GCE thread.
> > - */
> > - if (!(readl(thread->base + CMDQ_THR_ENABLE_TASK) & CMDQ_THR_ENABLED))
> > - return;
> > -
> > - if (irq_flag & CMDQ_THR_IRQ_ERROR)
> > - err = true;
> > - else if (irq_flag & CMDQ_THR_IRQ_DONE)
> > - err = false;
> > - else
> > - return;
> > -
> > curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR);
> > + end_pa = readl(thread->base + CMDQ_THR_END_ADDR);
> >
> > - list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> > - list_entry) {
> > - task_end_pa = task->pa_base + task->pkt->cmd_buf_size;
> > - if (curr_pa >= task->pa_base && curr_pa < task_end_pa)
> > - curr_task = task;
> > -
> > - if (!curr_task || curr_pa == task_end_pa - CMDQ_INST_SIZE) {
> > - cmdq_task_exec_done(task, CMDQ_CB_NORMAL);
> > - kfree(task);
> > - } else if (err) {
> > - cmdq_task_exec_done(task, CMDQ_CB_ERROR);
> > - cmdq_task_handle_error(curr_task);
> > - kfree(task);
> > - }
> > -
> > - if (curr_task)
> > - break;
> > - }
> > + if (curr_pa != end_pa || irq_flag & CMDQ_THR_IRQ_ERROR)
> > + ret = -EFAULT;
> >
> > - if (list_empty(&thread->task_busy_list)) {
> > - cmdq_thread_disable(cmdq, thread);
> > - clk_disable(cmdq->clock);
> > - }
> > + thread->pkt = NULL;
> > + cmdq_thread_disable(cmdq, thread);
> > + clk_disable(cmdq->clock);
> > + spin_unlock_irqrestore(&thread->chan->lock, flags);
> > + mbox_chan_txdone(thread->chan, ret);
> > }
> >
> > static irqreturn_t cmdq_irq_handler(int irq, void *dev) {
> > struct cmdq *cmdq = dev;
> > - unsigned long irq_status, flags = 0L;
> > + unsigned long irq_status;
> > int bit;
> >
> > irq_status = readl(cmdq->base + CMDQ_CURR_IRQ_STATUS) & CMDQ_IRQ_MASK;
> > if (!(irq_status ^ CMDQ_IRQ_MASK))
> > return IRQ_NONE;
> >
> > - for_each_clear_bit(bit, &irq_status, fls(CMDQ_IRQ_MASK)) {
> > - struct cmdq_thread *thread = &cmdq->thread[bit];
> > -
> > - spin_lock_irqsave(&thread->chan->lock, flags);
> > - cmdq_thread_irq_handler(cmdq, thread);
> > - spin_unlock_irqrestore(&thread->chan->lock, flags);
> > - }
> > + for_each_clear_bit(bit, &irq_status, fls(CMDQ_IRQ_MASK))
> > + cmdq_thread_irq_handler(cmdq, &cmdq->thread[bit]);
> >
> > return IRQ_HANDLED;
> > }
> > @@ -310,7 +172,7 @@ static int cmdq_suspend(struct device *dev)
> >
> > for (i = 0; i < cmdq->thread_nr; i++) {
> > thread = &cmdq->thread[i];
> > - if (!list_empty(&thread->task_busy_list)) {
> > + if (thread->pkt) {
> > task_running = true;
> > break;
> > }
> > @@ -347,72 +209,21 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
> > struct cmdq_pkt *pkt = (struct cmdq_pkt *)data;
> > struct cmdq_thread *thread = (struct cmdq_thread *)chan->con_priv;
> > struct cmdq *cmdq = dev_get_drvdata(chan->mbox->dev);
> > - struct cmdq_task *task;
> > - unsigned long curr_pa, end_pa;
> >
> > /* Client should not flush new tasks if suspended. */
> > WARN_ON(cmdq->suspended);
> >
> > - task = kzalloc(sizeof(*task), GFP_ATOMIC);
> > - if (!task)
> > - return -ENOMEM;
> > + thread->pkt = pkt;
> >
> > - task->cmdq = cmdq;
> > - INIT_LIST_HEAD(&task->list_entry);
> > - task->pa_base = pkt->pa_base;
> > - task->thread = thread;
> > - task->pkt = pkt;
> > -
> > - if (list_empty(&thread->task_busy_list)) {
> > - WARN_ON(clk_enable(cmdq->clock) < 0);
> > - WARN_ON(cmdq_thread_reset(cmdq, thread) < 0);
> > -
> > - writel(task->pa_base, thread->base + CMDQ_THR_CURR_ADDR);
> > - writel(task->pa_base + pkt->cmd_buf_size,
> > - thread->base + CMDQ_THR_END_ADDR);
> > - writel(thread->priority, thread->base + CMDQ_THR_PRIORITY);
> > - writel(CMDQ_THR_IRQ_EN, thread->base + CMDQ_THR_IRQ_ENABLE);
> > - writel(CMDQ_THR_ENABLED, thread->base + CMDQ_THR_ENABLE_TASK);
> > - } else {
> > - WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
> > - curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR);
> > - end_pa = readl(thread->base + CMDQ_THR_END_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);
> > - WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
> > - /* set to this task directly */
> > - writel(task->pa_base,
> > - 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 {
> > - /* check boundary */
> > - if (curr_pa == end_pa - CMDQ_INST_SIZE ||
> > - curr_pa == end_pa) {
> > - /* set to this task directly */
> > - writel(task->pa_base,
> > - 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,
> > - thread->base + CMDQ_THR_END_ADDR);
> > - cmdq_thread_resume(thread);
> > - }
> > - list_move_tail(&task->list_entry, &thread->task_busy_list);
> > + WARN_ON(clk_enable(cmdq->clock) < 0);
> > + WARN_ON(cmdq_thread_reset(cmdq, thread) < 0);
> > +
> > + writel(thread->pkt->pa_base, thread->base + CMDQ_THR_CURR_ADDR);
> > + writel(thread->pkt->pa_base + pkt->cmd_buf_size,
> > + thread->base + CMDQ_THR_END_ADDR);
> > + writel(thread->priority, thread->base + CMDQ_THR_PRIORITY);
> > + writel(CMDQ_THR_IRQ_EN, thread->base + CMDQ_THR_IRQ_ENABLE);
> > + writel(CMDQ_THR_ENABLED, thread->base + CMDQ_THR_ENABLE_TASK);
> >
> > return 0;
> > }
> > @@ -421,23 +232,18 @@ static void cmdq_mbox_abort_data(struct mbox_chan *chan) {
> > struct cmdq_thread *thread = (struct cmdq_thread *)chan->con_priv;
> > struct cmdq *cmdq = dev_get_drvdata(chan->mbox->dev);
> > - struct cmdq_task *task, *tmp;
> > unsigned long flags;
> > u32 enable;
> >
> > spin_lock_irqsave(&thread->chan->lock, flags);
> > - if (list_empty(&thread->task_busy_list))
> > + if (!thread->pkt)
> > goto out;
> >
> > WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
> > if (!cmdq_thread_is_in_wfe(thread))
> > goto wait;
> >
> > - list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> > - list_entry) {
> > - list_del(&task->list_entry);
> > - kfree(task);
> > - }
> > + thread->pkt = NULL;
> >
> > cmdq_thread_resume(thread);
> > cmdq_thread_disable(cmdq, thread);
> > @@ -483,7 +289,6 @@ 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];
> > @@ -539,8 +344,7 @@ static int cmdq_probe(struct platform_device *pdev)
> > cmdq->mbox.ops = &cmdq_mbox_chan_ops;
> > cmdq->mbox.of_xlate = cmdq_xlate;
> >
> > - /* make use of TXDONE_BY_ACK */
> > - cmdq->mbox.txdone_irq = false;
> > + cmdq->mbox.txdone_irq = true;
> > cmdq->mbox.txdone_poll = false;
> >
> > cmdq->thread = devm_kcalloc(dev, cmdq->thread_nr, @@ -551,7 +355,6 @@ static int cmdq_probe(struct platform_device *pdev)
> > for (i = 0; i < cmdq->thread_nr; i++) {
> > cmdq->thread[i].base = cmdq->base + CMDQ_THR_BASE +
> > CMDQ_THR_SIZE * i;
> > - INIT_LIST_HEAD(&cmdq->thread[i].task_busy_list);
> > cmdq->mbox.chans[i].con_priv = (void *)&cmdq->thread[i];
> > }
> >
> > --
> > 2.18.1
> >
>
>
Hi CK,
On Fri, 2019-02-15 at 00:01 +0800, CK Hu wrote:
> Hi, Dennis:
>
> On Tue, 2019-02-12 at 10:18 +0800, Dennis-YC Hsieh wrote:
> > Hi CK,
> >
> > On Tue, 2019-01-29 at 17:20 +0800, Houlong Wei (魏厚龙) wrote:
> > > -----Original Message-----
> > > From: CK Hu [mailto:[email protected]]
> > > Sent: Wednesday, January 16, 2019 1:05 PM
> > > To: Jassi Brar <[email protected]>; Matthias Brugger <[email protected]>; Houlong Wei (魏厚龙) <[email protected]>
> > > Cc: [email protected]; [email protected]; [email protected]; srv_heupstream <[email protected]>; CK Hu (胡俊光) <[email protected]>
> > > Subject: [PATCH 3/3] mailbox: mediatek: Remove busylist
> > >
> > > After implement abort_data, controller need not to implement its own queue. Remove busylist because it's useless.
> >
> > Remove busy list in controller makes client driver have no way to queue
> > pkt in gce hardware thread, which may hurt display and multimedia
> > performance since each pkt waits IRQ delay before previous pkt. Suggest
> > keep busy list design.
>
> If some client driver need gce to cascade pkt to prevent irq delay, I
> should keep busylist. For drm driver, I still want to apply the first
> two patches of this series and remove atomic feature because drm could
> keep just one pkt and reuse it to prevent frequently allocate/free pkt
> and the total commands executed in vblank period would be
> well-controlled. Do you have any concern about removing atomic feature?
>
> Regards,
> CK
>
I have no concern on remove atomic feature.
Regards,
Dennis
> >
> >
> > Regards,
> > Dennis
> >
> > >
> > > Signed-off-by: CK Hu <[email protected]>
> > > ---
> > > drivers/mailbox/mtk-cmdq-mailbox.c | 255 ++++-------------------------
> > > 1 file changed, 29 insertions(+), 226 deletions(-)
> > >
> > > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> > > index f2219f263ef6..45c59f677ecb 100644
> > > --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> > > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> > > @@ -16,9 +16,7 @@
> > > #include <linux/mailbox/mtk-cmdq-mailbox.h>
> > > #include <linux/of_device.h>
> > >
> > > -#define CMDQ_OP_CODE_MASK (0xff << CMDQ_OP_CODE_SHIFT)
> > > #define CMDQ_IRQ_MASK 0xffff
> > > -#define CMDQ_NUM_CMD(t) (t->cmd_buf_size / CMDQ_INST_SIZE)
> > >
> > > #define CMDQ_CURR_IRQ_STATUS 0x10
> > > #define CMDQ_THR_SLOT_CYCLES 0x30
> > > @@ -47,22 +45,10 @@
> > > #define CMDQ_THR_IRQ_EN (CMDQ_THR_IRQ_ERROR | CMDQ_THR_IRQ_DONE)
> > > #define CMDQ_THR_IS_WAITING BIT(31)
> > >
> > > -#define CMDQ_JUMP_BY_OFFSET 0x10000000
> > > -#define CMDQ_JUMP_BY_PA 0x10000001
> > > -
> > > struct cmdq_thread {
> > > struct mbox_chan *chan;
> > > void __iomem *base;
> > > - struct list_head task_busy_list;
> > > u32 priority;
> > > - bool atomic_exec;
> > > -};
> > > -
> > > -struct cmdq_task {
> > > - struct cmdq *cmdq;
> > > - struct list_head list_entry;
> > > - dma_addr_t pa_base;
> > > - struct cmdq_thread *thread;
> > > struct cmdq_pkt *pkt; /* the packet sent from mailbox client */
> > > };
> > >
> > > @@ -130,171 +116,47 @@ static void cmdq_thread_disable(struct cmdq *cmdq, struct cmdq_thread *thread)
> > > writel(CMDQ_THR_DISABLED, thread->base + CMDQ_THR_ENABLE_TASK); }
> > >
> > > -/* notify GCE to re-fetch commands by setting GCE thread PC */ -static void cmdq_thread_invalidate_fetched_data(struct cmdq_thread *thread) -{
> > > - writel(readl(thread->base + CMDQ_THR_CURR_ADDR),
> > > - thread->base + CMDQ_THR_CURR_ADDR);
> > > -}
> > > -
> > > -static void cmdq_task_insert_into_thread(struct cmdq_task *task) -{
> > > - struct device *dev = task->cmdq->mbox.dev;
> > > - struct cmdq_thread *thread = task->thread;
> > > - struct cmdq_task *prev_task = list_last_entry(
> > > - &thread->task_busy_list, typeof(*task), list_entry);
> > > - u64 *prev_task_base = prev_task->pkt->va_base;
> > > -
> > > - /* let previous task jump to this 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;
> > > - 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;
> > > - 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;
> > > - struct cmdq_cb_data data;
> > > -
> > > - WARN_ON(cb->cb == (cmdq_async_flush_cb)NULL);
> > > - data.sta = sta;
> > > - data.data = cb->data;
> > > - cb->cb(data);
> > > -
> > > - list_del(&task->list_entry);
> > > -}
> > > -
> > > -static void cmdq_task_handle_error(struct cmdq_task *task) -{
> > > - struct cmdq_thread *thread = task->thread;
> > > - struct cmdq_task *next_task;
> > > -
> > > - dev_err(task->cmdq->mbox.dev, "task 0x%p error\n", task);
> > > - WARN_ON(cmdq_thread_suspend(task->cmdq, thread) < 0);
> > > - next_task = list_first_entry_or_null(&thread->task_busy_list,
> > > - struct cmdq_task, list_entry);
> > > - if (next_task)
> > > - writel(next_task->pa_base, thread->base + CMDQ_THR_CURR_ADDR);
> > > - cmdq_thread_resume(thread);
> > > -}
> > > -
> > > static void cmdq_thread_irq_handler(struct cmdq *cmdq,
> > > struct cmdq_thread *thread)
> > > {
> > > - struct cmdq_task *task, *tmp, *curr_task = NULL;
> > > - u32 curr_pa, irq_flag, task_end_pa;
> > > - bool err;
> > > + unsigned long flags;
> > > + u32 curr_pa, irq_flag, end_pa;
> > > + int ret = 0;
> > >
> > > + spin_lock_irqsave(&thread->chan->lock, flags);
> > > irq_flag = readl(thread->base + CMDQ_THR_IRQ_STATUS);
> > > writel(~irq_flag, thread->base + CMDQ_THR_IRQ_STATUS);
> > >
> > > - /*
> > > - * When ISR call this function, another CPU core could run
> > > - * "release task" right before we acquire the spin lock, and thus
> > > - * reset / disable this GCE thread, so we need to check the enable
> > > - * bit of this GCE thread.
> > > - */
> > > - if (!(readl(thread->base + CMDQ_THR_ENABLE_TASK) & CMDQ_THR_ENABLED))
> > > - return;
> > > -
> > > - if (irq_flag & CMDQ_THR_IRQ_ERROR)
> > > - err = true;
> > > - else if (irq_flag & CMDQ_THR_IRQ_DONE)
> > > - err = false;
> > > - else
> > > - return;
> > > -
> > > curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR);
> > > + end_pa = readl(thread->base + CMDQ_THR_END_ADDR);
> > >
> > > - list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> > > - list_entry) {
> > > - task_end_pa = task->pa_base + task->pkt->cmd_buf_size;
> > > - if (curr_pa >= task->pa_base && curr_pa < task_end_pa)
> > > - curr_task = task;
> > > -
> > > - if (!curr_task || curr_pa == task_end_pa - CMDQ_INST_SIZE) {
> > > - cmdq_task_exec_done(task, CMDQ_CB_NORMAL);
> > > - kfree(task);
> > > - } else if (err) {
> > > - cmdq_task_exec_done(task, CMDQ_CB_ERROR);
> > > - cmdq_task_handle_error(curr_task);
> > > - kfree(task);
> > > - }
> > > -
> > > - if (curr_task)
> > > - break;
> > > - }
> > > + if (curr_pa != end_pa || irq_flag & CMDQ_THR_IRQ_ERROR)
> > > + ret = -EFAULT;
> > >
> > > - if (list_empty(&thread->task_busy_list)) {
> > > - cmdq_thread_disable(cmdq, thread);
> > > - clk_disable(cmdq->clock);
> > > - }
> > > + thread->pkt = NULL;
> > > + cmdq_thread_disable(cmdq, thread);
> > > + clk_disable(cmdq->clock);
> > > + spin_unlock_irqrestore(&thread->chan->lock, flags);
> > > + mbox_chan_txdone(thread->chan, ret);
> > > }
> > >
> > > static irqreturn_t cmdq_irq_handler(int irq, void *dev) {
> > > struct cmdq *cmdq = dev;
> > > - unsigned long irq_status, flags = 0L;
> > > + unsigned long irq_status;
> > > int bit;
> > >
> > > irq_status = readl(cmdq->base + CMDQ_CURR_IRQ_STATUS) & CMDQ_IRQ_MASK;
> > > if (!(irq_status ^ CMDQ_IRQ_MASK))
> > > return IRQ_NONE;
> > >
> > > - for_each_clear_bit(bit, &irq_status, fls(CMDQ_IRQ_MASK)) {
> > > - struct cmdq_thread *thread = &cmdq->thread[bit];
> > > -
> > > - spin_lock_irqsave(&thread->chan->lock, flags);
> > > - cmdq_thread_irq_handler(cmdq, thread);
> > > - spin_unlock_irqrestore(&thread->chan->lock, flags);
> > > - }
> > > + for_each_clear_bit(bit, &irq_status, fls(CMDQ_IRQ_MASK))
> > > + cmdq_thread_irq_handler(cmdq, &cmdq->thread[bit]);
> > >
> > > return IRQ_HANDLED;
> > > }
> > > @@ -310,7 +172,7 @@ static int cmdq_suspend(struct device *dev)
> > >
> > > for (i = 0; i < cmdq->thread_nr; i++) {
> > > thread = &cmdq->thread[i];
> > > - if (!list_empty(&thread->task_busy_list)) {
> > > + if (thread->pkt) {
> > > task_running = true;
> > > break;
> > > }
> > > @@ -347,72 +209,21 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
> > > struct cmdq_pkt *pkt = (struct cmdq_pkt *)data;
> > > struct cmdq_thread *thread = (struct cmdq_thread *)chan->con_priv;
> > > struct cmdq *cmdq = dev_get_drvdata(chan->mbox->dev);
> > > - struct cmdq_task *task;
> > > - unsigned long curr_pa, end_pa;
> > >
> > > /* Client should not flush new tasks if suspended. */
> > > WARN_ON(cmdq->suspended);
> > >
> > > - task = kzalloc(sizeof(*task), GFP_ATOMIC);
> > > - if (!task)
> > > - return -ENOMEM;
> > > + thread->pkt = pkt;
> > >
> > > - task->cmdq = cmdq;
> > > - INIT_LIST_HEAD(&task->list_entry);
> > > - task->pa_base = pkt->pa_base;
> > > - task->thread = thread;
> > > - task->pkt = pkt;
> > > -
> > > - if (list_empty(&thread->task_busy_list)) {
> > > - WARN_ON(clk_enable(cmdq->clock) < 0);
> > > - WARN_ON(cmdq_thread_reset(cmdq, thread) < 0);
> > > -
> > > - writel(task->pa_base, thread->base + CMDQ_THR_CURR_ADDR);
> > > - writel(task->pa_base + pkt->cmd_buf_size,
> > > - thread->base + CMDQ_THR_END_ADDR);
> > > - writel(thread->priority, thread->base + CMDQ_THR_PRIORITY);
> > > - writel(CMDQ_THR_IRQ_EN, thread->base + CMDQ_THR_IRQ_ENABLE);
> > > - writel(CMDQ_THR_ENABLED, thread->base + CMDQ_THR_ENABLE_TASK);
> > > - } else {
> > > - WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
> > > - curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR);
> > > - end_pa = readl(thread->base + CMDQ_THR_END_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);
> > > - WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
> > > - /* set to this task directly */
> > > - writel(task->pa_base,
> > > - 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 {
> > > - /* check boundary */
> > > - if (curr_pa == end_pa - CMDQ_INST_SIZE ||
> > > - curr_pa == end_pa) {
> > > - /* set to this task directly */
> > > - writel(task->pa_base,
> > > - 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,
> > > - thread->base + CMDQ_THR_END_ADDR);
> > > - cmdq_thread_resume(thread);
> > > - }
> > > - list_move_tail(&task->list_entry, &thread->task_busy_list);
> > > + WARN_ON(clk_enable(cmdq->clock) < 0);
> > > + WARN_ON(cmdq_thread_reset(cmdq, thread) < 0);
> > > +
> > > + writel(thread->pkt->pa_base, thread->base + CMDQ_THR_CURR_ADDR);
> > > + writel(thread->pkt->pa_base + pkt->cmd_buf_size,
> > > + thread->base + CMDQ_THR_END_ADDR);
> > > + writel(thread->priority, thread->base + CMDQ_THR_PRIORITY);
> > > + writel(CMDQ_THR_IRQ_EN, thread->base + CMDQ_THR_IRQ_ENABLE);
> > > + writel(CMDQ_THR_ENABLED, thread->base + CMDQ_THR_ENABLE_TASK);
> > >
> > > return 0;
> > > }
> > > @@ -421,23 +232,18 @@ static void cmdq_mbox_abort_data(struct mbox_chan *chan) {
> > > struct cmdq_thread *thread = (struct cmdq_thread *)chan->con_priv;
> > > struct cmdq *cmdq = dev_get_drvdata(chan->mbox->dev);
> > > - struct cmdq_task *task, *tmp;
> > > unsigned long flags;
> > > u32 enable;
> > >
> > > spin_lock_irqsave(&thread->chan->lock, flags);
> > > - if (list_empty(&thread->task_busy_list))
> > > + if (!thread->pkt)
> > > goto out;
> > >
> > > WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
> > > if (!cmdq_thread_is_in_wfe(thread))
> > > goto wait;
> > >
> > > - list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> > > - list_entry) {
> > > - list_del(&task->list_entry);
> > > - kfree(task);
> > > - }
> > > + thread->pkt = NULL;
> > >
> > > cmdq_thread_resume(thread);
> > > cmdq_thread_disable(cmdq, thread);
> > > @@ -483,7 +289,6 @@ 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];
> > > @@ -539,8 +344,7 @@ static int cmdq_probe(struct platform_device *pdev)
> > > cmdq->mbox.ops = &cmdq_mbox_chan_ops;
> > > cmdq->mbox.of_xlate = cmdq_xlate;
> > >
> > > - /* make use of TXDONE_BY_ACK */
> > > - cmdq->mbox.txdone_irq = false;
> > > + cmdq->mbox.txdone_irq = true;
> > > cmdq->mbox.txdone_poll = false;
> > >
> > > cmdq->thread = devm_kcalloc(dev, cmdq->thread_nr, @@ -551,7 +355,6 @@ static int cmdq_probe(struct platform_device *pdev)
> > > for (i = 0; i < cmdq->thread_nr; i++) {
> > > cmdq->thread[i].base = cmdq->base + CMDQ_THR_BASE +
> > > CMDQ_THR_SIZE * i;
> > > - INIT_LIST_HEAD(&cmdq->thread[i].task_busy_list);
> > > cmdq->mbox.chans[i].con_priv = (void *)&cmdq->thread[i];
> > > }
> > >
> > > --
> > > 2.18.1
> > >
> >
> >
>
>