* [PATCH v2 1/6] dt-bindings: arm: qcom: document google, bonito-tianma board
2026-02-17 0:08 [PATCH v2 0/6] Support for the Pixel 3a XL with the Tianma panel Richard Acayan
@ 2026-02-17 0:08 ` Richard Acayan
2026-02-17 5:51 ` Claude review: " Claude Code Review Bot
2026-02-17 0:08 ` [PATCH v2 2/6] dt-bindings: panel-simple-dsi: add nt37700f compatible Richard Acayan
` (5 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Richard Acayan @ 2026-02-17 0:08 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Thierry Reding, Sam Ravnborg, linux-arm-msm, devicetree,
dri-devel
Cc: yifei, Richard Acayan
The Pixel 3a XL is code-named by Google as "bonito". The
google,bonito-tianma compatible represents a variant of the Pixel 3a XL
board with a Tianma/Novatek NT37700F panel. Document the google,bonito
board compatible and the google,bonito-tianma variant in the bindings.
Signed-off-by: Richard Acayan <mailingradian@gmail.com>
---
Documentation/devicetree/bindings/arm/qcom.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index d84bd3bca201..59118faffb02 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -831,6 +831,12 @@ properties:
- google,sargo
- const: qcom,sdm670
+ - items:
+ - enum:
+ - google,bonito-tianma
+ - const: google,bonito
+ - const: qcom,sdm670
+
- items:
- enum:
- qcom,sdx55-mtp
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v2 2/6] dt-bindings: panel-simple-dsi: add nt37700f compatible
2026-02-17 0:08 [PATCH v2 0/6] Support for the Pixel 3a XL with the Tianma panel Richard Acayan
2026-02-17 0:08 ` [PATCH v2 1/6] dt-bindings: arm: qcom: document google, bonito-tianma board Richard Acayan
@ 2026-02-17 0:08 ` Richard Acayan
2026-02-17 5:51 ` Claude review: " Claude Code Review Bot
2026-02-17 0:08 ` [PATCH v2 3/6] drm/panel: Add Novatek/Tianma NT37700F panel Richard Acayan
` (4 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Richard Acayan @ 2026-02-17 0:08 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Thierry Reding, Sam Ravnborg, linux-arm-msm, devicetree,
dri-devel
Cc: yifei, Richard Acayan
Add the compatible for the NT37700F panel found on the Pixel 3a XL.
Signed-off-by: Richard Acayan <mailingradian@gmail.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
.../devicetree/bindings/display/panel/panel-simple-dsi.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
index 8d668979b62d..e56865d2771b 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
@@ -49,6 +49,8 @@ properties:
- lg,lh500wx1-sd03
# Lincoln LCD197 5" 1080x1920 LCD panel
- lincolntech,lcd197
+ # Novatek NT37700F 1080x2160 AMOLED panel
+ - novatek,nt37700f
# One Stop Displays OSD101T2587-53TS 10.1" 1920x1200 panel
- osddisplays,osd101t2587-53ts
# Panasonic 10" WUXGA TFT LCD panel
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Claude review: dt-bindings: panel-simple-dsi: add nt37700f compatible
2026-02-17 0:08 ` [PATCH v2 2/6] dt-bindings: panel-simple-dsi: add nt37700f compatible Richard Acayan
@ 2026-02-17 5:51 ` Claude Code Review Bot
0 siblings, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-02-17 5:51 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
> + # Novatek NT37700F 1080x2160 AMOLED panel
> + - novatek,nt37700f
Adding the compatible to panel-simple-dsi.yaml while patch 3 adds a dedicated panel driver might seem contradictory given the yaml file's note that "If the panel is more advanced a dedicated binding file is required." However, there is precedent for this pattern (e.g., `lincolntech,lcd197` is listed in panel-simple-dsi.yaml but also has a dedicated driver). No issues, though a dedicated binding yaml might be cleaner long-term.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 3/6] drm/panel: Add Novatek/Tianma NT37700F panel
2026-02-17 0:08 [PATCH v2 0/6] Support for the Pixel 3a XL with the Tianma panel Richard Acayan
2026-02-17 0:08 ` [PATCH v2 1/6] dt-bindings: arm: qcom: document google, bonito-tianma board Richard Acayan
2026-02-17 0:08 ` [PATCH v2 2/6] dt-bindings: panel-simple-dsi: add nt37700f compatible Richard Acayan
@ 2026-02-17 0:08 ` Richard Acayan
2026-02-17 5:51 ` Claude review: " Claude Code Review Bot
2026-02-17 0:08 ` [PATCH v2 4/6] arm64: dts: qcom: sdm670-google-sargo: add touchscreen and display labels Richard Acayan
` (3 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Richard Acayan @ 2026-02-17 0:08 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Thierry Reding, Sam Ravnborg, linux-arm-msm, devicetree,
dri-devel
Cc: yifei, Richard Acayan
Some Pixel 3a XL devices have a Tianma panel. Add support for it, with
the aid of linux-mdss-dsi-panel-driver-generator.
Link: https://github.com/msm8916-mainline/linux-mdss-dsi-panel-driver-generator
Signed-off-by: Richard Acayan <mailingradian@gmail.com>
Tested-by: Yifei Zhan <yifei@zhan.science>
---
drivers/gpu/drm/panel/Kconfig | 9 +
drivers/gpu/drm/panel/Makefile | 1 +
.../gpu/drm/panel/panel-novatek-nt37700f.c | 282 ++++++++++++++++++
3 files changed, 292 insertions(+)
create mode 100644 drivers/gpu/drm/panel/panel-novatek-nt37700f.c
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 76f6af819037..138d617e8195 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -553,6 +553,15 @@ config DRM_PANEL_NOVATEK_NT36672E
LCD panel module. The panel has a resolution of 1080x2408 and uses 24 bit
RGB per pixel.
+config DRM_PANEL_NOVATEK_NT37700F
+ tristate "Novatek NT37700F DSI 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 Novatek NT37700F DSI
+ panel module. The panel has a resolution of 1080x2160.
+
config DRM_PANEL_NOVATEK_NT37801
tristate "Novatek NT37801/NT37810 AMOLED DSI panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index b9562a6fdcb3..9218a7d7ff34 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -54,6 +54,7 @@ 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_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
obj-$(CONFIG_DRM_PANEL_NOVATEK_NT37801) += panel-novatek-nt37801.o
obj-$(CONFIG_DRM_PANEL_NOVATEK_NT39016) += panel-novatek-nt39016.o
obj-$(CONFIG_DRM_PANEL_MANTIX_MLAF057WE51) += panel-mantix-mlaf057we51.o
diff --git a/drivers/gpu/drm/panel/panel-novatek-nt37700f.c b/drivers/gpu/drm/panel/panel-novatek-nt37700f.c
new file mode 100644
index 000000000000..e7e6343b2963
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-novatek-nt37700f.c
@@ -0,0 +1,282 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2024, The Linux Foundation. All rights reserved.
+ * Generated with linux-mdss-dsi-panel-driver-generator from vendor device tree:
+ * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of.h>
+
+#include <video/mipi_display.h>
+
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_probe_helper.h>
+
+struct nt37700f_tianma {
+ struct drm_panel panel;
+ struct mipi_dsi_device *dsi;
+ struct gpio_desc *reset_gpio;
+};
+
+static inline
+struct nt37700f_tianma *to_nt37700f_tianma(struct drm_panel *panel)
+{
+ return container_of(panel, struct nt37700f_tianma, panel);
+}
+
+static void nt37700f_tianma_reset(struct nt37700f_tianma *ctx)
+{
+ gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+ usleep_range(1000, 2000);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ usleep_range(10000, 11000);
+}
+
+static int nt37700f_tianma_on(struct nt37700f_tianma *ctx)
+{
+ struct mipi_dsi_device *dsi = ctx->dsi;
+ struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
+
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xc0, 0x56);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xca, 0x52);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x06);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xb5, 0x2b, 0x1a);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x01);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xcd, 0x04, 0x82);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x02);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xcc, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xff, 0xaa, 0x55, 0xa5, 0x80);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x55);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf6, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x56);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf6, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xff, 0xaa, 0x55, 0xa5, 0x81);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x07);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf3, 0x07);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x05);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf3, 0x25);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x90, 0x01);
+
+ mipi_dsi_dcs_set_column_address_multi(&dsi_ctx, 0x0000, 1080 - 1);
+ mipi_dsi_dcs_set_page_address_multi(&dsi_ctx, 0x0000, 2160 - 1);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_WRITE_CONTROL_DISPLAY, 0x20);
+ mipi_dsi_dcs_set_tear_on_multi(&dsi_ctx, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
+
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xc0, 0x56);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x02);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xcd, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x04);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xd0, 0x11, 0x64);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x09);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xb1, 0x20);
+
+ mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+ mipi_dsi_msleep(&dsi_ctx, 120);
+ mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
+
+ return dsi_ctx.accum_err;
+}
+
+static int nt37700f_tianma_disable(struct drm_panel *panel)
+{
+ struct nt37700f_tianma *ctx = to_nt37700f_tianma(panel);
+ struct mipi_dsi_device *dsi = ctx->dsi;
+ struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
+
+ mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
+ mipi_dsi_msleep(&dsi_ctx, 50);
+ mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
+ mipi_dsi_msleep(&dsi_ctx, 100);
+
+ return dsi_ctx.accum_err;
+}
+
+static int nt37700f_tianma_prepare(struct drm_panel *panel)
+{
+ struct nt37700f_tianma *ctx = to_nt37700f_tianma(panel);
+ struct device *dev = &ctx->dsi->dev;
+ int ret;
+
+ nt37700f_tianma_reset(ctx);
+
+ ret = nt37700f_tianma_on(ctx);
+ if (ret < 0) {
+ dev_err(dev, "Failed to initialize panel: %d\n", ret);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int nt37700f_tianma_unprepare(struct drm_panel *panel)
+{
+ struct nt37700f_tianma *ctx = to_nt37700f_tianma(panel);
+
+ gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+
+ return 0;
+}
+
+static const struct drm_display_mode nt37700f_tianma_mode = {
+ .clock = (1080 + 32 + 32 + 98) * (2160 + 32 + 4 + 98) * 60 / 1000,
+ .hdisplay = 1080,
+ .hsync_start = 1080 + 32,
+ .hsync_end = 1080 + 32 + 32,
+ .htotal = 1080 + 32 + 32 + 98,
+ .vdisplay = 2160,
+ .vsync_start = 2160 + 32,
+ .vsync_end = 2160 + 32 + 4,
+ .vtotal = 2160 + 32 + 4 + 98,
+ .width_mm = 69,
+ .height_mm = 137,
+ .type = DRM_MODE_TYPE_DRIVER,
+};
+
+static int nt37700f_tianma_get_modes(struct drm_panel *panel,
+ struct drm_connector *connector)
+{
+ return drm_connector_helper_get_modes_fixed(connector, &nt37700f_tianma_mode);
+}
+
+static const struct drm_panel_funcs nt37700f_tianma_panel_funcs = {
+ .prepare = nt37700f_tianma_prepare,
+ .unprepare = nt37700f_tianma_unprepare,
+ .disable = nt37700f_tianma_disable,
+ .get_modes = nt37700f_tianma_get_modes,
+};
+
+static int nt37700f_tianma_bl_update_status(struct backlight_device *bl)
+{
+ struct mipi_dsi_device *dsi = bl_get_data(bl);
+ u16 brightness = backlight_get_brightness(bl);
+ int ret;
+
+ dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+ ret = mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);
+ if (ret < 0)
+ return ret;
+
+ dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+ return 0;
+}
+
+static int nt37700f_tianma_bl_get_brightness(struct backlight_device *bl)
+{
+ struct mipi_dsi_device *dsi = bl_get_data(bl);
+ u16 brightness;
+ int ret;
+
+ dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+ ret = mipi_dsi_dcs_get_display_brightness_large(dsi, &brightness);
+ if (ret < 0)
+ return ret;
+
+ dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+ return brightness;
+}
+
+static const struct backlight_ops nt37700f_tianma_bl_ops = {
+ .update_status = nt37700f_tianma_bl_update_status,
+ .get_brightness = nt37700f_tianma_bl_get_brightness,
+};
+
+static struct backlight_device *
+nt37700f_tianma_create_backlight(struct mipi_dsi_device *dsi)
+{
+ struct device *dev = &dsi->dev;
+ const struct backlight_properties props = {
+ .type = BACKLIGHT_RAW,
+ .brightness = 2047,
+ .max_brightness = 2047,
+ };
+
+ return devm_backlight_device_register(dev, dev_name(dev), dev, dsi,
+ &nt37700f_tianma_bl_ops, &props);
+}
+
+static int nt37700f_tianma_probe(struct mipi_dsi_device *dsi)
+{
+ struct device *dev = &dsi->dev;
+ struct nt37700f_tianma *ctx;
+ int ret;
+
+ ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+ if (!ctx)
+ return -ENOMEM;
+
+ ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(ctx->reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio),
+ "Failed to get reset-gpios\n");
+
+ ctx->dsi = dsi;
+ mipi_dsi_set_drvdata(dsi, ctx);
+
+ dsi->lanes = 4;
+ dsi->format = MIPI_DSI_FMT_RGB888;
+ dsi->mode_flags = MIPI_DSI_MODE_VIDEO_BURST |
+ MIPI_DSI_CLOCK_NON_CONTINUOUS | MIPI_DSI_MODE_LPM;
+
+ drm_panel_init(&ctx->panel, dev, &nt37700f_tianma_panel_funcs,
+ DRM_MODE_CONNECTOR_DSI);
+ ctx->panel.prepare_prev_first = true;
+
+ ctx->panel.backlight = nt37700f_tianma_create_backlight(dsi);
+ if (IS_ERR(ctx->panel.backlight))
+ return dev_err_probe(dev, PTR_ERR(ctx->panel.backlight),
+ "Failed to create backlight\n");
+
+ drm_panel_add(&ctx->panel);
+
+ ret = mipi_dsi_attach(dsi);
+ if (ret < 0) {
+ drm_panel_remove(&ctx->panel);
+ return dev_err_probe(dev, ret, "Failed to attach to DSI host\n");
+ }
+
+ return 0;
+}
+
+static void nt37700f_tianma_remove(struct mipi_dsi_device *dsi)
+{
+ struct nt37700f_tianma *ctx = mipi_dsi_get_drvdata(dsi);
+ int ret;
+
+ ret = mipi_dsi_detach(dsi);
+ if (ret < 0)
+ dev_err(&dsi->dev, "Failed to detach from DSI host: %d\n", ret);
+
+ drm_panel_remove(&ctx->panel);
+}
+
+static const struct of_device_id nt37700f_tianma_of_match[] = {
+ { .compatible = "novatek,nt37700f" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, nt37700f_tianma_of_match);
+
+static struct mipi_dsi_driver nt37700f_tianma_driver = {
+ .probe = nt37700f_tianma_probe,
+ .remove = nt37700f_tianma_remove,
+ .driver = {
+ .name = "panel-novatek-nt37700f",
+ .of_match_table = nt37700f_tianma_of_match,
+ },
+};
+module_mipi_dsi_driver(nt37700f_tianma_driver);
+
+MODULE_DESCRIPTION("DRM driver for nt37700f cmd mode dsi tianma panel");
+MODULE_LICENSE("GPL");
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Claude review: drm/panel: Add Novatek/Tianma NT37700F panel
2026-02-17 0:08 ` [PATCH v2 3/6] drm/panel: Add Novatek/Tianma NT37700F panel Richard Acayan
@ 2026-02-17 5:51 ` Claude Code Review Bot
0 siblings, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-02-17 5:51 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
The panel driver follows established patterns. A few observations:
> +static void nt37700f_tianma_reset(struct nt37700f_tianma *ctx)
> +{
> + gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> + usleep_range(1000, 2000);
> + gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> + usleep_range(10000, 11000);
> +}
The reset GPIO is requested with `GPIOD_OUT_HIGH` in probe, meaning the panel starts in reset. The reset sequence asserts (1), waits, deasserts (0), waits. This is consistent with the `unprepare` path which sets the GPIO to 1 (asserting reset). Fine.
> + dsi->mode_flags = MIPI_DSI_MODE_VIDEO_BURST |
> + MIPI_DSI_CLOCK_NON_CONTINUOUS | MIPI_DSI_MODE_LPM;
The mode flags combine video burst mode with LPM for command sequences. This is a standard pattern seen in other panel drivers (e.g., `panel-samsung-sofef00.c`).
> +static int nt37700f_tianma_bl_update_status(struct backlight_device *bl)
> +{
> + struct mipi_dsi_device *dsi = bl_get_data(bl);
> + u16 brightness = backlight_get_brightness(bl);
> + int ret;
> +
> + dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> +
> + ret = mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);
> + if (ret < 0)
> + return ret;
> +
> + dsi->mode_flags |= MIPI_DSI_MODE_LPM;
Temporarily clearing and restoring `MIPI_DSI_MODE_LPM` to send brightness commands in HS mode is not thread-safe -- if two callers hit this concurrently, the flag manipulation could race. That said, this is a common pattern found in many generated panel drivers and the backlight framework serializes calls through a mutex, so in practice this is fine.
> + * Copyright (c) 2024, The Linux Foundation. All rights reserved.
Minor: the copyright year says 2024, though the patch is being submitted in 2026. This is cosmetic and doesn't affect functionality. Also, attributing a generated driver to "The Linux Foundation" may not be accurate -- typically the author of the driver would be listed.
Overall, the driver is straightforward and correct. No functional bugs found.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 4/6] arm64: dts: qcom: sdm670-google-sargo: add touchscreen and display labels
2026-02-17 0:08 [PATCH v2 0/6] Support for the Pixel 3a XL with the Tianma panel Richard Acayan
` (2 preceding siblings ...)
2026-02-17 0:08 ` [PATCH v2 3/6] drm/panel: Add Novatek/Tianma NT37700F panel Richard Acayan
@ 2026-02-17 0:08 ` Richard Acayan
2026-02-17 5:51 ` Claude review: " Claude Code Review Bot
2026-02-17 0:08 ` [PATCH v2 5/6] arm64: dts: qcom: sdm670-google: add common device tree include Richard Acayan
` (2 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Richard Acayan @ 2026-02-17 0:08 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Thierry Reding, Sam Ravnborg, linux-arm-msm, devicetree,
dri-devel
Cc: yifei, Richard Acayan
The touchscreen and framebuffer can have different resolutions on the
Pixel 3a and 3a XL. Label them so the resolutions can be modified, in
anticipation of the addition of the Pixel 3a XL.
Signed-off-by: Richard Acayan <mailingradian@gmail.com>
---
arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts b/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
index e925cba0381f..2e86bed9ea8c 100644
--- a/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
+++ b/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
@@ -48,7 +48,7 @@ chosen {
#size-cells = <2>;
ranges;
- framebuffer@9c000000 {
+ framebuffer: framebuffer@9c000000 {
compatible = "simple-framebuffer";
reg = <0 0x9c000000 0 (1080 * 2220 * 4)>;
width = <1080>;
@@ -516,7 +516,7 @@ rmi4-f01@1 {
syna,nosleep-mode = <1>;
};
- rmi4-f12@12 {
+ rmi4_f12: rmi4-f12@12 {
reg = <0x12>;
touchscreen-x-mm = <62>;
touchscreen-y-mm = <127>;
@@ -533,7 +533,7 @@ &mdss_dsi0 {
vdda-supply = <&vreg_l1a_1p225>;
status = "okay";
- panel@0 {
+ panel: panel@0 {
compatible = "samsung,s6e3fa7-ams559nk06";
reg = <0>;
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v2 5/6] arm64: dts: qcom: sdm670-google: add common device tree include
2026-02-17 0:08 [PATCH v2 0/6] Support for the Pixel 3a XL with the Tianma panel Richard Acayan
` (3 preceding siblings ...)
2026-02-17 0:08 ` [PATCH v2 4/6] arm64: dts: qcom: sdm670-google-sargo: add touchscreen and display labels Richard Acayan
@ 2026-02-17 0:08 ` Richard Acayan
2026-02-17 5:51 ` Claude review: " Claude Code Review Bot
2026-02-17 0:08 ` [PATCH v2 6/6] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel Richard Acayan
2026-02-17 5:51 ` Claude review: Support for the Pixel 3a XL with the Tianma panel Claude Code Review Bot
6 siblings, 1 reply; 16+ messages in thread
From: Richard Acayan @ 2026-02-17 0:08 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Thierry Reding, Sam Ravnborg, linux-arm-msm, devicetree,
dri-devel
Cc: yifei, Richard Acayan
The Pixel 3a XL can be supported by reusing most of the device tree from
the Pixel 3a. Move the common elements to a common device tree include
like with other devices.
Signed-off-by: Richard Acayan <mailingradian@gmail.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
...le-sargo.dts => sdm670-google-common.dtsi} | 18 +-
.../boot/dts/qcom/sdm670-google-sargo.dts | 710 +-----------------
2 files changed, 21 insertions(+), 707 deletions(-)
copy arch/arm64/boot/dts/qcom/{sdm670-google-sargo.dts => sdm670-google-common.dtsi} (97%)
diff --git a/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts b/arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi
similarity index 97%
copy from arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
copy to arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi
index 2e86bed9ea8c..c47e78aeef9e 100644
--- a/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
+++ b/arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi
@@ -6,8 +6,6 @@
* Copyright (c) 2022, Richard Acayan. All rights reserved.
*/
-/dts-v1/;
-
#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/input/input.h>
#include <dt-bindings/leds/common.h>
@@ -28,9 +26,6 @@
/delete-node/ &gpu_mem;
/ {
- model = "Google Pixel 3a";
- compatible = "google,sargo", "qcom,sdm670";
-
aliases { };
battery: battery {
@@ -38,7 +33,6 @@ battery: battery {
voltage-min-design-microvolt = <3312000>;
voltage-max-design-microvolt = <4400000>;
- charge-full-design-microamp-hours = <3000000>;
};
chosen {
@@ -50,11 +44,9 @@ chosen {
framebuffer: framebuffer@9c000000 {
compatible = "simple-framebuffer";
- reg = <0 0x9c000000 0 (1080 * 2220 * 4)>;
- width = <1080>;
- height = <2220>;
- stride = <(1080 * 4)>;
+ /* dimensions and size are in specific device trees */
format = "a8r8g8b8";
+ status = "disabled";
};
};
@@ -518,8 +510,6 @@ rmi4-f01@1 {
rmi4_f12: rmi4-f12@12 {
reg = <0x12>;
- touchscreen-x-mm = <62>;
- touchscreen-y-mm = <127>;
syna,sensor-type = <1>;
};
};
@@ -534,7 +524,7 @@ &mdss_dsi0 {
status = "okay";
panel: panel@0 {
- compatible = "samsung,s6e3fa7-ams559nk06";
+ /* compatible is provided in specific device trees */
reg = <0>;
reset-gpios = <&tlmm 75 GPIO_ACTIVE_LOW>;
@@ -544,6 +534,8 @@ panel: panel@0 {
power-supply = <&vreg_l6b_3p3>;
+ status = "disabled";
+
port {
panel_in: endpoint {
remote-endpoint = <&mdss_dsi0_out>;
diff --git a/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts b/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
index 2e86bed9ea8c..7a3e24997232 100644
--- a/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
+++ b/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
@@ -1,715 +1,37 @@
-// SPDX-License-Identifier: GPL-2.0
+// SPDX-License-Identifier: GPL-2.0-only
/*
- * Device tree for Google Pixel 3a, adapted from google-blueline device tree,
- * xiaomi-lavender device tree, and oneplus-common device tree.
+ * Device tree for Google Pixel 3a (non-XL).
*
- * Copyright (c) 2022, Richard Acayan. All rights reserved.
+ * Copyright (c) 2023, Richard Acayan. All rights reserved.
*/
/dts-v1/;
-#include <dt-bindings/gpio/gpio.h>
-#include <dt-bindings/input/input.h>
-#include <dt-bindings/leds/common.h>
-#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
-#include <dt-bindings/power/qcom-rpmpd.h>
-#include "sdm670.dtsi"
-#include "pm660.dtsi"
-#include "pm660l.dtsi"
-
-/delete-node/ &mpss_region;
-/delete-node/ &venus_mem;
-/delete-node/ &wlan_msa_mem;
-/delete-node/ &cdsp_mem;
-/delete-node/ &mba_region;
-/delete-node/ &adsp_mem;
-/delete-node/ &ipa_fw_mem;
-/delete-node/ &ipa_gsi_mem;
-/delete-node/ &gpu_mem;
+#include "sdm670-google-common.dtsi"
/ {
model = "Google Pixel 3a";
compatible = "google,sargo", "qcom,sdm670";
-
- aliases { };
-
- battery: battery {
- compatible = "simple-battery";
-
- voltage-min-design-microvolt = <3312000>;
- voltage-max-design-microvolt = <4400000>;
- charge-full-design-microamp-hours = <3000000>;
- };
-
- chosen {
- stdout-path = "serial0:115200n8";
-
- #address-cells = <2>;
- #size-cells = <2>;
- ranges;
-
- framebuffer: framebuffer@9c000000 {
- compatible = "simple-framebuffer";
- reg = <0 0x9c000000 0 (1080 * 2220 * 4)>;
- width = <1080>;
- height = <2220>;
- stride = <(1080 * 4)>;
- format = "a8r8g8b8";
- };
- };
-
- gpio-keys {
- compatible = "gpio-keys";
- autorepeat;
-
- pinctrl-names = "default";
- pinctrl-0 = <&vol_up_pin>;
-
- key-vol-up {
- label = "Volume Up";
- linux,code = <KEY_VOLUMEUP>;
- gpios = <&pm660l_gpios 7 GPIO_ACTIVE_LOW>;
- };
- };
-
- reserved-memory {
- #address-cells = <2>;
- #size-cells = <2>;
-
- mpss_region: mpss@8b000000 {
- reg = <0 0x8b000000 0 0x9800000>;
- no-map;
- };
-
- venus_mem: venus@94800000 {
- reg = <0 0x94800000 0 0x500000>;
- no-map;
- };
-
- wlan_msa_mem: wlan-msa@94d00000 {
- reg = <0 0x94d00000 0 0x100000>;
- no-map;
- };
-
- cdsp_mem: cdsp@94e00000 {
- reg = <0 0x94e00000 0 0x800000>;
- no-map;
- };
-
- mba_region: mba@95600000 {
- reg = <0 0x95600000 0 0x200000>;
- no-map;
- };
-
- adsp_mem: adsp@95800000 {
- reg = <0 0x95800000 0 0x2200000>;
- no-map;
- };
-
- ipa_fw_mem: ipa-fw@97a00000 {
- reg = <0 0x97a00000 0 0x10000>;
- no-map;
- };
-
- ipa_gsi_mem: ipa-gsi@97a10000 {
- reg = <0 0x97a10000 0 0x5000>;
- no-map;
- };
-
- gpu_mem: gpu@97a15000 {
- reg = <0 0x97a15000 0 0x2000>;
- no-map;
- };
-
- framebuffer-region@9c000000 {
- reg = <0 0x9c000000 0 0x2400000>;
- no-map;
- };
-
- /* Also includes ramoops regions */
- debug_info_mem: debug-info@a1800000 {
- reg = <0 0xa1800000 0 0x411000>;
- no-map;
- };
- };
-
- /*
- * The touchscreen regulator seems to be controlled somehow by a gpio.
- * Model it as a fixed regulator and keep it on. Without schematics we
- * don't know how this is actually wired up...
- */
- ts_1p8_supply: ts-1p8-regulator {
- compatible = "regulator-fixed";
- regulator-name = "ts_1p8_supply";
-
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <1800000>;
-
- gpio = <&pm660_gpios 12 GPIO_ACTIVE_HIGH>;
- enable-active-high;
- };
-
- vph_pwr: vph-pwr-regulator {
- compatible = "regulator-fixed";
- regulator-name = "vph_pwr";
- regulator-min-microvolt = <3312000>;
- regulator-max-microvolt = <3312000>;
-
- regulator-always-on;
- regulator-boot-on;
- };
-
- /*
- * Supply map from xiaomi-lavender specifies this as the supply for
- * ldob1, ldob9, ldob10, ldoa2, and ldoa3, while downstream specifies
- * this as a power domain. Set this as a fixed regulator with the same
- * voltage as lavender until display is needed to avoid unneccessarily
- * using a deprecated binding (regulator-fixed-domain).
- */
- vreg_s2b_1p05: vreg-s2b-regulator {
- compatible = "regulator-fixed";
- regulator-name = "vreg_s2b";
- regulator-min-microvolt = <1050000>;
- regulator-max-microvolt = <1050000>;
- };
-
- cam_front_ldo: cam-front-ldo-regulator {
- compatible = "regulator-fixed";
- regulator-name = "cam_front_ldo";
- regulator-min-microvolt = <1352000>;
- regulator-max-microvolt = <1352000>;
- regulator-enable-ramp-delay = <135>;
-
- gpios = <&pm660l_gpios 4 GPIO_ACTIVE_HIGH>;
- enable-active-high;
-
- pinctrl-0 = <&cam_front_ldo_pin>;
- pinctrl-names = "default";
- };
-
- cam_vio_ldo: cam-vio-ldo-regulator {
- compatible = "regulator-fixed";
- regulator-name = "cam_vio_ldo";
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <1800000>;
- regulator-enable-ramp-delay = <233>;
-
- gpios = <&pm660_gpios 13 GPIO_ACTIVE_HIGH>;
- enable-active-high;
-
- pinctrl-0 = <&cam_vio_pin>;
- pinctrl-names = "default";
- };
-};
-
-&apps_rsc {
- regulators-0 {
- compatible = "qcom,pm660-rpmh-regulators";
- qcom,pmic-id = "a";
-
- vdd-s1-supply = <&vph_pwr>;
- vdd-s2-supply = <&vph_pwr>;
- vdd-s3-supply = <&vph_pwr>;
- vdd-s4-supply = <&vph_pwr>;
- vdd-s5-supply = <&vph_pwr>;
- vdd-s6-supply = <&vph_pwr>;
-
- vdd-l1-l6-l7-supply = <&vreg_s6a_0p87>;
- vdd-l2-l3-supply = <&vreg_s2b_1p05>;
- vdd-l5-supply = <&vreg_s2b_1p05>;
- vdd-l8-l9-l10-l11-l12-l13-l14-supply = <&vreg_s4a_2p04>;
- vdd-l15-l16-l17-l18-l19-supply = <&vreg_bob>;
-
- /*
- * S1A (FTAPC0), S2A (FTAPC1), S3A (HFAPC1) are managed
- * by the Core Power Reduction hardened (CPRh) and the
- * Operating State Manager (OSM) HW automatically.
- */
-
- vreg_s4a_2p04: smps4 {
- regulator-min-microvolt = <1808000>;
- regulator-max-microvolt = <2040000>;
- regulator-enable-ramp-delay = <200>;
- };
-
- vreg_s6a_0p87: smps6 {
- regulator-min-microvolt = <1224000>;
- regulator-max-microvolt = <1352000>;
- regulator-enable-ramp-delay = <150>;
- };
-
- /* LDOs */
- vreg_l1a_1p225: ldo1 {
- regulator-min-microvolt = <1200000>;
- regulator-max-microvolt = <1250000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l2a_1p0: ldo2 {
- regulator-min-microvolt = <1000000>;
- regulator-max-microvolt = <1000000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l3a_1p0: ldo3 {
- regulator-min-microvolt = <1000000>;
- regulator-max-microvolt = <1000000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l5a_0p848: ldo5 {
- regulator-min-microvolt = <800000>;
- regulator-max-microvolt = <800000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l6a_1p3: ldo6 {
- regulator-min-microvolt = <1248000>;
- regulator-max-microvolt = <1304000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l7a_1p2: ldo7 {
- regulator-min-microvolt = <1200000>;
- regulator-max-microvolt = <1200000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l8a_1p8: ldo8 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <1800000>;
- regulator-enable-ramp-delay = <250>;
- regulator-always-on;
- };
-
- vreg_l9a_1p8: ldo9 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <1800000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l10a_1p8: ldo10 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <1800000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l11a_1p8: ldo11 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <1800000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l12a_1p8: ldo12 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <1800000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l13a_1p8: ldo13 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <1800000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l14a_1p8: ldo14 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <1800000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l15a_1p8: ldo15 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <2950000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l16a_2p7: ldo16 {
- regulator-min-microvolt = <2696000>;
- regulator-max-microvolt = <2696000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l17a_1p8: ldo17 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <2950000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l19a_3p3: ldo19 {
- regulator-min-microvolt = <3000000>;
- regulator-max-microvolt = <3312000>;
- regulator-enable-ramp-delay = <250>;
- };
- };
-
- regulators-1 {
- compatible = "qcom,pm660l-rpmh-regulators";
- qcom,pmic-id = "b";
-
- vdd-s1-supply = <&vph_pwr>;
- vdd-s2-supply = <&vph_pwr>;
- vdd-s3-s4-supply = <&vph_pwr>;
- vdd-s5-supply = <&vph_pwr>;
-
- vdd-l1-l9-l10-supply = <&vreg_s2b_1p05>;
- vdd-l2-supply = <&vreg_bob>;
- vdd-l3-l5-l7-l8-supply = <&vreg_bob>;
- vdd-l4-l6-supply = <&vreg_bob>;
- vdd-bob-supply = <&vph_pwr>;
-
- /* LDOs */
- vreg_l1b_0p925: ldo1 {
- regulator-min-microvolt = <880000>;
- regulator-max-microvolt = <900000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l2b_2p95: ldo2 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <2960000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l3b_3p0: ldo3 {
- regulator-min-microvolt = <2850000>;
- regulator-max-microvolt = <3008000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l4b_2p95: ldo4 {
- regulator-min-microvolt = <2960000>;
- regulator-max-microvolt = <2960000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l5b_2p95: ldo5 {
- regulator-min-microvolt = <2960000>;
- regulator-max-microvolt = <2960000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l6b_3p3: ldo6 {
- regulator-min-microvolt = <3008000>;
- regulator-max-microvolt = <3300000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l7b_3p125: ldo7 {
- regulator-min-microvolt = <3088000>;
- regulator-max-microvolt = <3100000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- vreg_l8b_3p3: ldo8 {
- regulator-min-microvolt = <3300000>;
- regulator-max-microvolt = <3312000>;
- regulator-enable-ramp-delay = <250>;
- };
-
- /*
- * Downstream specifies a fixed voltage of 3.312 V, but the
- * PMIC4 BOB ranges don't support that. Widen the range a
- * little to avoid adding a new BOB regulator type.
- */
- vreg_bob: bob {
- regulator-min-microvolt = <3296000>;
- regulator-max-microvolt = <3328000>;
- regulator-enable-ramp-delay = <500>;
- };
- };
-};
-
-&camss {
- vdda-phy-supply = <&vreg_l1a_1p225>;
- vdda-pll-supply = <&vreg_s6a_0p87>;
-
- status = "okay";
-};
-
-&camss_port1 {
- camss_endpoint1: endpoint {
- data-lanes = <0 1 2 3>;
- remote-endpoint = <&cam_front_endpoint>;
- };
-};
-
-&cci {
- pinctrl-0 = <&cci1_default>;
- pinctrl-1 = <&cci1_sleep>;
- pinctrl-names = "default", "sleep";
-
- status = "okay";
-};
-
-&cci_i2c1 {
- camera@1a {
- compatible = "sony,imx355";
- reg = <0x1a>;
-
- clocks = <&camcc CAM_CC_MCLK2_CLK>;
-
- assigned-clocks = <&camcc CAM_CC_MCLK2_CLK>;
- assigned-clock-rates = <19200000>;
-
- reset-gpios = <&tlmm 9 GPIO_ACTIVE_LOW>;
-
- avdd-supply = <&cam_front_ldo>;
- dvdd-supply = <&cam_front_ldo>;
- dovdd-supply = <&cam_vio_ldo>;
-
- pinctrl-0 = <&cam_mclk2_default>;
- pinctrl-names = "default";
-
- rotation = <270>;
- orientation = <0>;
-
- port {
- cam_front_endpoint: endpoint {
- link-frequencies = /bits/ 64 <360000000>;
- remote-endpoint = <&camss_endpoint1>;
- };
- };
- };
-};
-
-&gcc {
- protected-clocks = <GCC_QSPI_CORE_CLK>,
- <GCC_QSPI_CORE_CLK_SRC>,
- <GCC_QSPI_CNOC_PERIPH_AHB_CLK>;
-};
-
-&gpi_dma1 {
- status = "okay";
-};
-
-&gpu {
- status = "okay";
-};
-
-&gpu_zap_shader {
- firmware-name = "qcom/sdm670/sargo/a615_zap.mbn";
-};
-
-&i2c9 {
- clock-frequency = <100000>;
- status = "okay";
-
- synaptics-rmi4-i2c@20 {
- compatible = "syna,rmi4-i2c";
- reg = <0x20>;
- interrupts-extended = <&tlmm 125 IRQ_TYPE_EDGE_FALLING>;
-
- pinctrl-names = "default";
- pinctrl-0 = <&touchscreen_default>;
-
- vio-supply = <&ts_1p8_supply>;
-
- syna,reset-delay-ms = <200>;
- syna,startup-delay-ms = <200>;
-
- #address-cells = <1>;
- #size-cells = <0>;
-
- rmi4-f01@1 {
- reg = <0x01>;
- syna,nosleep-mode = <1>;
- };
-
- rmi4_f12: rmi4-f12@12 {
- reg = <0x12>;
- touchscreen-x-mm = <62>;
- touchscreen-y-mm = <127>;
- syna,sensor-type = <1>;
- };
- };
};
-&mdss {
- status = "okay";
+&battery {
+ charge-full-design-microamp-hours = <3000000>;
};
-&mdss_dsi0 {
- vdda-supply = <&vreg_l1a_1p225>;
+&framebuffer {
+ reg = <0 0x9c000000 0 (1080 * 2220 * 4)>;
+ width = <1080>;
+ height = <2220>;
+ stride = <(1080 * 4)>;
status = "okay";
-
- panel: panel@0 {
- compatible = "samsung,s6e3fa7-ams559nk06";
- reg = <0>;
-
- reset-gpios = <&tlmm 75 GPIO_ACTIVE_LOW>;
-
- pinctrl-names = "default";
- pinctrl-0 = <&panel_default>;
-
- power-supply = <&vreg_l6b_3p3>;
-
- port {
- panel_in: endpoint {
- remote-endpoint = <&mdss_dsi0_out>;
- };
- };
- };
};
-&mdss_dsi0_out {
- remote-endpoint = <&panel_in>;
- data-lanes = <0 1 2 3>;
-};
-
-&mdss_dsi0_phy {
- vdds-supply = <&vreg_l1b_0p925>;
- status = "okay";
-};
-
-&mdss_mdp {
+&panel {
+ compatible = "samsung,s6e3fa7-ams559nk06";
status = "okay";
};
-&pm660_charger {
- monitored-battery = <&battery>;
- status = "okay";
-};
-
-&pm660_gpios {
- cam_vio_pin: cam-vio-state {
- pins = "gpio13";
- function = "normal";
- power-source = <0>;
- };
-};
-
-&pm660_rradc {
- status = "okay";
-};
-
-&pm660l_flash {
- status = "okay";
-
- led-0 {
- function = LED_FUNCTION_FLASH;
- color = <LED_COLOR_ID_WHITE>;
- led-sources = <1>, <2>;
- led-max-microamp = <500000>;
- flash-max-microamp = <1500000>;
- flash-max-timeout-us = <1280000>;
- };
-};
-
-&pm660l_gpios {
- cam_front_ldo_pin: cam-front-state {
- pins = "gpio4";
- function = "normal";
- power-source = <0>;
- };
-
- vol_up_pin: vol-up-state {
- pins = "gpio7";
- function = "normal";
- qcom,drive-strength = <PMIC_GPIO_STRENGTH_NO>;
- input-enable;
- bias-pull-up;
- };
-};
-
-&pon_pwrkey {
- status = "okay";
-};
-
-&pon_resin {
- linux,code = <KEY_VOLUMEDOWN>;
- status = "okay";
-};
-
-&qupv3_id_1 {
- status = "okay";
-};
-
-&sdhc_1 {
- supports-cqe;
- mmc-hs200-1_8v;
- mmc-hs400-1_8v;
- mmc-ddr-1_8v;
-
- qcom,ddr-config = <0xc3040873>;
-
- vmmc-supply = <&vreg_l4b_2p95>;
- vqmmc-supply = <&vreg_l8a_1p8>;
-
- status = "okay";
-};
-
-&tlmm {
- gpio-reserved-ranges = <0 4>, <81 4>;
-
- panel_default: panel-default-state {
- te-pins {
- pins = "gpio10";
- function = "mdp_vsync";
- drive-strength = <2>;
- bias-pull-down;
- };
-
- reset-pins {
- pins = "gpio75";
- function = "gpio";
- drive-strength = <8>;
- bias-disable;
- };
-
- mode-pins {
- pins = "gpio76";
- function = "gpio";
- drive-strength = <8>;
- bias-disable;
- };
- };
-
- touchscreen_default: ts-default-state {
- ts-reset-pins {
- pins = "gpio99";
- function = "gpio";
- drive-strength = <2>;
- bias-pull-up;
- output-high;
- };
-
- ts-irq-pins {
- pins = "gpio125";
- function = "gpio";
- drive-strength = <2>;
- bias-disable;
- };
-
- ts-switch-pins {
- pins = "gpio135";
- function = "gpio";
- drive-strength = <2>;
- bias-disable;
- output-low;
- };
- };
-};
-
-&usb_1_hsphy {
- vdd-supply = <&vreg_l1b_0p925>;
- vdda-pll-supply = <&vreg_l10a_1p8>;
- vdda-phy-dpdm-supply = <&vreg_l7b_3p125>;
-
- status = "okay";
-};
-
-&usb_1 {
- qcom,select-utmi-as-pipe-clk;
- status = "okay";
-};
-
-&usb_1_dwc3 {
- /* Only peripheral works for now */
- dr_mode = "peripheral";
-
- /* Do not assume that sdm670.dtsi will never support USB 3.0 */
- phys = <&usb_1_hsphy>;
- phy-names = "usb2-phy";
- maximum-speed = "high-speed";
+&rmi4_f12 {
+ touchscreen-x-mm = <63>;
+ touchscreen-y-mm = <127>;
};
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Claude review: arm64: dts: qcom: sdm670-google: add common device tree include
2026-02-17 0:08 ` [PATCH v2 5/6] arm64: dts: qcom: sdm670-google: add common device tree include Richard Acayan
@ 2026-02-17 5:51 ` Claude Code Review Bot
0 siblings, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-02-17 5:51 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
This is the main refactoring patch. The approach of extracting common elements into `sdm670-google-common.dtsi` and making per-device DTS files thin overlays is clean.
> - charge-full-design-microamp-hours = <3000000>;
The `charge-full-design-microamp-hours` property is removed from the common include and added back in the sargo-specific DTS with the same value. This makes sense since the 3a XL has a different battery capacity (3700 mAh vs 3000 mAh).
> + framebuffer: framebuffer@9c000000 {
> + compatible = "simple-framebuffer";
> + /* dimensions and size are in specific device trees */
> + format = "a8r8g8b8";
> + status = "disabled";
> };
The framebuffer is disabled in the common include and re-enabled with device-specific dimensions in each board DTS. However, `simple-framebuffer` nodes without `reg`, `width`, `height`, and `stride` properties are incomplete. Since the node is `status = "disabled"`, this is fine at runtime, but it may generate DT validation warnings. The `reg` property is also omitted from the common include but provided in the per-device overrides.
> +&rmi4_f12 {
> + touchscreen-x-mm = <63>;
> + touchscreen-y-mm = <127>;
> +};
The original sargo DTS had `touchscreen-x-mm = <62>`, but the new sargo DTS sets it to `<63>`. Is this an intentional correction, or a typo introduced during the refactoring? The commit message doesn't mention this change. If intentional, it would be worth noting in the commit message.
> - panel: panel@0 {
> - compatible = "samsung,s6e3fa7-ams559nk06";
> + panel: panel@0 {
> + /* compatible is provided in specific device trees */
> reg = <0>;
The panel node is left without a `compatible` and with `status = "disabled"` in the common include. This is correct -- the per-device DTS provides both the compatible and `status = "okay"`.
> -// SPDX-License-Identifier: GPL-2.0
> +// SPDX-License-Identifier: GPL-2.0-only
The SPDX license identifier changed from `GPL-2.0` to `GPL-2.0-only` in the new sargo DTS. While `GPL-2.0-only` is more precise, this is a license change that should probably be noted in the commit message or handled as a separate patch, as it changes the licensing terms.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 6/6] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel
2026-02-17 0:08 [PATCH v2 0/6] Support for the Pixel 3a XL with the Tianma panel Richard Acayan
` (4 preceding siblings ...)
2026-02-17 0:08 ` [PATCH v2 5/6] arm64: dts: qcom: sdm670-google: add common device tree include Richard Acayan
@ 2026-02-17 0:08 ` Richard Acayan
2026-02-17 5:51 ` Claude review: " Claude Code Review Bot
2026-02-17 5:51 ` Claude review: Support for the Pixel 3a XL with the Tianma panel Claude Code Review Bot
6 siblings, 1 reply; 16+ messages in thread
From: Richard Acayan @ 2026-02-17 0:08 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Thierry Reding, Sam Ravnborg, linux-arm-msm, devicetree,
dri-devel
Cc: yifei, Richard Acayan
The Pixel 3a XL has variants with either a Samsung Display Controller (SDC)
panel or a Tianma panel. Add the device tree for the variant with the
Tianma panel.
Signed-off-by: Richard Acayan <mailingradian@gmail.com>
---
arch/arm64/boot/dts/qcom/Makefile | 1 +
.../dts/qcom/sdm670-google-bonito-tianma.dts | 38 +++++++++++++++++++
2 files changed, 39 insertions(+)
create mode 100644 arch/arm64/boot/dts/qcom/sdm670-google-bonito-tianma.dts
diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 6f34d5ed331c..6237aeb0e36d 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -245,6 +245,7 @@ dtb-$(CONFIG_ARCH_QCOM) += sdm632-motorola-ocean.dtb
dtb-$(CONFIG_ARCH_QCOM) += sdm636-sony-xperia-ganges-mermaid.dtb
dtb-$(CONFIG_ARCH_QCOM) += sdm660-xiaomi-lavender.dtb
dtb-$(CONFIG_ARCH_QCOM) += sdm670-google-sargo.dtb
+dtb-$(CONFIG_ARCH_QCOM) += sdm670-google-bonito-tianma.dtb
dtb-$(CONFIG_ARCH_QCOM) += sdm845-db845c.dtb
sdm845-db845c-navigation-mezzanine-dtbs := sdm845-db845c.dtb sdm845-db845c-navigation-mezzanine.dtbo
diff --git a/arch/arm64/boot/dts/qcom/sdm670-google-bonito-tianma.dts b/arch/arm64/boot/dts/qcom/sdm670-google-bonito-tianma.dts
new file mode 100644
index 000000000000..da309434b511
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sdm670-google-bonito-tianma.dts
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Device tree for Google Pixel 3a XL with the panel connected to the Samsung
+ * Display Controller.
+ *
+ * Copyright (c) 2023-2024, Richard Acayan. All rights reserved.
+ */
+
+/dts-v1/;
+
+#include "sdm670-google-common.dtsi"
+
+/ {
+ model = "Google Pixel 3a XL (with Tianma panel)";
+ compatible = "google,bonito-tianma", "google,bonito", "qcom,sdm670";
+};
+
+&battery {
+ charge-full-design-microamp-hours = <3700000>;
+};
+
+&framebuffer {
+ reg = <0 0x9c000000 0 (1080 * 2160 * 4)>;
+ width = <1080>;
+ height = <2160>;
+ stride = <(1080 * 4)>;
+ status = "okay";
+};
+
+&panel {
+ compatible = "novatek,nt37700f";
+ status = "okay";
+};
+
+&rmi4_f12 {
+ touchscreen-x-mm = <69>;
+ touchscreen-y-mm = <137>;
+};
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Claude review: arm64: dts: qcom: add support for pixel 3a xl with the tianma panel
2026-02-17 0:08 ` [PATCH v2 6/6] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel Richard Acayan
@ 2026-02-17 5:51 ` Claude Code Review Bot
0 siblings, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-02-17 5:51 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
> +/*
> + * Device tree for Google Pixel 3a XL with the panel connected to the Samsung
> + * Display Controller.
This comment says "Samsung Display Controller" but the file is `sdm670-google-bonito-tianma.dts` and uses the `novatek,nt37700f` compatible. The cover letter mentions this was addressed in v2 ("mention the sdc panel"), but the comment still describes the wrong panel variant. This should say something like "with the Tianma panel" rather than "Samsung Display Controller."
> + compatible = "google,bonito-tianma", "google,bonito", "qcom,sdm670";
Matches the binding added in patch 1. Correct.
> +&battery {
> + charge-full-design-microamp-hours = <3700000>;
> +};
3700 mAh for the XL variant vs 3000 mAh for the standard 3a. Reasonable.
> +&panel {
> + compatible = "novatek,nt37700f";
> + status = "okay";
> +};
Uses the panel driver from patch 3. The `status = "okay"` overrides the `"disabled"` from the common include. Correct.
> +&rmi4_f12 {
> + touchscreen-x-mm = <69>;
> + touchscreen-y-mm = <137>;
> +};
Touchscreen dimensions match the panel physical size (69mm x 137mm) specified in the panel driver's `drm_display_mode`. Consistent.
The Makefile addition places the new dtb entry after `sdm670-google-sargo.dtb` but alphabetically `bonito` comes before `sargo`. Not a functional issue, but the Makefile entry could be placed in alphabetical order for consistency.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread
* Claude review: Support for the Pixel 3a XL with the Tianma panel
2026-02-17 0:08 [PATCH v2 0/6] Support for the Pixel 3a XL with the Tianma panel Richard Acayan
` (5 preceding siblings ...)
2026-02-17 0:08 ` [PATCH v2 6/6] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel Richard Acayan
@ 2026-02-17 5:51 ` Claude Code Review Bot
6 siblings, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-02-17 5:51 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: Support for the Pixel 3a XL with the Tianma panel
Author: Richard Acayan <mailingradian@gmail.com>
Patches: 7
Reviewed: 2026-02-17T15:51:43.373818
---
This series adds support for the Google Pixel 3a XL (codename "bonito") with the Tianma/Novatek NT37700F panel variant. It consists of DT bindings additions (patches 1-2), a new panel driver (patch 3), preparatory DTS label additions (patch 4), refactoring of the existing sargo DTS into a common include (patch 5), and the new bonito-tianma DTS (patch 6).
The overall approach is clean and follows established patterns. The DTS refactoring to extract common elements into a shared include is a sensible way to support the closely-related 3a and 3a XL devices. The panel driver is generated from the vendor's DSI command sequences using the well-known `linux-mdss-dsi-panel-driver-generator` tool and follows the modern `mipi_dsi_multi_context` pattern.
There are a few issues to address. The most notable is a comment in patch 6 that says the opposite of what the file actually describes. There is also a value change in patch 5 that changes the Pixel 3a's touchscreen-x-mm from 62 to 63 without explanation. Both should be clarified.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 3/5] drm/panel: Add Novatek/Tianma NT37700F panel
2026-03-10 0:26 [PATCH v3 0/5] " Richard Acayan
@ 2026-03-10 0:26 ` Richard Acayan
2026-03-10 1:42 ` Claude review: " Claude Code Review Bot
0 siblings, 1 reply; 16+ messages in thread
From: Richard Acayan @ 2026-03-10 0:26 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong, Jessica Zhang, David Airlie,
Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Thierry Reding, Sam Ravnborg, linux-arm-msm,
devicetree, dri-devel
Cc: Yifei Zhan, Richard Acayan
Some Pixel 3a XL devices have a Tianma panel. Add support for it, with
the aid of linux-mdss-dsi-panel-driver-generator.
Link: https://github.com/msm8916-mainline/linux-mdss-dsi-panel-driver-generator
Signed-off-by: Richard Acayan <mailingradian@gmail.com>
Tested-by: Yifei Zhan <yifei@zhan.science>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/panel/Kconfig | 9 +
drivers/gpu/drm/panel/Makefile | 1 +
.../gpu/drm/panel/panel-novatek-nt37700f.c | 299 ++++++++++++++++++
3 files changed, 309 insertions(+)
create mode 100644 drivers/gpu/drm/panel/panel-novatek-nt37700f.c
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 307152ad7759..21b38b640878 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -553,6 +553,15 @@ config DRM_PANEL_NOVATEK_NT36672E
LCD panel module. The panel has a resolution of 1080x2408 and uses 24 bit
RGB per pixel.
+config DRM_PANEL_NOVATEK_NT37700F
+ tristate "Novatek NT37700F DSI 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 Novatek NT37700F DSI
+ panel module. The panel has a resolution of 1080x2160.
+
config DRM_PANEL_NOVATEK_NT37801
tristate "Novatek NT37801/NT37810 AMOLED DSI panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index aeffaa95666d..8bde285336bd 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -54,6 +54,7 @@ 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_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
obj-$(CONFIG_DRM_PANEL_NOVATEK_NT37801) += panel-novatek-nt37801.o
obj-$(CONFIG_DRM_PANEL_NOVATEK_NT39016) += panel-novatek-nt39016.o
obj-$(CONFIG_DRM_PANEL_MANTIX_MLAF057WE51) += panel-mantix-mlaf057we51.o
diff --git a/drivers/gpu/drm/panel/panel-novatek-nt37700f.c b/drivers/gpu/drm/panel/panel-novatek-nt37700f.c
new file mode 100644
index 000000000000..74f46a268c0f
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-novatek-nt37700f.c
@@ -0,0 +1,299 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2024, The Linux Foundation. All rights reserved.
+ * Generated with linux-mdss-dsi-panel-driver-generator from vendor device tree:
+ * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+#include <linux/of.h>
+
+#include <video/mipi_display.h>
+
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_probe_helper.h>
+
+struct nt37700f_tianma {
+ struct drm_panel panel;
+ struct mipi_dsi_device *dsi;
+ struct regulator *supply;
+ struct gpio_desc *reset_gpio;
+};
+
+static inline
+struct nt37700f_tianma *to_nt37700f_tianma(struct drm_panel *panel)
+{
+ return container_of(panel, struct nt37700f_tianma, panel);
+}
+
+static void nt37700f_tianma_reset(struct nt37700f_tianma *ctx)
+{
+ gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+ usleep_range(1000, 2000);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ usleep_range(10000, 11000);
+}
+
+#define nt37700f_tianma_panel_switch_page(ctx, page) \
+ mipi_dsi_dcs_write_seq_multi((ctx), 0xf0, 0x55, 0xaa, 0x52, 0x08, (page))
+
+static int nt37700f_tianma_on(struct nt37700f_tianma *ctx)
+{
+ struct mipi_dsi_device *dsi = ctx->dsi;
+ struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
+
+ nt37700f_tianma_panel_switch_page(&dsi_ctx, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xc0, 0x56);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xca, 0x52);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x06);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xb5, 0x2b, 0x1a);
+ nt37700f_tianma_panel_switch_page(&dsi_ctx, 0x01);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xcd, 0x04, 0x82);
+ nt37700f_tianma_panel_switch_page(&dsi_ctx, 0x02);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xcc, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xff, 0xaa, 0x55, 0xa5, 0x80);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x55);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf6, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x56);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf6, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xff, 0xaa, 0x55, 0xa5, 0x81);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x07);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf3, 0x07);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x05);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf3, 0x25);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x90, 0x01);
+
+ mipi_dsi_dcs_set_column_address_multi(&dsi_ctx, 0x0000, 1080 - 1);
+ mipi_dsi_dcs_set_page_address_multi(&dsi_ctx, 0x0000, 2160 - 1);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_WRITE_CONTROL_DISPLAY, 0x20);
+ mipi_dsi_dcs_set_tear_on_multi(&dsi_ctx, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
+
+ nt37700f_tianma_panel_switch_page(&dsi_ctx, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xc0, 0x56);
+ nt37700f_tianma_panel_switch_page(&dsi_ctx, 0x02);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xcd, 0x00);
+ nt37700f_tianma_panel_switch_page(&dsi_ctx, 0x04);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xd0, 0x11, 0x64);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x09);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xb1, 0x20);
+
+ mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+ mipi_dsi_msleep(&dsi_ctx, 120);
+ mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
+
+ return dsi_ctx.accum_err;
+}
+
+static int nt37700f_tianma_disable(struct drm_panel *panel)
+{
+ struct nt37700f_tianma *ctx = to_nt37700f_tianma(panel);
+ struct mipi_dsi_device *dsi = ctx->dsi;
+ struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
+
+ mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
+ mipi_dsi_msleep(&dsi_ctx, 50);
+ mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
+ mipi_dsi_msleep(&dsi_ctx, 100);
+
+ return dsi_ctx.accum_err;
+}
+
+static int nt37700f_tianma_prepare(struct drm_panel *panel)
+{
+ struct nt37700f_tianma *ctx = to_nt37700f_tianma(panel);
+ struct device *dev = &ctx->dsi->dev;
+ int ret;
+
+ ret = regulator_enable(ctx->supply);
+ if (ret < 0) {
+ dev_err(dev, "Failed to enable power supply: %d\n", ret);
+ return ret;
+ }
+
+ nt37700f_tianma_reset(ctx);
+
+ ret = nt37700f_tianma_on(ctx);
+ if (ret < 0) {
+ dev_err(dev, "Failed to initialize panel: %d\n", ret);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int nt37700f_tianma_unprepare(struct drm_panel *panel)
+{
+ struct nt37700f_tianma *ctx = to_nt37700f_tianma(panel);
+
+ gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+ regulator_disable(ctx->supply);
+
+ return 0;
+}
+
+static const struct drm_display_mode nt37700f_tianma_mode = {
+ .clock = (1080 + 32 + 32 + 98) * (2160 + 32 + 4 + 98) * 60 / 1000,
+ .hdisplay = 1080,
+ .hsync_start = 1080 + 32,
+ .hsync_end = 1080 + 32 + 32,
+ .htotal = 1080 + 32 + 32 + 98,
+ .vdisplay = 2160,
+ .vsync_start = 2160 + 32,
+ .vsync_end = 2160 + 32 + 4,
+ .vtotal = 2160 + 32 + 4 + 98,
+ .width_mm = 69,
+ .height_mm = 137,
+ .type = DRM_MODE_TYPE_DRIVER,
+};
+
+static int nt37700f_tianma_get_modes(struct drm_panel *panel,
+ struct drm_connector *connector)
+{
+ return drm_connector_helper_get_modes_fixed(connector, &nt37700f_tianma_mode);
+}
+
+static const struct drm_panel_funcs nt37700f_tianma_panel_funcs = {
+ .prepare = nt37700f_tianma_prepare,
+ .unprepare = nt37700f_tianma_unprepare,
+ .disable = nt37700f_tianma_disable,
+ .get_modes = nt37700f_tianma_get_modes,
+};
+
+static int nt37700f_tianma_bl_update_status(struct backlight_device *bl)
+{
+ struct mipi_dsi_device *dsi = bl_get_data(bl);
+ u16 brightness = backlight_get_brightness(bl);
+ int ret;
+
+ dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+ ret = mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);
+ if (ret < 0)
+ return ret;
+
+ dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+ return 0;
+}
+
+static int nt37700f_tianma_bl_get_brightness(struct backlight_device *bl)
+{
+ struct mipi_dsi_device *dsi = bl_get_data(bl);
+ u16 brightness;
+ int ret;
+
+ dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+ ret = mipi_dsi_dcs_get_display_brightness_large(dsi, &brightness);
+ if (ret < 0)
+ return ret;
+
+ dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+ return brightness;
+}
+
+static const struct backlight_ops nt37700f_tianma_bl_ops = {
+ .update_status = nt37700f_tianma_bl_update_status,
+ .get_brightness = nt37700f_tianma_bl_get_brightness,
+};
+
+static struct backlight_device *
+nt37700f_tianma_create_backlight(struct mipi_dsi_device *dsi)
+{
+ struct device *dev = &dsi->dev;
+ const struct backlight_properties props = {
+ .type = BACKLIGHT_RAW,
+ .brightness = 2047,
+ .max_brightness = 2047,
+ };
+
+ return devm_backlight_device_register(dev, dev_name(dev), dev, dsi,
+ &nt37700f_tianma_bl_ops, &props);
+}
+
+static int nt37700f_tianma_probe(struct mipi_dsi_device *dsi)
+{
+ struct device *dev = &dsi->dev;
+ struct nt37700f_tianma *ctx;
+ int ret;
+
+ ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+ if (!ctx)
+ return -ENOMEM;
+
+ ctx->supply = devm_regulator_get(dev, "power");
+ if (IS_ERR(ctx->supply))
+ return dev_err_probe(dev, PTR_ERR(ctx->supply),
+ "Failed to get power-supply\n");
+
+ ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(ctx->reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio),
+ "Failed to get reset-gpios\n");
+
+ ctx->dsi = dsi;
+ mipi_dsi_set_drvdata(dsi, ctx);
+
+ dsi->lanes = 4;
+ dsi->format = MIPI_DSI_FMT_RGB888;
+ dsi->mode_flags = MIPI_DSI_MODE_VIDEO_BURST |
+ MIPI_DSI_CLOCK_NON_CONTINUOUS | MIPI_DSI_MODE_LPM;
+
+ drm_panel_init(&ctx->panel, dev, &nt37700f_tianma_panel_funcs,
+ DRM_MODE_CONNECTOR_DSI);
+ ctx->panel.prepare_prev_first = true;
+
+ ctx->panel.backlight = nt37700f_tianma_create_backlight(dsi);
+ if (IS_ERR(ctx->panel.backlight))
+ return dev_err_probe(dev, PTR_ERR(ctx->panel.backlight),
+ "Failed to create backlight\n");
+
+ drm_panel_add(&ctx->panel);
+
+ ret = mipi_dsi_attach(dsi);
+ if (ret < 0) {
+ drm_panel_remove(&ctx->panel);
+ return dev_err_probe(dev, ret, "Failed to attach to DSI host\n");
+ }
+
+ return 0;
+}
+
+static void nt37700f_tianma_remove(struct mipi_dsi_device *dsi)
+{
+ struct nt37700f_tianma *ctx = mipi_dsi_get_drvdata(dsi);
+ int ret;
+
+ ret = mipi_dsi_detach(dsi);
+ if (ret < 0)
+ dev_err(&dsi->dev, "Failed to detach from DSI host: %d\n", ret);
+
+ drm_panel_remove(&ctx->panel);
+}
+
+static const struct of_device_id nt37700f_tianma_of_match[] = {
+ { .compatible = "novatek,nt37700f" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, nt37700f_tianma_of_match);
+
+static struct mipi_dsi_driver nt37700f_tianma_driver = {
+ .probe = nt37700f_tianma_probe,
+ .remove = nt37700f_tianma_remove,
+ .driver = {
+ .name = "panel-novatek-nt37700f",
+ .of_match_table = nt37700f_tianma_of_match,
+ },
+};
+module_mipi_dsi_driver(nt37700f_tianma_driver);
+
+MODULE_DESCRIPTION("DRM driver for nt37700f cmd mode dsi tianma panel");
+MODULE_LICENSE("GPL");
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Claude review: drm/panel: Add Novatek/Tianma NT37700F panel
2026-03-10 0:26 ` [PATCH v3 3/5] drm/panel: Add Novatek/Tianma NT37700F panel Richard Acayan
@ 2026-03-10 1:42 ` Claude Code Review Bot
0 siblings, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-03-10 1:42 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Mostly good, minor observations.**
The driver is well-structured and uses modern APIs (`mipi_dsi_multi_context`, `drm_connector_helper_get_modes_fixed`). A few observations:
1. **Includes**: `<linux/of.h>` is included but not directly needed — the OF match table works through `<linux/module.h>` and the `MODULE_DEVICE_TABLE` macro. This is very minor and cosmetic.
2. **`prepare` error path missing regulator disable**: In `nt37700f_tianma_prepare()`:
```c
ret = nt37700f_tianma_on(ctx);
if (ret < 0) {
dev_err(dev, "Failed to initialize panel: %d\n", ret);
gpiod_set_value_cansleep(ctx->reset_gpio, 1);
return ret;
}
```
When `nt37700f_tianma_on()` fails, the reset GPIO is asserted but the regulator is **not** disabled. This will leave the `power` supply enabled after a failed prepare. The error path should call `regulator_disable(ctx->supply)` as well to match what `unprepare` does.
3. **Backlight mode flag toggling**: The backlight `update_status` and `get_brightness` callbacks toggle `MIPI_DSI_MODE_LPM`:
```c
dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
ret = mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);
...
dsi->mode_flags |= MIPI_DSI_MODE_LPM;
```
This is not thread-safe — if backlight updates could race with other DSI operations, the mode flags could be corrupted. This is a pattern seen in generated drivers, and in practice it may be fine for this single-panel setup, but it's worth noting.
4. **`MIPI_DSI_MODE_VIDEO_BURST` without `MIPI_DSI_MODE_VIDEO`**: The mode flags are:
```c
dsi->mode_flags = MIPI_DSI_MODE_VIDEO_BURST |
MIPI_DSI_CLOCK_NON_CONTINUOUS | MIPI_DSI_MODE_LPM;
```
`MODE_VIDEO_BURST` implies video mode, but `MIPI_DSI_MODE_VIDEO` itself is not set. This means the panel operates in **command mode** (since `MODE_VIDEO` is not set), and `MODE_VIDEO_BURST` has no effect. The MODULE_DESCRIPTION says "cmd mode" which is consistent, but having `MODE_VIDEO_BURST` set when not in video mode is misleading/unnecessary. Consider removing `MIPI_DSI_MODE_VIDEO_BURST` if this is indeed a command-mode panel.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/6] drm/panel: Add Novatek/Tianma NT37700F panel
2026-02-10 2:32 [PATCH 0/6] Support for the Pixel 3a XL with the Tianma panel Richard Acayan
@ 2026-02-10 2:32 ` Richard Acayan
2026-02-11 6:47 ` Claude review: " Claude Code Review Bot
0 siblings, 1 reply; 16+ messages in thread
From: Richard Acayan @ 2026-02-10 2:32 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Thierry Reding, Sam Ravnborg, linux-arm-msm, devicetree,
dri-devel
Cc: yifei, Richard Acayan
Some Pixel 3a XL devices have a Tianma panel. Add support for it, with
the aid of linux-mdss-dsi-panel-driver-generator.
Link: https://github.com/msm8916-mainline/linux-mdss-dsi-panel-driver-generator
Signed-off-by: Richard Acayan <mailingradian@gmail.com>
---
drivers/gpu/drm/panel/Kconfig | 9 +
drivers/gpu/drm/panel/Makefile | 1 +
.../gpu/drm/panel/panel-novatek-nt37700f.c | 294 ++++++++++++++++++
3 files changed, 304 insertions(+)
create mode 100644 drivers/gpu/drm/panel/panel-novatek-nt37700f.c
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 76f6af819037..138d617e8195 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -553,6 +553,15 @@ config DRM_PANEL_NOVATEK_NT36672E
LCD panel module. The panel has a resolution of 1080x2408 and uses 24 bit
RGB per pixel.
+config DRM_PANEL_NOVATEK_NT37700F
+ tristate "Novatek NT37700F DSI 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 Novatek NT37700F DSI
+ panel module. The panel has a resolution of 1080x2160.
+
config DRM_PANEL_NOVATEK_NT37801
tristate "Novatek NT37801/NT37810 AMOLED DSI panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index b9562a6fdcb3..9218a7d7ff34 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -54,6 +54,7 @@ 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_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
obj-$(CONFIG_DRM_PANEL_NOVATEK_NT37801) += panel-novatek-nt37801.o
obj-$(CONFIG_DRM_PANEL_NOVATEK_NT39016) += panel-novatek-nt39016.o
obj-$(CONFIG_DRM_PANEL_MANTIX_MLAF057WE51) += panel-mantix-mlaf057we51.o
diff --git a/drivers/gpu/drm/panel/panel-novatek-nt37700f.c b/drivers/gpu/drm/panel/panel-novatek-nt37700f.c
new file mode 100644
index 000000000000..491f1f30ce41
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-novatek-nt37700f.c
@@ -0,0 +1,294 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2024, The Linux Foundation. All rights reserved.
+ * Generated with linux-mdss-dsi-panel-driver-generator from vendor device tree:
+ * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of.h>
+
+#include <video/mipi_display.h>
+
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_probe_helper.h>
+
+struct nt37700f_tianma {
+ struct drm_panel panel;
+ struct mipi_dsi_device *dsi;
+ struct gpio_desc *reset_gpio;
+};
+
+static inline
+struct nt37700f_tianma *to_nt37700f_tianma(struct drm_panel *panel)
+{
+ return container_of(panel, struct nt37700f_tianma, panel);
+}
+
+static void nt37700f_tianma_reset(struct nt37700f_tianma *ctx)
+{
+ gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+ usleep_range(1000, 2000);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ usleep_range(10000, 11000);
+}
+
+static int nt37700f_tianma_on(struct nt37700f_tianma *ctx)
+{
+ struct mipi_dsi_device *dsi = ctx->dsi;
+ struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
+
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xc0, 0x56);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xca, 0x52);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x06);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xb5, 0x2b, 0x1a);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x01);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xcd, 0x04, 0x82);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x02);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xcc, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xff, 0xaa, 0x55, 0xa5, 0x80);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x55);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf6, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x56);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf6, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xff, 0xaa, 0x55, 0xa5, 0x81);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x07);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf3, 0x07);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x05);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf3, 0x25);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x90, 0x01);
+
+ mipi_dsi_dcs_set_column_address_multi(&dsi_ctx, 0x0000, 0x0437);
+ mipi_dsi_dcs_set_page_address_multi(&dsi_ctx, 0x0000, 0x086f);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_WRITE_CONTROL_DISPLAY, 0x20);
+ mipi_dsi_dcs_set_tear_on_multi(&dsi_ctx, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
+
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xc0, 0x56);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x02);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xcd, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x04);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xd0, 0x11, 0x64);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x09);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xb1, 0x20);
+
+ mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+ mipi_dsi_msleep(&dsi_ctx, 120);
+ mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
+
+ return dsi_ctx.accum_err;
+}
+
+static int nt37700f_tianma_disable(struct drm_panel *panel)
+{
+ struct nt37700f_tianma *ctx = to_nt37700f_tianma(panel);
+ struct mipi_dsi_device *dsi = ctx->dsi;
+ struct device *dev = &dsi->dev;
+ int ret;
+
+ ret = mipi_dsi_dcs_set_display_off(dsi);
+ if (ret < 0) {
+ dev_err(dev, "Failed to set display off: %d\n", ret);
+ return ret;
+ }
+ msleep(50);
+
+ ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
+ if (ret < 0) {
+ dev_err(dev, "Failed to enter sleep mode: %d\n", ret);
+ return ret;
+ }
+ msleep(100);
+
+ return 0;
+}
+
+static int nt37700f_tianma_prepare(struct drm_panel *panel)
+{
+ struct nt37700f_tianma *ctx = to_nt37700f_tianma(panel);
+ struct device *dev = &ctx->dsi->dev;
+ int ret;
+
+ nt37700f_tianma_reset(ctx);
+
+ ret = nt37700f_tianma_on(ctx);
+ if (ret < 0) {
+ dev_err(dev, "Failed to initialize panel: %d\n", ret);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int nt37700f_tianma_unprepare(struct drm_panel *panel)
+{
+ struct nt37700f_tianma *ctx = to_nt37700f_tianma(panel);
+
+ gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+
+ return 0;
+}
+
+static const struct drm_display_mode nt37700f_tianma_mode = {
+ .clock = (1080 + 32 + 32 + 98) * (2160 + 32 + 4 + 98) * 60 / 1000,
+ .hdisplay = 1080,
+ .hsync_start = 1080 + 32,
+ .hsync_end = 1080 + 32 + 32,
+ .htotal = 1080 + 32 + 32 + 98,
+ .vdisplay = 2160,
+ .vsync_start = 2160 + 32,
+ .vsync_end = 2160 + 32 + 4,
+ .vtotal = 2160 + 32 + 4 + 98,
+ .width_mm = 69,
+ .height_mm = 137,
+ .type = DRM_MODE_TYPE_DRIVER,
+};
+
+static int nt37700f_tianma_get_modes(struct drm_panel *panel,
+ struct drm_connector *connector)
+{
+ return drm_connector_helper_get_modes_fixed(connector, &nt37700f_tianma_mode);
+}
+
+static const struct drm_panel_funcs nt37700f_tianma_panel_funcs = {
+ .prepare = nt37700f_tianma_prepare,
+ .unprepare = nt37700f_tianma_unprepare,
+ .disable = nt37700f_tianma_disable,
+ .get_modes = nt37700f_tianma_get_modes,
+};
+
+static int nt37700f_tianma_bl_update_status(struct backlight_device *bl)
+{
+ struct mipi_dsi_device *dsi = bl_get_data(bl);
+ u16 brightness = backlight_get_brightness(bl);
+ int ret;
+
+ dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+ ret = mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);
+ if (ret < 0)
+ return ret;
+
+ dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+ return 0;
+}
+
+// TODO: Check if /sys/class/backlight/.../actual_brightness actually returns
+// correct values. If not, remove this function.
+static int nt37700f_tianma_bl_get_brightness(struct backlight_device *bl)
+{
+ struct mipi_dsi_device *dsi = bl_get_data(bl);
+ u16 brightness;
+ int ret;
+
+ dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+ ret = mipi_dsi_dcs_get_display_brightness_large(dsi, &brightness);
+ if (ret < 0)
+ return ret;
+
+ dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+ return brightness;
+}
+
+static const struct backlight_ops nt37700f_tianma_bl_ops = {
+ .update_status = nt37700f_tianma_bl_update_status,
+ .get_brightness = nt37700f_tianma_bl_get_brightness,
+};
+
+static struct backlight_device *
+nt37700f_tianma_create_backlight(struct mipi_dsi_device *dsi)
+{
+ struct device *dev = &dsi->dev;
+ const struct backlight_properties props = {
+ .type = BACKLIGHT_RAW,
+ .brightness = 2047,
+ .max_brightness = 2047,
+ };
+
+ return devm_backlight_device_register(dev, dev_name(dev), dev, dsi,
+ &nt37700f_tianma_bl_ops, &props);
+}
+
+static int nt37700f_tianma_probe(struct mipi_dsi_device *dsi)
+{
+ struct device *dev = &dsi->dev;
+ struct nt37700f_tianma *ctx;
+ int ret;
+
+ ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+ if (!ctx)
+ return -ENOMEM;
+
+ ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(ctx->reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio),
+ "Failed to get reset-gpios\n");
+
+ ctx->dsi = dsi;
+ mipi_dsi_set_drvdata(dsi, ctx);
+
+ dsi->lanes = 4;
+ dsi->format = MIPI_DSI_FMT_RGB888;
+ dsi->mode_flags = MIPI_DSI_MODE_VIDEO_BURST |
+ MIPI_DSI_CLOCK_NON_CONTINUOUS | MIPI_DSI_MODE_LPM;
+
+ drm_panel_init(&ctx->panel, dev, &nt37700f_tianma_panel_funcs,
+ DRM_MODE_CONNECTOR_DSI);
+ ctx->panel.prepare_prev_first = true;
+
+ ctx->panel.backlight = nt37700f_tianma_create_backlight(dsi);
+ if (IS_ERR(ctx->panel.backlight))
+ return dev_err_probe(dev, PTR_ERR(ctx->panel.backlight),
+ "Failed to create backlight\n");
+
+ drm_panel_add(&ctx->panel);
+
+ ret = mipi_dsi_attach(dsi);
+ if (ret < 0) {
+ drm_panel_remove(&ctx->panel);
+ return dev_err_probe(dev, ret, "Failed to attach to DSI host\n");
+ }
+
+ return 0;
+}
+
+static void nt37700f_tianma_remove(struct mipi_dsi_device *dsi)
+{
+ struct nt37700f_tianma *ctx = mipi_dsi_get_drvdata(dsi);
+ int ret;
+
+ ret = mipi_dsi_detach(dsi);
+ if (ret < 0)
+ dev_err(&dsi->dev, "Failed to detach from DSI host: %d\n", ret);
+
+ drm_panel_remove(&ctx->panel);
+}
+
+static const struct of_device_id nt37700f_tianma_of_match[] = {
+ { .compatible = "novatek,nt37700f" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, nt37700f_tianma_of_match);
+
+static struct mipi_dsi_driver nt37700f_tianma_driver = {
+ .probe = nt37700f_tianma_probe,
+ .remove = nt37700f_tianma_remove,
+ .driver = {
+ .name = "panel-novatek-nt37700f",
+ .of_match_table = nt37700f_tianma_of_match,
+ },
+};
+module_mipi_dsi_driver(nt37700f_tianma_driver);
+
+MODULE_DESCRIPTION("DRM driver for nt37700f cmd mode dsi tianma panel");
+MODULE_LICENSE("GPL");
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Claude review: drm/panel: Add Novatek/Tianma NT37700F panel
2026-02-10 2:32 ` [PATCH 3/6] drm/panel: Add Novatek/Tianma NT37700F panel Richard Acayan
@ 2026-02-11 6:47 ` Claude Code Review Bot
0 siblings, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-02-11 6:47 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Code Quality Issues:**
**1. TODO in Production Code (CRITICAL):**
```c
+// TODO: Check if /sys/class/backlight/.../actual_brightness actually returns
+// correct values. If not, remove this function.
+static int nt37700f_tianma_bl_get_brightness(struct backlight_device *bl)
```
Production code submitted to mainline must not contain TODO comments. Either test the functionality or remove the function. The backlight core doesn't require `get_brightness` - it's optional.
**2. Compatible String Issue (CRITICAL):**
```c
+static const struct of_device_id nt37700f_tianma_of_match[] = {
+ { .compatible = "novatek,nt37700f" },
```
The driver is named `nt37700f_tianma` (Tianma-specific), but uses generic compatible `novatek,nt37700f`. The cover letter states: "The Pixel 3a XL has two variants with panels from Samsung or from Tianma/Novatek."
This confirms multiple vendors use NT37700F with potentially different init sequences. The compatible MUST be vendor-specific:
```c
{ .compatible = "tianma,nt37700f" },
```
**3. Module Description Error:**
```c
+MODULE_DESCRIPTION("DRM driver for nt37700f cmd mode dsi tianma panel");
```
But the driver sets:
```c
+ dsi->mode_flags = MIPI_DSI_MODE_VIDEO_BURST |
```
This is VIDEO_BURST mode, not command mode. Fix:
```c
MODULE_DESCRIPTION("DRM driver for Tianma NT37700F DSI video mode panel");
```
**4. Undocumented Magic Values:**
```c
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x55, 0xaa, 0x52, 0x08, 0x00);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xc0, 0x56);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xca, 0x52);
```
Add a comment explaining the source of these sequences (downstream DT, datasheet, or reverse-engineered).
**5. Clock Calculation:**
```c
+ .clock = (1080 + 32 + 32 + 98) * (2160 + 32 + 4 + 98) * 60 / 1000,
```
While correct, add a comment indicating where porch values originate.
**Positive Aspects:**
- Proper use of `mipi_dsi_multi_context` for error accumulation
- Correct GPIO handling with `gpiod_set_value_cansleep`
- Good error handling with `dev_err_probe()`
- Proper resource cleanup
- Correct use of `DRM_MODE_CONNECTOR_DSI`
**Rating:** NEEDS REVISION - Remove TODO, fix compatible string, fix module description
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread