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);
