CMDQ refinement of Mediatek DRM driver

PATCH In review Chun-Kuang Hu 2 participants 12 messages Aug 8, 2021 – Sep 29, 2021

Subsystems (3)

DRM drivers Arm MediaTek DRM drivers for MediaTek

lore

Share & tools

lore

Chun-Kuang Hu

author maintainer

2021-08-08 23:47 UTC #0 lore raw

v2 cover CMDQ refinement of Mediatek DRM driver

These refinements include using standard mailbox callback interface, timeout detection, and a fixed cmdq_handle.

Changes in v2:
1. Define mtk_drm_cmdq_pkt_create() and mtk_drm_cmdq_pkt_destroy() when CONFIG_MTK_CMDQ is reachable.

Chun-Kuang Hu (4):
drm/mediatek: Use mailbox rx_callback instead of cmdq_task_cb drm/mediatek: Remove struct cmdq_client drm/mediatek: Detect CMDQ execution timeout drm/mediatek: Add cmdq_handle in mtk_crtc

 drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 110 ++++++++++++++++++++----
 1 file changed, 91 insertions(+), 19 deletions(-)

Signature · 1 line

2.25.1

Chun-Kuang Hu

author maintainer

2021-08-08 23:47 UTC #1 lore raw

PATCH v2 1/4 drm/mediatek: Use mailbox rx_callback instead of cmdq_task_cb

rx_callback is a standard mailbox callback mechanism and could cover the function of proprietary cmdq_task_cb, so use the standard one instead of the proprietary one.

Signed-off-by Chun-Kuang Hu <[email protected]>

---
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

drivers/gpu/drm/mediatek/mtk_drm_crtc.c +13 -3

index 474efb844249..cac8fe219c95 100644
@@ -4,6 +4,8 @@
  */
 
 #include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <linux/mailbox_controller.h>
 #include <linux/pm_runtime.h>
 #include <linux/soc/mediatek/mtk-cmdq.h>
 #include <linux/soc/mediatek/mtk-mmsys.h>

@@ -222,9 +224,11 @@ struct mtk_ddp_comp *mtk_drm_ddp_comp_for_plane(struct drm_crtc *crtc,
 }
 
 #if IS_REACHABLE(CONFIG_MTK_CMDQ)
-static void ddp_cmdq_cb(struct cmdq_cb_data data)
+static void ddp_cmdq_cb(struct mbox_client *cl, void *mssg)
 {
-	cmdq_pkt_destroy(data.data);
+	struct cmdq_cb_data *data = mssg;
+
+	cmdq_pkt_destroy(data->pkt);
 }
 #endif

@@ -475,7 +479,12 @@ static void mtk_drm_crtc_update_config(struct mtk_drm_crtc *mtk_crtc,
 		cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false);
 		mtk_crtc_ddp_config(crtc, cmdq_handle);
 		cmdq_pkt_finalize(cmdq_handle);
-		cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
+		dma_sync_single_for_device(mtk_crtc->cmdq_client->chan->mbox->dev,
+					    cmdq_handle->pa_base,
+					    cmdq_handle->cmd_buf_size,
+					    DMA_TO_DEVICE);
+		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;

@@ -842,6 +851,7 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
 	}
 
 	if (mtk_crtc->cmdq_client) {
+		mtk_crtc->cmdq_client->client.rx_callback = ddp_cmdq_cb;
 		ret = of_property_read_u32_index(priv->mutex_node,
 						 "mediatek,gce-events",
 						 drm_crtc_index(&mtk_crtc->base),

Signature · 1 line

2.25.1

Chun-Kuang Hu

author maintainer

2021-08-08 23:47 UTC #2 lore raw

PATCH v2 2/4 drm/mediatek: Remove struct cmdq_client

In reply to Chun-Kuang Hu

In mailbox rx_callback, it pass struct mbox_client to callback function, but it could not map back to mtk_drm_crtc instance because struct cmdq_client use a pointer to struct mbox_client:

struct cmdq_client {
	struct mbox_client client;
	struct mbox_chan *chan;
};

struct mtk_drm_crtc {
	/* client instance data */
	struct cmdq_client *cmdq_client;
};

so remove struct cmdq_client and let mtk_drm_crtc instance define mbox_client as:

struct mtk_drm_crtc {
	/* client instance data */
	struct mbox_client cl;
};

and in rx_callback function, use struct mbox_client to get struct mtk_drm_crtc.

Signed-off-by Chun-Kuang Hu <[email protected]>

---
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 80 +++++++++++++++++++------
 1 file changed, 62 insertions(+), 18 deletions(-)

drivers/gpu/drm/mediatek/mtk_drm_crtc.c +62 ~~-18~~

index cac8fe219c95..da92a3fedd0a 100644
@@ -52,7 +52,8 @@ struct mtk_drm_crtc {
 	bool				pending_async_planes;
 
 #if IS_REACHABLE(CONFIG_MTK_CMDQ)
-	struct cmdq_client		*cmdq_client;
+	struct mbox_client		cmdq_cl;
+	struct mbox_chan		*cmdq_chan;
 	u32				cmdq_event;
 #endif

@@ -224,11 +225,51 @@ struct mtk_ddp_comp *mtk_drm_ddp_comp_for_plane(struct drm_crtc *crtc,
 }
 
 #if IS_REACHABLE(CONFIG_MTK_CMDQ)
+static struct cmdq_pkt *mtk_drm_cmdq_pkt_create(struct mbox_chan *chan, size_t size)
+{
+	struct cmdq_pkt *pkt;
+	struct device *dev;
+	dma_addr_t dma_addr;
+
+	pkt = kzalloc(sizeof(*pkt), GFP_KERNEL);
+	if (!pkt)
+		return ERR_PTR(-ENOMEM);
+	pkt->va_base = kzalloc(size, GFP_KERNEL);
+	if (!pkt->va_base) {
+		kfree(pkt);
+		return ERR_PTR(-ENOMEM);
+	}
+	pkt->buf_size = size;
+
+	dev = 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 ERR_PTR(-ENOMEM);
+	}
+
+	pkt->pa_base = dma_addr;
+
+	return pkt;
+}
+
+static void mtk_drm_cmdq_pkt_destroy(struct mbox_chan *chan, struct cmdq_pkt *pkt)
+{
+	dma_unmap_single(chan->mbox->dev, pkt->pa_base, pkt->buf_size,
+			 DMA_TO_DEVICE);
+	kfree(pkt->va_base);
+	kfree(pkt);
+}
+
 static void ddp_cmdq_cb(struct mbox_client *cl, void *mssg)
 {
+	struct mtk_drm_crtc *mtk_crtc = container_of(cl, struct mtk_drm_crtc, cmdq_cl);
 	struct cmdq_cb_data *data = mssg;
 
-	cmdq_pkt_destroy(data->pkt);
+	mtk_drm_cmdq_pkt_destroy(mtk_crtc->cmdq_chan, data->pkt);
 }
 #endif

@@ -472,19 +513,19 @@ 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) {
-		mbox_flush(mtk_crtc->cmdq_client->chan, 2000);
-		cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE);
+	if (mtk_crtc->cmdq_chan) {
+		mbox_flush(mtk_crtc->cmdq_chan, 2000);
+		cmdq_handle = mtk_drm_cmdq_pkt_create(mtk_crtc->cmdq_chan, PAGE_SIZE);
 		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_chan->mbox->dev,
 					    cmdq_handle->pa_base,
 					    cmdq_handle->cmd_buf_size,
 					    DMA_TO_DEVICE);
-		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_chan, cmdq_handle);
+		mbox_client_txdone(mtk_crtc->cmdq_chan, 0);
 	}
 #endif
 	mtk_crtc->config_updating = false;

@@ -498,7 +539,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)
+	if (!priv->data->shadow_register && !mtk_crtc->cmdq_chan)
 #else
 	if (!priv->data->shadow_register)
 #endif

@@ -841,17 +882,20 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
 	mutex_init(&mtk_crtc->hw_lock);
 
 #if IS_REACHABLE(CONFIG_MTK_CMDQ)
-	mtk_crtc->cmdq_client =
-			cmdq_mbox_create(mtk_crtc->mmsys_dev,
-					 drm_crtc_index(&mtk_crtc->base));
-	if (IS_ERR(mtk_crtc->cmdq_client)) {
+	mtk_crtc->cmdq_cl.dev = mtk_crtc->mmsys_dev;
+	mtk_crtc->cmdq_cl.tx_block = false;
+	mtk_crtc->cmdq_cl.knows_txdone = true;
+	mtk_crtc->cmdq_cl.rx_callback = ddp_cmdq_cb;
+	mtk_crtc->cmdq_chan =
+			mbox_request_channel(&mtk_crtc->cmdq_cl,
+					      drm_crtc_index(&mtk_crtc->base));
+	if (IS_ERR(mtk_crtc->cmdq_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 = NULL;
+		mtk_crtc->cmdq_chan = NULL;
 	}
 
-	if (mtk_crtc->cmdq_client) {
-		mtk_crtc->cmdq_client->client.rx_callback = ddp_cmdq_cb;
+	if (mtk_crtc->cmdq_chan) {
 		ret = of_property_read_u32_index(priv->mutex_node,
 						 "mediatek,gce-events",
 						 drm_crtc_index(&mtk_crtc->base),

@@ -859,8 +903,8 @@ 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));
-			cmdq_mbox_destroy(mtk_crtc->cmdq_client);
-			mtk_crtc->cmdq_client = NULL;
+			mbox_free_channel(mtk_crtc->cmdq_chan);
+			mtk_crtc->cmdq_chan = NULL;
 		}
 	}
 #endif

Signature · 1 line

2.25.1

Chun-Kuang Hu

author maintainer

2021-08-08 23:47 UTC #3 lore raw

PATCH v2 3/4 drm/mediatek: Detect CMDQ execution timeout

In reply to Chun-Kuang Hu

CMDQ is used to update display register in vblank period, so it should be execute in next vblank. If it fail to execute in next 2 vblank, tiemout happen.

Signed-off-by Chun-Kuang Hu <[email protected]>

---
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

drivers/gpu/drm/mediatek/mtk_drm_crtc.c +12 -2

index da92a3fedd0a..ad4c1a3a9294 100644
@@ -55,6 +55,7 @@ struct mtk_drm_crtc {
 	struct mbox_client		cmdq_cl;
 	struct mbox_chan		*cmdq_chan;
 	u32				cmdq_event;
+	u32				cmdq_vblank_cnt;
 #endif
 
 	struct device			*mmsys_dev;

@@ -269,6 +270,7 @@ static void ddp_cmdq_cb(struct mbox_client *cl, void *mssg)
 	struct mtk_drm_crtc *mtk_crtc = container_of(cl, struct mtk_drm_crtc, cmdq_cl);
 	struct cmdq_cb_data *data = mssg;
 
+	mtk_crtc->cmdq_vblank_cnt = 0;
 	mtk_drm_cmdq_pkt_destroy(mtk_crtc->cmdq_chan, data->pkt);
 }
 #endif

@@ -524,6 +526,11 @@ static void mtk_drm_crtc_update_config(struct mtk_drm_crtc *mtk_crtc,
 					    cmdq_handle->pa_base,
 					    cmdq_handle->cmd_buf_size,
 					    DMA_TO_DEVICE);
+		/*
+		 * CMDQ command should execute in next vblank,
+		 * If it fail to execute in next 2 vblank, timeout happen.
+		 */
+		mtk_crtc->cmdq_vblank_cnt = 2;
 		mbox_send_message(mtk_crtc->cmdq_chan, cmdq_handle);
 		mbox_client_txdone(mtk_crtc->cmdq_chan, 0);
 	}

@@ -540,11 +547,14 @@ static void mtk_crtc_ddp_irq(void *data)
 
 #if IS_REACHABLE(CONFIG_MTK_CMDQ)
 	if (!priv->data->shadow_register && !mtk_crtc->cmdq_chan)
+		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",
+			  drm_crtc_index(&mtk_crtc->base));
 #else
 	if (!priv->data->shadow_register)
-#endif
 		mtk_crtc_ddp_config(crtc, NULL);
-
+#endif
 	mtk_drm_finish_page_flip(mtk_crtc);
 }

Signature · 1 line

2.25.1

Chun-Kuang Hu

author maintainer

2021-08-08 23:48 UTC #4 lore raw

PATCH v2 4/4 drm/mediatek: Add cmdq_handle in mtk_crtc

In reply to Chun-Kuang Hu

One mtk_crtc need just one cmdq_handle, so add one cmdq_handle in mtk_crtc to prevent frequently allocation and free of cmdq_handle.

Signed-off-by Chun-Kuang Hu <[email protected]>

---
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 28 ++++++++++++++++---------
 1 file changed, 18 insertions(+), 10 deletions(-)

drivers/gpu/drm/mediatek/mtk_drm_crtc.c +18 ~~-10~~

index ad4c1a3a9294..2d9becec04a9 100644
@@ -54,6 +54,7 @@ struct mtk_drm_crtc {
 #if IS_REACHABLE(CONFIG_MTK_CMDQ)
 	struct mbox_client		cmdq_cl;
 	struct mbox_chan		*cmdq_chan;
+	struct cmdq_pkt			cmdq_handle;
 	u32				cmdq_event;
 	u32				cmdq_vblank_cnt;
 #endif

@@ -226,19 +227,16 @@ struct mtk_ddp_comp *mtk_drm_ddp_comp_for_plane(struct drm_crtc *crtc,
 }
 
 #if IS_REACHABLE(CONFIG_MTK_CMDQ)
-static struct cmdq_pkt *mtk_drm_cmdq_pkt_create(struct mbox_chan *chan, size_t size)
+static int mtk_drm_cmdq_pkt_create(struct mbox_chan *chan, struct cmdq_pkt *pkt,
+				    size_t size)
 {
-	struct cmdq_pkt *pkt;
 	struct device *dev;
 	dma_addr_t dma_addr;
 
-	pkt = kzalloc(sizeof(*pkt), GFP_KERNEL);
-	if (!pkt)
-		return ERR_PTR(-ENOMEM);
 	pkt->va_base = kzalloc(size, GFP_KERNEL);
 	if (!pkt->va_base) {
 		kfree(pkt);
-		return ERR_PTR(-ENOMEM);
+		return -ENOMEM;
 	}
 	pkt->buf_size = size;

@@ -249,12 +247,12 @@ static struct cmdq_pkt *mtk_drm_cmdq_pkt_create(struct mbox_chan *chan, size_t s
 		dev_err(dev, "dma map failed, size=%u\n", (u32)(u64)size);
 		kfree(pkt->va_base);
 		kfree(pkt);
-		return ERR_PTR(-ENOMEM);
+		return -ENOMEM;
 	}
 
 	pkt->pa_base = dma_addr;
 
-	return pkt;
+	return 0;
 }
 
 static void mtk_drm_cmdq_pkt_destroy(struct mbox_chan *chan, struct cmdq_pkt *pkt)

@@ -477,7 +475,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;
+	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;

@@ -517,7 +515,7 @@ static void mtk_drm_crtc_update_config(struct mtk_drm_crtc *mtk_crtc,
 #if IS_REACHABLE(CONFIG_MTK_CMDQ)
 	if (mtk_crtc->cmdq_chan) {
 		mbox_flush(mtk_crtc->cmdq_chan, 2000);
-		cmdq_handle = mtk_drm_cmdq_pkt_create(mtk_crtc->cmdq_chan, PAGE_SIZE);
+		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);

@@ -915,6 +913,16 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
 				drm_crtc_index(&mtk_crtc->base));
 			mbox_free_channel(mtk_crtc->cmdq_chan);
 			mtk_crtc->cmdq_chan = NULL;
+		} else {
+			ret = mtk_drm_cmdq_pkt_create(mtk_crtc->cmdq_chan,
+						       &mtk_crtc->cmdq_handle,
+						       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_chan);
+				mtk_crtc->cmdq_chan = NULL;
+			}
 		}
 	}
 #endif

