Skip to contentSkip to conversation

Remove cl in struct cmdq_pkt

PATCH In review Chun-Kuang Hu 4 participants 12 messages Sep 1, 2024 – Sep 6, 2024

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

v4 cover Remove cl in struct cmdq_pkt

cl in struct cmdq_pkt is used to store struct cmdq_client, but every client driver has the struct cmdq_client information, so it's not necessary to store struct cmdq_client in struct cmdq_pkt. Because mailbox maintainer do not like to mix mailbox patch with other patches in a series, so mailbox patch [1] would be sent independently.

Changes in v4:
1. Rebase onto mediatek-drm-next-6.12
2. Fix build error in mpd3 driver
Changes in v3:
1. Rebase onto Linux 6.11-rc1 and fix conflicts. Changes in v2:
1. Fix typo of CMDQ_JUMP_RELATIVE
2. Refine cmdq_pkt_create() and cmdq_pkt_destroy()
3. Rename cmdq_pkt_jump() to cmdq_pkt_jump_abs()
4. Add cmdq_pkt_jump_rel() helper function
5. drm/mediatek: Use cmdq_pkt_create() and cmdq_pkt_destroy()
6. mtk-mdp3: Get fine-grain control of cmdq_pkt_finalize()
7. mtk-mdp3: Use cmdq_pkt_create() and cmdq_pkt_destroy()

[1] https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/commit/?h=mediatek-cmdq8&id=a1b2f7a7488285975c1f439086f1c4cc51a13bb9

Chun-Kuang Hu (3):
media: platform: mtk-mdp3: Get fine-grain control of
cmdq_pkt_finalize()
media: platform: mtk-mdp3: Use cmdq_pkt_create() and
cmdq_pkt_destroy()
soc: mediatek: cmdq: Remove cmdq_pkt_finalize() helper function

 .../platform/mediatek/mdp3/mtk-mdp3-cmdq.c    | 49 +++----------------
 .../platform/mediatek/mdp3/mtk-mdp3-cmdq.h    |  1 +
 .../platform/mediatek/mdp3/mtk-mdp3-core.c    |  2 +
 .../platform/mediatek/mdp3/mtk-mdp3-core.h    |  1 +
 drivers/soc/mediatek/mtk-cmdq-helper.c        | 22 ---------
 include/linux/soc/mediatek/mtk-cmdq.h         | 13 -----
 6 files changed, 11 insertions(+), 77 deletions(-)
Signature · 1 line
2.34.1
#1 lore raw

PATCH v4 1/3 media: platform: mtk-mdp3: Get fine-grain control of cmdq_pkt_finalize()

In order to have fine-grained control, use cmdq_pkt_eoc() and cmdq_pkt_jump_rel() to replace cmdq_pkt_finalize().

---
 drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c | 3 ++-
 drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.c | 2 ++
 drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.h | 1 +
 3 files changed, 5 insertions(+), 1 deletion(-)
drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c +2 -1
index ea2ea119dd2a..ef5dade35fd3 100644
@@ -676,7 +676,8 @@ static struct mdp_cmdq_cmd *mdp_cmdq_prepare(struct mdp_dev *mdp,
 		dev_err(dev, "mdp_path_config error %d\n", pp_idx);
 		goto err_free_path;
 	}
-	cmdq_pkt_finalize(&cmd->pkt);
+	cmdq_pkt_eoc(&cmd->pkt);
+	cmdq_pkt_jump_rel(&cmd->pkt, CMDQ_INST_SIZE, mdp->cmdq_shift_pa[pp_idx]);
 
 	for (i = 0; i < num_comp; i++) {
 		s32 inner_id = MDP_COMP_NONE;
drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.c +2 -0
index 37e7b985d52c..40623c1980bf 100644
@@ -312,6 +312,8 @@ static int mdp_probe(struct platform_device *pdev)
 			ret = PTR_ERR(mdp->cmdq_clt[i]);
 			goto err_mbox_destroy;
 		}
+
+		mdp->cmdq_shift_pa[i] = cmdq_get_shift_pa(mdp->cmdq_clt[i]->chan);
 	}
 
 	init_waitqueue_head(&mdp->callback_wq);
drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.h +1 -0
index 430251f63754..05cade1d098e 100644
@@ -126,6 +126,7 @@ struct mdp_dev {
 	u32					id_count;
 	struct ida				mdp_ida;
 	struct cmdq_client			*cmdq_clt[MDP_PP_MAX];
+	u8					cmdq_shift_pa[MDP_PP_MAX];
 	wait_queue_head_t			callback_wq;
 
 	struct v4l2_device			v4l2_dev;
Signature · 1 line
2.34.1
maintainer Reviewed-by on v4 1/3
#2 lore raw

Re: [PATCH v4 1/3] media: platform: mtk-mdp3: Get fine-grain control of cmdq_pkt_finalize()

Quoted 4 lines · Chun-Kuang Hu

In order to have fine-grained control, use cmdq_pkt_eoc() and cmdq_pkt_jump_rel() to replace cmdq_pkt_finalize().

Quoted 45 lines
---
  drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c | 3 ++-
  drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.c | 2 ++
  drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.h | 1 +
  3 files changed, 5 insertions(+), 1 deletion(-)
drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c +2 -1
index ea2ea119dd2a..ef5dade35fd3 100644
@@ -676,7 +676,8 @@ static struct mdp_cmdq_cmd *mdp_cmdq_prepare(struct mdp_dev *mdp,
  		dev_err(dev, "mdp_path_config error %d\n", pp_idx);
  		goto err_free_path;
  	}
-	cmdq_pkt_finalize(&cmd->pkt);
+	cmdq_pkt_eoc(&cmd->pkt);
+	cmdq_pkt_jump_rel(&cmd->pkt, CMDQ_INST_SIZE, mdp->cmdq_shift_pa[pp_idx]);
 
  	for (i = 0; i < num_comp; i++) {
  		s32 inner_id = MDP_COMP_NONE;
drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.c +2 -0
index 37e7b985d52c..40623c1980bf 100644
@@ -312,6 +312,8 @@ static int mdp_probe(struct platform_device *pdev)
  			ret = PTR_ERR(mdp->cmdq_clt[i]);
  			goto err_mbox_destroy;
  		}
+
+		mdp->cmdq_shift_pa[i] = cmdq_get_shift_pa(mdp->cmdq_clt[i]->chan);
  	}
 
  	init_waitqueue_head(&mdp->callback_wq);
drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.h +1 -0
index 430251f63754..05cade1d098e 100644
@@ -126,6 +126,7 @@ struct mdp_dev {
  	u32					id_count;
  	struct ida				mdp_ida;
  	struct cmdq_client			*cmdq_clt[MDP_PP_MAX];
+	u8					cmdq_shift_pa[MDP_PP_MAX];
  	wait_queue_head_t			callback_wq;
 
  	struct v4l2_device			v4l2_dev;
#4 lore raw

PATCH v4 2/3 media: platform: mtk-mdp3: Use cmdq_pkt_create() and cmdq_pkt_destroy()

In reply to Chun-Kuang Hu

Use cmdq_pkt_create() and cmdq_pkt_destroy() common function instead of implementing mdp3 version.

---
 .../platform/mediatek/mdp3/mtk-mdp3-cmdq.c    | 46 ++-----------------
 .../platform/mediatek/mdp3/mtk-mdp3-cmdq.h    |  1 +
 2 files changed, 6 insertions(+), 41 deletions(-)
drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c +5 -41
index ef5dade35fd3..740a484c8eb4 100644
@@ -471,43 +471,6 @@ static int mdp_path_config(struct mdp_dev *mdp, struct mdp_cmdq_cmd *cmd,
 	return 0;
 }
 
-static int mdp_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)
-		return -ENOMEM;
-
-	pkt->buf_size = size;
-	pkt->cl = (void *)client;
-
-	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);
-		return -ENOMEM;
-	}
-
-	pkt->pa_base = dma_addr;
-
-	return 0;
-}
-
-static void mdp_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);
-	pkt->va_base = NULL;
-}
-
 static void mdp_auto_release_work(struct work_struct *work)
 {
 	struct mdp_cmdq_cmd *cmd;
@@ -538,7 +501,7 @@ static void mdp_auto_release_work(struct work_struct *work)
 		wake_up(&mdp->callback_wq);
 	}
 
