* [PATCH v3 1/5] dt-bindings: arm: qcom: document google, bonito-tianma board
2026-03-10 0:26 [PATCH v3 0/5] Support for the Pixel 3a XL with the Tianma panel Richard Acayan
@ 2026-03-10 0:26 ` Richard Acayan
2026-03-10 1:42 ` Claude review: " Claude Code Review Bot
2026-03-10 0:26 ` [PATCH v3 2/5] dt-bindings: panel-simple-dsi: add nt37700f compatible Richard Acayan
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ 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
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>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.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 d48c625d3fc4..1a955d1b14bf 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -844,6 +844,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] 14+ messages in thread* [PATCH v3 2/5] dt-bindings: panel-simple-dsi: add nt37700f compatible
2026-03-10 0:26 [PATCH v3 0/5] Support for the Pixel 3a XL with the Tianma panel Richard Acayan
2026-03-10 0:26 ` [PATCH v3 1/5] dt-bindings: arm: qcom: document google, bonito-tianma board Richard Acayan
@ 2026-03-10 0:26 ` Richard Acayan
2026-03-10 1:42 ` Claude review: " Claude Code Review Bot
2026-03-10 0:26 ` [PATCH v3 3/5] drm/panel: Add Novatek/Tianma NT37700F panel Richard Acayan
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ 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
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 2f90c887b7b8..cc8d795df732 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] 14+ messages in thread* Claude review: dt-bindings: panel-simple-dsi: add nt37700f compatible
2026-03-10 0:26 ` [PATCH v3 2/5] dt-bindings: panel-simple-dsi: add nt37700f compatible Richard Acayan
@ 2026-03-10 1:42 ` Claude Code Review Bot
0 siblings, 0 replies; 14+ messages in thread
From: Claude Code Review Bot @ 2026-03-10 1:42 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Questionable — potential conflict with patch 3.**
This adds `novatek,nt37700f` to `panel-simple-dsi.yaml`:
```
+ # Novatek NT37700F 1080x2160 AMOLED panel
+ - novatek,nt37700f
```
However, patch 3 adds a **dedicated panel driver** (`panel-novatek-nt37700f.c`), not a `panel-simple` entry. The `panel-simple-dsi.yaml` binding describes panels that only need a single `power-supply`, a backlight, and an enable GPIO — with no init sequences. The NT37700F driver has an extensive DSI init sequence (`nt37700f_tianma_on()`), page-switching commands, and custom backlight handling, which makes it clearly *not* a "simple" panel.
Adding the compatible to `panel-simple-dsi.yaml` while also having a dedicated driver could cause DT validation confusion. Typically, panels with dedicated drivers get their own binding YAML file (or are documented in the driver's own binding). Unless this is intentionally done so the panel can fall back to `panel-simple` without init commands (e.g., if the bootloader already initializes it), this seems like a mistake. The binding should probably be a standalone `novatek,nt37700f.yaml` instead.
That said, I notice this already has an ack from Krzysztof, so perhaps this is intentional for this use case, but it's worth clarifying.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 3/5] drm/panel: Add Novatek/Tianma NT37700F panel
2026-03-10 0:26 [PATCH v3 0/5] Support for the Pixel 3a XL with the Tianma panel Richard Acayan
2026-03-10 0:26 ` [PATCH v3 1/5] dt-bindings: arm: qcom: document google, bonito-tianma board Richard Acayan
2026-03-10 0:26 ` [PATCH v3 2/5] dt-bindings: panel-simple-dsi: add nt37700f compatible Richard Acayan
@ 2026-03-10 0:26 ` Richard Acayan
2026-03-10 1:42 ` Claude review: " Claude Code Review Bot
2026-03-10 0:26 ` [PATCH v3 4/5] arm64: dts: qcom: sdm670-google: add common device tree include Richard Acayan
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ 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] 14+ 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; 14+ 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] 14+ messages in thread
* [PATCH v3 4/5] arm64: dts: qcom: sdm670-google: add common device tree include
2026-03-10 0:26 [PATCH v3 0/5] Support for the Pixel 3a XL with the Tianma panel Richard Acayan
` (2 preceding siblings ...)
2026-03-10 0:26 ` [PATCH v3 3/5] drm/panel: Add Novatek/Tianma NT37700F panel Richard Acayan
@ 2026-03-10 0:26 ` Richard Acayan
2026-03-10 1:42 ` Claude review: " Claude Code Review Bot
2026-03-10 0:26 ` [PATCH v3 5/5] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel Richard Acayan
2026-03-10 1:42 ` Claude review: Support for the Pixel 3a XL with the Tianma panel Claude Code Review Bot
5 siblings, 1 reply; 14+ 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
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.
Since the original devicetree should only specify non-XL properties, it
needs to be completely rewritten. Also change the SPDX license
identifier from GPL-2.0 (which was deprecated as it can be
misinterpreted as GPLv2 or later) into GPL-2.0-only.
Signed-off-by: Richard Acayan <mailingradian@gmail.com>
---
...le-sargo.dts => sdm670-google-common.dtsi} | 22 +-
.../boot/dts/qcom/sdm670-google-sargo.dts | 709 +-----------------
2 files changed, 20 insertions(+), 711 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 e925cba0381f..6116ca8dc426 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 {
@@ -48,11 +42,11 @@ chosen {
#size-cells = <2>;
ranges;
- framebuffer@9c000000 {
+ framebuffer: framebuffer@9c000000 {
compatible = "simple-framebuffer";
- reg = <0 0x9c000000 0 (1080 * 2220 * 4)>;
+ memory-region = <&fb_mem>;
width = <1080>;
- height = <2220>;
+ /* height is in specific device trees */
stride = <(1080 * 4)>;
format = "a8r8g8b8";
};
@@ -121,7 +115,7 @@ gpu_mem: gpu@97a15000 {
no-map;
};
- framebuffer-region@9c000000 {
+ fb_mem: framebuffer-region@9c000000 {
reg = <0 0x9c000000 0 0x2400000>;
no-map;
};
@@ -516,10 +510,8 @@ 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>;
syna,sensor-type = <1>;
};
};
@@ -533,8 +525,8 @@ &mdss_dsi0 {
vdda-supply = <&vreg_l1a_1p225>;
status = "okay";
- panel@0 {
- compatible = "samsung,s6e3fa7-ams559nk06";
+ panel: panel@0 {
+ /* compatible is provided in specific device trees */
reg = <0>;
reset-gpios = <&tlmm 75 GPIO_ACTIVE_LOW>;
diff --git a/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts b/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
index e925cba0381f..c67a0396436b 100644
--- a/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
+++ b/arch/arm64/boot/dts/qcom/sdm670-google-sargo.dts
@@ -1,715 +1,32 @@
-// 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) 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@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@12 {
- reg = <0x12>;
- touchscreen-x-mm = <62>;
- touchscreen-y-mm = <127>;
- syna,sensor-type = <1>;
- };
- };
-};
-
-&mdss {
- status = "okay";
-};
-
-&mdss_dsi0 {
- vdda-supply = <&vreg_l1a_1p225>;
- status = "okay";
-
- 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>;
+&battery {
+ charge-full-design-microamp-hours = <3000000>;
};
-&mdss_dsi0_phy {
- vdds-supply = <&vreg_l1b_0p925>;
- status = "okay";
+&framebuffer {
+ height = <2220>;
};
-&mdss_mdp {
- status = "okay";
+&panel {
+ compatible = "samsung,s6e3fa7-ams559nk06";
};
-&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 = <62>;
+ touchscreen-y-mm = <127>;
};
--
2.53.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Claude review: arm64: dts: qcom: sdm670-google: add common device tree include
2026-03-10 0:26 ` [PATCH v3 4/5] arm64: dts: qcom: sdm670-google: add common device tree include Richard Acayan
@ 2026-03-10 1:42 ` Claude Code Review Bot
0 siblings, 0 replies; 14+ messages in thread
From: Claude Code Review Bot @ 2026-03-10 1:42 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Good refactoring, one concern.**
The factoring approach is correct. The common dtsi removes device-specific properties (`model`, `compatible`, `charge-full-design-microamp-hours`, `height`, `touchscreen-x-mm`, `touchscreen-y-mm`, and panel `compatible`) and adds labels (`framebuffer:`, `fb_mem:`, `panel:`, `rmi4_f12:`) for overriding in device-specific files.
1. **Framebuffer `height` comment**: The common dtsi replaces the `height` property with a comment:
```
+ /* height is in specific device trees */
```
This is fine for documentation, but it means the common framebuffer node lacks a required `height` property for `simple-framebuffer`. If used standalone (which it isn't, as a `.dtsi`), this would be invalid. This is acceptable since the `.dtsi` is always included by a `.dts` that provides `height`, but the DT compiler may not warn if a `.dts` forgets to set it.
2. **Memory-region change**: The framebuffer changes from `reg` to `memory-region`:
```
- reg = <0 0x9c000000 0 (1080 * 2220 * 4)>;
+ memory-region = <&fb_mem>;
```
This is a good change since it references the reserved memory region properly, but it changes the binding usage for the existing sargo board. The `fb_mem` region is `0x2400000` bytes (about 37.7 MB) which is larger than `1080 * 2220 * 4 = 9,590,400` bytes. This should still work since `simple-framebuffer` only reads the needed portion, but the semantics are slightly different.
3. **License change**: The SPDX change from `GPL-2.0` to `GPL-2.0-only` is well-explained in the commit message.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 5/5] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel
2026-03-10 0:26 [PATCH v3 0/5] Support for the Pixel 3a XL with the Tianma panel Richard Acayan
` (3 preceding siblings ...)
2026-03-10 0:26 ` [PATCH v3 4/5] arm64: dts: qcom: sdm670-google: add common device tree include Richard Acayan
@ 2026-03-10 0:26 ` Richard Acayan
2026-03-10 1:42 ` Claude review: " Claude Code Review Bot
2026-03-10 1:42 ` Claude review: Support for the Pixel 3a XL with the Tianma panel Claude Code Review Bot
5 siblings, 1 reply; 14+ 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
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>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/Makefile | 1 +
.../dts/qcom/sdm670-google-bonito-tianma.dts | 32 +++++++++++++++++++
2 files changed, 33 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 f80b5d9cf1e8..7c158871ed1c 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -264,6 +264,7 @@ dtb-$(CONFIG_ARCH_QCOM) += sdm632-fairphone-fp3.dtb
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-bonito-tianma.dtb
dtb-$(CONFIG_ARCH_QCOM) += sdm670-google-sargo.dtb
dtb-$(CONFIG_ARCH_QCOM) += sdm845-db845c.dtb
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..42ddaf53d150
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sdm670-google-bonito-tianma.dts
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Device tree for Google Pixel 3a XL with the Tianma panel.
+ *
+ * Copyright (c) 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 {
+ height = <2160>;
+};
+
+&panel {
+ compatible = "novatek,nt37700f";
+};
+
+&rmi4_f12 {
+ touchscreen-x-mm = <69>;
+ touchscreen-y-mm = <137>;
+};
--
2.53.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Claude review: arm64: dts: qcom: add support for pixel 3a xl with the tianma panel
2026-03-10 0:26 ` [PATCH v3 5/5] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel Richard Acayan
@ 2026-03-10 1:42 ` Claude Code Review Bot
0 siblings, 0 replies; 14+ messages in thread
From: Claude Code Review Bot @ 2026-03-10 1:42 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Looks good.**
Clean device tree for the Pixel 3a XL Tianma variant. It includes the common dtsi and sets:
- `compatible = "google,bonito-tianma", "google,bonito", "qcom,sdm670"` — matches the binding from patch 1
- `charge-full-design-microamp-hours = <3700000>` — different battery from sargo (3000 vs 3700 mAh)
- `height = <2160>` — different panel resolution from sargo (2220 vs 2160)
- `compatible = "novatek,nt37700f"` — the Tianma panel
- Touchscreen dimensions matching the larger display
Already has Konrad's Reviewed-by. No issues.
**Summary of actionable items:**
1. **(Patch 2)** Clarify whether `panel-simple-dsi.yaml` is the correct place for the binding, given the dedicated driver in patch 3.
2. **(Patch 3)** Fix the error path in `nt37700f_tianma_prepare()` to disable the regulator on init failure.
3. **(Patch 3, minor)** Consider removing `MIPI_DSI_MODE_VIDEO_BURST` flag if this is a command-mode panel.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 14+ messages in thread
* Claude review: Support for the Pixel 3a XL with the Tianma panel
2026-03-10 0:26 [PATCH v3 0/5] Support for the Pixel 3a XL with the Tianma panel Richard Acayan
` (4 preceding siblings ...)
2026-03-10 0:26 ` [PATCH v3 5/5] arm64: dts: qcom: add support for pixel 3a xl with the tianma panel Richard Acayan
@ 2026-03-10 1:42 ` Claude Code Review Bot
5 siblings, 0 replies; 14+ messages in thread
From: Claude Code Review Bot @ 2026-03-10 1:42 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: 6
Reviewed: 2026-03-10T11:42:22.690547
---
This is a well-structured v3 series adding Pixel 3a XL (codename "bonito") support with the Tianma/Novatek NT37700F panel variant. The approach of factoring the existing sargo (Pixel 3a) device tree into a common `.dtsi` and device-specific `.dts` files is the correct pattern. The panel driver is generated via `linux-mdss-dsi-panel-driver-generator` and uses modern multi-context DSI APIs. The series has already collected several review/ack tags from maintainers (Krzysztof Kozlowski, Konrad Dybcio, Dmitry Baryshkov, Yifei Zhan).
Overall this looks good to merge. I have a few observations, mostly minor.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 14+ messages in thread