* [PATCH] drm/panel: himax-hx83112a: transition to mipi_dsi wrapped functions
@ 2024-09-04 14:15 Tejas Vipin
2024-09-04 17:52 ` Doug Anderson
2024-09-06 22:14 ` Jessica Zhang
0 siblings, 2 replies; 10+ messages in thread
From: Tejas Vipin @ 2024-09-04 14:15 UTC (permalink / raw)
To: neil.armstrong, maarten.lankhorst, mripard, tzimmermann, airlied, daniel
Cc: quic_jesszhan, dianders, dri-devel, linux-kernel, Tejas Vipin
Changes the himax-hx83112a panel to use multi style functions for
improved error handling.
Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
---
drivers/gpu/drm/panel/panel-himax-hx83112a.c | 297 +++++++++----------
1 file changed, 136 insertions(+), 161 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-himax-hx83112a.c b/drivers/gpu/drm/panel/panel-himax-hx83112a.c
index 466c27012abf..47bce087e339 100644
--- a/drivers/gpu/drm/panel/panel-himax-hx83112a.c
+++ b/drivers/gpu/drm/panel/panel-himax-hx83112a.c
@@ -56,198 +56,173 @@ static void hx83112a_reset(struct hx83112a_panel *ctx)
msleep(50);
}
-static int hx83112a_on(struct hx83112a_panel *ctx)
+static int hx83112a_on(struct mipi_dsi_device *dsi)
{
- 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;
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETEXTC, 0x83, 0x11, 0x2a);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPOWER1,
- 0x08, 0x28, 0x28, 0x83, 0x83, 0x4c, 0x4f, 0x33);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDISP,
- 0x00, 0x02, 0x00, 0x90, 0x24, 0x00, 0x08, 0x19,
- 0xea, 0x11, 0x11, 0x00, 0x11, 0xa3);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDRV,
- 0x58, 0x68, 0x58, 0x68, 0x0f, 0xef, 0x0b, 0xc0,
- 0x0b, 0xc0, 0x0b, 0xc0, 0x00, 0xff, 0x00, 0xff,
- 0x00, 0x00, 0x14, 0x15, 0x00, 0x29, 0x11, 0x07,
- 0x12, 0x00, 0x29);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDRV,
- 0x00, 0x12, 0x12, 0x11, 0x88, 0x12, 0x12, 0x00,
- 0x53);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x03);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT,
- 0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
- 0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
- 0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
- 0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
- 0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
- 0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
- 0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
- 0x40);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT,
- 0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
- 0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
- 0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
- 0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
- 0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
- 0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
- 0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
- 0x40);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT,
- 0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
- 0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
- 0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
- 0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
- 0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
- 0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
- 0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
- 0x40);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT, 0x01);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTCON,
- 0x70, 0x00, 0x04, 0xe0, 0x33, 0x00);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPANEL, 0x08);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPOWER2, 0x2b, 0x2b);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP0,
- 0x80, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x08,
- 0x08, 0x03, 0x03, 0x22, 0x18, 0x07, 0x07, 0x07,
- 0x07, 0x32, 0x10, 0x06, 0x00, 0x06, 0x32, 0x10,
- 0x07, 0x00, 0x07, 0x32, 0x19, 0x31, 0x09, 0x31,
- 0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x00, 0x08,
- 0x09, 0x30, 0x00, 0x00, 0x00, 0x06, 0x0d, 0x00,
- 0x0f);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP0,
- 0x00, 0x00, 0x19, 0x10, 0x00, 0x0a, 0x00, 0x81);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP1,
- 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
- 0xc0, 0xc0, 0x18, 0x18, 0x19, 0x19, 0x18, 0x18,
- 0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
- 0x28, 0x28, 0x24, 0x24, 0x02, 0x03, 0x02, 0x03,
- 0x00, 0x01, 0x00, 0x01, 0x31, 0x31, 0x31, 0x31,
- 0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP2,
- 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
- 0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x19, 0x19,
- 0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
- 0x24, 0x24, 0x28, 0x28, 0x01, 0x00, 0x01, 0x00,
- 0x03, 0x02, 0x03, 0x02, 0x31, 0x31, 0x31, 0x31,
- 0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
- 0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea,
- 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa,
- 0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa, 0xaa, 0xaa);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
- 0xaa, 0x2e, 0x28, 0x00, 0x00, 0x00, 0xaa, 0x2e,
- 0x28, 0x00, 0x00, 0x00, 0xaa, 0xee, 0xaa, 0xaa,
- 0xaa, 0xaa, 0xaa, 0xee, 0xaa, 0xaa, 0xaa, 0xaa);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
- 0xaa, 0xff, 0xff, 0xff, 0xff, 0xff, 0xaa, 0xff,
- 0xff, 0xff, 0xff, 0xff);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x03);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
- 0xaa, 0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa,
- 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xff, 0xff, 0xff,
- 0xff, 0xff, 0xaa, 0xff, 0xff, 0xff, 0xff, 0xff);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
- 0x0e, 0x0e, 0x1e, 0x65, 0x1c, 0x65, 0x00, 0x50,
- 0x20, 0x20, 0x00, 0x00, 0x02, 0x02, 0x02, 0x05,
- 0x14, 0x14, 0x32, 0xb9, 0x23, 0xb9, 0x08);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
- 0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
- 0x00, 0x00, 0x08, 0x00, 0x01, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00,
- 0x00, 0x00, 0x00, 0x02, 0x00);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0xc3);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETCLOCK, 0xd1, 0xd6);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0x3f);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0xc6);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPTBA, 0x37);
- mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0x3f);
-
- 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(150);
-
- 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(50);
-
- return 0;
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETEXTC, 0x83, 0x11, 0x2a);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPOWER1,
+ 0x08, 0x28, 0x28, 0x83, 0x83, 0x4c, 0x4f, 0x33);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDISP,
+ 0x00, 0x02, 0x00, 0x90, 0x24, 0x00, 0x08, 0x19,
+ 0xea, 0x11, 0x11, 0x00, 0x11, 0xa3);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDRV,
+ 0x58, 0x68, 0x58, 0x68, 0x0f, 0xef, 0x0b, 0xc0,
+ 0x0b, 0xc0, 0x0b, 0xc0, 0x00, 0xff, 0x00, 0xff,
+ 0x00, 0x00, 0x14, 0x15, 0x00, 0x29, 0x11, 0x07,
+ 0x12, 0x00, 0x29);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDRV,
+ 0x00, 0x12, 0x12, 0x11, 0x88, 0x12, 0x12, 0x00,
+ 0x53);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x03);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT,
+ 0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
+ 0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
+ 0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
+ 0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
+ 0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
+ 0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
+ 0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
+ 0x40);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT,
+ 0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
+ 0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
+ 0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
+ 0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
+ 0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
+ 0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
+ 0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
+ 0x40);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT,
+ 0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
+ 0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
+ 0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
+ 0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
+ 0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
+ 0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
+ 0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
+ 0x40);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT, 0x01);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTCON,
+ 0x70, 0x00, 0x04, 0xe0, 0x33, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPANEL, 0x08);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPOWER2, 0x2b, 0x2b);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP0,
+ 0x80, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x08,
+ 0x08, 0x03, 0x03, 0x22, 0x18, 0x07, 0x07, 0x07,
+ 0x07, 0x32, 0x10, 0x06, 0x00, 0x06, 0x32, 0x10,
+ 0x07, 0x00, 0x07, 0x32, 0x19, 0x31, 0x09, 0x31,
+ 0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x00, 0x08,
+ 0x09, 0x30, 0x00, 0x00, 0x00, 0x06, 0x0d, 0x00,
+ 0x0f);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP0,
+ 0x00, 0x00, 0x19, 0x10, 0x00, 0x0a, 0x00, 0x81);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP1,
+ 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
+ 0xc0, 0xc0, 0x18, 0x18, 0x19, 0x19, 0x18, 0x18,
+ 0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
+ 0x28, 0x28, 0x24, 0x24, 0x02, 0x03, 0x02, 0x03,
+ 0x00, 0x01, 0x00, 0x01, 0x31, 0x31, 0x31, 0x31,
+ 0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP2,
+ 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
+ 0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x19, 0x19,
+ 0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
+ 0x24, 0x24, 0x28, 0x28, 0x01, 0x00, 0x01, 0x00,
+ 0x03, 0x02, 0x03, 0x02, 0x31, 0x31, 0x31, 0x31,
+ 0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
+ 0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea,
+ 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa,
+ 0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa, 0xaa, 0xaa);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
+ 0xaa, 0x2e, 0x28, 0x00, 0x00, 0x00, 0xaa, 0x2e,
+ 0x28, 0x00, 0x00, 0x00, 0xaa, 0xee, 0xaa, 0xaa,
+ 0xaa, 0xaa, 0xaa, 0xee, 0xaa, 0xaa, 0xaa, 0xaa);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
+ 0xaa, 0xff, 0xff, 0xff, 0xff, 0xff, 0xaa, 0xff,
+ 0xff, 0xff, 0xff, 0xff);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x03);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
+ 0xaa, 0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa,
+ 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xaa, 0xff, 0xff, 0xff, 0xff, 0xff);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
+ 0x0e, 0x0e, 0x1e, 0x65, 0x1c, 0x65, 0x00, 0x50,
+ 0x20, 0x20, 0x00, 0x00, 0x02, 0x02, 0x02, 0x05,
+ 0x14, 0x14, 0x32, 0xb9, 0x23, 0xb9, 0x08);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
+ 0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
+ 0x00, 0x00, 0x08, 0x00, 0x01, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00,
+ 0x00, 0x00, 0x00, 0x02, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0xc3);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETCLOCK, 0xd1, 0xd6);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0x3f);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0xc6);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPTBA, 0x37);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0x3f);
+
+ mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+ mipi_dsi_msleep(&dsi_ctx, 150);
+
+ mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
+ mipi_dsi_msleep(&dsi_ctx, 50);
+
+ return dsi_ctx.accum_err;
}
static int hx83112a_disable(struct drm_panel *panel)
{
struct hx83112a_panel *ctx = to_hx83112a_panel(panel);
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);
+ mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
+ mipi_dsi_msleep(&dsi_ctx, 120);
- return 0;
+ return dsi_ctx.accum_err;
}
static int hx83112a_prepare(struct drm_panel *panel)
{
struct hx83112a_panel *ctx = to_hx83112a_panel(panel);
- struct device *dev = &ctx->dsi->dev;
int ret;
ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
- if (ret < 0) {
- dev_err(dev, "Failed to enable regulators: %d\n", ret);
+ if (ret < 0)
return ret;
- }
hx83112a_reset(ctx);
- ret = hx83112a_on(ctx);
+ ret = hx83112a_on(ctx->dsi);
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;
}
- return 0;
+ return ret;
}
static int hx83112a_unprepare(struct drm_panel *panel)
--
2.46.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/panel: himax-hx83112a: transition to mipi_dsi wrapped functions
2024-09-04 14:15 [PATCH] drm/panel: himax-hx83112a: transition to mipi_dsi wrapped functions Tejas Vipin
@ 2024-09-04 17:52 ` Doug Anderson
2024-09-06 22:14 ` Jessica Zhang
1 sibling, 0 replies; 10+ messages in thread
From: Doug Anderson @ 2024-09-04 17:52 UTC (permalink / raw)
To: Tejas Vipin
Cc: neil.armstrong, maarten.lankhorst, mripard, tzimmermann, airlied,
daniel, quic_jesszhan, dri-devel, linux-kernel
Hi,
On Wed, Sep 4, 2024 at 7:15 AM Tejas Vipin <tejasvipin76@gmail.com> wrote:
>
> Changes the himax-hx83112a panel to use multi style functions for
> improved error handling.
>
> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
> ---
> drivers/gpu/drm/panel/panel-himax-hx83112a.c | 297 +++++++++----------
> 1 file changed, 136 insertions(+), 161 deletions(-)
Reviewed-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/panel: himax-hx83112a: transition to mipi_dsi wrapped functions
2024-09-04 14:15 [PATCH] drm/panel: himax-hx83112a: transition to mipi_dsi wrapped functions Tejas Vipin
2024-09-04 17:52 ` Doug Anderson
@ 2024-09-06 22:14 ` Jessica Zhang
2024-09-06 22:23 ` Jessica Zhang
1 sibling, 1 reply; 10+ messages in thread
From: Jessica Zhang @ 2024-09-06 22:14 UTC (permalink / raw)
To: Tejas Vipin, neil.armstrong, maarten.lankhorst, mripard,
tzimmermann, airlied, daniel
Cc: dianders, dri-devel, linux-kernel
On 9/4/2024 7:15 AM, Tejas Vipin wrote:
> Changes the himax-hx83112a panel to use multi style functions for
> improved error handling.
>
> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> ---
> drivers/gpu/drm/panel/panel-himax-hx83112a.c | 297 +++++++++----------
> 1 file changed, 136 insertions(+), 161 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-himax-hx83112a.c b/drivers/gpu/drm/panel/panel-himax-hx83112a.c
> index 466c27012abf..47bce087e339 100644
> --- a/drivers/gpu/drm/panel/panel-himax-hx83112a.c
> +++ b/drivers/gpu/drm/panel/panel-himax-hx83112a.c
> @@ -56,198 +56,173 @@ static void hx83112a_reset(struct hx83112a_panel *ctx)
> msleep(50);
> }
>
> -static int hx83112a_on(struct hx83112a_panel *ctx)
> +static int hx83112a_on(struct mipi_dsi_device *dsi)
> {
> - 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;
>
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETEXTC, 0x83, 0x11, 0x2a);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPOWER1,
> - 0x08, 0x28, 0x28, 0x83, 0x83, 0x4c, 0x4f, 0x33);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDISP,
> - 0x00, 0x02, 0x00, 0x90, 0x24, 0x00, 0x08, 0x19,
> - 0xea, 0x11, 0x11, 0x00, 0x11, 0xa3);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDRV,
> - 0x58, 0x68, 0x58, 0x68, 0x0f, 0xef, 0x0b, 0xc0,
> - 0x0b, 0xc0, 0x0b, 0xc0, 0x00, 0xff, 0x00, 0xff,
> - 0x00, 0x00, 0x14, 0x15, 0x00, 0x29, 0x11, 0x07,
> - 0x12, 0x00, 0x29);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDRV,
> - 0x00, 0x12, 0x12, 0x11, 0x88, 0x12, 0x12, 0x00,
> - 0x53);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x03);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT,
> - 0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
> - 0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
> - 0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
> - 0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
> - 0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
> - 0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
> - 0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
> - 0x40);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT,
> - 0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
> - 0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
> - 0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
> - 0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
> - 0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
> - 0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
> - 0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
> - 0x40);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT,
> - 0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
> - 0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
> - 0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
> - 0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
> - 0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
> - 0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
> - 0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
> - 0x40);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT, 0x01);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTCON,
> - 0x70, 0x00, 0x04, 0xe0, 0x33, 0x00);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPANEL, 0x08);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPOWER2, 0x2b, 0x2b);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP0,
> - 0x80, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x08,
> - 0x08, 0x03, 0x03, 0x22, 0x18, 0x07, 0x07, 0x07,
> - 0x07, 0x32, 0x10, 0x06, 0x00, 0x06, 0x32, 0x10,
> - 0x07, 0x00, 0x07, 0x32, 0x19, 0x31, 0x09, 0x31,
> - 0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x00, 0x08,
> - 0x09, 0x30, 0x00, 0x00, 0x00, 0x06, 0x0d, 0x00,
> - 0x0f);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP0,
> - 0x00, 0x00, 0x19, 0x10, 0x00, 0x0a, 0x00, 0x81);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP1,
> - 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
> - 0xc0, 0xc0, 0x18, 0x18, 0x19, 0x19, 0x18, 0x18,
> - 0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
> - 0x28, 0x28, 0x24, 0x24, 0x02, 0x03, 0x02, 0x03,
> - 0x00, 0x01, 0x00, 0x01, 0x31, 0x31, 0x31, 0x31,
> - 0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP2,
> - 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
> - 0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x19, 0x19,
> - 0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
> - 0x24, 0x24, 0x28, 0x28, 0x01, 0x00, 0x01, 0x00,
> - 0x03, 0x02, 0x03, 0x02, 0x31, 0x31, 0x31, 0x31,
> - 0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
> - 0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea,
> - 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa,
> - 0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa, 0xaa, 0xaa);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
> - 0xaa, 0x2e, 0x28, 0x00, 0x00, 0x00, 0xaa, 0x2e,
> - 0x28, 0x00, 0x00, 0x00, 0xaa, 0xee, 0xaa, 0xaa,
> - 0xaa, 0xaa, 0xaa, 0xee, 0xaa, 0xaa, 0xaa, 0xaa);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
> - 0xaa, 0xff, 0xff, 0xff, 0xff, 0xff, 0xaa, 0xff,
> - 0xff, 0xff, 0xff, 0xff);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x03);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
> - 0xaa, 0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa,
> - 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xff, 0xff, 0xff,
> - 0xff, 0xff, 0xaa, 0xff, 0xff, 0xff, 0xff, 0xff);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
> - 0x0e, 0x0e, 0x1e, 0x65, 0x1c, 0x65, 0x00, 0x50,
> - 0x20, 0x20, 0x00, 0x00, 0x02, 0x02, 0x02, 0x05,
> - 0x14, 0x14, 0x32, 0xb9, 0x23, 0xb9, 0x08);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
> - 0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
> - 0x00, 0x00, 0x08, 0x00, 0x01, 0x00, 0x00, 0x00,
> - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00,
> - 0x00, 0x00, 0x00, 0x02, 0x00);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0xc3);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETCLOCK, 0xd1, 0xd6);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0x3f);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0xc6);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPTBA, 0x37);
> - mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0x3f);
> -
> - 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(150);
> -
> - 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(50);
> -
> - return 0;
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETEXTC, 0x83, 0x11, 0x2a);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPOWER1,
> + 0x08, 0x28, 0x28, 0x83, 0x83, 0x4c, 0x4f, 0x33);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDISP,
> + 0x00, 0x02, 0x00, 0x90, 0x24, 0x00, 0x08, 0x19,
> + 0xea, 0x11, 0x11, 0x00, 0x11, 0xa3);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDRV,
> + 0x58, 0x68, 0x58, 0x68, 0x0f, 0xef, 0x0b, 0xc0,
> + 0x0b, 0xc0, 0x0b, 0xc0, 0x00, 0xff, 0x00, 0xff,
> + 0x00, 0x00, 0x14, 0x15, 0x00, 0x29, 0x11, 0x07,
> + 0x12, 0x00, 0x29);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDRV,
> + 0x00, 0x12, 0x12, 0x11, 0x88, 0x12, 0x12, 0x00,
> + 0x53);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x03);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT,
> + 0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
> + 0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
> + 0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
> + 0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
> + 0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
> + 0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
> + 0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
> + 0x40);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT,
> + 0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
> + 0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
> + 0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
> + 0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
> + 0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
> + 0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
> + 0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
> + 0x40);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT,
> + 0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
> + 0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
> + 0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
> + 0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
> + 0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
> + 0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
> + 0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
> + 0x40);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT, 0x01);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTCON,
> + 0x70, 0x00, 0x04, 0xe0, 0x33, 0x00);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPANEL, 0x08);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPOWER2, 0x2b, 0x2b);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP0,
> + 0x80, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x08,
> + 0x08, 0x03, 0x03, 0x22, 0x18, 0x07, 0x07, 0x07,
> + 0x07, 0x32, 0x10, 0x06, 0x00, 0x06, 0x32, 0x10,
> + 0x07, 0x00, 0x07, 0x32, 0x19, 0x31, 0x09, 0x31,
> + 0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x00, 0x08,
> + 0x09, 0x30, 0x00, 0x00, 0x00, 0x06, 0x0d, 0x00,
> + 0x0f);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP0,
> + 0x00, 0x00, 0x19, 0x10, 0x00, 0x0a, 0x00, 0x81);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP1,
> + 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
> + 0xc0, 0xc0, 0x18, 0x18, 0x19, 0x19, 0x18, 0x18,
> + 0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
> + 0x28, 0x28, 0x24, 0x24, 0x02, 0x03, 0x02, 0x03,
> + 0x00, 0x01, 0x00, 0x01, 0x31, 0x31, 0x31, 0x31,
> + 0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP2,
> + 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
> + 0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x19, 0x19,
> + 0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
> + 0x24, 0x24, 0x28, 0x28, 0x01, 0x00, 0x01, 0x00,
> + 0x03, 0x02, 0x03, 0x02, 0x31, 0x31, 0x31, 0x31,
> + 0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
> + 0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea,
> + 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa,
> + 0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa, 0xaa, 0xaa);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
> + 0xaa, 0x2e, 0x28, 0x00, 0x00, 0x00, 0xaa, 0x2e,
> + 0x28, 0x00, 0x00, 0x00, 0xaa, 0xee, 0xaa, 0xaa,
> + 0xaa, 0xaa, 0xaa, 0xee, 0xaa, 0xaa, 0xaa, 0xaa);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
> + 0xaa, 0xff, 0xff, 0xff, 0xff, 0xff, 0xaa, 0xff,
> + 0xff, 0xff, 0xff, 0xff);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x03);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
> + 0xaa, 0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa,
> + 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xff, 0xff, 0xff,
> + 0xff, 0xff, 0xaa, 0xff, 0xff, 0xff, 0xff, 0xff);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> + 0x0e, 0x0e, 0x1e, 0x65, 0x1c, 0x65, 0x00, 0x50,
> + 0x20, 0x20, 0x00, 0x00, 0x02, 0x02, 0x02, 0x05,
> + 0x14, 0x14, 0x32, 0xb9, 0x23, 0xb9, 0x08);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> + 0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> + 0x00, 0x00, 0x08, 0x00, 0x01, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00,
> + 0x00, 0x00, 0x00, 0x02, 0x00);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0xc3);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETCLOCK, 0xd1, 0xd6);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0x3f);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0xc6);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPTBA, 0x37);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0x3f);
> +
> + mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> + mipi_dsi_msleep(&dsi_ctx, 150);
> +
> + mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> + mipi_dsi_msleep(&dsi_ctx, 50);
> +
> + return dsi_ctx.accum_err;
> }
>
> static int hx83112a_disable(struct drm_panel *panel)
> {
> struct hx83112a_panel *ctx = to_hx83112a_panel(panel);
> 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);
> + mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
> + mipi_dsi_msleep(&dsi_ctx, 120);
>
> - return 0;
> + return dsi_ctx.accum_err;
> }
>
> static int hx83112a_prepare(struct drm_panel *panel)
> {
> struct hx83112a_panel *ctx = to_hx83112a_panel(panel);
> - struct device *dev = &ctx->dsi->dev;
> int ret;
>
> ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> - if (ret < 0) {
> - dev_err(dev, "Failed to enable regulators: %d\n", ret);
> + if (ret < 0)
> return ret;
> - }
>
> hx83112a_reset(ctx);
>
> - ret = hx83112a_on(ctx);
> + ret = hx83112a_on(ctx->dsi);
> 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;
> }
>
> - return 0;
> + return ret;
> }
>
> static int hx83112a_unprepare(struct drm_panel *panel)
> --
> 2.46.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/panel: himax-hx83112a: transition to mipi_dsi wrapped functions
2024-09-06 22:14 ` Jessica Zhang
@ 2024-09-06 22:23 ` Jessica Zhang
2024-09-07 8:32 ` Tejas Vipin
0 siblings, 1 reply; 10+ messages in thread
From: Jessica Zhang @ 2024-09-06 22:23 UTC (permalink / raw)
To: Tejas Vipin, neil.armstrong, maarten.lankhorst, mripard,
tzimmermann, airlied, daniel
Cc: dianders, dri-devel, linux-kernel
On 9/6/2024 3:14 PM, Jessica Zhang wrote:
>
>
> On 9/4/2024 7:15 AM, Tejas Vipin wrote:
>> Changes the himax-hx83112a panel to use multi style functions for
>> improved error handling.
>>
>> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
>
> Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
Hi Tejas,
Just a heads up, it seems that this might be a duplicate of this change [1]?
Thanks,
Jessica Zhang
[1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
>
>> ---
>> drivers/gpu/drm/panel/panel-himax-hx83112a.c | 297 +++++++++----------
>> 1 file changed, 136 insertions(+), 161 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/panel/panel-himax-hx83112a.c
>> b/drivers/gpu/drm/panel/panel-himax-hx83112a.c
>> index 466c27012abf..47bce087e339 100644
>> --- a/drivers/gpu/drm/panel/panel-himax-hx83112a.c
>> +++ b/drivers/gpu/drm/panel/panel-himax-hx83112a.c
>> @@ -56,198 +56,173 @@ static void hx83112a_reset(struct hx83112a_panel
>> *ctx)
>> msleep(50);
>> }
>> -static int hx83112a_on(struct hx83112a_panel *ctx)
>> +static int hx83112a_on(struct mipi_dsi_device *dsi)
>> {
>> - 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;
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETEXTC, 0x83, 0x11, 0x2a);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPOWER1,
>> - 0x08, 0x28, 0x28, 0x83, 0x83, 0x4c, 0x4f, 0x33);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDISP,
>> - 0x00, 0x02, 0x00, 0x90, 0x24, 0x00, 0x08, 0x19,
>> - 0xea, 0x11, 0x11, 0x00, 0x11, 0xa3);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDRV,
>> - 0x58, 0x68, 0x58, 0x68, 0x0f, 0xef, 0x0b, 0xc0,
>> - 0x0b, 0xc0, 0x0b, 0xc0, 0x00, 0xff, 0x00, 0xff,
>> - 0x00, 0x00, 0x14, 0x15, 0x00, 0x29, 0x11, 0x07,
>> - 0x12, 0x00, 0x29);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDRV,
>> - 0x00, 0x12, 0x12, 0x11, 0x88, 0x12, 0x12, 0x00,
>> - 0x53);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x03);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT,
>> - 0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
>> - 0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
>> - 0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
>> - 0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
>> - 0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
>> - 0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
>> - 0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
>> - 0x40);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT,
>> - 0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
>> - 0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
>> - 0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
>> - 0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
>> - 0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
>> - 0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
>> - 0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
>> - 0x40);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT,
>> - 0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
>> - 0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
>> - 0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
>> - 0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
>> - 0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
>> - 0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
>> - 0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
>> - 0x40);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT, 0x01);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTCON,
>> - 0x70, 0x00, 0x04, 0xe0, 0x33, 0x00);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPANEL, 0x08);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPOWER2, 0x2b, 0x2b);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP0,
>> - 0x80, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x08,
>> - 0x08, 0x03, 0x03, 0x22, 0x18, 0x07, 0x07, 0x07,
>> - 0x07, 0x32, 0x10, 0x06, 0x00, 0x06, 0x32, 0x10,
>> - 0x07, 0x00, 0x07, 0x32, 0x19, 0x31, 0x09, 0x31,
>> - 0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x00, 0x08,
>> - 0x09, 0x30, 0x00, 0x00, 0x00, 0x06, 0x0d, 0x00,
>> - 0x0f);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP0,
>> - 0x00, 0x00, 0x19, 0x10, 0x00, 0x0a, 0x00, 0x81);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP1,
>> - 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
>> - 0xc0, 0xc0, 0x18, 0x18, 0x19, 0x19, 0x18, 0x18,
>> - 0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
>> - 0x28, 0x28, 0x24, 0x24, 0x02, 0x03, 0x02, 0x03,
>> - 0x00, 0x01, 0x00, 0x01, 0x31, 0x31, 0x31, 0x31,
>> - 0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP2,
>> - 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
>> - 0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x19, 0x19,
>> - 0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
>> - 0x24, 0x24, 0x28, 0x28, 0x01, 0x00, 0x01, 0x00,
>> - 0x03, 0x02, 0x03, 0x02, 0x31, 0x31, 0x31, 0x31,
>> - 0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
>> - 0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea,
>> - 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa,
>> - 0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa, 0xaa, 0xaa);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
>> - 0xaa, 0x2e, 0x28, 0x00, 0x00, 0x00, 0xaa, 0x2e,
>> - 0x28, 0x00, 0x00, 0x00, 0xaa, 0xee, 0xaa, 0xaa,
>> - 0xaa, 0xaa, 0xaa, 0xee, 0xaa, 0xaa, 0xaa, 0xaa);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
>> - 0xaa, 0xff, 0xff, 0xff, 0xff, 0xff, 0xaa, 0xff,
>> - 0xff, 0xff, 0xff, 0xff);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x03);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
>> - 0xaa, 0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa,
>> - 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xff, 0xff, 0xff,
>> - 0xff, 0xff, 0xaa, 0xff, 0xff, 0xff, 0xff, 0xff);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
>> - 0x0e, 0x0e, 0x1e, 0x65, 0x1c, 0x65, 0x00, 0x50,
>> - 0x20, 0x20, 0x00, 0x00, 0x02, 0x02, 0x02, 0x05,
>> - 0x14, 0x14, 0x32, 0xb9, 0x23, 0xb9, 0x08);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
>> - 0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
>> - 0x00, 0x00, 0x08, 0x00, 0x01, 0x00, 0x00, 0x00,
>> - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00,
>> - 0x00, 0x00, 0x00, 0x02, 0x00);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0xc3);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETCLOCK, 0xd1, 0xd6);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0x3f);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0xc6);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPTBA, 0x37);
>> - mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0x3f);
>> -
>> - 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(150);
>> -
>> - 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(50);
>> -
>> - return 0;
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETEXTC, 0x83,
>> 0x11, 0x2a);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPOWER1,
>> + 0x08, 0x28, 0x28, 0x83, 0x83, 0x4c, 0x4f, 0x33);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDISP,
>> + 0x00, 0x02, 0x00, 0x90, 0x24, 0x00, 0x08, 0x19,
>> + 0xea, 0x11, 0x11, 0x00, 0x11, 0xa3);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDRV,
>> + 0x58, 0x68, 0x58, 0x68, 0x0f, 0xef, 0x0b, 0xc0,
>> + 0x0b, 0xc0, 0x0b, 0xc0, 0x00, 0xff, 0x00, 0xff,
>> + 0x00, 0x00, 0x14, 0x15, 0x00, 0x29, 0x11, 0x07,
>> + 0x12, 0x00, 0x29);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDRV,
>> + 0x00, 0x12, 0x12, 0x11, 0x88, 0x12, 0x12, 0x00,
>> + 0x53);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x03);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT,
>> + 0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
>> + 0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
>> + 0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
>> + 0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
>> + 0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
>> + 0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
>> + 0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
>> + 0x40);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT,
>> + 0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
>> + 0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
>> + 0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
>> + 0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
>> + 0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
>> + 0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
>> + 0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
>> + 0x40);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT,
>> + 0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
>> + 0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
>> + 0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
>> + 0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
>> + 0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
>> + 0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
>> + 0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
>> + 0x40);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT, 0x01);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTCON,
>> + 0x70, 0x00, 0x04, 0xe0, 0x33, 0x00);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPANEL, 0x08);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPOWER2, 0x2b,
>> 0x2b);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP0,
>> + 0x80, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x08,
>> + 0x08, 0x03, 0x03, 0x22, 0x18, 0x07, 0x07, 0x07,
>> + 0x07, 0x32, 0x10, 0x06, 0x00, 0x06, 0x32, 0x10,
>> + 0x07, 0x00, 0x07, 0x32, 0x19, 0x31, 0x09, 0x31,
>> + 0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x00, 0x08,
>> + 0x09, 0x30, 0x00, 0x00, 0x00, 0x06, 0x0d, 0x00,
>> + 0x0f);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP0,
>> + 0x00, 0x00, 0x19, 0x10, 0x00, 0x0a, 0x00, 0x81);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP1,
>> + 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
>> + 0xc0, 0xc0, 0x18, 0x18, 0x19, 0x19, 0x18, 0x18,
>> + 0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
>> + 0x28, 0x28, 0x24, 0x24, 0x02, 0x03, 0x02, 0x03,
>> + 0x00, 0x01, 0x00, 0x01, 0x31, 0x31, 0x31, 0x31,
>> + 0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP2,
>> + 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
>> + 0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x19, 0x19,
>> + 0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
>> + 0x24, 0x24, 0x28, 0x28, 0x01, 0x00, 0x01, 0x00,
>> + 0x03, 0x02, 0x03, 0x02, 0x31, 0x31, 0x31, 0x31,
>> + 0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
>> + 0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea,
>> + 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa,
>> + 0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa, 0xaa, 0xaa);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
>> + 0xaa, 0x2e, 0x28, 0x00, 0x00, 0x00, 0xaa, 0x2e,
>> + 0x28, 0x00, 0x00, 0x00, 0xaa, 0xee, 0xaa, 0xaa,
>> + 0xaa, 0xaa, 0xaa, 0xee, 0xaa, 0xaa, 0xaa, 0xaa);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
>> + 0xaa, 0xff, 0xff, 0xff, 0xff, 0xff, 0xaa, 0xff,
>> + 0xff, 0xff, 0xff, 0xff);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x03);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
>> + 0xaa, 0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa,
>> + 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xff, 0xff, 0xff,
>> + 0xff, 0xff, 0xaa, 0xff, 0xff, 0xff, 0xff, 0xff);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
>> + 0x0e, 0x0e, 0x1e, 0x65, 0x1c, 0x65, 0x00, 0x50,
>> + 0x20, 0x20, 0x00, 0x00, 0x02, 0x02, 0x02, 0x05,
>> + 0x14, 0x14, 0x32, 0xb9, 0x23, 0xb9, 0x08);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
>> + 0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
>> + 0x00, 0x00, 0x08, 0x00, 0x01, 0x00, 0x00, 0x00,
>> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00,
>> + 0x00, 0x00, 0x00, 0x02, 0x00);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0xc3);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETCLOCK, 0xd1,
>> 0xd6);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0x3f);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0xc6);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPTBA, 0x37);
>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0x3f);
>> +
>> + mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
>> + mipi_dsi_msleep(&dsi_ctx, 150);
>> +
>> + mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
>> + mipi_dsi_msleep(&dsi_ctx, 50);
>> +
>> + return dsi_ctx.accum_err;
>> }
>> static int hx83112a_disable(struct drm_panel *panel)
>> {
>> struct hx83112a_panel *ctx = to_hx83112a_panel(panel);
>> 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);
>> + mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
>> + mipi_dsi_msleep(&dsi_ctx, 120);
>> - return 0;
>> + return dsi_ctx.accum_err;
>> }
>> static int hx83112a_prepare(struct drm_panel *panel)
>> {
>> struct hx83112a_panel *ctx = to_hx83112a_panel(panel);
>> - struct device *dev = &ctx->dsi->dev;
>> int ret;
>> ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies),
>> ctx->supplies);
>> - if (ret < 0) {
>> - dev_err(dev, "Failed to enable regulators: %d\n", ret);
>> + if (ret < 0)
>> return ret;
>> - }
>> hx83112a_reset(ctx);
>> - ret = hx83112a_on(ctx);
>> + ret = hx83112a_on(ctx->dsi);
>> 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;
>> }
>> - return 0;
>> + return ret;
>> }
>> static int hx83112a_unprepare(struct drm_panel *panel)
>> --
>> 2.46.0
>>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/panel: himax-hx83112a: transition to mipi_dsi wrapped functions
2024-09-06 22:23 ` Jessica Zhang
@ 2024-09-07 8:32 ` Tejas Vipin
2024-09-10 21:19 ` Doug Anderson
0 siblings, 1 reply; 10+ messages in thread
From: Tejas Vipin @ 2024-09-07 8:32 UTC (permalink / raw)
To: Jessica Zhang, neil.armstrong, maarten.lankhorst, mripard,
tzimmermann, airlied, daniel
Cc: dianders, dri-devel, linux-kernel
On 9/7/24 3:53 AM, Jessica Zhang wrote:
>
>
> On 9/6/2024 3:14 PM, Jessica Zhang wrote:
>>
>>
>> On 9/4/2024 7:15 AM, Tejas Vipin wrote:
>>> Changes the himax-hx83112a panel to use multi style functions for
>>> improved error handling.
>>>
>>> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
>>
>> Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
>
> Hi Tejas,
>
> Just a heads up, it seems that this might be a duplicate of this change [1]?
>
> Thanks,
>
> Jessica Zhang
>
> [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
Ah, thanks for letting me know. I hadn't realized someone else had
started working on this too.
However, I would argue that my patch [2] is a better candidate for merging
because of the following reasons:
1) Removes unnecessary error printing:
The mipi_dsi_*_multi() functions all have inbuilt error printing which
makes printing errors after hx83112a_on unnecessary as is addressed in
[2] like so:
> - ret = hx83112a_on(ctx);
> + ret = hx83112a_on(ctx->dsi);
> 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;
> }
[2] also removes the unnecessary dev_err after regulator_bulk_enable as was
addressed in [3] like so:
> ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> - if (ret < 0) {
> - dev_err(dev, "Failed to enable regulators: %d\n", ret);
> + if (ret < 0)
> return ret;
> - }
2) Better formatting
The mipi_dsi_dcs_write_seq_multi statements in [1] aren't formatted
quite right according to what has been done so far. They are written as
such in [1]:
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> 0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
Where they should be written as such in [2]:
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> + 0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
All in all, the module generated using my patch ends up being a teensy
bit smaller (1% smaller). But if chronology is what is important, then
it would at least be nice to see the above changes as part of Abhishek's
patch too. And I'll be sure to look at the mail in the drm inbox now
onwards :p
[1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
[2] https://lore.kernel.org/all/20240904141521.554451-1-tejasvipin76@gmail.com/
[3] https://lore.kernel.org/all/CAD=FV=XRZKL_ppjUKDK61fQkWhHiQCJLfmVBS7wSo4sUux2g8Q@mail.gmail.com/
--
Tejas Vipin
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/panel: himax-hx83112a: transition to mipi_dsi wrapped functions
2024-09-07 8:32 ` Tejas Vipin
@ 2024-09-10 21:19 ` Doug Anderson
2024-09-11 7:41 ` neil.armstrong
2024-09-12 0:26 ` Abhishek Tamboli
0 siblings, 2 replies; 10+ messages in thread
From: Doug Anderson @ 2024-09-10 21:19 UTC (permalink / raw)
To: Tejas Vipin, neil.armstrong, Abhishek Tamboli
Cc: Jessica Zhang, maarten.lankhorst, mripard, tzimmermann, airlied,
daniel, dri-devel, linux-kernel, Javier Martinez Canillas
Hi,
On Sat, Sep 7, 2024 at 1:32 AM Tejas Vipin <tejasvipin76@gmail.com> wrote:
>
> On 9/7/24 3:53 AM, Jessica Zhang wrote:
> >
> >
> > On 9/6/2024 3:14 PM, Jessica Zhang wrote:
> >>
> >>
> >> On 9/4/2024 7:15 AM, Tejas Vipin wrote:
> >>> Changes the himax-hx83112a panel to use multi style functions for
> >>> improved error handling.
> >>>
> >>> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
> >>
> >> Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> >
> > Hi Tejas,
> >
> > Just a heads up, it seems that this might be a duplicate of this change [1]?
> >
> > Thanks,
> >
> > Jessica Zhang
> >
> > [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
>
> Ah, thanks for letting me know. I hadn't realized someone else had
> started working on this too.
>
> However, I would argue that my patch [2] is a better candidate for merging
> because of the following reasons:
>
> 1) Removes unnecessary error printing:
> The mipi_dsi_*_multi() functions all have inbuilt error printing which
> makes printing errors after hx83112a_on unnecessary as is addressed in
> [2] like so:
>
> > - ret = hx83112a_on(ctx);
> > + ret = hx83112a_on(ctx->dsi);
> > 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;
> > }
>
> [2] also removes the unnecessary dev_err after regulator_bulk_enable as was
> addressed in [3] like so:
>
> > ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> > - if (ret < 0) {
> > - dev_err(dev, "Failed to enable regulators: %d\n", ret);
> > + if (ret < 0)
> > return ret;
> > - }
>
> 2) Better formatting
>
> The mipi_dsi_dcs_write_seq_multi statements in [1] aren't formatted
> quite right according to what has been done so far. They are written as
> such in [1]:
>
> > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> > 0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
>
> Where they should be written as such in [2]:
>
> > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> > + 0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
>
> All in all, the module generated using my patch ends up being a teensy
> bit smaller (1% smaller). But if chronology is what is important, then
> it would at least be nice to see the above changes as part of Abhishek's
> patch too. And I'll be sure to look at the mail in the drm inbox now
> onwards :p
>
> [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
> [2] https://lore.kernel.org/all/20240904141521.554451-1-tejasvipin76@gmail.com/
> [3] https://lore.kernel.org/all/CAD=FV=XRZKL_ppjUKDK61fQkWhHiQCJLfmVBS7wSo4sUux2g8Q@mail.gmail.com/
I would tend to agree that Tejas's patch looks slightly better, but
Abhishek's patch appears to have been posted first.
Neil: any idea what to do here? Maybe a Co-Developed-by or something?
...or we could land Abhishek and Tejas could post a followup for the
extra cleanup?
Abhishek: are you planning to post more _multi cleanups? If so, please
make sure to CC Tejas (who has been posting a bunch of them) and
perhaps me since I've been helping to review them a bit.
-Doug
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/panel: himax-hx83112a: transition to mipi_dsi wrapped functions
2024-09-10 21:19 ` Doug Anderson
@ 2024-09-11 7:41 ` neil.armstrong
2024-09-11 20:07 ` Doug Anderson
2024-09-12 0:32 ` Abhishek Tamboli
2024-09-12 0:26 ` Abhishek Tamboli
1 sibling, 2 replies; 10+ messages in thread
From: neil.armstrong @ 2024-09-11 7:41 UTC (permalink / raw)
To: Doug Anderson, Tejas Vipin, Abhishek Tamboli
Cc: Jessica Zhang, maarten.lankhorst, mripard, tzimmermann, airlied,
daniel, dri-devel, linux-kernel, Javier Martinez Canillas
On 10/09/2024 23:19, Doug Anderson wrote:
> Hi,
>
> On Sat, Sep 7, 2024 at 1:32 AM Tejas Vipin <tejasvipin76@gmail.com> wrote:
>>
>> On 9/7/24 3:53 AM, Jessica Zhang wrote:
>>>
>>>
>>> On 9/6/2024 3:14 PM, Jessica Zhang wrote:
>>>>
>>>>
>>>> On 9/4/2024 7:15 AM, Tejas Vipin wrote:
>>>>> Changes the himax-hx83112a panel to use multi style functions for
>>>>> improved error handling.
>>>>>
>>>>> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
>>>>
>>>> Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
>>>
>>> Hi Tejas,
>>>
>>> Just a heads up, it seems that this might be a duplicate of this change [1]?
>>>
>>> Thanks,
>>>
>>> Jessica Zhang
>>>
>>> [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
>>
>> Ah, thanks for letting me know. I hadn't realized someone else had
>> started working on this too.
>>
>> However, I would argue that my patch [2] is a better candidate for merging
>> because of the following reasons:
>>
>> 1) Removes unnecessary error printing:
>> The mipi_dsi_*_multi() functions all have inbuilt error printing which
>> makes printing errors after hx83112a_on unnecessary as is addressed in
>> [2] like so:
>>
>>> - ret = hx83112a_on(ctx);
>>> + ret = hx83112a_on(ctx->dsi);
>>> 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;
>>> }
>>
>> [2] also removes the unnecessary dev_err after regulator_bulk_enable as was
>> addressed in [3] like so:
>>
>>> ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
>>> - if (ret < 0) {
>>> - dev_err(dev, "Failed to enable regulators: %d\n", ret);
>>> + if (ret < 0)
>>> return ret;
>>> - }
>>
>> 2) Better formatting
>>
>> The mipi_dsi_dcs_write_seq_multi statements in [1] aren't formatted
>> quite right according to what has been done so far. They are written as
>> such in [1]:
>>
>>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
>>> 0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
>>
>> Where they should be written as such in [2]:
>>
>>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
>>> + 0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
>>
>> All in all, the module generated using my patch ends up being a teensy
>> bit smaller (1% smaller). But if chronology is what is important, then
>> it would at least be nice to see the above changes as part of Abhishek's
>> patch too. And I'll be sure to look at the mail in the drm inbox now
>> onwards :p
>>
>> [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
>> [2] https://lore.kernel.org/all/20240904141521.554451-1-tejasvipin76@gmail.com/
>> [3] https://lore.kernel.org/all/CAD=FV=XRZKL_ppjUKDK61fQkWhHiQCJLfmVBS7wSo4sUux2g8Q@mail.gmail.com/
>
> I would tend to agree that Tejas's patch looks slightly better, but
> Abhishek's patch appears to have been posted first.
>
> Neil: any idea what to do here? Maybe a Co-Developed-by or something?
> ...or we could land Abhishek and Tejas could post a followup for the
> extra cleanup?
Yeah usually I take the first one when they are equal, but indeed Tejas
cleanup up a little further and better aligned the parameters so I think
Tejas's one is a better looking version.
In this case we should apply Teja's one, nothing personal Abhishek!
>
> Abhishek: are you planning to post more _multi cleanups? If so, please
> make sure to CC Tejas (who has been posting a bunch of them) and
> perhaps me since I've been helping to review them a bit.
>
> -Doug
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/panel: himax-hx83112a: transition to mipi_dsi wrapped functions
2024-09-11 7:41 ` neil.armstrong
@ 2024-09-11 20:07 ` Doug Anderson
2024-09-12 0:32 ` Abhishek Tamboli
1 sibling, 0 replies; 10+ messages in thread
From: Doug Anderson @ 2024-09-11 20:07 UTC (permalink / raw)
To: neil.armstrong
Cc: Tejas Vipin, Abhishek Tamboli, Jessica Zhang, maarten.lankhorst,
mripard, tzimmermann, airlied, daniel, dri-devel, linux-kernel,
Javier Martinez Canillas
Hi,
On Wed, Sep 11, 2024 at 12:41 AM <neil.armstrong@linaro.org> wrote:
>
> On 10/09/2024 23:19, Doug Anderson wrote:
> > Hi,
> >
> > On Sat, Sep 7, 2024 at 1:32 AM Tejas Vipin <tejasvipin76@gmail.com> wrote:
> >>
> >> On 9/7/24 3:53 AM, Jessica Zhang wrote:
> >>>
> >>>
> >>> On 9/6/2024 3:14 PM, Jessica Zhang wrote:
> >>>>
> >>>>
> >>>> On 9/4/2024 7:15 AM, Tejas Vipin wrote:
> >>>>> Changes the himax-hx83112a panel to use multi style functions for
> >>>>> improved error handling.
> >>>>>
> >>>>> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
> >>>>
> >>>> Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> >>>
> >>> Hi Tejas,
> >>>
> >>> Just a heads up, it seems that this might be a duplicate of this change [1]?
> >>>
> >>> Thanks,
> >>>
> >>> Jessica Zhang
> >>>
> >>> [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
> >>
> >> Ah, thanks for letting me know. I hadn't realized someone else had
> >> started working on this too.
> >>
> >> However, I would argue that my patch [2] is a better candidate for merging
> >> because of the following reasons:
> >>
> >> 1) Removes unnecessary error printing:
> >> The mipi_dsi_*_multi() functions all have inbuilt error printing which
> >> makes printing errors after hx83112a_on unnecessary as is addressed in
> >> [2] like so:
> >>
> >>> - ret = hx83112a_on(ctx);
> >>> + ret = hx83112a_on(ctx->dsi);
> >>> 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;
> >>> }
> >>
> >> [2] also removes the unnecessary dev_err after regulator_bulk_enable as was
> >> addressed in [3] like so:
> >>
> >>> ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> >>> - if (ret < 0) {
> >>> - dev_err(dev, "Failed to enable regulators: %d\n", ret);
> >>> + if (ret < 0)
> >>> return ret;
> >>> - }
> >>
> >> 2) Better formatting
> >>
> >> The mipi_dsi_dcs_write_seq_multi statements in [1] aren't formatted
> >> quite right according to what has been done so far. They are written as
> >> such in [1]:
> >>
> >>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> >>> 0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
> >>
> >> Where they should be written as such in [2]:
> >>
> >>> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> >>> + 0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
> >>
> >> All in all, the module generated using my patch ends up being a teensy
> >> bit smaller (1% smaller). But if chronology is what is important, then
> >> it would at least be nice to see the above changes as part of Abhishek's
> >> patch too. And I'll be sure to look at the mail in the drm inbox now
> >> onwards :p
> >>
> >> [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
> >> [2] https://lore.kernel.org/all/20240904141521.554451-1-tejasvipin76@gmail.com/
> >> [3] https://lore.kernel.org/all/CAD=FV=XRZKL_ppjUKDK61fQkWhHiQCJLfmVBS7wSo4sUux2g8Q@mail.gmail.com/
> >
> > I would tend to agree that Tejas's patch looks slightly better, but
> > Abhishek's patch appears to have been posted first.
> >
> > Neil: any idea what to do here? Maybe a Co-Developed-by or something?
> > ...or we could land Abhishek and Tejas could post a followup for the
> > extra cleanup?
>
> Yeah usually I take the first one when they are equal, but indeed Tejas
> cleanup up a little further and better aligned the parameters so I think
> Tejas's one is a better looking version.
>
> In this case we should apply Teja's one, nothing personal Abhishek!
Pushed to drm-misc-next:
[1/1] drm/panel: himax-hx83112a: transition to mipi_dsi wrapped functions
commit: 32e5666b8a4d0f2aee39a0b2f8386cf9f86a8225
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/panel: himax-hx83112a: transition to mipi_dsi wrapped functions
2024-09-10 21:19 ` Doug Anderson
2024-09-11 7:41 ` neil.armstrong
@ 2024-09-12 0:26 ` Abhishek Tamboli
1 sibling, 0 replies; 10+ messages in thread
From: Abhishek Tamboli @ 2024-09-12 0:26 UTC (permalink / raw)
To: Doug Anderson
Cc: tejasvipin76, neil.armstrong, quic_jesszhan, maarten.lankhorst,
mripard, tzimmermann, airlied, daniel, dri-devel, linux-kernel,
javierm
On Tue, Sep 10, 2024 at 02:19:53PM -0700, Doug Anderson wrote:
> Hi,
>
> On Sat, Sep 7, 2024 at 1:32 AM Tejas Vipin <tejasvipin76@gmail.com> wrote:
> >
> > On 9/7/24 3:53 AM, Jessica Zhang wrote:
> > >
> > >
> > > On 9/6/2024 3:14 PM, Jessica Zhang wrote:
> > >>
> > >>
> > >> On 9/4/2024 7:15 AM, Tejas Vipin wrote:
> > >>> Changes the himax-hx83112a panel to use multi style functions for
> > >>> improved error handling.
> > >>>
> > >>> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
> > >>
> > >> Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> > >
> > > Hi Tejas,
> > >
> > > Just a heads up, it seems that this might be a duplicate of this change [1]?
> > >
> > > Thanks,
> > >
> > > Jessica Zhang
> > >
> > > [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
> >
> > Ah, thanks for letting me know. I hadn't realized someone else had
> > started working on this too.
> >
> > However, I would argue that my patch [2] is a better candidate for merging
> > because of the following reasons:
> >
> > 1) Removes unnecessary error printing:
> > The mipi_dsi_*_multi() functions all have inbuilt error printing which
> > makes printing errors after hx83112a_on unnecessary as is addressed in
> > [2] like so:
> >
> > > - ret = hx83112a_on(ctx);
> > > + ret = hx83112a_on(ctx->dsi);
> > > 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;
> > > }
> >
> > [2] also removes the unnecessary dev_err after regulator_bulk_enable as was
> > addressed in [3] like so:
> >
> > > ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> > > - if (ret < 0) {
> > > - dev_err(dev, "Failed to enable regulators: %d\n", ret);
> > > + if (ret < 0)
> > > return ret;
> > > - }
> >
> > 2) Better formatting
> >
> > The mipi_dsi_dcs_write_seq_multi statements in [1] aren't formatted
> > quite right according to what has been done so far. They are written as
> > such in [1]:
> >
> > > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> > > 0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
> >
> > Where they should be written as such in [2]:
> >
> > > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> > > + 0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
> >
> > All in all, the module generated using my patch ends up being a teensy
> > bit smaller (1% smaller). But if chronology is what is important, then
> > it would at least be nice to see the above changes as part of Abhishek's
> > patch too. And I'll be sure to look at the mail in the drm inbox now
> > onwards :p
> >
> > [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
> > [2] https://lore.kernel.org/all/20240904141521.554451-1-tejasvipin76@gmail.com/
> > [3] https://lore.kernel.org/all/CAD=FV=XRZKL_ppjUKDK61fQkWhHiQCJLfmVBS7wSo4sUux2g8Q@mail.gmail.com/
>
> I would tend to agree that Tejas's patch looks slightly better, but
> Abhishek's patch appears to have been posted first.
>
> Neil: any idea what to do here? Maybe a Co-Developed-by or something?
> ...or we could land Abhishek and Tejas could post a followup for the
> extra cleanup?
>
> Abhishek: are you planning to post more _multi cleanups?
I’ve just started learning about the DRM subsystem, I don’t have specific
plans for _multi cleanups at the moment. However, I would be happy
to contribute further.
>If so, please
> make sure to CC Tejas (who has been posting a bunch of them) and
> perhaps me since I've been helping to review them a bit.
Yes, I’ll make sure to CC Tejas and you.
Regards,
ABhishek
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/panel: himax-hx83112a: transition to mipi_dsi wrapped functions
2024-09-11 7:41 ` neil.armstrong
2024-09-11 20:07 ` Doug Anderson
@ 2024-09-12 0:32 ` Abhishek Tamboli
1 sibling, 0 replies; 10+ messages in thread
From: Abhishek Tamboli @ 2024-09-12 0:32 UTC (permalink / raw)
To: neil.armstrong
Cc: dianders, tejasvipin76, quic_jesszhan, maarten.lankhorst,
mripard, tzimmermann, airlied, daniel, dri-devel, linux-kernel,
javierm
On Wed, Sep 11, 2024 at 09:41:30AM +0200, neil.armstrong@linaro.org wrote:
> On 10/09/2024 23:19, Doug Anderson wrote:
> > Hi,
> >
> > On Sat, Sep 7, 2024 at 1:32 AM Tejas Vipin <tejasvipin76@gmail.com> wrote:
> > >
> > > On 9/7/24 3:53 AM, Jessica Zhang wrote:
> > > >
> > > >
> > > > On 9/6/2024 3:14 PM, Jessica Zhang wrote:
> > > > >
> > > > >
> > > > > On 9/4/2024 7:15 AM, Tejas Vipin wrote:
> > > > > > Changes the himax-hx83112a panel to use multi style functions for
> > > > > > improved error handling.
> > > > > >
> > > > > > Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
> > > > >
> > > > > Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> > > >
> > > > Hi Tejas,
> > > >
> > > > Just a heads up, it seems that this might be a duplicate of this change [1]?
> > > >
> > > > Thanks,
> > > >
> > > > Jessica Zhang
> > > >
> > > > [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
> > >
> > > Ah, thanks for letting me know. I hadn't realized someone else had
> > > started working on this too.
> > >
> > > However, I would argue that my patch [2] is a better candidate for merging
> > > because of the following reasons:
> > >
> > > 1) Removes unnecessary error printing:
> > > The mipi_dsi_*_multi() functions all have inbuilt error printing which
> > > makes printing errors after hx83112a_on unnecessary as is addressed in
> > > [2] like so:
> > >
> > > > - ret = hx83112a_on(ctx);
> > > > + ret = hx83112a_on(ctx->dsi);
> > > > 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;
> > > > }
> > >
> > > [2] also removes the unnecessary dev_err after regulator_bulk_enable as was
> > > addressed in [3] like so:
> > >
> > > > ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> > > > - if (ret < 0) {
> > > > - dev_err(dev, "Failed to enable regulators: %d\n", ret);
> > > > + if (ret < 0)
> > > > return ret;
> > > > - }
> > >
> > > 2) Better formatting
> > >
> > > The mipi_dsi_dcs_write_seq_multi statements in [1] aren't formatted
> > > quite right according to what has been done so far. They are written as
> > > such in [1]:
> > >
> > > > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> > > > 0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
> > >
> > > Where they should be written as such in [2]:
> > >
> > > > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> > > > + 0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
> > >
> > > All in all, the module generated using my patch ends up being a teensy
> > > bit smaller (1% smaller). But if chronology is what is important, then
> > > it would at least be nice to see the above changes as part of Abhishek's
> > > patch too. And I'll be sure to look at the mail in the drm inbox now
> > > onwards :p
> > >
> > > [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
> > > [2] https://lore.kernel.org/all/20240904141521.554451-1-tejasvipin76@gmail.com/
> > > [3] https://lore.kernel.org/all/CAD=FV=XRZKL_ppjUKDK61fQkWhHiQCJLfmVBS7wSo4sUux2g8Q@mail.gmail.com/
> >
> > I would tend to agree that Tejas's patch looks slightly better, but
> > Abhishek's patch appears to have been posted first.
> >
> > Neil: any idea what to do here? Maybe a Co-Developed-by or something?
> > ...or we could land Abhishek and Tejas could post a followup for the
> > extra cleanup?
>
> Yeah usually I take the first one when they are equal, but indeed Tejas
> cleanup up a little further and better aligned the parameters so I think
> Tejas's one is a better looking version.
>
> In this case we should apply Teja's one, nothing personal Abhishek!
No problem at all, I completely understand. It makes sense to go with
Tejas's version.
Thanks for letting me know, and I appreciate the feedback!
Regards,
Abhishek
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-09-12 0:32 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-09-04 14:15 [PATCH] drm/panel: himax-hx83112a: transition to mipi_dsi wrapped functions Tejas Vipin
2024-09-04 17:52 ` Doug Anderson
2024-09-06 22:14 ` Jessica Zhang
2024-09-06 22:23 ` Jessica Zhang
2024-09-07 8:32 ` Tejas Vipin
2024-09-10 21:19 ` Doug Anderson
2024-09-11 7:41 ` neil.armstrong
2024-09-11 20:07 ` Doug Anderson
2024-09-12 0:32 ` Abhishek Tamboli
2024-09-12 0:26 ` Abhishek Tamboli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®