Messages in this thread | | | Date | Thu, 3 Aug 2023 10:25:36 +0200 | Subject | Re: [PATCH 1/3] drm/mediatek: Dynamically allocate CMDQ and use helper functions | From | AngeloGioacchino Del Regno <> |
| |
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@collabora.com> >>>> --- >>>> 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 :-)
> Regards, > CK > >> >> Regards, >> Angelo >>
| |