Re: [PATCH 2/2] drm/panel: Add Truly NT35521 panel driver

[Stephan Gerhold <stephan@gerhold.net>]

On Sun, Aug 08, 2021 at 09:44:57PM +0800, Shawn Guo wrote:
> On Wed, Aug 04, 2021 at 02:09:19PM +0200, Stephan Gerhold wrote:
> > On Wed, Aug 04, 2021 at 04:13:52PM +0800, Shawn Guo wrote:
> > > + ...
> > > +	nt_dcs_write(0xb1, 0x6c, 0x21);
> > > +	nt_dcs_write(0xf0, 0x55, 0xaa, 0x52, 0x00, 0x00);
> > > +	nt_dcs_write(0x35, 0x00);
> > > +	nt_gen_write(0x11, 0x00);
> > > +	msleep(120);
> > > +	nt_gen_write(0x29, 0x00);
> > > +	usleep_range(1000, 1500);
> > > +	nt_dcs_write(0x53, 0x24);
> > 
> > Did you mix up "nt_dcs_write" and "nt_gen_write" here?
> > The nt_gen_write(0x11, 0x00); looks like MIPI_DCS_EXIT_SLEEP_MODE
> > and the nt_gen_write(0x29, 0x00); looks like MIPI_DCS_SET_DISPLAY_ON.
> > 
> > For reference you can pull your original reference DTB from Sony through
> > my panel driver generator: https://github.com/msm8916-mainline/linux-mdss-dsi-panel-driver-generator
> 
> Wow, very nice!  It really deserves wider spread!
> 
> > 
> > It produces the following (I compiled "msm8939-kanuti_tulip.dtb"
> > from https://github.com/sonyxperiadev/kernel/tree/aosp/LA.BR.1.3.3_rb2.14,
> > not sure if that is right):
> > 
> > 	// ...
> > 	dsi_generic_write_seq(dsi, 0x35, 0x00);
> > 
> > 	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
> > 	if (ret < 0) {
> > 		dev_err(dev, "Failed to exit sleep mode: %d\n", ret);
> > 		return ret;
> > 	}
> > 	msleep(120);
> > 
> > 	ret = mipi_dsi_dcs_set_display_on(dsi);
> > 	if (ret < 0) {
> > 		dev_err(dev, "Failed to set display on: %d\n", ret);
> > 		return ret;
> > 	}
> > 	usleep_range(1000, 2000);
> > 
> > 	dsi_generic_write_seq(dsi, 0x53, 0x24);
> > 
> > Which also suggests that generic and DCS writes are mixed up here.
> > 
> > Note however that you could not use the generated driver as-is,
> > because Sony seems to use their own display driver instead of Qualcomm's
> > and some things seem to be different.
> 
> I re-created the driver using your generator.  With modeling the 5v
> control GPIOs with regulators and adding backlight-gpios support, the
> driver works quite nicely, except the following two problems:
> 
> 1) I have to drop the MIPI_DSI_MODE_LPM configuration from .update_status
>    hook. Otherwise brightness did not get updated to panel.
> 
> ---8<------
> diff --git a/drivers/gpu/drm/panel/panel-sony-tulip-truly-nt35521.c b/drivers/gpu/drm/panel/panel-sony-tulip-truly-nt35521.c
> index 31e5f942a039..eba926c6f722 100644
> --- a/drivers/gpu/drm/panel/panel-sony-tulip-truly-nt35521.c
> +++ b/drivers/gpu/drm/panel/panel-sony-tulip-truly-nt35521.c
> @@ -420,33 +420,23 @@ static int truly_nt35521_bl_update_status(struct backlight_device *bl)
>         u16 brightness = backlight_get_brightness(bl);
>         int ret;
>  
> -       dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> -
>         ret = mipi_dsi_dcs_set_display_brightness(dsi, brightness);
>         if (ret < 0)
>                 return ret;
>  
> -       dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> -
>         return 0;
>  }
> ------>8---
> 

I have to admit I don't know much about Low Power Mode vs High Speed
Mode. As long it works it is good I guess :-)

> 2) The driver works good, if the kernel is launched via "fastboot boot".
>    But if the kernel is flashed to eMMC and launched by bootloader with
>    splash screen, kernel will fail to bring up the panel.  After kernel
>    boots up, a blank & unblank cycle can get panel work though.
> 
> The problem 2) is not driver generator related.  @Konrad, did you see
> it on asus-z00t-tm5p5-n35596 driver?
> 

Do you have CONFIG_DRM_MSM=y (built-in) instead of =m (module) maybe?
I think a similar issue exists on MSM8916 but it does not happen
for some reason if CONFIG_DRM_MSM=m instead of =y. Somehow having it
load later during the boot process fixes some things there.

Thanks,
Stephan