drm/panel: Add Novatek NT36536 panel driver
Subsystems (4)
v2 cover drm/panel: Add Novatek NT36536 panel driver
Add a driver for panels using the Novatek NT36536 Display Driver IC, including support for the CSOT PP8807HB1-1, a dual-link 10-bit panel found in LENOVO Legion Y700 Gen4.
---
Changes in v2:
- Collect tags
- Mention the CSOT PP8807HB1-1 is a dual-link panel (Krzysztof)
- Use devm_drm_panel_add() (Neil)
- Link to v1: https://lore.kernel.org/dri-devel/[email protected]
Pengyu Luo (2):
dt-bindings: display: panel: Add Novatek NT36536 drm/panel: Add Novatek NT36536 panel driver
.../display/panel/novatek,nt36536.yaml | 91 ++++ drivers/gpu/drm/panel/Kconfig | 11 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-novatek-nt36536.c | 488 ++++++++++++++++++ 4 files changed, 591 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/novatek,nt36536.yaml create mode 100644 drivers/gpu/drm/panel/panel-novatek-nt36536.c
Signature · 1 line
2.54.0
PATCH v2 1/2 dt-bindings: display: panel: Add Novatek NT36536
NT36536 is a driver IC used to drive MIPI-DSI panels. It is found in LENOVO Legion Y700 Gen4 with a dual-link 10-bit CSOT panel.
---
v2:
- Collect tags
- Mention the CSOT PP8807HB1-1 is a dual-link panel (Krzysztof)
--- .../display/panel/novatek,nt36536.yaml | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/novatek,nt36536.yaml
Documentation/devicetree/bindings/display/panel/novatek,nt36536.yaml new file +91 -0
@@ -0,0 +1,91 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/panel/novatek,nt36536.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Novatek NT36536 based DSI display Panels + +maintainers: + - Pengyu Luo <[email protected]> + +description: + The Novatek NT36536 is a generic DSI Panel IC used to drive dsi + panels. Support video mode panels from China Star Optoelectronics + Technology (CSOT), such as PP8807HB1-1 which is a dual-link 10-bit + panel. + +allOf: + - $ref: panel-common-dual.yaml# + +properties: + compatible: + items: + - enum: + - csot,pp8807hb1-1 + - const: novatek,nt36536 + + reg: + maxItems: 1 + + reset-gpios: + maxItems: 1 + + vddio-supply: + description: I/O source voltage rail + + vsp-supply: + description: Positive source voltage rail + + vsn-supply: + description: Negative source voltage rail + + backlight: true + ports: true + +required: + - compatible + - reg + - vddio-supply + - reset-gpios + - ports + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + dsi { + #address-cells = <1>; + #size-cells = <0>; + + panel@0 { + compatible = "csot,pp8807hb1-1", "novatek,nt36536"; + reg = <0>; + + vddio-supply = <&vreg_iovdd_1p8>; + reset-gpios = <&tlmm 98 GPIO_ACTIVE_LOW>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + panel_in_0: endpoint { + remote-endpoint = <&dsi0_out>; + }; + }; + + port@1{ + reg = <1>; + panel_in_1: endpoint { + remote-endpoint = <&dsi1_out>; + }; + }; + }; + }; + }; + +...
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v2 1/2] dt-bindings: display: panel: Add Novatek NT36536
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Incorrect use of `additionalProperties: false` instead of `unevaluatedProperties: false` in a…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Incorrect use of additionalProperties: false instead of unevaluatedProperties: false in a schema containing an allOf reference. --
commit 40b975ec362c58607cb7c58d87a808388df1aa11
Author: Pengyu Luo <[email protected]>
dt-bindings: display: panel: Add Novatek NT36536
This commit adds a new devicetree binding schema for the Novatek NT36536 DSI panel IC, which supports video mode panels like the dual-link 10-bit CSOT PP8807HB1-1.
Quoted 5 lines
Documentation/devicetree/bindings/display/panel/novatek,nt36536.yaml new file +0
-0
[ ... ]
Quoted 4 lines
Documentation/devicetree/bindings/display/panel/novatek,nt36536.yaml new file +4
-0+allOf: + - $ref: panel-common-dual.yaml# + +properties:
[ ... ]
Documentation/devicetree/bindings/display/panel/novatek,nt36536.yaml line 44
Quoted 5 lines
Documentation/devicetree/bindings/display/panel/novatek,nt36536.yaml new file +5
-0+ vsn-supply: + description: Negative source voltage rail + + backlight: true + ports: true
[Severity: Low]
Should these properties be omitted? They are already defined in the referenced common schemas and appear to be redundantly redefined here solely because of how additionalProperties is evaluated below.
Documentation/devicetree/bindings/display/panel/novatek,nt36536.yaml line 53
Quoted 9 lines
Documentation/devicetree/bindings/display/panel/novatek,nt36536.yaml new file +9
-0+ +required: + - compatible + - reg + - vddio-supply + - reset-gpios + - ports + +additionalProperties: false
[Severity: Low]
Does this directive incorrectly restrict the schema since there is an allOf reference above? Using additionalProperties: false will cause dtbs_check to reject valid board configurations that use standard panel properties defined in panel-common-dual.yaml and panel-common.yaml (such as rotation or width-mm).
Should this be changed to unevaluatedProperties: false instead? Once that is corrected, the redundant backlight and ports entries above can likely be removed.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
PATCH v2 2/2 drm/panel: Add Novatek NT36536 panel driver
In reply to Pengyu LuoAdd a driver for panels using the Novatek NT36536 Display Driver IC, including support for the CSOT PP8807HB1-1, a dual-link 10-bit panel found in LENOVO Legion Y700 Gen4.
---
v2:
- Collect tags
- Use devm_drm_panel_add() (Neil)
--- drivers/gpu/drm/panel/Kconfig | 11 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-novatek-nt36536.c | 488 ++++++++++++++++++ 3 files changed, 500 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-novatek-nt36536.c
drivers/gpu/drm/panel/Kconfig +11 -0
@@ -662,6 +662,17 @@ config DRM_PANEL_NOVATEK_NT36523 around the Novatek NT36523 display controller, such as some Boe panels used in Xiaomi Mi Pad 5 and 5 Pro tablets. +config DRM_PANEL_NOVATEK_NT36536 + tristate "Novatek NT36536 panel driver" + depends on OF + depends on DRM_MIPI_DSI + depends on BACKLIGHT_CLASS_DEVICE + select DRM_KMS_HELPER + help + Say Y here if you want to enable support for Novatek NT36536-based + display panels, such as the one found in the LENOVO Legion Y700 + Gen4. + config DRM_PANEL_NOVATEK_NT36672A tristate "Novatek NT36672A DSI panel" depends on GPIOLIB
drivers/gpu/drm/panel/Makefile +1 -0
@@ -64,6 +64,7 @@ obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35532) += panel-novatek-nt35532.o obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35560) += panel-novatek-nt35560.o obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35950) += panel-novatek-nt35950.o obj-$(CONFIG_DRM_PANEL_NOVATEK_NT36523) += panel-novatek-nt36523.o +obj-$(CONFIG_DRM_PANEL_NOVATEK_NT36536) += panel-novatek-nt36536.o obj-$(CONFIG_DRM_PANEL_NOVATEK_NT36672A) += panel-novatek-nt36672a.o obj-$(CONFIG_DRM_PANEL_NOVATEK_NT36672E) += panel-novatek-nt36672e.o obj-$(CONFIG_DRM_PANEL_NOVATEK_NT37700F) += panel-novatek-nt37700f.o
drivers/gpu/drm/panel/panel-novatek-nt36536.c new file +488 -0
488 lines not on this page · raw message
Signature · 1 line
2.54.0
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider: - [Medium] Missing 120ms delay after MIPI_DCS_ENTER_SLEEP_MODE before asserting reset and removing power.…
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Medium] Missing 120ms delay after MIPI_DCS_ENTER_SLEEP_MODE before asserting reset and removing power.
- [Medium] The driver ignores transmission errors from DSC-related MIPI DSI multi-context commands in novatek_prepare(), masking failures and skipping proper hardware cleanup.
- [Medium] The backlight update_status callback races with the panel unprepare path, allowing DCS commands to be sent to an unpowered panel.
- [Low] Missing dependency for DRM DSC helpers in Kconfig, leading to undefined references at link time.
- [Low] Explicit invocation of drm_panel_remove() in the error path for a panel registered with devm_drm_panel_add().
--
commit 51663d35dbaba4a4aee3bdfad1a94da90787d8f6
Author: Pengyu Luo <[email protected]>
Subject: drm/panel: Add Novatek NT36536 panel driver
This commit adds a new driver for Novatek NT36536 display panels. It specifically implements support for the CSOT PP8807HB1-1 dual-link 10-bit panel used in the LENOVO Legion Y700 Gen4.
drivers/gpu/drm/panel/Kconfig config DRM_PANEL_NOVATEK_NT36523 · line 628
Quoted 13 lines
drivers/gpu/drm/panel/Kconfig +6
-0@@ -620,6 +620,17 @@ config DRM_PANEL_NOVATEK_NT36523 around the Novatek NT36523 display controller, such as some Boe panels used in Xiaomi Mi Pad 5 and 5 Pro tablets. +config DRM_PANEL_NOVATEK_NT36536 + tristate "Novatek NT36536 panel driver" + depends on OF + depends on DRM_MIPI_DSI + depends on BACKLIGHT_CLASS_DEVICE + select DRM_KMS_HELPER
[Severity: Low]
Does this Kconfig entry need to select DRM_DISPLAY_DSC_HELPER and DRM_DISPLAY_HELPER? Building this driver on a configuration that does not incidentally enable CONFIG_DRM_DISPLAY_DSC_HELPER could result in a build failure due to an undefined reference to drm_dsc_pps_payload_pack().
Quoted 5 lines
drivers/gpu/drm/panel/Kconfig +5
-0+ help + Say Y here if you want to enable support for Novatek NT36536-based + display panels, such as the one found in the LENOVO Legion Y700 + Gen4. +
[ ... ]
Quoted 3 lines
drivers/gpu/drm/panel/panel-novatek-nt36536.c new file +0
-0
[ ... ]
Quoted 2 lines
drivers/gpu/drm/panel/panel-novatek-nt36536.c new file +2
-0+static int novatek_prepare (struct drm_panel *panel) +{
[ ... ]
drivers/gpu/drm/panel/panel-novatek-nt36536.c line 114
Quoted 6 lines
drivers/gpu/drm/panel/panel-novatek-nt36536.c new file +6
-0+drm_dsc_pps_payload_pack (&pps, &ctx->dsc); +mipi_dsi_picture_parameter_set_multi (&dsi_ctx, &pps); +mipi_dsi_compression_mode_multi (&dsi_ctx, true); +mipi_dsi_msleep (&dsi_ctx,28 ); + +return backlight_enable (ctx->backlight);
[Severity: Medium]
Should the accumulated error in dsi_ctx.accum_err be checked before returning? If any of the DSC MIPI DSI commands fail, the error is recorded but never evaluated by novatek_prepare(). Returning the result of backlight_enable() directly could mask failures, skip proper hardware cleanup, and leave the panel in a corrupted state.
Quoted 12 lines
drivers/gpu/drm/panel/panel-novatek-nt36536.c new file +12
-0+} + +static int novatek_off (struct mipi_dsi_multi_context *dsi_ctx) +{ +mipi_dsi_dcs_set_display_off_multi (dsi_ctx); +mipi_dsi_dcs_enter_sleep_mode_multi (dsi_ctx); + +return dsi_ctx->accum_err; +} + +static int novatek_unprepare (struct drm_panel *panel) +{
[ ... ]
drivers/gpu/drm/panel/panel-novatek-nt36536.c line 140
Quoted 6 lines
drivers/gpu/drm/panel/panel-novatek-nt36536.c new file +6
-0+ ret =novatek_off (&dsi_ctx); +if (ret <0 ) +dev_err (dev,"Failed to un-initialize panel: %d\n" , ret); + +gpiod_set_value_cansleep (ctx->reset_gpio,1 ); +regulator_bulk_disable (ARRAY_SIZE (novatek_supplies), ctx->supplies);
[Severity: Medium]
Should there be a delay before removing power? novatek_off() sends the MIPI_DCS_ENTER_SLEEP_MODE command, but the regulators are disabled and the reset GPIO is asserted immediately without waiting. Removing power immediately after sending Sleep In can violate the MIPI DCS specification.
Quoted 3 lines
drivers/gpu/drm/panel/panel-novatek-nt36536.c new file +3
-0+ +return 0 ; +}
[ ... ]
drivers/gpu/drm/panel/panel-novatek-nt36536.c line 190
Quoted 7 lines
drivers/gpu/drm/panel/panel-novatek-nt36536.c new file +7
-0+static int novatek_bl_update_status (struct backlight_device *bl) +{ +struct novatek *ctx =bl_get_data (bl); +u16 brightness =backlight_get_brightness (bl); + +return mipi_dsi_dcs_set_display_brightness_large (to_primary_dsi (ctx), + brightness);
[Severity: Medium]
Could this DCS command be sent to an unpowered panel? A user-space write to the backlight sysfs interface can trigger novatek_bl_update_status() asynchronously. If this happens after novatek_unprepare() has disabled the regulators and asserted reset, it could result in DSI host timeouts or display controller lockups. Should there be a check to ensure the panel is currently prepared?
Quoted 1 line
+}
[ ... ]
Quoted 2 lines
drivers/gpu/drm/panel/panel-novatek-nt36536.c new file +2
-0+static int novatek_probe (struct mipi_dsi_device *dsi) +{
[ ... ]
Quoted 4 lines
drivers/gpu/drm/panel/panel-novatek-nt36536.c new file +4
-0+ ret =devm_drm_panel_add (dev, &ctx->panel); +if (ret <0 ) +return dev_err_probe (dev, ret,"failed to add panel\n" ); +
drivers/gpu/drm/panel/panel-novatek-nt36536.c line 435
Quoted 10 lines
drivers/gpu/drm/panel/panel-novatek-nt36536.c new file +10
-0+for (i =0 ; i < num_dsi; i++) { + ctx->dsi[i]->lanes = desc->lanes; + ctx->dsi[i]->format = desc->format; + ctx->dsi[i]->mode_flags = desc->mode_flags; + ctx->dsi[i]->dsc = &ctx->dsc; + ret =devm_mipi_dsi_attach (dev, ctx->dsi[i]); +if (ret <0 ) { +drm_panel_remove (&ctx->panel); +return dev_err_probe (dev, ret, +"Failed to attach to DSI host\n" );
[Severity: Low]
Is the manual drm_panel_remove() call necessary here? The panel was registered using the managed API devm_drm_panel_add(), which automatically handles cleanup. Calling drm_panel_remove() explicitly on the error path could cause the devres framework to execute it a second time.
Quoted 2 lines
drivers/gpu/drm/panel/panel-novatek-nt36536.c new file +2
-0+ } + }
[ ... ]
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
Hello,
Quoted 1 line · Pengyu Luo
+#include <
linux/mod_devicetable.h>
Please don't add new users for this header file. Just use the <linux/device-id/*.h> headers that you actually need.
Thanks
Uwe