<html><body><p>
<pre>
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@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 {
> > > > > > > boolpending_async_pla
> > > > > > > nes;
> > > > > > >
> > > > > > > #if IS_REACHABLE(CONFIG_MTK_CMDQ)
> > > > > > > -struct cmdq_clientcmdq_client;
> > > > > > > -struct cmdq_pktcmdq_handle;
> > > > > > > +struct cmdq_client*cmdq_client;
> > > > > > > +struct cmdq_pkt*cmdq_handle;
> > > > > > > u32cmdq_event;
> > > > > > > u32cmdq_vblank_cnt;
> > > > > > > wait_queue_head_tcb_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
>
>
>
</pre>
</p></body></html><!--type:text--><!--{--><pre>************* MEDIATEK Confidentiality Notice ********************
The information contained in this e-mail message (including any
attachments) may be confidential, proprietary, privileged, or otherwise
exempt from disclosure under applicable laws. It is intended to be
conveyed only to the designated recipient(s). Any use, dissemination,
distribution, printing, retaining or copying of this e-mail (including its
attachments) by unintended recipient(s) is strictly prohibited and may
be unlawful. If you are not an intended recipient of this e-mail, or believe
that you have received this e-mail in error, please notify the sender
immediately (by replying to this e-mail), delete any and all copies of
this e-mail (including any attachments) from your system, and do not
disclose the content of this e-mail to any other person. Thank you!
</pre><!--}-->