Hi, On Fri, Oct 6, 2023 at 11:07 PM 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, i2c_hid_core_suspend before ili9882t_disable. > This doesn't look very spec-compliant. Presumably you could be more spec compliant if we used "panel_follower" in this case? Would that be a better solution? > 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 | 21 ++++++++++++++++++- > 1 file changed, 20 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c b/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c > index bbfcffe65623..0a1dd987b204 100644 > --- a/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c > +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c > @@ -423,6 +423,23 @@ static inline struct ili9882t *to_ili9882t(struct drm_panel *panel) > return container_of(panel, struct ili9882t, base); > } > > +static int ili9882t_switch_page(struct mipi_dsi_device *dsi, u8 page) > +{ > + u8 switch_cmd[] = {0x98, 0x82, 0x00}; Can't you just replace the last 0x00 above with "page" and get rid of the manual assignment below? > + int ret; > + > + switch_cmd[2] = page; > + > + ret = mipi_dsi_dcs_write(dsi, ILI9882T_DCS_SWITCH_PAGE, switch_cmd, 3); Instead of hardcoding 3, should use ARRAY_SIZE(). > + if (ret) { > + dev_err(&dsi->dev, > + "error switching panel controller page (%d)\n", ret); > + return ret; > + } > + > + return 0; > +} optional: It feels like it would be nice to somehow use the "_INIT_SWITCH_PAGE_CMD" macro I suggested in patch #1 instead of having to hardcode 0x98, 0x82 again. In patch #1 I already suggested breaking out the function to send a sequence of commands. If you had that function take a pointer instead of hardcoding it to look at ->init_cmds then you could probably use the same function that you do at init time? > static int ili9882t_enter_sleep_mode(struct ili9882t *ili) > { > struct mipi_dsi_device *dsi = ili->dsi; > @@ -444,8 +461,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); > @@ -507,7 +526,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(40000, 50000); nit: use 40000, 41000 instead of 40000, 50000. Linux almost always uses the longer delay, so that'll save ~9 ms. The only reason for the range is to optimize kernel wakeups which is really not a concern here.
