| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <8811a5e04514954cf379ac561023a7e1884a2da1.camel@mediatek.com>
Date: Wed, 7 Feb 2024 03:49:19 +0000
From: CK Hu (胡俊光) <ck.hu@...iatek.com>
To: "angelogioacchino.delregno@...labora.com"
<angelogioacchino.delregno@...labora.com>, "chunkuang.hu@...nel.org"
<chunkuang.hu@...nel.org>
CC: "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "kernel@...labora.com"
<kernel@...labora.com>, "matthias.bgg@...il.com" <matthias.bgg@...il.com>,
"wenst@...omium.org" <wenst@...omium.org>
Subject: Re: [PATCH 1/3] drm/mediatek: Dynamically allocate CMDQ and use
helper functions
Hi, Angelo:
On Tue, 2024-02-06 at 14:33 +0100, AngeloGioacchino Del Regno wrote:
> 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@...labora.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_pla
> > > > > > > nes;
> > > > > > >
> > > > > > > #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?
It's busy last year and I almost forget this. Thank you to notice me. I
would send patch after Chinese new year (Feb 8th ~ Feb 14th).
Regards,
CK
>
> Regards,
> Angelo
>
>
>
Powered by blists - more mailing lists