* [PATCH RFC 01/10] dt-bindings: backlight: add Kinetic KTD3136
2026-09-11 8:50 [PATCH RFC 00/10] arm64: qcom: sm6125-ginkgo: display, backlight, and touch support YİĞİTCAN KAVAKLI via B4 Relay
@ 2026-09-11 8:50 ` YİĞİTCAN KAVAKLI via B4 Relay
2026-09-11 8:56 ` sashiko-bot
2026-09-11 8:50 ` [PATCH RFC 02/10] backlight: add Kinetic KTD3136 driver YİĞİTCAN KAVAKLI via B4 Relay
` (8 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: YİĞİTCAN KAVAKLI via B4 Relay @ 2026-09-11 8:50 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Liam Girdwood,
Mark Brown, Sumit Semwal, Neil Armstrong, Jessica Zhang,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Dmitry Torokhov, Henrik Rydberg, Rob Clark,
Dmitry Baryshkov, Abhinav Kumar, Sean Paul, Marijn Suijten,
Bjorn Andersson, Konrad Dybcio
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev,
linux-input, linux-arm-msm, freedreno,
YİĞİTCAN KAVAKLI
From: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
Add devicetree binding documentation for the Kinetic Technologies
KTD3136 3-channel LED backlight driver using the official "kinetic"
vendor prefix.
The device is controlled over I2C and supports an optional hardware
enable GPIO line (enable-gpios), supply regulators, and standard
backlight brightness range properties.
Signed-off-by: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
---
.../bindings/leds/backlight/kinetic,ktd3136.yaml | 65 ++++++++++++++++++++++
1 file changed, 65 insertions(+)
diff --git a/Documentation/devicetree/bindings/leds/backlight/kinetic,ktd3136.yaml b/Documentation/devicetree/bindings/leds/backlight/kinetic,ktd3136.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..f85381c29964529a09989a1ff60d9c9fe4897bb7
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/kinetic,ktd3136.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/kinetic,ktd3136.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Kinetic Technologies KTD3136 3-Channel LED Backlight Driver
+
+maintainers:
+ - Lee Jones <lee@kernel.org>
+
+description: |
+ The Kinetic Technologies KTD3136 is a high efficiency, 3-channel LED
+ backlight driver with 11-bit exponential or linear dimming control
+ via I2C interface.
+
+allOf:
+ - $ref: common.yaml#
+
+properties:
+ compatible:
+ const: kinetic,ktd3136
+
+ reg:
+ maxItems: 1
+
+ enable-gpios:
+ description: GPIO connected to the HWEN (Chip Enable) pin.
+ maxItems: 1
+
+ vin-supply:
+ description: Regulator providing power to the VIN pin.
+
+ default-brightness:
+ minimum: 0
+ maximum: 2047
+ default: 2047
+
+ max-brightness:
+ minimum: 0
+ maximum: 2047
+ default: 2047
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ backlight@36 {
+ compatible = "kinetic,ktd3136";
+ reg = <0x36>;
+ enable-gpios = <&gpio 6 GPIO_ACTIVE_HIGH>;
+ default-brightness = <2047>;
+ max-brightness = <2047>;
+ };
+ };
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH RFC 01/10] dt-bindings: backlight: add Kinetic KTD3136
2026-09-11 8:50 ` [PATCH RFC 01/10] dt-bindings: backlight: add Kinetic KTD3136 YİĞİTCAN KAVAKLI via B4 Relay
@ 2026-09-11 8:56 ` sashiko-bot
0 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-09-11 8:56 UTC (permalink / raw)
To: YİĞİTCAN KAVAKLI
Cc: robh, conor+dt, devicetree, dri-devel, linux-input,
dmitry.torokhov, linux-leds, lee
> From: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
>
> Add devicetree binding documentation for the Kinetic Technologies
> KTD3136 3-channel LED backlight driver using the official "kinetic"
> vendor prefix.
>
> The device is controlled over I2C and supports an optional hardware
> enable GPIO line (enable-gpios), supply regulators, and standard
> backlight brightness range properties.
>
> Signed-off-by: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-ginkgo-submission-final-v1-0-0e68e63a18d4@gmail.com?part=1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH RFC 02/10] backlight: add Kinetic KTD3136 driver
2026-09-11 8:50 [PATCH RFC 00/10] arm64: qcom: sm6125-ginkgo: display, backlight, and touch support YİĞİTCAN KAVAKLI via B4 Relay
2026-09-11 8:50 ` [PATCH RFC 01/10] dt-bindings: backlight: add Kinetic KTD3136 YİĞİTCAN KAVAKLI via B4 Relay
@ 2026-09-11 8:50 ` YİĞİTCAN KAVAKLI via B4 Relay
2026-09-11 8:59 ` sashiko-bot
2026-09-11 8:51 ` [PATCH RFC 03/10] dt-bindings: display: panel: novatek,nt36672a: add Tianma FHD+ video mode variant YİĞİTCAN KAVAKLI via B4 Relay
` (7 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: YİĞİTCAN KAVAKLI via B4 Relay @ 2026-09-11 8:50 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Liam Girdwood,
Mark Brown, Sumit Semwal, Neil Armstrong, Jessica Zhang,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Dmitry Torokhov, Henrik Rydberg, Rob Clark,
Dmitry Baryshkov, Abhinav Kumar, Sean Paul, Marijn Suijten,
Bjorn Andersson, Konrad Dybcio
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev,
linux-input, linux-arm-msm, freedreno,
YİĞİTCAN KAVAKLI
From: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
Implement a generic Linux backlight driver for the Kinetic Technologies
KTD3136 36V, 6-channel LED driver.
The driver provides:
- Standard Linux backlight class interface (struct backlight_ops)
- 11-bit linear dimming resolution (0..2047 steps)
- Optional hardware enable (HWEN) GPIO sequencing
- Chip shutdown and register re-initialization upon resume/power-up
- Clean devm-managed resource allocation
Hardware evidence:
Tested on Qualcomm SM6125 platforms (Xiaomi Redmi Note 8) with smooth
dimming across the full 11-bit range and robust recovery across display
power cycles.
Signed-off-by: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
---
drivers/video/backlight/Kconfig | 12 ++
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/ktd3136-backlight.c | 262 ++++++++++++++++++++++++++++
3 files changed, 275 insertions(+)
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index dbf4ca23a9b6d8575a86e39618b312b4df43ea19..010e2ae3fa8cbd8e2f38e7941b373fbfeb5ede3f 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -207,6 +207,18 @@ config BACKLIGHT_KTZ8866
Say Y to enable the backlight driver for the Kinetic KTZ8866
found in Xiaomi Mi Pad 5 series.
+config BACKLIGHT_KTD3136
+ tristate "Backlight Driver for Kinetic KTD3136"
+ depends on I2C
+ select REGMAP_I2C
+ help
+ Say Y to enable the backlight driver for the Kinetic Technologies
+ KTD3136 3-channel I2C-controlled LED backlight driver found in mobile
+ phones such as the Xiaomi Redmi Note 8.
+
+ To compile this driver as a module, choose M here: the module will
+ be called ktd3136-backlight.
+
config BACKLIGHT_SY7758
tristate "Backlight Driver for Silergy SY7758"
depends on I2C
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 34469711c6cd957dae63c7072ade4830879aba1a..c6d324f60e021afaab1a3da1b4de74f6f2acc8f8 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_BACKLIGHT_IPAQ_MICRO) += ipaq_micro_bl.o
obj-$(CONFIG_BACKLIGHT_KTD253) += ktd253-backlight.o
obj-$(CONFIG_BACKLIGHT_KTD2801) += ktd2801-backlight.o
obj-$(CONFIG_BACKLIGHT_KTZ8866) += ktz8866.o
+obj-$(CONFIG_BACKLIGHT_KTD3136) += ktd3136-backlight.o
obj-$(CONFIG_BACKLIGHT_LM3509) += lm3509_bl.o
obj-$(CONFIG_BACKLIGHT_LM3533) += lm3533_bl.o
obj-$(CONFIG_BACKLIGHT_LM3630A) += lm3630a_bl.o
diff --git a/drivers/video/backlight/ktd3136-backlight.c b/drivers/video/backlight/ktd3136-backlight.c
new file mode 100644
index 0000000000000000000000000000000000000000..a8ebe3b23c99f3ada9f45c60688f8b8c78d72def
--- /dev/null
+++ b/drivers/video/backlight/ktd3136-backlight.c
@@ -0,0 +1,262 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Kinetic Technologies KTD3136 3-Channel LED Backlight Driver
+ *
+ * Copyright (C) 2026
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+
+#define KTD3136_REG_DEV_ID 0x00
+#define KTD3136_REG_SW_RESET 0x01
+#define KTD3136_REG_MODE 0x02
+#define KTD3136_REG_CONTROL 0x03
+#define KTD3136_REG_RATIO_LSB 0x04
+#define KTD3136_REG_RATIO_MSB 0x05
+#define KTD3136_REG_PWM 0x06
+#define KTD3136_REG_STATUS 0x0A
+
+#define KTD3136_DEV_ID_VAL 0x18
+#define KTD3136_DEV_ID_VAL2 0x19
+
+#define KTD3136_MODE_ON 0xC9 /* Boost ON, 3 channels enabled */
+#define KTD3136_MODE_STANDBY 0x98 /* Standby mode */
+#define KTD3136_CONTROL_LINEAR 0x02 /* Linear dimming mapping */
+#define KTD3136_PWM_DEFAULT 0x1B /* Default PWM/boost frequency */
+
+#define KTD3136_DEFAULT_MAX_BRIGHTNESS 2047 /* 11-bit resolution */
+#define KTD3136_DEFAULT_BRIGHTNESS 2047
+
+struct ktd3136_data {
+ struct i2c_client *client;
+ struct backlight_device *bd;
+ struct gpio_desc *enable_gpio;
+ struct regulator *vin;
+ /* Protects chip registers and state */
+ struct mutex lock;
+ bool is_enabled;
+};
+
+static int ktd3136_write(struct ktd3136_data *chip, u8 reg, u8 val)
+{
+ int ret;
+
+ ret = i2c_smbus_write_byte_data(chip->client, reg, val);
+ if (ret < 0)
+ dev_err(&chip->client->dev, "failed to write reg 0x%02x: %d\n", reg, ret);
+
+ return ret;
+}
+
+static int ktd3136_read(struct ktd3136_data *chip, u8 reg)
+{
+ int ret;
+
+ ret = i2c_smbus_read_byte_data(chip->client, reg);
+ if (ret < 0)
+ dev_err(&chip->client->dev, "failed to read reg 0x%02x: %d\n", reg, ret);
+
+ return ret;
+}
+
+static int ktd3136_power_on(struct ktd3136_data *chip)
+{
+ int ret;
+
+ if (chip->is_enabled)
+ return 0;
+
+ if (chip->enable_gpio) {
+ gpiod_set_value_cansleep(chip->enable_gpio, 1);
+ /* Allow oscillator and internal logic to stabilize */
+ usleep_range(2000, 3000);
+ }
+
+ ret = ktd3136_write(chip, KTD3136_REG_CONTROL, KTD3136_CONTROL_LINEAR);
+ if (ret < 0)
+ goto err_off;
+
+ ret = ktd3136_write(chip, KTD3136_REG_PWM, KTD3136_PWM_DEFAULT);
+ if (ret < 0)
+ goto err_off;
+
+ chip->is_enabled = true;
+ return 0;
+
+err_off:
+ if (chip->enable_gpio)
+ gpiod_set_value_cansleep(chip->enable_gpio, 0);
+ return ret;
+}
+
+static void ktd3136_power_off(struct ktd3136_data *chip)
+{
+ if (!chip->is_enabled)
+ return;
+
+ ktd3136_write(chip, KTD3136_REG_MODE, KTD3136_MODE_STANDBY);
+
+ if (chip->enable_gpio)
+ gpiod_set_value_cansleep(chip->enable_gpio, 0);
+
+ chip->is_enabled = false;
+}
+
+static int ktd3136_update_status(struct backlight_device *bd)
+{
+ struct ktd3136_data *chip = bl_get_data(bd);
+ int brightness = backlight_get_brightness(bd);
+ int ret = 0;
+ u8 lsb, msb;
+
+ mutex_lock(&chip->lock);
+
+ if (backlight_is_blank(bd) || brightness == 0) {
+ ktd3136_power_off(chip);
+ goto out;
+ }
+
+ ret = ktd3136_power_on(chip);
+ if (ret < 0)
+ goto out;
+
+ lsb = brightness & 0x07;
+ msb = (brightness >> 3) & 0xFF;
+
+ ret = ktd3136_write(chip, KTD3136_REG_RATIO_LSB, lsb);
+ if (ret < 0)
+ goto out;
+
+ ret = ktd3136_write(chip, KTD3136_REG_RATIO_MSB, msb);
+ if (ret < 0)
+ goto out;
+
+ ret = ktd3136_write(chip, KTD3136_REG_MODE, KTD3136_MODE_ON);
+
+out:
+ mutex_unlock(&chip->lock);
+ return ret;
+}
+
+static const struct backlight_ops ktd3136_backlight_ops = {
+ .options = BL_CORE_SUSPENDRESUME,
+ .update_status = ktd3136_update_status,
+};
+
+static int ktd3136_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct backlight_properties props;
+ struct ktd3136_data *chip;
+ int ret, val;
+ u32 def_brightness = KTD3136_DEFAULT_BRIGHTNESS;
+ u32 max_brightness = KTD3136_DEFAULT_MAX_BRIGHTNESS;
+
+ chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+ if (!chip)
+ return -ENOMEM;
+
+ chip->client = client;
+ mutex_init(&chip->lock);
+
+ chip->vin = devm_regulator_get_optional(dev, "vin");
+ if (IS_ERR(chip->vin)) {
+ ret = PTR_ERR(chip->vin);
+ if (ret != -ENODEV)
+ return dev_err_probe(dev, ret, "Failed to get vin regulator\n");
+ chip->vin = NULL;
+ }
+
+ if (chip->vin) {
+ ret = regulator_enable(chip->vin);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to enable vin regulator\n");
+ }
+
+ chip->enable_gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_HIGH);
+ if (IS_ERR(chip->enable_gpio))
+ return dev_err_probe(dev, PTR_ERR(chip->enable_gpio),
+ "Failed to get enable GPIO\n");
+
+ if (chip->enable_gpio)
+ usleep_range(2000, 3000);
+
+ val = ktd3136_read(chip, KTD3136_REG_DEV_ID);
+ if (val < 0)
+ return dev_err_probe(dev, val, "Failed to read device ID\n");
+
+ if (val != KTD3136_DEV_ID_VAL && val != KTD3136_DEV_ID_VAL2) {
+ dev_err(dev, "Unknown device ID: 0x%02x\n", val);
+ return -ENODEV;
+ }
+
+ dev_info(dev, "Kinetic KTD3136 detected (ID: 0x%02x)\n", val);
+
+ device_property_read_u32(dev, "max-brightness", &max_brightness);
+ if (max_brightness > KTD3136_DEFAULT_MAX_BRIGHTNESS)
+ max_brightness = KTD3136_DEFAULT_MAX_BRIGHTNESS;
+
+ device_property_read_u32(dev, "default-brightness", &def_brightness);
+ if (def_brightness > max_brightness)
+ def_brightness = max_brightness;
+
+ memset(&props, 0, sizeof(props));
+ props.type = BACKLIGHT_RAW;
+ props.max_brightness = max_brightness;
+ props.brightness = def_brightness;
+
+ chip->bd = devm_backlight_device_register(dev, dev_name(dev), dev, chip,
+ &ktd3136_backlight_ops, &props);
+ if (IS_ERR(chip->bd))
+ return dev_err_probe(dev, PTR_ERR(chip->bd),
+ "Failed to register backlight device\n");
+
+ backlight_update_status(chip->bd);
+
+ return 0;
+}
+
+static void ktd3136_remove(struct i2c_client *client)
+{
+ struct ktd3136_data *chip = i2c_get_clientdata(client);
+
+ if (chip)
+ ktd3136_power_off(chip);
+}
+
+static const struct of_device_id ktd3136_of_match[] = {
+ { .compatible = "kinetic,ktd3136" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ktd3136_of_match);
+
+static const struct i2c_device_id ktd3136_i2c_id[] = {
+ { "ktd3136", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, ktd3136_i2c_id);
+
+static struct i2c_driver ktd3136_driver = {
+ .driver = {
+ .name = "ktd3136-backlight",
+ .of_match_table = ktd3136_of_match,
+ },
+ .probe = ktd3136_probe,
+ .remove = ktd3136_remove,
+ .id_table = ktd3136_i2c_id,
+};
+module_i2c_driver(ktd3136_driver);
+
+MODULE_AUTHOR("Antigravity Team");
+MODULE_DESCRIPTION("Kinetic Technologies KTD3136 Backlight Driver");
+MODULE_LICENSE("GPL");
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH RFC 02/10] backlight: add Kinetic KTD3136 driver
2026-09-11 8:50 ` [PATCH RFC 02/10] backlight: add Kinetic KTD3136 driver YİĞİTCAN KAVAKLI via B4 Relay
@ 2026-09-11 8:59 ` sashiko-bot
0 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-09-11 8:59 UTC (permalink / raw)
To: YİĞİTCAN KAVAKLI
Cc: lee, linux-leds, dmitry.torokhov, conor+dt, dri-devel,
linux-input, robh, devicetree
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] The 'vin' regulator is enabled during probe but never disabled on error paths or during module removal.
- [High] Missing `i2c_set_clientdata()` in probe causes `ktd3136_remove()` to silently skip hardware power-off.
- [Medium] Missing lock acquisition in `ktd3136_remove` causes a data race with concurrent backlight sysfs updates.
- [Medium] State mismatch leaves backlight physically ON when initial brightness is 0.
- [Low] Unnecessary and unused `select REGMAP_I2C` in Kconfig.
--
commit 15d6d172969378ae4661195e2320fcd1b4ed08c1
Author: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
backlight: add Kinetic KTD3136 driver
This commit implements a generic Linux backlight driver for the Kinetic
Technologies KTD3136 36V, 6-channel LED driver. It uses the standard
backlight class interface over I2C.
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index dbf4ca23a9b6d..010e2ae3fa8cb 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
[ ... ]
> +config BACKLIGHT_KTD3136
> + tristate "Backlight Driver for Kinetic KTD3136"
> + depends on I2C
> + select REGMAP_I2C
[Severity: Low]
Does this driver actually use the regmap API? Looking at the code, it appears
to exclusively use raw I2C SMBus operations (e.g., i2c_smbus_read_byte_data).
Could this select REGMAP_I2C be safely removed to avoid forcing an unnecessary
dependency?
> + help
> + Say Y to enable the backlight driver for the Kinetic Technologies
[ ... ]
> diff --git a/drivers/video/backlight/ktd3136-backlight.c b/drivers/video/backlight/ktd3136-backlight.c
> new file mode 100644
> index 0000000000000..a8ebe3b23c99f
> --- /dev/null
> +++ b/drivers/video/backlight/ktd3136-backlight.c
[ ... ]
> +static int ktd3136_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct backlight_properties props;
> + struct ktd3136_data *chip;
> + int ret, val;
> + u32 def_brightness = KTD3136_DEFAULT_BRIGHTNESS;
> + u32 max_brightness = KTD3136_DEFAULT_MAX_BRIGHTNESS;
> +
> + chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
> + if (!chip)
> + return -ENOMEM;
> +
> + chip->client = client;
> + mutex_init(&chip->lock);
[Severity: High]
Is a call to i2c_set_clientdata(client, chip) missing here? Later in
ktd3136_remove(), the code calls i2c_get_clientdata(client), which will
return NULL if it wasn't set, silently bypassing the hardware power-off.
> +
> + chip->vin = devm_regulator_get_optional(dev, "vin");
> + if (IS_ERR(chip->vin)) {
> + ret = PTR_ERR(chip->vin);
> + if (ret != -ENODEV)
> + return dev_err_probe(dev, ret, "Failed to get vin regulator\n");
> + chip->vin = NULL;
> + }
> +
> + if (chip->vin) {
> + ret = regulator_enable(chip->vin);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to enable vin regulator\n");
> + }
> +
> + chip->enable_gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_HIGH);
> + if (IS_ERR(chip->enable_gpio))
> + return dev_err_probe(dev, PTR_ERR(chip->enable_gpio),
> + "Failed to get enable GPIO\n");
[Severity: High]
If devm_gpiod_get_optional() fails here, the function returns immediately.
Does this leave the vin regulator enabled since there is no devm action or
manual regulator_disable(chip->vin) on this error path?
[Severity: Medium]
The enable GPIO is asserted HIGH here (powering the hardware), but
chip->is_enabled remains false. If the default brightness evaluates to 0,
backlight_update_status() calls ktd3136_power_off(). Because chip->is_enabled
is false, ktd3136_power_off() returns early without de-asserting the GPIO.
Could this state mismatch leave the backlight physically ON when the initial
brightness is 0?
> +
> + if (chip->enable_gpio)
> + usleep_range(2000, 3000);
[ ... ]
> +static void ktd3136_remove(struct i2c_client *client)
> +{
> + struct ktd3136_data *chip = i2c_get_clientdata(client);
> +
> + if (chip)
> + ktd3136_power_off(chip);
[Severity: Medium]
Does ktd3136_power_off(chip) need to be protected by mutex_lock(&chip->lock)
here? Since the backlight device is registered with
devm_backlight_device_register, its sysfs interface might still be active
during .remove(). Could this cause a data race with concurrent sysfs writes
to the brightness attribute?
[Severity: High]
Does the vin regulator need to be disabled here? It was enabled during probe
but there doesn't seem to be a corresponding regulator_disable() call when the
driver is unbound.
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-ginkgo-submission-final-v1-0-0e68e63a18d4@gmail.com?part=2
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH RFC 03/10] dt-bindings: display: panel: novatek,nt36672a: add Tianma FHD+ video mode variant
2026-09-11 8:50 [PATCH RFC 00/10] arm64: qcom: sm6125-ginkgo: display, backlight, and touch support YİĞİTCAN KAVAKLI via B4 Relay
2026-09-11 8:50 ` [PATCH RFC 01/10] dt-bindings: backlight: add Kinetic KTD3136 YİĞİTCAN KAVAKLI via B4 Relay
2026-09-11 8:50 ` [PATCH RFC 02/10] backlight: add Kinetic KTD3136 driver YİĞİTCAN KAVAKLI via B4 Relay
@ 2026-09-11 8:51 ` YİĞİTCAN KAVAKLI via B4 Relay
2026-09-11 8:53 ` sashiko-bot
2026-09-11 8:51 ` [PATCH RFC 04/10] drm/panel: novatek-nt36672a: add Tianma FHD+ video mode panel YİĞİTCAN KAVAKLI via B4 Relay
` (6 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: YİĞİTCAN KAVAKLI via B4 Relay @ 2026-09-11 8:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Liam Girdwood,
Mark Brown, Sumit Semwal, Neil Armstrong, Jessica Zhang,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Dmitry Torokhov, Henrik Rydberg, Rob Clark,
Dmitry Baryshkov, Abhinav Kumar, Sean Paul, Marijn Suijten,
Bjorn Andersson, Konrad Dybcio
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev,
linux-input, linux-arm-msm, freedreno,
YİĞİTCAN KAVAKLI
From: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
Add compatible string "tianma,fhd-video-ginkgo" for the 1080x2340 Tianma
video mode panel found on the Xiaomi Redmi Note 8 (ginkgo / SM6125).
The downstream vendor DT identifies the panel as "nt36672a video mode
dsi tianma panel" without documenting a glass model number. Following
the established convention of "tianma,fhd-video" for Poco F1, this
variant is added under the existing NT36672A binding schema.
Signed-off-by: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
---
Documentation/devicetree/bindings/display/panel/novatek,nt36672a.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/display/panel/novatek,nt36672a.yaml b/Documentation/devicetree/bindings/display/panel/novatek,nt36672a.yaml
index 5d16d85117254ce479cfe9b9b33df8eeafeed1e1..9f42b1ef07669a3b0e6d2285a39a961e2e04cec7 100644
--- a/Documentation/devicetree/bindings/display/panel/novatek,nt36672a.yaml
+++ b/Documentation/devicetree/bindings/display/panel/novatek,nt36672a.yaml
@@ -23,6 +23,7 @@ properties:
items:
- enum:
- tianma,fhd-video
+ - tianma,fhd-video-ginkgo
- const: novatek,nt36672a
description: This indicates the panel manufacturer of the panel that is
in turn using the NT36672A panel driver. This compatible string
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH RFC 03/10] dt-bindings: display: panel: novatek,nt36672a: add Tianma FHD+ video mode variant
2026-09-11 8:51 ` [PATCH RFC 03/10] dt-bindings: display: panel: novatek,nt36672a: add Tianma FHD+ video mode variant YİĞİTCAN KAVAKLI via B4 Relay
@ 2026-09-11 8:53 ` sashiko-bot
0 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-09-11 8:53 UTC (permalink / raw)
To: YİĞİTCAN KAVAKLI
Cc: dmitry.torokhov, devicetree, lee, conor+dt, linux-leds,
linux-input, dri-devel, robh
> From: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
>
> Add compatible string "tianma,fhd-video-ginkgo" for the 1080x2340 Tianma
> video mode panel found on the Xiaomi Redmi Note 8 (ginkgo / SM6125).
>
> The downstream vendor DT identifies the panel as "nt36672a video mode
> dsi tianma panel" without documenting a glass model number. Following
> the established convention of "tianma,fhd-video" for Poco F1, this
> variant is added under the existing NT36672A binding schema.
>
> Signed-off-by: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-ginkgo-submission-final-v1-0-0e68e63a18d4@gmail.com?part=3
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH RFC 04/10] drm/panel: novatek-nt36672a: add Tianma FHD+ video mode panel
2026-09-11 8:50 [PATCH RFC 00/10] arm64: qcom: sm6125-ginkgo: display, backlight, and touch support YİĞİTCAN KAVAKLI via B4 Relay
` (2 preceding siblings ...)
2026-09-11 8:51 ` [PATCH RFC 03/10] dt-bindings: display: panel: novatek,nt36672a: add Tianma FHD+ video mode variant YİĞİTCAN KAVAKLI via B4 Relay
@ 2026-09-11 8:51 ` YİĞİTCAN KAVAKLI via B4 Relay
2026-09-11 9:09 ` sashiko-bot
2026-09-11 8:51 ` [PATCH RFC 05/10] dt-bindings: input: touchscreen: add Novatek NT36672A SPI touchscreen YİĞİTCAN KAVAKLI via B4 Relay
` (5 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: YİĞİTCAN KAVAKLI via B4 Relay @ 2026-09-11 8:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Liam Girdwood,
Mark Brown, Sumit Semwal, Neil Armstrong, Jessica Zhang,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Dmitry Torokhov, Henrik Rydberg, Rob Clark,
Dmitry Baryshkov, Abhinav Kumar, Sean Paul, Marijn Suijten,
Bjorn Andersson, Konrad Dybcio
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev,
linux-input, linux-arm-msm, freedreno,
YİĞİTCAN KAVAKLI
From: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
Add support for the 1080x2340 Tianma video mode panel with Novatek
NT36672A driver IC found in the Xiaomi Redmi Note 8 (ginkgo / SM6125).
Changes:
- Add 1080x2340 display mode timings (183.025 MHz pixel clock)
- Add verified DCS initialization sequences for pages 0x25, 0x20, 0x24,
0x26, and user command page 0x10
- Add standard DRM panel .enable and .disable callbacks to coordinate
display on/off commands with active DSI video stream
- Set prepare_prev_first = true for host controller synchronization
- Optimize power-on reset sequence to nominal 10ms timings
Signed-off-by: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
---
drivers/gpu/drm/panel/panel-novatek-nt36672a.c | 141 ++++++++++++++++++++-----
1 file changed, 117 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
index 464d9ce47d87dc857c3a565a63156ac79ddeb30c..b69213790441f204016895a391b58f676762b1aa 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
@@ -34,7 +34,7 @@ static const char * const nt36672a_regulator_names[] = {
"vddneg",
};
-static unsigned long const nt36672a_regulator_enable_loads[] = {
+static const unsigned long nt36672a_regulator_enable_loads[] = {
62000,
100000,
100000
@@ -92,18 +92,12 @@ static int nt36672a_panel_unprepare(struct drm_panel *panel)
if (pinfo->desc->send_deinit_cmds)
pinfo->desc->send_deinit_cmds(&dsi_ctx);
- /* Reset error to continue with display off even if send_cmds failed */
+ /* Reset error to continue power-down even if send_cmds failed */
dsi_ctx.accum_err = 0;
- mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
- /* Reset error to continue power-down even if display off failed */
- dsi_ctx.accum_err = 0;
-
- /* 120ms delay required here as per DCS spec */
- msleep(120);
mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
- /* 0x3C = 60ms delay */
+ /* 60ms delay as per DCS spec */
msleep(60);
nt36672a_panel_power_off(panel);
@@ -120,15 +114,14 @@ static int nt36672a_panel_power_on(struct nt36672a_panel *pinfo)
return ret;
/*
- * As per downstream kernel, Reset sequence of Tianma FHD panel requires the panel to
- * be out of reset for 10ms, followed by being held in reset for 10ms. But for Android
- * AOSP, we needed to bump it upto 200ms otherwise we get white screen sometimes.
- * FIXME: Try to reduce this 200ms to a lesser value.
+ * Reset sequence: 10ms out of reset, 10ms held in reset, 10ms out of reset.
*/
- gpiod_set_value(pinfo->reset_gpio, 1);
- msleep(200);
- gpiod_set_value(pinfo->reset_gpio, 0);
- msleep(200);
+ gpiod_set_value_cansleep(pinfo->reset_gpio, 0);
+ usleep_range(10000, 11000);
+ gpiod_set_value_cansleep(pinfo->reset_gpio, 1);
+ usleep_range(10000, 11000);
+ gpiod_set_value_cansleep(pinfo->reset_gpio, 0);
+ usleep_range(10000, 11000);
return 0;
}
@@ -146,19 +139,41 @@ static int nt36672a_panel_prepare(struct drm_panel *panel)
mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
- /* 0x46 = 70 ms delay */
- mipi_dsi_msleep(&dsi_ctx, 70);
-
- mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
+ /* 80 ms delay as per DCS spec / downstream timing */
+ msleep(80);
/* Send rest of the init cmds */
if (pinfo->desc->send_init_cmds_2)
pinfo->desc->send_init_cmds_2(&dsi_ctx);
- mipi_dsi_msleep(&dsi_ctx, 120);
-
if (dsi_ctx.accum_err < 0)
- gpiod_set_value(pinfo->reset_gpio, 0);
+ gpiod_set_value_cansleep(pinfo->reset_gpio, 0);
+
+ return dsi_ctx.accum_err;
+}
+
+static int nt36672a_panel_enable(struct drm_panel *panel)
+{
+ struct nt36672a_panel *pinfo = to_nt36672a_panel(panel);
+ struct mipi_dsi_multi_context dsi_ctx = { .dsi = pinfo->link };
+
+ mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
+
+ /* Frame settling delay */
+ msleep(20);
+
+ return dsi_ctx.accum_err;
+}
+
+static int nt36672a_panel_disable(struct drm_panel *panel)
+{
+ struct nt36672a_panel *pinfo = to_nt36672a_panel(panel);
+ struct mipi_dsi_multi_context dsi_ctx = { .dsi = pinfo->link };
+
+ mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
+
+ /* 120ms delay as per DCS spec */
+ msleep(120);
return dsi_ctx.accum_err;
}
@@ -189,6 +204,8 @@ static int nt36672a_panel_get_modes(struct drm_panel *panel,
static const struct drm_panel_funcs panel_funcs = {
.unprepare = nt36672a_panel_unprepare,
.prepare = nt36672a_panel_prepare,
+ .enable = nt36672a_panel_enable,
+ .disable = nt36672a_panel_disable,
.get_modes = nt36672a_panel_get_modes,
};
@@ -404,6 +421,79 @@ static const struct nt36672a_panel_desc tianma_fhd_video_panel_desc = {
.send_deinit_cmds = tianma_fhd_video_send_deinit_cmds,
};
+static void tianma_ginkgo_send_init_cmds(struct mipi_dsi_multi_context *dsi_ctx)
+{
+ /* Page 0x25 init */
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0xff, 0x25);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0xfb, 0x01);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x18, 0x96);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x05, 0x04);
+
+ /* Page 0x20 init */
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0xff, 0x20);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0xfb, 0x01);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x78, 0x01);
+
+ /* Page 0x24 init */
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0xff, 0x24);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0xfb, 0x01);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x82, 0x13);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x84, 0x31);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x88, 0x13);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x8a, 0x31);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x8e, 0xe4);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x8f, 0x01);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x90, 0x80);
+
+ /* Page 0x26 init */
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0xff, 0x26);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0xfb, 0x01);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0xa9, 0x12);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0xaa, 0x10);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0xae, 0x8a);
+
+ /* Page 0x10 (User command page) */
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0xff, 0x10);
+}
+
+static void tianma_ginkgo_send_post_on_cmds(struct mipi_dsi_multi_context *dsi_ctx)
+{
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0xb0, 0x01);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x35, 0x00);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x68, 0x04, 0x03);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x51, 0xb8);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x53, 0x2c);
+ mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x55, 0x00);
+}
+
+static const struct drm_display_mode tianma_ginkgo_fhd_video_mode = {
+ .clock = 183025,
+
+ .hdisplay = 1080,
+ .hsync_start = 1080 + 90,
+ .hsync_end = 1080 + 90 + 2,
+ .htotal = 1080 + 90 + 2 + 120,
+
+ .vdisplay = 2340,
+ .vsync_start = 2340 + 10,
+ .vsync_end = 2340 + 10 + 3,
+ .vtotal = 2340 + 10 + 3 + 8,
+
+ .type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
+};
+
+static const struct nt36672a_panel_desc tianma_ginkgo_fhd_video_panel_desc = {
+ .display_mode = &tianma_ginkgo_fhd_video_mode,
+ .panel_name = "tianma-ginkgo-fhd-video",
+ .width_mm = 67,
+ .height_mm = 145,
+ .mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_LPM,
+ .format = MIPI_DSI_FMT_RGB888,
+ .lanes = 4,
+ .send_init_cmds_1 = tianma_ginkgo_send_init_cmds,
+ .send_init_cmds_2 = tianma_ginkgo_send_post_on_cmds,
+};
+
static int nt36672a_panel_add(struct nt36672a_panel *pinfo)
{
struct device *dev = &pinfo->link->dev;
@@ -428,6 +518,8 @@ static int nt36672a_panel_add(struct nt36672a_panel *pinfo)
if (ret)
return dev_err_probe(dev, ret, "Failed to get backlight\n");
+ pinfo->base.prepare_prev_first = true;
+
drm_panel_add(&pinfo->base);
return 0;
@@ -481,6 +573,7 @@ static void nt36672a_panel_remove(struct mipi_dsi_device *dsi)
static const struct of_device_id tianma_fhd_video_of_match[] = {
{ .compatible = "tianma,fhd-video", .data = &tianma_fhd_video_panel_desc },
+ { .compatible = "tianma,fhd-video-ginkgo", .data = &tianma_ginkgo_fhd_video_panel_desc },
{ },
};
MODULE_DEVICE_TABLE(of, tianma_fhd_video_of_match);
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH RFC 04/10] drm/panel: novatek-nt36672a: add Tianma FHD+ video mode panel
2026-09-11 8:51 ` [PATCH RFC 04/10] drm/panel: novatek-nt36672a: add Tianma FHD+ video mode panel YİĞİTCAN KAVAKLI via B4 Relay
@ 2026-09-11 9:09 ` sashiko-bot
0 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-09-11 9:09 UTC (permalink / raw)
To: YİĞİTCAN KAVAKLI
Cc: dmitry.torokhov, linux-leds, linux-input, conor+dt, robh,
dri-devel, devicetree, lee
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Hardware initialization and teardown sequence reversal
- [Low] Unconditional delay on error path due to bypassing `mipi_dsi_msleep()`
--
commit 3b4bbeb999e6951e81e05057ded69945b990ecb4
Author: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
drm/panel: novatek-nt36672a: add Tianma FHD+ video mode panel
This commit adds support for the 1080x2340 Tianma video mode panel with
Novatek NT36672A driver IC. It adds display mode timings, DCS initialization
sequences, and standard DRM panel enable and disable callbacks to coordinate
display commands with the active DSI video stream.
> diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
> index 464d9ce47d87d..b69213790441f 100644
> --- a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
> +++ b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
> @@ -92,18 +92,12 @@ static int nt36672a_panel_unprepare(struct drm_panel *panel)
> struct mipi_dsi_multi_context dsi_ctx = { .dsi = pinfo->link };
>
> /* send off cmds */
> if (pinfo->desc->send_deinit_cmds)
> pinfo->desc->send_deinit_cmds(&dsi_ctx);
[Severity: High]
Does this sequence execute deinitialization commands after the panel is already
turned off? The new disable callback will turn off the display before the
unprepare callback runs, meaning the panel might not be able to process these
deinitialization commands correctly.
[ ... ]
> @@ -146,19 +139,41 @@ static int nt36672a_panel_prepare(struct drm_panel *panel)
>
> mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
>
> - /* 0x46 = 70 ms delay */
> - mipi_dsi_msleep(&dsi_ctx, 70);
> -
> - mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> + /* 80 ms delay as per DCS spec / downstream timing */
> + msleep(80);
[Severity: Low]
Does this unconditional msleep() cause unnecessary delays on error paths?
Because this replaces mipi_dsi_msleep(), if the preceding DCS commands
fail, dsi_ctx.accum_err is set, but this code will still unconditionally
sleep for 80ms.
>
> /* Send rest of the init cmds */
> if (pinfo->desc->send_init_cmds_2)
> pinfo->desc->send_init_cmds_2(&dsi_ctx);
[Severity: High]
Will the panel properly handle these secondary initialization commands while
the display is still off? Since this patch moves the set display on command
into the enable callback, these post-on brightness settings are being sent
before the display is turned on, which might cause initialization failures or
visual artifacts.
[ ... ]
> +static int nt36672a_panel_enable(struct drm_panel *panel)
> +{
> + struct nt36672a_panel *pinfo = to_nt36672a_panel(panel);
> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = pinfo->link };
> +
> + mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> +
> + /* Frame settling delay */
> + msleep(20);
[Severity: Low]
Does this msleep() also need to check for preceding errors? This creates an
unconditional sleep that ignores potential errors from the preceding
mipi_dsi_dcs_set_display_on_multi() call.
> +
> + return dsi_ctx.accum_err;
> +}
> +
> +static int nt36672a_panel_disable(struct drm_panel *panel)
> +{
> + struct nt36672a_panel *pinfo = to_nt36672a_panel(panel);
> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = pinfo->link };
> +
> + mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> +
> + /* 120ms delay as per DCS spec */
> + msleep(120);
[Severity: Low]
Similar to the enable path, is this unconditional sleep intended? It ignores
potential errors from the preceding mipi_dsi_dcs_set_display_off_multi() call,
causing unnecessary delays during teardown failures.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-ginkgo-submission-final-v1-0-0e68e63a18d4@gmail.com?part=4
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH RFC 05/10] dt-bindings: input: touchscreen: add Novatek NT36672A SPI touchscreen
2026-09-11 8:50 [PATCH RFC 00/10] arm64: qcom: sm6125-ginkgo: display, backlight, and touch support YİĞİTCAN KAVAKLI via B4 Relay
` (3 preceding siblings ...)
2026-09-11 8:51 ` [PATCH RFC 04/10] drm/panel: novatek-nt36672a: add Tianma FHD+ video mode panel YİĞİTCAN KAVAKLI via B4 Relay
@ 2026-09-11 8:51 ` YİĞİTCAN KAVAKLI via B4 Relay
2026-09-11 8:58 ` sashiko-bot
2026-09-11 8:51 ` [PATCH RFC 06/10] Input: novatek-nt36672a-spi: add driver for " YİĞİTCAN KAVAKLI via B4 Relay
` (4 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: YİĞİTCAN KAVAKLI via B4 Relay @ 2026-09-11 8:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Liam Girdwood,
Mark Brown, Sumit Semwal, Neil Armstrong, Jessica Zhang,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Dmitry Torokhov, Henrik Rydberg, Rob Clark,
Dmitry Baryshkov, Abhinav Kumar, Sean Paul, Marijn Suijten,
Bjorn Andersson, Konrad Dybcio
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev,
linux-input, linux-arm-msm, freedreno,
YİĞİTCAN KAVAKLI
From: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
Add devicetree binding documentation for the Novatek NT36672A SPI
touchscreen controller.
The binding supports standard SPI peripheral properties, interrupt
configuration, the optional 'panel' phandle for DRM panel follower
synchronization, and an optional 'firmware-name' property.
Signed-off-by: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
---
.../input/touchscreen/novatek,nt36672a-spi.yaml | 66 ++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/novatek,nt36672a-spi.yaml b/Documentation/devicetree/bindings/input/touchscreen/novatek,nt36672a-spi.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..0d89d60f2a93d26e2f7903371ecc6eb9ed683b33
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/novatek,nt36672a-spi.yaml
@@ -0,0 +1,66 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/novatek,nt36672a-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Novatek NT36672A SPI Touchscreen Controller
+
+maintainers:
+ - Dmitry Torokhov <dmitry.torokhov@gmail.com>
+
+description:
+ The Novatek NT36672A TDDI controller is connected over SPI for touchscreen
+ interaction, with host-downloaded firmware into controller SRAM. It integrates
+ with the display panel lifecycle using the DRM panel follower API.
+
+allOf:
+ - $ref: touchscreen.yaml#
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+ compatible:
+ enum:
+ - novatek,nt36672a-spi
+ - novatek,nt36672a-ts
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ spi-max-frequency: true
+
+ panel:
+ description: phandle of the display panel that this touchscreen follows.
+ $ref: /schemas/types.yaml#/definitions/phandle
+
+ firmware-name:
+ description: Name of the firmware file to load into controller SRAM.
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ touchscreen@0 {
+ compatible = "novatek,nt36672a-spi";
+ reg = <0>;
+ spi-max-frequency = <4000000>;
+ interrupts = <88 IRQ_TYPE_EDGE_RISING>;
+ panel = <&panel>;
+ touchscreen-size-x = <1080>;
+ touchscreen-size-y = <2340>;
+ };
+ };
+...
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH RFC 05/10] dt-bindings: input: touchscreen: add Novatek NT36672A SPI touchscreen
2026-09-11 8:51 ` [PATCH RFC 05/10] dt-bindings: input: touchscreen: add Novatek NT36672A SPI touchscreen YİĞİTCAN KAVAKLI via B4 Relay
@ 2026-09-11 8:58 ` sashiko-bot
0 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-09-11 8:58 UTC (permalink / raw)
To: YİĞİTCAN KAVAKLI
Cc: dri-devel, robh, dmitry.torokhov, linux-input, devicetree,
linux-leds, lee, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Device tree schema conflict for the 'novatek,nt36672a-ts' compatible string causes `dtbs_check` validation failures.
--
commit d198703693d9ad88fe332afc5287a4b5bb64a630
Author: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
dt-bindings: input: touchscreen: add Novatek NT36672A SPI touchscreen
Add devicetree binding documentation for the Novatek NT36672A SPI
touchscreen controller.
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/novatek,nt36672a-spi.yaml b/Documentation/devicetree/bindings/input/touchscreen/novatek,nt36672a-spi.yaml
> index 0000000000000..0d89d60f2a93d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/novatek,nt36672a-spi.yaml
[ ... ]
> +properties:
> + compatible:
> + enum:
> + - novatek,nt36672a-spi
> + - novatek,nt36672a-ts
[Severity: Low]
Does adding "novatek,nt36672a-ts" here conflict with the existing I2C
binding in novatek,nvt-ts.yaml?
Since "novatek,nt36672a-ts" is already bound to the I2C schema, and both
schemas define unevaluatedProperties: false without a custom select rule
based on the parent bus, nodes with this compatible string will match both
schemas simultaneously.
Will this cause dtbs_check validation to fail for existing I2C touchscreens
(which lack SPI properties like spi-max-frequency) and new SPI touchscreens
(which lack I2C properties like vcc-supply)?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-ginkgo-submission-final-v1-0-0e68e63a18d4@gmail.com?part=5
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH RFC 06/10] Input: novatek-nt36672a-spi: add driver for Novatek NT36672A SPI touchscreen
2026-09-11 8:50 [PATCH RFC 00/10] arm64: qcom: sm6125-ginkgo: display, backlight, and touch support YİĞİTCAN KAVAKLI via B4 Relay
` (4 preceding siblings ...)
2026-09-11 8:51 ` [PATCH RFC 05/10] dt-bindings: input: touchscreen: add Novatek NT36672A SPI touchscreen YİĞİTCAN KAVAKLI via B4 Relay
@ 2026-09-11 8:51 ` YİĞİTCAN KAVAKLI via B4 Relay
2026-09-11 9:02 ` sashiko-bot
2026-09-11 8:51 ` [PATCH RFC 07/10] drm/msm/dpu: describe SM6125 programmable-fetch delayed-start quirk YİĞİTCAN KAVAKLI via B4 Relay
` (3 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: YİĞİTCAN KAVAKLI via B4 Relay @ 2026-09-11 8:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Liam Girdwood,
Mark Brown, Sumit Semwal, Neil Armstrong, Jessica Zhang,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Dmitry Torokhov, Henrik Rydberg, Rob Clark,
Dmitry Baryshkov, Abhinav Kumar, Sean Paul, Marijn Suijten,
Bjorn Andersson, Konrad Dybcio
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev,
linux-input, linux-arm-msm, freedreno,
YİĞİTCAN KAVAKLI
From: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
Add support for the Novatek NT36672A TDDI touchscreen controller over SPI.
The NT36672A is connected over SPI (QUPv3 SE2) on SM6125 and requires
dynamic host-download firmware loaded into controller volatile SRAM.
A dedicated SPI driver is introduced (Option B) to avoid breaking or
complicating the existing I2C-only driver (novatek-nvt-ts.c).
Features:
- Dynamic firmware host-download into SRAM via request_firmware()
- Multi-partition binary parsing with hardware CRC verification
- Threaded interrupt handler supporting Linux MT Protocol Type B
(up to 10 fingers)
- Standard touchscreen_properties coordinate reporting
- DRM panel follower integration via devm_drm_panel_add_follower()
to follow panel sleep/wake transitions cleanly
- Pure interrupt-driven architecture with zero polling loops
Signed-off-by: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
---
drivers/input/touchscreen/Kconfig | 11 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/novatek-nt36672a-spi.c | 674 +++++++++++++++++++++++
3 files changed, 686 insertions(+)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 9b9ae8ac3f7fd3b9e948083fcd05afabdcd69bca..fb9a34d3f983084af75e85aad2d9740d066e85fb 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -701,6 +701,17 @@ config TOUCHSCREEN_NOVATEK_NVT_TS
To compile this driver as a module, choose M here: the
module will be called novatek-nvt-ts.
+config TOUCHSCREEN_NOVATEK_NT36672A_SPI
+ tristate "Novatek NT36672A SPI touchscreen support"
+ depends on SPI_MASTER
+ select FW_LOADER
+ help
+ Say Y here if you have a Novatek NT36672A touchscreen
+ connected over SPI.
+
+ To compile this driver as a module, choose M here: the
+ module will be called novatek-nt36672a-spi.
+
config TOUCHSCREEN_IMAGIS
tristate "Imagis touchscreen support"
depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index bfd9de83389d8833dbe7cc2641cb24493cdf3807..aca74a0afd141356c7b5d3d3393a882e0530be17 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -69,6 +69,7 @@ obj-$(CONFIG_TOUCHSCREEN_MMS114) += mms114.o
obj-$(CONFIG_TOUCHSCREEN_MSG2638) += msg2638.o
obj-$(CONFIG_TOUCHSCREEN_MTOUCH) += mtouch.o
obj-$(CONFIG_TOUCHSCREEN_NOVATEK_NVT_TS) += novatek-nvt-ts.o
+obj-$(CONFIG_TOUCHSCREEN_NOVATEK_NT36672A_SPI) += novatek-nt36672a-spi.o
obj-$(CONFIG_TOUCHSCREEN_HP600) += hp680_ts_input.o
obj-$(CONFIG_TOUCHSCREEN_HP7XX) += jornada720_ts.o
obj-$(CONFIG_TOUCHSCREEN_IPAQ_MICRO) += ipaq-micro-ts.o
diff --git a/drivers/input/touchscreen/novatek-nt36672a-spi.c b/drivers/input/touchscreen/novatek-nt36672a-spi.c
new file mode 100644
index 0000000000000000000000000000000000000000..03d2042b0bb30d3420379497fd822153bc90171e
--- /dev/null
+++ b/drivers/input/touchscreen/novatek-nt36672a-spi.c
@@ -0,0 +1,674 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Novatek NT36672A SPI Touchscreen Driver
+ *
+ * Copyright (c) 2026
+ *
+ * Hardware:
+ * Novatek NT36672A TDDI controller connected over SPI.
+ * Firmware is loaded dynamically into controller SRAM via request_firmware().
+ * Panel lifecycle synchronization uses the DRM panel follower API.
+ */
+
+#include <linux/delay.h>
+#include <linux/firmware.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/input/mt.h>
+#include <linux/input/touchscreen.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/pm.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <linux/workqueue.h>
+
+#include <drm/drm_panel.h>
+
+#define NVT_MAX_TOUCHES 10
+#define NVT_POINT_DATA_LEN 65
+#define NVT_TRANSFER_LEN 1024
+#define NVT_DEFAULT_FW_NAME "novatek/nt36672a_tianma.bin"
+
+/* NT36672A Memory Map & Registers */
+#define EVENT_BUF_ADDR 0x21c00
+#define EVENT_MAP_HOST_CMD 0x50
+#define EVENT_MAP_RESET_COMPLETE 0x60
+#define EVENT_MAP_FWINFO 0x78
+
+#define BOOT_RDY_ADDR 0x3f10d
+#define BLD_CRC_EN_ADDR 0x3f30e
+#define ILM_DES_ADDR 0x3f128
+#define ILM_LENGTH_ADDR 0x3f118
+#define G_ILM_CHECKSUM_ADDR 0x3f100
+#define DLM_DES_ADDR 0x3f12c
+#define DLM_LENGTH_ADDR 0x3f130
+#define G_DLM_CHECKSUM_ADDR 0x3f104
+#define SPI_RD_FAST_ADDR 0x03f310
+
+struct nvt_ts_bin_map {
+ u32 bin_addr;
+ u32 sram_addr;
+ u32 size;
+ u32 crc;
+};
+
+struct novatek_ts_data {
+ struct spi_device *spi;
+ struct input_dev *input_dev;
+ struct touchscreen_properties prop;
+ /* Protects SPI bus access, firmware loading and controller state */
+ struct mutex lock;
+ struct drm_panel_follower panel_follower;
+ struct workqueue_struct *fw_wq;
+ struct delayed_work fw_reload_work;
+ const char *fw_name;
+
+ u16 abs_x_max;
+ u16 abs_y_max;
+ u8 fw_ver;
+ u8 fw_reset_state;
+ bool fw_running;
+ bool slot_active[NVT_MAX_TOUCHES];
+};
+
+static inline u32 byte_to_word(const u8 *b)
+{
+ return b[0] | (b[1] << 8) | (b[2] << 16) | (b[3] << 24);
+}
+
+static int nvt_spi_write(struct spi_device *spi, const u8 *buf, size_t len)
+{
+ struct spi_transfer t = {0};
+ struct spi_message m;
+ u8 *xbuf;
+ int ret;
+
+ xbuf = kmemdup(buf, len, GFP_KERNEL);
+ if (!xbuf)
+ return -ENOMEM;
+
+ xbuf[0] |= 0x80; /* SPI_WRITE_MASK */
+
+ t.tx_buf = xbuf;
+ t.len = len;
+
+ spi_message_init(&m);
+ spi_message_add_tail(&t, &m);
+ ret = spi_sync(spi, &m);
+ kfree(xbuf);
+
+ return ret;
+}
+
+static int nvt_spi_read(struct spi_device *spi, u8 *buf, size_t len)
+{
+ struct spi_transfer t = {0};
+ struct spi_message m;
+ u8 *xbuf, *rbuf;
+ int ret;
+
+ xbuf = kzalloc(len + 1, GFP_KERNEL);
+ rbuf = kzalloc(len + 1, GFP_KERNEL);
+ if (!xbuf || !rbuf) {
+ kfree(xbuf);
+ kfree(rbuf);
+ return -ENOMEM;
+ }
+
+ xbuf[0] = buf[0] & 0x7f; /* SPI_READ_MASK */
+
+ t.tx_buf = xbuf;
+ t.rx_buf = rbuf;
+ t.len = len + 1; /* 1 dummy byte */
+
+ spi_message_init(&m);
+ spi_message_add_tail(&t, &m);
+ ret = spi_sync(spi, &m);
+ if (!ret)
+ memcpy(buf + 1, rbuf + 2, len - 1);
+
+ kfree(xbuf);
+ kfree(rbuf);
+
+ return ret;
+}
+
+static int nvt_set_page(struct spi_device *spi, u32 addr)
+{
+ u8 buf[3];
+
+ buf[0] = 0xff; /* page command */
+ buf[1] = (addr >> 15) & 0xff;
+ buf[2] = (addr >> 7) & 0xff;
+
+ return nvt_spi_write(spi, buf, sizeof(buf));
+}
+
+static int nvt_write_addr(struct spi_device *spi, u32 addr, u8 data)
+{
+ u8 buf[2];
+ int ret;
+
+ ret = nvt_set_page(spi, addr);
+ if (ret)
+ return ret;
+
+ buf[0] = addr & 0x7f;
+ buf[1] = data;
+
+ return nvt_spi_write(spi, buf, sizeof(buf));
+}
+
+static void nvt_bootloader_reset(struct spi_device *spi)
+{
+ /*
+ * Configure SPI Fast Read to 0x00 so bootloader SRAM writes succeed.
+ * Do NOT write 0x69 to SWRST_N8_ADDR because NT36672A is a TDDI chip:
+ * writing 0x69 resets the entire IC (display timing generator, DCS registers, PWM)
+ * and causes the physical LCD to go completely black.
+ * The controller is already reset into ROM bootloader mode by hardware panel reset.
+ */
+ nvt_write_addr(spi, SPI_RD_FAST_ADDR, 0x00);
+}
+
+static void nvt_set_bld_crc_bank(struct spi_device *spi, u32 des_addr, u32 sram_addr,
+ u32 len_addr, u32 size, u32 g_chk_addr, u32 crc)
+{
+ u8 buf[5];
+
+ /* Destination address */
+ nvt_set_page(spi, des_addr);
+ buf[0] = des_addr & 0x7f;
+ buf[1] = sram_addr & 0xff;
+ buf[2] = (sram_addr >> 8) & 0xff;
+ buf[3] = (sram_addr >> 16) & 0xff;
+ nvt_spi_write(spi, buf, 4);
+
+ /* Length (for NT36672A hw_crc=1, 3 bytes) */
+ buf[0] = len_addr & 0x7f;
+ buf[1] = size & 0xff;
+ buf[2] = (size >> 8) & 0xff;
+ nvt_spi_write(spi, buf, 3);
+
+ /* Golden Checksum (4 bytes) */
+ buf[0] = g_chk_addr & 0x7f;
+ buf[1] = crc & 0xff;
+ buf[2] = (crc >> 8) & 0xff;
+ buf[3] = (crc >> 16) & 0xff;
+ buf[4] = (crc >> 24) & 0xff;
+ nvt_spi_write(spi, buf, 5);
+}
+
+static int nvt_write_sram(struct spi_device *spi, const u8 *fwdata,
+ u32 sram_addr, u32 size, u32 bin_addr)
+{
+ u32 count, i, len;
+ int ret = 0;
+ u8 *fwbuf;
+
+ fwbuf = kmalloc(NVT_TRANSFER_LEN + 2, GFP_KERNEL);
+ if (!fwbuf)
+ return -ENOMEM;
+
+ count = (size + NVT_TRANSFER_LEN - 1) / NVT_TRANSFER_LEN;
+
+ for (i = 0; i < count; i++) {
+ len = (size < NVT_TRANSFER_LEN) ? size : NVT_TRANSFER_LEN;
+
+ ret = nvt_set_page(spi, sram_addr);
+ if (ret)
+ goto out;
+
+ fwbuf[0] = sram_addr & 0x7f;
+ memcpy(fwbuf + 1, &fwdata[bin_addr], len);
+ ret = nvt_spi_write(spi, fwbuf, len + 1);
+ if (ret)
+ goto out;
+
+ sram_addr += NVT_TRANSFER_LEN;
+ bin_addr += NVT_TRANSFER_LEN;
+ size -= len;
+ }
+
+out:
+ kfree(fwbuf);
+ return ret;
+}
+
+static int nvt_parse_and_download(struct novatek_ts_data *ts, const u8 *fwdata, size_t fwsize)
+{
+ struct spi_device *spi = ts->spi;
+ struct nvt_ts_bin_map bin_map[16];
+ u8 info_sec_num = 0;
+ u8 ovly_sec_num = 0;
+ u32 partition, list;
+ u32 header_end, pos;
+ u8 buf[18];
+ int ret, retry;
+
+ if (fwsize < 0x40)
+ return -EINVAL;
+
+ header_end = byte_to_word(&fwdata[0]);
+ pos = 0x30;
+ while (pos < header_end && info_sec_num < 14) {
+ info_sec_num++;
+ pos += 0x10;
+ }
+
+ ovly_sec_num = (fwdata[0x28] & 0x10) ? (fwdata[0x28] & 0x0f) : 0;
+ partition = 2 + ovly_sec_num + info_sec_num;
+ if (partition > 16)
+ partition = 16;
+
+ for (list = 0; list < partition; list++) {
+ if (list < 2) {
+ bin_map[list].bin_addr = byte_to_word(&fwdata[0 + list * 12]);
+ bin_map[list].sram_addr = byte_to_word(&fwdata[4 + list * 12]);
+ bin_map[list].size = byte_to_word(&fwdata[8 + list * 12]);
+ bin_map[list].crc = byte_to_word(&fwdata[0x18 + list * 4]);
+ } else {
+ pos = 0x30 + (0x10 * (list - 2));
+ bin_map[list].sram_addr = byte_to_word(&fwdata[pos]);
+ bin_map[list].size = byte_to_word(&fwdata[pos + 4]);
+ bin_map[list].bin_addr = byte_to_word(&fwdata[pos + 8]);
+ bin_map[list].crc = byte_to_word(&fwdata[pos + 12]);
+ }
+ }
+
+ nvt_bootloader_reset(spi);
+
+ for (list = 0; list < partition; list++) {
+ u32 size = bin_map[list].size;
+
+ if (!size)
+ continue;
+ size += 1;
+
+ ret = nvt_write_sram(spi, fwdata, bin_map[list].sram_addr, size,
+ bin_map[list].bin_addr);
+ if (ret) {
+ dev_err(&spi->dev, "SRAM write partition %u failed: %d\n", list, ret);
+ return ret;
+ }
+ }
+
+ nvt_set_bld_crc_bank(spi, ILM_DES_ADDR, bin_map[0].sram_addr,
+ ILM_LENGTH_ADDR, bin_map[0].size,
+ G_ILM_CHECKSUM_ADDR, bin_map[0].crc);
+ nvt_set_bld_crc_bank(spi, DLM_DES_ADDR, bin_map[1].sram_addr,
+ DLM_LENGTH_ADDR, bin_map[1].size,
+ G_DLM_CHECKSUM_ADDR, bin_map[1].crc);
+
+ /* Enable BLD CRC (bit 7 of 0x3F30E) */
+ nvt_set_page(spi, BLD_CRC_EN_ADDR);
+ buf[0] = BLD_CRC_EN_ADDR & 0x7f;
+ buf[1] = 0xff;
+ nvt_spi_read(spi, buf, 2);
+ buf[0] = BLD_CRC_EN_ADDR & 0x7f;
+ buf[1] = buf[1] | 0x80;
+ nvt_spi_write(spi, buf, 2);
+
+ /* Clear reset status and enable firmware CRC */
+ nvt_set_page(spi, EVENT_BUF_ADDR);
+ buf[0] = EVENT_MAP_RESET_COMPLETE & 0x7f;
+ buf[1] = 0x00;
+ nvt_spi_write(spi, buf, 2);
+
+ buf[0] = EVENT_MAP_HOST_CMD & 0x7f;
+ buf[1] = 0xae;
+ nvt_spi_write(spi, buf, 2);
+
+ /* Boot Ready */
+ nvt_write_addr(spi, BOOT_RDY_ADDR, 1);
+ msleep(10);
+
+ /* Verify Firmware Reset State (>= 0xA0) */
+ ts->fw_running = false;
+ nvt_set_page(spi, EVENT_BUF_ADDR | EVENT_MAP_RESET_COMPLETE);
+ for (retry = 0; retry < 20; retry++) {
+ buf[0] = EVENT_MAP_RESET_COMPLETE;
+ buf[1] = 0x00;
+ ret = nvt_spi_read(spi, buf, 6);
+ if (ret == 0 && buf[1] >= 0xa0 && buf[1] <= 0xaf) {
+ ts->fw_reset_state = buf[1];
+ ts->fw_running = true;
+ break;
+ }
+ msleep(10);
+ }
+
+ /* Wait for baseline calibration */
+ for (retry = 0; retry < 50; retry++) {
+ buf[0] = EVENT_MAP_RESET_COMPLETE;
+ buf[1] = 0x00;
+ ret = nvt_spi_read(spi, buf, 6);
+ if (ret == 0 && buf[1] >= 0xa1 && buf[1] <= 0xaf) {
+ ts->fw_reset_state = buf[1];
+ break;
+ }
+ msleep(10);
+ }
+
+ if (!ts->fw_running) {
+ dev_err(&spi->dev, "firmware boot timeout: state=0x%02x\n", buf[1]);
+ return -EIO;
+ }
+
+ /* Read FW Information */
+ nvt_set_page(spi, EVENT_BUF_ADDR | EVENT_MAP_FWINFO);
+ buf[0] = EVENT_MAP_FWINFO;
+ ret = nvt_spi_read(spi, buf, 17);
+ if (ret == 0) {
+ ts->fw_ver = buf[1];
+ ts->abs_x_max = (u16)((buf[5] << 8) | buf[6]);
+ ts->abs_y_max = (u16)((buf[7] << 8) | buf[8]);
+ }
+
+ if (!ts->abs_x_max || ts->abs_x_max > 4096)
+ ts->abs_x_max = 1080;
+ if (!ts->abs_y_max || ts->abs_y_max > 4096)
+ ts->abs_y_max = 2340;
+
+ nvt_set_page(spi, EVENT_BUF_ADDR);
+
+ return 0;
+}
+
+static int novatek_ts_load_firmware(struct novatek_ts_data *ts)
+{
+ const struct firmware *fw;
+ int ret;
+
+ ret = request_firmware(&fw, ts->fw_name, &ts->spi->dev);
+ if (ret) {
+ dev_err(&ts->spi->dev, "failed to request firmware '%s': %d\n",
+ ts->fw_name, ret);
+ return ret;
+ }
+
+ ret = nvt_parse_and_download(ts, fw->data, fw->size);
+ release_firmware(fw);
+
+ return ret;
+}
+
+static void novatek_ts_fw_work(struct work_struct *work)
+{
+ struct delayed_work *dwork = to_delayed_work(work);
+ struct novatek_ts_data *ts = container_of(dwork, struct novatek_ts_data, fw_reload_work);
+ u8 rst_chk[6] = {EVENT_MAP_RESET_COMPLETE, 0};
+ int ret;
+
+ mutex_lock(&ts->lock);
+
+ nvt_set_page(ts->spi, EVENT_BUF_ADDR);
+ ret = nvt_spi_read(ts->spi, rst_chk, 2);
+ if (ret == 0 && rst_chk[1] >= 0xa0 && rst_chk[1] <= 0xaf) {
+ /* Controller is already running valid firmware */
+ mutex_unlock(&ts->lock);
+ return;
+ }
+
+ ret = novatek_ts_load_firmware(ts);
+ if (ret)
+ dev_err(&ts->spi->dev, "firmware load failed: %d\n", ret);
+
+ nvt_set_page(ts->spi, EVENT_BUF_ADDR);
+ mutex_unlock(&ts->lock);
+}
+
+static int novatek_ts_panel_prepared(struct drm_panel_follower *follower)
+{
+ struct novatek_ts_data *ts = container_of(follower, struct novatek_ts_data, panel_follower);
+
+ if (ts->fw_wq) {
+ cancel_delayed_work_sync(&ts->fw_reload_work);
+ queue_delayed_work(ts->fw_wq, &ts->fw_reload_work, msecs_to_jiffies(1500));
+ }
+ return 0;
+}
+
+static int novatek_ts_panel_unpreparing(struct drm_panel_follower *follower)
+{
+ struct novatek_ts_data *ts = container_of(follower, struct novatek_ts_data, panel_follower);
+
+ if (ts->fw_wq)
+ cancel_delayed_work_sync(&ts->fw_reload_work);
+ return 0;
+}
+
+static const struct drm_panel_follower_funcs novatek_ts_panel_follower_funcs = {
+ .panel_prepared = novatek_ts_panel_prepared,
+ .panel_unpreparing = novatek_ts_panel_unpreparing,
+};
+
+static irqreturn_t novatek_ts_threaded_irq(int irq, void *dev_id)
+{
+ struct novatek_ts_data *ts = dev_id;
+ u8 point_data[NVT_POINT_DATA_LEN + 2] = {0};
+ bool slot_pressed[NVT_MAX_TOUCHES] = {false};
+ int finger_cnt = 0;
+ int ret, i;
+
+ mutex_lock(&ts->lock);
+
+ nvt_set_page(ts->spi, EVENT_BUF_ADDR);
+
+ point_data[0] = 0x00;
+ ret = nvt_spi_read(ts->spi, point_data, NVT_POINT_DATA_LEN + 1);
+ if (ret < 0) {
+ dev_err_ratelimited(&ts->spi->dev, "failed to read touch points: %d\n", ret);
+ mutex_unlock(&ts->lock);
+ return IRQ_HANDLED;
+ }
+
+ for (i = 0; i < NVT_MAX_TOUCHES; i++) {
+ int pos = 1 + 6 * i;
+ u8 raw0 = point_data[pos];
+ u8 input_id = raw0 >> 3;
+ u8 status = raw0 & 0x07;
+
+ if (input_id < 1 || input_id > NVT_MAX_TOUCHES)
+ continue;
+
+ if (status == 0x01 || status == 0x02) { /* 1: down, 2: move */
+ u32 x = ((u32)point_data[pos + 1] << 4) | ((u32)point_data[pos + 3] >> 4);
+ u32 y = ((u32)point_data[pos + 2] << 4) | ((u32)point_data[pos + 3] & 0x0f);
+ u32 w = point_data[pos + 4] ? point_data[pos + 4] : 1;
+ u32 p = point_data[pos + 5] ? point_data[pos + 5] : 1;
+
+ if (x <= ts->abs_x_max && y <= ts->abs_y_max) {
+ int slot = input_id - 1;
+
+ slot_pressed[slot] = true;
+ finger_cnt++;
+
+ input_mt_slot(ts->input_dev, slot);
+ input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER, true);
+ input_report_abs(ts->input_dev, ABS_MT_POSITION_X, x);
+ input_report_abs(ts->input_dev, ABS_MT_POSITION_Y, y);
+ input_report_abs(ts->input_dev, ABS_MT_TOUCH_MAJOR, w);
+ input_report_abs(ts->input_dev, ABS_MT_PRESSURE, p);
+ }
+ }
+ }
+
+ for (i = 0; i < NVT_MAX_TOUCHES; i++) {
+ if (!slot_pressed[i] && ts->slot_active[i]) {
+ input_mt_slot(ts->input_dev, i);
+ input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER, false);
+ ts->slot_active[i] = false;
+ } else if (slot_pressed[i]) {
+ ts->slot_active[i] = true;
+ }
+ }
+
+ input_report_key(ts->input_dev, BTN_TOUCH, finger_cnt > 0);
+ input_mt_sync_frame(ts->input_dev);
+ input_sync(ts->input_dev);
+
+ mutex_unlock(&ts->lock);
+
+ return IRQ_HANDLED;
+}
+
+static int novatek_ts_init_input(struct novatek_ts_data *ts)
+{
+ struct spi_device *spi = ts->spi;
+ struct input_dev *input;
+ int ret;
+
+ input = devm_input_allocate_device(&spi->dev);
+ if (!input)
+ return -ENOMEM;
+
+ input->name = "Novatek NT36672A Touchscreen";
+ input->id.bustype = BUS_SPI;
+ input->dev.parent = &spi->dev;
+
+ __set_bit(EV_SYN, input->evbit);
+ __set_bit(EV_KEY, input->evbit);
+ __set_bit(EV_ABS, input->evbit);
+ __set_bit(INPUT_PROP_DIRECT, input->propbit);
+
+ input_set_capability(input, EV_KEY, BTN_TOUCH);
+ input_set_abs_params(input, ABS_MT_POSITION_X, 0, ts->abs_x_max, 0, 0);
+ input_set_abs_params(input, ABS_MT_POSITION_Y, 0, ts->abs_y_max, 0, 0);
+ input_set_abs_params(input, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
+ input_set_abs_params(input, ABS_MT_PRESSURE, 0, 255, 0, 0);
+
+ ret = input_mt_init_slots(input, NVT_MAX_TOUCHES,
+ INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
+ if (ret)
+ return ret;
+
+ ret = input_register_device(input);
+ if (ret)
+ return ret;
+
+ ts->input_dev = input;
+
+ return 0;
+}
+
+static int novatek_ts_probe(struct spi_device *spi)
+{
+ struct device *dev = &spi->dev;
+ struct novatek_ts_data *ts;
+ int ret;
+
+ ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
+ if (!ts)
+ return -ENOMEM;
+
+ ts->spi = spi;
+ mutex_init(&ts->lock);
+
+ ts->fw_wq = create_singlethread_workqueue("novatek_ts_fw");
+ if (!ts->fw_wq)
+ return -ENOMEM;
+
+ INIT_DELAYED_WORK(&ts->fw_reload_work, novatek_ts_fw_work);
+
+ ret = device_property_read_string(dev, "firmware-name", &ts->fw_name);
+ if (ret)
+ ts->fw_name = NVT_DEFAULT_FW_NAME;
+
+ spi->mode = SPI_MODE_0;
+ spi->bits_per_word = 8;
+ if (!spi->max_speed_hz)
+ spi->max_speed_hz = 4000000;
+
+ ret = spi_setup(spi);
+ if (ret)
+ goto err_destroy_wq;
+
+ spi_set_drvdata(spi, ts);
+
+ ts->abs_x_max = 1080;
+ ts->abs_y_max = 2340;
+
+ ret = novatek_ts_load_firmware(ts);
+ if (ret) {
+ dev_err(dev, "failed to download firmware during probe: %d\n", ret);
+ goto err_destroy_wq;
+ }
+
+ dev_info(dev, "Novatek NT36672A initialized (fw_ver=0x%02x, reset_state=0x%02x)\n",
+ ts->fw_ver, ts->fw_reset_state);
+
+ ret = novatek_ts_init_input(ts);
+ if (ret)
+ goto err_destroy_wq;
+
+ if (spi->irq <= 0) {
+ ret = dev_err_probe(dev, -EINVAL, "no IRQ specified\n");
+ goto err_destroy_wq;
+ }
+
+ ret = devm_request_threaded_irq(dev, spi->irq, NULL,
+ novatek_ts_threaded_irq,
+ IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ "novatek-nt36672a-ts", ts);
+ if (ret) {
+ dev_err_probe(dev, ret, "failed to request threaded IRQ\n");
+ goto err_destroy_wq;
+ }
+
+ if (drm_is_panel_follower(dev)) {
+ ts->panel_follower.funcs = &novatek_ts_panel_follower_funcs;
+ ret = devm_drm_panel_add_follower(dev, &ts->panel_follower);
+ if (ret) {
+ dev_err_probe(dev, ret, "failed to add panel follower\n");
+ goto err_destroy_wq;
+ }
+ }
+
+ return 0;
+
+err_destroy_wq:
+ cancel_delayed_work_sync(&ts->fw_reload_work);
+ destroy_workqueue(ts->fw_wq);
+ return ret;
+}
+
+static void novatek_ts_remove(struct spi_device *spi)
+{
+ struct novatek_ts_data *ts = spi_get_drvdata(spi);
+
+ cancel_delayed_work_sync(&ts->fw_reload_work);
+ destroy_workqueue(ts->fw_wq);
+}
+
+static const struct of_device_id novatek_ts_of_match[] = {
+ { .compatible = "novatek,nt36672a-spi" },
+ { .compatible = "novatek,nt36672a-ts" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, novatek_ts_of_match);
+
+static const struct spi_device_id novatek_ts_spi_id[] = {
+ { "nt36672a-ts" },
+ { }
+};
+MODULE_DEVICE_TABLE(spi, novatek_ts_spi_id);
+
+static struct spi_driver novatek_ts_driver = {
+ .driver = {
+ .name = "novatek-nt36672a-spi",
+ .of_match_table = novatek_ts_of_match,
+ },
+ .id_table = novatek_ts_spi_id,
+ .probe = novatek_ts_probe,
+ .remove = novatek_ts_remove,
+};
+module_spi_driver(novatek_ts_driver);
+
+MODULE_AUTHOR("Antigravity Pair Programmer");
+MODULE_DESCRIPTION("Novatek NT36672A SPI Touchscreen Driver");
+MODULE_LICENSE("GPL");
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH RFC 06/10] Input: novatek-nt36672a-spi: add driver for Novatek NT36672A SPI touchscreen
2026-09-11 8:51 ` [PATCH RFC 06/10] Input: novatek-nt36672a-spi: add driver for " YİĞİTCAN KAVAKLI via B4 Relay
@ 2026-09-11 9:02 ` sashiko-bot
0 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-09-11 9:02 UTC (permalink / raw)
To: YİĞİTCAN KAVAKLI
Cc: devicetree, robh, linux-leds, linux-input, dmitry.torokhov,
dri-devel, conor+dt, lee
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Use-after-free vulnerability due to mismatched devm/manual cleanup lifetimes.
- [High] Out-of-bounds memory read during firmware partition parsing and payload download.
- [Medium] Incomplete implementation of standard touchscreen properties API.
- [Medium] Synchronous request_firmware() in probe causes permanent failure if firmware is not immediately available.
- [Low] Redundant manual configuration violating input subsystem guidelines.
--
commit eca71b1ff2a43ef234d60ca689183c4d75eed0bc
Author: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
Input: novatek-nt36672a-spi: add driver for Novatek NT36672A SPI touchscreen
Add support for the Novatek NT36672A TDDI touchscreen controller over SPI,
featuring dynamic firmware download, multi-partition binary parsing, and
DRM panel follower integration.
> diff --git a/drivers/input/touchscreen/novatek-nt36672a-spi.c b/drivers/input/touchscreen/novatek-nt36672a-spi.c
> new file mode 100644
> index 0000000000000..03d2042b0bb30
> --- /dev/null
> +++ b/drivers/input/touchscreen/novatek-nt36672a-spi.c
[ ... ]
> +static int nvt_write_sram(struct spi_device *spi, const u8 *fwdata,
> + u32 sram_addr, u32 size, u32 bin_addr)
> +{
> + u32 count, i, len;
> + int ret = 0;
> + u8 *fwbuf;
> +
> + fwbuf = kmalloc(NVT_TRANSFER_LEN + 2, GFP_KERNEL);
> + if (!fwbuf)
> + return -ENOMEM;
> +
> + count = (size + NVT_TRANSFER_LEN - 1) / NVT_TRANSFER_LEN;
> +
> + for (i = 0; i < count; i++) {
> + len = (size < NVT_TRANSFER_LEN) ? size : NVT_TRANSFER_LEN;
> +
> + ret = nvt_set_page(spi, sram_addr);
> + if (ret)
> + goto out;
> +
> + fwbuf[0] = sram_addr & 0x7f;
> + memcpy(fwbuf + 1, &fwdata[bin_addr], len);
[Severity: High]
Does this code perform an out-of-bounds read if the provided firmware file is
malformed?
The untrusted bin_addr and size are directly used to index fwdata without
validating that bin_addr + size <= fwsize, resulting in arbitrary
out-of-bounds reads during the copy.
[ ... ]
> +static int nvt_parse_and_download(struct novatek_ts_data *ts, const u8 *fwdata, size_t fwsize)
> +{
> + struct spi_device *spi = ts->spi;
> + struct nvt_ts_bin_map bin_map[16];
> + u8 info_sec_num = 0;
> + u8 ovly_sec_num = 0;
> + u32 partition, list;
> + u32 header_end, pos;
> + u8 buf[18];
> + int ret, retry;
> +
> + if (fwsize < 0x40)
> + return -EINVAL;
> +
> + header_end = byte_to_word(&fwdata[0]);
> + pos = 0x30;
> + while (pos < header_end && info_sec_num < 14) {
> + info_sec_num++;
> + pos += 0x10;
> + }
> +
> + ovly_sec_num = (fwdata[0x28] & 0x10) ? (fwdata[0x28] & 0x0f) : 0;
> + partition = 2 + ovly_sec_num + info_sec_num;
> + if (partition > 16)
> + partition = 16;
> +
> + for (list = 0; list < partition; list++) {
> + if (list < 2) {
> + bin_map[list].bin_addr = byte_to_word(&fwdata[0 + list * 12]);
> + bin_map[list].sram_addr = byte_to_word(&fwdata[4 + list * 12]);
> + bin_map[list].size = byte_to_word(&fwdata[8 + list * 12]);
> + bin_map[list].crc = byte_to_word(&fwdata[0x18 + list * 4]);
> + } else {
> + pos = 0x30 + (0x10 * (list - 2));
> + bin_map[list].sram_addr = byte_to_word(&fwdata[pos]);
> + bin_map[list].size = byte_to_word(&fwdata[pos + 4]);
> + bin_map[list].bin_addr = byte_to_word(&fwdata[pos + 8]);
[Severity: High]
Can this loop read out of bounds?
The parsing loop computes 'pos' dynamically up to 16 partitions without bounds
checking against fwsize. This can reach offsets past the minimum 0x40 bytes
checked earlier.
[ ... ]
> +static int novatek_ts_panel_prepared(struct drm_panel_follower *follower)
> +{
> + struct novatek_ts_data *ts = container_of(follower, struct novatek_ts_data, panel_follower);
> +
> + if (ts->fw_wq) {
> + cancel_delayed_work_sync(&ts->fw_reload_work);
> + queue_delayed_work(ts->fw_wq, &ts->fw_reload_work, msecs_to_jiffies(1500));
[Severity: High]
Could this result in a use-after-free if triggered after the driver is
unbound?
If a panel prepare event fires after novatek_ts_remove() destroys the
workqueue but before devres removes the panel follower, this will invoke
queue_delayed_work() on the already-freed ts->fw_wq pointer.
[ ... ]
> +static irqreturn_t novatek_ts_threaded_irq(int irq, void *dev_id)
> +{
> + struct novatek_ts_data *ts = dev_id;
> + u8 point_data[NVT_POINT_DATA_LEN + 2] = {0};
> + bool slot_pressed[NVT_MAX_TOUCHES] = {false};
> + int finger_cnt = 0;
> + int ret, i;
> +
> + mutex_lock(&ts->lock);
> +
> + nvt_set_page(ts->spi, EVENT_BUF_ADDR);
> +
> + point_data[0] = 0x00;
> + ret = nvt_spi_read(ts->spi, point_data, NVT_POINT_DATA_LEN + 1);
> + if (ret < 0) {
> + dev_err_ratelimited(&ts->spi->dev, "failed to read touch points: %d\n", ret);
> + mutex_unlock(&ts->lock);
> + return IRQ_HANDLED;
> + }
> +
> + for (i = 0; i < NVT_MAX_TOUCHES; i++) {
> + int pos = 1 + 6 * i;
> + u8 raw0 = point_data[pos];
> + u8 input_id = raw0 >> 3;
> + u8 status = raw0 & 0x07;
> +
> + if (input_id < 1 || input_id > NVT_MAX_TOUCHES)
> + continue;
> +
> + if (status == 0x01 || status == 0x02) { /* 1: down, 2: move */
> + u32 x = ((u32)point_data[pos + 1] << 4) | ((u32)point_data[pos + 3] >> 4);
> + u32 y = ((u32)point_data[pos + 2] << 4) | ((u32)point_data[pos + 3] & 0x0f);
> + u32 w = point_data[pos + 4] ? point_data[pos + 4] : 1;
> + u32 p = point_data[pos + 5] ? point_data[pos + 5] : 1;
> +
> + if (x <= ts->abs_x_max && y <= ts->abs_y_max) {
> + int slot = input_id - 1;
> +
> + slot_pressed[slot] = true;
> + finger_cnt++;
> +
> + input_mt_slot(ts->input_dev, slot);
> + input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER, true);
> + input_report_abs(ts->input_dev, ABS_MT_POSITION_X, x);
> + input_report_abs(ts->input_dev, ABS_MT_POSITION_Y, y);
[Severity: Medium]
Should this use touchscreen_report_pos() instead of reporting the coordinates
raw?
The driver defines a touchscreen_properties struct but bypasses orientation
and scaling adjustments by not using the standard reporting helper here.
[ ... ]
> +static int novatek_ts_init_input(struct novatek_ts_data *ts)
> +{
> + struct spi_device *spi = ts->spi;
> + struct input_dev *input;
> + int ret;
> +
> + input = devm_input_allocate_device(&spi->dev);
> + if (!input)
> + return -ENOMEM;
> +
> + input->name = "Novatek NT36672A Touchscreen";
> + input->id.bustype = BUS_SPI;
> + input->dev.parent = &spi->dev;
> +
> + __set_bit(EV_SYN, input->evbit);
> + __set_bit(EV_KEY, input->evbit);
> + __set_bit(EV_ABS, input->evbit);
> + __set_bit(INPUT_PROP_DIRECT, input->propbit);
> +
> + input_set_capability(input, EV_KEY, BTN_TOUCH);
[Severity: Low]
Are these manual configurations redundant?
According to the input subsystem guidelines, devm_input_allocate_device()
automatically sets the device parent. Additionally, the core automatically
manages EV_SYN, and input_mt_init_slots() with INPUT_MT_DIRECT automatically
adds EV_KEY, BTN_TOUCH, and INPUT_PROP_DIRECT.
> + input_set_abs_params(input, ABS_MT_POSITION_X, 0, ts->abs_x_max, 0, 0);
> + input_set_abs_params(input, ABS_MT_POSITION_Y, 0, ts->abs_y_max, 0, 0);
> + input_set_abs_params(input, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
> + input_set_abs_params(input, ABS_MT_PRESSURE, 0, 255, 0, 0);
> +
> + ret = input_mt_init_slots(input, NVT_MAX_TOUCHES,
> + INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
[Severity: Medium]
Is there a missing call to touchscreen_parse_properties() here?
Without it, the driver ignores standard properties like touchscreen-inverted-x,
leading to incorrect coordinate reporting on devices that rely on them.
[ ... ]
> +static int novatek_ts_probe(struct spi_device *spi)
> +{
> + struct device *dev = &spi->dev;
> + struct novatek_ts_data *ts;
> + int ret;
> +
> + ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
> + if (!ts)
> + return -ENOMEM;
> +
> + ts->spi = spi;
> + mutex_init(&ts->lock);
> +
> + ts->fw_wq = create_singlethread_workqueue("novatek_ts_fw");
> + if (!ts->fw_wq)
> + return -ENOMEM;
> +
> + INIT_DELAYED_WORK(&ts->fw_reload_work, novatek_ts_fw_work);
> +
> + ret = device_property_read_string(dev, "firmware-name", &ts->fw_name);
> + if (ret)
> + ts->fw_name = NVT_DEFAULT_FW_NAME;
> +
> + spi->mode = SPI_MODE_0;
> + spi->bits_per_word = 8;
> + if (!spi->max_speed_hz)
> + spi->max_speed_hz = 4000000;
> +
> + ret = spi_setup(spi);
> + if (ret)
> + goto err_destroy_wq;
> +
> + spi_set_drvdata(spi, ts);
> +
> + ts->abs_x_max = 1080;
> + ts->abs_y_max = 2340;
> +
> + ret = novatek_ts_load_firmware(ts);
> + if (ret) {
> + dev_err(dev, "failed to download firmware during probe: %d\n", ret);
> + goto err_destroy_wq;
> + }
[Severity: Medium]
Will this synchronous request_firmware() call cause a permanent probe failure if
the firmware is not immediately available at boot?
If the firmware is on a filesystem that hasn't mounted yet, returning an error
here prevents the device from ever initializing properly, even though the
driver implements a background firmware loader.
[ ... ]
> + if (drm_is_panel_follower(dev)) {
> + ts->panel_follower.funcs = &novatek_ts_panel_follower_funcs;
> + ret = devm_drm_panel_add_follower(dev, &ts->panel_follower);
[Severity: High]
Does this create a lifetime mismatch with the manually destroyed workqueue?
The panel follower relies on devm for cleanup, extending its lifetime beyond
the explicitly freed workqueue in novatek_ts_remove().
[ ... ]
> +static void novatek_ts_remove(struct spi_device *spi)
> +{
> + struct novatek_ts_data *ts = spi_get_drvdata(spi);
> +
> + cancel_delayed_work_sync(&ts->fw_reload_work);
> + destroy_workqueue(ts->fw_wq);
> +}
[Severity: High]
Is it safe to destroy the workqueue manually here before the panel follower
is removed by devm?
This manual destruction happens before devm automatically unregisters the DRM
panel follower, leaving a window where panel events can trigger workqueue
access on the freed pointer.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-ginkgo-submission-final-v1-0-0e68e63a18d4@gmail.com?part=6
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH RFC 07/10] drm/msm/dpu: describe SM6125 programmable-fetch delayed-start quirk
2026-09-11 8:50 [PATCH RFC 00/10] arm64: qcom: sm6125-ginkgo: display, backlight, and touch support YİĞİTCAN KAVAKLI via B4 Relay
` (5 preceding siblings ...)
2026-09-11 8:51 ` [PATCH RFC 06/10] Input: novatek-nt36672a-spi: add driver for " YİĞİTCAN KAVAKLI via B4 Relay
@ 2026-09-11 8:51 ` YİĞİTCAN KAVAKLI via B4 Relay
2026-09-11 9:01 ` sashiko-bot
2026-09-11 8:51 ` [PATCH RFC 08/10] drm/msm/dsi: make command and video mode configuration mutually exclusive YİĞİTCAN KAVAKLI via B4 Relay
` (2 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: YİĞİTCAN KAVAKLI via B4 Relay @ 2026-09-11 8:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Liam Girdwood,
Mark Brown, Sumit Semwal, Neil Armstrong, Jessica Zhang,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Dmitry Torokhov, Henrik Rydberg, Rob Clark,
Dmitry Baryshkov, Abhinav Kumar, Sean Paul, Marijn Suijten,
Bjorn Andersson, Konrad Dybcio
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev,
linux-input, linux-arm-msm, freedreno,
YİĞİTCAN KAVAKLI
From: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
On Qualcomm SM6125 (Snapdragon 665 / Trinket DPU 5.4.0), calculating the
programmable prefetch start counter strictly as:
(vert_total - vfp_fetch_lines) * horiz_total + 1
causes the fetch trigger to evaluate on the active-video to vertical
front porch line boundary. In hardware, this triggers an internal line
buffer FIFO conflict within the INTF timing generator, resulting in
immediate DSI FIFO overflow and timeout errors (status=5) and complete
loss of pixel output.
Qualcomm downstream display drivers mitigate this erratum via
delay_prg_fetch_start = true, delaying the fetch start by one scanline
(+ horiz_total) into vertical front porch line 1.
Introduce 'has_prg_fetch_delay' in struct dpu_caps, enable it for SM6125
in its catalog entry, and throttle fetch start in programmable_fetch_config()
when the capability is set.
Signed-off-by: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
---
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_4_sm6125.h | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 11 +++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++
3 files changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_4_sm6125.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_4_sm6125.h
index 64be51e301591b7f17918475a957623c6fea23d4..ec0315284e0663f57a41a1fdf515d948c3d40d68 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_4_sm6125.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_4_sm6125.h
@@ -13,6 +13,7 @@ static const struct dpu_caps sm6125_dpu_caps = {
.max_mixer_blendstages = 0x6,
.has_dim_layer = true,
.has_idle_pc = true,
+ .has_prg_fetch_delay = true,
.max_linewidth = 2160,
.pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
.max_hdeci_exp = MAX_HORZ_DECIMATION,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index ba810f26ea3099106aac1783f7f6bf5e014e41a0..15445eaf9f3baf4661e22bee133cc2059a3cd2b1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -237,10 +237,21 @@ static void programmable_fetch_config(struct dpu_encoder_phys *phys_enc,
vfp_fetch_lines = programmable_fetch_get_num_lines(phys_enc, timing);
if (vfp_fetch_lines) {
+ const struct dpu_mdss_cfg *catalog = phys_enc->dpu_kms->catalog;
+
vert_total = get_vertical_total(timing);
horiz_total = get_horizontal_total(timing);
vfp_fetch_start_vsync_counter =
(vert_total - vfp_fetch_lines) * horiz_total + 1;
+
+ /*
+ * SM6125 (DPU 5.4 / Trinket) quirk matching downstream
+ * delay_prg_fetch_start = true. Throttle fetch start
+ * to the second line after active video (VFP line 1).
+ */
+ if (catalog && catalog->caps && catalog->caps->has_prg_fetch_delay)
+ vfp_fetch_start_vsync_counter += horiz_total;
+
f.enable = 1;
f.fetch_start = vfp_fetch_start_vsync_counter;
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index f45faf87333e89bbd463504ae3219609ef6209bd..db3cbf9fa04a13baf3ac40390757ed24120ccccd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -273,6 +273,7 @@ struct dpu_rotation_cfg {
* @has_dim_layer dim layer feature status
* @has_idle_pc indicate if idle power collapse feature is supported
* @has_3d_merge indicate if 3D merge is supported
+ * @has_prg_fetch_delay indicate if programmable fetch start requires a 1-line delay
* @max_linewidth max linewidth for sspp
* @pixel_ram_size size of latency hiding and de-tiling buffer in bytes
* @max_hdeci_exp max horizontal decimation supported (max is 2^value)
@@ -285,6 +286,7 @@ struct dpu_caps {
bool has_dim_layer;
bool has_idle_pc;
bool has_3d_merge;
+ bool has_prg_fetch_delay;
/* SSPP limits */
u32 max_linewidth;
u32 pixel_ram_size;
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH RFC 07/10] drm/msm/dpu: describe SM6125 programmable-fetch delayed-start quirk
2026-09-11 8:51 ` [PATCH RFC 07/10] drm/msm/dpu: describe SM6125 programmable-fetch delayed-start quirk YİĞİTCAN KAVAKLI via B4 Relay
@ 2026-09-11 9:01 ` sashiko-bot
0 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-09-11 9:01 UTC (permalink / raw)
To: YİĞİTCAN KAVAKLI
Cc: lee, dri-devel, dmitry.torokhov, conor+dt, robh, linux-leds,
linux-input, devicetree
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional addition of `horiz_total` to `vfp_fetch_start_vsync_counter` can push the fetch trigger out of bounds, preventing it from firing.
--
commit 2270768f2936b4e384a8e10940ee1267f2738202
Author: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
drm/msm/dpu: describe SM6125 programmable-fetch delayed-start quirk
This commit introduces a workaround for the SM6125 DPU where the programmable
prefetch start counter evaluating on the active-video to vertical front porch
boundary causes FIFO overflows. It adds a has_prg_fetch_delay quirk to delay
the fetch start by one scanline by adding horiz_total to the counter.
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> index ba810f26ea309..15445eaf9f3ba 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
[ ... ]
> @@ -237,10 +237,21 @@ static void programmable_fetch_config(struct dpu_encoder_phys *phys_enc,
>
> vfp_fetch_lines = programmable_fetch_get_num_lines(phys_enc, timing);
> if (vfp_fetch_lines) {
> + const struct dpu_mdss_cfg *catalog = phys_enc->dpu_kms->catalog;
> +
> vert_total = get_vertical_total(timing);
> horiz_total = get_horizontal_total(timing);
> vfp_fetch_start_vsync_counter =
> (vert_total - vfp_fetch_lines) * horiz_total + 1;
> +
> + /*
> + * SM6125 (DPU 5.4 / Trinket) quirk matching downstream
> + * delay_prg_fetch_start = true. Throttle fetch start
> + * to the second line after active video (VFP line 1).
> + */
> + if (catalog && catalog->caps && catalog->caps->has_prg_fetch_delay)
> + vfp_fetch_start_vsync_counter += horiz_total;
[Severity: High]
Could this unconditional addition push the fetch start counter out of bounds?
If a panel is configured such that vfp_fetch_lines evaluates to 1,
vfp_fetch_start_vsync_counter initially calculates to:
(vert_total - 1) * horiz_total + 1
Adding horiz_total then makes the counter evaluate to:
vert_total * horiz_total + 1
Since a full frame consists of exactly vert_total * horiz_total pixels, the
hardware vsync counter will wrap at the end of the frame before reaching this
new value. Will this prevent the programmable fetch trigger from ever firing
and result in a complete loss of pixel output?
> +
> f.enable = 1;
> f.fetch_start = vfp_fetch_start_vsync_counter;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-ginkgo-submission-final-v1-0-0e68e63a18d4@gmail.com?part=7
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH RFC 08/10] drm/msm/dsi: make command and video mode configuration mutually exclusive
2026-09-11 8:50 [PATCH RFC 00/10] arm64: qcom: sm6125-ginkgo: display, backlight, and touch support YİĞİTCAN KAVAKLI via B4 Relay
` (6 preceding siblings ...)
2026-09-11 8:51 ` [PATCH RFC 07/10] drm/msm/dpu: describe SM6125 programmable-fetch delayed-start quirk YİĞİTCAN KAVAKLI via B4 Relay
@ 2026-09-11 8:51 ` YİĞİTCAN KAVAKLI via B4 Relay
2026-09-11 9:00 ` sashiko-bot
2026-09-11 8:51 ` [PATCH RFC 09/10] drm/msm/dsi: separate host link enable from video stream enable YİĞİTCAN KAVAKLI via B4 Relay
2026-09-11 8:51 ` [PATCH RFC 10/10] arm64: dts: qcom: sm6125-xiaomi-ginkgo: enable display, backlight and touchscreen YİĞİTCAN KAVAKLI via B4 Relay
9 siblings, 1 reply; 21+ messages in thread
From: YİĞİTCAN KAVAKLI via B4 Relay @ 2026-09-11 8:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Liam Girdwood,
Mark Brown, Sumit Semwal, Neil Armstrong, Jessica Zhang,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Dmitry Torokhov, Henrik Rydberg, Rob Clark,
Dmitry Baryshkov, Abhinav Kumar, Sean Paul, Marijn Suijten,
Bjorn Andersson, Konrad Dybcio
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev,
linux-input, linux-arm-msm, freedreno,
YİĞİTCAN KAVAKLI
From: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
In dsi_op_mode_config(), entering video mode sets DSI_CTRL_VID_MODE_EN
without clearing DSI_CTRL_CMD_MODE_EN, and entering command mode sets
DSI_CTRL_CMD_MODE_EN without clearing DSI_CTRL_VID_MODE_EN.
Ensure explicit mutual exclusion by clearing the opposing enable bit
when switching operational modes to prevent hardware bus collisions.
Signed-off-by: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 7e4e3718b536c92dc555de213042857379499447..b9e2867d435a3ad8b6db6b57c440bbd827180b6e 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1094,7 +1094,9 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
dsi_write(msm_host, REG_DSI_ACTIVE_HSYNC,
DSI_ACTIVE_HSYNC_START(hs_start) |
DSI_ACTIVE_HSYNC_END(hs_end));
- dsi_write(msm_host, REG_DSI_ACTIVE_VSYNC_HPOS, 0);
+ dsi_write(msm_host, REG_DSI_ACTIVE_VSYNC_HPOS,
+ DSI_ACTIVE_VSYNC_HPOS_START(hs_start) |
+ DSI_ACTIVE_VSYNC_HPOS_END(vs_end));
dsi_write(msm_host, REG_DSI_ACTIVE_VSYNC_VPOS,
DSI_ACTIVE_VSYNC_VPOS_START(vs_start) |
DSI_ACTIVE_VSYNC_VPOS_END(vs_end));
@@ -1168,8 +1170,10 @@ static void dsi_op_mode_config(struct msm_dsi_host *msm_host,
DSI_IRQ_MASK_VIDEO_DONE, 0);
} else {
if (video_mode) {
+ dsi_ctrl &= ~DSI_CTRL_CMD_MODE_EN;
dsi_ctrl |= DSI_CTRL_VID_MODE_EN;
} else { /* command mode */
+ dsi_ctrl &= ~DSI_CTRL_VID_MODE_EN;
dsi_ctrl |= DSI_CTRL_CMD_MODE_EN;
dsi_intr_ctrl(msm_host, DSI_IRQ_MASK_CMD_MDP_DONE, 1);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH RFC 08/10] drm/msm/dsi: make command and video mode configuration mutually exclusive
2026-09-11 8:51 ` [PATCH RFC 08/10] drm/msm/dsi: make command and video mode configuration mutually exclusive YİĞİTCAN KAVAKLI via B4 Relay
@ 2026-09-11 9:00 ` sashiko-bot
0 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-09-11 9:00 UTC (permalink / raw)
To: YİĞİTCAN KAVAKLI
Cc: lee, devicetree, dri-devel, conor+dt, linux-leds, linux-input,
dmitry.torokhov, robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch silently introduces an undocumented change that incorrectly configures the `REG_DSI_ACTIVE_VSYNC_HPOS` register by assigning a vertical timing value (`vs_end`) to a horizontal position field.
--
commit 2bb1060f4bf71f32baecff799ef80f36dc371942
Author: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
drm/msm/dsi: make command and video mode configuration mutually exclusive
This commit ensures explicit mutual exclusion between command and video
modes by clearing the opposing enable bit in dsi_op_mode_config() to
prevent hardware collisions.
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -1094,7 +1094,9 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
> dsi_write(msm_host, REG_DSI_ACTIVE_HSYNC,
> DSI_ACTIVE_HSYNC_START(hs_start) |
> DSI_ACTIVE_HSYNC_END(hs_end));
> - dsi_write(msm_host, REG_DSI_ACTIVE_VSYNC_HPOS, 0);
> + dsi_write(msm_host, REG_DSI_ACTIVE_VSYNC_HPOS,
> + DSI_ACTIVE_VSYNC_HPOS_START(hs_start) |
> + DSI_ACTIVE_VSYNC_HPOS_END(vs_end));
[Severity: High]
Could this inadvertently corrupt the hardware timing configuration? It looks
like vs_end, which is a vertical timing parameter, is being written into the
DSI_ACTIVE_VSYNC_HPOS_END field, which expects a horizontal position.
Because this happens unconditionally in dsi_timing_setup() for video-mode
displays during the DSI host power-on sequence, this might lead to display
tearing, sync failure, or a blank screen.
Additionally, since it uses vs_end instead of hs_end, this appears to bypass
the bonded DSI scaling logic where hs_end is halved, which might feed
incorrect horizontal synchronization bounds to bonded configurations. Was
this an accidental copy-paste inclusion?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-ginkgo-submission-final-v1-0-0e68e63a18d4@gmail.com?part=8
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH RFC 09/10] drm/msm/dsi: separate host link enable from video stream enable
2026-09-11 8:50 [PATCH RFC 00/10] arm64: qcom: sm6125-ginkgo: display, backlight, and touch support YİĞİTCAN KAVAKLI via B4 Relay
` (7 preceding siblings ...)
2026-09-11 8:51 ` [PATCH RFC 08/10] drm/msm/dsi: make command and video mode configuration mutually exclusive YİĞİTCAN KAVAKLI via B4 Relay
@ 2026-09-11 8:51 ` YİĞİTCAN KAVAKLI via B4 Relay
2026-09-11 9:04 ` sashiko-bot
2026-09-11 8:51 ` [PATCH RFC 10/10] arm64: dts: qcom: sm6125-xiaomi-ginkgo: enable display, backlight and touchscreen YİĞİTCAN KAVAKLI via B4 Relay
9 siblings, 1 reply; 21+ messages in thread
From: YİĞİTCAN KAVAKLI via B4 Relay @ 2026-09-11 8:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Liam Girdwood,
Mark Brown, Sumit Semwal, Neil Armstrong, Jessica Zhang,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Dmitry Torokhov, Henrik Rydberg, Rob Clark,
Dmitry Baryshkov, Abhinav Kumar, Sean Paul, Marijn Suijten,
Bjorn Andersson, Konrad Dybcio
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev,
linux-input, linux-arm-msm, freedreno,
YİĞİTCAN KAVAKLI
From: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
In video mode DSI displays, enabling the host previously engaged the
hardware video engine immediately during atomic_pre_enable. When panel
prepare routines transmit initial DCS sleep-out and setup commands,
concurrent active video transmission causes DMA timeouts (-110).
Keep the DSI host in command mode during atomic_pre_enable to permit
clean panel preparation, and start video streaming in atomic_enable
after the panel has completed initialization.
Signed-off-by: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
---
drivers/gpu/drm/msm/dsi/dsi.h | 1 +
drivers/gpu/drm/msm/dsi/dsi_host.c | 23 ++++++++++++-----------
drivers/gpu/drm/msm/dsi/dsi_manager.c | 20 ++++++++++++++++++++
3 files changed, 33 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
index 93c028a122f3a59b1632da76472e0a3e781c6ae8..cfb2b51cae0a9227fbe88ecc5291ac1a9f82a5ce 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -69,6 +69,7 @@ int msm_dsi_host_cmd_rx(struct mipi_dsi_host *host,
void msm_dsi_host_cmd_xfer_commit(struct mipi_dsi_host *host,
u32 dma_base, u32 len);
int msm_dsi_host_enable(struct mipi_dsi_host *host);
+void msm_dsi_host_enable_video(struct mipi_dsi_host *host);
int msm_dsi_host_disable(struct mipi_dsi_host *host);
void msm_dsi_host_enable_irq(struct mipi_dsi_host *host);
void msm_dsi_host_disable_irq(struct mipi_dsi_host *host);
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index b9e2867d435a3ad8b6db6b57c440bbd827180b6e..b2702f772ee195b2758433d98d0f071e3f55955e 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -2438,22 +2438,23 @@ int msm_dsi_host_enable(struct mipi_dsi_host *host)
{
struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
- dsi_op_mode_config(msm_host,
- !!(msm_host->mode_flags & MIPI_DSI_MODE_VIDEO), true);
-
- /* TODO: clock should be turned off for command mode,
- * and only turned on before MDP START.
- * This part of code should be enabled once mdp driver support it.
- */
- /* if (msm_panel->mode == MSM_DSI_CMD_MODE) {
- * dsi_link_clk_disable(msm_host);
- * pm_runtime_put(&msm_host->pdev->dev);
- * }
+ /* Keep DSI host in command mode (VID_MODE_EN = 0) during panel init
+ * to allow clean DCS communication without premature video engine startup.
*/
+ dsi_op_mode_config(msm_host, false, true);
+
msm_host->enabled = true;
return 0;
}
+void msm_dsi_host_enable_video(struct mipi_dsi_host *host)
+{
+ struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
+
+ if (msm_host->mode_flags & MIPI_DSI_MODE_VIDEO)
+ dsi_op_mode_config(msm_host, true, true);
+}
+
int msm_dsi_host_disable(struct mipi_dsi_host *host)
{
struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 959daa88b1d022da5f552bb21ca7c04006524fa4..9e3f125ec4ea8e3fae50c8226e67b4106c01dc76 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -446,12 +446,32 @@ static int dsi_mgr_bridge_attach(struct drm_bridge *bridge,
bridge, flags);
}
+static void dsi_mgr_bridge_enable(struct drm_bridge *bridge,
+ struct drm_atomic_commit *commit)
+{
+ int id = dsi_mgr_bridge_get_id(bridge);
+ struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
+ struct msm_dsi *msm_dsi1 = dsi_mgr_get_dsi(DSI_1);
+ struct mipi_dsi_host *host = msm_dsi->host;
+ bool is_bonded_dsi = IS_BONDED_DSI();
+
+ DBG("id=%d", id);
+
+ if (is_bonded_dsi && !IS_MASTER_DSI_LINK(id))
+ return;
+
+ msm_dsi_host_enable_video(host);
+ if (is_bonded_dsi && msm_dsi1)
+ msm_dsi_host_enable_video(msm_dsi1->host);
+}
+
static const struct drm_bridge_funcs dsi_mgr_bridge_funcs = {
.atomic_create_state = drm_atomic_helper_bridge_create_state,
.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
.attach = dsi_mgr_bridge_attach,
.atomic_pre_enable = dsi_mgr_bridge_pre_enable,
+ .atomic_enable = dsi_mgr_bridge_enable,
.atomic_post_disable = dsi_mgr_bridge_post_disable,
.mode_set = dsi_mgr_bridge_mode_set,
.mode_valid = dsi_mgr_bridge_mode_valid,
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH RFC 09/10] drm/msm/dsi: separate host link enable from video stream enable
2026-09-11 8:51 ` [PATCH RFC 09/10] drm/msm/dsi: separate host link enable from video stream enable YİĞİTCAN KAVAKLI via B4 Relay
@ 2026-09-11 9:04 ` sashiko-bot
0 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-09-11 9:04 UTC (permalink / raw)
To: YİĞİTCAN KAVAKLI
Cc: linux-input, robh, linux-leds, dri-devel, conor+dt, lee,
dmitry.torokhov, devicetree
> From: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
>
> In video mode DSI displays, enabling the host previously engaged the
> hardware video engine immediately during atomic_pre_enable. When panel
> prepare routines transmit initial DCS sleep-out and setup commands,
> concurrent active video transmission causes DMA timeouts (-110).
>
> Keep the DSI host in command mode during atomic_pre_enable to permit
> clean panel preparation, and start video streaming in atomic_enable
> after the panel has completed initialization.
>
> Signed-off-by: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-ginkgo-submission-final-v1-0-0e68e63a18d4@gmail.com?part=9
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH RFC 10/10] arm64: dts: qcom: sm6125-xiaomi-ginkgo: enable display, backlight and touchscreen
2026-09-11 8:50 [PATCH RFC 00/10] arm64: qcom: sm6125-ginkgo: display, backlight, and touch support YİĞİTCAN KAVAKLI via B4 Relay
` (8 preceding siblings ...)
2026-09-11 8:51 ` [PATCH RFC 09/10] drm/msm/dsi: separate host link enable from video stream enable YİĞİTCAN KAVAKLI via B4 Relay
@ 2026-09-11 8:51 ` YİĞİTCAN KAVAKLI via B4 Relay
2026-09-11 9:04 ` sashiko-bot
9 siblings, 1 reply; 21+ messages in thread
From: YİĞİTCAN KAVAKLI via B4 Relay @ 2026-09-11 8:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Liam Girdwood,
Mark Brown, Sumit Semwal, Neil Armstrong, Jessica Zhang,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Dmitry Torokhov, Henrik Rydberg, Rob Clark,
Dmitry Baryshkov, Abhinav Kumar, Sean Paul, Marijn Suijten,
Bjorn Andersson, Konrad Dybcio
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev,
linux-input, linux-arm-msm, freedreno,
YİĞİTCAN KAVAKLI
From: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
Enable the display subsystem (MDSS/MDP/DSI0), Kinetic KTD3136 backlight,
and Novatek NT36672A SPI touchscreen on the Xiaomi Redmi Note 8 (ginkgo).
Hardware nodes added:
- Fixed LCDB regulators (lcdb_lab and lcdb_ibb at +/- 5.5V)
- MDSS and DSI0 with Tianma 1080x2340 FHD+ video panel
- I2C1 backlight node at 0x36 with PMI632 GPIO 6 hardware enable
- SPI2 touchscreen node at CS0 with GPIO 88 interrupt and panel follower
reference
- Touchscreen active pinctrl definition on GPIO 88
Signed-off-by: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
---
.../boot/dts/qcom/sm6125-xiaomi-ginkgo-common.dtsi | 100 +++++++++++++++++++++
1 file changed, 100 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm6125-xiaomi-ginkgo-common.dtsi b/arch/arm64/boot/dts/qcom/sm6125-xiaomi-ginkgo-common.dtsi
index 59042a5937535c4acda064fa0f865f4f5a52d01b..f4b9e2092a814e56cdc23218025cb4166dec110a 100644
--- a/arch/arm64/boot/dts/qcom/sm6125-xiaomi-ginkgo-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6125-xiaomi-ginkgo-common.dtsi
@@ -26,11 +26,30 @@ / {
chassis-type = "handset";
qcom,msm-id = <QCOM_ID_SM6125 0x10000>;
+ qcom,board-id = <34 0>, <34 1>, <34 2>, <34 3>, <34 4>, <0 0>;
aliases {
serial0 = &uart4;
};
+ lcdb_lab: regulator-lcdb-lab {
+ compatible = "regulator-fixed";
+ regulator-name = "lcdb_lab";
+ regulator-min-microvolt = <5500000>;
+ regulator-max-microvolt = <5500000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ lcdb_ibb: regulator-lcdb-ibb {
+ compatible = "regulator-fixed";
+ regulator-name = "lcdb_ibb";
+ regulator-min-microvolt = <5500000>;
+ regulator-max-microvolt = <5500000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
chosen {
#address-cells = <2>;
#size-cells = <2>;
@@ -356,6 +375,13 @@ irled@1 {
&tlmm {
gpio-reserved-ranges = <0 4>, <30 4>;
+
+ ts_int_active: ts-int-active-state {
+ pins = "gpio88";
+ function = "gpio";
+ drive-strength = <8>;
+ bias-pull-up;
+ };
};
&uart4 {
@@ -369,3 +395,77 @@ &usb3 {
&usb_dwc3_hs {
remote-endpoint = <&pmi632_hs_in>;
};
+
+&mdss {
+ status = "okay";
+};
+
+&mdss_mdp {
+ status = "okay";
+};
+
+&mdss_dsi0_phy {
+ vdds-supply = <&vreg_l7a>;
+ status = "okay";
+};
+
+&mdss_dsi0 {
+ vdda-supply = <&vreg_l18a>;
+ status = "okay";
+
+ panel: panel@0 {
+ compatible = "tianma,fhd-video-ginkgo", "novatek,nt36672a";
+ reg = <0>;
+
+ reset-gpios = <&tlmm 90 GPIO_ACTIVE_LOW>;
+ vddio-supply = <&vreg_l9a>;
+ vddpos-supply = <&lcdb_lab>;
+ vddneg-supply = <&lcdb_ibb>;
+ backlight = <&backlight>;
+
+ port {
+ panel_in: endpoint {
+ remote-endpoint = <&mdss_dsi0_out>;
+ };
+ };
+ };
+};
+
+&mdss_dsi0_out {
+ remote-endpoint = <&panel_in>;
+ data-lanes = <0 1 2 3>;
+};
+
+&qupv3_id_0 {
+ status = "okay";
+};
+
+&i2c1 {
+ clock-frequency = <400000>;
+ status = "okay";
+
+ backlight: backlight@36 {
+ compatible = "kinetic,ktd3136";
+ reg = <0x36>;
+ enable-gpios = <&pmi632_gpios 6 GPIO_ACTIVE_HIGH>;
+ default-brightness = <2047>;
+ max-brightness = <2047>;
+ };
+};
+
+&spi2 {
+ status = "okay";
+
+ touchscreen@0 {
+ compatible = "novatek,nt36672a-spi", "novatek,nt36672a-ts";
+ reg = <0>;
+ spi-max-frequency = <4000000>;
+ interrupt-parent = <&tlmm>;
+ interrupts = <88 IRQ_TYPE_EDGE_RISING>;
+ pinctrl-0 = <&ts_int_active>;
+ pinctrl-names = "default";
+ panel = <&panel>;
+ touchscreen-size-x = <1080>;
+ touchscreen-size-y = <2340>;
+ };
+};
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH RFC 10/10] arm64: dts: qcom: sm6125-xiaomi-ginkgo: enable display, backlight and touchscreen
2026-09-11 8:51 ` [PATCH RFC 10/10] arm64: dts: qcom: sm6125-xiaomi-ginkgo: enable display, backlight and touchscreen YİĞİTCAN KAVAKLI via B4 Relay
@ 2026-09-11 9:04 ` sashiko-bot
0 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-09-11 9:04 UTC (permalink / raw)
To: YİĞİTCAN KAVAKLI
Cc: dri-devel, dmitry.torokhov, linux-input, lee, robh, devicetree,
conor+dt, linux-leds
> From: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
>
> Enable the display subsystem (MDSS/MDP/DSI0), Kinetic KTD3136 backlight,
> and Novatek NT36672A SPI touchscreen on the Xiaomi Redmi Note 8 (ginkgo).
>
> Hardware nodes added:
> - Fixed LCDB regulators (lcdb_lab and lcdb_ibb at +/- 5.5V)
> - MDSS and DSI0 with Tianma 1080x2340 FHD+ video panel
> - I2C1 backlight node at 0x36 with PMI632 GPIO 6 hardware enable
> - SPI2 touchscreen node at CS0 with GPIO 88 interrupt and panel follower
> reference
> - Touchscreen active pinctrl definition on GPIO 88
>
> Signed-off-by: YİĞİTCAN KAVAKLI <kavakliyigitcan@gmail.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-ginkgo-submission-final-v1-0-0e68e63a18d4@gmail.com?part=10
^ permalink raw reply [flat|nested] 21+ messages in thread