Hi, On Fri, Oct 13, 2023 at 10:28 AM Doug Anderson <dianders@xxxxxxxxxx> wrote: > > Hi, > > On Thu, Oct 12, 2023 at 6:12 PM cong yang > <yangcong5@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote: > > > > Hi, > > > > On Thu, Oct 12, 2023 at 11:15 PM Doug Anderson <dianders@xxxxxxxxxx> wrote: > > > > > > Hi, > > > > > > On Thu, Oct 12, 2023 at 5:10 AM Cong Yang > > > <yangcong5@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote: > > > > > > > > At present, we have found that there may be a problem of blurred > > > > screen during fast sleep/resume. The direct cause of the blurred > > > > screen is that the IC does not receive 0x28/0x10. Because of the > > > > particularity of the IC, before the panel enters sleep hid must > > > > stop scanning, as i2c_hid_core_suspend before ili9882t_disable. > > > > If move the ili9882t_enter_sleep_mode function to ili9882t_unprepare, > > > > touch reset will pull low before panel entersleep, which does not meet > > > > the timing requirements.. > > > > > > The above makes me believe that the reset GPIO should be moved out of > > > the input driver and into the panel driver. I could just imagine that > > > the kernel might have some reason it wants to suspend the i2c hid > > > device. If that causes the panel to suddenly start failing then that > > > would be bad... I think we should fix this. > > > > Thanks, I will confirm with ilitek in further analysis and use "move > > the ili9882t_enter_sleep_mode > > function to ili9882t_unprepare". Is the test failure really because > > the touch reset timing > > does not match? There is also a separate reset GPIO on the panel. > > Shouldn't touch reset not > > affect the panel? > > > > If we find a better solution I will continue upstream,。 So is it > > possible to apply this plan now? > > I wouldn't be too upset at applying the current code as long as you're > going to continue to investigate. We can always continue to iterate on > it and having something working reasonably well is better than nothing > at all. However, I probably would wait at least 1 week before applying > any patch from you just simply out of courtesy to give others on the > mailing list time to express their comments. ...presumably we could > get to the bottom of the problem in that 1 week time anyway... > > I'm not trying to be an obstinate pain here--I'm merely trying to make > sure that whatever we land will continue to work across kernel uprevs, > even if driver probe order / timing changes in the kernel. If the > panel is really so tied to the touchscreen device's reset GPIO timing > then it worries me. What happens, for instance, if you disable the > touchscreen CONFIG in the kernel? Does the panel still work, or is > that extra reset GPIO totally critical to the functioning of the > panel. If it's totally critical then it probably makes sense to move > to the panel driver given that the touchscreen is a panel follower > anyway... Thanks. It looks like the panel works fine after I disable the touch screen device. So the panel may not depend on touch screen reset. Need to continue investigating the root cause for current status. > > > > > > So in order to solve this problem, the IC > > > > can handle it through the exception mechanism when it cannot receive > > > > 0x28/0x10 command. Handling exceptions requires a reset 50ms delay. > > > > Refer to vendor detailed analysis [1]. > > > > > > > > Ilitek vendor also suggested switching the page before entering sleep to > > > > avoid panel IC not receiving 0x28/0x10 command. > > > > > > > > Note: 0x28 is display off, 0x10 is sleep in. > > > > > > > > [1]: https://github.com/ILITEK-LoganLin/Document/tree/main/ILITEK_Power_Sequence > > > > > > > > Signed-off-by: Cong Yang <yangcong5@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> > > > > --- > > > > drivers/gpu/drm/panel/panel-ilitek-ili9882t.c | 22 ++++++++++++++++++- > > > > 1 file changed, 21 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c b/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c > > > > index 93a40c2f1483..54ff1efb94aa 100644 > > > > --- a/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c > > > > +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c > > > > @@ -463,6 +463,24 @@ static int ili9882t_init_dcs_cmd(struct ili9882t *ili) > > > > return 0; > > > > } > > > > > > > > +static int ili9882t_switch_page(struct mipi_dsi_device *dsi, u8 page) > > > > +{ > > > > + int ret; > > > > + const struct panel_init_cmd cmd = _INIT_SWITCH_PAGE_CMD(page); > > > > + > > > > + ret = mipi_dsi_dcs_write(dsi, cmd.data[0], > > > > + cmd.len <= 1 ? NULL : > > > > + &cmd.data[1], > > > > + cmd.len - 1); > > > > + if (ret) { > > > > + dev_err(&dsi->dev, > > > > + "error switching panel controller page (%d)\n", ret); > > > > + return ret; > > > > + } > > > > + > > > > + return 0; > > > > +} > > > > + > > > > static int ili9882t_enter_sleep_mode(struct ili9882t *ili) > > > > { > > > > struct mipi_dsi_device *dsi = ili->dsi; > > > > @@ -484,8 +502,10 @@ static int ili9882t_enter_sleep_mode(struct ili9882t *ili) > > > > static int ili9882t_disable(struct drm_panel *panel) > > > > { > > > > struct ili9882t *ili = to_ili9882t(panel); > > > > + struct mipi_dsi_device *dsi = ili->dsi; > > > > int ret; > > > > > > > > + ili9882t_switch_page(dsi, 0x00); > > > > ret = ili9882t_enter_sleep_mode(ili); > > > > if (ret < 0) { > > > > dev_err(panel->dev, "failed to set panel off: %d\n", ret); > > > > @@ -546,7 +566,7 @@ static int ili9882t_prepare(struct drm_panel *panel) > > > > gpiod_set_value(ili->enable_gpio, 1); > > > > usleep_range(1000, 2000); > > > > gpiod_set_value(ili->enable_gpio, 0); > > > > - usleep_range(1000, 2000); > > > > + usleep_range(50000, 51000); > > > > > > From my previous response, I think the above is better as msleep(50). > > > > Sorry. Will be corrected in V4. > > Thanks! It's not a huge deal, but it's nice to fix. > > -Doug
