GCE can only fetch the command buffer address from a 32-bit register.
Some SoCs support a 35-bit command buffer address for GCE, which
requires a right shift of 3 bits before setting the address into
the 32-bit register. A comment has been added to the header of
cmdq_get_shift_pa() to explain this requirement.
To prevent the GCE command buffer address from being DMA mapped beyond
its supported bit range, the DMA bit mask for the device is set during
initialization.
Additionally, to ensure the correct shift is applied when setting or
reading the register that stores the GCE command buffer address,
new APIs, cmdq_reg_shift_addr() and cmdq_reg_revert_addr(), have been
introduced for consistent operations on this register.
The variable type for the command buffer address has been standardized
to dma_addr_t to prevent handling issues caused by type mismatches.
Fixes: 0858fde496f8 ("mailbox: cmdq: variablize address shift in platform")
Signed-off-by: Jason-JH Lin <jason-jh.lin@mediatek.com>
---
drivers/mailbox/mtk-cmdq-mailbox.c | 43 ++++++++++++++++--------
include/linux/mailbox/mtk-cmdq-mailbox.h | 10 ++++++
2 files changed, 39 insertions(+), 14 deletions(-)
diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
index 532929916e99..a60486cbb533 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -92,6 +92,16 @@ struct gce_plat {
u32 gce_num;
};
+static inline u32 cmdq_reg_shift_addr(dma_addr_t addr, const struct gce_plat *pdata)
+{
+ return (addr >> pdata->shift);
+}
+
+static inline dma_addr_t cmdq_reg_revert_addr(u32 addr, const struct gce_plat *pdata)
+{
+ return ((dma_addr_t)addr << pdata->shift);
+}
+
u8 cmdq_get_shift_pa(struct mbox_chan *chan)
{
struct cmdq *cmdq = container_of(chan->mbox, struct cmdq, mbox);
@@ -188,13 +198,12 @@ static void cmdq_task_insert_into_thread(struct cmdq_task *task)
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;
+ u32 shift_addr = cmdq_reg_shift_addr(task->pa_base, task->cmdq->pdata);
/* 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 >> task->cmdq->pdata->shift);
+ prev_task_base[CMDQ_NUM_CMD(prev_task->pkt) - 1] = (u64)CMDQ_JUMP_BY_PA << 32 | shift_addr;
dma_sync_single_for_device(dev, prev_task->pa_base,
prev_task->pkt->cmd_buf_size, DMA_TO_DEVICE);
@@ -237,7 +246,8 @@ 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;
+ u32 irq_flag, shift_addr;
+ dma_addr_t curr_pa, task_end_pa;
bool err;
irq_flag = readl(thread->base + CMDQ_THR_IRQ_STATUS);
@@ -259,7 +269,8 @@ static void cmdq_thread_irq_handler(struct cmdq *cmdq,
else
return;
- curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR) << cmdq->pdata->shift;
+ shift_addr = readl(thread->base + CMDQ_THR_CURR_ADDR);
+ curr_pa = cmdq_reg_revert_addr(shift_addr, cmdq->pdata);
list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
list_entry) {
@@ -378,7 +389,8 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *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;
+ u32 shift_addr;
+ dma_addr_t curr_pa, end_pa;
int ret;
/* Client should not flush new tasks if suspended. */
@@ -409,20 +421,20 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
*/
WARN_ON(cmdq_thread_reset(cmdq, thread) < 0);
- writel(task->pa_base >> cmdq->pdata->shift,
- thread->base + CMDQ_THR_CURR_ADDR);
- writel((task->pa_base + pkt->cmd_buf_size) >> cmdq->pdata->shift,
- thread->base + CMDQ_THR_END_ADDR);
+ shift_addr = cmdq_reg_shift_addr(task->pa_base, cmdq->pdata);
+ writel(shift_addr, thread->base + CMDQ_THR_CURR_ADDR);
+ shift_addr = cmdq_reg_shift_addr(task->pa_base + pkt->cmd_buf_size, cmdq->pdata);
+ writel(shift_addr, 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) <<
- cmdq->pdata->shift;
- end_pa = readl(thread->base + CMDQ_THR_END_ADDR) <<
- cmdq->pdata->shift;
+ shift_addr = readl(thread->base + CMDQ_THR_CURR_ADDR);
+ curr_pa = cmdq_reg_revert_addr(shift_addr, cmdq->pdata);
+ shift_addr = readl(thread->base + CMDQ_THR_END_ADDR);
+ end_pa = cmdq_reg_revert_addr(shift_addr, cmdq->pdata);
/* check boundary */
if (curr_pa == end_pa - CMDQ_INST_SIZE ||
curr_pa == end_pa) {
@@ -656,6 +668,9 @@ static int cmdq_probe(struct platform_device *pdev)
if (err)
return err;
+ dma_set_coherent_mask(dev,
+ DMA_BIT_MASK(sizeof(u32) * BITS_PER_BYTE + cmdq->pdata->shift));
+
cmdq->mbox.dev = dev;
cmdq->mbox.chans = devm_kcalloc(dev, cmdq->pdata->thread_nr,
sizeof(*cmdq->mbox.chans), GFP_KERNEL);
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 4c1a91b07de3..e1555e06e7e5 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -77,6 +77,16 @@ struct cmdq_pkt {
size_t buf_size; /* real buffer size */
};
+/**
+ * cmdq_get_shift_pa() - get the shift bits of physical address
+ * @chan: mailbox channel
+ *
+ * GCE can only fetch the command buffer address from a 32-bit register.
+ * Some SOCs support more than 32-bit command buffer address for GCE, which
+ * requires some shift bits to make the address fit into the 32-bit register.
+ *
+ * Return: the shift bits of physical address
+ */
u8 cmdq_get_shift_pa(struct mbox_chan *chan);
#endif /* __MTK_CMDQ_MAILBOX_H__ */
--
2.43.0Il 27/08/25 13:37, Jason-JH Lin ha scritto:
> GCE can only fetch the command buffer address from a 32-bit register.
> Some SoCs support a 35-bit command buffer address for GCE, which
> requires a right shift of 3 bits before setting the address into
> the 32-bit register. A comment has been added to the header of
> cmdq_get_shift_pa() to explain this requirement.
>
> To prevent the GCE command buffer address from being DMA mapped beyond
> its supported bit range, the DMA bit mask for the device is set during
> initialization.
>
> Additionally, to ensure the correct shift is applied when setting or
> reading the register that stores the GCE command buffer address,
> new APIs, cmdq_reg_shift_addr() and cmdq_reg_revert_addr(), have been
> introduced for consistent operations on this register.
>
> The variable type for the command buffer address has been standardized
> to dma_addr_t to prevent handling issues caused by type mismatches.
>
> Fixes: 0858fde496f8 ("mailbox: cmdq: variablize address shift in platform")
> Signed-off-by: Jason-JH Lin <jason-jh.lin@mediatek.com>
> ---
> drivers/mailbox/mtk-cmdq-mailbox.c | 43 ++++++++++++++++--------
> include/linux/mailbox/mtk-cmdq-mailbox.h | 10 ++++++
> 2 files changed, 39 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> index 532929916e99..a60486cbb533 100644
> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> @@ -92,6 +92,16 @@ struct gce_plat {
> u32 gce_num;
> };
>
> +static inline u32 cmdq_reg_shift_addr(dma_addr_t addr, const struct gce_plat *pdata)
cmdq_format_gce_addr() or cmdq_pa_to_gce_addr()
(or cmdq_va_to_gce_addr() if this is not a physical address)
explains what you're doing here a bit better I think.
> +{
> + return (addr >> pdata->shift);
You don't need parenthesis; just `return addr >> pdata->shift;` is fine
> +}
> +
> +static inline dma_addr_t cmdq_reg_revert_addr(u32 addr, const struct gce_plat *pdata)
cmdq_gce_to_pa_addr() or cmdq_get_pa_addr() perhaps? :-)
(same comments for s/pa/va/g if this is not physical address)
Everything else looks ok.
Cheers,
Angelo
> +{
> + return ((dma_addr_t)addr << pdata->shift);
> +}
> +
> u8 cmdq_get_shift_pa(struct mbox_chan *chan)
> {
> struct cmdq *cmdq = container_of(chan->mbox, struct cmdq, mbox);
> @@ -188,13 +198,12 @@ static void cmdq_task_insert_into_thread(struct cmdq_task *task)
> 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;
> + u32 shift_addr = cmdq_reg_shift_addr(task->pa_base, task->cmdq->pdata);
>
> /* 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 >> task->cmdq->pdata->shift);
> + prev_task_base[CMDQ_NUM_CMD(prev_task->pkt) - 1] = (u64)CMDQ_JUMP_BY_PA << 32 | shift_addr;
> dma_sync_single_for_device(dev, prev_task->pa_base,
> prev_task->pkt->cmd_buf_size, DMA_TO_DEVICE);
>
> @@ -237,7 +246,8 @@ 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;
> + u32 irq_flag, shift_addr;
> + dma_addr_t curr_pa, task_end_pa;
> bool err;
>
> irq_flag = readl(thread->base + CMDQ_THR_IRQ_STATUS);
> @@ -259,7 +269,8 @@ static void cmdq_thread_irq_handler(struct cmdq *cmdq,
> else
> return;
>
> - curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR) << cmdq->pdata->shift;
> + shift_addr = readl(thread->base + CMDQ_THR_CURR_ADDR);
> + curr_pa = cmdq_reg_revert_addr(shift_addr, cmdq->pdata);
>
> list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> list_entry) {
> @@ -378,7 +389,8 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *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;
> + u32 shift_addr;
> + dma_addr_t curr_pa, end_pa;
> int ret;
>
> /* Client should not flush new tasks if suspended. */
> @@ -409,20 +421,20 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
> */
> WARN_ON(cmdq_thread_reset(cmdq, thread) < 0);
>
> - writel(task->pa_base >> cmdq->pdata->shift,
> - thread->base + CMDQ_THR_CURR_ADDR);
> - writel((task->pa_base + pkt->cmd_buf_size) >> cmdq->pdata->shift,
> - thread->base + CMDQ_THR_END_ADDR);
> + shift_addr = cmdq_reg_shift_addr(task->pa_base, cmdq->pdata);
> + writel(shift_addr, thread->base + CMDQ_THR_CURR_ADDR);
> + shift_addr = cmdq_reg_shift_addr(task->pa_base + pkt->cmd_buf_size, cmdq->pdata);
> + writel(shift_addr, 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) <<
> - cmdq->pdata->shift;
> - end_pa = readl(thread->base + CMDQ_THR_END_ADDR) <<
> - cmdq->pdata->shift;
> + shift_addr = readl(thread->base + CMDQ_THR_CURR_ADDR);
> + curr_pa = cmdq_reg_revert_addr(shift_addr, cmdq->pdata);
> + shift_addr = readl(thread->base + CMDQ_THR_END_ADDR);
> + end_pa = cmdq_reg_revert_addr(shift_addr, cmdq->pdata);
> /* check boundary */
> if (curr_pa == end_pa - CMDQ_INST_SIZE ||
> curr_pa == end_pa) {
> @@ -656,6 +668,9 @@ static int cmdq_probe(struct platform_device *pdev)
> if (err)
> return err;
>
> + dma_set_coherent_mask(dev,
> + DMA_BIT_MASK(sizeof(u32) * BITS_PER_BYTE + cmdq->pdata->shift));
> +
> cmdq->mbox.dev = dev;
> cmdq->mbox.chans = devm_kcalloc(dev, cmdq->pdata->thread_nr,
> sizeof(*cmdq->mbox.chans), GFP_KERNEL);
> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> index 4c1a91b07de3..e1555e06e7e5 100644
> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> @@ -77,6 +77,16 @@ struct cmdq_pkt {
> size_t buf_size; /* real buffer size */
> };
>
> +/**
> + * cmdq_get_shift_pa() - get the shift bits of physical address
> + * @chan: mailbox channel
> + *
> + * GCE can only fetch the command buffer address from a 32-bit register.
> + * Some SOCs support more than 32-bit command buffer address for GCE, which
> + * requires some shift bits to make the address fit into the 32-bit register.
> + *
> + * Return: the shift bits of physical address
> + */
> u8 cmdq_get_shift_pa(struct mbox_chan *chan);
>
> #endif /* __MTK_CMDQ_MAILBOX_H__ */
On Thu, 2025-10-09 at 13:27 +0200, AngeloGioacchino Del Regno wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
>
> Il 27/08/25 13:37, Jason-JH Lin ha scritto:
> > GCE can only fetch the command buffer address from a 32-bit
> > register.
> > Some SoCs support a 35-bit command buffer address for GCE, which
> > requires a right shift of 3 bits before setting the address into
> > the 32-bit register. A comment has been added to the header of
> > cmdq_get_shift_pa() to explain this requirement.
> >
> > To prevent the GCE command buffer address from being DMA mapped
> > beyond
> > its supported bit range, the DMA bit mask for the device is set
> > during
> > initialization.
> >
> > Additionally, to ensure the correct shift is applied when setting
> > or
> > reading the register that stores the GCE command buffer address,
> > new APIs, cmdq_reg_shift_addr() and cmdq_reg_revert_addr(), have
> > been
> > introduced for consistent operations on this register.
> >
> > The variable type for the command buffer address has been
> > standardized
> > to dma_addr_t to prevent handling issues caused by type mismatches.
> >
> > Fixes: 0858fde496f8 ("mailbox: cmdq: variablize address shift in
> > platform")
> > Signed-off-by: Jason-JH Lin <jason-jh.lin@mediatek.com>
> > ---
> > drivers/mailbox/mtk-cmdq-mailbox.c | 43 ++++++++++++++++---
> > -----
> > include/linux/mailbox/mtk-cmdq-mailbox.h | 10 ++++++
> > 2 files changed, 39 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c
> > b/drivers/mailbox/mtk-cmdq-mailbox.c
> > index 532929916e99..a60486cbb533 100644
> > --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> > @@ -92,6 +92,16 @@ struct gce_plat {
> > u32 gce_num;
> > };
> >
> > +static inline u32 cmdq_reg_shift_addr(dma_addr_t addr, const
> > struct gce_plat *pdata)
>
> cmdq_format_gce_addr() or cmdq_pa_to_gce_addr()
>
> (or cmdq_va_to_gce_addr() if this is not a physical address)
>
> explains what you're doing here a bit better I think.
OK, I'll change the API name and add comments for both APIs:
static inline u32 cmdq_convert_gce_addr()
{
/* Convert DMA addr (PA or IOVA) to GCE readable addr */
}
static inline u32 cmdq_revert_gce_addr()
{
/* Revert GCE readable addr to DMA addr (PA or IOVA) */
}
>
> > +{
> > + return (addr >> pdata->shift);
>
> You don't need parenthesis; just `return addr >> pdata->shift;` is
> fine
>
OK, I'll remove this.
> > +}
> > +
> > +static inline dma_addr_t cmdq_reg_revert_addr(u32 addr, const
> > struct gce_plat *pdata)
>
> cmdq_gce_to_pa_addr() or cmdq_get_pa_addr() perhaps? :-)
>
> (same comments for s/pa/va/g if this is not physical address)
>
> Everything else looks ok.
>
Thanks for the reviews!
Regards,
Jason-JH Lin
> Cheers,
> Angelo
© 2016 - 2026 Red Hat, Inc.