-	mdp_cmdq_pkt_destroy(&cmd->pkt);
+	cmdq_pkt_destroy(mdp->cmdq_clt[cmd->pp_idx], &cmd->pkt);
 	kfree(cmd->comps);
 	cmd->comps = NULL;
 	kfree(cmd);
@@ -578,7 +541,7 @@ static void mdp_handle_cmdq_callback(struct mbox_client *cl, void *mssg)
 		if (refcount_dec_and_test(&mdp->job_count))
 			wake_up(&mdp->callback_wq);
 
-		mdp_cmdq_pkt_destroy(&cmd->pkt);
+		cmdq_pkt_destroy(mdp->cmdq_clt[cmd->pp_idx], &cmd->pkt);
 		kfree(cmd->comps);
 		cmd->comps = NULL;
 		kfree(cmd);
@@ -620,7 +583,7 @@ static struct mdp_cmdq_cmd *mdp_cmdq_prepare(struct mdp_dev *mdp,
 		goto err_uninit;
 	}
 
-	ret = mdp_cmdq_pkt_create(mdp->cmdq_clt[pp_idx], &cmd->pkt, SZ_16K);
+	ret = cmdq_pkt_create(mdp->cmdq_clt[pp_idx], &cmd->pkt, SZ_16K);
 	if (ret)
 		goto err_free_cmd;
 
@@ -700,6 +663,7 @@ static struct mdp_cmdq_cmd *mdp_cmdq_prepare(struct mdp_dev *mdp,
 	cmd->comps = comps;
 	cmd->num_comps = num_comp;
 	cmd->mdp_ctx = param->mdp_ctx;
+	cmd->pp_idx = pp_idx;
 
 	kfree(path);
 	return cmd;
@@ -711,7 +675,7 @@ static struct mdp_cmdq_cmd *mdp_cmdq_prepare(struct mdp_dev *mdp,
 err_free_comps:
 	kfree(comps);
 err_destroy_pkt:
-	mdp_cmdq_pkt_destroy(&cmd->pkt);
+	cmdq_pkt_destroy(mdp->cmdq_clt[pp_idx], &cmd->pkt);
 err_free_cmd:
 	kfree(cmd);
 err_uninit:
drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.h +1 -0
index 53a30ad7e0b0..935ae9825728 100644
@@ -35,6 +35,7 @@ struct mdp_cmdq_cmd {
 	struct mdp_comp *comps;
 	void *mdp_ctx;
 	u8 num_comps;
+	u8 pp_idx;
 };
 
 struct mdp_dev;
Signature · 1 line
2.34.1
maintainer
#5 lore raw

Re: [PATCH v4 2/3] media: platform: mtk-mdp3: Use cmdq_pkt_create() and cmdq_pkt_destroy()

Quoted 19 lines · Chun-Kuang Hu

Use cmdq_pkt_create() and cmdq_pkt_destroy() common function instead of implementing mdp3 version.

---
  .../platform/mediatek/mdp3/mtk-mdp3-cmdq.c    | 46 ++-----------------
  .../platform/mediatek/mdp3/mtk-mdp3-cmdq.h    |  1 +
  2 files changed, 6 insertions(+), 41 deletions(-)
drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c +0 -2
index ef5dade35fd3..740a484c8eb4 100644
@@ -471,43 +471,6 @@ static int mdp_path_config(struct mdp_dev *mdp, struct mdp_cmdq_cmd *cmd,
  	return 0;
  }
 
-static int mdp_cmdq_pkt_create(struct cmdq_client *client, struct cmdq_pkt *pkt,
-			       size_t size)

drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c static int mdp_path_config(struct mdp_dev *mdp, struct mdp_cmdq_cmd *cmd, · line 485

Quoted 10 lines
drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c +0 -10
-{
-	struct device *dev;
-	dma_addr_t dma_addr;
-
-	pkt->va_base = kzalloc(size, GFP_KERNEL);
-	if (!pkt->va_base)
-		return -ENOMEM;
-
-	pkt->buf_size = size;
-	pkt->cl = (void *)client;

cmdq_pkt_create does not set the callback. Why doesn't that break things? Same holds for the crtc driver that is already in linux-next.

Regards,
Matthias

Quoted 84 lines
drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c +5 -29
-
-	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);
-		return -ENOMEM;
-	}
-
-	pkt->pa_base = dma_addr;
-
-	return 0;
-}
-
-static void mdp_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);
-	pkt->va_base = NULL;
-}
-
  static void mdp_auto_release_work(struct work_struct *work)
  {
  	struct mdp_cmdq_cmd *cmd;
@@ -538,7 +501,7 @@ static void mdp_auto_release_work(struct work_struct *work)
  		wake_up(&mdp->callback_wq);
  	}
 
-	mdp_cmdq_pkt_destroy(&cmd->pkt);
+	cmdq_pkt_destroy(mdp->cmdq_clt[cmd->pp_idx], &cmd->pkt);
  	kfree(cmd->comps);
  	cmd->comps = NULL;
  	kfree(cmd);
@@ -578,7 +541,7 @@ static void mdp_handle_cmdq_callback(struct mbox_client *cl, void *mssg)
  		if (refcount_dec_and_test(&mdp->job_count))
  			wake_up(&mdp->callback_wq);
 
-		mdp_cmdq_pkt_destroy(&cmd->pkt);
+		cmdq_pkt_destroy(mdp->cmdq_clt[cmd->pp_idx], &cmd->pkt);
  		kfree(cmd->comps);
  		cmd->comps = NULL;
  		kfree(cmd);
@@ -620,7 +583,7 @@ static struct mdp_cmdq_cmd *mdp_cmdq_prepare(struct mdp_dev *mdp,
  		goto err_uninit;
  	}
 
-	ret = mdp_cmdq_pkt_create(mdp->cmdq_clt[pp_idx], &cmd->pkt, SZ_16K);
+	ret = cmdq_pkt_create(mdp->cmdq_clt[pp_idx], &cmd->pkt, SZ_16K);
  	if (ret)
  		goto err_free_cmd;
 
@@ -700,6 +663,7 @@ static struct mdp_cmdq_cmd *mdp_cmdq_prepare(struct mdp_dev *mdp,
  	cmd->comps = comps;
  	cmd->num_comps = num_comp;
  	cmd->mdp_ctx = param->mdp_ctx;
+	cmd->pp_idx = pp_idx;
 
  	kfree(path);
  	return cmd;
@@ -711,7 +675,7 @@ static struct mdp_cmdq_cmd *mdp_cmdq_prepare(struct mdp_dev *mdp,
  err_free_comps:
  	kfree(comps);
  err_destroy_pkt:
-	mdp_cmdq_pkt_destroy(&cmd->pkt);
+	cmdq_pkt_destroy(mdp->cmdq_clt[pp_idx], &cmd->pkt);
  err_free_cmd:
  	kfree(cmd);
  err_uninit:
drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.h +1 -0
index 53a30ad7e0b0..935ae9825728 100644
@@ -35,6 +35,7 @@ struct mdp_cmdq_cmd {
  	struct mdp_comp *comps;
  	void *mdp_ctx;
  	u8 num_comps;
+	u8 pp_idx;
  };
 
  struct mdp_dev;
#6 lore raw

Re: [PATCH v4 2/3] media: platform: mtk-mdp3: Use cmdq_pkt_create() and cmdq_pkt_destroy()

Hi, Matthias:

Quoted 22 lines · Matthias Brugger

External email : Please do not click links or open attachments until you have verified the sender or the content.

Quoted 17 lines · Chun-Kuang Hu

Use cmdq_pkt_create() and cmdq_pkt_destroy() common function instead of implementing mdp3 version.

---
  .../platform/mediatek/mdp3/mtk-mdp3-cmdq.c    | 46 ++-----------------
  .../platform/mediatek/mdp3/mtk-mdp3-cmdq.h    |  1 +
  2 files changed, 6 insertions(+), 41 deletions(-)
drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c +0 -0
index ef5dade35fd3..740a484c8eb4 100644
@@ -471,43 +471,6 @@ static int mdp_path_config(struct mdp_dev *mdp, struct mdp_cmdq_cmd *cmd,
  return 0;
  }
 
Quoted 15 lines

drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c static int mdp_path_config(struct mdp_dev *mdp, struct mdp_cmdq_cmd *cmd, · line 485

Quoted 12 lines
drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c +0 -12
-static int mdp_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)
-return -ENOMEM;
-
-pkt->buf_size = size;
-pkt->cl = (void *)client;

cmdq_pkt_create does not set the callback. Why doesn't that break things? Same holds for the crtc driver that is already in linux-next.

This series is to remove pkt->cl.
For the helper function whick need pkt->cl, add parameter to pass the cmdq_client information.
Up to now, only cmdq_pkt_finalize() use pkt->cl, but it is replaced by cmdq_pkt_eoc() and cmdq_pkt_jump_rel() in [PATCH v4 1/3]. So this would not break any thing.

Regards,
CK

Quoted 88 lines

Regards,
Matthias

Quoted 84 lines
drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c +5 -29
-
-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);
-return -ENOMEM;
-}
-
-pkt->pa_base = dma_addr;
-
-return 0;
-}
-
-static void mdp_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);
-pkt->va_base = NULL;
-}
-
  static void mdp_auto_release_work(struct work_struct *work)
  {
  struct mdp_cmdq_cmd *cmd;
@@ -538,7 +501,7 @@ static void mdp_auto_release_work(struct work_struct *work)
  wake_up(&mdp->callback_wq);
  }
 
-mdp_cmdq_pkt_destroy(&cmd->pkt);
+cmdq_pkt_destroy(mdp->cmdq_clt[cmd->pp_idx], &cmd->pkt);
  kfree(cmd->comps);
  cmd->comps = NULL;
  kfree(cmd);
@@ -578,7 +541,7 @@ static void mdp_handle_cmdq_callback(struct mbox_client *cl, void *mssg)
  if (refcount_dec_and_test(&mdp->job_count))
  wake_up(&mdp->callback_wq);
 
-mdp_cmdq_pkt_destroy(&cmd->pkt);
+cmdq_pkt_destroy(mdp->cmdq_clt[cmd->pp_idx], &cmd->pkt);
  kfree(cmd->comps);
  cmd->comps = NULL;
  kfree(cmd);
@@ -620,7 +583,7 @@ static struct mdp_cmdq_cmd *mdp_cmdq_prepare(struct mdp_dev *mdp,
  goto err_uninit;
  }
 
-ret = mdp_cmdq_pkt_create(mdp->cmdq_clt[pp_idx], &cmd->pkt, SZ_16K);
+ret = cmdq_pkt_create(mdp->cmdq_clt[pp_idx], &cmd->pkt, SZ_16K);
  if (ret)
  goto err_free_cmd;
 
@@ -700,6 +663,7 @@ static struct mdp_cmdq_cmd *mdp_cmdq_prepare(struct mdp_dev *mdp,
  cmd->comps = comps;
  cmd->num_comps = num_comp;
  cmd->mdp_ctx = param->mdp_ctx;
+cmd->pp_idx = pp_idx;
 
  kfree(path);
  return cmd;
@@ -711,7 +675,7 @@ static struct mdp_cmdq_cmd *mdp_cmdq_prepare(struct mdp_dev *mdp,
  err_free_comps:
  kfree(comps);
  err_destroy_pkt:
-mdp_cmdq_pkt_destroy(&cmd->pkt);
+cmdq_pkt_destroy(mdp->cmdq_clt[pp_idx], &cmd->pkt);
  err_free_cmd:
  kfree(cmd);
  err_uninit:
drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.h +1 -0
index 53a30ad7e0b0..935ae9825728 100644
@@ -35,6 +35,7 @@ struct mdp_cmdq_cmd {
  struct mdp_comp *comps;
  void *mdp_ctx;
  u8 num_comps;
+u8 pp_idx;
  };
 
  struct mdp_dev;
maintainer Reviewed-by
#7 lore raw

Re: [PATCH v4 2/3] media: platform: mtk-mdp3: Use cmdq_pkt_create() and cmdq_pkt_destroy()

Quoted 41 lines · CK Hu (胡俊光)

Hi, Matthias:

Quoted 22 lines · Matthias Brugger

External email : Please do not click links or open attachments until you have verified the sender or the content.

Quoted 17 lines · Chun-Kuang Hu

Use cmdq_pkt_create() and cmdq_pkt_destroy() common function instead of implementing mdp3 version.

---
   .../platform/mediatek/mdp3/mtk-mdp3-cmdq.c    | 46 ++-----------------
   .../platform/mediatek/mdp3/mtk-mdp3-cmdq.h    |  1 +
   2 files changed, 6 insertions(+), 41 deletions(-)
drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c +0 -0
index ef5dade35fd3..740a484c8eb4 100644
@@ -471,43 +471,6 @@ static int mdp_path_config(struct mdp_dev *mdp, struct mdp_cmdq_cmd *cmd,
   return 0;
   }
 
Quoted 15 lines

drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c static int mdp_path_config(struct mdp_dev *mdp, struct mdp_cmdq_cmd *cmd, · line 485

Quoted 12 lines
drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c +0 -12
-static int mdp_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)
-return -ENOMEM;
-
-pkt->buf_size = size;
-pkt->cl = (void *)client;

cmdq_pkt_create does not set the callback. Why doesn't that break things? Same holds for the crtc driver that is already in linux-next.

Quoted 5 lines

This series is to remove pkt->cl.
For the helper function whick need pkt->cl, add parameter to pass the cmdq_client information.
Up to now, only cmdq_pkt_finalize() use pkt->cl, but it is replaced by cmdq_pkt_eoc() and cmdq_pkt_jump_rel() in [PATCH v4 1/3]. So this would not break any thing.

Right, actually this is explained in the cover letter. Sorry for the noise.

Quoted 91 lines

Regards,
CK

Quoted 88 lines

Regards,
Matthias

Quoted 84 lines
drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c +5 -29
-
-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);
-return -ENOMEM;
-}
-
-pkt->pa_base = dma_addr;
-
-return 0;
-}
-
-static void mdp_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);
-pkt->va_base = NULL;
-}
-
   static void mdp_auto_release_work(struct work_struct *work)
   {
   struct mdp_cmdq_cmd *cmd;
@@ -538,7 +501,7 @@ static void mdp_auto_release_work(struct work_struct *work)
   wake_up(&mdp->callback_wq);
   }
 
-mdp_cmdq_pkt_destroy(&cmd->pkt);
+cmdq_pkt_destroy(mdp->cmdq_clt[cmd->pp_idx], &cmd->pkt);
   kfree(cmd->comps);
   cmd->comps = NULL;
   kfree(cmd);
@@ -578,7 +541,7 @@ static void mdp_handle_cmdq_callback(struct mbox_client *cl, void *mssg)
   if (refcount_dec_and_test(&mdp->job_count))
   wake_up(&mdp->callback_wq);
 
-mdp_cmdq_pkt_destroy(&cmd->pkt);
+cmdq_pkt_destroy(mdp->cmdq_clt[cmd->pp_idx], &cmd->pkt);
   kfree(cmd->comps);
   cmd->comps = NULL;
   kfree(cmd);
@@ -620,7 +583,7 @@ static struct mdp_cmdq_cmd *mdp_cmdq_prepare(struct mdp_dev *mdp,
   goto err_uninit;
   }
 
