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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Il 03/08/23 10:37, CK Hu (胡俊光) ha scritto:
Hi, Angelo:

On Thu, 2023-08-03 at 10:25 +0200, AngeloGioacchino Del Regno wrote:
Il 03/08/23 08:28, CK Hu (胡俊光) ha scritto:
Hi, Angelo:

On Wed, 2023-08-02 at 12:41 +0200, AngeloGioacchino Del Regno
wrote:
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
structures
switch them to pointers, allowing us to migrate this driver
to
use
the
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 <
angelogioacchino.delregno@xxxxxxxxxxxxx>
---
    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
b/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;
#if IS_REACHABLE(CONFIG_MTK_CMDQ)
-	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)
    	}
    }
-#if IS_REACHABLE(CONFIG_MTK_CMDQ)
-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?

I think removing ifdefs and using helper function are different
things.
You could remove ifdefs and keep mtk_drm_cmdq_pkt_create().


I chose to do it like that because this function would otherwise be a
100% duplicate of the related cmdq helper :-)

Removing cl would change the interface of cmdq_pkt_create(). And this
is related to different maintainer's tree. So it would be a long time
to process. For you, only removing ifdes is urgent, so use
cmdq_pkt_create() is not urgent. So let's keep
mtk_drm_cmdq_pkt_create() and you could remove ifdefs.


Hello CK,

my CMDQ cleanup has been stuck on your intention to remove `cl` from the CMDQ
helpers for ** six months ** now.

Are you performing that removal, or can we just get this cleanup finally done?

Regards,
Angelo





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux