Support gce on mt6779 platform.
These patches depend on patch:
support gce on mt8183 platform
(https://patchwork.kernel.org/cover/11255147/)
Support gce function on mt6779 platform.
dt-binding: gce: add gce header file for mt6779
mailbox: cmdq: variablize address shift in platform
mailbox: cmdq: support mt6779 gce platform definition
arm64: dts: add gce node for mt6779
Refine driver to support stop hardware with safe callback.
mailbox: mediatek: cmdq: clear task in channel before shutdown
Dennis YC Hsieh (14):
dt-binding: gce: add gce header file for mt6779
mailbox: cmdq: variablize address shift in platform
mailbox: cmdq: support mt6779 gce platform definition
mailbox: mediatek: cmdq: clear task in channel before shutdown
arm64: dts: add gce node for mt6779
soc: mediatek: cmdq: return send msg error code
soc: mediatek: cmdq: add assign function
soc: mediatek: cmdq: add write_s function
soc: mediatek: cmdq: add read_s function
soc: mediatek: cmdq: add write_s value function
soc: mediatek: cmdq: export finalize function
soc: mediatek: cmdq: add loop function
soc: mediatek: cmdq: add wait no clear event function
soc: mediatek: cmdq: add set event function
.../devicetree/bindings/mailbox/mtk-gce.txt | 8 +-
arch/arm64/boot/dts/mediatek/mt6779.dtsi | 10 +
drivers/mailbox/mtk-cmdq-mailbox.c | 85 ++++++-
drivers/soc/mediatek/mtk-cmdq-helper.c | 181 +++++++++++++-
include/dt-bindings/gce/mt6779-gce.h | 222 ++++++++++++++++++
include/linux/mailbox/mtk-cmdq-mailbox.h | 7 +
include/linux/soc/mediatek/mtk-cmdq.h | 87 +++++++
7 files changed, 575 insertions(+), 25 deletions(-)
create mode 100644 include/dt-bindings/gce/mt6779-gce.h
Add wait no clear event function in cmdq helper functions to wait specific
event without clear to 0 after receive it.
Signed-off-by: Dennis YC Hsieh <[email protected]>
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 15 +++++++++++++++
include/linux/soc/mediatek/mtk-cmdq.h | 10 ++++++++++
2 files changed, 25 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 10a9b4481e58..6f270fadfb50 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -330,6 +330,21 @@ int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
}
EXPORT_SYMBOL(cmdq_pkt_wfe);
+int cmdq_pkt_wait_no_clear(struct cmdq_pkt *pkt, u16 event)
+{
+ struct cmdq_instruction inst = { {0} };
+
+ if (event >= CMDQ_MAX_EVENT)
+ return -EINVAL;
+
+ inst.op = CMDQ_CODE_WFE;
+ inst.value = CMDQ_WFE_WAIT | CMDQ_WFE_WAIT_VALUE;
+ inst.event = event;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_wait_no_clear);
+
int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
index d15d8c941992..40bc61ad8d31 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -149,6 +149,16 @@ int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, dma_addr_t addr,
*/
int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event);
+/**
+ * cmdq_pkt_wait_no_clear() - Append wait for event command to the CMDQ packet,
+ * without update event to 0 after receive it.
+ * @pkt: the CMDQ packet
+ * @event: the desired event type to wait
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_wait_no_clear(struct cmdq_pkt *pkt, u16 event);
+
/**
* cmdq_pkt_clear_event() - append clear event command to the CMDQ packet
* @pkt: the CMDQ packet
--
2.18.0
Some gce hardware shift pc and end address in register to support
large dram addressing.
Implement gce address shift when write or read pc and end register.
And add shift bit in platform definition.
Signed-off-by: Dennis YC Hsieh <[email protected]>
---
drivers/mailbox/mtk-cmdq-mailbox.c | 57 ++++++++++++++++++------
drivers/soc/mediatek/mtk-cmdq-helper.c | 3 +-
include/linux/mailbox/mtk-cmdq-mailbox.h | 2 +
3 files changed, 48 insertions(+), 14 deletions(-)
diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
index 9a6ce9f5a7db..d5536563fce1 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -76,8 +76,21 @@ struct cmdq {
struct cmdq_thread *thread;
struct clk *clock;
bool suspended;
+ u8 shift_pa;
};
+struct gce_plat {
+ u32 thread_nr;
+ u8 shift;
+};
+
+u8 cmdq_mbox_shift(struct mbox_chan *chan)
+{
+ struct cmdq *cmdq = container_of(chan->mbox, struct cmdq, mbox);
+
+ return cmdq->shift_pa;
+}
+
static int cmdq_thread_suspend(struct cmdq *cmdq, struct cmdq_thread *thread)
{
u32 status;
@@ -176,6 +189,7 @@ static void cmdq_task_remove_wfe(struct cmdq_task *task)
{
struct device *dev = task->cmdq->mbox.dev;
u64 *base = task->pkt->va_base;
+ struct cmdq *cmdq = task->cmdq;
int i;
dma_sync_single_for_cpu(dev, task->pa_base, task->pkt->cmd_buf_size,
@@ -183,7 +197,7 @@ static void cmdq_task_remove_wfe(struct cmdq_task *task)
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;
+ CMDQ_JUMP_PASS >> cmdq->shift_pa;
dma_sync_single_for_device(dev, task->pa_base, task->pkt->cmd_buf_size,
DMA_TO_DEVICE);
}
@@ -221,13 +235,15 @@ static void cmdq_task_handle_error(struct cmdq_task *task)
{
struct cmdq_thread *thread = task->thread;
struct cmdq_task *next_task;
+ struct cmdq *cmdq = task->cmdq;
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);
+ writel(next_task->pa_base >> cmdq->shift_pa,
+ thread->base + CMDQ_THR_CURR_ADDR);
cmdq_thread_resume(thread);
}
@@ -257,7 +273,7 @@ static void cmdq_thread_irq_handler(struct cmdq *cmdq,
else
return;
- curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR);
+ curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR) << cmdq->shift_pa;
list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
list_entry) {
@@ -373,16 +389,20 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
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,
+ writel(task->pa_base >> cmdq->shift_pa,
+ thread->base + CMDQ_THR_CURR_ADDR);
+ writel((task->pa_base + pkt->cmd_buf_size) >> cmdq->shift_pa,
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);
+ curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR) <<
+ cmdq->shift_pa;
+ end_pa = readl(thread->base + CMDQ_THR_END_ADDR) <<
+ cmdq->shift_pa;
/*
* Atomic execution should remove the following wfe, i.e. only
@@ -395,7 +415,7 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
cmdq_thread_wait_end(thread, end_pa);
WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
/* set to this task directly */
- writel(task->pa_base,
+ writel(task->pa_base >> cmdq->shift_pa,
thread->base + CMDQ_THR_CURR_ADDR);
} else {
cmdq_task_insert_into_thread(task);
@@ -407,14 +427,14 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
if (curr_pa == end_pa - CMDQ_INST_SIZE ||
curr_pa == end_pa) {
/* set to this task directly */
- writel(task->pa_base,
+ writel(task->pa_base >> cmdq->shift_pa,
thread->base + CMDQ_THR_CURR_ADDR);
} else {
cmdq_task_insert_into_thread(task);
smp_mb(); /* modify jump before enable thread */
}
}
- writel(task->pa_base + pkt->cmd_buf_size,
+ writel((task->pa_base + pkt->cmd_buf_size) >> cmdq->shift_pa,
thread->base + CMDQ_THR_END_ADDR);
cmdq_thread_resume(thread);
}
@@ -461,6 +481,7 @@ static int cmdq_probe(struct platform_device *pdev)
struct resource *res;
struct cmdq *cmdq;
int err, i;
+ struct gce_plat *plat_data;
cmdq = devm_kzalloc(dev, sizeof(*cmdq), GFP_KERNEL);
if (!cmdq)
@@ -479,7 +500,14 @@ static int cmdq_probe(struct platform_device *pdev)
return -EINVAL;
}
- cmdq->thread_nr = (u32)(unsigned long)of_device_get_match_data(dev);
+ plat_data = (struct gce_plat *)of_device_get_match_data(dev);
+ if (!plat_data) {
+ dev_err(dev, "failed to get match data\n");
+ return -EINVAL;
+ }
+
+ cmdq->thread_nr = plat_data->thread_nr;
+ cmdq->shift_pa = plat_data->shift;
cmdq->irq_mask = GENMASK(cmdq->thread_nr - 1, 0);
err = devm_request_irq(dev, cmdq->irq, cmdq_irq_handler, IRQF_SHARED,
"mtk_cmdq", cmdq);
@@ -542,9 +570,12 @@ static const struct dev_pm_ops cmdq_pm_ops = {
.resume = cmdq_resume,
};
+static const struct gce_plat gce_plat_v2 = {.thread_nr = 16, .shift = 0};
+static const struct gce_plat gce_plat_v3 = {.thread_nr = 24, .shift = 0};
+
static const struct of_device_id cmdq_of_ids[] = {
- {.compatible = "mediatek,mt8173-gce", .data = (void *)16},
- {.compatible = "mediatek,mt8183-gce", .data = (void *)24},
+ {.compatible = "mediatek,mt8173-gce", .data = (void *)&gce_plat_v2},
+ {.compatible = "mediatek,mt8183-gce", .data = (void *)&gce_plat_v3},
{}
};
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 9add0fd5fa6c..274f6f311d05 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -281,6 +281,7 @@ EXPORT_SYMBOL(cmdq_pkt_poll_mask);
static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
{
+ struct cmdq_client *cl = pkt->cl;
struct cmdq_instruction inst = { {0} };
int err;
@@ -293,7 +294,7 @@ static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
/* JUMP to end */
inst.op = CMDQ_CODE_JUMP;
- inst.value = CMDQ_JUMP_PASS;
+ inst.value = CMDQ_JUMP_PASS >> cmdq_mbox_shift(cl->chan);
err = cmdq_pkt_append_command(pkt, inst);
return err;
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
index a4dc45fbec0a..dfe5b2eb85cc 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -88,4 +88,6 @@ struct cmdq_pkt {
void *cl;
};
+u8 cmdq_mbox_shift(struct mbox_chan *chan);
+
#endif /* __MTK_CMDQ_MAILBOX_H__ */
--
2.18.0
add gce device node for mt6779
Signed-off-by: Dennis YC Hsieh <[email protected]>
---
arch/arm64/boot/dts/mediatek/mt6779.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt6779.dtsi b/arch/arm64/boot/dts/mediatek/mt6779.dtsi
index daa25b75788f..10d59385f4a1 100644
--- a/arch/arm64/boot/dts/mediatek/mt6779.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt6779.dtsi
@@ -8,6 +8,7 @@
#include <dt-bindings/clock/mt6779-clk.h>
#include <dt-bindings/interrupt-controller/irq.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/gce/mt6779-gce.h>
/ {
compatible = "mediatek,mt6779";
@@ -159,6 +160,15 @@
#clock-cells = <1>;
};
+ gce: mailbox@10228000 {
+ compatible = "mediatek,mt6779-gce";
+ reg = <0 0x10228000 0 0x4000>;
+ interrupts = <GIC_SPI 185 IRQ_TYPE_LEVEL_LOW>;
+ #mbox-cells = <3>;
+ clocks = <&infracfg_ao CLK_INFRA_GCE>;
+ clock-names = "gce";
+ };
+
uart0: serial@11002000 {
compatible = "mediatek,mt6779-uart",
"mediatek,mt6577-uart";
--
2.18.0
Return error code to client if send message fail,
so that client has chance to error handling.
Signed-off-by: Dennis YC Hsieh <[email protected]>
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 274f6f311d05..8421b4090304 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -353,11 +353,11 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt, cmdq_async_flush_cb cb,
spin_unlock_irqrestore(&client->lock, flags);
}
- mbox_send_message(client->chan, pkt);
+ err = mbox_send_message(client->chan, pkt);
/* We can send next packet immediately, so just call txdone. */
mbox_client_txdone(client->chan, 0);
- return 0;
+ return err;
}
EXPORT_SYMBOL(cmdq_pkt_flush_async);
--
2.18.0
Add gce v4 hardware support with different thread number and shift.
Signed-off-by: Dennis YC Hsieh <[email protected]>
---
drivers/mailbox/mtk-cmdq-mailbox.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
index d5536563fce1..fd519b6f518b 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -572,10 +572,12 @@ static const struct dev_pm_ops cmdq_pm_ops = {
static const struct gce_plat gce_plat_v2 = {.thread_nr = 16, .shift = 0};
static const struct gce_plat gce_plat_v3 = {.thread_nr = 24, .shift = 0};
+static const struct gce_plat gce_plat_v4 = {.thread_nr = 24, .shift = 3};
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,mt6779-gce", .data = (void *)&gce_plat_v4},
{}
};
--
2.18.0
add write_s function in cmdq helper functions which
writes value contains in internal register to address
with large dma access support.
Signed-off-by: Dennis YC Hsieh <[email protected]>
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 40 ++++++++++++++++++++++++
include/linux/mailbox/mtk-cmdq-mailbox.h | 2 ++
include/linux/soc/mediatek/mtk-cmdq.h | 12 +++++++
3 files changed, 54 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 9cc234f08ec5..2edbc0954d97 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -15,11 +15,18 @@
#define CMDQ_EOC_CMD ((u64)((CMDQ_CODE_EOC << CMDQ_OP_CODE_SHIFT)) \
<< 32 | CMDQ_EOC_IRQ_EN)
#define CMDQ_REG_TYPE 1
+#define CMDQ_ADDR_HIGH(addr) ((u32)(((addr) >> 16) & GENMASK(31, 0)))
+#define CMDQ_ADDR_LOW_BIT BIT(1)
+#define CMDQ_ADDR_LOW(addr) ((u16)(addr) | CMDQ_ADDR_LOW_BIT)
struct cmdq_instruction {
union {
u32 value;
u32 mask;
+ struct {
+ u16 arg_c;
+ u16 arg_b;
+ };
};
union {
u16 offset;
@@ -224,6 +231,39 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
}
EXPORT_SYMBOL(cmdq_pkt_write_mask);
+int cmdq_pkt_write_s(struct cmdq_pkt *pkt, phys_addr_t addr, u16 reg_idx,
+ u32 mask)
+{
+ struct cmdq_instruction inst = { {0} };
+ const u16 dst_reg_idx = CMDQ_SPR_TEMP;
+ int err;
+
+ if (mask != U32_MAX) {
+ inst.op = CMDQ_CODE_MASK;
+ inst.mask = ~mask;
+ err = cmdq_pkt_append_command(pkt, inst);
+ if (err < 0)
+ return err;
+
+ inst.mask = 0;
+ inst.op = CMDQ_CODE_WRITE_S_MASK;
+ } else {
+ inst.op = CMDQ_CODE_WRITE_S;
+ }
+
+ err = cmdq_pkt_assign(pkt, dst_reg_idx, CMDQ_ADDR_HIGH(addr));
+ if (err < 0)
+ return err;
+
+ inst.arg_b_t = CMDQ_REG_TYPE;
+ inst.sop = dst_reg_idx;
+ inst.offset = CMDQ_ADDR_LOW(addr);
+ inst.arg_b = reg_idx;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_s);
+
int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 121c3bb6d3de..8ef87e1bd03b 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -59,6 +59,8 @@ enum cmdq_code {
CMDQ_CODE_JUMP = 0x10,
CMDQ_CODE_WFE = 0x20,
CMDQ_CODE_EOC = 0x40,
+ CMDQ_CODE_WRITE_S = 0x90,
+ CMDQ_CODE_WRITE_S_MASK = 0x91,
CMDQ_CODE_LOGIC = 0xa0,
};
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
index c66b3a0da2a2..56ff1970197c 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -106,6 +106,18 @@ int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16 offset, u32 value);
int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
u16 offset, u32 value, u32 mask);
+/**
+ * cmdq_pkt_write_s_mask() - append write_s command to the CMDQ packet
+ * @pkt: the CMDQ packet
+ * @addr: the physical address of register or dma
+ * @reg_idx: the CMDQ internal register ID which cache source value
+ * @mask: the specified target register mask
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_write_s(struct cmdq_pkt *pkt, phys_addr_t addr, u16 reg_idx,
+ u32 mask);
+
/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
* @pkt: the CMDQ packet
--
2.18.0
*** BLURB HERE ***
Dennis YC Hsieh (14):
dt-binding: gce: add gce header file for mt6779
mailbox: cmdq: variablize address shift in platform
mailbox: cmdq: support mt6779 gce platform definition
mailbox: mediatek: cmdq: clear task in channel before shutdown
arm64: dts: add gce node for mt6779
soc: mediatek: cmdq: return send msg error code
soc: mediatek: cmdq: add assign function
soc: mediatek: cmdq: add write_s function
soc: mediatek: cmdq: add read_s function
soc: mediatek: cmdq: add write_s value function
soc: mediatek: cmdq: export finalize function
soc: mediatek: cmdq: add loop function
soc: mediatek: cmdq: add wait no clear event function
soc: mediatek: cmdq: add set event function
.../devicetree/bindings/mailbox/mtk-gce.txt | 8 +-
arch/arm64/boot/dts/mediatek/mt6779.dtsi | 10 +
drivers/mailbox/mtk-cmdq-mailbox.c | 85 ++++++-
drivers/soc/mediatek/mtk-cmdq-helper.c | 181 +++++++++++++-
include/dt-bindings/gce/mt6779-gce.h | 222 ++++++++++++++++++
include/linux/mailbox/mtk-cmdq-mailbox.h | 7 +
include/linux/soc/mediatek/mtk-cmdq.h | 87 +++++++
7 files changed, 575 insertions(+), 25 deletions(-)
create mode 100644 include/dt-bindings/gce/mt6779-gce.h
--
2.18.0
Do success callback in channel when shutdown. For those task not finish,
callback with error code thus client has chance to cleanup or reset.
Signed-off-by: Dennis YC Hsieh <[email protected]>
---
drivers/mailbox/mtk-cmdq-mailbox.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
index fd519b6f518b..c12a768d1175 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -450,6 +450,32 @@ static int cmdq_mbox_startup(struct mbox_chan *chan)
static void cmdq_mbox_shutdown(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;
+
+ spin_lock_irqsave(&thread->chan->lock, flags);
+ if (list_empty(&thread->task_busy_list))
+ goto done;
+
+ WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
+
+ /* make sure executed tasks have success callback */
+ cmdq_thread_irq_handler(cmdq, thread);
+ if (list_empty(&thread->task_busy_list))
+ goto done;
+
+ list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
+ list_entry) {
+ cmdq_task_exec_done(task, -ECONNABORTED);
+ kfree(task);
+ }
+
+ cmdq_thread_disable(cmdq, thread);
+ clk_disable(cmdq->clock);
+done:
+ spin_unlock_irqrestore(&thread->chan->lock, flags);
}
static const struct mbox_chan_ops cmdq_mbox_chan_ops = {
--
2.18.0
Add read_s function in cmdq helper functions which support read value from
register or dma physical address into gce internal register.
Signed-off-by: Dennis YC Hsieh <[email protected]>
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 20 ++++++++++++++++++++
include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
include/linux/soc/mediatek/mtk-cmdq.h | 10 ++++++++++
3 files changed, 31 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 2edbc0954d97..2cd693e34980 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -231,6 +231,26 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
}
EXPORT_SYMBOL(cmdq_pkt_write_mask);
+int cmdq_pkt_read_s(struct cmdq_pkt *pkt, phys_addr_t addr, u16 reg_idx)
+{
+ struct cmdq_instruction inst = { {0} };
+ int err;
+ const u16 src_reg_idx = CMDQ_SPR_TEMP;
+
+ err = cmdq_pkt_assign(pkt, src_reg_idx, CMDQ_ADDR_HIGH(addr));
+ if (err < 0)
+ return err;
+
+ inst.op = CMDQ_CODE_READ_S;
+ inst.dst_t = CMDQ_REG_TYPE;
+ inst.sop = src_reg_idx;
+ inst.reg_dst = reg_idx;
+ inst.arg_b = CMDQ_ADDR_LOW(addr);
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_read_s);
+
int cmdq_pkt_write_s(struct cmdq_pkt *pkt, phys_addr_t addr, u16 reg_idx,
u32 mask)
{
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 8ef87e1bd03b..3f6bc0dfd5da 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -59,6 +59,7 @@ enum cmdq_code {
CMDQ_CODE_JUMP = 0x10,
CMDQ_CODE_WFE = 0x20,
CMDQ_CODE_EOC = 0x40,
+ CMDQ_CODE_READ_S = 0x80,
CMDQ_CODE_WRITE_S = 0x90,
CMDQ_CODE_WRITE_S_MASK = 0x91,
CMDQ_CODE_LOGIC = 0xa0,
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
index 56ff1970197c..bc28a41d7780 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -106,6 +106,16 @@ int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16 offset, u32 value);
int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
u16 offset, u32 value, u32 mask);
+/**
+ * cmdq_pkt_read_s() - append read_s command to the CMDQ packet
+ * @pkt: the CMDQ packet
+ * @addr: the physical address of register or dma to read
+ * @reg_idx: the CMDQ internal register ID to cache read data
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_read_s(struct cmdq_pkt *pkt, phys_addr_t addr, u16 reg_idx);
+
/**
* cmdq_pkt_write_s_mask() - append write_s command to the CMDQ packet
* @pkt: the CMDQ packet
--
2.18.0
On Wed, 2019-11-27 at 09:58 +0800, Dennis YC Hsieh wrote:
> Return error code to client if send message fail,
> so that client has chance to error handling.
>
This patches seems like a fix patch.
Please add fixes, thanks.
Bibby
> Signed-off-by: Dennis YC Hsieh <[email protected]>
> ---
> drivers/soc/mediatek/mtk-cmdq-helper.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> index 274f6f311d05..8421b4090304 100644
> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> @@ -353,11 +353,11 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt, cmdq_async_flush_cb cb,
> spin_unlock_irqrestore(&client->lock, flags);
> }
>
> - mbox_send_message(client->chan, pkt);
> + err = mbox_send_message(client->chan, pkt);
> /* We can send next packet immediately, so just call txdone. */
> mbox_client_txdone(client->chan, 0);
>
> - return 0;
> + return err;
> }
> EXPORT_SYMBOL(cmdq_pkt_flush_async);
>
On Wed, 2019-11-27 at 09:58 +0800, Dennis YC Hsieh wrote:
> add gce device node for mt6779
>
> Signed-off-by: Dennis YC Hsieh <[email protected]>
Reviewed-by: Bibby Hsieh <[email protected]>
> ---
> arch/arm64/boot/dts/mediatek/mt6779.dtsi | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt6779.dtsi b/arch/arm64/boot/dts/mediatek/mt6779.dtsi
> index daa25b75788f..10d59385f4a1 100644
> --- a/arch/arm64/boot/dts/mediatek/mt6779.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt6779.dtsi
> @@ -8,6 +8,7 @@
> #include <dt-bindings/clock/mt6779-clk.h>
> #include <dt-bindings/interrupt-controller/irq.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/gce/mt6779-gce.h>
>
> / {
> compatible = "mediatek,mt6779";
> @@ -159,6 +160,15 @@
> #clock-cells = <1>;
> };
>
> + gce: mailbox@10228000 {
> + compatible = "mediatek,mt6779-gce";
> + reg = <0 0x10228000 0 0x4000>;
> + interrupts = <GIC_SPI 185 IRQ_TYPE_LEVEL_LOW>;
> + #mbox-cells = <3>;
> + clocks = <&infracfg_ao CLK_INFRA_GCE>;
> + clock-names = "gce";
> + };
> +
> uart0: serial@11002000 {
> compatible = "mediatek,mt6779-uart",
> "mediatek,mt6577-uart";
Hi, Dennis:
On Wed, 2019-11-27 at 09:58 +0800, Dennis YC Hsieh wrote:
> Some gce hardware shift pc and end address in register to support
> large dram addressing.
> Implement gce address shift when write or read pc and end register.
> And add shift bit in platform definition.
>
> Signed-off-by: Dennis YC Hsieh <[email protected]>
> ---
> drivers/mailbox/mtk-cmdq-mailbox.c | 57 ++++++++++++++++++------
> drivers/soc/mediatek/mtk-cmdq-helper.c | 3 +-
> include/linux/mailbox/mtk-cmdq-mailbox.h | 2 +
> 3 files changed, 48 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> index 9a6ce9f5a7db..d5536563fce1 100644
> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> @@ -76,8 +76,21 @@ struct cmdq {
> struct cmdq_thread *thread;
> struct clk *clock;
> bool suspended;
> + u8 shift_pa;
> };
>
> +struct gce_plat {
> + u32 thread_nr;
> + u8 shift;
> +};
> +
> +u8 cmdq_mbox_shift(struct mbox_chan *chan)
> +{
> + struct cmdq *cmdq = container_of(chan->mbox, struct cmdq, mbox);
> +
> + return cmdq->shift_pa;
> +}
EXPORT_SYMBOL(cmdq_mbox_shift);
> +
> static int cmdq_thread_suspend(struct cmdq *cmdq, struct cmdq_thread *thread)
> {
> u32 status;
> @@ -176,6 +189,7 @@ static void cmdq_task_remove_wfe(struct cmdq_task *task)
> {
> struct device *dev = task->cmdq->mbox.dev;
> u64 *base = task->pkt->va_base;
> + struct cmdq *cmdq = task->cmdq;
> int i;
>
> dma_sync_single_for_cpu(dev, task->pa_base, task->pkt->cmd_buf_size,
> @@ -183,7 +197,7 @@ static void cmdq_task_remove_wfe(struct cmdq_task *task)
> 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;
> + CMDQ_JUMP_PASS >> cmdq->shift_pa;
cmdq is only used here, so I would like
CMDQ_JUMP_PASS >> task->cmdq->shift_pa;
> dma_sync_single_for_device(dev, task->pa_base, task->pkt->cmd_buf_size,
> DMA_TO_DEVICE);
> }
> @@ -221,13 +235,15 @@ static void cmdq_task_handle_error(struct cmdq_task *task)
> {
> struct cmdq_thread *thread = task->thread;
> struct cmdq_task *next_task;
> + struct cmdq *cmdq = task->cmdq;
>
> 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);
> + writel(next_task->pa_base >> cmdq->shift_pa,
> + thread->base + CMDQ_THR_CURR_ADDR);
> cmdq_thread_resume(thread);
> }
>
> @@ -257,7 +273,7 @@ static void cmdq_thread_irq_handler(struct cmdq *cmdq,
> else
> return;
>
> - curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR);
> + curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR) << cmdq->shift_pa;
>
> list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> list_entry) {
> @@ -373,16 +389,20 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
> 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,
> + writel(task->pa_base >> cmdq->shift_pa,
> + thread->base + CMDQ_THR_CURR_ADDR);
> + writel((task->pa_base + pkt->cmd_buf_size) >> cmdq->shift_pa,
> 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);
> + curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR) <<
> + cmdq->shift_pa;
> + end_pa = readl(thread->base + CMDQ_THR_END_ADDR) <<
> + cmdq->shift_pa;
>
> /*
> * Atomic execution should remove the following wfe, i.e. only
> @@ -395,7 +415,7 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
> cmdq_thread_wait_end(thread, end_pa);
> WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
> /* set to this task directly */
> - writel(task->pa_base,
> + writel(task->pa_base >> cmdq->shift_pa,
> thread->base + CMDQ_THR_CURR_ADDR);
> } else {
> cmdq_task_insert_into_thread(task);
> @@ -407,14 +427,14 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
> if (curr_pa == end_pa - CMDQ_INST_SIZE ||
> curr_pa == end_pa) {
> /* set to this task directly */
> - writel(task->pa_base,
> + writel(task->pa_base >> cmdq->shift_pa,
> thread->base + CMDQ_THR_CURR_ADDR);
> } else {
> cmdq_task_insert_into_thread(task);
> smp_mb(); /* modify jump before enable thread */
> }
> }
> - writel(task->pa_base + pkt->cmd_buf_size,
> + writel((task->pa_base + pkt->cmd_buf_size) >> cmdq->shift_pa,
> thread->base + CMDQ_THR_END_ADDR);
> cmdq_thread_resume(thread);
> }
> @@ -461,6 +481,7 @@ static int cmdq_probe(struct platform_device *pdev)
> struct resource *res;
> struct cmdq *cmdq;
> int err, i;
> + struct gce_plat *plat_data;
>
> cmdq = devm_kzalloc(dev, sizeof(*cmdq), GFP_KERNEL);
> if (!cmdq)
> @@ -479,7 +500,14 @@ static int cmdq_probe(struct platform_device *pdev)
> return -EINVAL;
> }
>
> - cmdq->thread_nr = (u32)(unsigned long)of_device_get_match_data(dev);
> + plat_data = (struct gce_plat *)of_device_get_match_data(dev);
> + if (!plat_data) {
> + dev_err(dev, "failed to get match data\n");
> + return -EINVAL;
> + }
> +
> + cmdq->thread_nr = plat_data->thread_nr;
> + cmdq->shift_pa = plat_data->shift;
> cmdq->irq_mask = GENMASK(cmdq->thread_nr - 1, 0);
> err = devm_request_irq(dev, cmdq->irq, cmdq_irq_handler, IRQF_SHARED,
> "mtk_cmdq", cmdq);
> @@ -542,9 +570,12 @@ static const struct dev_pm_ops cmdq_pm_ops = {
> .resume = cmdq_resume,
> };
>
> +static const struct gce_plat gce_plat_v2 = {.thread_nr = 16, .shift = 0};
> +static const struct gce_plat gce_plat_v3 = {.thread_nr = 24, .shift = 0};
For global variable, you need not to initialize it to zero.
> +
> static const struct of_device_id cmdq_of_ids[] = {
> - {.compatible = "mediatek,mt8173-gce", .data = (void *)16},
> - {.compatible = "mediatek,mt8183-gce", .data = (void *)24},
> + {.compatible = "mediatek,mt8173-gce", .data = (void *)&gce_plat_v2},
> + {.compatible = "mediatek,mt8183-gce", .data = (void *)&gce_plat_v3},
> {}
> };
>
> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> index 9add0fd5fa6c..274f6f311d05 100644
> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> @@ -281,6 +281,7 @@ EXPORT_SYMBOL(cmdq_pkt_poll_mask);
>
> static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
> {
> + struct cmdq_client *cl = pkt->cl;
> struct cmdq_instruction inst = { {0} };
> int err;
>
> @@ -293,7 +294,7 @@ static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
>
> /* JUMP to end */
> inst.op = CMDQ_CODE_JUMP;
> - inst.value = CMDQ_JUMP_PASS;
> + inst.value = CMDQ_JUMP_PASS >> cmdq_mbox_shift(cl->chan);
cl is used only here, so I would like
cmdq_mbox_shift(pkt->cl->chan);
Regards,
CK
> err = cmdq_pkt_append_command(pkt, inst);
>
> return err;
> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> index a4dc45fbec0a..dfe5b2eb85cc 100644
> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> @@ -88,4 +88,6 @@ struct cmdq_pkt {
> void *cl;
> };
>
> +u8 cmdq_mbox_shift(struct mbox_chan *chan);
> +
> #endif /* __MTK_CMDQ_MAILBOX_H__ */
Hi, Dennis:
On Wed, 2019-11-27 at 09:58 +0800, Dennis YC Hsieh wrote:
> Add gce v4 hardware support with different thread number and shift.
>
Reviewed-by: CK Hu <[email protected]>
> Signed-off-by: Dennis YC Hsieh <[email protected]>
> ---
> drivers/mailbox/mtk-cmdq-mailbox.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> index d5536563fce1..fd519b6f518b 100644
> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> @@ -572,10 +572,12 @@ static const struct dev_pm_ops cmdq_pm_ops = {
>
> static const struct gce_plat gce_plat_v2 = {.thread_nr = 16, .shift = 0};
> static const struct gce_plat gce_plat_v3 = {.thread_nr = 24, .shift = 0};
> +static const struct gce_plat gce_plat_v4 = {.thread_nr = 24, .shift = 3};
>
> 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,mt6779-gce", .data = (void *)&gce_plat_v4},
> {}
> };
>
Hi, Dennis:
On Wed, 2019-11-27 at 09:58 +0800, Dennis YC Hsieh wrote:
> Do success callback in channel when shutdown. For those task not finish,
> callback with error code thus client has chance to cleanup or reset.
>
> Signed-off-by: Dennis YC Hsieh <[email protected]>
> ---
> drivers/mailbox/mtk-cmdq-mailbox.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> index fd519b6f518b..c12a768d1175 100644
> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> @@ -450,6 +450,32 @@ static int cmdq_mbox_startup(struct mbox_chan *chan)
>
> static void cmdq_mbox_shutdown(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;
> +
> + spin_lock_irqsave(&thread->chan->lock, flags);
> + if (list_empty(&thread->task_busy_list))
> + goto done;
> +
> + WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
> +
> + /* make sure executed tasks have success callback */
> + cmdq_thread_irq_handler(cmdq, thread);
> + if (list_empty(&thread->task_busy_list))
> + goto done;
> +
> + list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> + list_entry) {
> + cmdq_task_exec_done(task, -ECONNABORTED);
> + kfree(task);
> + }
> +
> + cmdq_thread_disable(cmdq, thread);
> + clk_disable(cmdq->clock);
> +done:
cmdq_thread_resume(thread);
Regards,
CK
> + spin_unlock_irqrestore(&thread->chan->lock, flags);
> }
>
> static const struct mbox_chan_ops cmdq_mbox_chan_ops = {
Hi, Dennis:
On Wed, 2019-11-27 at 09:58 +0800, Dennis YC Hsieh wrote:
> add write_s function in cmdq helper functions which
> writes value contains in internal register to address
> with large dma access support.
>
> Signed-off-by: Dennis YC Hsieh <[email protected]>
> ---
> drivers/soc/mediatek/mtk-cmdq-helper.c | 40 ++++++++++++++++++++++++
> include/linux/mailbox/mtk-cmdq-mailbox.h | 2 ++
> include/linux/soc/mediatek/mtk-cmdq.h | 12 +++++++
> 3 files changed, 54 insertions(+)
>
> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> index 9cc234f08ec5..2edbc0954d97 100644
> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> @@ -15,11 +15,18 @@
> #define CMDQ_EOC_CMD ((u64)((CMDQ_CODE_EOC << CMDQ_OP_CODE_SHIFT)) \
> << 32 | CMDQ_EOC_IRQ_EN)
> #define CMDQ_REG_TYPE 1
> +#define CMDQ_ADDR_HIGH(addr) ((u32)(((addr) >> 16) & GENMASK(31, 0)))
> +#define CMDQ_ADDR_LOW_BIT BIT(1)
> +#define CMDQ_ADDR_LOW(addr) ((u16)(addr) | CMDQ_ADDR_LOW_BIT)
>
> struct cmdq_instruction {
> union {
> u32 value;
> u32 mask;
> + struct {
> + u16 arg_c;
> + u16 arg_b;
> + };
> };
> union {
> u16 offset;
> @@ -224,6 +231,39 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
> }
> EXPORT_SYMBOL(cmdq_pkt_write_mask);
>
> +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, phys_addr_t addr, u16 reg_idx,
> + u32 mask)
> +{
> + struct cmdq_instruction inst = { {0} };
> + const u16 dst_reg_idx = CMDQ_SPR_TEMP;
> + int err;
> +
> + if (mask != U32_MAX) {
> + inst.op = CMDQ_CODE_MASK;
> + inst.mask = ~mask;
> + err = cmdq_pkt_append_command(pkt, inst);
> + if (err < 0)
> + return err;
> +
> + inst.mask = 0;
> + inst.op = CMDQ_CODE_WRITE_S_MASK;
> + } else {
> + inst.op = CMDQ_CODE_WRITE_S;
> + }
> +
> + err = cmdq_pkt_assign(pkt, dst_reg_idx, CMDQ_ADDR_HIGH(addr));
You combine assign and write_s in this function, so you always occupy
register CMDQ_SPR_TEMP for this purpose, client could not use
CMDQ_SPR_TEMP for other purpose. So I would like you just do write_s in
this function. So the code in client would be:
cmdq_pkt_assign(pkt, high_addr_reg_idx, CMDQ_ADDR_HIGH(addr));
cmdq_pkt_write_s(pkt, high_addr_reg_idx, CMDQ_ADDR_LOW(addr),
src_reg_idx, mask);
Let client to decide which register for high address.
Another benefit of not combining instruction is that client driver owner
would be more clear about which command is in command buffer and it's
easier for them to debug.
> + if (err < 0)
> + return err;
> +
> + inst.arg_b_t = CMDQ_REG_TYPE;
> + inst.sop = dst_reg_idx;
> + inst.offset = CMDQ_ADDR_LOW(addr);
> + inst.arg_b = reg_idx;
I seems arg_b has a meaningful name.
Regards,
CK
> +
> + return cmdq_pkt_append_command(pkt, inst);
> +}
> +EXPORT_SYMBOL(cmdq_pkt_write_s);
> +
> int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
> {
> struct cmdq_instruction inst = { {0} };
> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> index 121c3bb6d3de..8ef87e1bd03b 100644
> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> @@ -59,6 +59,8 @@ enum cmdq_code {
> CMDQ_CODE_JUMP = 0x10,
> CMDQ_CODE_WFE = 0x20,
> CMDQ_CODE_EOC = 0x40,
> + CMDQ_CODE_WRITE_S = 0x90,
> + CMDQ_CODE_WRITE_S_MASK = 0x91,
> CMDQ_CODE_LOGIC = 0xa0,
> };
>
> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> index c66b3a0da2a2..56ff1970197c 100644
> --- a/include/linux/soc/mediatek/mtk-cmdq.h
> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> @@ -106,6 +106,18 @@ int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16 offset, u32 value);
> int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
> u16 offset, u32 value, u32 mask);
>
> +/**
> + * cmdq_pkt_write_s_mask() - append write_s command to the CMDQ packet
> + * @pkt: the CMDQ packet
> + * @addr: the physical address of register or dma
> + * @reg_idx: the CMDQ internal register ID which cache source value
> + * @mask: the specified target register mask
> + *
> + * Return: 0 for success; else the error code is returned
> + */
> +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, phys_addr_t addr, u16 reg_idx,
> + u32 mask);
> +
> /**
> * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
> * @pkt: the CMDQ packet
On Wed, 2019-11-27 at 09:58 +0800, Dennis YC Hsieh wrote:
> add write_s function in cmdq helper functions which
> writes value contains in internal register to address
> with large dma access support.
>
> Signed-off-by: Dennis YC Hsieh <[email protected]>
> ---
> drivers/soc/mediatek/mtk-cmdq-helper.c | 40 ++++++++++++++++++++++++
> include/linux/mailbox/mtk-cmdq-mailbox.h | 2 ++
> include/linux/soc/mediatek/mtk-cmdq.h | 12 +++++++
> 3 files changed, 54 insertions(+)
>
> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> index 9cc234f08ec5..2edbc0954d97 100644
> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> @@ -15,11 +15,18 @@
> #define CMDQ_EOC_CMD ((u64)((CMDQ_CODE_EOC << CMDQ_OP_CODE_SHIFT)) \
> << 32 | CMDQ_EOC_IRQ_EN)
> #define CMDQ_REG_TYPE 1
> +#define CMDQ_ADDR_HIGH(addr) ((u32)(((addr) >> 16) & GENMASK(31, 0)))
> +#define CMDQ_ADDR_LOW_BIT BIT(1)
> +#define CMDQ_ADDR_LOW(addr) ((u16)(addr) | CMDQ_ADDR_LOW_BIT)
>
> struct cmdq_instruction {
> union {
> u32 value;
> u32 mask;
> + struct {
> + u16 arg_c;
> + u16 arg_b;
> + };
> };
> union {
> u16 offset;
> @@ -224,6 +231,39 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
> }
> EXPORT_SYMBOL(cmdq_pkt_write_mask);
>
> +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, phys_addr_t addr, u16 reg_idx,
> + u32 mask)
> +{
> + struct cmdq_instruction inst = { {0} };
> + const u16 dst_reg_idx = CMDQ_SPR_TEMP;
> + int err;
> +
> + if (mask != U32_MAX) {
> + inst.op = CMDQ_CODE_MASK;
> + inst.mask = ~mask;
> + err = cmdq_pkt_append_command(pkt, inst);
> + if (err < 0)
> + return err;
> +
> + inst.mask = 0;
> + inst.op = CMDQ_CODE_WRITE_S_MASK;
> + } else {
> + inst.op = CMDQ_CODE_WRITE_S;
> + }
> +
> + err = cmdq_pkt_assign(pkt, dst_reg_idx, CMDQ_ADDR_HIGH(addr));
> + if (err < 0)
> + return err;
> +
> + inst.arg_b_t = CMDQ_REG_TYPE;
> + inst.sop = dst_reg_idx;
> + inst.offset = CMDQ_ADDR_LOW(addr);
> + inst.arg_b = reg_idx;
> +
> + return cmdq_pkt_append_command(pkt, inst);
> +}
> +EXPORT_SYMBOL(cmdq_pkt_write_s);
> +
> int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
> {
> struct cmdq_instruction inst = { {0} };
> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> index 121c3bb6d3de..8ef87e1bd03b 100644
> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> @@ -59,6 +59,8 @@ enum cmdq_code {
> CMDQ_CODE_JUMP = 0x10,
> CMDQ_CODE_WFE = 0x20,
> CMDQ_CODE_EOC = 0x40,
> + CMDQ_CODE_WRITE_S = 0x90,
> + CMDQ_CODE_WRITE_S_MASK = 0x91,
> CMDQ_CODE_LOGIC = 0xa0,
> };
>
> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> index c66b3a0da2a2..56ff1970197c 100644
> --- a/include/linux/soc/mediatek/mtk-cmdq.h
> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> @@ -106,6 +106,18 @@ int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16 offset, u32 value);
> int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
> u16 offset, u32 value, u32 mask);
>
> +/**
> + * cmdq_pkt_write_s_mask() - append write_s command to the CMDQ packet
I think we need add more descriptions about difference between write.
Bibby
> + * @pkt: the CMDQ packet
> + * @addr: the physical address of register or dma
> + * @reg_idx: the CMDQ internal register ID which cache source value
> + * @mask: the specified target register mask
> + *
> + * Return: 0 for success; else the error code is returned
> + */
> +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, phys_addr_t addr, u16 reg_idx,
> + u32 mask);
> +
> /**
> * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
> * @pkt: the CMDQ packet
Hi, Dennis:
On Wed, 2019-11-27 at 09:58 +0800, Dennis YC Hsieh wrote:
> Add read_s function in cmdq helper functions which support read value from
> register or dma physical address into gce internal register.
>
> Signed-off-by: Dennis YC Hsieh <[email protected]>
> ---
> drivers/soc/mediatek/mtk-cmdq-helper.c | 20 ++++++++++++++++++++
> include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
> include/linux/soc/mediatek/mtk-cmdq.h | 10 ++++++++++
> 3 files changed, 31 insertions(+)
>
> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> index 2edbc0954d97..2cd693e34980 100644
> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> @@ -231,6 +231,26 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
> }
> EXPORT_SYMBOL(cmdq_pkt_write_mask);
>
> +int cmdq_pkt_read_s(struct cmdq_pkt *pkt, phys_addr_t addr, u16 reg_idx)
> +{
Should addr be shifted in mt6779?
Regards,
CK
> + struct cmdq_instruction inst = { {0} };
> + int err;
> + const u16 src_reg_idx = CMDQ_SPR_TEMP;
> +
> + err = cmdq_pkt_assign(pkt, src_reg_idx, CMDQ_ADDR_HIGH(addr));
> + if (err < 0)
> + return err;
> +
> + inst.op = CMDQ_CODE_READ_S;
> + inst.dst_t = CMDQ_REG_TYPE;
> + inst.sop = src_reg_idx;
> + inst.reg_dst = reg_idx;
> + inst.arg_b = CMDQ_ADDR_LOW(addr);
> +
> + return cmdq_pkt_append_command(pkt, inst);
> +}
> +EXPORT_SYMBOL(cmdq_pkt_read_s);
> +
> int cmdq_pkt_write_s(struct cmdq_pkt *pkt, phys_addr_t addr, u16 reg_idx,
> u32 mask)
> {
> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> index 8ef87e1bd03b..3f6bc0dfd5da 100644
> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> @@ -59,6 +59,7 @@ enum cmdq_code {
> CMDQ_CODE_JUMP = 0x10,
> CMDQ_CODE_WFE = 0x20,
> CMDQ_CODE_EOC = 0x40,
> + CMDQ_CODE_READ_S = 0x80,
> CMDQ_CODE_WRITE_S = 0x90,
> CMDQ_CODE_WRITE_S_MASK = 0x91,
> CMDQ_CODE_LOGIC = 0xa0,
> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> index 56ff1970197c..bc28a41d7780 100644
> --- a/include/linux/soc/mediatek/mtk-cmdq.h
> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> @@ -106,6 +106,16 @@ int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16 offset, u32 value);
> int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
> u16 offset, u32 value, u32 mask);
>
> +/**
> + * cmdq_pkt_read_s() - append read_s command to the CMDQ packet
> + * @pkt: the CMDQ packet
> + * @addr: the physical address of register or dma to read
> + * @reg_idx: the CMDQ internal register ID to cache read data
> + *
> + * Return: 0 for success; else the error code is returned
> + */
> +int cmdq_pkt_read_s(struct cmdq_pkt *pkt, phys_addr_t addr, u16 reg_idx);
> +
> /**
> * cmdq_pkt_write_s_mask() - append write_s command to the CMDQ packet
> * @pkt: the CMDQ packet
Hi, Dennis:
On Wed, 2019-11-27 at 09:58 +0800, Dennis YC Hsieh wrote:
> Add wait no clear event function in cmdq helper functions to wait specific
> event without clear to 0 after receive it.
>
> Signed-off-by: Dennis YC Hsieh <[email protected]>
> ---
> drivers/soc/mediatek/mtk-cmdq-helper.c | 15 +++++++++++++++
> include/linux/soc/mediatek/mtk-cmdq.h | 10 ++++++++++
> 2 files changed, 25 insertions(+)
>
> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> index 10a9b4481e58..6f270fadfb50 100644
> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> @@ -330,6 +330,21 @@ int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
> }
> EXPORT_SYMBOL(cmdq_pkt_wfe);
>
> +int cmdq_pkt_wait_no_clear(struct cmdq_pkt *pkt, u16 event)
> +{
> + struct cmdq_instruction inst = { {0} };
> +
> + if (event >= CMDQ_MAX_EVENT)
> + return -EINVAL;
> +
> + inst.op = CMDQ_CODE_WFE;
> + inst.value = CMDQ_WFE_WAIT | CMDQ_WFE_WAIT_VALUE;
> + inst.event = event;
> +
> + return cmdq_pkt_append_command(pkt, inst);
> +}
> +EXPORT_SYMBOL(cmdq_pkt_wait_no_clear);
So the wait command has two version, one is wait and then clear event,
another is wait and not clear event. The name of cmdq_pkt_wfe() is 'wait
for event', so it's trivial that we think it does not clear event. I've
three suggestion for this:
1. Let cmdq_pkt_wfe() wait and not clear event, and
cmdq_pkt_wfe_clear_event() wait and clear event.
or
2. Let cmdq_pkt_wfe() has a parameter to indicate that clear event or
not after wait.
or
3. Let cmdq_pkt_wfe() wait and not clear event, and not provide wait and
clear event version. For DRM and MDP, I think both just need wait and
not clear event.
Regards,
CK
> +
> int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event)
> {
> struct cmdq_instruction inst = { {0} };
> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> index d15d8c941992..40bc61ad8d31 100644
> --- a/include/linux/soc/mediatek/mtk-cmdq.h
> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> @@ -149,6 +149,16 @@ int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, dma_addr_t addr,
> */
> int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event);
>
> +/**
> + * cmdq_pkt_wait_no_clear() - Append wait for event command to the CMDQ packet,
> + * without update event to 0 after receive it.
> + * @pkt: the CMDQ packet
> + * @event: the desired event type to wait
> + *
> + * Return: 0 for success; else the error code is returned
> + */
> +int cmdq_pkt_wait_no_clear(struct cmdq_pkt *pkt, u16 event);
> +
> /**
> * cmdq_pkt_clear_event() - append clear event command to the CMDQ packet
> * @pkt: the CMDQ packet
Hi CK,
Thanks for your comment.
On Tue, 2019-12-10 at 09:55 +0800, CK Hu wrote:
> Hi, Dennis:
>
> On Wed, 2019-11-27 at 09:58 +0800, Dennis YC Hsieh wrote:
> > Some gce hardware shift pc and end address in register to support
> > large dram addressing.
> > Implement gce address shift when write or read pc and end register.
> > And add shift bit in platform definition.
> >
> > Signed-off-by: Dennis YC Hsieh <[email protected]>
> > ---
> > drivers/mailbox/mtk-cmdq-mailbox.c | 57 ++++++++++++++++++------
> > drivers/soc/mediatek/mtk-cmdq-helper.c | 3 +-
> > include/linux/mailbox/mtk-cmdq-mailbox.h | 2 +
> > 3 files changed, 48 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> > index 9a6ce9f5a7db..d5536563fce1 100644
> > --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> > @@ -76,8 +76,21 @@ struct cmdq {
> > struct cmdq_thread *thread;
> > struct clk *clock;
> > bool suspended;
> > + u8 shift_pa;
> > };
> >
> > +struct gce_plat {
> > + u32 thread_nr;
> > + u8 shift;
> > +};
> > +
> > +u8 cmdq_mbox_shift(struct mbox_chan *chan)
> > +{
> > + struct cmdq *cmdq = container_of(chan->mbox, struct cmdq, mbox);
> > +
> > + return cmdq->shift_pa;
> > +}
>
> EXPORT_SYMBOL(cmdq_mbox_shift);
>
will do
> > +
> > static int cmdq_thread_suspend(struct cmdq *cmdq, struct cmdq_thread *thread)
> > {
> > u32 status;
> > @@ -176,6 +189,7 @@ static void cmdq_task_remove_wfe(struct cmdq_task *task)
> > {
> > struct device *dev = task->cmdq->mbox.dev;
> > u64 *base = task->pkt->va_base;
> > + struct cmdq *cmdq = task->cmdq;
> > int i;
> >
> > dma_sync_single_for_cpu(dev, task->pa_base, task->pkt->cmd_buf_size,
> > @@ -183,7 +197,7 @@ static void cmdq_task_remove_wfe(struct cmdq_task *task)
> > 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;
> > + CMDQ_JUMP_PASS >> cmdq->shift_pa;
>
> cmdq is only used here, so I would like
>
> CMDQ_JUMP_PASS >> task->cmdq->shift_pa;
>
ok, will fix
> > dma_sync_single_for_device(dev, task->pa_base, task->pkt->cmd_buf_size,
> > DMA_TO_DEVICE);
> > }
> > @@ -221,13 +235,15 @@ static void cmdq_task_handle_error(struct cmdq_task *task)
> > {
> > struct cmdq_thread *thread = task->thread;
> > struct cmdq_task *next_task;
> > + struct cmdq *cmdq = task->cmdq;
> >
> > 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);
> > + writel(next_task->pa_base >> cmdq->shift_pa,
> > + thread->base + CMDQ_THR_CURR_ADDR);
> > cmdq_thread_resume(thread);
> > }
> >
> > @@ -257,7 +273,7 @@ static void cmdq_thread_irq_handler(struct cmdq *cmdq,
> > else
> > return;
> >
> > - curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR);
> > + curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR) << cmdq->shift_pa;
> >
> > list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> > list_entry) {
> > @@ -373,16 +389,20 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
> > 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,
> > + writel(task->pa_base >> cmdq->shift_pa,
> > + thread->base + CMDQ_THR_CURR_ADDR);
> > + writel((task->pa_base + pkt->cmd_buf_size) >> cmdq->shift_pa,
> > 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);
> > + curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR) <<
> > + cmdq->shift_pa;
> > + end_pa = readl(thread->base + CMDQ_THR_END_ADDR) <<
> > + cmdq->shift_pa;
> >
> > /*
> > * Atomic execution should remove the following wfe, i.e. only
> > @@ -395,7 +415,7 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
> > cmdq_thread_wait_end(thread, end_pa);
> > WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
> > /* set to this task directly */
> > - writel(task->pa_base,
> > + writel(task->pa_base >> cmdq->shift_pa,
> > thread->base + CMDQ_THR_CURR_ADDR);
> > } else {
> > cmdq_task_insert_into_thread(task);
> > @@ -407,14 +427,14 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
> > if (curr_pa == end_pa - CMDQ_INST_SIZE ||
> > curr_pa == end_pa) {
> > /* set to this task directly */
> > - writel(task->pa_base,
> > + writel(task->pa_base >> cmdq->shift_pa,
> > thread->base + CMDQ_THR_CURR_ADDR);
> > } else {
> > cmdq_task_insert_into_thread(task);
> > smp_mb(); /* modify jump before enable thread */
> > }
> > }
> > - writel(task->pa_base + pkt->cmd_buf_size,
> > + writel((task->pa_base + pkt->cmd_buf_size) >> cmdq->shift_pa,
> > thread->base + CMDQ_THR_END_ADDR);
> > cmdq_thread_resume(thread);
> > }
> > @@ -461,6 +481,7 @@ static int cmdq_probe(struct platform_device *pdev)
> > struct resource *res;
> > struct cmdq *cmdq;
> > int err, i;
> > + struct gce_plat *plat_data;
> >
> > cmdq = devm_kzalloc(dev, sizeof(*cmdq), GFP_KERNEL);
> > if (!cmdq)
> > @@ -479,7 +500,14 @@ static int cmdq_probe(struct platform_device *pdev)
> > return -EINVAL;
> > }
> >
> > - cmdq->thread_nr = (u32)(unsigned long)of_device_get_match_data(dev);
> > + plat_data = (struct gce_plat *)of_device_get_match_data(dev);
> > + if (!plat_data) {
> > + dev_err(dev, "failed to get match data\n");
> > + return -EINVAL;
> > + }
> > +
> > + cmdq->thread_nr = plat_data->thread_nr;
> > + cmdq->shift_pa = plat_data->shift;
> > cmdq->irq_mask = GENMASK(cmdq->thread_nr - 1, 0);
> > err = devm_request_irq(dev, cmdq->irq, cmdq_irq_handler, IRQF_SHARED,
> > "mtk_cmdq", cmdq);
> > @@ -542,9 +570,12 @@ static const struct dev_pm_ops cmdq_pm_ops = {
> > .resume = cmdq_resume,
> > };
> >
> > +static const struct gce_plat gce_plat_v2 = {.thread_nr = 16, .shift = 0};
> > +static const struct gce_plat gce_plat_v3 = {.thread_nr = 24, .shift = 0};
>
> For global variable, you need not to initialize it to zero.
>
will fix
> > +
> > static const struct of_device_id cmdq_of_ids[] = {
> > - {.compatible = "mediatek,mt8173-gce", .data = (void *)16},
> > - {.compatible = "mediatek,mt8183-gce", .data = (void *)24},
> > + {.compatible = "mediatek,mt8173-gce", .data = (void *)&gce_plat_v2},
> > + {.compatible = "mediatek,mt8183-gce", .data = (void *)&gce_plat_v3},
> > {}
> > };
> >
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index 9add0fd5fa6c..274f6f311d05 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -281,6 +281,7 @@ EXPORT_SYMBOL(cmdq_pkt_poll_mask);
> >
> > static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
> > {
> > + struct cmdq_client *cl = pkt->cl;
> > struct cmdq_instruction inst = { {0} };
> > int err;
> >
> > @@ -293,7 +294,7 @@ static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
> >
> > /* JUMP to end */
> > inst.op = CMDQ_CODE_JUMP;
> > - inst.value = CMDQ_JUMP_PASS;
> > + inst.value = CMDQ_JUMP_PASS >> cmdq_mbox_shift(cl->chan);
>
> cl is used only here, so I would like
>
> cmdq_mbox_shift(pkt->cl->chan);
>
> Regards,
> CK
>
will fix
Regards,
Dennis
> > err = cmdq_pkt_append_command(pkt, inst);
> >
> > return err;
> > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > index a4dc45fbec0a..dfe5b2eb85cc 100644
> > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > @@ -88,4 +88,6 @@ struct cmdq_pkt {
> > void *cl;
> > };
> >
> > +u8 cmdq_mbox_shift(struct mbox_chan *chan);
> > +
> > #endif /* __MTK_CMDQ_MAILBOX_H__ */
>
>
Hi CK,
On Tue, 2019-12-10 at 10:49 +0800, CK Hu wrote:
> Hi, Dennis:
>
> On Wed, 2019-11-27 at 09:58 +0800, Dennis YC Hsieh wrote:
> > Do success callback in channel when shutdown. For those task not finish,
> > callback with error code thus client has chance to cleanup or reset.
> >
> > Signed-off-by: Dennis YC Hsieh <[email protected]>
> > ---
> > drivers/mailbox/mtk-cmdq-mailbox.c | 26 ++++++++++++++++++++++++++
> > 1 file changed, 26 insertions(+)
> >
> > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> > index fd519b6f518b..c12a768d1175 100644
> > --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> > @@ -450,6 +450,32 @@ static int cmdq_mbox_startup(struct mbox_chan *chan)
> >
> > static void cmdq_mbox_shutdown(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;
> > +
> > + spin_lock_irqsave(&thread->chan->lock, flags);
> > + if (list_empty(&thread->task_busy_list))
> > + goto done;
> > +
> > + WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
> > +
> > + /* make sure executed tasks have success callback */
> > + cmdq_thread_irq_handler(cmdq, thread);
> > + if (list_empty(&thread->task_busy_list))
> > + goto done;
> > +
> > + list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> > + list_entry) {
> > + cmdq_task_exec_done(task, -ECONNABORTED);
> > + kfree(task);
> > + }
> > +
> > + cmdq_thread_disable(cmdq, thread);
> > + clk_disable(cmdq->clock);
> > +done:
>
> cmdq_thread_resume(thread);
>
> Regards,
> CK
>
Call resume here will cause violation. The thread->task_busy_list empty
means no task work in gce and thread state should already disable
without clock, which is what we want since client try to shut down this
mbox channel. So I think we don't need resume here.
Regards,
Dennis
> > + spin_unlock_irqrestore(&thread->chan->lock, flags);
> > }
> >
> > static const struct mbox_chan_ops cmdq_mbox_chan_ops = {
>
>
Hi CK,
On Tue, 2019-12-10 at 13:18 +0800, CK Hu wrote:
> Hi, Dennis:
>
> On Wed, 2019-11-27 at 09:58 +0800, Dennis YC Hsieh wrote:
> > add write_s function in cmdq helper functions which
> > writes value contains in internal register to address
> > with large dma access support.
> >
> > Signed-off-by: Dennis YC Hsieh <[email protected]>
> > ---
> > drivers/soc/mediatek/mtk-cmdq-helper.c | 40 ++++++++++++++++++++++++
> > include/linux/mailbox/mtk-cmdq-mailbox.h | 2 ++
> > include/linux/soc/mediatek/mtk-cmdq.h | 12 +++++++
> > 3 files changed, 54 insertions(+)
> >
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index 9cc234f08ec5..2edbc0954d97 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -15,11 +15,18 @@
> > #define CMDQ_EOC_CMD ((u64)((CMDQ_CODE_EOC << CMDQ_OP_CODE_SHIFT)) \
> > << 32 | CMDQ_EOC_IRQ_EN)
> > #define CMDQ_REG_TYPE 1
> > +#define CMDQ_ADDR_HIGH(addr) ((u32)(((addr) >> 16) & GENMASK(31, 0)))
> > +#define CMDQ_ADDR_LOW_BIT BIT(1)
> > +#define CMDQ_ADDR_LOW(addr) ((u16)(addr) | CMDQ_ADDR_LOW_BIT)
> >
> > struct cmdq_instruction {
> > union {
> > u32 value;
> > u32 mask;
> > + struct {
> > + u16 arg_c;
> > + u16 arg_b;
> > + };
> > };
> > union {
> > u16 offset;
> > @@ -224,6 +231,39 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
> > }
> > EXPORT_SYMBOL(cmdq_pkt_write_mask);
> >
> > +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, phys_addr_t addr, u16 reg_idx,
> > + u32 mask)
> > +{
> > + struct cmdq_instruction inst = { {0} };
> > + const u16 dst_reg_idx = CMDQ_SPR_TEMP;
> > + int err;
> > +
> > + if (mask != U32_MAX) {
> > + inst.op = CMDQ_CODE_MASK;
> > + inst.mask = ~mask;
> > + err = cmdq_pkt_append_command(pkt, inst);
> > + if (err < 0)
> > + return err;
> > +
> > + inst.mask = 0;
> > + inst.op = CMDQ_CODE_WRITE_S_MASK;
> > + } else {
> > + inst.op = CMDQ_CODE_WRITE_S;
> > + }
> > +
> > + err = cmdq_pkt_assign(pkt, dst_reg_idx, CMDQ_ADDR_HIGH(addr));
>
> You combine assign and write_s in this function, so you always occupy
> register CMDQ_SPR_TEMP for this purpose, client could not use
> CMDQ_SPR_TEMP for other purpose. So I would like you just do write_s in
> this function. So the code in client would be:
>
> cmdq_pkt_assign(pkt, high_addr_reg_idx, CMDQ_ADDR_HIGH(addr));
> cmdq_pkt_write_s(pkt, high_addr_reg_idx, CMDQ_ADDR_LOW(addr),
> src_reg_idx, mask);
>
> Let client to decide which register for high address.
>
> Another benefit of not combining instruction is that client driver owner
> would be more clear about which command is in command buffer and it's
> easier for them to debug.
>
ok, i will expose reg idx as parameter
> > + if (err < 0)
> > + return err;
> > +
> > + inst.arg_b_t = CMDQ_REG_TYPE;
> > + inst.sop = dst_reg_idx;
> > + inst.offset = CMDQ_ADDR_LOW(addr);
> > + inst.arg_b = reg_idx;
>
> I seems arg_b has a meaningful name.
>
> Regards,
> CK
>
ok, will change name
Regards,
Dennis
> > +
> > + return cmdq_pkt_append_command(pkt, inst);
> > +}
> > +EXPORT_SYMBOL(cmdq_pkt_write_s);
> > +
> > int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
> > {
> > struct cmdq_instruction inst = { {0} };
> > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > index 121c3bb6d3de..8ef87e1bd03b 100644
> > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > @@ -59,6 +59,8 @@ enum cmdq_code {
> > CMDQ_CODE_JUMP = 0x10,
> > CMDQ_CODE_WFE = 0x20,
> > CMDQ_CODE_EOC = 0x40,
> > + CMDQ_CODE_WRITE_S = 0x90,
> > + CMDQ_CODE_WRITE_S_MASK = 0x91,
> > CMDQ_CODE_LOGIC = 0xa0,
> > };
> >
> > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> > index c66b3a0da2a2..56ff1970197c 100644
> > --- a/include/linux/soc/mediatek/mtk-cmdq.h
> > +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> > @@ -106,6 +106,18 @@ int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16 offset, u32 value);
> > int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
> > u16 offset, u32 value, u32 mask);
> >
> > +/**
> > + * cmdq_pkt_write_s_mask() - append write_s command to the CMDQ packet
> > + * @pkt: the CMDQ packet
> > + * @addr: the physical address of register or dma
> > + * @reg_idx: the CMDQ internal register ID which cache source value
> > + * @mask: the specified target register mask
> > + *
> > + * Return: 0 for success; else the error code is returned
> > + */
> > +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, phys_addr_t addr, u16 reg_idx,
> > + u32 mask);
> > +
> > /**
> > * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
> > * @pkt: the CMDQ packet
>
>
Hi, Dennis:
On Thu, 2019-12-12 at 09:13 +0800, Dennis-YC Hsieh wrote:
> Hi CK,
>
> On Tue, 2019-12-10 at 10:49 +0800, CK Hu wrote:
> > Hi, Dennis:
> >
> > On Wed, 2019-11-27 at 09:58 +0800, Dennis YC Hsieh wrote:
> > > Do success callback in channel when shutdown. For those task not finish,
> > > callback with error code thus client has chance to cleanup or reset.
> > >
> > > Signed-off-by: Dennis YC Hsieh <[email protected]>
> > > ---
> > > drivers/mailbox/mtk-cmdq-mailbox.c | 26 ++++++++++++++++++++++++++
> > > 1 file changed, 26 insertions(+)
> > >
> > > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> > > index fd519b6f518b..c12a768d1175 100644
> > > --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> > > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> > > @@ -450,6 +450,32 @@ static int cmdq_mbox_startup(struct mbox_chan *chan)
> > >
> > > static void cmdq_mbox_shutdown(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;
> > > +
> > > + spin_lock_irqsave(&thread->chan->lock, flags);
> > > + if (list_empty(&thread->task_busy_list))
> > > + goto done;
> > > +
> > > + WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
> > > +
> > > + /* make sure executed tasks have success callback */
> > > + cmdq_thread_irq_handler(cmdq, thread);
> > > + if (list_empty(&thread->task_busy_list))
> > > + goto done;
> > > +
> > > + list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> > > + list_entry) {
> > > + cmdq_task_exec_done(task, -ECONNABORTED);
> > > + kfree(task);
> > > + }
> > > +
> > > + cmdq_thread_disable(cmdq, thread);
> > > + clk_disable(cmdq->clock);
> > > +done:
> >
> > cmdq_thread_resume(thread);
> >
> > Regards,
> > CK
> >
>
> Call resume here will cause violation. The thread->task_busy_list empty
> means no task work in gce and thread state should already disable
> without clock, which is what we want since client try to shut down this
> mbox channel. So I think we don't need resume here.
>
OK. When client free channel, thread is suspended. Then client request
channel, where do you resume thread?
Regards,
CK
>
> Regards,
> Dennis
>
> > > + spin_unlock_irqrestore(&thread->chan->lock, flags);
> > > }
> > >
> > > static const struct mbox_chan_ops cmdq_mbox_chan_ops = {
> >
> >
>
>
Hi Bibby,
thanks for your comment
On Tue, 2019-12-10 at 15:35 +0800, Bibby Hsieh wrote:
> On Wed, 2019-11-27 at 09:58 +0800, Dennis YC Hsieh wrote:
> > add write_s function in cmdq helper functions which
> > writes value contains in internal register to address
> > with large dma access support.
> >
> > Signed-off-by: Dennis YC Hsieh <[email protected]>
> > ---
> > drivers/soc/mediatek/mtk-cmdq-helper.c | 40 ++++++++++++++++++++++++
> > include/linux/mailbox/mtk-cmdq-mailbox.h | 2 ++
> > include/linux/soc/mediatek/mtk-cmdq.h | 12 +++++++
> > 3 files changed, 54 insertions(+)
> >
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index 9cc234f08ec5..2edbc0954d97 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -15,11 +15,18 @@
> > #define CMDQ_EOC_CMD ((u64)((CMDQ_CODE_EOC << CMDQ_OP_CODE_SHIFT)) \
> > << 32 | CMDQ_EOC_IRQ_EN)
> > #define CMDQ_REG_TYPE 1
> > +#define CMDQ_ADDR_HIGH(addr) ((u32)(((addr) >> 16) & GENMASK(31, 0)))
> > +#define CMDQ_ADDR_LOW_BIT BIT(1)
> > +#define CMDQ_ADDR_LOW(addr) ((u16)(addr) | CMDQ_ADDR_LOW_BIT)
> >
> > struct cmdq_instruction {
> > union {
> > u32 value;
> > u32 mask;
> > + struct {
> > + u16 arg_c;
> > + u16 arg_b;
> > + };
> > };
> > union {
> > u16 offset;
> > @@ -224,6 +231,39 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
> > }
> > EXPORT_SYMBOL(cmdq_pkt_write_mask);
> >
> > +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, phys_addr_t addr, u16 reg_idx,
> > + u32 mask)
> > +{
> > + struct cmdq_instruction inst = { {0} };
> > + const u16 dst_reg_idx = CMDQ_SPR_TEMP;
> > + int err;
> > +
> > + if (mask != U32_MAX) {
> > + inst.op = CMDQ_CODE_MASK;
> > + inst.mask = ~mask;
> > + err = cmdq_pkt_append_command(pkt, inst);
> > + if (err < 0)
> > + return err;
> > +
> > + inst.mask = 0;
> > + inst.op = CMDQ_CODE_WRITE_S_MASK;
> > + } else {
> > + inst.op = CMDQ_CODE_WRITE_S;
> > + }
> > +
> > + err = cmdq_pkt_assign(pkt, dst_reg_idx, CMDQ_ADDR_HIGH(addr));
> > + if (err < 0)
> > + return err;
> > +
> > + inst.arg_b_t = CMDQ_REG_TYPE;
> > + inst.sop = dst_reg_idx;
> > + inst.offset = CMDQ_ADDR_LOW(addr);
> > + inst.arg_b = reg_idx;
> > +
> > + return cmdq_pkt_append_command(pkt, inst);
> > +}
> > +EXPORT_SYMBOL(cmdq_pkt_write_s);
> > +
> > int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
> > {
> > struct cmdq_instruction inst = { {0} };
> > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > index 121c3bb6d3de..8ef87e1bd03b 100644
> > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > @@ -59,6 +59,8 @@ enum cmdq_code {
> > CMDQ_CODE_JUMP = 0x10,
> > CMDQ_CODE_WFE = 0x20,
> > CMDQ_CODE_EOC = 0x40,
> > + CMDQ_CODE_WRITE_S = 0x90,
> > + CMDQ_CODE_WRITE_S_MASK = 0x91,
> > CMDQ_CODE_LOGIC = 0xa0,
> > };
> >
> > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> > index c66b3a0da2a2..56ff1970197c 100644
> > --- a/include/linux/soc/mediatek/mtk-cmdq.h
> > +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> > @@ -106,6 +106,18 @@ int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16 offset, u32 value);
> > int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
> > u16 offset, u32 value, u32 mask);
> >
> > +/**
> > + * cmdq_pkt_write_s_mask() - append write_s command to the CMDQ packet
> I think we need add more descriptions about difference between write.
>
> Bibby
ok, will change
Regards,
Dennis
> > + * @pkt: the CMDQ packet
> > + * @addr: the physical address of register or dma
> > + * @reg_idx: the CMDQ internal register ID which cache source value
> > + * @mask: the specified target register mask
> > + *
> > + * Return: 0 for success; else the error code is returned
> > + */
> > +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, phys_addr_t addr, u16 reg_idx,
> > + u32 mask);
> > +
> > /**
> > * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
> > * @pkt: the CMDQ packet
>
>
Hi CK,
On Tue, 2019-12-10 at 15:55 +0800, CK Hu wrote:
> Hi, Dennis:
>
> On Wed, 2019-11-27 at 09:58 +0800, Dennis YC Hsieh wrote:
> > Add read_s function in cmdq helper functions which support read value from
> > register or dma physical address into gce internal register.
> >
> > Signed-off-by: Dennis YC Hsieh <[email protected]>
> > ---
> > drivers/soc/mediatek/mtk-cmdq-helper.c | 20 ++++++++++++++++++++
> > include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
> > include/linux/soc/mediatek/mtk-cmdq.h | 10 ++++++++++
> > 3 files changed, 31 insertions(+)
> >
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index 2edbc0954d97..2cd693e34980 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -231,6 +231,26 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
> > }
> > EXPORT_SYMBOL(cmdq_pkt_write_mask);
> >
> > +int cmdq_pkt_read_s(struct cmdq_pkt *pkt, phys_addr_t addr, u16 reg_idx)
> > +{
>
> Should addr be shifted in mt6779?
>
> Regards,
> CK
>
no, only pc and end register shift address
no need to shift in instruction
Regards,
Dennis
> > + struct cmdq_instruction inst = { {0} };
> > + int err;
> > + const u16 src_reg_idx = CMDQ_SPR_TEMP;
> > +
> > + err = cmdq_pkt_assign(pkt, src_reg_idx, CMDQ_ADDR_HIGH(addr));
> > + if (err < 0)
> > + return err;
> > +
> > + inst.op = CMDQ_CODE_READ_S;
> > + inst.dst_t = CMDQ_REG_TYPE;
> > + inst.sop = src_reg_idx;
> > + inst.reg_dst = reg_idx;
> > + inst.arg_b = CMDQ_ADDR_LOW(addr);
> > +
> > + return cmdq_pkt_append_command(pkt, inst);
> > +}
> > +EXPORT_SYMBOL(cmdq_pkt_read_s);
> > +
> > int cmdq_pkt_write_s(struct cmdq_pkt *pkt, phys_addr_t addr, u16 reg_idx,
> > u32 mask)
> > {
> > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > index 8ef87e1bd03b..3f6bc0dfd5da 100644
> > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > @@ -59,6 +59,7 @@ enum cmdq_code {
> > CMDQ_CODE_JUMP = 0x10,
> > CMDQ_CODE_WFE = 0x20,
> > CMDQ_CODE_EOC = 0x40,
> > + CMDQ_CODE_READ_S = 0x80,
> > CMDQ_CODE_WRITE_S = 0x90,
> > CMDQ_CODE_WRITE_S_MASK = 0x91,
> > CMDQ_CODE_LOGIC = 0xa0,
> > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> > index 56ff1970197c..bc28a41d7780 100644
> > --- a/include/linux/soc/mediatek/mtk-cmdq.h
> > +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> > @@ -106,6 +106,16 @@ int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16 offset, u32 value);
> > int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
> > u16 offset, u32 value, u32 mask);
> >
> > +/**
> > + * cmdq_pkt_read_s() - append read_s command to the CMDQ packet
> > + * @pkt: the CMDQ packet
> > + * @addr: the physical address of register or dma to read
> > + * @reg_idx: the CMDQ internal register ID to cache read data
> > + *
> > + * Return: 0 for success; else the error code is returned
> > + */
> > +int cmdq_pkt_read_s(struct cmdq_pkt *pkt, phys_addr_t addr, u16 reg_idx);
> > +
> > /**
> > * cmdq_pkt_write_s_mask() - append write_s command to the CMDQ packet
> > * @pkt: the CMDQ packet
>
>
On Wed, 2019-12-11 at 10:04 +0800, CK Hu wrote:
> Hi, Dennis:
>
> On Wed, 2019-11-27 at 09:58 +0800, Dennis YC Hsieh wrote:
> > Add wait no clear event function in cmdq helper functions to wait specific
> > event without clear to 0 after receive it.
> >
> > Signed-off-by: Dennis YC Hsieh <[email protected]>
> > ---
> > drivers/soc/mediatek/mtk-cmdq-helper.c | 15 +++++++++++++++
> > include/linux/soc/mediatek/mtk-cmdq.h | 10 ++++++++++
> > 2 files changed, 25 insertions(+)
> >
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index 10a9b4481e58..6f270fadfb50 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -330,6 +330,21 @@ int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
> > }
> > EXPORT_SYMBOL(cmdq_pkt_wfe);
> >
> > +int cmdq_pkt_wait_no_clear(struct cmdq_pkt *pkt, u16 event)
> > +{
> > + struct cmdq_instruction inst = { {0} };
> > +
> > + if (event >= CMDQ_MAX_EVENT)
> > + return -EINVAL;
> > +
> > + inst.op = CMDQ_CODE_WFE;
> > + inst.value = CMDQ_WFE_WAIT | CMDQ_WFE_WAIT_VALUE;
> > + inst.event = event;
> > +
> > + return cmdq_pkt_append_command(pkt, inst);
> > +}
> > +EXPORT_SYMBOL(cmdq_pkt_wait_no_clear);
>
> So the wait command has two version, one is wait and then clear event,
> another is wait and not clear event. The name of cmdq_pkt_wfe() is 'wait
> for event', so it's trivial that we think it does not clear event. I've
> three suggestion for this:
>
> 1. Let cmdq_pkt_wfe() wait and not clear event, and
> cmdq_pkt_wfe_clear_event() wait and clear event.
>
> or
> 2. Let cmdq_pkt_wfe() has a parameter to indicate that clear event or
> not after wait.
>
> or
> 3. Let cmdq_pkt_wfe() wait and not clear event, and not provide wait and
> clear event version. For DRM and MDP, I think both just need wait and
> not clear event.
>
> Regards,
> CK
>
ok, I will change cmdq_pkt_wfe wait and not clear, and expose another
cmdq_pkt_wfe_clear_event() api
Regards,
Dennis
>
> > +
> > int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event)
> > {
> > struct cmdq_instruction inst = { {0} };
> > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> > index d15d8c941992..40bc61ad8d31 100644
> > --- a/include/linux/soc/mediatek/mtk-cmdq.h
> > +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> > @@ -149,6 +149,16 @@ int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, dma_addr_t addr,
> > */
> > int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event);
> >
> > +/**
> > + * cmdq_pkt_wait_no_clear() - Append wait for event command to the CMDQ packet,
> > + * without update event to 0 after receive it.
> > + * @pkt: the CMDQ packet
> > + * @event: the desired event type to wait
> > + *
> > + * Return: 0 for success; else the error code is returned
> > + */
> > +int cmdq_pkt_wait_no_clear(struct cmdq_pkt *pkt, u16 event);
> > +
> > /**
> > * cmdq_pkt_clear_event() - append clear event command to the CMDQ packet
> > * @pkt: the CMDQ packet
>
>
Hi CK,
On Thu, 2019-12-12 at 09:31 +0800, CK Hu wrote:
> Hi, Dennis:
>
> On Thu, 2019-12-12 at 09:13 +0800, Dennis-YC Hsieh wrote:
> > Hi CK,
> >
> > On Tue, 2019-12-10 at 10:49 +0800, CK Hu wrote:
> > > Hi, Dennis:
> > >
> > > On Wed, 2019-11-27 at 09:58 +0800, Dennis YC Hsieh wrote:
> > > > Do success callback in channel when shutdown. For those task not finish,
> > > > callback with error code thus client has chance to cleanup or reset.
> > > >
> > > > Signed-off-by: Dennis YC Hsieh <[email protected]>
> > > > ---
> > > > drivers/mailbox/mtk-cmdq-mailbox.c | 26 ++++++++++++++++++++++++++
> > > > 1 file changed, 26 insertions(+)
> > > >
> > > > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> > > > index fd519b6f518b..c12a768d1175 100644
> > > > --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> > > > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> > > > @@ -450,6 +450,32 @@ static int cmdq_mbox_startup(struct mbox_chan *chan)
> > > >
> > > > static void cmdq_mbox_shutdown(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;
> > > > +
> > > > + spin_lock_irqsave(&thread->chan->lock, flags);
> > > > + if (list_empty(&thread->task_busy_list))
> > > > + goto done;
> > > > +
> > > > + WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
> > > > +
> > > > + /* make sure executed tasks have success callback */
> > > > + cmdq_thread_irq_handler(cmdq, thread);
> > > > + if (list_empty(&thread->task_busy_list))
> > > > + goto done;
> > > > +
> > > > + list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> > > > + list_entry) {
> > > > + cmdq_task_exec_done(task, -ECONNABORTED);
> > > > + kfree(task);
> > > > + }
> > > > +
> > > > + cmdq_thread_disable(cmdq, thread);
> > > > + clk_disable(cmdq->clock);
> > > > +done:
> > >
> > > cmdq_thread_resume(thread);
> > >
> > > Regards,
> > > CK
> > >
> >
> > Call resume here will cause violation. The thread->task_busy_list empty
> > means no task work in gce and thread state should already disable
> > without clock, which is what we want since client try to shut down this
> > mbox channel. So I think we don't need resume here.
> >
>
> OK. When client free channel, thread is suspended. Then client request
> channel, where do you resume thread?
>
when client send new pkt to new channel, cmdq_mbox_send_data() will
enable thread.
Regards,
Dennis
> Regards,
> CK
>
> >
> > Regards,
> > Dennis
> >
> > > > + spin_unlock_irqrestore(&thread->chan->lock, flags);
> > > > }
> > > >
> > > > static const struct mbox_chan_ops cmdq_mbox_chan_ops = {
> > >
> > >
> >
> >
>
>
Hi, Dennis:
On Thu, 2019-12-12 at 09:51 +0800, Dennis-YC Hsieh wrote:
> Hi CK,
>
> On Thu, 2019-12-12 at 09:31 +0800, CK Hu wrote:
> > Hi, Dennis:
> >
> > On Thu, 2019-12-12 at 09:13 +0800, Dennis-YC Hsieh wrote:
> > > Hi CK,
> > >
> > > On Tue, 2019-12-10 at 10:49 +0800, CK Hu wrote:
> > > > Hi, Dennis:
> > > >
> > > > On Wed, 2019-11-27 at 09:58 +0800, Dennis YC Hsieh wrote:
> > > > > Do success callback in channel when shutdown. For those task not finish,
> > > > > callback with error code thus client has chance to cleanup or reset.
> > > > >
> > > > > Signed-off-by: Dennis YC Hsieh <[email protected]>
> > > > > ---
> > > > > drivers/mailbox/mtk-cmdq-mailbox.c | 26 ++++++++++++++++++++++++++
> > > > > 1 file changed, 26 insertions(+)
> > > > >
> > > > > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> > > > > index fd519b6f518b..c12a768d1175 100644
> > > > > --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> > > > > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> > > > > @@ -450,6 +450,32 @@ static int cmdq_mbox_startup(struct mbox_chan *chan)
> > > > >
> > > > > static void cmdq_mbox_shutdown(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;
> > > > > +
> > > > > + spin_lock_irqsave(&thread->chan->lock, flags);
> > > > > + if (list_empty(&thread->task_busy_list))
> > > > > + goto done;
> > > > > +
> > > > > + WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
> > > > > +
> > > > > + /* make sure executed tasks have success callback */
> > > > > + cmdq_thread_irq_handler(cmdq, thread);
> > > > > + if (list_empty(&thread->task_busy_list))
> > > > > + goto done;
> > > > > +
> > > > > + list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> > > > > + list_entry) {
> > > > > + cmdq_task_exec_done(task, -ECONNABORTED);
> > > > > + kfree(task);
> > > > > + }
> > > > > +
> > > > > + cmdq_thread_disable(cmdq, thread);
> > > > > + clk_disable(cmdq->clock);
> > > > > +done:
> > > >
> > > > cmdq_thread_resume(thread);
> > > >
> > > > Regards,
> > > > CK
> > > >
> > >
> > > Call resume here will cause violation. The thread->task_busy_list empty
> > > means no task work in gce and thread state should already disable
> > > without clock, which is what we want since client try to shut down this
> > > mbox channel. So I think we don't need resume here.
> > >
> >
> > OK. When client free channel, thread is suspended. Then client request
> > channel, where do you resume thread?
> >
>
> when client send new pkt to new channel, cmdq_mbox_send_data() will
> enable thread.
in cmdq_mbox_send_data(), it would run below command:
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);
Do you mean CMDQ_THR_ENABLE_TASK is set to CMDQ_THR_ENABLED, then
CMDQ_THR_SUSPEND_TASK would be automatically set to CMDQ_THR_RESUME? If
this hardware work in so special behavior, please add comment for this.
Regards,
CK
>
>
> Regards,
> Dennis
>
> > Regards,
> > CK
> >
> > >
> > > Regards,
> > > Dennis
> > >
> > > > > + spin_unlock_irqrestore(&thread->chan->lock, flags);
> > > > > }
> > > > >
> > > > > static const struct mbox_chan_ops cmdq_mbox_chan_ops = {
> > > >
> > > >
> > >
> > >
> >
> >
>
>
Hi CK,
On Thu, 2019-12-12 at 10:03 +0800, CK Hu wrote:
> Hi, Dennis:
>
> On Thu, 2019-12-12 at 09:51 +0800, Dennis-YC Hsieh wrote:
> > Hi CK,
> >
> > On Thu, 2019-12-12 at 09:31 +0800, CK Hu wrote:
> > > Hi, Dennis:
> > >
> > > On Thu, 2019-12-12 at 09:13 +0800, Dennis-YC Hsieh wrote:
> > > > Hi CK,
> > > >
> > > > On Tue, 2019-12-10 at 10:49 +0800, CK Hu wrote:
> > > > > Hi, Dennis:
> > > > >
> > > > > On Wed, 2019-11-27 at 09:58 +0800, Dennis YC Hsieh wrote:
> > > > > > Do success callback in channel when shutdown. For those task not finish,
> > > > > > callback with error code thus client has chance to cleanup or reset.
> > > > > >
> > > > > > Signed-off-by: Dennis YC Hsieh <[email protected]>
> > > > > > ---
> > > > > > drivers/mailbox/mtk-cmdq-mailbox.c | 26 ++++++++++++++++++++++++++
> > > > > > 1 file changed, 26 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> > > > > > index fd519b6f518b..c12a768d1175 100644
> > > > > > --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> > > > > > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> > > > > > @@ -450,6 +450,32 @@ static int cmdq_mbox_startup(struct mbox_chan *chan)
> > > > > >
> > > > > > static void cmdq_mbox_shutdown(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;
> > > > > > +
> > > > > > + spin_lock_irqsave(&thread->chan->lock, flags);
> > > > > > + if (list_empty(&thread->task_busy_list))
> > > > > > + goto done;
> > > > > > +
> > > > > > + WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
> > > > > > +
> > > > > > + /* make sure executed tasks have success callback */
> > > > > > + cmdq_thread_irq_handler(cmdq, thread);
> > > > > > + if (list_empty(&thread->task_busy_list))
> > > > > > + goto done;
> > > > > > +
> > > > > > + list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> > > > > > + list_entry) {
> > > > > > + cmdq_task_exec_done(task, -ECONNABORTED);
> > > > > > + kfree(task);
> > > > > > + }
> > > > > > +
> > > > > > + cmdq_thread_disable(cmdq, thread);
> > > > > > + clk_disable(cmdq->clock);
> > > > > > +done:
> > > > >
> > > > > cmdq_thread_resume(thread);
> > > > >
> > > > > Regards,
> > > > > CK
> > > > >
> > > >
> > > > Call resume here will cause violation. The thread->task_busy_list empty
> > > > means no task work in gce and thread state should already disable
> > > > without clock, which is what we want since client try to shut down this
> > > > mbox channel. So I think we don't need resume here.
> > > >
> > >
> > > OK. When client free channel, thread is suspended. Then client request
> > > channel, where do you resume thread?
> > >
> >
> > when client send new pkt to new channel, cmdq_mbox_send_data() will
> > enable thread.
>
> in cmdq_mbox_send_data(), it would run below command:
>
> 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);
>
> Do you mean CMDQ_THR_ENABLE_TASK is set to CMDQ_THR_ENABLED, then
> CMDQ_THR_SUSPEND_TASK would be automatically set to CMDQ_THR_RESUME? If
> this hardware work in so special behavior, please add comment for this.
>
> Regards,
> CK
>
sorry for not clearly explain before
call to cmdq_thread_reset() will reset thread status, which include
suspend state. the CMDQ_THR_SUSPEND_TASK will be clear after reset, thus
we can simply set CMDQ_THR_ENABLE_TASK to CMDQ_THR_ENABLED and then
thread running again.
I will add comment in both cmdq_mbox_send_data() and
cmdq_mbox_shutdown() to clarify this hardware behavior.
Regards,
Dennis
> >
> >
> > Regards,
> > Dennis
> >
> > > Regards,
> > > CK
> > >
> > > >
> > > > Regards,
> > > > Dennis
> > > >
> > > > > > + spin_unlock_irqrestore(&thread->chan->lock, flags);
> > > > > > }
> > > > > >
> > > > > > static const struct mbox_chan_ops cmdq_mbox_chan_ops = {
> > > > >
> > > > >
> > > >
> > > >
> > >
> > >
> >
> >
>
>