[PATCH v7 02/20] mailbox: mtk-cmdq: Refine DMA address handling for the command buffer
AngeloGioacchino Del Regno
angelogioacchino.delregno at collabora.com
Thu Oct 9 04:27:16 PDT 2025
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 at 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__ */
More information about the Linux-mediatek
mailing list