Re: [PATCH 8/8] drm/panel: himax-hx83102: use wrapped MIPI DCS functions

2024-05-19 Thread Dmitry Baryshkov
On Fri, May 17, 2024 at 02:36:43PM -0700, Douglas Anderson wrote:
> Take advantage of some of the new wrapped routines introduced by
> commit f79d6d28d8fe ("drm/mipi-dsi: wrap more functions for streamline
> handling") to simplify the himax-hx83102 driver a bit more. This gets
> rid of some extra error prints (since the _multi functions all print
> errors for you) and simplifies the code a bit.
> 
> One thing here that isn't just refactoring is that in a few places we
> now check with errors with "if (err)" instead of "if (err < 0)". All
> errors are expected to be negative so this is not expected to have any
> impact. The _multi code internally considers anything non-zero to be
> an error so this just makes things consistent.
> 
> It can also be noted that hx83102_prepare() has a mix of things that
> can take advantage of _multi calls and things that can't. The cleanest
> seemed to be to use the multi_ctx still but consistently use the
> "accum_err" variable for error returns, though that's definitely a
> style decision with pros and cons.
> 
> Signed-off-by: Douglas Anderson 
> ---
> 
>  drivers/gpu/drm/panel/panel-himax-hx83102.c | 92 +++--
>  1 file changed, 28 insertions(+), 64 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov 

-- 
With best wishes
Dmitry


[PATCH 8/8] drm/panel: himax-hx83102: use wrapped MIPI DCS functions

2024-05-17 Thread Douglas Anderson
Take advantage of some of the new wrapped routines introduced by
commit f79d6d28d8fe ("drm/mipi-dsi: wrap more functions for streamline
handling") to simplify the himax-hx83102 driver a bit more. This gets
rid of some extra error prints (since the _multi functions all print
errors for you) and simplifies the code a bit.

One thing here that isn't just refactoring is that in a few places we
now check with errors with "if (err)" instead of "if (err < 0)". All
errors are expected to be negative so this is not expected to have any
impact. The _multi code internally considers anything non-zero to be
an error so this just makes things consistent.

It can also be noted that hx83102_prepare() has a mix of things that
can take advantage of _multi calls and things that can't. The cleanest
seemed to be to use the multi_ctx still but consistently use the
"accum_err" variable for error returns, though that's definitely a
style decision with pros and cons.

Signed-off-by: Douglas Anderson 
---

 drivers/gpu/drm/panel/panel-himax-hx83102.c | 92 +++--
 1 file changed, 28 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-himax-hx83102.c 
b/drivers/gpu/drm/panel/panel-himax-hx83102.c
index 1ba623e41924..6009a3fe1b8f 100644
--- a/drivers/gpu/drm/panel/panel-himax-hx83102.c
+++ b/drivers/gpu/drm/panel/panel-himax-hx83102.c
@@ -285,12 +285,10 @@ static int boe_nv110wum_init(struct hx83102 *ctx)
mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83102_SETSPCCMD, 0x3f);
mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83102_SETBANK, 0x00);
hx83102_enable_extended_cmds(&dsi_ctx, false);
-   if (dsi_ctx.accum_err)
-   return dsi_ctx.accum_err;
 
-   msleep(50);
+   mipi_dsi_msleep(dsi_ctx, 50);
 
-   return 0;
+   return dsi_ctx.accum_err;
 };
 
 static int ivo_t109nw41_init(struct hx83102 *ctx)
@@ -392,12 +390,10 @@ static int ivo_t109nw41_init(struct hx83102 *ctx)
mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83102_SETSPCCMD, 0x3f);
mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83102_SETBANK, 0x00);
hx83102_enable_extended_cmds(&dsi_ctx, false);
-   if (dsi_ctx.accum_err)
-   return dsi_ctx.accum_err;
 
-   msleep(60);
+   mipi_dsi_msleep(dsi_ctx, 60);
 
-   return 0;
+   return dsi_ctx.accum_err;
 };
 
 static const struct drm_display_mode starry_mode = {
@@ -472,40 +468,20 @@ static int hx83102_enable(struct drm_panel *panel)
return 0;
 }
 
-static int hx83102_panel_enter_sleep_mode(struct hx83102 *ctx)
-{
-   struct mipi_dsi_device *dsi = ctx->dsi;
-   int ret;
-
-   dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
-
-   ret = mipi_dsi_dcs_set_display_off(dsi);
-   if (ret < 0)
-   return ret;
-
-   ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
-   if (ret < 0)
-   return ret;
-
-   return 0;
-}
-
 static int hx83102_disable(struct drm_panel *panel)
 {
struct hx83102 *ctx = panel_to_hx83102(panel);
struct mipi_dsi_device *dsi = ctx->dsi;
-   struct device *dev = &dsi->dev;
-   int ret;
+   struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
 
-   ret = hx83102_panel_enter_sleep_mode(ctx);
-   if (ret < 0) {
-   dev_err(dev, "failed to set panel off: %d\n", ret);
-   return ret;
-   }
+   dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
 
-   msleep(150);
+   mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
+   mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
 
-   return 0;
+   mipi_dsi_msleep(&dsi_ctx, 150);
+
+   return dsi_ctx.accum_err;
 }
 
 static int hx83102_unprepare(struct drm_panel *panel)
@@ -526,32 +502,30 @@ static int hx83102_prepare(struct drm_panel *panel)
 {
struct hx83102 *ctx = panel_to_hx83102(panel);
struct mipi_dsi_device *dsi = ctx->dsi;
-   struct device *dev = &dsi->dev;
-   int ret;
+   struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
 
gpiod_set_value(ctx->enable_gpio, 0);
usleep_range(1000, 1500);
 
-   ret = regulator_enable(ctx->pp1800);
-   if (ret < 0)
-   return ret;
+   dsi_ctx.accum_err = regulator_enable(ctx->pp1800);
+   if (dsi_ctx.accum_err)
+   return dsi_ctx.accum_err;
 
usleep_range(3000, 5000);
 
-   ret = regulator_enable(ctx->avdd);
-   if (ret < 0)
+   dsi_ctx.accum_err = regulator_enable(ctx->avdd);
+   if (dsi_ctx.accum_err)
goto poweroff1v8;
-   ret = regulator_enable(ctx->avee);
-   if (ret < 0)
+   dsi_ctx.accum_err = regulator_enable(ctx->avee);
+   if (dsi_ctx.accum_err)
goto poweroffavdd;
 
usleep_range(1, 11000);
 
-   ret = mipi_dsi_dcs_nop(ctx->dsi);
-   if (ret < 0) {
-   dev_err(dev, "Failed to send NOP: %d\n", ret);
+   mipi_dsi_dcs_nop_multi(&dsi_ctx);
+   if (dsi_ctx.accum_err)
goto poweroff;
-