| Message ID | 20240915080830.11318-2-lvzhaoxiong@huaqin.corp-partner.google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Modify the timing of three panels | expand |
On Sun, 15 Sept 2024 at 10:10, Zhaoxiong Lv <lvzhaoxiong@huaqin.corp-partner.google.com> wrote: > > In order to meet the timing, remove the delay between "backlight off" > and "display off" > > Removing variables: display_off_to_enter_sleep_delay_ms This is not enough. If this is a fix, then describe why the original commit is incorrect, provide necessary explanation, details, etc. Otherwise it looks as if you are removing the delay that was necessary for other panels Also if this is a fix, it should be properly notated with the Fixes tag, maybe cc:stable, etc. In its current state: NAK. > > Signed-off-by: Zhaoxiong Lv <lvzhaoxiong@huaqin.corp-partner.google.com> > --- > drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c > index 44897e5218a6..2f8af86bc2a0 100644 > --- a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c > +++ b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c > @@ -31,7 +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 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; > }; > @@ -69,9 +68,6 @@ static int jadard_disable(struct drm_panel *panel) > struct jadard *jadard = panel_to_jadard(panel); > struct mipi_dsi_multi_context dsi_ctx = { .dsi = jadard->dsi }; > > - if (jadard->desc->backlight_off_to_display_off_delay_ms) > - mipi_dsi_msleep(&dsi_ctx, jadard->desc->backlight_off_to_display_off_delay_ms); > - > mipi_dsi_dcs_set_display_off_multi(&dsi_ctx); > > if (jadard->desc->display_off_to_enter_sleep_delay_ms) > @@ -858,7 +854,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, > - .backlight_off_to_display_off_delay_ms = 100, > .display_off_to_enter_sleep_delay_ms = 50, > .enter_sleep_to_reset_down_delay_ms = 100, > }; > @@ -1109,7 +1104,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, > - .backlight_off_to_display_off_delay_ms = 100, > .display_off_to_enter_sleep_delay_ms = 50, > .enter_sleep_to_reset_down_delay_ms = 100, > }; > -- > 2.17.1 >
On Mon, Sep 16, 2024 at 1:11 PM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Sun, 15 Sept 2024 at 10:10, Zhaoxiong Lv > <lvzhaoxiong@huaqin.corp-partner.google.com> wrote: > > > > In order to meet the timing, remove the delay between "backlight off" > > and "display off" > > > > Removing variables: display_off_to_enter_sleep_delay_ms > > This is not enough. If this is a fix, then describe why the original > commit is incorrect, provide necessary explanation, details, etc. > Otherwise it looks as if you are removing the delay that was necessary > for other panels > > Also if this is a fix, it should be properly notated with the Fixes > tag, maybe cc:stable, etc. > > In its current state: NAK. hi Dmitry Our machine encountered an issue where, after entering S3 in the black screen state, there is a probability that the screen remains black after waking up. It seems that the DRM runtime resume has not yet completed, but the system has already started the suspend process. After reducing the delay for disable, this issue no longer occurs, Therefore, under the premise of meeting the timing requirements, we removed the delay between "backlight off" and "display off". Previously, "backlight_off_to_display_off_delay_ms" was added between "backlight off" and "display off" to prevent "display off" from being executed when the backlight is not fully powered off, which may cause a white screen. However, we removed this "backlight_off_to_display_off_delay_ms" and found that this situation did not occur. Therefore, in order to solve the problem mentioned above, we removed this delay. This is the timing specification for the two panels: 1. Kingdisplay panel timing spec: https://github.com/KD54183/-JD9365DA_Power-On-Off-Sequence_V0120240923 2. LMFBX101117480 timing spec: https://github.com/chiohsin-lo/TDY-JD_LIB thanks > > > > > Signed-off-by: Zhaoxiong Lv <lvzhaoxiong@huaqin.corp-partner.google.com> > > --- > > drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c | 6 ------ > > 1 file changed, 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c > > index 44897e5218a6..2f8af86bc2a0 100644 > > --- a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c > > +++ b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c > > @@ -31,7 +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 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; > > }; > > @@ -69,9 +68,6 @@ static int jadard_disable(struct drm_panel *panel) > > struct jadard *jadard = panel_to_jadard(panel); > > struct mipi_dsi_multi_context dsi_ctx = { .dsi = jadard->dsi }; > > > > - if (jadard->desc->backlight_off_to_display_off_delay_ms) > > - mipi_dsi_msleep(&dsi_ctx, jadard->desc->backlight_off_to_display_off_delay_ms); > > - > > mipi_dsi_dcs_set_display_off_multi(&dsi_ctx); > > > > if (jadard->desc->display_off_to_enter_sleep_delay_ms) > > @@ -858,7 +854,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, > > - .backlight_off_to_display_off_delay_ms = 100, > > .display_off_to_enter_sleep_delay_ms = 50, > > .enter_sleep_to_reset_down_delay_ms = 100, > > }; > > @@ -1109,7 +1104,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, > > - .backlight_off_to_display_off_delay_ms = 100, > > .display_off_to_enter_sleep_delay_ms = 50, > > .enter_sleep_to_reset_down_delay_ms = 100, > > }; > > -- > > 2.17.1 > > > > > -- > With best wishes > Dmitry
On Mon, Sep 23, 2024 at 05:22:11PM GMT, zhaoxiong lv wrote: > On Mon, Sep 16, 2024 at 1:11 PM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: > > > > On Sun, 15 Sept 2024 at 10:10, Zhaoxiong Lv > > <lvzhaoxiong@huaqin.corp-partner.google.com> wrote: > > > > > > In order to meet the timing, remove the delay between "backlight off" > > > and "display off" > > > > > > Removing variables: display_off_to_enter_sleep_delay_ms > > > > This is not enough. If this is a fix, then describe why the original > > commit is incorrect, provide necessary explanation, details, etc. > > Otherwise it looks as if you are removing the delay that was necessary > > for other panels > > > > Also if this is a fix, it should be properly notated with the Fixes > > tag, maybe cc:stable, etc. > > > > In its current state: NAK. > > hi Dmitry > > Our machine encountered an issue where, after entering S3 in the black > screen state, there is a probability that the screen remains black > after waking up. It seems that the DRM runtime resume has not yet > completed, but the system has already started the suspend process. > After reducing the delay for disable, this issue no longer occurs, > Therefore, under the premise of meeting the timing requirements, we > removed the delay between "backlight off" and "display off". > > Previously, "backlight_off_to_display_off_delay_ms" was added between > "backlight off" and "display off" to prevent "display off" from being > executed when the backlight is not fully powered off, which may cause > a white screen. However, we removed this > "backlight_off_to_display_off_delay_ms" and found that this situation > did not occur. Therefore, in order to solve the problem mentioned > above, we removed this delay. This should go to the commit message, rewritten to use imperative language, with the appropriate Fixes tags. > > This is the timing specification for the two panels: > 1. Kingdisplay panel timing spec: > https://github.com/KD54183/-JD9365DA_Power-On-Off-Sequence_V0120240923 > 2. LMFBX101117480 timing spec: https://github.com/chiohsin-lo/TDY-JD_LIB > > thanks > > > > > > > > > Signed-off-by: Zhaoxiong Lv <lvzhaoxiong@huaqin.corp-partner.google.com> > > > --- > > > drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c | 6 ------ > > > 1 file changed, 6 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c > > > index 44897e5218a6..2f8af86bc2a0 100644 > > > --- a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c > > > +++ b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c > > > @@ -31,7 +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 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; > > > }; > > > @@ -69,9 +68,6 @@ static int jadard_disable(struct drm_panel *panel) > > > struct jadard *jadard = panel_to_jadard(panel); > > > struct mipi_dsi_multi_context dsi_ctx = { .dsi = jadard->dsi }; > > > > > > - if (jadard->desc->backlight_off_to_display_off_delay_ms) > > > - mipi_dsi_msleep(&dsi_ctx, jadard->desc->backlight_off_to_display_off_delay_ms); > > > - > > > mipi_dsi_dcs_set_display_off_multi(&dsi_ctx); > > > > > > if (jadard->desc->display_off_to_enter_sleep_delay_ms) > > > @@ -858,7 +854,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, > > > - .backlight_off_to_display_off_delay_ms = 100, > > > .display_off_to_enter_sleep_delay_ms = 50, > > > .enter_sleep_to_reset_down_delay_ms = 100, > > > }; > > > @@ -1109,7 +1104,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, > > > - .backlight_off_to_display_off_delay_ms = 100, > > > .display_off_to_enter_sleep_delay_ms = 50, > > > .enter_sleep_to_reset_down_delay_ms = 100, > > > }; > > > -- > > > 2.17.1 > > > > > > > > > -- > > With best wishes > > Dmitry
On Mon, Sep 23, 2024 at 8:05 PM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Mon, Sep 23, 2024 at 05:22:11PM GMT, zhaoxiong lv wrote: > > On Mon, Sep 16, 2024 at 1:11 PM Dmitry Baryshkov > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > On Sun, 15 Sept 2024 at 10:10, Zhaoxiong Lv > > > <lvzhaoxiong@huaqin.corp-partner.google.com> wrote: > > > > > > > > In order to meet the timing, remove the delay between "backlight off" > > > > and "display off" > > > > > > > > Removing variables: display_off_to_enter_sleep_delay_ms > > > > > > This is not enough. If this is a fix, then describe why the original > > > commit is incorrect, provide necessary explanation, details, etc. > > > Otherwise it looks as if you are removing the delay that was necessary > > > for other panels > > > > > > Also if this is a fix, it should be properly notated with the Fixes > > > tag, maybe cc:stable, etc. > > > > > > In its current state: NAK. > > > > hi Dmitry > > > > Our machine encountered an issue where, after entering S3 in the black > > screen state, there is a probability that the screen remains black > > after waking up. It seems that the DRM runtime resume has not yet > > completed, but the system has already started the suspend process. > > After reducing the delay for disable, this issue no longer occurs, > > Therefore, under the premise of meeting the timing requirements, we > > removed the delay between "backlight off" and "display off". > > > > Previously, "backlight_off_to_display_off_delay_ms" was added between > > "backlight off" and "display off" to prevent "display off" from being > > executed when the backlight is not fully powered off, which may cause > > a white screen. However, we removed this > > "backlight_off_to_display_off_delay_ms" and found that this situation > > did not occur. Therefore, in order to solve the problem mentioned > > above, we removed this delay. > > This should go to the commit message, rewritten to use imperative > language, with the appropriate Fixes tags. hi Dmitry OK, thank you for your correction, we will fix it in the next patch. > > > > > This is the timing specification for the two panels: > > 1. Kingdisplay panel timing spec: > > https://github.com/KD54183/-JD9365DA_Power-On-Off-Sequence_V0120240923 > > 2. LMFBX101117480 timing spec: https://github.com/chiohsin-lo/TDY-JD_LIB > > > > thanks > > > > > > > > > > > > > Signed-off-by: Zhaoxiong Lv <lvzhaoxiong@huaqin.corp-partner.google.com> > > > > --- > > > > drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c | 6 ------ > > > > 1 file changed, 6 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c > > > > index 44897e5218a6..2f8af86bc2a0 100644 > > > > --- a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c > > > > +++ b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c > > > > @@ -31,7 +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 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; > > > > }; > > > > @@ -69,9 +68,6 @@ static int jadard_disable(struct drm_panel *panel) > > > > struct jadard *jadard = panel_to_jadard(panel); > > > > struct mipi_dsi_multi_context dsi_ctx = { .dsi = jadard->dsi }; > > > > > > > > - if (jadard->desc->backlight_off_to_display_off_delay_ms) > > > > - mipi_dsi_msleep(&dsi_ctx, jadard->desc->backlight_off_to_display_off_delay_ms); > > > > - > > > > mipi_dsi_dcs_set_display_off_multi(&dsi_ctx); > > > > > > > > if (jadard->desc->display_off_to_enter_sleep_delay_ms) > > > > @@ -858,7 +854,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, > > > > - .backlight_off_to_display_off_delay_ms = 100, > > > > .display_off_to_enter_sleep_delay_ms = 50, > > > > .enter_sleep_to_reset_down_delay_ms = 100, > > > > }; > > > > @@ -1109,7 +1104,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, > > > > - .backlight_off_to_display_off_delay_ms = 100, > > > > .display_off_to_enter_sleep_delay_ms = 50, > > > > .enter_sleep_to_reset_down_delay_ms = 100, > > > > }; > > > > -- > > > > 2.17.1 > > > > > > > > > > > > > -- > > > With best wishes > > > Dmitry > > -- > With best wishes > Dmitry
diff --git a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c index 44897e5218a6..2f8af86bc2a0 100644 --- a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c +++ b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c @@ -31,7 +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 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; }; @@ -69,9 +68,6 @@ static int jadard_disable(struct drm_panel *panel) struct jadard *jadard = panel_to_jadard(panel); struct mipi_dsi_multi_context dsi_ctx = { .dsi = jadard->dsi }; - if (jadard->desc->backlight_off_to_display_off_delay_ms) - mipi_dsi_msleep(&dsi_ctx, jadard->desc->backlight_off_to_display_off_delay_ms); - mipi_dsi_dcs_set_display_off_multi(&dsi_ctx); if (jadard->desc->display_off_to_enter_sleep_delay_ms) @@ -858,7 +854,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, - .backlight_off_to_display_off_delay_ms = 100, .display_off_to_enter_sleep_delay_ms = 50, .enter_sleep_to_reset_down_delay_ms = 100, }; @@ -1109,7 +1104,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, - .backlight_off_to_display_off_delay_ms = 100, .display_off_to_enter_sleep_delay_ms = 50, .enter_sleep_to_reset_down_delay_ms = 100, };
In order to meet the timing, remove the delay between "backlight off" and "display off" Removing variables: display_off_to_enter_sleep_delay_ms Signed-off-by: Zhaoxiong Lv <lvzhaoxiong@huaqin.corp-partner.google.com> --- drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c | 6 ------ 1 file changed, 6 deletions(-)