diff mbox series

[1/2] drm/panel: visionox-vtdr6130: switch to mipi_dsi wrapped functions

Message ID 20240828-topic-sm8x50-upstream-vtdr6130-multi-v1-1-0cae20d4c55d@linaro.org (mailing list archive)
State New, archived
Headers show
Series drm/panel: annual cleanup of visionox vtdr6130 driver | expand

Commit Message

Neil Armstrong Aug. 28, 2024, 4:03 p.m. UTC 
Make usage of the new _multi() mipi_dsi functions instead of the
deprecated macros, improving error handling and printing.

bloat-o-meter gives a 12% gain on arm64:
Function                                     old     new   delta
visionox_vtdr6130_unprepare                  208     204      -4
visionox_vtdr6130_prepare                   1192     896    -296
Total: Before=2348, After=2048, chg -12.78%

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/gpu/drm/panel/panel-visionox-vtdr6130.c | 186 +++++++++++-------------
 1 file changed, 82 insertions(+), 104 deletions(-)

Comments

Douglas Anderson Aug. 28, 2024, 4:32 p.m. UTC | #1 
Hi,

On Wed, Aug 28, 2024 at 9:03 AM Neil Armstrong
<neil.armstrong@linaro.org> wrote:
>
> Make usage of the new _multi() mipi_dsi functions instead of the
> deprecated macros, improving error handling and printing.
>
> bloat-o-meter gives a 12% gain on arm64:
> Function                                     old     new   delta
> visionox_vtdr6130_unprepare                  208     204      -4
> visionox_vtdr6130_prepare                   1192     896    -296
> Total: Before=2348, After=2048, chg -12.78%
>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  drivers/gpu/drm/panel/panel-visionox-vtdr6130.c | 186 +++++++++++-------------
>  1 file changed, 82 insertions(+), 104 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
> index 540099253e1b..ebe92871dbb6 100644
> --- a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
> +++ b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
> @@ -40,120 +40,103 @@ static void visionox_vtdr6130_reset(struct visionox_vtdr6130 *ctx)
>  static int visionox_vtdr6130_on(struct visionox_vtdr6130 *ctx)
>  {
>         struct mipi_dsi_device *dsi = ctx->dsi;
> -       struct device *dev = &dsi->dev;
> -       int ret;
> +       struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>
>         dsi->mode_flags |= MIPI_DSI_MODE_LPM;

This isn't something you introduced in your patch, but I wonder if we
should avoid setting the "MIPI_DSI_MODE_LPM" bit when the function
returns an error?

In any case:

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Neil Armstrong Aug. 29, 2024, 11 a.m. UTC | #2 
On 28/08/2024 18:32, Doug Anderson wrote:
> Hi,
> 
> On Wed, Aug 28, 2024 at 9:03 AM Neil Armstrong
> <neil.armstrong@linaro.org> wrote:
>>
>> Make usage of the new _multi() mipi_dsi functions instead of the
>> deprecated macros, improving error handling and printing.
>>
>> bloat-o-meter gives a 12% gain on arm64:
>> Function                                     old     new   delta
>> visionox_vtdr6130_unprepare                  208     204      -4
>> visionox_vtdr6130_prepare                   1192     896    -296
>> Total: Before=2348, After=2048, chg -12.78%
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>>   drivers/gpu/drm/panel/panel-visionox-vtdr6130.c | 186 +++++++++++-------------
>>   1 file changed, 82 insertions(+), 104 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
>> index 540099253e1b..ebe92871dbb6 100644
>> --- a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
>> +++ b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
>> @@ -40,120 +40,103 @@ static void visionox_vtdr6130_reset(struct visionox_vtdr6130 *ctx)
>>   static int visionox_vtdr6130_on(struct visionox_vtdr6130 *ctx)
>>   {
>>          struct mipi_dsi_device *dsi = ctx->dsi;
>> -       struct device *dev = &dsi->dev;
>> -       int ret;
>> +       struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>>
>>          dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> 
> This isn't something you introduced in your patch, but I wonder if we
> should avoid setting the "MIPI_DSI_MODE_LPM" bit when the function
> returns an error?

Yeah it's unrelated to this change, but I'll investigate.

> 
> In any case:
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

Thanks,
Neil
diff mbox series

Patch

diff --git a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
index 540099253e1b..ebe92871dbb6 100644
--- a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
+++ b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
@@ -40,120 +40,103 @@  static void visionox_vtdr6130_reset(struct visionox_vtdr6130 *ctx)
 static int visionox_vtdr6130_on(struct visionox_vtdr6130 *ctx)
 {
 	struct mipi_dsi_device *dsi = ctx->dsi;
-	struct device *dev = &dsi->dev;
-	int ret;
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
 
 	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
 
-	ret = mipi_dsi_dcs_set_tear_on(dsi, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
-	if (ret)
-		return ret;
-
-	mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_WRITE_CONTROL_DISPLAY, 0x20);
-	mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_SET_DISPLAY_BRIGHTNESS, 0x00, 0x00);
-	mipi_dsi_dcs_write_seq(dsi, 0x59, 0x09);
-	mipi_dsi_dcs_write_seq(dsi, 0x6c, 0x01);
-	mipi_dsi_dcs_write_seq(dsi, 0x6d, 0x00);
-	mipi_dsi_dcs_write_seq(dsi, 0x6f, 0x01);
-	mipi_dsi_dcs_write_seq(dsi, 0x70,
-			       0x12, 0x00, 0x00, 0xab, 0x30, 0x80, 0x09, 0x60, 0x04,
-			       0x38, 0x00, 0x28, 0x02, 0x1c, 0x02, 0x1c, 0x02, 0x00,
-			       0x02, 0x0e, 0x00, 0x20, 0x03, 0xdd, 0x00, 0x07, 0x00,
-			       0x0c, 0x02, 0x77, 0x02, 0x8b, 0x18, 0x00, 0x10, 0xf0,
-			       0x07, 0x10, 0x20, 0x00, 0x06, 0x0f, 0x0f, 0x33, 0x0e,
-			       0x1c, 0x2a, 0x38, 0x46, 0x54, 0x62, 0x69, 0x70, 0x77,
-			       0x79, 0x7b, 0x7d, 0x7e, 0x02, 0x02, 0x22, 0x00, 0x2a,
-			       0x40, 0x2a, 0xbe, 0x3a, 0xfc, 0x3a, 0xfa, 0x3a, 0xf8,
-			       0x3b, 0x38, 0x3b, 0x78, 0x3b, 0xb6, 0x4b, 0xb6, 0x4b,
-			       0xf4, 0x4b, 0xf4, 0x6c, 0x34, 0x84, 0x74, 0x00, 0x00,
-			       0x00, 0x00, 0x00, 0x00);
-	mipi_dsi_dcs_write_seq(dsi, 0xf0, 0xaa, 0x10);
-	mipi_dsi_dcs_write_seq(dsi, 0xb1,
-			       0x01, 0x38, 0x00, 0x14, 0x00, 0x1c, 0x00, 0x01, 0x66,
-			       0x00, 0x14, 0x00, 0x14, 0x00, 0x01, 0x66, 0x00, 0x14,
-			       0x05, 0xcc, 0x00);
-	mipi_dsi_dcs_write_seq(dsi, 0xf0, 0xaa, 0x13);
-	mipi_dsi_dcs_write_seq(dsi, 0xce,
-			       0x09, 0x11, 0x09, 0x11, 0x08, 0xc1, 0x07, 0xfa, 0x05,
-			       0xa4, 0x00, 0x3c, 0x00, 0x34, 0x00, 0x24, 0x00, 0x0c,
-			       0x00, 0x0c, 0x04, 0x00, 0x35);
-	mipi_dsi_dcs_write_seq(dsi, 0xf0, 0xaa, 0x14);
-	mipi_dsi_dcs_write_seq(dsi, 0xb2, 0x03, 0x33);
-	mipi_dsi_dcs_write_seq(dsi, 0xb4,
-			       0x00, 0x33, 0x00, 0x00, 0x00, 0x3e, 0x00, 0x00, 0x00,
-			       0x3e, 0x00, 0x00);
-	mipi_dsi_dcs_write_seq(dsi, 0xb5,
-			       0x00, 0x09, 0x09, 0x09, 0x09, 0x09, 0x09, 0x06, 0x01);
-	mipi_dsi_dcs_write_seq(dsi, 0xb9, 0x00, 0x00, 0x08, 0x09, 0x09, 0x09);
-	mipi_dsi_dcs_write_seq(dsi, 0xbc,
-			       0x10, 0x00, 0x00, 0x06, 0x11, 0x09, 0x3b, 0x09, 0x47,
-			       0x09, 0x47, 0x00);
-	mipi_dsi_dcs_write_seq(dsi, 0xbe,
-			       0x10, 0x10, 0x00, 0x08, 0x22, 0x09, 0x19, 0x09, 0x25,
-			       0x09, 0x25, 0x00);
-	mipi_dsi_dcs_write_seq(dsi, 0xff, 0x5a, 0x80);
-	mipi_dsi_dcs_write_seq(dsi, 0x65, 0x14);
-	mipi_dsi_dcs_write_seq(dsi, 0xfa, 0x08, 0x08, 0x08);
-	mipi_dsi_dcs_write_seq(dsi, 0xff, 0x5a, 0x81);
-	mipi_dsi_dcs_write_seq(dsi, 0x65, 0x05);
-	mipi_dsi_dcs_write_seq(dsi, 0xf3, 0x0f);
-	mipi_dsi_dcs_write_seq(dsi, 0xf0, 0xaa, 0x00);
-	mipi_dsi_dcs_write_seq(dsi, 0xff, 0x5a, 0x82);
-	mipi_dsi_dcs_write_seq(dsi, 0xf9, 0x00);
-	mipi_dsi_dcs_write_seq(dsi, 0xff, 0x51, 0x83);
-	mipi_dsi_dcs_write_seq(dsi, 0x65, 0x04);
-	mipi_dsi_dcs_write_seq(dsi, 0xf8, 0x00);
-	mipi_dsi_dcs_write_seq(dsi, 0xff, 0x5a, 0x00);
-	mipi_dsi_dcs_write_seq(dsi, 0x65, 0x01);
-	mipi_dsi_dcs_write_seq(dsi, 0xf4, 0x9a);
-	mipi_dsi_dcs_write_seq(dsi, 0xff, 0x5a, 0x00);
-
-	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
-	if (ret < 0) {
-		dev_err(dev, "Failed to exit sleep mode: %d\n", ret);
-		return ret;
-	}
-	msleep(120);
-
-	ret = mipi_dsi_dcs_set_display_on(dsi);
-	if (ret < 0) {
-		dev_err(dev, "Failed to set display on: %d\n", ret);
-		return ret;
-	}
-	msleep(20);
-
-	return 0;
+	mipi_dsi_dcs_set_tear_on_multi(&dsi_ctx, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
+
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx,
+				     MIPI_DCS_WRITE_CONTROL_DISPLAY, 0x20);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx,
+				     MIPI_DCS_SET_DISPLAY_BRIGHTNESS, 0x00,
+				     0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x59, 0x09);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6c, 0x01);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6d, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x01);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x70, 0x12, 0x00, 0x00, 0xab,
+				     0x30, 0x80, 0x09, 0x60, 0x04, 0x38, 0x00,
+				     0x28, 0x02, 0x1c, 0x02, 0x1c, 0x02, 0x00,
+				     0x02, 0x0e, 0x00, 0x20, 0x03, 0xdd, 0x00,
+				     0x07, 0x00, 0x0c, 0x02, 0x77, 0x02, 0x8b,
+				     0x18, 0x00, 0x10, 0xf0, 0x07, 0x10, 0x20,
+				     0x00, 0x06, 0x0f, 0x0f, 0x33, 0x0e, 0x1c,
+				     0x2a, 0x38, 0x46, 0x54, 0x62, 0x69, 0x70,
+				     0x77, 0x79, 0x7b, 0x7d, 0x7e, 0x02, 0x02,
+				     0x22, 0x00, 0x2a, 0x40, 0x2a, 0xbe, 0x3a,
+				     0xfc, 0x3a, 0xfa, 0x3a, 0xf8, 0x3b, 0x38,
+				     0x3b, 0x78, 0x3b, 0xb6, 0x4b, 0xb6, 0x4b,
+				     0xf4, 0x4b, 0xf4, 0x6c, 0x34, 0x84, 0x74,
+				     0x00, 0x00, 0x00, 0x00, 0x00, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0xaa, 0x10);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xb1, 0x01, 0x38, 0x00, 0x14,
+				     0x00, 0x1c, 0x00, 0x01, 0x66, 0x00, 0x14,
+				     0x00, 0x14, 0x00, 0x01, 0x66, 0x00, 0x14,
+				     0x05, 0xcc, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0xaa, 0x13);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xce, 0x09, 0x11, 0x09, 0x11,
+				     0x08, 0xc1, 0x07, 0xfa, 0x05, 0xa4, 0x00,
+				     0x3c, 0x00, 0x34, 0x00, 0x24, 0x00, 0x0c,
+				     0x00, 0x0c, 0x04, 0x00, 0x35);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0xaa, 0x14);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xb2, 0x03, 0x33);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xb4, 0x00, 0x33, 0x00, 0x00,
+				     0x00, 0x3e, 0x00, 0x00, 0x00, 0x3e, 0x00,
+				     0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xb5, 0x00, 0x09, 0x09, 0x09,
+				     0x09, 0x09, 0x09, 0x06, 0x01);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xb9, 0x00, 0x00, 0x08, 0x09,
+				     0x09, 0x09);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xbc, 0x10, 0x00, 0x00, 0x06,
+				     0x11, 0x09, 0x3b, 0x09, 0x47, 0x09, 0x47,
+				     0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xbe, 0x10, 0x10, 0x00, 0x08,
+				     0x22, 0x09, 0x19, 0x09, 0x25, 0x09, 0x25,
+				     0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xff, 0x5a, 0x80);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x65, 0x14);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfa, 0x08, 0x08, 0x08);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xff, 0x5a, 0x81);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x65, 0x05);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf3, 0x0f);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0xaa, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xff, 0x5a, 0x82);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf9, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xff, 0x51, 0x83);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x65, 0x04);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf8, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xff, 0x5a, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x65, 0x01);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf4, 0x9a);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xff, 0x5a, 0x00);
+
+	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+	mipi_dsi_msleep(&dsi_ctx, 120);
+
+	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
+	mipi_dsi_msleep(&dsi_ctx, 20);
+
+	return dsi_ctx.accum_err;
 }
 
-static int visionox_vtdr6130_off(struct visionox_vtdr6130 *ctx)
+static void visionox_vtdr6130_off(struct visionox_vtdr6130 *ctx)
 {
 	struct mipi_dsi_device *dsi = ctx->dsi;
-	struct device *dev = &dsi->dev;
-	int ret;
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
 
 	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
 
-	ret = mipi_dsi_dcs_set_display_off(dsi);
-	if (ret < 0) {
-		dev_err(dev, "Failed to set display off: %d\n", ret);
-		return ret;
-	}
-	msleep(20);
-
-	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
-	if (ret < 0) {
-		dev_err(dev, "Failed to enter sleep mode: %d\n", ret);
-		return ret;
-	}
-	msleep(120);
+	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
+	mipi_dsi_msleep(&dsi_ctx, 20);
 
-	return 0;
+	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
+	mipi_dsi_msleep(&dsi_ctx, 120);
 }
 
 static int visionox_vtdr6130_prepare(struct drm_panel *panel)
 {
 	struct visionox_vtdr6130 *ctx = to_visionox_vtdr6130(panel);
-	struct device *dev = &ctx->dsi->dev;
 	int ret;
 
 	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies),
@@ -165,7 +148,6 @@  static int visionox_vtdr6130_prepare(struct drm_panel *panel)
 
 	ret = visionox_vtdr6130_on(ctx);
 	if (ret < 0) {
-		dev_err(dev, "Failed to initialize panel: %d\n", ret);
 		gpiod_set_value_cansleep(ctx->reset_gpio, 1);
 		regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
 		return ret;
@@ -177,12 +159,8 @@  static int visionox_vtdr6130_prepare(struct drm_panel *panel)
 static int visionox_vtdr6130_unprepare(struct drm_panel *panel)
 {
 	struct visionox_vtdr6130 *ctx = to_visionox_vtdr6130(panel);
-	struct device *dev = &ctx->dsi->dev;
-	int ret;
 
-	ret = visionox_vtdr6130_off(ctx);
-	if (ret < 0)
-		dev_err(dev, "Failed to un-initialize panel: %d\n", ret);
+	visionox_vtdr6130_off(ctx);
 
 	gpiod_set_value_cansleep(ctx->reset_gpio, 1);