Re: [PATCH v5 25/29] drm/omap: dsi: split video mode enable/disable into separate func

2020-12-14 Thread Sebastian Reichel 
Hi,

On Tue, Dec 08, 2020 at 02:28:51PM +0200, Tomi Valkeinen wrote:
> Clean up the code by separating video-mode enable/disable code into
> functions of their own.
> 
> Signed-off-by: Tomi Valkeinen 
> Reviewed-by: Laurent Pinchart 
> ---

Reviewed-by: Sebastian Reichel 

-- Sebastian

>  drivers/gpu/drm/omapdrm/dss/dsi.c | 101 +-
>  1 file changed, 57 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c 
> b/drivers/gpu/drm/omapdrm/dss/dsi.c
> index c32884f167b8..71de6119d2de 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
> @@ -3207,12 +3207,61 @@ static int dsi_configure_pins(struct dsi_data *dsi,
>   return 0;
>  }
>  
> -static void dsi_enable_video_output(struct omap_dss_device *dssdev, int vc)
> +static int dsi_enable_video_mode(struct dsi_data *dsi, int vc)
>  {
> - struct dsi_data *dsi = to_dsi_data(dssdev);
>   int bpp = mipi_dsi_pixel_format_to_bpp(dsi->pix_fmt);
>   u8 data_type;
>   u16 word_count;
> +
> + switch (dsi->pix_fmt) {
> + case MIPI_DSI_FMT_RGB888:
> + data_type = MIPI_DSI_PACKED_PIXEL_STREAM_24;
> + break;
> + case MIPI_DSI_FMT_RGB666:
> + data_type = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
> + break;
> + case MIPI_DSI_FMT_RGB666_PACKED:
> + data_type = MIPI_DSI_PACKED_PIXEL_STREAM_18;
> + break;
> + case MIPI_DSI_FMT_RGB565:
> + data_type = MIPI_DSI_PACKED_PIXEL_STREAM_16;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + dsi_if_enable(dsi, false);
> + dsi_vc_enable(dsi, vc, false);
> +
> + /* MODE, 1 = video mode */
> + REG_FLD_MOD(dsi, DSI_VC_CTRL(vc), 1, 4, 4);
> +
> + word_count = DIV_ROUND_UP(dsi->vm.hactive * bpp, 8);
> +
> + dsi_vc_write_long_header(dsi, vc, dsi->dsidev->channel, data_type,
> + word_count, 0);
> +
> + dsi_vc_enable(dsi, vc, true);
> + dsi_if_enable(dsi, true);
> +
> + return 0;
> +}
> +
> +static void dsi_disable_video_mode(struct dsi_data *dsi, int vc)
> +{
> + dsi_if_enable(dsi, false);
> + dsi_vc_enable(dsi, vc, false);
> +
> + /* MODE, 0 = command mode */
> + REG_FLD_MOD(dsi, DSI_VC_CTRL(vc), 0, 4, 4);
> +
> + dsi_vc_enable(dsi, vc, true);
> + dsi_if_enable(dsi, true);
> +}
> +
> +static void dsi_enable_video_output(struct omap_dss_device *dssdev, int vc)
> +{
> + struct dsi_data *dsi = to_dsi_data(dssdev);
>   int r;
>  
>   r = dsi_init_dispc(dsi);
> @@ -3222,37 +3271,9 @@ static void dsi_enable_video_output(struct 
> omap_dss_device *dssdev, int vc)
>   }
>  
>   if (dsi->mode == OMAP_DSS_DSI_VIDEO_MODE) {
> - switch (dsi->pix_fmt) {
> - case MIPI_DSI_FMT_RGB888:
> - data_type = MIPI_DSI_PACKED_PIXEL_STREAM_24;
> - break;
> - case MIPI_DSI_FMT_RGB666:
> - data_type = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
> - break;
> - case MIPI_DSI_FMT_RGB666_PACKED:
> - data_type = MIPI_DSI_PACKED_PIXEL_STREAM_18;
> - break;
> - case MIPI_DSI_FMT_RGB565:
> - data_type = MIPI_DSI_PACKED_PIXEL_STREAM_16;
> - break;
> - default:
> - r = -EINVAL;
> - goto err_pix_fmt;
> - }
> -
> - dsi_if_enable(dsi, false);
> - dsi_vc_enable(dsi, vc, false);
> -
> - /* MODE, 1 = video mode */
> - REG_FLD_MOD(dsi, DSI_VC_CTRL(vc), 1, 4, 4);
> -
> - word_count = DIV_ROUND_UP(dsi->vm.hactive * bpp, 8);
> -
> - dsi_vc_write_long_header(dsi, vc, dsi->dsidev->channel, 
> data_type,
> - word_count, 0);
> -
> - dsi_vc_enable(dsi, vc, true);
> - dsi_if_enable(dsi, true);
> + r = dsi_enable_video_mode(dsi, vc);
> + if (r)
> + goto err_video_mode;
>   }
>  
>   r = dss_mgr_enable(>output);
> @@ -3266,7 +3287,7 @@ static void dsi_enable_video_output(struct 
> omap_dss_device *dssdev, int vc)
>   dsi_if_enable(dsi, false);
>   dsi_vc_enable(dsi, vc, false);
>   }
> -err_pix_fmt:
> +err_video_mode:
>   dsi_uninit_dispc(dsi);
>   dev_err(dsi->dev, "failed to enable DSI encoder!\n");
>   return;
> @@ -3276,16 +3297,8 @@ static void dsi_disable_video_output(struct 
> omap_dss_device *dssdev, int vc)
>  {
>   struct dsi_data *dsi = to_dsi_data(dssdev);
>  
> - if (dsi->mode == OMAP_DSS_DSI_VIDEO_MODE) {
> - dsi_if_enable(dsi, false);
> - dsi_vc_enable(dsi, vc, false);
> -
> - /* MODE, 0 = command mode */
> - REG_FLD_MOD(dsi, DSI_VC_CTRL(vc), 0, 4, 4);
> -
> - dsi_vc_enable(dsi, vc, true);
> -

[PATCH v5 25/29] drm/omap: dsi: split video mode enable/disable into separate func

2020-12-08 Thread Tomi Valkeinen 
Clean up the code by separating video-mode enable/disable code into
functions of their own.

Signed-off-by: Tomi Valkeinen 
Reviewed-by: Laurent Pinchart 
---
 drivers/gpu/drm/omapdrm/dss/dsi.c | 101 +-
 1 file changed, 57 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c 
b/drivers/gpu/drm/omapdrm/dss/dsi.c
index c32884f167b8..71de6119d2de 100644
--- a/drivers/gpu/drm/omapdrm/dss/dsi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
@@ -3207,12 +3207,61 @@ static int dsi_configure_pins(struct dsi_data *dsi,
return 0;
 }
 
-static void dsi_enable_video_output(struct omap_dss_device *dssdev, int vc)
+static int dsi_enable_video_mode(struct dsi_data *dsi, int vc)
 {
-   struct dsi_data *dsi = to_dsi_data(dssdev);
int bpp = mipi_dsi_pixel_format_to_bpp(dsi->pix_fmt);
u8 data_type;
u16 word_count;
+
+   switch (dsi->pix_fmt) {
+   case MIPI_DSI_FMT_RGB888:
+   data_type = MIPI_DSI_PACKED_PIXEL_STREAM_24;
+   break;
+   case MIPI_DSI_FMT_RGB666:
+   data_type = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
+   break;
+   case MIPI_DSI_FMT_RGB666_PACKED:
+   data_type = MIPI_DSI_PACKED_PIXEL_STREAM_18;
+   break;
+   case MIPI_DSI_FMT_RGB565:
+   data_type = MIPI_DSI_PACKED_PIXEL_STREAM_16;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   dsi_if_enable(dsi, false);
+   dsi_vc_enable(dsi, vc, false);
+
+   /* MODE, 1 = video mode */
+   REG_FLD_MOD(dsi, DSI_VC_CTRL(vc), 1, 4, 4);
+
+   word_count = DIV_ROUND_UP(dsi->vm.hactive * bpp, 8);
+
+   dsi_vc_write_long_header(dsi, vc, dsi->dsidev->channel, data_type,
+   word_count, 0);
+
+   dsi_vc_enable(dsi, vc, true);
+   dsi_if_enable(dsi, true);
+
+   return 0;
+}
+
+static void dsi_disable_video_mode(struct dsi_data *dsi, int vc)
+{
+   dsi_if_enable(dsi, false);
+   dsi_vc_enable(dsi, vc, false);
+
+   /* MODE, 0 = command mode */
+   REG_FLD_MOD(dsi, DSI_VC_CTRL(vc), 0, 4, 4);
+
+   dsi_vc_enable(dsi, vc, true);
+   dsi_if_enable(dsi, true);
+}
+
+static void dsi_enable_video_output(struct omap_dss_device *dssdev, int vc)
+{
+   struct dsi_data *dsi = to_dsi_data(dssdev);
int r;
 
r = dsi_init_dispc(dsi);
@@ -3222,37 +3271,9 @@ static void dsi_enable_video_output(struct 
omap_dss_device *dssdev, int vc)
}
 
if (dsi->mode == OMAP_DSS_DSI_VIDEO_MODE) {
-   switch (dsi->pix_fmt) {
-   case MIPI_DSI_FMT_RGB888:
-   data_type = MIPI_DSI_PACKED_PIXEL_STREAM_24;
-   break;
-   case MIPI_DSI_FMT_RGB666:
-   data_type = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
-   break;
-   case MIPI_DSI_FMT_RGB666_PACKED:
-   data_type = MIPI_DSI_PACKED_PIXEL_STREAM_18;
-   break;
-   case MIPI_DSI_FMT_RGB565:
-   data_type = MIPI_DSI_PACKED_PIXEL_STREAM_16;
-   break;
-   default:
-   r = -EINVAL;
-   goto err_pix_fmt;
-   }
-
-   dsi_if_enable(dsi, false);
-   dsi_vc_enable(dsi, vc, false);
-
-   /* MODE, 1 = video mode */
-   REG_FLD_MOD(dsi, DSI_VC_CTRL(vc), 1, 4, 4);
-
-   word_count = DIV_ROUND_UP(dsi->vm.hactive * bpp, 8);
-
-   dsi_vc_write_long_header(dsi, vc, dsi->dsidev->channel, 
data_type,
-   word_count, 0);
-
-   dsi_vc_enable(dsi, vc, true);
-   dsi_if_enable(dsi, true);
+   r = dsi_enable_video_mode(dsi, vc);
+   if (r)
+   goto err_video_mode;
}
 
r = dss_mgr_enable(>output);
@@ -3266,7 +3287,7 @@ static void dsi_enable_video_output(struct 
omap_dss_device *dssdev, int vc)
dsi_if_enable(dsi, false);
dsi_vc_enable(dsi, vc, false);
}
-err_pix_fmt:
+err_video_mode:
dsi_uninit_dispc(dsi);
dev_err(dsi->dev, "failed to enable DSI encoder!\n");
return;
@@ -3276,16 +3297,8 @@ static void dsi_disable_video_output(struct 
omap_dss_device *dssdev, int vc)
 {
struct dsi_data *dsi = to_dsi_data(dssdev);
 
-   if (dsi->mode == OMAP_DSS_DSI_VIDEO_MODE) {
-   dsi_if_enable(dsi, false);
-   dsi_vc_enable(dsi, vc, false);
-
-   /* MODE, 0 = command mode */
-   REG_FLD_MOD(dsi, DSI_VC_CTRL(vc), 0, 4, 4);
-
-   dsi_vc_enable(dsi, vc, true);
-   dsi_if_enable(dsi, true);
-   }
+   if (dsi->mode == OMAP_DSS_DSI_VIDEO_MODE)
+   dsi_disable_video_mode(dsi, vc);
 
dss_mgr_disable(>output);
 
-- 
Texas Instruments Finland Oy, Porkkalankatu 22,