Re: [PATCH 1/3] drm/mediatek: Dynamically allocate CMDQ and use helper functions

From: AngeloGioacchino Del Regno
Date: Wed Aug 02 2023 - 06:43:50 EST

Il 02/08/23 08:24, CK Hu (胡俊光) ha scritto:
Hi, Angelo:

On Fri, 2023-06-23 at 11:49 +0200, AngeloGioacchino Del Regno wrote:

External email : Please do not click links or open attachments until
you have verified the sender or the content.
Instead of stack allocating the cmdq_client and cmdq_handle
switch them to pointers, allowing us to migrate this driver to use
common functions provided by mtk-cmdq-helper.
In order to do this, it was also necessary to add a `priv` pointer to
struct cmdq_client, as that's used to pass (in this case) a mtk_crtc
handle to the ddp_cmdq_cb() mailbox RX callback function.

Signed-off-by: AngeloGioacchino Del Regno <
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 107 +++++++---------------
include/linux/soc/mediatek/mtk-cmdq.h | 1 +
2 files changed, 32 insertions(+), 76 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 0df62b076f49..b63289ab6787 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -50,8 +50,8 @@ struct mtk_drm_crtc {
bool pending_async_planes;
- struct cmdq_client cmdq_client;
- struct cmdq_pkt cmdq_handle;
+ struct cmdq_client *cmdq_client;
+ struct cmdq_pkt *cmdq_handle;
u32 cmdq_event;
u32 cmdq_vblank_cnt;
wait_queue_head_t cb_blocking_queue;
@@ -108,47 +108,6 @@ static void mtk_drm_finish_page_flip(struct
mtk_drm_crtc *mtk_crtc)
-static int mtk_drm_cmdq_pkt_create(struct cmdq_client *client,
struct cmdq_pkt *pkt,
- size_t size)
- struct device *dev;
- dma_addr_t dma_addr;
- pkt->va_base = kzalloc(size, GFP_KERNEL);
- if (!pkt->va_base) {
- kfree(pkt);
- return -ENOMEM;
- }
- pkt->buf_size = size;
- pkt->cl = (void *)client;

I have a plan to remove cl in struct cmdq_pkt. But this modification
would make this plan more difficult. So I would pending this patch
until cl is removed from struct cmdq_pkt.

I think that this ifdef cleanup is more urgent than the removal of `cl` from
struct cmdq_pkt, as those ifdefs shouldn't have reached upstream in the first
place, don't you agree?