Re: [PATCH 2/2] drm/panel: Add Truly NT35521 panel driver
On Sun, Aug 08, 2021 at 05:29:30PM +0200, Stephan Gerhold wrote: > > 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. Indeed! I have CONFIG_DRM_MSM=y in my build, and changing it to module removes the problem. Thanks much for the hint, Stephan! Shawn
Re: [PATCH 2/2] drm/panel: Add Truly NT35521 panel driver
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
Re: [PATCH 2/2] drm/panel: Add Truly NT35521 panel driver
Hi Sam, On Wed, Aug 04, 2021 at 06:24:12PM +0200, Sam Ravnborg wrote: > Hi Shawn, > > see a few comments in the following. Thanks for the review comments! All of them will be addressed in v2. Shawn > On Wed, Aug 04, 2021 at 04:13:52PM +0800, Shawn Guo wrote: > > It adds a drm driver for Truly NT35521 5.24" 1280x720 DSI panel, which > > can be found on Sony Xperia M4 Aqua phone. The panel backlight is > > managed through DSI link. > > > > Signed-off-by: Shawn Guo > > --- > > drivers/gpu/drm/panel/Kconfig | 9 + > > drivers/gpu/drm/panel/Makefile | 1 + > > drivers/gpu/drm/panel/panel-truly-nt35521.c | 491 > > 3 files changed, 501 insertions(+) > > create mode 100644 drivers/gpu/drm/panel/panel-truly-nt35521.c
Re: [PATCH 2/2] drm/panel: Add Truly NT35521 panel driver
Hi Stephan, Thanks for looking at the patch! On Wed, Aug 04, 2021 at 02:09:19PM +0200, Stephan Gerhold wrote: > Hi Shawn, > > Thanks for the patch! > > On Wed, Aug 04, 2021 at 04:13:52PM +0800, Shawn Guo wrote: > > It adds a drm driver for Truly NT35521 5.24" 1280x720 DSI panel, which > > can be found on Sony Xperia M4 Aqua phone. The panel backlight is > > managed through DSI link. > > > > Signed-off-by: Shawn Guo > > --- > > drivers/gpu/drm/panel/Kconfig | 9 + > > drivers/gpu/drm/panel/Makefile | 1 + > > drivers/gpu/drm/panel/panel-truly-nt35521.c | 491 > > 3 files changed, 501 insertions(+) > > create mode 100644 drivers/gpu/drm/panel/panel-truly-nt35521.c > > > > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig > > index ef87d92cdf49..cdc4abd5c40c 100644 > > --- a/drivers/gpu/drm/panel/Kconfig > > +++ b/drivers/gpu/drm/panel/Kconfig > > @@ -537,6 +537,15 @@ config DRM_PANEL_TPO_TPG110 > > 400CH LTPS TFT LCD Single Chip Digital Driver for up to > > 800x400 LCD panels. > > > > +config DRM_PANEL_TRULY_NT35521 > > + tristate "Truly NT35521 panel" > > I think the name "Truly NT35521" is a bit too generic. AFAIK "Truly" is > a panel vendor and the NovaTek NT35521 is the panel controller. But > there are almost certainly other Truly panels that were also combined > with a NT35521 but need a slightly different configuration. > > If you don't know more than "Truly NT35521" based on the Sony sources, > maybe do it similar to "asus,z00t-tm5p5-n35596" and use a compatible > like "sony,-truly-nt35521". Would be good to clarify the Kconfig > option here too. Sounds good! > > > + depends on OF > > + depends on DRM_MIPI_DSI > > + depends on BACKLIGHT_CLASS_DEVICE > > + help > > + Say Y here if you want to enable support for Truly NT35521 > > + 1280x720 DSI panel. > > + > > config DRM_PANEL_TRULY_NT35597_WQXGA > > tristate "Truly WQXGA" > > depends on OF > > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile > > index cae4d976c069..3d3c98cb7a7b 100644 > > --- a/drivers/gpu/drm/panel/Makefile > > +++ b/drivers/gpu/drm/panel/Makefile > > @@ -56,6 +56,7 @@ obj-$(CONFIG_DRM_PANEL_TDO_TL070WSH30) += > > panel-tdo-tl070wsh30.o > > obj-$(CONFIG_DRM_PANEL_TPO_TD028TTEC1) += panel-tpo-td028ttec1.o > > obj-$(CONFIG_DRM_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o > > obj-$(CONFIG_DRM_PANEL_TPO_TPG110) += panel-tpo-tpg110.o > > +obj-$(CONFIG_DRM_PANEL_TRULY_NT35521) += panel-truly-nt35521.o > > obj-$(CONFIG_DRM_PANEL_TRULY_NT35597_WQXGA) += panel-truly-nt35597.o > > obj-$(CONFIG_DRM_PANEL_VISIONOX_RM69299) += panel-visionox-rm69299.o > > obj-$(CONFIG_DRM_PANEL_XINPENG_XPP055C272) += panel-xinpeng-xpp055c272.o > > diff --git a/drivers/gpu/drm/panel/panel-truly-nt35521.c > > b/drivers/gpu/drm/panel/panel-truly-nt35521.c > > new file mode 100644 > > index ..ea3cfb46be7e > > --- /dev/null > > +++ b/drivers/gpu/drm/panel/panel-truly-nt35521.c > > @@ -0,0 +1,491 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (c) 2021, Linaro Limited > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > + > > +#include > > +#include > > +#include > > +#include > > + > > +struct nt35521_panel { > > + struct drm_panel panel; > > + struct device *dev; > > + struct gpio_desc *rst_gpio; > > + struct gpio_desc *pwrp5_gpio; > > + struct gpio_desc *pwrn5_gpio; > > + struct gpio_desc *en_gpio; > > + bool prepared; > > + bool enabled; > > +}; > > + > > +static inline struct nt35521_panel *panel_to_nt35521(struct drm_panel > > *panel) > > +{ > > + return container_of(panel, struct nt35521_panel, panel); > > +} > > + > > +#define nt_dcs_write(seq...) > > \ > > +({ \ > > + const u8 d[] = { seq }; \ > > + if (mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)) < 0) \ > > + DRM_DEV_ERROR(dev, "dcs write buffer failed\n");\ > > +}) > > + > > +#define nt_gen_write(seq...) > > \ > > +({ \ > > + const u8 d[] = { seq }; \ > > + if (mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d)) < 0) \ > > + DRM_DEV_ERROR(dev, "generic write buffer failed\n");\ > > +}) > > + > > +static void nt35521_panel_on(struct nt35521_panel *nt) > > +{ > > + struct mipi_dsi_device *dsi = to_mipi_dsi_device(nt->dev); > > + struct device *dev = nt->dev; > > + > > + /* Transmit data in low power mode */ > > + dsi->mode_flags |= MIPI_DSI_MODE_LPM; > > + > > + nt_dcs_write(0xf0, 0x55, 0xaa, 0x52, 0x08, 0x00); > > + nt_dcs_write(0xff, 0xaa, 0x55, 0xa5, 0x80); > > + nt_dcs_wri
Re: [PATCH 2/2] drm/panel: Add Truly NT35521 panel driver
Hi Shawn, see a few comments in the following. Sam On Wed, Aug 04, 2021 at 04:13:52PM +0800, Shawn Guo wrote: > It adds a drm driver for Truly NT35521 5.24" 1280x720 DSI panel, which > can be found on Sony Xperia M4 Aqua phone. The panel backlight is > managed through DSI link. > > Signed-off-by: Shawn Guo > --- > drivers/gpu/drm/panel/Kconfig | 9 + > drivers/gpu/drm/panel/Makefile | 1 + > drivers/gpu/drm/panel/panel-truly-nt35521.c | 491 > 3 files changed, 501 insertions(+) > create mode 100644 drivers/gpu/drm/panel/panel-truly-nt35521.c > > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig > index ef87d92cdf49..cdc4abd5c40c 100644 > --- a/drivers/gpu/drm/panel/Kconfig > +++ b/drivers/gpu/drm/panel/Kconfig > @@ -537,6 +537,15 @@ config DRM_PANEL_TPO_TPG110 > 400CH LTPS TFT LCD Single Chip Digital Driver for up to > 800x400 LCD panels. > > +config DRM_PANEL_TRULY_NT35521 > + tristate "Truly NT35521 panel" > + depends on OF > + depends on DRM_MIPI_DSI > + depends on BACKLIGHT_CLASS_DEVICE > + help > + Say Y here if you want to enable support for Truly NT35521 > + 1280x720 DSI panel. Here you could mention that the first use is the Sone eXperia M4 Aqua to help the user. > + > config DRM_PANEL_TRULY_NT35597_WQXGA > tristate "Truly WQXGA" > depends on OF > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile > index cae4d976c069..3d3c98cb7a7b 100644 > --- a/drivers/gpu/drm/panel/Makefile > +++ b/drivers/gpu/drm/panel/Makefile > @@ -56,6 +56,7 @@ obj-$(CONFIG_DRM_PANEL_TDO_TL070WSH30) += > panel-tdo-tl070wsh30.o > obj-$(CONFIG_DRM_PANEL_TPO_TD028TTEC1) += panel-tpo-td028ttec1.o > obj-$(CONFIG_DRM_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o > obj-$(CONFIG_DRM_PANEL_TPO_TPG110) += panel-tpo-tpg110.o > +obj-$(CONFIG_DRM_PANEL_TRULY_NT35521) += panel-truly-nt35521.o > obj-$(CONFIG_DRM_PANEL_TRULY_NT35597_WQXGA) += panel-truly-nt35597.o > obj-$(CONFIG_DRM_PANEL_VISIONOX_RM69299) += panel-visionox-rm69299.o > obj-$(CONFIG_DRM_PANEL_XINPENG_XPP055C272) += panel-xinpeng-xpp055c272.o > diff --git a/drivers/gpu/drm/panel/panel-truly-nt35521.c > b/drivers/gpu/drm/panel/panel-truly-nt35521.c > new file mode 100644 > index ..ea3cfb46be7e > --- /dev/null > +++ b/drivers/gpu/drm/panel/panel-truly-nt35521.c > @@ -0,0 +1,491 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2021, Linaro Limited > + */ > + > +#include > +#include > +#include > +#include > + > +#include > + > +#include > +#include > +#include > +#include drm_print is not used by panel drivers - this also imply that you need to replace all uses of DRM_ERROR and friends. > + > +struct nt35521_panel { > + struct drm_panel panel; > + struct device *dev; > + struct gpio_desc *rst_gpio; > + struct gpio_desc *pwrp5_gpio; > + struct gpio_desc *pwrn5_gpio; > + struct gpio_desc *en_gpio; > + bool prepared; > + bool enabled; > +}; > + > +static inline struct nt35521_panel *panel_to_nt35521(struct drm_panel *panel) > +{ > + return container_of(panel, struct nt35521_panel, panel); > +} > + > +#define nt_dcs_write(seq...) \ > +({ \ > + const u8 d[] = { seq }; \ > + if (mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)) < 0) \ > + DRM_DEV_ERROR(dev, "dcs write buffer failed\n");\ > +}) So in case of an error the code just continues with the next write, that likely also errors out. Please see what is for example implemented in panel-elida-kd35t133.c That pattern looks much saner. > + > +#define nt_gen_write(seq...) \ > +({ \ > + const u8 d[] = { seq }; \ > + if (mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d)) < 0) \ > + DRM_DEV_ERROR(dev, "generic write buffer failed\n");\ > +}) For the two uses, please consider open coding this. > + > +static void nt35521_panel_on(struct nt35521_panel *nt) > +{ > + struct mipi_dsi_device *dsi = to_mipi_dsi_device(nt->dev); > + struct device *dev = nt->dev; > + > + /* Transmit data in low power mode */ > + dsi->mode_flags |= MIPI_DSI_MODE_LPM; > + > + nt_dcs_write(0xf0, 0x55, 0xaa, 0x52, 0x08, 0x00); > + nt_dcs_write(0xff, 0xaa, 0x55, 0xa5, 0x80); > + nt_dcs_write(0x6f, 0x11, 0x00); > + nt_dcs_write(0xf7, 0x20, 0x00); > + nt_dcs_write(0x6f, 0x01); > + nt_dcs_write(0xb1, 0x21); > + nt_dcs_write(0xbd, 0x01, 0xa0, 0x10, 0x08, 0x01); > + nt_dcs_write(0xb8, 0x01, 0x02, 0x0c, 0x02); > + nt_dcs_write(0xbb, 0x11, 0x11); > + nt_dcs_write(0xbc, 0x00, 0x00); > + nt_dcs_write(0xb
Re: [PATCH 2/2] drm/panel: Add Truly NT35521 panel driver
Hi Shawn, Thanks for the patch! On Wed, Aug 04, 2021 at 04:13:52PM +0800, Shawn Guo wrote: > It adds a drm driver for Truly NT35521 5.24" 1280x720 DSI panel, which > can be found on Sony Xperia M4 Aqua phone. The panel backlight is > managed through DSI link. > > Signed-off-by: Shawn Guo > --- > drivers/gpu/drm/panel/Kconfig | 9 + > drivers/gpu/drm/panel/Makefile | 1 + > drivers/gpu/drm/panel/panel-truly-nt35521.c | 491 > 3 files changed, 501 insertions(+) > create mode 100644 drivers/gpu/drm/panel/panel-truly-nt35521.c > > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig > index ef87d92cdf49..cdc4abd5c40c 100644 > --- a/drivers/gpu/drm/panel/Kconfig > +++ b/drivers/gpu/drm/panel/Kconfig > @@ -537,6 +537,15 @@ config DRM_PANEL_TPO_TPG110 > 400CH LTPS TFT LCD Single Chip Digital Driver for up to > 800x400 LCD panels. > > +config DRM_PANEL_TRULY_NT35521 > + tristate "Truly NT35521 panel" I think the name "Truly NT35521" is a bit too generic. AFAIK "Truly" is a panel vendor and the NovaTek NT35521 is the panel controller. But there are almost certainly other Truly panels that were also combined with a NT35521 but need a slightly different configuration. If you don't know more than "Truly NT35521" based on the Sony sources, maybe do it similar to "asus,z00t-tm5p5-n35596" and use a compatible like "sony,-truly-nt35521". Would be good to clarify the Kconfig option here too. > + depends on OF > + depends on DRM_MIPI_DSI > + depends on BACKLIGHT_CLASS_DEVICE > + help > + Say Y here if you want to enable support for Truly NT35521 > + 1280x720 DSI panel. > + > config DRM_PANEL_TRULY_NT35597_WQXGA > tristate "Truly WQXGA" > depends on OF > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile > index cae4d976c069..3d3c98cb7a7b 100644 > --- a/drivers/gpu/drm/panel/Makefile > +++ b/drivers/gpu/drm/panel/Makefile > @@ -56,6 +56,7 @@ obj-$(CONFIG_DRM_PANEL_TDO_TL070WSH30) += > panel-tdo-tl070wsh30.o > obj-$(CONFIG_DRM_PANEL_TPO_TD028TTEC1) += panel-tpo-td028ttec1.o > obj-$(CONFIG_DRM_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o > obj-$(CONFIG_DRM_PANEL_TPO_TPG110) += panel-tpo-tpg110.o > +obj-$(CONFIG_DRM_PANEL_TRULY_NT35521) += panel-truly-nt35521.o > obj-$(CONFIG_DRM_PANEL_TRULY_NT35597_WQXGA) += panel-truly-nt35597.o > obj-$(CONFIG_DRM_PANEL_VISIONOX_RM69299) += panel-visionox-rm69299.o > obj-$(CONFIG_DRM_PANEL_XINPENG_XPP055C272) += panel-xinpeng-xpp055c272.o > diff --git a/drivers/gpu/drm/panel/panel-truly-nt35521.c > b/drivers/gpu/drm/panel/panel-truly-nt35521.c > new file mode 100644 > index ..ea3cfb46be7e > --- /dev/null > +++ b/drivers/gpu/drm/panel/panel-truly-nt35521.c > @@ -0,0 +1,491 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2021, Linaro Limited > + */ > + > +#include > +#include > +#include > +#include > + > +#include > + > +#include > +#include > +#include > +#include > + > +struct nt35521_panel { > + struct drm_panel panel; > + struct device *dev; > + struct gpio_desc *rst_gpio; > + struct gpio_desc *pwrp5_gpio; > + struct gpio_desc *pwrn5_gpio; > + struct gpio_desc *en_gpio; > + bool prepared; > + bool enabled; > +}; > + > +static inline struct nt35521_panel *panel_to_nt35521(struct drm_panel *panel) > +{ > + return container_of(panel, struct nt35521_panel, panel); > +} > + > +#define nt_dcs_write(seq...) \ > +({ \ > + const u8 d[] = { seq }; \ > + if (mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)) < 0) \ > + DRM_DEV_ERROR(dev, "dcs write buffer failed\n");\ > +}) > + > +#define nt_gen_write(seq...) \ > +({ \ > + const u8 d[] = { seq }; \ > + if (mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d)) < 0) \ > + DRM_DEV_ERROR(dev, "generic write buffer failed\n");\ > +}) > + > +static void nt35521_panel_on(struct nt35521_panel *nt) > +{ > + struct mipi_dsi_device *dsi = to_mipi_dsi_device(nt->dev); > + struct device *dev = nt->dev; > + > + /* Transmit data in low power mode */ > + dsi->mode_flags |= MIPI_DSI_MODE_LPM; > + > + nt_dcs_write(0xf0, 0x55, 0xaa, 0x52, 0x08, 0x00); > + nt_dcs_write(0xff, 0xaa, 0x55, 0xa5, 0x80); > + nt_dcs_write(0x6f, 0x11, 0x00); > + nt_dcs_write(0xf7, 0x20, 0x00); > + nt_dcs_write(0x6f, 0x01); > + nt_dcs_write(0xb1, 0x21); > + nt_dcs_write(0xbd, 0x01, 0xa0, 0x10, 0x08, 0x01); > + nt_dcs_write(0xb8, 0x01, 0x02, 0x0c, 0x02); > + nt_dcs_write(0xbb, 0x11, 0x11); > + nt_dcs_write(0xbc, 0x00, 0x00); > + nt_dcs_w