Signature · 1 line

2.25.1

Chun-Kuang Hu

author maintainer

2021-08-12 00:13 UTC #5 lore raw

In reply to Chun-Kuang Hu

Chun-Kuang Hu <[email protected]> 於 2021年8月9日週一上午7:47寫道：

Quoted 3 lines

These refinements include using standard mailbox callback interface, timeout detection, and a fixed cmdq_handle.

For this series, applied to mediatek-drm-next [1].

[1] https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next

Regards,
Chun-Kuang.

Quoted 17 lines

Changes in v2:
1. Define mtk_drm_cmdq_pkt_create() and mtk_drm_cmdq_pkt_destroy() when CONFIG_MTK_CMDQ is reachable.
Chun-Kuang Hu (4):
drm/mediatek: Use mailbox rx_callback instead of cmdq_task_cb drm/mediatek: Remove struct cmdq_client drm/mediatek: Detect CMDQ execution timeout drm/mediatek: Add cmdq_handle in mtk_crtc
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 110 ++++++++++++++++++++----
 1 file changed, 91 insertions(+), 19 deletions(-)
--
2.25.1

Enric Balletbo Serra

2021-09-21 08:33 UTC #6 lore raw

Hi Chun-Kuang,

Missatge de Chun-Kuang Hu <[email protected]> del dia dj., 12 d’ag. 2021 a les 2:13:

Quoted 11 lines

Chun-Kuang Hu <[email protected]> 於 2021年8月9日週一上午7:47寫道：
Quoted 3 lines
These refinements include using standard mailbox callback interface, timeout detection, and a fixed cmdq_handle.
For this series, applied to mediatek-drm-next [1].
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next

These patches seem to break the display on the Acer Chromebook R 13 (MT8173) in the current mainline. After running a bisection it pointed me to the following commit

commit f4be17cd5b14dd73545b0e014a63ebe9ab5ef837

Author: Chun-Kuang Hu <[email protected]>
Date:   Sun Jul 4 15:36:48 2021 +0800

drm/mediatek: Remove struct cmdq_client

Reverting this patch alone is not trivial, so I ended up reverting the full series, and I can confirm that reverting the full series makes the display work again.

Unfortunately, after the merge window, different things broke for this device, and I didn't finish isolating them, and it is not clear to me yet whether the logs I'm getting are useful for this specific issue or not. Basically with this series merged the kernel seems to be stuck, and the display is not working. Latest message is

[   12.329173] mtk-iommu 10205000.iommu: Partial TLB flush timed out,

falling back to full flush

Without the series, the kernel goes far and display works, however there are other issues affecting the cros-ec, but I think that's another issue.

I'll try to dig a bit more, but, meanwhile, if you have any idea please let me know.

Thanks,
Enric

Quoted 21 lines

Regards,
Chun-Kuang.
Quoted 17 lines
Changes in v2:
1. Define mtk_drm_cmdq_pkt_create() and mtk_drm_cmdq_pkt_destroy() when CONFIG_MTK_CMDQ is reachable.
Chun-Kuang Hu (4):
drm/mediatek: Use mailbox rx_callback instead of cmdq_task_cb drm/mediatek: Remove struct cmdq_client drm/mediatek: Detect CMDQ execution timeout drm/mediatek: Add cmdq_handle in mtk_crtc
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 110 ++++++++++++++++++++----
 1 file changed, 91 insertions(+), 19 deletions(-)
--
2.25.1

Enric Balletbo Serra

2021-09-21 08:36 UTC #7 lore raw

In reply to Chun-Kuang Hu

Hi Chun-Kuang,

(again without html format, sorry for the noise)

Missatge de Chun-Kuang Hu <[email protected]> del dia dj., 12 d’ag. 2021 a les 2:13:

Quoted 10 lines

Chun-Kuang Hu <[email protected]> 於 2021年8月9日週一上午7:47寫道：
Quoted 3 lines
These refinements include using standard mailbox callback interface, timeout detection, and a fixed cmdq_handle.
For this series, applied to mediatek-drm-next [1].
[1] https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next

These patches seem to break the display on the Acer Chromebook R 13 (MT8173) in the current mainline. After running a bisection it pointed me to the following commit

commit f4be17cd5b14dd73545b0e014a63ebe9ab5ef837

Author: Chun-Kuang Hu <[email protected]>
Date:   Sun Jul 4 15:36:48 2021 +0800

drm/mediatek: Remove struct cmdq_client

Reverting this patch alone is not trivial, so I ended up reverting the full series, and I can confirm that reverting the full series makes the display work again.

[   12.329173] mtk-iommu 10205000.iommu: Partial TLB flush timed out,

falling back to full flush

Without the series, the kernel goes far and display works, however there are other issues affecting the cros-ec, but I think that's another issue.

I'll try to dig a bit more, but, meanwhile, if you have any idea please let me know.

Thanks,
Enric

Quoted 20 lines

Regards,
Chun-Kuang.
Quoted 17 lines
Changes in v2:
1. Define mtk_drm_cmdq_pkt_create() and mtk_drm_cmdq_pkt_destroy() when CONFIG_MTK_CMDQ is reachable.
Chun-Kuang Hu (4):
drm/mediatek: Use mailbox rx_callback instead of cmdq_task_cb drm/mediatek: Remove struct cmdq_client drm/mediatek: Detect CMDQ execution timeout drm/mediatek: Add cmdq_handle in mtk_crtc
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 110 ++++++++++++++++++++----
 1 file changed, 91 insertions(+), 19 deletions(-)
--
2.25.1

Chun-Kuang Hu

author maintainer

2021-09-21 13:15 UTC #8 lore raw

Hi, Enric:

Enric Balletbo Serra <[email protected]> 於 2021年9月21日週二下午4:36寫道：

Quoted 23 lines

Hi Chun-Kuang,
(again without html format, sorry for the noise)
Missatge de Chun-Kuang Hu <[email protected]> del dia dj., 12 d’ag. 2021 a les 2:13:
Quoted 10 lines
Chun-Kuang Hu <[email protected]> 於 2021年8月9日週一上午7:47寫道：
Quoted 3 lines
These refinements include using standard mailbox callback interface, timeout detection, and a fixed cmdq_handle.
For this series, applied to mediatek-drm-next [1].
[1] https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next
These patches seem to break the display on the Acer Chromebook R 13 (MT8173) in the current mainline. After running a bisection it pointed me to the following commit
commit f4be17cd5b14dd73545b0e014a63ebe9ab5ef837

Quoted 8 lines

Author: Chun-Kuang Hu <[email protected]>
Date:   Sun Jul 4 15:36:48 2021 +0800
drm/mediatek: Remove struct cmdq_client
Reverting this patch alone is not trivial, so I ended up reverting the full series, and I can confirm that reverting the full series makes the display work again.

I think you could not just revert "drm/mediatek: Remove struct cmdq_client", you should also revert the patches after it, such as

"drm/mediatek: Clear pending flag when cmdq packet is done" "drm/mediatek: Add cmdq_handle in mtk_crtc" "drm/mediatek: Detect CMDQ execution timeout"

If "drm/mediatek: Remove struct cmdq_client" is the patch cause display abnormal, I think you could compare code w/ and w/o this patch. Focus on the value accuracy, such as cmdq_cl and cmdq_chan. And focus on the flow accuracy, such as mtk_drm_crtc_update_config() and ddp_cmdq_cb(). If this could not find the problem, I think the latest way is to break this patch into small patches, changes little in each small patches and we could finally find out the problem.

Regards,
Chun-Kuang.

Quoted 41 lines

Unfortunately, after the merge window, different things broke for this device, and I didn't finish isolating them, and it is not clear to me yet whether the logs I'm getting are useful for this specific issue or not. Basically with this series merged the kernel seems to be stuck, and the display is not working. Latest message is
[   12.329173] mtk-iommu 10205000.iommu: Partial TLB flush timed out,
falling back to full flush
Without the series, the kernel goes far and display works, however there are other issues affecting the cros-ec, but I think that's another issue.
I'll try to dig a bit more, but, meanwhile, if you have any idea please let me know.
Thanks,
Enric
Quoted 20 lines
Regards,
Chun-Kuang.
Quoted 17 lines
Changes in v2:
1. Define mtk_drm_cmdq_pkt_create() and mtk_drm_cmdq_pkt_destroy() when CONFIG_MTK_CMDQ is reachable.
Chun-Kuang Hu (4):
drm/mediatek: Use mailbox rx_callback instead of cmdq_task_cb drm/mediatek: Remove struct cmdq_client drm/mediatek: Detect CMDQ execution timeout drm/mediatek: Add cmdq_handle in mtk_crtc
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 110 ++++++++++++++++++++----
 1 file changed, 91 insertions(+), 19 deletions(-)
--
2.25.1

Enric Balletbo Serra

2021-09-23 16:36 UTC #9 lore raw

Hi Chun-Kuang,

Missatge de Chun-Kuang Hu <[email protected]> del dia dt., 21 de set. 2021 a les 15:15:

Quoted 36 lines

Hi, Enric:
Enric Balletbo Serra <[email protected]> 於 2021年9月21日週二下午4:36寫道：
Quoted 23 lines
Hi Chun-Kuang,
(again without html format, sorry for the noise)
Missatge de Chun-Kuang Hu <[email protected]> del dia dj., 12 d’ag. 2021 a les 2:13:
Quoted 10 lines
Chun-Kuang Hu <[email protected]> 於 2021年8月9日週一上午7:47寫道：
Quoted 3 lines
These refinements include using standard mailbox callback interface, timeout detection, and a fixed cmdq_handle.
For this series, applied to mediatek-drm-next [1].
[1] https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next
These patches seem to break the display on the Acer Chromebook R 13 (MT8173) in the current mainline. After running a bisection it pointed me to the following commit
commit f4be17cd5b14dd73545b0e014a63ebe9ab5ef837
Quoted 8 lines
Author: Chun-Kuang Hu <[email protected]>
Date:   Sun Jul 4 15:36:48 2021 +0800
drm/mediatek: Remove struct cmdq_client
Reverting this patch alone is not trivial, so I ended up reverting the full series, and I can confirm that reverting the full series makes the display work again.

Quoted 7 lines

I think you could not just revert "drm/mediatek: Remove struct cmdq_client", you should also revert the patches after it, such as
"drm/mediatek: Clear pending flag when cmdq packet is done" "drm/mediatek: Add cmdq_handle in mtk_crtc" "drm/mediatek: Detect CMDQ execution timeout"

Yes, in fact I reverted:

9efb16c2fdd6 drm/mediatek: Clear pending flag when cmdq packet is done bc9241be73d9 drm/mediatek: Add cmdq_handle in mtk_crtc 8cdcb3653424 drm/mediatek: Detect CMDQ execution timeout f4be17cd5b14 drm/mediatek: Remove struct cmdq_client c1ec54b7b5af drm/mediatek: Use mailbox rx_callback instead of cmdq_task_cb

Without these patches 5.15-rc2 works again on my platform.

The commit 'c1ec54b7b5af drm/mediatek: Use mailbox rx_callback instead of cmdq_task_cb' alone introduces lots of warnings in the kernel

WARNING: CPU: 0 PID: 0 at drivers/mailbox/mtk-cmdq-mailbox.c:198
cmdq_task_exec_done+0xb8/0xe0

I think is just a leftover or the mentioned warning, but that confused me a bit doing the bisection. Then, after commit 'f4be17cd5b14 drm/mediatek: Remove struct cmdq_client' my system simply gets stuck. For now I don't see any obvious mistake but will dig further.

Can I ask you in which platform did you test? And if you can double check if your platform is broken too in current mainline?

Thanks,
Enric

Quoted 52 lines

If "drm/mediatek: Remove struct cmdq_client" is the patch cause display abnormal, I think you could compare code w/ and w/o this patch. Focus on the value accuracy, such as cmdq_cl and cmdq_chan. And focus on the flow accuracy, such as mtk_drm_crtc_update_config() and ddp_cmdq_cb(). If this could not find the problem, I think the latest way is to break this patch into small patches, changes little in each small patches and we could finally find out the problem.
Regards,
Chun-Kuang.
Quoted 41 lines
Unfortunately, after the merge window, different things broke for this device, and I didn't finish isolating them, and it is not clear to me yet whether the logs I'm getting are useful for this specific issue or not. Basically with this series merged the kernel seems to be stuck, and the display is not working. Latest message is
[   12.329173] mtk-iommu 10205000.iommu: Partial TLB flush timed out,
falling back to full flush
Without the series, the kernel goes far and display works, however there are other issues affecting the cros-ec, but I think that's another issue.
I'll try to dig a bit more, but, meanwhile, if you have any idea please let me know.
Thanks,
Enric
Quoted 20 lines
Regards,
Chun-Kuang.
Quoted 17 lines
Changes in v2:
1. Define mtk_drm_cmdq_pkt_create() and mtk_drm_cmdq_pkt_destroy() when CONFIG_MTK_CMDQ is reachable.
Chun-Kuang Hu (4):
drm/mediatek: Use mailbox rx_callback instead of cmdq_task_cb drm/mediatek: Remove struct cmdq_client drm/mediatek: Detect CMDQ execution timeout drm/mediatek: Add cmdq_handle in mtk_crtc
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 110 ++++++++++++++++++++----
 1 file changed, 91 insertions(+), 19 deletions(-)
--
2.25.1

Chun-Kuang Hu

author maintainer

2021-09-23 23:43 UTC #10 lore raw

Hi, Enric:

Enric Balletbo Serra <[email protected]> 於 2021年9月24日週五上午12:36寫道：

Quoted 57 lines

Hi Chun-Kuang,
Missatge de Chun-Kuang Hu <[email protected]> del dia dt., 21 de set. 2021 a les 15:15:
Quoted 36 lines
Hi, Enric:
Enric Balletbo Serra <[email protected]> 於 2021年9月21日週二下午4:36寫道：
Quoted 23 lines
Hi Chun-Kuang,
(again without html format, sorry for the noise)
Missatge de Chun-Kuang Hu <[email protected]> del dia dj., 12 d’ag. 2021 a les 2:13:
Quoted 10 lines
Chun-Kuang Hu <[email protected]> 於 2021年8月9日週一上午7:47寫道：
Quoted 3 lines
These refinements include using standard mailbox callback interface, timeout detection, and a fixed cmdq_handle.
For this series, applied to mediatek-drm-next [1].
[1] https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next
These patches seem to break the display on the Acer Chromebook R 13 (MT8173) in the current mainline. After running a bisection it pointed me to the following commit
commit f4be17cd5b14dd73545b0e014a63ebe9ab5ef837
Quoted 8 lines
Author: Chun-Kuang Hu <[email protected]>
Date:   Sun Jul 4 15:36:48 2021 +0800
drm/mediatek: Remove struct cmdq_client
Reverting this patch alone is not trivial, so I ended up reverting the full series, and I can confirm that reverting the full series makes the display work again.
Quoted 7 lines
I think you could not just revert "drm/mediatek: Remove struct cmdq_client", you should also revert the patches after it, such as
"drm/mediatek: Clear pending flag when cmdq packet is done" "drm/mediatek: Add cmdq_handle in mtk_crtc" "drm/mediatek: Detect CMDQ execution timeout"
Yes, in fact I reverted:
9efb16c2fdd6 drm/mediatek: Clear pending flag when cmdq packet is done bc9241be73d9 drm/mediatek: Add cmdq_handle in mtk_crtc 8cdcb3653424 drm/mediatek: Detect CMDQ execution timeout f4be17cd5b14 drm/mediatek: Remove struct cmdq_client c1ec54b7b5af drm/mediatek: Use mailbox rx_callback instead of cmdq_task_cb

Quoted 7 lines

Without these patches 5.15-rc2 works again on my platform.
The commit 'c1ec54b7b5af drm/mediatek: Use mailbox rx_callback instead of cmdq_task_cb' alone introduces lots of warnings in the kernel
WARNING: CPU: 0 PID: 0 at drivers/mailbox/mtk-cmdq-mailbox.c:198
cmdq_task_exec_done+0xb8/0xe0

I think the WARN_ON in cmdq driver should be remove because that warning show that cmdq_task_cb is not used but I that is what I want.

Quoted 8 lines

I think is just a leftover or the mentioned warning, but that confused me a bit doing the bisection. Then, after commit 'f4be17cd5b14 drm/mediatek: Remove struct cmdq_client' my system simply gets stuck. For now I don't see any obvious mistake but will dig further.
Can I ask you in which platform did you test? And if you can double check if your platform is broken too in current mainline?

I've no environment to test code now. I apply this series because I assume Yongqiang has test his patch "Clear pending flag when cmdq packet is done". Before I setup the environment (this may take a long time), I would find others to fix this problem.
According to your information, "c1ec54b7b5af drm/mediatek: Use mailbox rx_callback instead of cmdq_task_cb" would cause many warning but display still work, right? If so, I think we should focus on "f4be17cd5b14 drm/mediatek: Remove struct cmdq_client".

Regards,
Chun-Kuang.

Quoted 56 lines

Thanks,
Enric
Quoted 52 lines
If "drm/mediatek: Remove struct cmdq_client" is the patch cause display abnormal, I think you could compare code w/ and w/o this patch. Focus on the value accuracy, such as cmdq_cl and cmdq_chan. And focus on the flow accuracy, such as mtk_drm_crtc_update_config() and ddp_cmdq_cb(). If this could not find the problem, I think the latest way is to break this patch into small patches, changes little in each small patches and we could finally find out the problem.
Regards,
Chun-Kuang.
Quoted 41 lines
Unfortunately, after the merge window, different things broke for this device, and I didn't finish isolating them, and it is not clear to me yet whether the logs I'm getting are useful for this specific issue or not. Basically with this series merged the kernel seems to be stuck, and the display is not working. Latest message is
[   12.329173] mtk-iommu 10205000.iommu: Partial TLB flush timed out,
falling back to full flush
Without the series, the kernel goes far and display works, however there are other issues affecting the cros-ec, but I think that's another issue.
I'll try to dig a bit more, but, meanwhile, if you have any idea please let me know.
Thanks,
Enric
Quoted 20 lines
Regards,
Chun-Kuang.
Quoted 17 lines
Changes in v2:
1. Define mtk_drm_cmdq_pkt_create() and mtk_drm_cmdq_pkt_destroy() when CONFIG_MTK_CMDQ is reachable.
Chun-Kuang Hu (4):
drm/mediatek: Use mailbox rx_callback instead of cmdq_task_cb drm/mediatek: Remove struct cmdq_client drm/mediatek: Detect CMDQ execution timeout drm/mediatek: Add cmdq_handle in mtk_crtc
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 110 ++++++++++++++++++++----
 1 file changed, 91 insertions(+), 19 deletions(-)
--
2.25.1

Chun-Kuang Hu

author maintainer

2021-09-29 14:25 UTC #11 lore raw

+Jason:

Hi, Enric:

Please test Jason's series [1], [2]. Does these series fixes your problem?

[1] https://patchwork.kernel.org/project/linux-mediatek/list/?series=554759
[2] https://patchwork.kernel.org/project/linux-mediatek/list/?series=554767

Regards,
Chun-Kuang.

Chun-Kuang Hu <[email protected]> 於 2021年9月24日週五上午7:43寫道：

Quoted 149 lines

Hi, Enric:
Enric Balletbo Serra <[email protected]> 於 2021年9月24日週五上午12:36寫道：
Quoted 57 lines
Hi Chun-Kuang,
Missatge de Chun-Kuang Hu <[email protected]> del dia dt., 21 de set. 2021 a les 15:15:
Quoted 36 lines
Hi, Enric:
Enric Balletbo Serra <[email protected]> 於 2021年9月21日週二下午4:36寫道：
Quoted 23 lines
Hi Chun-Kuang,
(again without html format, sorry for the noise)
Missatge de Chun-Kuang Hu <[email protected]> del dia dj., 12 d’ag. 2021 a les 2:13:
Quoted 10 lines
Chun-Kuang Hu <[email protected]> 於 2021年8月9日週一上午7:47寫道：
Quoted 3 lines
These refinements include using standard mailbox callback interface, timeout detection, and a fixed cmdq_handle.
For this series, applied to mediatek-drm-next [1].
[1] https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next
These patches seem to break the display on the Acer Chromebook R 13 (MT8173) in the current mainline. After running a bisection it pointed me to the following commit
commit f4be17cd5b14dd73545b0e014a63ebe9ab5ef837
Quoted 8 lines
Author: Chun-Kuang Hu <[email protected]>
Date:   Sun Jul 4 15:36:48 2021 +0800
drm/mediatek: Remove struct cmdq_client
Reverting this patch alone is not trivial, so I ended up reverting the full series, and I can confirm that reverting the full series makes the display work again.
Quoted 7 lines
I think you could not just revert "drm/mediatek: Remove struct cmdq_client", you should also revert the patches after it, such as
"drm/mediatek: Clear pending flag when cmdq packet is done" "drm/mediatek: Add cmdq_handle in mtk_crtc" "drm/mediatek: Detect CMDQ execution timeout"
Yes, in fact I reverted:
9efb16c2fdd6 drm/mediatek: Clear pending flag when cmdq packet is done bc9241be73d9 drm/mediatek: Add cmdq_handle in mtk_crtc 8cdcb3653424 drm/mediatek: Detect CMDQ execution timeout f4be17cd5b14 drm/mediatek: Remove struct cmdq_client c1ec54b7b5af drm/mediatek: Use mailbox rx_callback instead of cmdq_task_cb
Quoted 7 lines
Without these patches 5.15-rc2 works again on my platform.
The commit 'c1ec54b7b5af drm/mediatek: Use mailbox rx_callback instead of cmdq_task_cb' alone introduces lots of warnings in the kernel
WARNING: CPU: 0 PID: 0 at drivers/mailbox/mtk-cmdq-mailbox.c:198
cmdq_task_exec_done+0xb8/0xe0
I think the WARN_ON in cmdq driver should be remove because that warning show that cmdq_task_cb is not used but I that is what I want.
Quoted 8 lines
I think is just a leftover or the mentioned warning, but that confused me a bit doing the bisection. Then, after commit 'f4be17cd5b14 drm/mediatek: Remove struct cmdq_client' my system simply gets stuck. For now I don't see any obvious mistake but will dig further.
Can I ask you in which platform did you test? And if you can double check if your platform is broken too in current mainline?
I've no environment to test code now. I apply this series because I assume Yongqiang has test his patch "Clear pending flag when cmdq packet is done". Before I setup the environment (this may take a long time), I would find others to fix this problem.
According to your information, "c1ec54b7b5af drm/mediatek: Use mailbox rx_callback instead of cmdq_task_cb" would cause many warning but display still work, right? If so, I think we should focus on "f4be17cd5b14 drm/mediatek: Remove struct cmdq_client".
Regards,
Chun-Kuang.
Quoted 56 lines
Thanks,
Enric
Quoted 52 lines
If "drm/mediatek: Remove struct cmdq_client" is the patch cause display abnormal, I think you could compare code w/ and w/o this patch. Focus on the value accuracy, such as cmdq_cl and cmdq_chan. And focus on the flow accuracy, such as mtk_drm_crtc_update_config() and ddp_cmdq_cb(). If this could not find the problem, I think the latest way is to break this patch into small patches, changes little in each small patches and we could finally find out the problem.
Regards,
Chun-Kuang.
Quoted 41 lines
Unfortunately, after the merge window, different things broke for this device, and I didn't finish isolating them, and it is not clear to me yet whether the logs I'm getting are useful for this specific issue or not. Basically with this series merged the kernel seems to be stuck, and the display is not working. Latest message is
[   12.329173] mtk-iommu 10205000.iommu: Partial TLB flush timed out,
falling back to full flush
Without the series, the kernel goes far and display works, however there are other issues affecting the cros-ec, but I think that's another issue.
I'll try to dig a bit more, but, meanwhile, if you have any idea please let me know.
Thanks,
Enric
Quoted 20 lines
Regards,
Chun-Kuang.
Quoted 17 lines
Changes in v2:
1. Define mtk_drm_cmdq_pkt_create() and mtk_drm_cmdq_pkt_destroy() when CONFIG_MTK_CMDQ is reachable.
Chun-Kuang Hu (4):
drm/mediatek: Use mailbox rx_callback instead of cmdq_task_cb drm/mediatek: Remove struct cmdq_client drm/mediatek: Detect CMDQ execution timeout drm/mediatek: Add cmdq_handle in mtk_crtc
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 110 ++++++++++++++++++++----
 1 file changed, 91 insertions(+), 19 deletions(-)
--
2.25.1