<pre>
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_planes;
>
> #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.

Regards,
CK

> -
> -dev = client->chan->mbox->dev;
> -dma_addr = dma_map_single(dev, pkt->va_base, pkt->buf_size,
> - DMA_TO_DEVICE);
> -if (dma_mapping_error(dev, dma_addr)) {
> -dev_err(dev, "dma map failed, size=%u\n",
> (u32)(u64)size);
> -kfree(pkt->va_base);
> -kfree(pkt);
> -return -ENOMEM;
> -}
> -
> -pkt->pa_base = dma_addr;
> -
> -return 0;
> -}
> -
> -static void mtk_drm_cmdq_pkt_destroy(struct cmdq_pkt *pkt)
> -{
> -struct cmdq_client *client = (struct cmdq_client *)pkt->cl;
> -
> -dma_unmap_single(client->chan->mbox->dev, pkt->pa_base, pkt-
> >buf_size,
> - DMA_TO_DEVICE);
> -kfree(pkt->va_base);
> -kfree(pkt);
> -}
> -#endif
> -
> static void mtk_drm_crtc_destroy(struct drm_crtc *crtc)
> {
> struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> @@ -156,12 +115,9 @@ static void mtk_drm_crtc_destroy(struct drm_crtc
> *crtc)
>
> mtk_mutex_put(mtk_crtc->mutex);
> #if IS_REACHABLE(CONFIG_MTK_CMDQ)
> -mtk_drm_cmdq_pkt_destroy(&mtk_crtc->cmdq_handle);
> -
> -if (mtk_crtc->cmdq_client.chan) {
> -mbox_free_channel(mtk_crtc->cmdq_client.chan);
> -mtk_crtc->cmdq_client.chan = NULL;
> -}
> +cmdq_pkt_destroy(mtk_crtc->cmdq_handle);
> +cmdq_mbox_destroy(mtk_crtc->cmdq_client);
> +mtk_crtc->cmdq_client = NULL;
> #endif
>
> for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
> @@ -288,7 +244,7 @@ static void ddp_cmdq_cb(struct mbox_client *cl,
> void *mssg)
> {
> struct cmdq_cb_data *data = mssg;
> struct cmdq_client *cmdq_cl = container_of(cl, struct
> cmdq_client, client);
> -struct mtk_drm_crtc *mtk_crtc = container_of(cmdq_cl, struct
> mtk_drm_crtc, cmdq_client);
> +struct mtk_drm_crtc *mtk_crtc = (struct mtk_drm_crtc *)cmdq_cl-
> >priv;
> struct mtk_crtc_state *state;
> unsigned int i;
>
> @@ -546,7 +502,7 @@ static void mtk_drm_crtc_update_config(struct
> mtk_drm_crtc *mtk_crtc,
> bool needs_vblank)
> {
> #if IS_REACHABLE(CONFIG_MTK_CMDQ)
> -struct cmdq_pkt *cmdq_handle = &mtk_crtc->cmdq_handle;
> +struct cmdq_pkt *cmdq_handle = mtk_crtc->cmdq_handle;
> #endif
> struct drm_crtc *crtc = &mtk_crtc->base;
> struct mtk_drm_private *priv = crtc->dev->dev_private;
> @@ -584,14 +540,14 @@ static void mtk_drm_crtc_update_config(struct
> mtk_drm_crtc *mtk_crtc,
> mtk_mutex_release(mtk_crtc->mutex);
> }
> #if IS_REACHABLE(CONFIG_MTK_CMDQ)
> -if (mtk_crtc->cmdq_client.chan) {
> -mbox_flush(mtk_crtc->cmdq_client.chan, 2000);
> +if (mtk_crtc->cmdq_client) {
> +mbox_flush(mtk_crtc->cmdq_client->chan, 2000);
> cmdq_handle->cmd_buf_size = 0;
> cmdq_pkt_clear_event(cmdq_handle, mtk_crtc-
> >cmdq_event);
> cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false);
> mtk_crtc_ddp_config(crtc, cmdq_handle);
> cmdq_pkt_finalize(cmdq_handle);
> -dma_sync_single_for_device(mtk_crtc->cmdq_client.chan-
> >mbox->dev,
> +dma_sync_single_for_device(mtk_crtc->cmdq_client->chan-
> >mbox->dev,
> cmdq_handle->pa_base,
> cmdq_handle->cmd_buf_size,
> DMA_TO_DEVICE);
> @@ -604,8 +560,8 @@ static void mtk_drm_crtc_update_config(struct
> mtk_drm_crtc *mtk_crtc,
> */
> mtk_crtc->cmdq_vblank_cnt = 3;
>
> -mbox_send_message(mtk_crtc->cmdq_client.chan,
> cmdq_handle);
> -mbox_client_txdone(mtk_crtc->cmdq_client.chan, 0);
> +mbox_send_message(mtk_crtc->cmdq_client->chan,
> cmdq_handle);
> +mbox_client_txdone(mtk_crtc->cmdq_client->chan, 0);
> }
> #endif
> mtk_crtc->config_updating = false;
> @@ -619,7 +575,7 @@ static void mtk_crtc_ddp_irq(void *data)
> struct mtk_drm_private *priv = crtc->dev->dev_private;
>
> #if IS_REACHABLE(CONFIG_MTK_CMDQ)
> -if (!priv->data->shadow_register && !mtk_crtc-
> >cmdq_client.chan)
> +if (!priv->data->shadow_register && !mtk_crtc->cmdq_client)
> mtk_crtc_ddp_config(crtc, NULL);
> else if (mtk_crtc->cmdq_vblank_cnt > 0 && --mtk_crtc-
> >cmdq_vblank_cnt == 0)
> DRM_ERROR("mtk_crtc %d CMDQ execute command
> timeout!\n",
> @@ -722,7 +678,7 @@ static void mtk_drm_crtc_atomic_disable(struct
> drm_crtc *crtc,
> mtk_drm_crtc_update_config(mtk_crtc, false);
> #if IS_REACHABLE(CONFIG_MTK_CMDQ)
> /* Wait for planes to be disabled by cmdq */
> -if (mtk_crtc->cmdq_client.chan)
> +if (mtk_crtc->cmdq_client)
> wait_event_timeout(mtk_crtc->cb_blocking_queue,
> mtk_crtc->cmdq_vblank_cnt == 0,
> msecs_to_jiffies(500));
> @@ -1002,19 +958,20 @@ int mtk_drm_crtc_create(struct drm_device
> *drm_dev,
>
> #if IS_REACHABLE(CONFIG_MTK_CMDQ)
> i = priv->mbox_index++;
> -mtk_crtc->cmdq_client.client.dev = mtk_crtc->mmsys_dev;
> -mtk_crtc->cmdq_client.client.tx_block = false;
> -mtk_crtc->cmdq_client.client.knows_txdone = true;
> -mtk_crtc->cmdq_client.client.rx_callback = ddp_cmdq_cb;
> -mtk_crtc->cmdq_client.chan =
> -mbox_request_channel(&mtk_crtc-
> >cmdq_client.client, i);
> -if (IS_ERR(mtk_crtc->cmdq_client.chan)) {
> -dev_dbg(dev, "mtk_crtc %d failed to create mailbox
> client, writing register by CPU now\n",
> -drm_crtc_index(&mtk_crtc->base));
> -mtk_crtc->cmdq_client.chan = NULL;
> +
> +mtk_crtc->cmdq_client = cmdq_mbox_create(mtk_crtc->mmsys_dev,
> i);
> +if (IS_ERR(mtk_crtc->cmdq_client)) {
> +ret = PTR_ERR(mtk_crtc->cmdq_client);
> +dev_dbg(dev, "Failed to create CMDQ client: %d\n",
> ret);
> +mtk_crtc->cmdq_client = NULL;
> +return 0;
> }
>
> -if (mtk_crtc->cmdq_client.chan) {
> +/* Setup the CMDQ handler callback */
> +mtk_crtc->cmdq_client->priv = mtk_crtc;
> +mtk_crtc->cmdq_client->client.rx_callback = ddp_cmdq_cb;
> +
> +if (mtk_crtc->cmdq_client) {
> ret = of_property_read_u32_index(priv->mutex_node,
> "mediatek,gce-events",
> i,
> @@ -1022,17 +979,15 @@ int mtk_drm_crtc_create(struct drm_device
> *drm_dev,
> if (ret) {
> dev_dbg(dev, "mtk_crtc %d failed to get
> mediatek,gce-events property\n",
> drm_crtc_index(&mtk_crtc->base));
> -mbox_free_channel(mtk_crtc->cmdq_client.chan);
> -mtk_crtc->cmdq_client.chan = NULL;
> +cmdq_mbox_destroy(mtk_crtc->cmdq_client);
> +mtk_crtc->cmdq_client = NULL;
> } else {
> -ret = mtk_drm_cmdq_pkt_create(&mtk_crtc-
> >cmdq_client,
> - &mtk_crtc-
> >cmdq_handle,
> - PAGE_SIZE);
> +mtk_crtc->cmdq_handle =
> cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE);
> if (ret) {
> dev_dbg(dev, "mtk_crtc %d failed to
> create cmdq packet\n",
> drm_crtc_index(&mtk_crtc-
> >base));
> -mbox_free_channel(mtk_crtc-
> >cmdq_client.chan);
> -mtk_crtc->cmdq_client.chan = NULL;
> +cmdq_mbox_destroy(mtk_crtc-
> >cmdq_client);
> +mtk_crtc->cmdq_client = NULL;
> }
> }
>
> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
> b/include/linux/soc/mediatek/mtk-cmdq.h
> index 649955d2cf5c..2a1dc8b12db3 100644
> --- a/include/linux/soc/mediatek/mtk-cmdq.h
> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> @@ -25,6 +25,7 @@ struct cmdq_client_reg {
> struct cmdq_client {
> struct mbox_client client;
> struct mbox_chan *chan;
> +void *priv;
> };
>
> #if IS_ENABLED(CONFIG_MTK_CMDQ)
> --
> 2.40.1

</pre><!--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><!--}-->