-ret = mdp_cmdq_pkt_create(mdp->cmdq_clt[pp_idx], &cmd->pkt, SZ_16K);
+ret = cmdq_pkt_create(mdp->cmdq_clt[pp_idx], &cmd->pkt, SZ_16K);
   if (ret)
   goto err_free_cmd;
 
@@ -700,6 +663,7 @@ static struct mdp_cmdq_cmd *mdp_cmdq_prepare(struct mdp_dev *mdp,
   cmd->comps = comps;
   cmd->num_comps = num_comp;
   cmd->mdp_ctx = param->mdp_ctx;
+cmd->pp_idx = pp_idx;
 
   kfree(path);
   return cmd;
@@ -711,7 +675,7 @@ static struct mdp_cmdq_cmd *mdp_cmdq_prepare(struct mdp_dev *mdp,
   err_free_comps:
   kfree(comps);
   err_destroy_pkt:
-mdp_cmdq_pkt_destroy(&cmd->pkt);
+cmdq_pkt_destroy(mdp->cmdq_clt[pp_idx], &cmd->pkt);
   err_free_cmd:
   kfree(cmd);
   err_uninit:
drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.h +1 -0
index 53a30ad7e0b0..935ae9825728 100644
@@ -35,6 +35,7 @@ struct mdp_cmdq_cmd {
   struct mdp_comp *comps;
   void *mdp_ctx;
   u8 num_comps;
+u8 pp_idx;
   };
 
   struct mdp_dev;
#9 lore raw

PATCH v4 3/3 soc: mediatek: cmdq: Remove cmdq_pkt_finalize() helper function

In reply to Chun-Kuang Hu

In order to have fine-grained control, use cmdq_pkt_eoc() and cmdq_pkt_jump_rel() to replace cmdq_pkt_finalize().

---
 drivers/soc/mediatek/mtk-cmdq-helper.c | 22 ----------------------
 include/linux/soc/mediatek/mtk-cmdq.h  | 13 -------------
 2 files changed, 35 deletions(-)
drivers/soc/mediatek/mtk-cmdq-helper.c +0 -22
index a8fccedba83f..2a47dda4dd4a 100644
@@ -538,27 +538,5 @@ int cmdq_pkt_eoc(struct cmdq_pkt *pkt)
 }
 EXPORT_SYMBOL(cmdq_pkt_eoc);
 
-int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
-{
-	struct cmdq_instruction inst = { {0} };
-	int err;
-
-	/* insert EOC and generate IRQ for each command iteration */
-	inst.op = CMDQ_CODE_EOC;
-	inst.value = CMDQ_EOC_IRQ_EN;
-	err = cmdq_pkt_append_command(pkt, inst);
-	if (err < 0)
-		return err;
-
-	/* JUMP to end */
-	inst.op = CMDQ_CODE_JUMP;
-	inst.value = CMDQ_JUMP_PASS >>
-		cmdq_get_shift_pa(((struct cmdq_client *)pkt->cl)->chan);
-	err = cmdq_pkt_append_command(pkt, inst);
-
-	return err;
-}
-EXPORT_SYMBOL(cmdq_pkt_finalize);
-
 MODULE_DESCRIPTION("MediaTek Command Queue (CMDQ) driver");
 MODULE_LICENSE("GPL v2");
include/linux/soc/mediatek/mtk-cmdq.h +0 -13
index 5bee6f7fc400..0c3906e8ad19 100644
@@ -391,14 +391,6 @@ int cmdq_pkt_jump_rel(struct cmdq_pkt *pkt, s32 offset, u8 shift_pa);
  */
 int cmdq_pkt_eoc(struct cmdq_pkt *pkt);
 
-/**
- * cmdq_pkt_finalize() - Append EOC and jump command to pkt.
- * @pkt:	the CMDQ packet
- *
- * Return: 0 for success; else the error code is returned
- */
-int cmdq_pkt_finalize(struct cmdq_pkt *pkt);
-
 #else /* IS_ENABLED(CONFIG_MTK_CMDQ) */
 
 static inline int cmdq_dev_get_client_reg(struct device *dev,
@@ -519,11 +511,6 @@ static inline int cmdq_pkt_eoc(struct cmdq_pkt *pkt)
 	return -EINVAL;
 }
 
-static inline int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
-{
-	return -EINVAL;
-}
-
 #endif /* IS_ENABLED(CONFIG_MTK_CMDQ) */
 
 #endif	/* __MTK_CMDQ_H__ */
Signature · 1 line
2.34.1
maintainer Acked-by on v4 3/3
#10 lore raw

Re: [PATCH v4 3/3] soc: mediatek: cmdq: Remove cmdq_pkt_finalize() helper function

Quoted 4 lines · Chun-Kuang Hu

In order to have fine-grained control, use cmdq_pkt_eoc() and cmdq_pkt_jump_rel() to replace cmdq_pkt_finalize().

Quoted 68 lines
---
  drivers/soc/mediatek/mtk-cmdq-helper.c | 22 ----------------------
  include/linux/soc/mediatek/mtk-cmdq.h  | 13 -------------
  2 files changed, 35 deletions(-)
drivers/soc/mediatek/mtk-cmdq-helper.c +0 -22
index a8fccedba83f..2a47dda4dd4a 100644
@@ -538,27 +538,5 @@ int cmdq_pkt_eoc(struct cmdq_pkt *pkt)
  }
  EXPORT_SYMBOL(cmdq_pkt_eoc);
 
