.../gpu/drm/panel/panel-xinpeng-xpp055c272.c | 185 ++++++++---------- 1 file changed, 84 insertions(+), 101 deletions(-)
Changes the xinpeng-xpp055c272 panel to use multi style functions for
improved error handling.
Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
---
.../gpu/drm/panel/panel-xinpeng-xpp055c272.c | 185 ++++++++----------
1 file changed, 84 insertions(+), 101 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c b/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
index 22a14006765e..45a405669593 100644
--- a/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
+++ b/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
@@ -59,90 +59,82 @@ static inline struct xpp055c272 *panel_to_xpp055c272(struct drm_panel *panel)
return container_of(panel, struct xpp055c272, panel);
}
-static int xpp055c272_init_sequence(struct xpp055c272 *ctx)
+static void xpp055c272_init_sequence(struct mipi_dsi_multi_context *dsi_ctx)
{
- struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
- struct device *dev = ctx->dev;
-
/*
* Init sequence was supplied by the panel vendor without much
* documentation.
*/
- mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETEXTC, 0xf1, 0x12, 0x83);
- mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETMIPI,
- 0x33, 0x81, 0x05, 0xf9, 0x0e, 0x0e, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x44, 0x25,
- 0x00, 0x91, 0x0a, 0x00, 0x00, 0x02, 0x4f, 0x01,
- 0x00, 0x00, 0x37);
- mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETPOWER_EXT, 0x25);
- mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETPCR, 0x02, 0x11, 0x00);
- mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETRGBIF,
- 0x0c, 0x10, 0x0a, 0x50, 0x03, 0xff, 0x00, 0x00,
- 0x00, 0x00);
- mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETSCR,
- 0x73, 0x73, 0x50, 0x50, 0x00, 0x00, 0x08, 0x70,
- 0x00);
- mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETVDC, 0x46);
- mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETPANEL, 0x0b);
- mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETCYC, 0x80);
- mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETDISP, 0xc8, 0x12, 0x30);
- mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETEQ,
- 0x07, 0x07, 0x0B, 0x0B, 0x03, 0x0B, 0x00, 0x00,
- 0x00, 0x00, 0xFF, 0x00, 0xC0, 0x10);
- mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETPOWER,
- 0x53, 0x00, 0x1e, 0x1e, 0x77, 0xe1, 0xcc, 0xdd,
- 0x67, 0x77, 0x33, 0x33);
- mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETECO, 0x00, 0x00, 0xff,
- 0xff, 0x01, 0xff);
- mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETBGP, 0x09, 0x09);
- msleep(20);
-
- mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETVCOM, 0x87, 0x95);
- mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETGIP1,
- 0xc2, 0x10, 0x05, 0x05, 0x10, 0x05, 0xa0, 0x12,
- 0x31, 0x23, 0x3f, 0x81, 0x0a, 0xa0, 0x37, 0x18,
- 0x00, 0x80, 0x01, 0x00, 0x00, 0x00, 0x00, 0x80,
- 0x01, 0x00, 0x00, 0x00, 0x48, 0xf8, 0x86, 0x42,
- 0x08, 0x88, 0x88, 0x80, 0x88, 0x88, 0x88, 0x58,
- 0xf8, 0x87, 0x53, 0x18, 0x88, 0x88, 0x81, 0x88,
- 0x88, 0x88, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00);
- mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETGIP2,
- 0x00, 0x1a, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x1f, 0x88, 0x81, 0x35,
- 0x78, 0x88, 0x88, 0x85, 0x88, 0x88, 0x88, 0x0f,
- 0x88, 0x80, 0x24, 0x68, 0x88, 0x88, 0x84, 0x88,
- 0x88, 0x88, 0x23, 0x10, 0x00, 0x00, 0x1c, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x30, 0x05,
- 0xa0, 0x00, 0x00, 0x00, 0x00);
- mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETGAMMA,
- 0x00, 0x06, 0x08, 0x2a, 0x31, 0x3f, 0x38, 0x36,
- 0x07, 0x0c, 0x0d, 0x11, 0x13, 0x12, 0x13, 0x11,
- 0x18, 0x00, 0x06, 0x08, 0x2a, 0x31, 0x3f, 0x38,
- 0x36, 0x07, 0x0c, 0x0d, 0x11, 0x13, 0x12, 0x13,
- 0x11, 0x18);
-
- msleep(60);
-
- dev_dbg(dev, "Panel init sequence done\n");
- return 0;
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETEXTC, 0xf1, 0x12, 0x83);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETMIPI,
+ 0x33, 0x81, 0x05, 0xf9, 0x0e, 0x0e, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x44, 0x25,
+ 0x00, 0x91, 0x0a, 0x00, 0x00, 0x02, 0x4f, 0x01,
+ 0x00, 0x00, 0x37);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETPOWER_EXT, 0x25);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETPCR, 0x02, 0x11, 0x00);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETRGBIF,
+ 0x0c, 0x10, 0x0a, 0x50, 0x03, 0xff, 0x00, 0x00,
+ 0x00, 0x00);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETSCR,
+ 0x73, 0x73, 0x50, 0x50, 0x00, 0x00, 0x08, 0x70,
+ 0x00);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETVDC, 0x46);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETPANEL, 0x0b);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETCYC, 0x80);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETDISP, 0xc8, 0x12, 0x30);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETEQ,
+ 0x07, 0x07, 0x0b, 0x0b, 0x03, 0x0b, 0x00, 0x00,
+ 0x00, 0x00, 0xff, 0x00, 0xC0, 0x10);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETPOWER,
+ 0x53, 0x00, 0x1e, 0x1e, 0x77, 0xe1, 0xcc, 0xdd,
+ 0x67, 0x77, 0x33, 0x33);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETECO, 0x00, 0x00, 0xff,
+ 0xff, 0x01, 0xff);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETBGP, 0x09, 0x09);
+ mipi_dsi_msleep(dsi_ctx, 20);
+
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETVCOM, 0x87, 0x95);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETGIP1,
+ 0xc2, 0x10, 0x05, 0x05, 0x10, 0x05, 0xa0, 0x12,
+ 0x31, 0x23, 0x3f, 0x81, 0x0a, 0xa0, 0x37, 0x18,
+ 0x00, 0x80, 0x01, 0x00, 0x00, 0x00, 0x00, 0x80,
+ 0x01, 0x00, 0x00, 0x00, 0x48, 0xf8, 0x86, 0x42,
+ 0x08, 0x88, 0x88, 0x80, 0x88, 0x88, 0x88, 0x58,
+ 0xf8, 0x87, 0x53, 0x18, 0x88, 0x88, 0x81, 0x88,
+ 0x88, 0x88, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETGIP2,
+ 0x00, 0x1a, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x1f, 0x88, 0x81, 0x35,
+ 0x78, 0x88, 0x88, 0x85, 0x88, 0x88, 0x88, 0x0f,
+ 0x88, 0x80, 0x24, 0x68, 0x88, 0x88, 0x84, 0x88,
+ 0x88, 0x88, 0x23, 0x10, 0x00, 0x00, 0x1c, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x30, 0x05,
+ 0xa0, 0x00, 0x00, 0x00, 0x00);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETGAMMA,
+ 0x00, 0x06, 0x08, 0x2a, 0x31, 0x3f, 0x38, 0x36,
+ 0x07, 0x0c, 0x0d, 0x11, 0x13, 0x12, 0x13, 0x11,
+ 0x18, 0x00, 0x06, 0x08, 0x2a, 0x31, 0x3f, 0x38,
+ 0x36, 0x07, 0x0c, 0x0d, 0x11, 0x13, 0x12, 0x13,
+ 0x11, 0x18);
+
+ mipi_dsi_msleep(dsi_ctx, 60);
}
static int xpp055c272_unprepare(struct drm_panel *panel)
{
struct xpp055c272 *ctx = panel_to_xpp055c272(panel);
struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
- int ret;
-
- ret = mipi_dsi_dcs_set_display_off(dsi);
- if (ret < 0)
- dev_err(ctx->dev, "failed to set display off: %d\n", ret);
-
- mipi_dsi_dcs_enter_sleep_mode(dsi);
- if (ret < 0) {
- dev_err(ctx->dev, "failed to enter sleep mode: %d\n", ret);
- return ret;
+ struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
+
+ mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
+ mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
+ if (dsi_ctx.accum_err) {
+ dev_err(ctx->dev, "failed to enter sleep mode: %d\n",
+ dsi_ctx.accum_err);
+ return dsi_ctx.accum_err;
}
regulator_disable(ctx->iovcc);
@@ -155,17 +147,19 @@ static int xpp055c272_prepare(struct drm_panel *panel)
{
struct xpp055c272 *ctx = panel_to_xpp055c272(panel);
struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
- int ret;
+ struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
dev_dbg(ctx->dev, "Resetting the panel\n");
- ret = regulator_enable(ctx->vci);
- if (ret < 0) {
- dev_err(ctx->dev, "Failed to enable vci supply: %d\n", ret);
- return ret;
+ dsi_ctx.accum_err = regulator_enable(ctx->vci);
+ if (dsi_ctx.accum_err) {
+ dev_err(ctx->dev, "Failed to enable vci supply: %d\n",
+ dsi_ctx.accum_err);
+ return dsi_ctx.accum_err;
}
- ret = regulator_enable(ctx->iovcc);
- if (ret < 0) {
- dev_err(ctx->dev, "Failed to enable iovcc supply: %d\n", ret);
+ dsi_ctx.accum_err = regulator_enable(ctx->iovcc);
+ if (dsi_ctx.accum_err) {
+ dev_err(ctx->dev, "Failed to enable iovcc supply: %d\n",
+ dsi_ctx.accum_err);
goto disable_vci;
}
@@ -175,30 +169,19 @@ static int xpp055c272_prepare(struct drm_panel *panel)
gpiod_set_value_cansleep(ctx->reset_gpio, 0);
/* T8: 20ms */
- msleep(20);
+ mipi_dsi_msleep(&dsi_ctx, 20);
- ret = xpp055c272_init_sequence(ctx);
- if (ret < 0) {
- dev_err(ctx->dev, "Panel init sequence failed: %d\n", ret);
- goto disable_iovcc;
- }
-
- ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
- if (ret < 0) {
- dev_err(ctx->dev, "Failed to exit sleep mode: %d\n", ret);
- goto disable_iovcc;
- }
+ xpp055c272_init_sequence(&dsi_ctx);
+ dev_dbg(ctx->dev, "Panel init sequence done\n");
+ mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
/* T9: 120ms */
- msleep(120);
+ mipi_dsi_msleep(&dsi_ctx, 120);
+ mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
+ mipi_dsi_msleep(&dsi_ctx, 50);
- ret = mipi_dsi_dcs_set_display_on(dsi);
- if (ret < 0) {
- dev_err(ctx->dev, "Failed to set display on: %d\n", ret);
+ if (dsi_ctx.accum_err)
goto disable_iovcc;
- }
-
- msleep(50);
return 0;
@@ -206,7 +189,7 @@ static int xpp055c272_prepare(struct drm_panel *panel)
regulator_disable(ctx->iovcc);
disable_vci:
regulator_disable(ctx->vci);
- return ret;
+ return dsi_ctx.accum_err;
}
static const struct drm_display_mode default_mode = {
--
2.47.1On 23/12/2024 06:20, Tejas Vipin wrote:
> Changes the xinpeng-xpp055c272 panel to use multi style functions for
> improved error handling.
>
> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
> ---
> .../gpu/drm/panel/panel-xinpeng-xpp055c272.c | 185 ++++++++----------
> 1 file changed, 84 insertions(+), 101 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c b/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
> index 22a14006765e..45a405669593 100644
> --- a/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
> +++ b/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
> @@ -59,90 +59,82 @@ static inline struct xpp055c272 *panel_to_xpp055c272(struct drm_panel *panel)
> return container_of(panel, struct xpp055c272, panel);
> }
>
> -static int xpp055c272_init_sequence(struct xpp055c272 *ctx)
> +static void xpp055c272_init_sequence(struct mipi_dsi_multi_context *dsi_ctx)
> {
> - struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> - struct device *dev = ctx->dev;
> -
> /*
> * Init sequence was supplied by the panel vendor without much
> * documentation.
> */
> - mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETEXTC, 0xf1, 0x12, 0x83);
> - mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETMIPI,
> - 0x33, 0x81, 0x05, 0xf9, 0x0e, 0x0e, 0x00, 0x00,
> - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x44, 0x25,
> - 0x00, 0x91, 0x0a, 0x00, 0x00, 0x02, 0x4f, 0x01,
> - 0x00, 0x00, 0x37);
> - mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETPOWER_EXT, 0x25);
> - mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETPCR, 0x02, 0x11, 0x00);
> - mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETRGBIF,
> - 0x0c, 0x10, 0x0a, 0x50, 0x03, 0xff, 0x00, 0x00,
> - 0x00, 0x00);
> - mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETSCR,
> - 0x73, 0x73, 0x50, 0x50, 0x00, 0x00, 0x08, 0x70,
> - 0x00);
> - mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETVDC, 0x46);
> - mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETPANEL, 0x0b);
> - mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETCYC, 0x80);
> - mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETDISP, 0xc8, 0x12, 0x30);
> - mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETEQ,
> - 0x07, 0x07, 0x0B, 0x0B, 0x03, 0x0B, 0x00, 0x00,
> - 0x00, 0x00, 0xFF, 0x00, 0xC0, 0x10);
> - mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETPOWER,
> - 0x53, 0x00, 0x1e, 0x1e, 0x77, 0xe1, 0xcc, 0xdd,
> - 0x67, 0x77, 0x33, 0x33);
> - mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETECO, 0x00, 0x00, 0xff,
> - 0xff, 0x01, 0xff);
> - mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETBGP, 0x09, 0x09);
> - msleep(20);
> -
> - mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETVCOM, 0x87, 0x95);
> - mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETGIP1,
> - 0xc2, 0x10, 0x05, 0x05, 0x10, 0x05, 0xa0, 0x12,
> - 0x31, 0x23, 0x3f, 0x81, 0x0a, 0xa0, 0x37, 0x18,
> - 0x00, 0x80, 0x01, 0x00, 0x00, 0x00, 0x00, 0x80,
> - 0x01, 0x00, 0x00, 0x00, 0x48, 0xf8, 0x86, 0x42,
> - 0x08, 0x88, 0x88, 0x80, 0x88, 0x88, 0x88, 0x58,
> - 0xf8, 0x87, 0x53, 0x18, 0x88, 0x88, 0x81, 0x88,
> - 0x88, 0x88, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00,
> - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00);
> - mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETGIP2,
> - 0x00, 0x1a, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00,
> - 0x00, 0x00, 0x00, 0x00, 0x1f, 0x88, 0x81, 0x35,
> - 0x78, 0x88, 0x88, 0x85, 0x88, 0x88, 0x88, 0x0f,
> - 0x88, 0x80, 0x24, 0x68, 0x88, 0x88, 0x84, 0x88,
> - 0x88, 0x88, 0x23, 0x10, 0x00, 0x00, 0x1c, 0x00,
> - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x30, 0x05,
> - 0xa0, 0x00, 0x00, 0x00, 0x00);
> - mipi_dsi_dcs_write_seq(dsi, XPP055C272_CMD_SETGAMMA,
> - 0x00, 0x06, 0x08, 0x2a, 0x31, 0x3f, 0x38, 0x36,
> - 0x07, 0x0c, 0x0d, 0x11, 0x13, 0x12, 0x13, 0x11,
> - 0x18, 0x00, 0x06, 0x08, 0x2a, 0x31, 0x3f, 0x38,
> - 0x36, 0x07, 0x0c, 0x0d, 0x11, 0x13, 0x12, 0x13,
> - 0x11, 0x18);
> -
> - msleep(60);
> -
> - dev_dbg(dev, "Panel init sequence done\n");
> - return 0;
> + mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETEXTC, 0xf1, 0x12, 0x83);
> + mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETMIPI,
> + 0x33, 0x81, 0x05, 0xf9, 0x0e, 0x0e, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x44, 0x25,
> + 0x00, 0x91, 0x0a, 0x00, 0x00, 0x02, 0x4f, 0x01,
> + 0x00, 0x00, 0x37);
> + mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETPOWER_EXT, 0x25);
> + mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETPCR, 0x02, 0x11, 0x00);
> + mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETRGBIF,
> + 0x0c, 0x10, 0x0a, 0x50, 0x03, 0xff, 0x00, 0x00,
> + 0x00, 0x00);
> + mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETSCR,
> + 0x73, 0x73, 0x50, 0x50, 0x00, 0x00, 0x08, 0x70,
> + 0x00);
> + mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETVDC, 0x46);
> + mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETPANEL, 0x0b);
> + mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETCYC, 0x80);
> + mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETDISP, 0xc8, 0x12, 0x30);
> + mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETEQ,
> + 0x07, 0x07, 0x0b, 0x0b, 0x03, 0x0b, 0x00, 0x00,
> + 0x00, 0x00, 0xff, 0x00, 0xC0, 0x10);
> + mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETPOWER,
> + 0x53, 0x00, 0x1e, 0x1e, 0x77, 0xe1, 0xcc, 0xdd,
> + 0x67, 0x77, 0x33, 0x33);
> + mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETECO, 0x00, 0x00, 0xff,
> + 0xff, 0x01, 0xff);
> + mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETBGP, 0x09, 0x09);
> + mipi_dsi_msleep(dsi_ctx, 20);
> +
> + mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETVCOM, 0x87, 0x95);
> + mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETGIP1,
> + 0xc2, 0x10, 0x05, 0x05, 0x10, 0x05, 0xa0, 0x12,
> + 0x31, 0x23, 0x3f, 0x81, 0x0a, 0xa0, 0x37, 0x18,
> + 0x00, 0x80, 0x01, 0x00, 0x00, 0x00, 0x00, 0x80,
> + 0x01, 0x00, 0x00, 0x00, 0x48, 0xf8, 0x86, 0x42,
> + 0x08, 0x88, 0x88, 0x80, 0x88, 0x88, 0x88, 0x58,
> + 0xf8, 0x87, 0x53, 0x18, 0x88, 0x88, 0x81, 0x88,
> + 0x88, 0x88, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00);
> + mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETGIP2,
> + 0x00, 0x1a, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x1f, 0x88, 0x81, 0x35,
> + 0x78, 0x88, 0x88, 0x85, 0x88, 0x88, 0x88, 0x0f,
> + 0x88, 0x80, 0x24, 0x68, 0x88, 0x88, 0x84, 0x88,
> + 0x88, 0x88, 0x23, 0x10, 0x00, 0x00, 0x1c, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x30, 0x05,
> + 0xa0, 0x00, 0x00, 0x00, 0x00);
> + mipi_dsi_dcs_write_seq_multi(dsi_ctx, XPP055C272_CMD_SETGAMMA,
> + 0x00, 0x06, 0x08, 0x2a, 0x31, 0x3f, 0x38, 0x36,
> + 0x07, 0x0c, 0x0d, 0x11, 0x13, 0x12, 0x13, 0x11,
> + 0x18, 0x00, 0x06, 0x08, 0x2a, 0x31, 0x3f, 0x38,
> + 0x36, 0x07, 0x0c, 0x0d, 0x11, 0x13, 0x12, 0x13,
> + 0x11, 0x18);
> +
> + mipi_dsi_msleep(dsi_ctx, 60);
> }
>
> static int xpp055c272_unprepare(struct drm_panel *panel)
> {
> struct xpp055c272 *ctx = panel_to_xpp055c272(panel);
> struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> - int ret;
> -
> - ret = mipi_dsi_dcs_set_display_off(dsi);
> - if (ret < 0)
> - dev_err(ctx->dev, "failed to set display off: %d\n", ret);
> -
> - mipi_dsi_dcs_enter_sleep_mode(dsi);
> - if (ret < 0) {
> - dev_err(ctx->dev, "failed to enter sleep mode: %d\n", ret);
> - return ret;
> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
> +
> + mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> + mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
> + if (dsi_ctx.accum_err) {
> + dev_err(ctx->dev, "failed to enter sleep mode: %d\n",
> + dsi_ctx.accum_err);
> + return dsi_ctx.accum_err;
> }
>
> regulator_disable(ctx->iovcc);
> @@ -155,17 +147,19 @@ static int xpp055c272_prepare(struct drm_panel *panel)
> {
> struct xpp055c272 *ctx = panel_to_xpp055c272(panel);
> struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> - int ret;
> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>
> dev_dbg(ctx->dev, "Resetting the panel\n");
> - ret = regulator_enable(ctx->vci);
> - if (ret < 0) {
> - dev_err(ctx->dev, "Failed to enable vci supply: %d\n", ret);
> - return ret;
> + dsi_ctx.accum_err = regulator_enable(ctx->vci);
> + if (dsi_ctx.accum_err) {
I would rather keep ret instead of abusing dsi_ctx.accum_err, but it's already like
that in other converted driver so I won't oppose it...
> + dev_err(ctx->dev, "Failed to enable vci supply: %d\n",
> + dsi_ctx.accum_err);
> + return dsi_ctx.accum_err;
> }
> - ret = regulator_enable(ctx->iovcc);
> - if (ret < 0) {
> - dev_err(ctx->dev, "Failed to enable iovcc supply: %d\n", ret);
> + dsi_ctx.accum_err = regulator_enable(ctx->iovcc);
> + if (dsi_ctx.accum_err) {
> + dev_err(ctx->dev, "Failed to enable iovcc supply: %d\n",
> + dsi_ctx.accum_err);
> goto disable_vci;
> }
>
> @@ -175,30 +169,19 @@ static int xpp055c272_prepare(struct drm_panel *panel)
> gpiod_set_value_cansleep(ctx->reset_gpio, 0);
>
> /* T8: 20ms */
> - msleep(20);
> + mipi_dsi_msleep(&dsi_ctx, 20);
>
> - ret = xpp055c272_init_sequence(ctx);
> - if (ret < 0) {
> - dev_err(ctx->dev, "Panel init sequence failed: %d\n", ret);
> - goto disable_iovcc;
> - }
> -
> - ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
> - if (ret < 0) {
> - dev_err(ctx->dev, "Failed to exit sleep mode: %d\n", ret);
> - goto disable_iovcc;
> - }
> + xpp055c272_init_sequence(&dsi_ctx);
> + dev_dbg(ctx->dev, "Panel init sequence done\n");
>
> + mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> /* T9: 120ms */
> - msleep(120);
> + mipi_dsi_msleep(&dsi_ctx, 120);
> + mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> + mipi_dsi_msleep(&dsi_ctx, 50);
>
> - ret = mipi_dsi_dcs_set_display_on(dsi);
> - if (ret < 0) {
> - dev_err(ctx->dev, "Failed to set display on: %d\n", ret);
> + if (dsi_ctx.accum_err)
> goto disable_iovcc;
> - }
> -
> - msleep(50);
>
> return 0;
>
> @@ -206,7 +189,7 @@ static int xpp055c272_prepare(struct drm_panel *panel)
> regulator_disable(ctx->iovcc);
> disable_vci:
> regulator_disable(ctx->vci);
> - return ret;
> + return dsi_ctx.accum_err;
> }
>
> static const struct drm_display_mode default_mode = {
Otherwise looks good
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Neil
Hi,
On Mon, Dec 30, 2024 at 2:10 AM <neil.armstrong@linaro.org> wrote:
>
> > static int xpp055c272_unprepare(struct drm_panel *panel)
> > {
> > struct xpp055c272 *ctx = panel_to_xpp055c272(panel);
> > struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> > - int ret;
> > -
> > - ret = mipi_dsi_dcs_set_display_off(dsi);
> > - if (ret < 0)
> > - dev_err(ctx->dev, "failed to set display off: %d\n", ret);
> > -
> > - mipi_dsi_dcs_enter_sleep_mode(dsi);
> > - if (ret < 0) {
> > - dev_err(ctx->dev, "failed to enter sleep mode: %d\n", ret);
> > - return ret;
> > + struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
> > +
> > + mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> > + mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
> > + if (dsi_ctx.accum_err) {
> > + dev_err(ctx->dev, "failed to enter sleep mode: %d\n",
> > + dsi_ctx.accum_err);
You should delete the above error message, right?
mipi_dsi_dcs_enter_sleep_mode_multi() reports the error for you, I
think.
> > @@ -155,17 +147,19 @@ static int xpp055c272_prepare(struct drm_panel *panel)
> > {
> > struct xpp055c272 *ctx = panel_to_xpp055c272(panel);
> > struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> > - int ret;
> > + struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
> >
> > dev_dbg(ctx->dev, "Resetting the panel\n");
> > - ret = regulator_enable(ctx->vci);
> > - if (ret < 0) {
> > - dev_err(ctx->dev, "Failed to enable vci supply: %d\n", ret);
> > - return ret;
> > + dsi_ctx.accum_err = regulator_enable(ctx->vci);
> > + if (dsi_ctx.accum_err) {
>
> I would rather keep ret instead of abusing dsi_ctx.accum_err, but it's already like
> that in other converted driver so I won't oppose it...
FWIW, we had this discussion before. I agree with what Tejas did here
and I managed to convince Dmitry Baryshkov in the past. See:
https://lore.kernel.org/all/CAA8EJpr_HYkXnP3XR9LpDhi1xkQfE_CKJzfzGrO5qd_pQYtiOw@mail.gmail.com/
Looking specifically at this driver, using "ret" would have added
complexity when we wanted to do "goto disable_vci" because in some
cases the error code would be in "ret" and sometimes in "accum_err"...
> > @@ -175,30 +169,19 @@ static int xpp055c272_prepare(struct drm_panel *panel)
> > gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> >
> > /* T8: 20ms */
> > - msleep(20);
> > + mipi_dsi_msleep(&dsi_ctx, 20);
Personally, I would have left the above msleep() alone. There can be
no errors at this point in the code, right?
> > - ret = xpp055c272_init_sequence(ctx);
> > - if (ret < 0) {
> > - dev_err(ctx->dev, "Panel init sequence failed: %d\n", ret);
> > - goto disable_iovcc;
> > - }
> > -
> > - ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
> > - if (ret < 0) {
> > - dev_err(ctx->dev, "Failed to exit sleep mode: %d\n", ret);
> > - goto disable_iovcc;
> > - }
> > + xpp055c272_init_sequence(&dsi_ctx);
> > + dev_dbg(ctx->dev, "Panel init sequence done\n");
Should the above print be only if "accum_err" is 0? That would match
the previous behavior. I guess I would have also left the print as
part of xpp055c272_init_sequence() unless there's a reason for moving
it...
On 1/7/25 5:37 AM, Doug Anderson wrote:
> Hi,
>
> On Mon, Dec 30, 2024 at 2:10 AM <neil.armstrong@linaro.org> wrote:
>>
>>> static int xpp055c272_unprepare(struct drm_panel *panel)
>>> {
>>> struct xpp055c272 *ctx = panel_to_xpp055c272(panel);
>>> struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
>>> - int ret;
>>> -
>>> - ret = mipi_dsi_dcs_set_display_off(dsi);
>>> - if (ret < 0)
>>> - dev_err(ctx->dev, "failed to set display off: %d\n", ret);
>>> -
>>> - mipi_dsi_dcs_enter_sleep_mode(dsi);
>>> - if (ret < 0) {
>>> - dev_err(ctx->dev, "failed to enter sleep mode: %d\n", ret);
>>> - return ret;
>>> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>>> +
>>> + mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
>>> + mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
>>> + if (dsi_ctx.accum_err) {
>>> + dev_err(ctx->dev, "failed to enter sleep mode: %d\n",
>>> + dsi_ctx.accum_err);
>
> You should delete the above error message, right?
> mipi_dsi_dcs_enter_sleep_mode_multi() reports the error for you, I
> think.
>
>
>>> @@ -155,17 +147,19 @@ static int xpp055c272_prepare(struct drm_panel *panel)
>>> {
>>> struct xpp055c272 *ctx = panel_to_xpp055c272(panel);
>>> struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
>>> - int ret;
>>> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>>>
>>> dev_dbg(ctx->dev, "Resetting the panel\n");
>>> - ret = regulator_enable(ctx->vci);
>>> - if (ret < 0) {
>>> - dev_err(ctx->dev, "Failed to enable vci supply: %d\n", ret);
>>> - return ret;
>>> + dsi_ctx.accum_err = regulator_enable(ctx->vci);
>>> + if (dsi_ctx.accum_err) {
>>
>> I would rather keep ret instead of abusing dsi_ctx.accum_err, but it's already like
>> that in other converted driver so I won't oppose it...
>
> FWIW, we had this discussion before. I agree with what Tejas did here
> and I managed to convince Dmitry Baryshkov in the past. See:
>
> https://lore.kernel.org/all/CAA8EJpr_HYkXnP3XR9LpDhi1xkQfE_CKJzfzGrO5qd_pQYtiOw@mail.gmail.com/
>
> Looking specifically at this driver, using "ret" would have added
> complexity when we wanted to do "goto disable_vci" because in some
> cases the error code would be in "ret" and sometimes in "accum_err"...
>
>
>>> @@ -175,30 +169,19 @@ static int xpp055c272_prepare(struct drm_panel *panel)
>>> gpiod_set_value_cansleep(ctx->reset_gpio, 0);
>>>
>>> /* T8: 20ms */
>>> - msleep(20);
>>> + mipi_dsi_msleep(&dsi_ctx, 20);
>
> Personally, I would have left the above msleep() alone. There can be
> no errors at this point in the code, right?
>
>
>>> - ret = xpp055c272_init_sequence(ctx);
>>> - if (ret < 0) {
>>> - dev_err(ctx->dev, "Panel init sequence failed: %d\n", ret);
>>> - goto disable_iovcc;
>>> - }
>>> -
>>> - ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
>>> - if (ret < 0) {
>>> - dev_err(ctx->dev, "Failed to exit sleep mode: %d\n", ret);
>>> - goto disable_iovcc;
>>> - }
>>> + xpp055c272_init_sequence(&dsi_ctx);
>>> + dev_dbg(ctx->dev, "Panel init sequence done\n");
>
> Should the above print be only if "accum_err" is 0? That would match
> the previous behavior. I guess I would have also left the print as
> part of xpp055c272_init_sequence() unless there's a reason for moving
> it...
I don't think it should print only if accum_err is 0. In the previous
code, it would just print after all the msleeps and write_seqs are done,
with no error checking at any point.
The reason I've moved the print outside the function is because we are
able to reduce a couple lines of code by passing dsi_ctx to the function
instead of ctx. If I'd kept the print inside, it would require us to
declare a `struct device*` variable which would require ctx as far as
I've seen and just overall introduces some lines that we could otherwise
avoid. I've done this in a couple other panels too.
I'll do a v2 with the other suggested changes.
--
Tejas Vipin
Hi,
On Mon, Jan 6, 2025 at 8:21 PM Tejas Vipin <tejasvipin76@gmail.com> wrote:
>
> >>> - ret = xpp055c272_init_sequence(ctx);
> >>> - if (ret < 0) {
> >>> - dev_err(ctx->dev, "Panel init sequence failed: %d\n", ret);
> >>> - goto disable_iovcc;
> >>> - }
> >>> -
> >>> - ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
> >>> - if (ret < 0) {
> >>> - dev_err(ctx->dev, "Failed to exit sleep mode: %d\n", ret);
> >>> - goto disable_iovcc;
> >>> - }
> >>> + xpp055c272_init_sequence(&dsi_ctx);
> >>> + dev_dbg(ctx->dev, "Panel init sequence done\n");
> >
> > Should the above print be only if "accum_err" is 0? That would match
> > the previous behavior. I guess I would have also left the print as
> > part of xpp055c272_init_sequence() unless there's a reason for moving
> > it...
>
> I don't think it should print only if accum_err is 0. In the previous
> code, it would just print after all the msleeps and write_seqs are done,
> with no error checking at any point.
How sure are you about this? Remember the reason why we wanted to
deprecate mipi_dsi_dcs_write_seq()? All those dang hidden return
values. So if any one of the old mipi_dsi_dcs_write_seq() got an error
they would have had a non-obvious return out of the function, right?
So the print would have only happened if all of the commands executed
successfully...
:-P
> The reason I've moved the print outside the function is because we are
> able to reduce a couple lines of code by passing dsi_ctx to the function
> instead of ctx. If I'd kept the print inside, it would require us to
> declare a `struct device*` variable which would require ctx as far as
> I've seen and just overall introduces some lines that we could otherwise
> avoid. I've done this in a couple other panels too.
Ah, OK. That's a reasonable reason. Thanks for the explanation...
-Doug
On 1/7/25 10:18 AM, Doug Anderson wrote:
> Hi,
>
> On Mon, Jan 6, 2025 at 8:21 PM Tejas Vipin <tejasvipin76@gmail.com> wrote:
>>
>>>>> - ret = xpp055c272_init_sequence(ctx);
>>>>> - if (ret < 0) {
>>>>> - dev_err(ctx->dev, "Panel init sequence failed: %d\n", ret);
>>>>> - goto disable_iovcc;
>>>>> - }
>>>>> -
>>>>> - ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
>>>>> - if (ret < 0) {
>>>>> - dev_err(ctx->dev, "Failed to exit sleep mode: %d\n", ret);
>>>>> - goto disable_iovcc;
>>>>> - }
>>>>> + xpp055c272_init_sequence(&dsi_ctx);
>>>>> + dev_dbg(ctx->dev, "Panel init sequence done\n");
>>>
>>> Should the above print be only if "accum_err" is 0? That would match
>>> the previous behavior. I guess I would have also left the print as
>>> part of xpp055c272_init_sequence() unless there's a reason for moving
>>> it...
>>
>> I don't think it should print only if accum_err is 0. In the previous
>> code, it would just print after all the msleeps and write_seqs are done,
>> with no error checking at any point.
>
> How sure are you about this? Remember the reason why we wanted to
> deprecate mipi_dsi_dcs_write_seq()? All those dang hidden return
> values. So if any one of the old mipi_dsi_dcs_write_seq() got an error
> they would have had a non-obvious return out of the function, right?
> So the print would have only happened if all of the commands executed
> successfully...
>
> :-P
Right yes. I'd kind of forgotten how mipi_dsi_dcs_write_seq worked. I'll
fix it then along with the rest of the changes.
>
>
>> The reason I've moved the print outside the function is because we are
>> able to reduce a couple lines of code by passing dsi_ctx to the function
>> instead of ctx. If I'd kept the print inside, it would require us to
>> declare a `struct device*` variable which would require ctx as far as
>> I've seen and just overall introduces some lines that we could otherwise
>> avoid. I've done this in a couple other panels too.
>
> Ah, OK. That's a reasonable reason. Thanks for the explanation...
>
>
> -Doug
--
Tejas Vipin
© 2016 - 2025 Red Hat, Inc.