Hi, On Mon, Aug 5, 2024 at 8:40 PM Zhaoxiong Lv <lvzhaoxiong@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote: > > Move the "exit sleep mode" and "set display on" command from > enable() to init() function. > > As mentioned in the patch: > https://lore.kernel.org/all/20240624141926.5250-2-lvzhaoxiong@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/ > > Our DSI host has different modes in prepare() and enable() nit: it's not obvious to the reader of this patch which DSI host is "our"s. Maybe spell out which SoC you're using? I assume this is a Mediatek SoC? > functions. prepare() is in LP mode and enable() is in HS mode. > Since the "exit sleep mode" and "set display on" command must > also be sent in LP mode, so we also move "exit sleep mode" and > "set display on" command to the init() function. > > We have no other actions in the enable() function after moves > "exit sleep mode" and "set display on", and we checked the call > of the enable() function during the "startup" process. It seems > that only one judgment was made in drm_panel_enabel(). If the > panel does not define enable(), the judgment will skip the > enable() and continue execution. This does not seem to have > any other effects,and we found that some drivers also seem s/effects,and/effect, and/ > to have no enable() function added, for example: > panel-asus-z00t-tm5p5-n35596 / panel-boe-himax8279d ... > In addition, we briefly tested the kingdisplay_kd101ne3 panel and > melfas_lmfbx101117480 panel, and it seems that there is no garbage > on the panel, so we delete enable() function. > > After moving the "exit sleep mode" and "set display on" command > to the init() function, we no longer need additional delay > judgment, so we deletevariables "exit_sleep_to_display_on_delay_ms" nit: s/deletevariables/delete variables/ > and "display_on_delay_ms". > > Signed-off-by: Zhaoxiong Lv <lvzhaoxiong@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> > --- > Changes between V2 and V1: > - 1. The code has not changed, Modify the commit information. > v1: https://lore.kernel.org/all/20240725083245.12253-2-lvzhaoxiong@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/ > --- > .../gpu/drm/panel/panel-jadard-jd9365da-h3.c | 59 ++++++++++--------- > 1 file changed, 32 insertions(+), 27 deletions(-) nit: ${SUBJECT} is a bit long. In general it's worth abbreviating a bit more so that the subject doesn't go to crazy. drm/panel: jd9365da: Move "exit sleep mode" and "set display on" cmds Aside from the above nits, this looks OK to me. I wouldn't object to fixing some of my own nits when applying or you could send a v3 if there is no other feedback. In any case: Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx> I'd prefer someone with more MIPI panel experience give a review, though, so I'll expect that Jessica or Neil or someone else gives a review. -Doug
