From: Hsiao Chien Sung <shawn.sung@mediatek.com>

Register CRC related function pointers to support
CRC retrieval.

Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_crtc.c     | 280 ++++++++++++++++++++++++
 drivers/gpu/drm/mediatek/mtk_crtc.h     |  38 ++++
 drivers/gpu/drm/mediatek/mtk_ddp_comp.h |   5 +
 3 files changed, 323 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_crtc.c b/drivers/gpu/drm/mediatek/mtk_crtc.c
index 6f34f573e127..be7cf61b9f9b 100644
--- a/drivers/gpu/drm/mediatek/mtk_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_crtc.c
@@ -18,6 +18,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_vblank.h>
+#include <drm/drm_vblank_work.h>

 #include "mtk_crtc.h"
 #include "mtk_ddp_comp.h"
@@ -69,6 +70,9 @@ struct mtk_crtc {
 	/* lock for display hardware access */
 	struct mutex			hw_lock;
 	bool				config_updating;
+
+	struct mtk_ddp_comp		*crc_provider;
+	struct drm_vblank_work		crc_work;
 };

 struct mtk_crtc_state {
@@ -703,6 +707,88 @@ static void mtk_crtc_update_output(struct drm_crtc *crtc,
 	}
 }

+static void mtk_crtc_crc_work(struct kthread_work *base)
+{
+	struct drm_vblank_work *work = to_drm_vblank_work(base);
+	struct mtk_crtc *mtk_crtc =
+		container_of(work, typeof(*mtk_crtc), crc_work);
+	struct mtk_ddp_comp *comp = mtk_crtc->crc_provider;
+
+	if (!comp) {
+		DRM_WARN("%s(crtc-%d): no crc provider\n",
+			 __func__, drm_crtc_index(&mtk_crtc->base));
+		return;
+	}
+
+	if (mtk_crtc->base.crc.opened) {
+		u64 vblank = drm_crtc_vblank_count(&mtk_crtc->base);
+
+		comp->funcs->crc_read(comp->dev);
+
+		/* could take more than 50ms to finish */
+		drm_crtc_add_crc_entry(&mtk_crtc->base, true, vblank,
+				       comp->funcs->crc_entry(comp->dev));
+
+		drm_vblank_work_schedule(&mtk_crtc->crc_work, vblank + 1, true);
+	} else {
+		comp->funcs->crc_stop(comp->dev);
+	}
+}
+
+static int mtk_crtc_set_crc_source(struct drm_crtc *crtc, const char *src)
+{
+	struct mtk_crtc *mtk_crtc = to_mtk_crtc(crtc);
+	struct mtk_ddp_comp *comp = mtk_crtc->crc_provider;
+
+	if (!comp) {
+		DRM_ERROR("%s(crtc-%d): no crc provider\n",
+			  __func__, drm_crtc_index(crtc));
+		return -ENOENT;
+	}
+
+	if (!src)
+		return -EINVAL;
+
+	if (strcmp(src, "auto") != 0) {
+		DRM_ERROR("%s(crtc-%d): unknown source '%s'\n",
+			  __func__, drm_crtc_index(crtc), src);
+		return -EINVAL;
+	}
+
+	comp->funcs->crc_start(comp->dev);
+
+	/*
+	 * skip the first crc because the first frame (vblank + 1) is configured
+	 * by mtk_crtc_ddp_hw_init() when atomic enable
+	 */
+	drm_vblank_work_schedule(&mtk_crtc->crc_work,
+				 drm_crtc_vblank_count(crtc) + 2, false);
+	return 0;
+}
+
+static int mtk_crtc_verify_crc_source(struct drm_crtc *crtc, const char *src,
+				      size_t *cnt)
+{
+	struct mtk_crtc *mtk_crtc = to_mtk_crtc(crtc);
+	struct mtk_ddp_comp *comp = mtk_crtc->crc_provider;
+
+	if (!comp) {
+		DRM_ERROR("%s(crtc-%d): no crc provider\n",
+			  __func__, drm_crtc_index(crtc));
+		return -ENOENT;
+	}
+
+	if (src && strcmp(src, "auto") != 0) {
+		DRM_ERROR("%s(crtc-%d): unknown source '%s'\n",
+			  __func__, drm_crtc_index(crtc), src);
+		return -EINVAL;
+	}
+
+	*cnt = comp->funcs->crc_cnt(comp->dev);
+
+	return 0;
+}
+
 int mtk_crtc_plane_check(struct drm_crtc *crtc, struct drm_plane *plane,
 			 struct mtk_plane_state *state)
 {
@@ -751,6 +837,8 @@ static void mtk_crtc_atomic_enable(struct drm_crtc *crtc,

 	drm_crtc_vblank_on(crtc);
 	mtk_crtc->enabled = true;
+
+	drm_vblank_work_init(&mtk_crtc->crc_work, crtc, mtk_crtc_crc_work);
 }

 static void mtk_crtc_atomic_disable(struct drm_crtc *crtc,
@@ -840,6 +928,8 @@ static const struct drm_crtc_funcs mtk_crtc_funcs = {
 	.atomic_destroy_state	= mtk_crtc_destroy_state,
 	.enable_vblank		= mtk_crtc_enable_vblank,
 	.disable_vblank		= mtk_crtc_disable_vblank,
+	.set_crc_source		= mtk_crtc_set_crc_source,
+	.verify_crc_source	= mtk_crtc_verify_crc_source,
 };

 static const struct drm_crtc_helper_funcs mtk_crtc_helper_funcs = {
@@ -1033,6 +1123,13 @@ int mtk_crtc_create(struct drm_device *drm_dev, const unsigned int *path,

 			if (comp->funcs->ctm_set)
 				has_ctm = true;
+
+			if (comp->funcs->crc_cnt   &&
+			    comp->funcs->crc_entry &&
+			    comp->funcs->crc_read  &&
+			    comp->funcs->crc_start &&
+			    comp->funcs->crc_stop)
+				mtk_crtc->crc_provider = comp;
 		}

 		mtk_ddp_comp_register_vblank_cb(comp, mtk_crtc_ddp_irq,
@@ -1136,3 +1233,186 @@ int mtk_crtc_create(struct drm_device *drm_dev, const unsigned int *path,

 	return 0;
 }
+
+void mtk_crtc_init_crc(struct mtk_crtc_crc *crc, const u32 *crc_offset_table,
+		       size_t crc_count, u32 reset_offset, u32 reset_mask)
+{
+	crc->ofs = crc_offset_table;
+	crc->cnt = crc_count;
+	crc->rst_ofs = reset_offset;
+	crc->rst_msk = reset_mask;
+	crc->va = kcalloc(crc->cnt, sizeof(*crc->va), GFP_KERNEL);
+	if (!crc->va) {
+		DRM_ERROR("failed to allocate memory for crc\n");
+		crc->cnt = 0;
+	}
+}
+
+void mtk_crtc_read_crc(struct mtk_crtc_crc *crc, void __iomem *reg)
+{
+	if (!crc->cnt || !crc->ofs || !crc->va)
+		return;
+
+#if IS_REACHABLE(CONFIG_MTK_CMDQ)
+	/* sync to see the most up-to-date copy of the DMA buffer */
+	dma_sync_single_for_cpu(crc->cmdq_client.chan->mbox->dev,
+				crc->pa, crc->cnt * sizeof(*crc->va),
+				DMA_FROM_DEVICE);
+#endif
+}
+
+void mtk_crtc_destroy_crc(struct mtk_crtc_crc *crc)
+{
+	if (!crc->cnt)
+		return;
+
+#if IS_REACHABLE(CONFIG_MTK_CMDQ)
+	if (crc->pa) {
+		dma_unmap_single(crc->cmdq_client.chan->mbox->dev,
+				 crc->pa, crc->cnt * sizeof(*crc->va),
+				 DMA_TO_DEVICE);
+		crc->pa = 0;
+	}
+	if (crc->cmdq_client.chan) {
+		mtk_drm_cmdq_pkt_destroy(&crc->cmdq_handle);
+		mbox_free_channel(crc->cmdq_client.chan);
+		crc->cmdq_client.chan = NULL;
+	}
+#endif
+	kfree(crc->va);
+	crc->va = NULL;
+	crc->cnt = 0;
+}
+
+#if IS_REACHABLE(CONFIG_MTK_CMDQ)
+/**
+ * mtk_crtc_create_crc_cmdq - Create a CMDQ thread for syncing the CRCs
+ * @dev: Kernel device node of the CRC provider
+ * @crc: Pointer of the CRC to init
+ *
+ * This function will create a looping thread on GCE (Global Command Engine) to
+ * keep the CRC up to date by monitoring the assigned event (usually the frame
+ * done event) of the CRC provider, and read the CRCs from the registers to a
+ * shared memory for the workqueue to read. To start/stop the looping thread,
+ * please call `mtk_crtc_start_crc_cmdq()` and `mtk_crtc_stop_crc_cmdq()`
+ * defined blow.
+ *
+ * The reason why we don't update the CRCs with CPU is that the front porch of
+ * 4K60 timing in CEA-861 is less than 60us, and register read/write speed is
+ * relatively unreliable comparing to GCE due to the bus design.
+ *
+ * We must create a new thread instead of using the original one for plane
+ * update is because:
+ * 1. We cannot add another wait-for-event command at the end of cmdq packet, or
+ *    the cmdq callback will delay for too long
+ * 2. Will get the CRC of the previous frame if using the existed wait-for-event
+ *    command which is at the beginning of the packet
+ */
+void mtk_crtc_create_crc_cmdq(struct device *dev, struct mtk_crtc_crc *crc)
+{
+	int i;
+
+	if (!crc->cnt) {
+		dev_warn(dev, "%s: not support\n", __func__);
+		goto cleanup;
+	}
+
+	if (!crc->ofs) {
+		dev_warn(dev, "%s: not defined\n", __func__);
+		goto cleanup;
+	}
+
+	crc->cmdq_client.client.dev = dev;
+	crc->cmdq_client.client.tx_block = false;
+	crc->cmdq_client.client.knows_txdone = true;
+	crc->cmdq_client.client.rx_callback = NULL;
+	crc->cmdq_client.chan = mbox_request_channel(&crc->cmdq_client.client, 0);
+	if (IS_ERR(crc->cmdq_client.chan)) {
+		dev_warn(dev, "%s: failed to create mailbox client\n", __func__);
+		crc->cmdq_client.chan = NULL;
+		goto cleanup;
+	}
+
+	if (mtk_drm_cmdq_pkt_create(&crc->cmdq_client, &crc->cmdq_handle, PAGE_SIZE)) {
+		dev_warn(dev, "%s: failed to create cmdq packet\n", __func__);
+		goto cleanup;
+	}
+
+	if (!crc->va) {
+		dev_warn(dev, "%s: no memory\n", __func__);
+		goto cleanup;
+	}
+
+	/* map the entry to get a dma address for cmdq to store the crc */
+	crc->pa = dma_map_single(crc->cmdq_client.chan->mbox->dev,
+				 crc->va, crc->cnt * sizeof(*crc->va),
+				 DMA_FROM_DEVICE);
+
+	if (dma_mapping_error(crc->cmdq_client.chan->mbox->dev, crc->pa)) {
+		dev_err(dev, "%s: failed to map dma\n", __func__);
+		goto cleanup;
+	}
+
+	if (crc->cmdq_event)
+		cmdq_pkt_wfe(&crc->cmdq_handle, crc->cmdq_event, true);
+
+	for (i = 0; i < crc->cnt; i++) {
+		/* put crc to spr1 register */
+		cmdq_pkt_read_s(&crc->cmdq_handle, crc->cmdq_reg->subsys,
+				crc->cmdq_reg->offset + crc->ofs[i],
+				CMDQ_THR_SPR_IDX1);
+
+		/* copy spr1 register to physical address of the crc */
+		cmdq_pkt_assign(&crc->cmdq_handle, CMDQ_THR_SPR_IDX0,
+				CMDQ_ADDR_HIGH(crc->pa + i * sizeof(*crc->va)));
+		cmdq_pkt_write_s(&crc->cmdq_handle, CMDQ_THR_SPR_IDX0,
+				 CMDQ_ADDR_LOW(crc->pa + i * sizeof(*crc->va)),
+				 CMDQ_THR_SPR_IDX1);
+	}
+	/* reset crc */
+	mtk_ddp_write_mask(&crc->cmdq_handle, ~0, crc->cmdq_reg, 0,
+			   crc->rst_ofs, crc->rst_msk);
+
+	/* clear reset bit */
+	mtk_ddp_write_mask(&crc->cmdq_handle, 0, crc->cmdq_reg, 0,
+			   crc->rst_ofs, crc->rst_msk);
+
+	/* jump to head of the cmdq packet */
+	cmdq_pkt_jump_abs(&crc->cmdq_handle, crc->cmdq_handle.pa_base,
+			  cmdq_get_shift_pa(crc->cmdq_client.chan));
+
+	return;
+cleanup:
+	mtk_crtc_destroy_crc(crc);
+}
+
+/**
+ * mtk_crtc_start_crc_cmdq - Start the GCE looping thread for CRC update
+ * @crc: Pointer of the CRC information
+ */
+void mtk_crtc_start_crc_cmdq(struct mtk_crtc_crc *crc)
+{
+	if (!crc->cmdq_client.chan)
+		return;
+
+	dma_sync_single_for_device(crc->cmdq_client.chan->mbox->dev,
+				   crc->cmdq_handle.pa_base,
+				   crc->cmdq_handle.cmd_buf_size,
+				   DMA_TO_DEVICE);
+	mbox_send_message(crc->cmdq_client.chan, &crc->cmdq_handle);
+	mbox_client_txdone(crc->cmdq_client.chan, 0);
+}
+
+/**
+ * mtk_crtc_stop_crc_cmdq - Stop the GCE looping thread for CRC update
+ * @crc: Pointer of the CRC information
+ */
+void mtk_crtc_stop_crc_cmdq(struct mtk_crtc_crc *crc)
+{
+	if (!crc->cmdq_client.chan)
+		return;
+
+	/* remove all the commands from the cmdq packet */
+	mbox_flush(crc->cmdq_client.chan, 2000);
+}
+#endif
diff --git a/drivers/gpu/drm/mediatek/mtk_crtc.h b/drivers/gpu/drm/mediatek/mtk_crtc.h
index 388e900b6f4d..a79c4611754e 100644
--- a/drivers/gpu/drm/mediatek/mtk_crtc.h
+++ b/drivers/gpu/drm/mediatek/mtk_crtc.h
@@ -14,6 +14,34 @@
 #define MTK_MAX_BPC	10
 #define MTK_MIN_BPC	3

+/**
+ * struct mtk_crtc_crc - crc related information
+ * @ofs: register offset of crc
+ * @rst_ofs: register offset of crc reset
+ * @rst_msk: register mask of crc reset
+ * @cnt: count of crc
+ * @va: pointer to the start of crc array
+ * @pa: physical address of the crc for gce to access
+ * @cmdq_event: the event to trigger the cmdq
+ * @cmdq_reg: address of the register that cmdq is going to access
+ * @cmdq_client: handler to control cmdq (mbox channel, thread ...etc.)
+ * @cmdq_handle: cmdq packet to store the commands
+ */
+struct mtk_crtc_crc {
+	const u32 *ofs;
+	u32 rst_ofs;
+	u32 rst_msk;
+	size_t cnt;
+	u32 *va;
+#if IS_REACHABLE(CONFIG_MTK_CMDQ)
+	dma_addr_t pa;
+	u32 cmdq_event;
+	struct cmdq_client_reg *cmdq_reg;
+	struct cmdq_client cmdq_client;
+	struct cmdq_pkt cmdq_handle;
+#endif
+};
+
 void mtk_crtc_commit(struct drm_crtc *crtc);
 int mtk_crtc_create(struct drm_device *drm_dev, const unsigned int *path,
 		    unsigned int path_len, int priv_data_index,
@@ -25,4 +53,14 @@ void mtk_crtc_async_update(struct drm_crtc *crtc, struct drm_plane *plane,
 			   struct drm_atomic_state *plane_state);
 struct device *mtk_crtc_dma_dev_get(struct drm_crtc *crtc);

+void mtk_crtc_init_crc(struct mtk_crtc_crc *crc, const u32 *crc_offset_table,
+		       size_t crc_count, u32 reset_offset, u32 reset_mask);
+void mtk_crtc_read_crc(struct mtk_crtc_crc *crc, void __iomem *reg);
+void mtk_crtc_destroy_crc(struct mtk_crtc_crc *crc);
+#if IS_REACHABLE(CONFIG_MTK_CMDQ)
+void mtk_crtc_create_crc_cmdq(struct device *dev, struct mtk_crtc_crc *crc);
+void mtk_crtc_start_crc_cmdq(struct mtk_crtc_crc *crc);
+void mtk_crtc_stop_crc_cmdq(struct mtk_crtc_crc *crc);
+#endif
+
 #endif /* MTK_CRTC_H */
diff --git a/drivers/gpu/drm/mediatek/mtk_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_ddp_comp.h
index f7fe2e08dc8e..b220a672d182 100644
--- a/drivers/gpu/drm/mediatek/mtk_ddp_comp.h
+++ b/drivers/gpu/drm/mediatek/mtk_ddp_comp.h
@@ -88,6 +88,11 @@ struct mtk_ddp_comp_funcs {
 	void (*remove)(struct device *dev, struct mtk_mutex *mutex);
 	unsigned int (*encoder_index)(struct device *dev);
 	enum drm_mode_status (*mode_valid)(struct device *dev, const struct drm_display_mode *mode);
+	size_t (*crc_cnt)(struct device *dev);
+	u32 *(*crc_entry)(struct device *dev);
+	void (*crc_read)(struct device *dev);
+	void (*crc_start)(struct device *dev);
+	void (*crc_stop)(struct device *dev);
 };

 struct mtk_ddp_comp {
--
2.18.0

Hi Shawn,

kernel test robot noticed the following build warnings:

[auto build test WARNING on v6.10-rc2]
[also build test WARNING on linus/master next-20240613]
[cannot apply to chrome-os/chromeos-6.1]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Shawn-Sung/soc-mediatek-Disable-9-bit-alpha-in-ETHDR/20240606-172904
base:   v6.10-rc2
patch link:    https://lore.kernel.org/r/20240606092635.27981-15-shawn.sung%40mediatek.com
patch subject: [PATCH v8 14/16] drm/mediatek: Support CRC in display driver
config: arm64-randconfig-r113-20240615 (https://download.01.org/0day-ci/archive/20240616/202406160407.yfZ2cNh2-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240616/202406160407.yfZ2cNh2-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406160407.yfZ2cNh2-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/gpu/drm/mediatek/mtk_crtc.c:1373:66: sparse: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/mediatek/mtk_crtc.c:1377:65: sparse: sparse: Using plain integer as NULL pointer

vim +1373 drivers/gpu/drm/mediatek/mtk_crtc.c

  1286	
  1287	#if IS_REACHABLE(CONFIG_MTK_CMDQ)
  1288	/**
  1289	 * mtk_crtc_create_crc_cmdq - Create a CMDQ thread for syncing the CRCs
  1290	 * @dev: Kernel device node of the CRC provider
  1291	 * @crc: Pointer of the CRC to init
  1292	 *
  1293	 * This function will create a looping thread on GCE (Global Command Engine) to
  1294	 * keep the CRC up to date by monitoring the assigned event (usually the frame
  1295	 * done event) of the CRC provider, and read the CRCs from the registers to a
  1296	 * shared memory for the workqueue to read. To start/stop the looping thread,
  1297	 * please call `mtk_crtc_start_crc_cmdq()` and `mtk_crtc_stop_crc_cmdq()`
  1298	 * defined blow.
  1299	 *
  1300	 * The reason why we don't update the CRCs with CPU is that the front porch of
  1301	 * 4K60 timing in CEA-861 is less than 60us, and register read/write speed is
  1302	 * relatively unreliable comparing to GCE due to the bus design.
  1303	 *
  1304	 * We must create a new thread instead of using the original one for plane
  1305	 * update is because:
  1306	 * 1. We cannot add another wait-for-event command at the end of cmdq packet, or
  1307	 *    the cmdq callback will delay for too long
  1308	 * 2. Will get the CRC of the previous frame if using the existed wait-for-event
  1309	 *    command which is at the beginning of the packet
  1310	 */
  1311	void mtk_crtc_create_crc_cmdq(struct device *dev, struct mtk_crtc_crc *crc)
  1312	{
  1313		int i;
  1314	
  1315		if (!crc->cnt) {
  1316			dev_warn(dev, "%s: not support\n", __func__);
  1317			goto cleanup;
  1318		}
  1319	
  1320		if (!crc->ofs) {
  1321			dev_warn(dev, "%s: not defined\n", __func__);
  1322			goto cleanup;
  1323		}
  1324	
  1325		crc->cmdq_client.client.dev = dev;
  1326		crc->cmdq_client.client.tx_block = false;
  1327		crc->cmdq_client.client.knows_txdone = true;
  1328		crc->cmdq_client.client.rx_callback = NULL;
  1329		crc->cmdq_client.chan = mbox_request_channel(&crc->cmdq_client.client, 0);
  1330		if (IS_ERR(crc->cmdq_client.chan)) {
  1331			dev_warn(dev, "%s: failed to create mailbox client\n", __func__);
  1332			crc->cmdq_client.chan = NULL;
  1333			goto cleanup;
  1334		}
  1335	
  1336		if (mtk_drm_cmdq_pkt_create(&crc->cmdq_client, &crc->cmdq_handle, PAGE_SIZE)) {
  1337			dev_warn(dev, "%s: failed to create cmdq packet\n", __func__);
  1338			goto cleanup;
  1339		}
  1340	
  1341		if (!crc->va) {
  1342			dev_warn(dev, "%s: no memory\n", __func__);
  1343			goto cleanup;
  1344		}
  1345	
  1346		/* map the entry to get a dma address for cmdq to store the crc */
  1347		crc->pa = dma_map_single(crc->cmdq_client.chan->mbox->dev,
  1348					 crc->va, crc->cnt * sizeof(*crc->va),
  1349					 DMA_FROM_DEVICE);
  1350	
  1351		if (dma_mapping_error(crc->cmdq_client.chan->mbox->dev, crc->pa)) {
  1352			dev_err(dev, "%s: failed to map dma\n", __func__);
  1353			goto cleanup;
  1354		}
  1355	
  1356		if (crc->cmdq_event)
  1357			cmdq_pkt_wfe(&crc->cmdq_handle, crc->cmdq_event, true);
  1358	
  1359		for (i = 0; i < crc->cnt; i++) {
  1360			/* put crc to spr1 register */
  1361			cmdq_pkt_read_s(&crc->cmdq_handle, crc->cmdq_reg->subsys,
  1362					crc->cmdq_reg->offset + crc->ofs[i],
  1363					CMDQ_THR_SPR_IDX1);
  1364	
  1365			/* copy spr1 register to physical address of the crc */
  1366			cmdq_pkt_assign(&crc->cmdq_handle, CMDQ_THR_SPR_IDX0,
  1367					CMDQ_ADDR_HIGH(crc->pa + i * sizeof(*crc->va)));
  1368			cmdq_pkt_write_s(&crc->cmdq_handle, CMDQ_THR_SPR_IDX0,
  1369					 CMDQ_ADDR_LOW(crc->pa + i * sizeof(*crc->va)),
  1370					 CMDQ_THR_SPR_IDX1);
  1371		}
  1372		/* reset crc */
> 1373		mtk_ddp_write_mask(&crc->cmdq_handle, ~0, crc->cmdq_reg, 0,
  1374				   crc->rst_ofs, crc->rst_msk);
  1375	
  1376		/* clear reset bit */
  1377		mtk_ddp_write_mask(&crc->cmdq_handle, 0, crc->cmdq_reg, 0,
  1378				   crc->rst_ofs, crc->rst_msk);
  1379	
  1380		/* jump to head of the cmdq packet */
  1381		cmdq_pkt_jump_abs(&crc->cmdq_handle, crc->cmdq_handle.pa_base,
  1382				  cmdq_get_shift_pa(crc->cmdq_client.chan));
  1383	
  1384		return;
  1385	cleanup:
  1386		mtk_crtc_destroy_crc(crc);
  1387	}
  1388	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Hi, Shawn:

On Thu, 2024-06-06 at 17:26 +0800, Shawn Sung wrote:
> From: Hsiao Chien Sung <shawn.sung@mediatek.com>
> 
> Register CRC related function pointers to support
> CRC retrieval.
> 
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> ---

[snip]

> 
> +static void mtk_crtc_crc_work(struct kthread_work *base)
> +{
> +	struct drm_vblank_work *work = to_drm_vblank_work(base);
> +	struct mtk_crtc *mtk_crtc =
> +		container_of(work, typeof(*mtk_crtc), crc_work);
> +	struct mtk_ddp_comp *comp = mtk_crtc->crc_provider;
> +
> +	if (!comp) {
> +		DRM_WARN("%s(crtc-%d): no crc provider\n",
> +			 __func__, drm_crtc_index(&mtk_crtc->base));
> +		return;
> +	}
> +
> +	if (mtk_crtc->base.crc.opened) {
> +		u64 vblank = drm_crtc_vblank_count(&mtk_crtc->base);
> +
> +		comp->funcs->crc_read(comp->dev);
> +
> +		/* could take more than 50ms to finish */
> +		drm_crtc_add_crc_entry(&mtk_crtc->base, true, vblank,
> +				       comp->funcs->crc_entry(comp->dev));

It seems that you could regenerate cmdq packet for crc here. So crtc
atomic flush and crc could use the same mailbox channel.

Regards,
CK

> +
> +		drm_vblank_work_schedule(&mtk_crtc->crc_work, vblank + 1, true);
> +	} else {
> +		comp->funcs->crc_stop(comp->dev);
> +	}
> +}
> +

Hi CK,

On Wed, 2024-06-12 at 01:22 +0000, CK Hu (胡俊光) wrote:
> Hi, Shawn:
> 
> On Thu, 2024-06-06 at 17:26 +0800, Shawn Sung wrote:
> > From: Hsiao Chien Sung <shawn.sung@mediatek.com>
> > 
> > Register CRC related function pointers to support
> > CRC retrieval.
> > 
> > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> > ---
> 
> [snip]
> 
> > 
> > +static void mtk_crtc_crc_work(struct kthread_work *base)
> > +{
> > +	struct drm_vblank_work *work = to_drm_vblank_work(base);
> > +	struct mtk_crtc *mtk_crtc =
> > +		container_of(work, typeof(*mtk_crtc), crc_work);
> > +	struct mtk_ddp_comp *comp = mtk_crtc->crc_provider;
> > +
> > +	if (!comp) {
> > +		DRM_WARN("%s(crtc-%d): no crc provider\n",
> > +			 __func__, drm_crtc_index(&mtk_crtc->base));
> > +		return;
> > +	}
> > +
> > +	if (mtk_crtc->base.crc.opened) {
> > +		u64 vblank = drm_crtc_vblank_count(&mtk_crtc->base);
> > +
> > +		comp->funcs->crc_read(comp->dev);
> > +
> > +		/* could take more than 50ms to finish */
> > +		drm_crtc_add_crc_entry(&mtk_crtc->base, true, vblank,
> > +				       comp->funcs->crc_entry(comp-
> > >dev));
> 
> It seems that you could regenerate cmdq packet for crc here. So crtc
> atomic flush and crc could use the same mailbox channel.
> 
> Regards,
> CK
> 
It seems that it's possible to call mtk_ddp_update_config() here and
add a new pending_read_crc flag to insert the read crc related
instructions into the same cmdq_pkt and use the same mailbox channel as
atomic flush.

I'll try this, but I'll need some time to modify this and test it to 
make sure no other issues arise.

Regards,
Jason-Jh Lin

> > +
> > +		drm_vblank_work_schedule(&mtk_crtc->crc_work, vblank +
> > 1, true);
> > +	} else {
> > +		comp->funcs->crc_stop(comp->dev);
> > +	}
> > +}
> > +