On Wed, 2019-04-03 at 10:11 -0700, Joe Perches wrote:
On Wed, 2019-04-03 at 18:17 +0200, Thierry Reding wrote:
On Mon, Apr 01, 2019 at 12:35:32PM +0200, Guido Günther wrote:
v4 fixes up the DT binding example and uses a wider cc list since I failed to extend that when touching more files.
[]
Applied, thanks.
checkpatch does complain about the dsi_generic_write_seq() macro definition, because it uses flow control statements, but there are already similar macros in other drivers, so I let this slide. We may want to eventually come up with something better and then replace these macros for the other drivers as well.
Dunno about the other drivers, but the mechanism isn't particularly nice as it separates the init identifier from the data being written.
It might be better to use something like a struct for each command and a for loop for each block of commands.
Also the 0xBF value used in one of the init sequence writes does not have an identifier #define in the 'Manufacturer specific Commands send via DSI' block which is odd.
Perhaps something like this below (though adding a bunch of lines to avoid a macro goto isn't great) It does seem to read a bit better to me though. --- drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c | 210 +++++++++++++-------- 1 file changed, 136 insertions(+), 74 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c index 158a6d548068..7862863db5f7 100644 --- a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c +++ b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c @@ -20,27 +20,6 @@
#define DRV_NAME "panel-rocktech-jh057n00900"
-/* Manufacturer specific Commands send via DSI */ -#define ST7703_CMD_ALL_PIXEL_OFF 0x22 -#define ST7703_CMD_ALL_PIXEL_ON 0x23 -#define ST7703_CMD_SETDISP 0xB2 -#define ST7703_CMD_SETRGBIF 0xB3 -#define ST7703_CMD_SETCYC 0xB4 -#define ST7703_CMD_SETBGP 0xB5 -#define ST7703_CMD_SETVCOM 0xB6 -#define ST7703_CMD_SETOTP 0xB7 -#define ST7703_CMD_SETPOWER_EXT 0xB8 -#define ST7703_CMD_SETEXTC 0xB9 -#define ST7703_CMD_SETMIPI 0xBA -#define ST7703_CMD_SETVDC 0xBC -#define ST7703_CMD_SETSCR 0xC0 -#define ST7703_CMD_SETPOWER 0xC1 -#define ST7703_CMD_SETPANEL 0xCC -#define ST7703_CMD_SETGAMMA 0xE0 -#define ST7703_CMD_SETEQ 0xE3 -#define ST7703_CMD_SETGIP1 0xE9 -#define ST7703_CMD_SETGIP2 0xEA - struct jh057n { struct device *dev; struct drm_panel panel; @@ -51,75 +30,153 @@ struct jh057n { struct dentry *debugfs; };
+struct st7703_cmd { + const size_t size; + const u8 data[]; +}; + +#define st7703_cmd_data(cmd, ...) \ + .size = 1 + (sizeof((u8[]){__VA_ARGS__})/sizeof(u8)), \ + .data = {cmd, __VA_ARGS__} + +/* Manufacturer specific Commands send via DSI */ +static const struct st7703_cmd ST7703_CMD_ALL_PIXEL_OFF = { + st7703_cmd_data(0x22) +}; +static const struct st7703_cmd ST7703_CMD_ALL_PIXEL_ON = { + st7703_cmd_data(0x23) +}; +static const struct st7703_cmd ST7703_CMD_SETDISP = { + st7703_cmd_data(0xB2, + 0xF0, 0x12, 0x30) +}; +static const struct st7703_cmd ST7703_CMD_SETRGBIF = { + st7703_cmd_data(0xB3, + 0x10, 0x10, 0x05, 0x05, 0x03, 0xFF, 0x00, 0x00, + 0x00, 0x00) +}; +static const struct st7703_cmd ST7703_CMD_SETCYC = { + st7703_cmd_data(0xB4, + 0x80) +}; +static const struct st7703_cmd ST7703_CMD_SETBGP = { + st7703_cmd_data(0xB5, + 0x08, 0x08) +}; +static const struct st7703_cmd ST7703_CMD_SETVCOM = { + st7703_cmd_data(0xB6, + 0x3F, 0x3F) +}; +static const struct st7703_cmd ST7703_CMD_SETOTP = { + st7703_cmd_data(0xB7) +}; +static const struct st7703_cmd ST7703_CMD_SETPOWER_EXT = { + st7703_cmd_data(0xB8) +}; +static const struct st7703_cmd ST7703_CMD_SETEXTC = { + st7703_cmd_data(0xB9, + 0xF1, 0x12, 0x83) +}; +static const struct st7703_cmd ST7703_CMD_SETMIPI = { + st7703_cmd_data(0xBA) +}; +static const struct st7703_cmd ST7703_CMD_SETVDC = { + st7703_cmd_data(0xBC, + 0x4E) +}; +static const struct st7703_cmd ST7703_CMD_SETSCR = { + st7703_cmd_data(0xC0, + 0x73, 0x73, 0x50, 0x50, 0x00, 0x00, 0x08, 0x70, + 0x00) +}; +static const struct st7703_cmd ST7703_CMD_SETPOWER = { + st7703_cmd_data(0xC1) +}; +static const struct st7703_cmd ST7703_CMD_SETPANEL = { + st7703_cmd_data(0xCC, + 0x0B) +}; +static const struct st7703_cmd ST7703_CMD_SETGAMMA = { + st7703_cmd_data(0xE0, + 0x00, 0x09, 0x0E, 0x29, 0x2D, 0x3C, 0x41, 0x37, + 0x07, 0x0B, 0x0D, 0x10, 0x11, 0x0F, 0x10, 0x11, + 0x18, 0x00, 0x09, 0x0E, 0x29, 0x2D, 0x3C, 0x41, + 0x37, 0x07, 0x0B, 0x0D, 0x10, 0x11, 0x0F, 0x10, + 0x11, 0x18) +}; +static const struct st7703_cmd ST7703_CMD_SETEQ = { + st7703_cmd_data(0xE3, + 0x07, 0x07, 0x0B, 0x0B, 0x03, 0x0B, 0x00, 0x00, + 0x00, 0x00, 0xFF, 0x00, 0xC0, 0x10) +}; +static const struct st7703_cmd ST7703_CMD_SETGIP1 = { + st7703_cmd_data(0xE9, + 0x82, 0x10, 0x06, 0x05, 0x9E, 0x0A, 0xA5, 0x12, + 0x31, 0x23, 0x37, 0x83, 0x04, 0xBC, 0x27, 0x38, + 0x0C, 0x00, 0x03, 0x00, 0x00, 0x00, 0x0C, 0x00, + 0x03, 0x00, 0x00, 0x00, 0x75, 0x75, 0x31, 0x88, + 0x88, 0x88, 0x88, 0x88, 0x88, 0x13, 0x88, 0x64, + 0x64, 0x20, 0x88, 0x88, 0x88, 0x88, 0x88, 0x88, + 0x02, 0x88, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00) +}; + +static const struct st7703_cmd ST7703_CMD_SETGIP2 = { + st7703_cmd_data(0xEA, + 0x02, 0x21, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x02, 0x46, 0x02, 0x88, + 0x88, 0x88, 0x88, 0x88, 0x88, 0x64, 0x88, 0x13, + 0x57, 0x13, 0x88, 0x88, 0x88, 0x88, 0x88, 0x88, + 0x75, 0x88, 0x23, 0x14, 0x00, 0x00, 0x02, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x30, 0x0A, + 0xA5, 0x00, 0x00, 0x00, 0x00) +}; + static inline struct jh057n *panel_to_jh057n(struct drm_panel *panel) { return container_of(panel, struct jh057n, panel); }
-#define dsi_generic_write_seq(dsi, seq...) do { \ - static const u8 d[] = { seq }; \ - int ret; \ - ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d)); \ - if (ret < 0) \ - return ret; \ - } while (0) - static int jh057n_init_sequence(struct jh057n *ctx) { struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev); struct device *dev = ctx->dev; + int i; int ret; + static const struct { + const struct st7703_cmd *cmd; + int msleep; + } st7703_init[] = { + {&ST7703_CMD_SETEXTC, 0}, + {&ST7703_CMD_SETRGBIF, 0}, + {&ST7703_CMD_SETSCR, 0}, + {&ST7703_CMD_SETVDC, 0}, + {&ST7703_CMD_SETPANEL, 0}, + {&ST7703_CMD_SETCYC, 0}, + {&ST7703_CMD_SETDISP, 0}, + {&ST7703_CMD_SETEQ, 0}, + {&ST7703_CMD_SETBGP, 20}, + {&ST7703_CMD_SETVCOM, 0}, + {&ST7703_CMD_SETGIP1, 0}, + {&ST7703_CMD_SETGIP2, 0}, + {&ST7703_CMD_SETGAMMA, 20}, +};
/* * Init sequence was supplied by the panel vendor. Most of the commands * resemble the ST7703 but the number of parameters often don't match * so it's likely a clone. */ - dsi_generic_write_seq(dsi, ST7703_CMD_SETEXTC, - 0xF1, 0x12, 0x83); - dsi_generic_write_seq(dsi, ST7703_CMD_SETRGBIF, - 0x10, 0x10, 0x05, 0x05, 0x03, 0xFF, 0x00, 0x00, - 0x00, 0x00); - dsi_generic_write_seq(dsi, ST7703_CMD_SETSCR, - 0x73, 0x73, 0x50, 0x50, 0x00, 0x00, 0x08, 0x70, - 0x00); - dsi_generic_write_seq(dsi, ST7703_CMD_SETVDC, 0x4E); - dsi_generic_write_seq(dsi, ST7703_CMD_SETPANEL, 0x0B); - dsi_generic_write_seq(dsi, ST7703_CMD_SETCYC, 0x80); - dsi_generic_write_seq(dsi, ST7703_CMD_SETDISP, 0xF0, 0x12, 0x30); - dsi_generic_write_seq(dsi, ST7703_CMD_SETEQ, - 0x07, 0x07, 0x0B, 0x0B, 0x03, 0x0B, 0x00, 0x00, - 0x00, 0x00, 0xFF, 0x00, 0xC0, 0x10); - dsi_generic_write_seq(dsi, ST7703_CMD_SETBGP, 0x08, 0x08); - msleep(20);
- dsi_generic_write_seq(dsi, ST7703_CMD_SETVCOM, 0x3F, 0x3F); - dsi_generic_write_seq(dsi, 0xBF, 0x02, 0x11, 0x00); - dsi_generic_write_seq(dsi, ST7703_CMD_SETGIP1, - 0x82, 0x10, 0x06, 0x05, 0x9E, 0x0A, 0xA5, 0x12, - 0x31, 0x23, 0x37, 0x83, 0x04, 0xBC, 0x27, 0x38, - 0x0C, 0x00, 0x03, 0x00, 0x00, 0x00, 0x0C, 0x00, - 0x03, 0x00, 0x00, 0x00, 0x75, 0x75, 0x31, 0x88, - 0x88, 0x88, 0x88, 0x88, 0x88, 0x13, 0x88, 0x64, - 0x64, 0x20, 0x88, 0x88, 0x88, 0x88, 0x88, 0x88, - 0x02, 0x88, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00); - dsi_generic_write_seq(dsi, ST7703_CMD_SETGIP2, - 0x02, 0x21, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x02, 0x46, 0x02, 0x88, - 0x88, 0x88, 0x88, 0x88, 0x88, 0x64, 0x88, 0x13, - 0x57, 0x13, 0x88, 0x88, 0x88, 0x88, 0x88, 0x88, - 0x75, 0x88, 0x23, 0x14, 0x00, 0x00, 0x02, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x30, 0x0A, - 0xA5, 0x00, 0x00, 0x00, 0x00); - dsi_generic_write_seq(dsi, ST7703_CMD_SETGAMMA, - 0x00, 0x09, 0x0E, 0x29, 0x2D, 0x3C, 0x41, 0x37, - 0x07, 0x0B, 0x0D, 0x10, 0x11, 0x0F, 0x10, 0x11, - 0x18, 0x00, 0x09, 0x0E, 0x29, 0x2D, 0x3C, 0x41, - 0x37, 0x07, 0x0B, 0x0D, 0x10, 0x11, 0x0F, 0x10, - 0x11, 0x18); - msleep(20); + for (i = 0; i < ARRAY_SIZE(st7703_init); i++) { + ret = mipi_dsi_generic_write(dsi, st7703_init[i].cmd->data, + st7703_init[i].cmd->size); + if (ret < 0) + return ret; + if (st7703_init[i].msleep) + msleep(st7703_init[i].msleep); + }
ret = mipi_dsi_dcs_exit_sleep_mode(dsi); if (ret < 0) { @@ -240,9 +297,14 @@ static int allpixelson_set(void *data, u64 val) { struct jh057n *ctx = data; struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev); + int ret;
DRM_DEV_DEBUG_DRIVER(ctx->dev, "Setting all pixels on"); - dsi_generic_write_seq(dsi, ST7703_CMD_ALL_PIXEL_ON); + ret = mipi_dsi_generic_write(dsi, ST7703_CMD_ALL_PIXEL_ON.data, + ST7703_CMD_ALL_PIXEL_ON.size); + if (ret < 0) \ + return ret; \ + msleep(val * 1000); /* Reset the panel to get video back */ drm_panel_disable(&ctx->panel);