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

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

[Julien STEPHAN]

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@baylibre.com>
Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
---
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;
+			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;
+		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;
 }
 
-- 
2.35.1

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

[Chun-Kuang Hu]

Hi, Julien:

Julien STEPHAN <jstephan@baylibre.com> 於 2022年2月10日 週四 下午8:47寫道：
>
> 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.

Reviewed-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>

>
> Signed-off-by: Julien STEPHAN <jstephan@baylibre.com>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> ---
> 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;
> +                       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;
> +               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;
>  }
>
> --
> 2.35.1
>

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

[AngeloGioacchino Del Regno]

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@baylibre.com>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> Reviewed-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>

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