Re: [PATCH v2] drm/mediatek: allow commands to be sent during video mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Il 10/02/22 13:46, Julien STEPHAN ha scritto:
Mipi dsi panel drivers can use mipi_dsi_dcs_{set,get}_display_brightness()
to request backlight changes.

This can be done during panel initialization (dsi is in command mode)
or afterwards (dsi is in Video Mode).

When the DSI is in Video Mode, all commands are rejected.

Detect current DSI mode in mtk_dsi_host_transfer() and switch modes
temporarily to allow commands to be sent.

Signed-off-by: Julien STEPHAN <jstephan@xxxxxxxxxxxx>
Signed-off-by: Mattijs Korpershoek <mkorpershoek@xxxxxxxxxxxx>
Reviewed-by: Chun-Kuang Hu <chunkuang.hu@xxxxxxxxxx>

Hello Julien,
thanks for the patch!

However, there's a severe issue to solve.

---
Changes in v2:
   - update commit message to be more descriptive

  drivers/gpu/drm/mediatek/mtk_dsi.c | 34 ++++++++++++++++++++++--------
  1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 5d90d2eb0019..7d66fdc7f81d 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -891,24 +891,34 @@ static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host,
  	u8 read_data[16];
  	void *src_addr;
  	u8 irq_flag = CMD_DONE_INT_FLAG;
-
-	if (readl(dsi->regs + DSI_MODE_CTRL) & MODE) {
-		DRM_ERROR("dsi engine is not command mode\n");
-		return -EINVAL;
+	u32 dsi_mode;
+
+	dsi_mode = readl(dsi->regs + DSI_MODE_CTRL);
+	if (dsi_mode & MODE) {
+		mtk_dsi_stop(dsi);
+		if (mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500)) {
+			recv_cnt = -EINVAL;

Variable recv_cnt is u32, hence unsigned... You cannot assign a negative error
number to that variable.

While at it, please add a `int ret` variable to increase readability of this
function after your additions... in which case, this would then be

		ret = mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500);
		if (ret)
			goto restore_dsi_mode;

...which also simplifies the flow, in my opinion (but that's personal preference).

+			goto restore_dsi_mode;
+		}
  	}
if (MTK_DSI_HOST_IS_READ(msg->type))
  		irq_flag |= LPRX_RD_RDY_INT_FLAG;
- if (mtk_dsi_host_send_cmd(dsi, msg, irq_flag) < 0)
-		return -ETIME;
+	if (mtk_dsi_host_send_cmd(dsi, msg, irq_flag) < 0) {
+		recv_cnt = -ETIME;

This can be improved: mtk_dsi_host_send_cmd() already returns either zero or
-ETIME if mtk_dsi_wait_for_irq_done() times out.

I would also suggest, at this point, to make function mtk_dsi_wait_for_irq_done()
directly return -ETIME, so that also mtk_dsi_switch_to_cmd_mode() and
mtk_dsi_host_send_cmd() are simplified.

Whether you want to improve this file, or want to avoid improving it right now,
this should anyway be like:

	ret = mtk_dsi_host_send_cmd(..blah)
	if (ret)
		goto restore_dsi_mode;

+		goto restore_dsi_mode;
+	}
- if (!MTK_DSI_HOST_IS_READ(msg->type))
-		return 0;
+	if (!MTK_DSI_HOST_IS_READ(msg->type)) {
+		recv_cnt = 0;
+		goto restore_dsi_mode;
+	}
if (!msg->rx_buf) {
  		DRM_ERROR("dsi receive buffer size may be NULL\n");
-		return -EINVAL;
+		recv_cnt = -EINVAL;
+		goto restore_dsi_mode;
  	}
for (i = 0; i < 16; i++)
@@ -933,6 +943,12 @@ static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host,
  	DRM_INFO("dsi get %d byte data from the panel address(0x%x)\n",
  		 recv_cnt, *((u8 *)(msg->tx_buf)));
+restore_dsi_mode:
+	if (dsi_mode & MODE) {
+		mtk_dsi_set_mode(dsi);
+		mtk_dsi_start(dsi);
+	}
+
  	return recv_cnt;

P.S.:   return ret < 0 ? ret : recv_cnt;

  }

Thanks,
Angelo



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux