lkml.org 
[lkml]   [2023]   [Aug]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 1/3] drm/mediatek: Dynamically allocate CMDQ and use helper functions
Date
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().

Regards,
CK

>
> Regards,
> Angelo
>
\
 
 \ /
  Last update: 2023-08-03 08:30    [W:0.104 / U:0.476 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site