-int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
-{
-	struct cmdq_instruction inst = { {0} };
-	int err;
-
-	/* insert EOC and generate IRQ for each command iteration */
-	inst.op = CMDQ_CODE_EOC;
-	inst.value = CMDQ_EOC_IRQ_EN;
-	err = cmdq_pkt_append_command(pkt, inst);
-	if (err < 0)
-		return err;
-
-	/* JUMP to end */
-	inst.op = CMDQ_CODE_JUMP;
-	inst.value = CMDQ_JUMP_PASS >>
-		cmdq_get_shift_pa(((struct cmdq_client *)pkt->cl)->chan);
-	err = cmdq_pkt_append_command(pkt, inst);
-
-	return err;
-}
-EXPORT_SYMBOL(cmdq_pkt_finalize);
-
  MODULE_DESCRIPTION("MediaTek Command Queue (CMDQ) driver");
  MODULE_LICENSE("GPL v2");
include/linux/soc/mediatek/mtk-cmdq.h +0 -13
index 5bee6f7fc400..0c3906e8ad19 100644
@@ -391,14 +391,6 @@ int cmdq_pkt_jump_rel(struct cmdq_pkt *pkt, s32 offset, u8 shift_pa);
   */
  int cmdq_pkt_eoc(struct cmdq_pkt *pkt);
 
-/**
- * cmdq_pkt_finalize() - Append EOC and jump command to pkt.
- * @pkt:	the CMDQ packet
- *
- * Return: 0 for success; else the error code is returned
- */
-int cmdq_pkt_finalize(struct cmdq_pkt *pkt);
-
  #else /* IS_ENABLED(CONFIG_MTK_CMDQ) */
 
  static inline int cmdq_dev_get_client_reg(struct device *dev,
@@ -519,11 +511,6 @@ static inline int cmdq_pkt_eoc(struct cmdq_pkt *pkt)
  	return -EINVAL;
  }
 
-static inline int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
-{
-	return -EINVAL;
-}
-
  #endif /* IS_ENABLED(CONFIG_MTK_CMDQ) */
 
  #endif	/* __MTK_CMDQ_H__ */
maintainer Reviewed-by on v4 3/3
#11 lore raw

Re: [PATCH v4 3/3] soc: mediatek: cmdq: Remove cmdq_pkt_finalize() helper function

In reply to Chun-Kuang Hu

Il 01/09/24 16:32, Chun-Kuang Hu ha scritto:

Quoted 4 lines

In order to have fine-grained control, use cmdq_pkt_eoc() and cmdq_pkt_jump_rel() to replace cmdq_pkt_finalize().

I'll pick this one after the media patches will be mainlined to avoid issues.

In the meanwhile, mainly as a self-reminder: