Re: [PATCH v5 10/13] drm/mediatek: Support CRC in display driver

Hi Angelo,

On Thu, 2024-02-15 at 12:06 +0100, AngeloGioacchino Del Regno wrote:

Il 15/02/24 11:11, Hsiao Chien Sung ha scritto:

quoted

Register CRC related function pointers to support
CRC retrieval.

Signed-off-by: Hsiao Chien Sung <redacted>
---
  drivers/gpu/drm/mediatek/mtk_drm_crtc.c     | 239
++++++++++++++++++++
  drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |  39 ++++
  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   3 +
  3 files changed, 281 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c

b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 14cf75fa217f9..6cb1ed419dee7 100644

--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c

@@ -68,6 +68,9 @@ struct mtk_drm_crtc {
  	/* lock for display hardware access */
  	struct mutex			hw_lock;
  	bool				config_updating;
+
+	struct mtk_ddp_comp		*crc_provider;
+	unsigned int			frames;
  };
  
  struct mtk_crtc_state {

@@ -635,6 +638,14 @@ static void mtk_crtc_ddp_irq(void *data)
  	struct drm_crtc *crtc = data;
  	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
  	struct mtk_drm_private *priv = crtc->dev->dev_private;
+	struct mtk_ddp_comp *comp = mtk_crtc->crc_provider;
+
+	/*
+	 * crc providers should make sure the crc is always correct
+	 * by resetting it in .crc_read()
+	 */
+	if (crtc->crc.opened)
+		comp->funcs->crc_read(comp->dev);
  
  #if IS_REACHABLE(CONFIG_MTK_CMDQ)
  	if (!priv->data->shadow_register && !mtk_crtc-

quoted

cmdq_client.chan)

@@ -646,6 +657,24 @@ static void mtk_crtc_ddp_irq(void *data)
  	if (!priv->data->shadow_register)
  		mtk_crtc_ddp_config(crtc, NULL);
  #endif
+
+	/*
+	 * drm_crtc_add_crc_entry() could take more than 50ms to finish
+	 * put it at the end of the isr
+	 */
+	if (crtc->crc.opened) {
+		/*
+		 * skip the first crc because the first frame is

configured by
+		 * mtk_crtc_ddp_hw_init() when atomic enable
+		 */
+		if (++mtk_crtc->frames > 1) {
+			drm_crtc_add_crc_entry(crtc, true,
+					       drm_crtc_vblank_count(cr
tc),
+					       comp->funcs-

quoted

crc_entry(comp->dev));

+		}
+	} else {
+		mtk_crtc->frames = 0;
+	}
  	mtk_drm_finish_page_flip(mtk_crtc);
  }
  

@@ -704,6 +733,40 @@ static void mtk_drm_crtc_update_output(struct

drm_crtc *crtc,
  	}
  }
  
+static int mtk_drm_crtc_set_crc_source(struct drm_crtc *crtc,
const char *src)
+{
+	if (src && strcmp(src, "auto") != 0) {
+		DRM_ERROR("%s(crtc-%d): unknown source '%s'\n",
+			  __func__, drm_crtc_index(crtc), src);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int mtk_drm_crtc_verify_crc_source(struct drm_crtc *crtc,
+					  const char *src,
+					  size_t *cnt)
+{
+	struct mtk_drm_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_drm_crtc_plane_check(struct drm_crtc *crtc, struct
drm_plane *plane,
  			     struct mtk_plane_state *state)
  {

@@ -841,6 +904,8 @@ static const struct drm_crtc_funcs

mtk_crtc_funcs = {
  	.atomic_destroy_state	= mtk_drm_crtc_destroy_state,
  	.enable_vblank		= mtk_drm_crtc_enable_vblank,
  	.disable_vblank		= mtk_drm_crtc_disable_vblank,
+	.set_crc_source		= mtk_drm_crtc_set_crc_source,
+	.verify_crc_source	= mtk_drm_crtc_verify_crc_source,
  };
  
  static const struct drm_crtc_helper_funcs mtk_crtc_helper_funcs =
{

@@ -1033,6 +1098,11 @@ int mtk_drm_crtc_create(struct drm_device

*drm_dev,
  
  			if (comp->funcs->ctm_set)
  				has_ctm = true;
+
+			if (comp->funcs->crc_cnt &&
+			    comp->funcs->crc_entry &&
+			    comp->funcs->crc_read)
+				mtk_crtc->crc_provider = comp;
  		}
  
  		mtk_ddp_comp_register_vblank_cb(comp, mtk_crtc_ddp_irq,

@@ -1137,3 +1207,172 @@ int mtk_drm_crtc_create(struct drm_device

*drm_dev,
  
  	return 0;
  }
+
+void mtk_drm_crc_init(struct mtk_drm_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_drm_crc_read(struct mtk_drm_crc *crc, void __iomem *reg)
+{

	u32 n; goes here

quoted

+	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);
+#else
+	/* read crc with cpu for the platforms without cmdq */
+	{

then you don't need the braces.

Regards,
Angelo

The variable n is placed within the braces because when CONFIG_MTK_CMDQ
is defined, the variable is not being used and causes a build error.

This is a workaround in order to have only one #if macro in that
function for a more clean code.

Thanks,
Shawn


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel