Hi, On Thu, Jun 20, 2024 at 1:05 AM Zhaoxiong Lv <lvzhaoxiong@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote: > > Currently, the init_code of the jd9365da driver is placed > in the enable() function and sent, but this seems to take > a long time. It takes 17ms to send each instruction (an init > code consists of about 200 instructions), so it takes > about 3.5s to send the init_code. So we moved the sending > of the inti_code to the prepare() function, and each > instruction seemed to take only 25μs. > > We checked the DSI host and found that the difference in > command sending time is caused by the different modes of > the DSI host in prepare() and enable() functions. > Our DSI Host only supports sending cmd in LP mode, The > prepare() function can directly send init_code (LP->cmd) > in LP mode, but the enable() function is in HS mode and > needs to switch to LP mode before sending init code > (HS->LP->cmd->HS). Therefore, it takes longer to send > the command. > > Signed-off-by: Zhaoxiong Lv <lvzhaoxiong@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> > --- > > Changes between V4 and V3: > - 1. Only move mipi_dsi_dcs_write_buffer from enable() function to prepare() function, > - and no longer use mipi_dsi_dcs_write_seq_multi. > > V3:https://lore.kernel.org/all/20240614145510.22965-2-lvzhaoxiong@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/ > > --- > .../gpu/drm/panel/panel-jadard-jd9365da-h3.c | 24 +++++++++---------- > 1 file changed, 11 insertions(+), 13 deletions(-) This seems reasonable to me, but I'd prefer someone with more MIPI experience than me to actually give it a good review to make sure that the argument about things being in prepare() vs enable() makes sense to them. Actually, that makes me look at your CC list, which seems to be missing some important people. You should have run `./scripts/get_maintainer.pl` and used that to help you make your CC list, but you're missing some important people including the maintainer of this driver and the panel maintainer. I've added a few of those people to this patch. If you send future versions please include folks that get_maintainer points out. > @@ -117,7 +107,15 @@ static int jadard_prepare(struct drm_panel *panel) > msleep(10); > > gpiod_set_value(jadard->reset, 1); > - msleep(120); > + msleep(130); > + > + for (i = 0; i < desc->num_init_cmds; i++) { > + const struct jadard_init_cmd *cmd = &desc->init_cmds[i]; > + > + ret = mipi_dsi_dcs_write_buffer(dsi, cmd->data, JD9365DA_INIT_CMD_LEN); In general people don't like the table-based approach for DSI init. For this patch it's probably OK, but before the later patch where you add a new panel you'll probably need to transition to a separate init per panel and then use mipi_dsi_dcs_write_seq_multi() in there. I'll comment more on the later patch. In other words, one possible v5 should be a 5-patch series: Patch #1: this patch Patch #2: DT binding Patch #3: switch jd9365da to use mipi_dsi_dcs_write_seq_multi() but no functional changes. Patch #4: add your new panel. Patch #5: orientation. -Doug
