Re: [PATCH v2 1/2] drm/panel: jd9365da: Move the location of "exit sleep mode" and "set display on" commands

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux