diff mbox series

[v1,1/2] drm/panel: jd9365da: Move the sending location of the 11/29 command

Message ID 20240725083245.12253-2-lvzhaoxiong@huaqin.corp-partner.google.com (mailing list archive)
State New, archived
Headers show
Series Modify the method of sending 11/29 commands | expand

Commit Message

Zhaoxiong Lv July 25, 2024, 8:32 a.m. UTC
Move the 11/29 command from enable() to init() function

As mentioned in the patch:
https://lore.kernel.org/all/20240624141926.5250-2-lvzhaoxiong@huaqin.corp-partner.google.com/

Our DSI host has different modes in prepare() and enable()
functions. prepare() is in LP mode and enable() is in HS mode.
Since the 11/29 command must also be sent in LP mode,
so we also move 11/29 command to the init() function.

After moving the 11/29 command to the init() function,
we no longer need additional delay judgment, so we delete
variables "exit_sleep_to_display_on_delay_ms" and
"display_on_delay_ms".

Signed-off-by: Zhaoxiong Lv <lvzhaoxiong@huaqin.corp-partner.google.com>
---
 .../gpu/drm/panel/panel-jadard-jd9365da-h3.c  | 59 ++++++++++---------
 1 file changed, 32 insertions(+), 27 deletions(-)

Comments

Jani Nikula July 25, 2024, 8:41 a.m. UTC | #1
On Thu, 25 Jul 2024, Zhaoxiong Lv <lvzhaoxiong@huaqin.corp-partner.google.com> wrote:
> Move the 11/29 command from enable() to init() function

OOC, what is the "11/29" command?

BR,
Jani.

>
> As mentioned in the patch:
> https://lore.kernel.org/all/20240624141926.5250-2-lvzhaoxiong@huaqin.corp-partner.google.com/
>
> Our DSI host has different modes in prepare() and enable()
> functions. prepare() is in LP mode and enable() is in HS mode.
> Since the 11/29 command must also be sent in LP mode,
> so we also move 11/29 command to the init() function.
>
> After moving the 11/29 command to the init() function,
> we no longer need additional delay judgment, so we delete
> variables "exit_sleep_to_display_on_delay_ms" and
> "display_on_delay_ms".
>
> Signed-off-by: Zhaoxiong Lv <lvzhaoxiong@huaqin.corp-partner.google.com>
> ---
>  .../gpu/drm/panel/panel-jadard-jd9365da-h3.c  | 59 ++++++++++---------
>  1 file changed, 32 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
> index 04d315d96bff..ce73e8cb1db5 100644
> --- a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
> +++ b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
> @@ -31,8 +31,6 @@ struct jadard_panel_desc {
>  	bool reset_before_power_off_vcioo;
>  	unsigned int vcioo_to_lp11_delay_ms;
>  	unsigned int lp11_to_reset_delay_ms;
> -	unsigned int exit_sleep_to_display_on_delay_ms;
> -	unsigned int display_on_delay_ms;
>  	unsigned int backlight_off_to_display_off_delay_ms;
>  	unsigned int display_off_to_enter_sleep_delay_ms;
>  	unsigned int enter_sleep_to_reset_down_delay_ms;
> @@ -66,26 +64,6 @@ static inline struct jadard *panel_to_jadard(struct drm_panel *panel)
>  	return container_of(panel, struct jadard, panel);
>  }
>  
> -static int jadard_enable(struct drm_panel *panel)
> -{
> -	struct jadard *jadard = panel_to_jadard(panel);
> -	struct mipi_dsi_multi_context dsi_ctx = { .dsi = jadard->dsi };
> -
> -	msleep(120);
> -
> -	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> -
> -	if (jadard->desc->exit_sleep_to_display_on_delay_ms)
> -		mipi_dsi_msleep(&dsi_ctx, jadard->desc->exit_sleep_to_display_on_delay_ms);
> -
> -	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> -
> -	if (jadard->desc->display_on_delay_ms)
> -		mipi_dsi_msleep(&dsi_ctx, jadard->desc->display_on_delay_ms);
> -
> -	return dsi_ctx.accum_err;
> -}
> -
>  static int jadard_disable(struct drm_panel *panel)
>  {
>  	struct jadard *jadard = panel_to_jadard(panel);
> @@ -202,7 +180,6 @@ static const struct drm_panel_funcs jadard_funcs = {
>  	.disable = jadard_disable,
>  	.unprepare = jadard_unprepare,
>  	.prepare = jadard_prepare,
> -	.enable = jadard_enable,
>  	.get_modes = jadard_get_modes,
>  	.get_orientation = jadard_panel_get_orientation,
>  };
> @@ -382,6 +359,12 @@ static int radxa_display_8hd_ad002_init_cmds(struct jadard *jadard)
>  
>  	jd9365da_switch_page(&dsi_ctx, 0x00);
>  
> +	mipi_dsi_msleep(&dsi_ctx, 120);
> +
> +	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> +
> +	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> +
>  	return dsi_ctx.accum_err;
>  };
>  
> @@ -608,6 +591,12 @@ static int cz101b4001_init_cmds(struct jadard *jadard)
>  	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xE6, 0x02);
>  	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xE7, 0x0C);
>  
> +	mipi_dsi_msleep(&dsi_ctx, 120);
> +
> +	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> +
> +	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> +
>  	return dsi_ctx.accum_err;
>  };
>  
> @@ -831,6 +820,16 @@ static int kingdisplay_kd101ne3_init_cmds(struct jadard *jadard)
>  
>  	jd9365da_switch_page(&dsi_ctx, 0x00);
>  
> +	mipi_dsi_msleep(&dsi_ctx, 120);
> +
> +	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> +
> +	mipi_dsi_msleep(&dsi_ctx, 120);
> +
> +	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> +
> +	mipi_dsi_msleep(&dsi_ctx, 20);
> +
>  	return dsi_ctx.accum_err;
>  };
>  
> @@ -859,8 +858,6 @@ static const struct jadard_panel_desc kingdisplay_kd101ne3_40ti_desc = {
>  	.reset_before_power_off_vcioo = true,
>  	.vcioo_to_lp11_delay_ms = 5,
>  	.lp11_to_reset_delay_ms = 10,
> -	.exit_sleep_to_display_on_delay_ms = 120,
> -	.display_on_delay_ms = 20,
>  	.backlight_off_to_display_off_delay_ms = 100,
>  	.display_off_to_enter_sleep_delay_ms = 50,
>  	.enter_sleep_to_reset_down_delay_ms = 100,
> @@ -1074,6 +1071,16 @@ static int melfas_lmfbx101117480_init_cmds(struct jadard *jadard)
>  	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xe6, 0x02);
>  	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xe7, 0x06);
>  
> +	mipi_dsi_msleep(&dsi_ctx, 120);
> +
> +	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> +
> +	mipi_dsi_msleep(&dsi_ctx, 120);
> +
> +	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> +
> +	mipi_dsi_msleep(&dsi_ctx, 20);
> +
>  	return dsi_ctx.accum_err;
>  };
>  
> @@ -1102,8 +1109,6 @@ static const struct jadard_panel_desc melfas_lmfbx101117480_desc = {
>  	.reset_before_power_off_vcioo = true,
>  	.vcioo_to_lp11_delay_ms = 5,
>  	.lp11_to_reset_delay_ms = 10,
> -	.exit_sleep_to_display_on_delay_ms = 120,
> -	.display_on_delay_ms = 20,
>  	.backlight_off_to_display_off_delay_ms = 100,
>  	.display_off_to_enter_sleep_delay_ms = 50,
>  	.enter_sleep_to_reset_down_delay_ms = 100,
Zhaoxiong Lv July 26, 2024, 1:17 a.m. UTC | #2
On Thu, Jul 25, 2024 at 4:41 PM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>
> On Thu, 25 Jul 2024, Zhaoxiong Lv <lvzhaoxiong@huaqin.corp-partner.google.com> wrote:
> > Move the 11/29 command from enable() to init() function
>
> OOC, what is the "11/29" command?
>
> BR,
> Jani.

hi Jani
Sorry, maybe I didn't describe it clearly. The 11/29 commands are sent
by these two functions.

mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);

MIPI_DCS_EXIT_SLEEP_MODE = 0x11,
MIPI_DCS_SET_DISPLAY_ON= 0x29,

BR,
>
> >
> > As mentioned in the patch:
> > https://lore.kernel.org/all/20240624141926.5250-2-lvzhaoxiong@huaqin.corp-partner.google.com/
> >
> > Our DSI host has different modes in prepare() and enable()
> > functions. prepare() is in LP mode and enable() is in HS mode.
> > Since the 11/29 command must also be sent in LP mode,
> > so we also move 11/29 command to the init() function.
> >
> > After moving the 11/29 command to the init() function,
> > we no longer need additional delay judgment, so we delete
> > variables "exit_sleep_to_display_on_delay_ms" and
> > "display_on_delay_ms".
> >
> > Signed-off-by: Zhaoxiong Lv <lvzhaoxiong@huaqin.corp-partner.google.com>
> > ---
> >  .../gpu/drm/panel/panel-jadard-jd9365da-h3.c  | 59 ++++++++++---------
> >  1 file changed, 32 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
> > index 04d315d96bff..ce73e8cb1db5 100644
> > --- a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
> > +++ b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
> > @@ -31,8 +31,6 @@ struct jadard_panel_desc {
> >       bool reset_before_power_off_vcioo;
> >       unsigned int vcioo_to_lp11_delay_ms;
> >       unsigned int lp11_to_reset_delay_ms;
> > -     unsigned int exit_sleep_to_display_on_delay_ms;
> > -     unsigned int display_on_delay_ms;
> >       unsigned int backlight_off_to_display_off_delay_ms;
> >       unsigned int display_off_to_enter_sleep_delay_ms;
> >       unsigned int enter_sleep_to_reset_down_delay_ms;
> > @@ -66,26 +64,6 @@ static inline struct jadard *panel_to_jadard(struct drm_panel *panel)
> >       return container_of(panel, struct jadard, panel);
> >  }
> >
> > -static int jadard_enable(struct drm_panel *panel)
> > -{
> > -     struct jadard *jadard = panel_to_jadard(panel);
> > -     struct mipi_dsi_multi_context dsi_ctx = { .dsi = jadard->dsi };
> > -
> > -     msleep(120);
> > -
> > -     mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> > -
> > -     if (jadard->desc->exit_sleep_to_display_on_delay_ms)
> > -             mipi_dsi_msleep(&dsi_ctx, jadard->desc->exit_sleep_to_display_on_delay_ms);
> > -
> > -     mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> > -
> > -     if (jadard->desc->display_on_delay_ms)
> > -             mipi_dsi_msleep(&dsi_ctx, jadard->desc->display_on_delay_ms);
> > -
> > -     return dsi_ctx.accum_err;
> > -}
> > -
> >  static int jadard_disable(struct drm_panel *panel)
> >  {
> >       struct jadard *jadard = panel_to_jadard(panel);
> > @@ -202,7 +180,6 @@ static const struct drm_panel_funcs jadard_funcs = {
> >       .disable = jadard_disable,
> >       .unprepare = jadard_unprepare,
> >       .prepare = jadard_prepare,
> > -     .enable = jadard_enable,
> >       .get_modes = jadard_get_modes,
> >       .get_orientation = jadard_panel_get_orientation,
> >  };
> > @@ -382,6 +359,12 @@ static int radxa_display_8hd_ad002_init_cmds(struct jadard *jadard)
> >
> >       jd9365da_switch_page(&dsi_ctx, 0x00);
> >
> > +     mipi_dsi_msleep(&dsi_ctx, 120);
> > +
> > +     mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> > +
> > +     mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> > +
> >       return dsi_ctx.accum_err;
> >  };
> >
> > @@ -608,6 +591,12 @@ static int cz101b4001_init_cmds(struct jadard *jadard)
> >       mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xE6, 0x02);
> >       mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xE7, 0x0C);
> >
> > +     mipi_dsi_msleep(&dsi_ctx, 120);
> > +
> > +     mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> > +
> > +     mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> > +
> >       return dsi_ctx.accum_err;
> >  };
> >
> > @@ -831,6 +820,16 @@ static int kingdisplay_kd101ne3_init_cmds(struct jadard *jadard)
> >
> >       jd9365da_switch_page(&dsi_ctx, 0x00);
> >
> > +     mipi_dsi_msleep(&dsi_ctx, 120);
> > +
> > +     mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> > +
> > +     mipi_dsi_msleep(&dsi_ctx, 120);
> > +
> > +     mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> > +
> > +     mipi_dsi_msleep(&dsi_ctx, 20);
> > +
> >       return dsi_ctx.accum_err;
> >  };
> >
> > @@ -859,8 +858,6 @@ static const struct jadard_panel_desc kingdisplay_kd101ne3_40ti_desc = {
> >       .reset_before_power_off_vcioo = true,
> >       .vcioo_to_lp11_delay_ms = 5,
> >       .lp11_to_reset_delay_ms = 10,
> > -     .exit_sleep_to_display_on_delay_ms = 120,
> > -     .display_on_delay_ms = 20,
> >       .backlight_off_to_display_off_delay_ms = 100,
> >       .display_off_to_enter_sleep_delay_ms = 50,
> >       .enter_sleep_to_reset_down_delay_ms = 100,
> > @@ -1074,6 +1071,16 @@ static int melfas_lmfbx101117480_init_cmds(struct jadard *jadard)
> >       mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xe6, 0x02);
> >       mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xe7, 0x06);
> >
> > +     mipi_dsi_msleep(&dsi_ctx, 120);
> > +
> > +     mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> > +
> > +     mipi_dsi_msleep(&dsi_ctx, 120);
> > +
> > +     mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> > +
> > +     mipi_dsi_msleep(&dsi_ctx, 20);
> > +
> >       return dsi_ctx.accum_err;
> >  };
> >
> > @@ -1102,8 +1109,6 @@ static const struct jadard_panel_desc melfas_lmfbx101117480_desc = {
> >       .reset_before_power_off_vcioo = true,
> >       .vcioo_to_lp11_delay_ms = 5,
> >       .lp11_to_reset_delay_ms = 10,
> > -     .exit_sleep_to_display_on_delay_ms = 120,
> > -     .display_on_delay_ms = 20,
> >       .backlight_off_to_display_off_delay_ms = 100,
> >       .display_off_to_enter_sleep_delay_ms = 50,
> >       .enter_sleep_to_reset_down_delay_ms = 100,
>
> --
> Jani Nikula, Intel
Jani Nikula July 26, 2024, 8:06 a.m. UTC | #3
On Fri, 26 Jul 2024, zhaoxiong lv <lvzhaoxiong@huaqin.corp-partner.google.com> wrote:
> On Thu, Jul 25, 2024 at 4:41 PM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>>
>> On Thu, 25 Jul 2024, Zhaoxiong Lv <lvzhaoxiong@huaqin.corp-partner.google.com> wrote:
>> > Move the 11/29 command from enable() to init() function
>>
>> OOC, what is the "11/29" command?
>>
>> BR,
>> Jani.
>
> hi Jani
> Sorry, maybe I didn't describe it clearly. The 11/29 commands are sent
> by these two functions.
>
> mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
>
> MIPI_DCS_EXIT_SLEEP_MODE = 0x11,
> MIPI_DCS_SET_DISPLAY_ON= 0x29,

Maybe refer to the commands with their names then? Exit sleep mode and
set display on.

BR,
Jani.



>
> BR,
>>
>> >
>> > As mentioned in the patch:
>> > https://lore.kernel.org/all/20240624141926.5250-2-lvzhaoxiong@huaqin.corp-partner.google.com/
>> >
>> > Our DSI host has different modes in prepare() and enable()
>> > functions. prepare() is in LP mode and enable() is in HS mode.
>> > Since the 11/29 command must also be sent in LP mode,
>> > so we also move 11/29 command to the init() function.
>> >
>> > After moving the 11/29 command to the init() function,
>> > we no longer need additional delay judgment, so we delete
>> > variables "exit_sleep_to_display_on_delay_ms" and
>> > "display_on_delay_ms".
>> >
>> > Signed-off-by: Zhaoxiong Lv <lvzhaoxiong@huaqin.corp-partner.google.com>
>> > ---
>> >  .../gpu/drm/panel/panel-jadard-jd9365da-h3.c  | 59 ++++++++++---------
>> >  1 file changed, 32 insertions(+), 27 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
>> > index 04d315d96bff..ce73e8cb1db5 100644
>> > --- a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
>> > +++ b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
>> > @@ -31,8 +31,6 @@ struct jadard_panel_desc {
>> >       bool reset_before_power_off_vcioo;
>> >       unsigned int vcioo_to_lp11_delay_ms;
>> >       unsigned int lp11_to_reset_delay_ms;
>> > -     unsigned int exit_sleep_to_display_on_delay_ms;
>> > -     unsigned int display_on_delay_ms;
>> >       unsigned int backlight_off_to_display_off_delay_ms;
>> >       unsigned int display_off_to_enter_sleep_delay_ms;
>> >       unsigned int enter_sleep_to_reset_down_delay_ms;
>> > @@ -66,26 +64,6 @@ static inline struct jadard *panel_to_jadard(struct drm_panel *panel)
>> >       return container_of(panel, struct jadard, panel);
>> >  }
>> >
>> > -static int jadard_enable(struct drm_panel *panel)
>> > -{
>> > -     struct jadard *jadard = panel_to_jadard(panel);
>> > -     struct mipi_dsi_multi_context dsi_ctx = { .dsi = jadard->dsi };
>> > -
>> > -     msleep(120);
>> > -
>> > -     mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
>> > -
>> > -     if (jadard->desc->exit_sleep_to_display_on_delay_ms)
>> > -             mipi_dsi_msleep(&dsi_ctx, jadard->desc->exit_sleep_to_display_on_delay_ms);
>> > -
>> > -     mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
>> > -
>> > -     if (jadard->desc->display_on_delay_ms)
>> > -             mipi_dsi_msleep(&dsi_ctx, jadard->desc->display_on_delay_ms);
>> > -
>> > -     return dsi_ctx.accum_err;
>> > -}
>> > -
>> >  static int jadard_disable(struct drm_panel *panel)
>> >  {
>> >       struct jadard *jadard = panel_to_jadard(panel);
>> > @@ -202,7 +180,6 @@ static const struct drm_panel_funcs jadard_funcs = {
>> >       .disable = jadard_disable,
>> >       .unprepare = jadard_unprepare,
>> >       .prepare = jadard_prepare,
>> > -     .enable = jadard_enable,
>> >       .get_modes = jadard_get_modes,
>> >       .get_orientation = jadard_panel_get_orientation,
>> >  };
>> > @@ -382,6 +359,12 @@ static int radxa_display_8hd_ad002_init_cmds(struct jadard *jadard)
>> >
>> >       jd9365da_switch_page(&dsi_ctx, 0x00);
>> >
>> > +     mipi_dsi_msleep(&dsi_ctx, 120);
>> > +
>> > +     mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
>> > +
>> > +     mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
>> > +
>> >       return dsi_ctx.accum_err;
>> >  };
>> >
>> > @@ -608,6 +591,12 @@ static int cz101b4001_init_cmds(struct jadard *jadard)
>> >       mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xE6, 0x02);
>> >       mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xE7, 0x0C);
>> >
>> > +     mipi_dsi_msleep(&dsi_ctx, 120);
>> > +
>> > +     mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
>> > +
>> > +     mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
>> > +
>> >       return dsi_ctx.accum_err;
>> >  };
>> >
>> > @@ -831,6 +820,16 @@ static int kingdisplay_kd101ne3_init_cmds(struct jadard *jadard)
>> >
>> >       jd9365da_switch_page(&dsi_ctx, 0x00);
>> >
>> > +     mipi_dsi_msleep(&dsi_ctx, 120);
>> > +
>> > +     mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
>> > +
>> > +     mipi_dsi_msleep(&dsi_ctx, 120);
>> > +
>> > +     mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
>> > +
>> > +     mipi_dsi_msleep(&dsi_ctx, 20);
>> > +
>> >       return dsi_ctx.accum_err;
>> >  };
>> >
>> > @@ -859,8 +858,6 @@ static const struct jadard_panel_desc kingdisplay_kd101ne3_40ti_desc = {
>> >       .reset_before_power_off_vcioo = true,
>> >       .vcioo_to_lp11_delay_ms = 5,
>> >       .lp11_to_reset_delay_ms = 10,
>> > -     .exit_sleep_to_display_on_delay_ms = 120,
>> > -     .display_on_delay_ms = 20,
>> >       .backlight_off_to_display_off_delay_ms = 100,
>> >       .display_off_to_enter_sleep_delay_ms = 50,
>> >       .enter_sleep_to_reset_down_delay_ms = 100,
>> > @@ -1074,6 +1071,16 @@ static int melfas_lmfbx101117480_init_cmds(struct jadard *jadard)
>> >       mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xe6, 0x02);
>> >       mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xe7, 0x06);
>> >
>> > +     mipi_dsi_msleep(&dsi_ctx, 120);
>> > +
>> > +     mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
>> > +
>> > +     mipi_dsi_msleep(&dsi_ctx, 120);
>> > +
>> > +     mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
>> > +
>> > +     mipi_dsi_msleep(&dsi_ctx, 20);
>> > +
>> >       return dsi_ctx.accum_err;
>> >  };
>> >
>> > @@ -1102,8 +1109,6 @@ static const struct jadard_panel_desc melfas_lmfbx101117480_desc = {
>> >       .reset_before_power_off_vcioo = true,
>> >       .vcioo_to_lp11_delay_ms = 5,
>> >       .lp11_to_reset_delay_ms = 10,
>> > -     .exit_sleep_to_display_on_delay_ms = 120,
>> > -     .display_on_delay_ms = 20,
>> >       .backlight_off_to_display_off_delay_ms = 100,
>> >       .display_off_to_enter_sleep_delay_ms = 50,
>> >       .enter_sleep_to_reset_down_delay_ms = 100,
>>
>> --
>> Jani Nikula, Intel
Dmitry Baryshkov July 27, 2024, 4:59 p.m. UTC | #4
On Thu, Jul 25, 2024 at 04:32:44PM GMT, Zhaoxiong Lv wrote:
> Move the 11/29 command from enable() to init() function
> 
> As mentioned in the patch:
> https://lore.kernel.org/all/20240624141926.5250-2-lvzhaoxiong@huaqin.corp-partner.google.com/
> 
> Our DSI host has different modes in prepare() and enable()
> functions. prepare() is in LP mode and enable() is in HS mode.
> Since the 11/29 command must also be sent in LP mode,
> so we also move 11/29 command to the init() function.
> 
> After moving the 11/29 command to the init() function,
> we no longer need additional delay judgment, so we delete
> variables "exit_sleep_to_display_on_delay_ms" and
> "display_on_delay_ms".

Won't this result in a garbage being displayed on the panel during
startup?

> 
> Signed-off-by: Zhaoxiong Lv <lvzhaoxiong@huaqin.corp-partner.google.com>
> ---
>  .../gpu/drm/panel/panel-jadard-jd9365da-h3.c  | 59 ++++++++++---------
>  1 file changed, 32 insertions(+), 27 deletions(-)
Zhaoxiong Lv July 29, 2024, 3:10 a.m. UTC | #5
On Sun, Jul 28, 2024 at 12:59 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Thu, Jul 25, 2024 at 04:32:44PM GMT, Zhaoxiong Lv wrote:
> > Move the 11/29 command from enable() to init() function
> >
> > As mentioned in the patch:
> > https://lore.kernel.org/all/20240624141926.5250-2-lvzhaoxiong@huaqin.corp-partner.google.com/
> >
> > Our DSI host has different modes in prepare() and enable()
> > functions. prepare() is in LP mode and enable() is in HS mode.
> > Since the 11/29 command must also be sent in LP mode,
> > so we also move 11/29 command to the init() function.
> >
> > After moving the 11/29 command to the init() function,
> > we no longer need additional delay judgment, so we delete
> > variables "exit_sleep_to_display_on_delay_ms" and
> > "display_on_delay_ms".
>
> Won't this result in a garbage being displayed on the panel during
> startup?

Hi Dmitry

We just moved "Exit sleep mode" and "set display on" from the enable()
function to the init() function and did not make any other changes.
It seems that many drivers also put the "init code" and "Exit sleep
mode" in one function.
In addition, we briefly tested the kingdisplay_kd101ne3 panel and
melfas_lmfbx101117480 panel, and it seems that there is no garbage on
the panel.

BR
>
> >
> > Signed-off-by: Zhaoxiong Lv <lvzhaoxiong@huaqin.corp-partner.google.com>
> > ---
> >  .../gpu/drm/panel/panel-jadard-jd9365da-h3.c  | 59 ++++++++++---------
> >  1 file changed, 32 insertions(+), 27 deletions(-)
>
Dmitry Baryshkov July 29, 2024, 8:09 p.m. UTC | #6
On Mon, 29 Jul 2024 at 06:10, zhaoxiong lv
<lvzhaoxiong@huaqin.corp-partner.google.com> wrote:
>
> On Sun, Jul 28, 2024 at 12:59 AM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > On Thu, Jul 25, 2024 at 04:32:44PM GMT, Zhaoxiong Lv wrote:
> > > Move the 11/29 command from enable() to init() function
> > >
> > > As mentioned in the patch:
> > > https://lore.kernel.org/all/20240624141926.5250-2-lvzhaoxiong@huaqin.corp-partner.google.com/
> > >
> > > Our DSI host has different modes in prepare() and enable()
> > > functions. prepare() is in LP mode and enable() is in HS mode.
> > > Since the 11/29 command must also be sent in LP mode,
> > > so we also move 11/29 command to the init() function.
> > >
> > > After moving the 11/29 command to the init() function,
> > > we no longer need additional delay judgment, so we delete
> > > variables "exit_sleep_to_display_on_delay_ms" and
> > > "display_on_delay_ms".
> >
> > Won't this result in a garbage being displayed on the panel during
> > startup?
>
> Hi Dmitry
>
> We just moved "Exit sleep mode" and "set display on" from the enable()
> function to the init() function and did not make any other changes.
> It seems that many drivers also put the "init code" and "Exit sleep
> mode" in one function.

You have moved the functions that actually enable the display out. And
by the definition it's expected that there is no video stream during
pre_enable(), it gets turned on afterwards. That's why I asked if
there is any kind of garbage or not.

> In addition, we briefly tested the kingdisplay_kd101ne3 panel and
> melfas_lmfbx101117480 panel, and it seems that there is no garbage on
> the panel.

Ack.

>
> BR
> >
> > >
> > > Signed-off-by: Zhaoxiong Lv <lvzhaoxiong@huaqin.corp-partner.google.com>
> > > ---
> > >  .../gpu/drm/panel/panel-jadard-jd9365da-h3.c  | 59 ++++++++++---------
> > >  1 file changed, 32 insertions(+), 27 deletions(-)
> >
Zhaoxiong Lv Aug. 5, 2024, 2:38 a.m. UTC | #7
Hi all

Do you have any other suggestions for this patch?
Looking forward to your reply, thank you

BR

On Tue, Jul 30, 2024 at 4:09 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Mon, 29 Jul 2024 at 06:10, zhaoxiong lv
> <lvzhaoxiong@huaqin.corp-partner.google.com> wrote:
> >
> > On Sun, Jul 28, 2024 at 12:59 AM Dmitry Baryshkov
> > <dmitry.baryshkov@linaro.org> wrote:
> > >
> > > On Thu, Jul 25, 2024 at 04:32:44PM GMT, Zhaoxiong Lv wrote:
> > > > Move the 11/29 command from enable() to init() function
> > > >
> > > > As mentioned in the patch:
> > > > https://lore.kernel.org/all/20240624141926.5250-2-lvzhaoxiong@huaqin.corp-partner.google.com/
> > > >
> > > > Our DSI host has different modes in prepare() and enable()
> > > > functions. prepare() is in LP mode and enable() is in HS mode.
> > > > Since the 11/29 command must also be sent in LP mode,
> > > > so we also move 11/29 command to the init() function.
> > > >
> > > > After moving the 11/29 command to the init() function,
> > > > we no longer need additional delay judgment, so we delete
> > > > variables "exit_sleep_to_display_on_delay_ms" and
> > > > "display_on_delay_ms".
> > >
> > > Won't this result in a garbage being displayed on the panel during
> > > startup?
> >
> > Hi Dmitry
> >
> > We just moved "Exit sleep mode" and "set display on" from the enable()
> > function to the init() function and did not make any other changes.
> > It seems that many drivers also put the "init code" and "Exit sleep
> > mode" in one function.
>
> You have moved the functions that actually enable the display out. And
> by the definition it's expected that there is no video stream during
> pre_enable(), it gets turned on afterwards. That's why I asked if
> there is any kind of garbage or not.
>
> > In addition, we briefly tested the kingdisplay_kd101ne3 panel and
> > melfas_lmfbx101117480 panel, and it seems that there is no garbage on
> > the panel.
>
> Ack.
>
> >
> > BR
> > >
> > > >
> > > > Signed-off-by: Zhaoxiong Lv <lvzhaoxiong@huaqin.corp-partner.google.com>
> > > > ---
> > > >  .../gpu/drm/panel/panel-jadard-jd9365da-h3.c  | 59 ++++++++++---------
> > > >  1 file changed, 32 insertions(+), 27 deletions(-)
> > >
>
>
>
> --
> With best wishes
> Dmitry
Doug Anderson Aug. 5, 2024, 3:56 p.m. UTC | #8
Hi,

On Sun, Aug 4, 2024 at 7:38 PM zhaoxiong lv
<lvzhaoxiong@huaqin.corp-partner.google.com> wrote:
>
> Hi all
>
> Do you have any other suggestions for this patch?
> Looking forward to your reply, thank you

Please make sure not to "top post". Folks on the mailing lists
generally frown on this and it's a good way to get your email ignored
by some people.

At this point I think folks are waiting for you to post the next
version addressing comments. Specifically, things you'd want to change
for the next version:

* In the commit message (and subject), "refer to the commands with
their names" (Jani)

* In the commit message, address Dmitry's concern. In other words, say
something about the fact that this doesn't cause garbage being
displayed on the panel during startup and why not.


When sending v2, don't forget to include Jessica's "Ack" on patch #2.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
index 04d315d96bff..ce73e8cb1db5 100644
--- a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
+++ b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
@@ -31,8 +31,6 @@  struct jadard_panel_desc {
 	bool reset_before_power_off_vcioo;
 	unsigned int vcioo_to_lp11_delay_ms;
 	unsigned int lp11_to_reset_delay_ms;
-	unsigned int exit_sleep_to_display_on_delay_ms;
-	unsigned int display_on_delay_ms;
 	unsigned int backlight_off_to_display_off_delay_ms;
 	unsigned int display_off_to_enter_sleep_delay_ms;
 	unsigned int enter_sleep_to_reset_down_delay_ms;
@@ -66,26 +64,6 @@  static inline struct jadard *panel_to_jadard(struct drm_panel *panel)
 	return container_of(panel, struct jadard, panel);
 }
 
-static int jadard_enable(struct drm_panel *panel)
-{
-	struct jadard *jadard = panel_to_jadard(panel);
-	struct mipi_dsi_multi_context dsi_ctx = { .dsi = jadard->dsi };
-
-	msleep(120);
-
-	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
-
-	if (jadard->desc->exit_sleep_to_display_on_delay_ms)
-		mipi_dsi_msleep(&dsi_ctx, jadard->desc->exit_sleep_to_display_on_delay_ms);
-
-	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
-
-	if (jadard->desc->display_on_delay_ms)
-		mipi_dsi_msleep(&dsi_ctx, jadard->desc->display_on_delay_ms);
-
-	return dsi_ctx.accum_err;
-}
-
 static int jadard_disable(struct drm_panel *panel)
 {
 	struct jadard *jadard = panel_to_jadard(panel);
@@ -202,7 +180,6 @@  static const struct drm_panel_funcs jadard_funcs = {
 	.disable = jadard_disable,
 	.unprepare = jadard_unprepare,
 	.prepare = jadard_prepare,
-	.enable = jadard_enable,
 	.get_modes = jadard_get_modes,
 	.get_orientation = jadard_panel_get_orientation,
 };
@@ -382,6 +359,12 @@  static int radxa_display_8hd_ad002_init_cmds(struct jadard *jadard)
 
 	jd9365da_switch_page(&dsi_ctx, 0x00);
 
+	mipi_dsi_msleep(&dsi_ctx, 120);
+
+	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+
+	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
+
 	return dsi_ctx.accum_err;
 };
 
@@ -608,6 +591,12 @@  static int cz101b4001_init_cmds(struct jadard *jadard)
 	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xE6, 0x02);
 	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xE7, 0x0C);
 
+	mipi_dsi_msleep(&dsi_ctx, 120);
+
+	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+
+	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
+
 	return dsi_ctx.accum_err;
 };
 
@@ -831,6 +820,16 @@  static int kingdisplay_kd101ne3_init_cmds(struct jadard *jadard)
 
 	jd9365da_switch_page(&dsi_ctx, 0x00);
 
+	mipi_dsi_msleep(&dsi_ctx, 120);
+
+	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+
+	mipi_dsi_msleep(&dsi_ctx, 120);
+
+	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
+
+	mipi_dsi_msleep(&dsi_ctx, 20);
+
 	return dsi_ctx.accum_err;
 };
 
@@ -859,8 +858,6 @@  static const struct jadard_panel_desc kingdisplay_kd101ne3_40ti_desc = {
 	.reset_before_power_off_vcioo = true,
 	.vcioo_to_lp11_delay_ms = 5,
 	.lp11_to_reset_delay_ms = 10,
-	.exit_sleep_to_display_on_delay_ms = 120,
-	.display_on_delay_ms = 20,
 	.backlight_off_to_display_off_delay_ms = 100,
 	.display_off_to_enter_sleep_delay_ms = 50,
 	.enter_sleep_to_reset_down_delay_ms = 100,
@@ -1074,6 +1071,16 @@  static int melfas_lmfbx101117480_init_cmds(struct jadard *jadard)
 	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xe6, 0x02);
 	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xe7, 0x06);
 
+	mipi_dsi_msleep(&dsi_ctx, 120);
+
+	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+
+	mipi_dsi_msleep(&dsi_ctx, 120);
+
+	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
+
+	mipi_dsi_msleep(&dsi_ctx, 20);
+
 	return dsi_ctx.accum_err;
 };
 
@@ -1102,8 +1109,6 @@  static const struct jadard_panel_desc melfas_lmfbx101117480_desc = {
 	.reset_before_power_off_vcioo = true,
 	.vcioo_to_lp11_delay_ms = 5,
 	.lp11_to_reset_delay_ms = 10,
-	.exit_sleep_to_display_on_delay_ms = 120,
-	.display_on_delay_ms = 20,
 	.backlight_off_to_display_off_delay_ms = 100,
 	.display_off_to_enter_sleep_delay_ms = 50,
 	.enter_sleep_to_reset_down_delay_ms = 100,