* [PATCH v3 0/3] drm/panel: novatek-nt35560: Fix bug and clean up
@ 2025-07-30 6:17 Brigham Campbell
2025-07-30 6:17 ` [PATCH v3 1/3] drm/panel: novatek-nt35560: Fix invalid return value Brigham Campbell
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Brigham Campbell @ 2025-07-30 6:17 UTC (permalink / raw)
To: maarten.lankhorst, mripard, tzimmermann, airlied, simona,
linus.walleij, neil.armstrong, jessica.zhang, sam
Cc: dianders, skhan, linux-kernel-mentees, dri-devel, linux-kernel,
Brigham Campbell
Fix bug in novatek-nt35560 driver's nt35560_set_brightness() which
causes the driver to incorrectly report that it "failed to disable
display backlight".
Add mipi_dsi_dcs_read_multi() to drm_mipi_dsi.c for improved error
handling in drivers which use mipi_dsi_dcs_read() multiple times in a
row. Add mipi_dsi_dcs_write_var_seq_multi() and
mipi_dsi_generic_write_var_seq_multi() to drm_mipi_dsi.h to allow
drivers to more conveniently construct MIPI payloads at runtime.
Clean up novatek-nt35560 driver to use "multi" variants of MIPI.
Changes to v3:
- Fix incorrect Fixes tag. The bug was introduced by an earlier commit.
- Minor formatting improvements.
Changes to v2:
- Separate bug fix into its own commit for backporting.
- Add var_seq_multi() variants of MIPI write macros for sending
non-static MIPI messages.
- Minor formatting improvements.
Brigham Campbell (3):
drm/panel: novatek-nt35560: Fix invalid return value
drm: Add MIPI read_multi func and two write macros
drm/panel: novatek-nt35560: Clean up driver
drivers/gpu/drm/drm_mipi_dsi.c | 37 ++++
drivers/gpu/drm/panel/panel-novatek-nt35560.c | 198 ++++++------------
include/drm/drm_mipi_dsi.h | 35 ++++
3 files changed, 132 insertions(+), 138 deletions(-)
base-commit: 33f8f321e7aa7715ce19560801ee5223ba8b9a7d
--
2.50.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 1/3] drm/panel: novatek-nt35560: Fix invalid return value
2025-07-30 6:17 [PATCH v3 0/3] drm/panel: novatek-nt35560: Fix bug and clean up Brigham Campbell
@ 2025-07-30 6:17 ` Brigham Campbell
2025-08-18 9:24 ` Linus Walleij
2025-07-30 6:17 ` [PATCH v3 2/3] drm: Add MIPI read_multi func and two write macros Brigham Campbell
2025-07-30 6:17 ` [PATCH v3 3/3] drm/panel: novatek-nt35560: Clean up driver Brigham Campbell
2 siblings, 1 reply; 8+ messages in thread
From: Brigham Campbell @ 2025-07-30 6:17 UTC (permalink / raw)
To: maarten.lankhorst, mripard, tzimmermann, airlied, simona,
linus.walleij, neil.armstrong, jessica.zhang, sam
Cc: dianders, skhan, linux-kernel-mentees, dri-devel, linux-kernel,
Brigham Campbell
Fix bug in nt35560_set_brightness() which causes the function to
erroneously report an error. mipi_dsi_dcs_write() returns either a
negative value when an error occurred or a positive number of bytes
written when no error occurred. The buggy code reports an error under
either condition.
Fixes: 8152c2bfd780 ("drm/panel: Add driver for Sony ACX424AKP panel")
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Brigham Campbell <me@brighamcampbell.com>
---
drivers/gpu/drm/panel/panel-novatek-nt35560.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35560.c b/drivers/gpu/drm/panel/panel-novatek-nt35560.c
index 98f0782c8411..17898a29efe8 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt35560.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt35560.c
@@ -161,7 +161,7 @@ static int nt35560_set_brightness(struct backlight_device *bl)
par = 0x00;
ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_WRITE_CONTROL_DISPLAY,
&par, 1);
- if (ret) {
+ if (ret < 0) {
dev_err(nt->dev, "failed to disable display backlight (%d)\n", ret);
return ret;
}
--
2.50.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 2/3] drm: Add MIPI read_multi func and two write macros
2025-07-30 6:17 [PATCH v3 0/3] drm/panel: novatek-nt35560: Fix bug and clean up Brigham Campbell
2025-07-30 6:17 ` [PATCH v3 1/3] drm/panel: novatek-nt35560: Fix invalid return value Brigham Campbell
@ 2025-07-30 6:17 ` Brigham Campbell
2025-07-30 15:56 ` Doug Anderson
2025-07-30 6:17 ` [PATCH v3 3/3] drm/panel: novatek-nt35560: Clean up driver Brigham Campbell
2 siblings, 1 reply; 8+ messages in thread
From: Brigham Campbell @ 2025-07-30 6:17 UTC (permalink / raw)
To: maarten.lankhorst, mripard, tzimmermann, airlied, simona,
linus.walleij, neil.armstrong, jessica.zhang, sam
Cc: dianders, skhan, linux-kernel-mentees, dri-devel, linux-kernel,
Brigham Campbell
Create mipi_dsi_dcs_read_multi(), which accepts a mipi_dsi_multi_context
struct for improved error handling and cleaner panel driver code.
Create mipi_dsi_dcs_write_var_seq_multi() and
mipi_dsi_generic_write_var_seq_multi() macros which allow MIPI panel
drivers to write non-static data to display controllers.
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Brigham Campbell <me@brighamcampbell.com>
---
Doug, you had suggested a slightly different shortlog for this patch. I
adjusted your suggestion to fit within the canonical recommended
shortlog length of 50 characters. I understand that the 50 character
limit isn't a rule as much as it is a guideline, but the current
shortlog seems to me like a good compromise.
drivers/gpu/drm/drm_mipi_dsi.c | 37 ++++++++++++++++++++++++++++++++++
include/drm/drm_mipi_dsi.h | 35 ++++++++++++++++++++++++++++++++
2 files changed, 72 insertions(+)
diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index a00d76443128..0f2c3be98212 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -1075,6 +1075,43 @@ ssize_t mipi_dsi_dcs_read(struct mipi_dsi_device *dsi, u8 cmd, void *data,
}
EXPORT_SYMBOL(mipi_dsi_dcs_read);
+/**
+ * mipi_dsi_dcs_read_multi() - mipi_dsi_dcs_read() w/ accum_err
+ * @ctx: Context for multiple DSI transactions
+ * @cmd: DCS command
+ * @data: buffer in which to receive data
+ * @len: size of receive buffer
+ *
+ * Like mipi_dsi_dcs_read() but deals with errors in a way that makes it
+ * convenient to make several calls in a row.
+ */
+void mipi_dsi_dcs_read_multi(struct mipi_dsi_multi_context *ctx, u8 cmd,
+ void *data, size_t len)
+{
+ struct mipi_dsi_device *dsi = ctx->dsi;
+ struct device *dev = &dsi->dev;
+ struct mipi_dsi_msg msg = {
+ .channel = dsi->channel,
+ .type = MIPI_DSI_DCS_READ,
+ .tx_buf = &cmd,
+ .tx_len = 1,
+ .rx_buf = data,
+ .rx_len = len
+ };
+ ssize_t ret;
+
+ if (ctx->accum_err)
+ return;
+
+ ret = mipi_dsi_device_transfer(dsi, &msg);
+ if (ret < 0) {
+ ctx->accum_err = ret;
+ dev_err(dev, "dcs read with command %#x failed: %d\n", cmd,
+ ctx->accum_err);
+ }
+}
+EXPORT_SYMBOL(mipi_dsi_dcs_read_multi);
+
/**
* mipi_dsi_dcs_nop() - send DCS nop packet
* @dsi: DSI peripheral device
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 369b0d8830c3..296ffdc9cd02 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -333,6 +333,8 @@ ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, u8 cmd,
const void *data, size_t len);
ssize_t mipi_dsi_dcs_read(struct mipi_dsi_device *dsi, u8 cmd, void *data,
size_t len);
+void mipi_dsi_dcs_read_multi(struct mipi_dsi_multi_context *ctx, u8 cmd,
+ void *data, size_t len);
int mipi_dsi_dcs_nop(struct mipi_dsi_device *dsi);
int mipi_dsi_dcs_soft_reset(struct mipi_dsi_device *dsi);
int mipi_dsi_dcs_get_power_mode(struct mipi_dsi_device *dsi, u8 *mode);
@@ -415,6 +417,22 @@ void mipi_dsi_dcs_set_tear_off_multi(struct mipi_dsi_multi_context *ctx);
mipi_dsi_generic_write_multi(ctx, d, ARRAY_SIZE(d)); \
} while (0)
+/**
+ * mipi_dsi_generic_write_var_seq_multi - transmit non-constant data using a
+ * generic write packet
+ *
+ * This macro will print errors for you and error handling is optimized for
+ * callers that call this multiple times in a row.
+ *
+ * @ctx: Context for multiple DSI transactions
+ * @seq: buffer containing the payload
+ */
+#define mipi_dsi_generic_write_var_seq_multi(ctx, seq...) \
+ do { \
+ const u8 d[] = { seq }; \
+ mipi_dsi_generic_write_multi(ctx, d, ARRAY_SIZE(d)); \
+ } while (0)
+
/**
* mipi_dsi_dcs_write_seq_multi - transmit a DCS command with payload
*
@@ -431,6 +449,23 @@ void mipi_dsi_dcs_set_tear_off_multi(struct mipi_dsi_multi_context *ctx);
mipi_dsi_dcs_write_buffer_multi(ctx, d, ARRAY_SIZE(d)); \
} while (0)
+/**
+ * mipi_dsi_dcs_write_var_seq_multi - transmit a DCS command with non-static
+ * payload
+ *
+ * This macro will print errors for you and error handling is optimized for
+ * callers that call this multiple times in a row.
+ *
+ * @ctx: Context for multiple DSI transactions
+ * @cmd: Command
+ * @seq: buffer containing data to be transmitted
+ */
+#define mipi_dsi_dcs_write_var_seq_multi(ctx, cmd, seq...) \
+ do { \
+ const u8 d[] = { cmd, seq }; \
+ mipi_dsi_dcs_write_buffer_multi(ctx, d, ARRAY_SIZE(d)); \
+ } while (0)
+
/**
* struct mipi_dsi_driver - DSI driver
* @driver: device driver model driver
--
2.50.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 3/3] drm/panel: novatek-nt35560: Clean up driver
2025-07-30 6:17 [PATCH v3 0/3] drm/panel: novatek-nt35560: Fix bug and clean up Brigham Campbell
2025-07-30 6:17 ` [PATCH v3 1/3] drm/panel: novatek-nt35560: Fix invalid return value Brigham Campbell
2025-07-30 6:17 ` [PATCH v3 2/3] drm: Add MIPI read_multi func and two write macros Brigham Campbell
@ 2025-07-30 6:17 ` Brigham Campbell
2025-08-18 9:25 ` Linus Walleij
2 siblings, 1 reply; 8+ messages in thread
From: Brigham Campbell @ 2025-07-30 6:17 UTC (permalink / raw)
To: maarten.lankhorst, mripard, tzimmermann, airlied, simona,
linus.walleij, neil.armstrong, jessica.zhang, sam
Cc: dianders, skhan, linux-kernel-mentees, dri-devel, linux-kernel,
Brigham Campbell
Update driver to use the "multi" variants of MIPI functions which
facilitate improved error handling and cleaner driver code.
Remove information from a comment which was made obsolete by commit
994ea402c767 ("drm/panel: Rename Sony ACX424 to Novatek NT35560"), which
determined that this driver supports the Novatek NT35560 panel
controller.
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Brigham Campbell <me@brighamcampbell.com>
---
drivers/gpu/drm/panel/panel-novatek-nt35560.c | 198 ++++++------------
1 file changed, 60 insertions(+), 138 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35560.c b/drivers/gpu/drm/panel/panel-novatek-nt35560.c
index 17898a29efe8..561e6643dcbb 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt35560.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt35560.c
@@ -148,24 +148,20 @@ static inline struct nt35560 *panel_to_nt35560(struct drm_panel *panel)
static int nt35560_set_brightness(struct backlight_device *bl)
{
struct nt35560 *nt = bl_get_data(bl);
- struct mipi_dsi_device *dsi = to_mipi_dsi_device(nt->dev);
- int period_ns = 1023;
+ struct mipi_dsi_multi_context dsi_ctx = {
+ .dsi = to_mipi_dsi_device(nt->dev)
+ };
int duty_ns = bl->props.brightness;
+ int period_ns = 1023;
u8 pwm_ratio;
u8 pwm_div;
- u8 par;
- int ret;
if (backlight_is_blank(bl)) {
/* Disable backlight */
- par = 0x00;
- ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_WRITE_CONTROL_DISPLAY,
- &par, 1);
- if (ret < 0) {
- dev_err(nt->dev, "failed to disable display backlight (%d)\n", ret);
- return ret;
- }
- return 0;
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx,
+ MIPI_DCS_WRITE_CONTROL_DISPLAY,
+ 0x00);
+ return dsi_ctx.accum_err;
}
/* Calculate the PWM duty cycle in n/256's */
@@ -176,12 +172,6 @@ static int nt35560_set_brightness(struct backlight_device *bl)
/* Set up PWM dutycycle ONE byte (differs from the standard) */
dev_dbg(nt->dev, "calculated duty cycle %02x\n", pwm_ratio);
- ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_DISPLAY_BRIGHTNESS,
- &pwm_ratio, 1);
- if (ret < 0) {
- dev_err(nt->dev, "failed to set display PWM ratio (%d)\n", ret);
- return ret;
- }
/*
* Sequence to write PWMDIV:
@@ -192,46 +182,23 @@ static int nt35560_set_brightness(struct backlight_device *bl)
* 0x22 PWMDIV
* 0x7F 0xAA CMD2 page 1 lock
*/
- par = 0xaa;
- ret = mipi_dsi_dcs_write(dsi, 0xf3, &par, 1);
- if (ret < 0) {
- dev_err(nt->dev, "failed to unlock CMD 2 (%d)\n", ret);
- return ret;
- }
- par = 0x01;
- ret = mipi_dsi_dcs_write(dsi, 0x00, &par, 1);
- if (ret < 0) {
- dev_err(nt->dev, "failed to enter page 1 (%d)\n", ret);
- return ret;
- }
- par = 0x01;
- ret = mipi_dsi_dcs_write(dsi, 0x7d, &par, 1);
- if (ret < 0) {
- dev_err(nt->dev, "failed to disable MTP reload (%d)\n", ret);
- return ret;
- }
- ret = mipi_dsi_dcs_write(dsi, 0x22, &pwm_div, 1);
- if (ret < 0) {
- dev_err(nt->dev, "failed to set PWM divisor (%d)\n", ret);
- return ret;
- }
- par = 0xaa;
- ret = mipi_dsi_dcs_write(dsi, 0x7f, &par, 1);
- if (ret < 0) {
- dev_err(nt->dev, "failed to lock CMD 2 (%d)\n", ret);
- return ret;
- }
+ mipi_dsi_dcs_write_var_seq_multi(&dsi_ctx,
+ MIPI_DCS_SET_DISPLAY_BRIGHTNESS,
+ pwm_ratio);
+
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf3, 0xaa);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x00, 0x01);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7d, 0x01);
+
+ mipi_dsi_dcs_write_var_seq_multi(&dsi_ctx, 0x22, pwm_div);
+
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7f, 0xaa);
/* Enable backlight */
- par = 0x24;
- ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_WRITE_CONTROL_DISPLAY,
- &par, 1);
- if (ret < 0) {
- dev_err(nt->dev, "failed to enable display backlight (%d)\n", ret);
- return ret;
- }
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_WRITE_CONTROL_DISPLAY,
+ 0x24);
- return 0;
+ return dsi_ctx.accum_err;
}
static const struct backlight_ops nt35560_bl_ops = {
@@ -244,32 +211,23 @@ static const struct backlight_properties nt35560_bl_props = {
.max_brightness = 1023,
};
-static int nt35560_read_id(struct nt35560 *nt)
+static void nt35560_read_id(struct mipi_dsi_multi_context *dsi_ctx)
{
- struct mipi_dsi_device *dsi = to_mipi_dsi_device(nt->dev);
+ struct device dev = dsi_ctx->dsi->dev;
u8 vendor, version, panel;
u16 val;
- int ret;
- ret = mipi_dsi_dcs_read(dsi, NT35560_DCS_READ_ID1, &vendor, 1);
- if (ret < 0) {
- dev_err(nt->dev, "could not vendor ID byte\n");
- return ret;
- }
- ret = mipi_dsi_dcs_read(dsi, NT35560_DCS_READ_ID2, &version, 1);
- if (ret < 0) {
- dev_err(nt->dev, "could not read device version byte\n");
- return ret;
- }
- ret = mipi_dsi_dcs_read(dsi, NT35560_DCS_READ_ID3, &panel, 1);
- if (ret < 0) {
- dev_err(nt->dev, "could not read panel ID byte\n");
- return ret;
- }
+ mipi_dsi_dcs_read_multi(dsi_ctx, NT35560_DCS_READ_ID1, &vendor, 1);
+ mipi_dsi_dcs_read_multi(dsi_ctx, NT35560_DCS_READ_ID2, &version, 1);
+ mipi_dsi_dcs_read_multi(dsi_ctx, NT35560_DCS_READ_ID3, &panel, 1);
+
+ if (dsi_ctx->accum_err < 0)
+ return;
if (vendor == 0x00) {
- dev_err(nt->dev, "device vendor ID is zero\n");
- return -ENODEV;
+ dev_err(&dev, "device vendor ID is zero\n");
+ dsi_ctx->accum_err = -ENODEV;
+ return;
}
val = (vendor << 8) | panel;
@@ -278,16 +236,16 @@ static int nt35560_read_id(struct nt35560 *nt)
case DISPLAY_SONY_ACX424AKP_ID2:
case DISPLAY_SONY_ACX424AKP_ID3:
case DISPLAY_SONY_ACX424AKP_ID4:
- dev_info(nt->dev, "MTP vendor: %02x, version: %02x, panel: %02x\n",
+ dev_info(&dev,
+ "MTP vendor: %02x, version: %02x, panel: %02x\n",
vendor, version, panel);
break;
default:
- dev_info(nt->dev, "unknown vendor: %02x, version: %02x, panel: %02x\n",
+ dev_info(&dev,
+ "unknown vendor: %02x, version: %02x, panel: %02x\n",
vendor, version, panel);
break;
}
-
- return 0;
}
static int nt35560_power_on(struct nt35560 *nt)
@@ -322,92 +280,56 @@ static void nt35560_power_off(struct nt35560 *nt)
static int nt35560_prepare(struct drm_panel *panel)
{
struct nt35560 *nt = panel_to_nt35560(panel);
- struct mipi_dsi_device *dsi = to_mipi_dsi_device(nt->dev);
- const u8 mddi = 3;
+ struct mipi_dsi_multi_context dsi_ctx = {
+ .dsi = to_mipi_dsi_device(nt->dev)
+ };
int ret;
ret = nt35560_power_on(nt);
if (ret)
return ret;
- ret = nt35560_read_id(nt);
- if (ret) {
- dev_err(nt->dev, "failed to read panel ID (%d)\n", ret);
- goto err_power_off;
- }
+ nt35560_read_id(&dsi_ctx);
- /* Enabe tearing mode: send TE (tearing effect) at VBLANK */
- ret = mipi_dsi_dcs_set_tear_on(dsi,
+ /* Enable tearing mode: send TE (tearing effect) at VBLANK */
+ mipi_dsi_dcs_set_tear_on_multi(&dsi_ctx,
MIPI_DSI_DCS_TEAR_MODE_VBLANK);
- if (ret) {
- dev_err(nt->dev, "failed to enable vblank TE (%d)\n", ret);
- goto err_power_off;
- }
/*
* Set MDDI
*
* This presumably deactivates the Qualcomm MDDI interface and
* selects DSI, similar code is found in other drivers such as the
- * Sharp LS043T1LE01 which makes us suspect that this panel may be
- * using a Novatek NT35565 or similar display driver chip that shares
- * this command. Due to the lack of documentation we cannot know for
- * sure.
+ * Sharp LS043T1LE01.
*/
- ret = mipi_dsi_dcs_write(dsi, NT35560_DCS_SET_MDDI,
- &mddi, sizeof(mddi));
- if (ret < 0) {
- dev_err(nt->dev, "failed to set MDDI (%d)\n", ret);
- goto err_power_off;
- }
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, NT35560_DCS_SET_MDDI, 3);
- /* Exit sleep mode */
- ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
- if (ret) {
- dev_err(nt->dev, "failed to exit sleep mode (%d)\n", ret);
- goto err_power_off;
- }
- msleep(140);
+ mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+ mipi_dsi_msleep(&dsi_ctx, 140);
- ret = mipi_dsi_dcs_set_display_on(dsi);
- if (ret) {
- dev_err(nt->dev, "failed to turn display on (%d)\n", ret);
- goto err_power_off;
- }
+ mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
if (nt->video_mode) {
- /* In video mode turn peripheral on */
- ret = mipi_dsi_turn_on_peripheral(dsi);
- if (ret) {
- dev_err(nt->dev, "failed to turn on peripheral\n");
- goto err_power_off;
- }
+ mipi_dsi_turn_on_peripheral_multi(&dsi_ctx);
}
- return 0;
-
-err_power_off:
- nt35560_power_off(nt);
- return ret;
+ if (dsi_ctx.accum_err < 0)
+ nt35560_power_off(nt);
+ return dsi_ctx.accum_err;
}
static int nt35560_unprepare(struct drm_panel *panel)
{
struct nt35560 *nt = panel_to_nt35560(panel);
- struct mipi_dsi_device *dsi = to_mipi_dsi_device(nt->dev);
- int ret;
+ struct mipi_dsi_multi_context dsi_ctx = {
+ .dsi = to_mipi_dsi_device(nt->dev)
+ };
- ret = mipi_dsi_dcs_set_display_off(dsi);
- if (ret) {
- dev_err(nt->dev, "failed to turn display off (%d)\n", ret);
- return ret;
- }
+ mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
+ mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
+
+ if (dsi_ctx.accum_err < 0)
+ return dsi_ctx.accum_err;
- /* Enter sleep mode */
- ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
- if (ret) {
- dev_err(nt->dev, "failed to enter sleep mode (%d)\n", ret);
- return ret;
- }
msleep(85);
nt35560_power_off(nt);
--
2.50.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/3] drm: Add MIPI read_multi func and two write macros
2025-07-30 6:17 ` [PATCH v3 2/3] drm: Add MIPI read_multi func and two write macros Brigham Campbell
@ 2025-07-30 15:56 ` Doug Anderson
2025-07-31 0:49 ` Brigham Campbell
0 siblings, 1 reply; 8+ messages in thread
From: Doug Anderson @ 2025-07-30 15:56 UTC (permalink / raw)
To: Brigham Campbell
Cc: maarten.lankhorst, mripard, tzimmermann, airlied, simona,
linus.walleij, neil.armstrong, jessica.zhang, sam, skhan,
linux-kernel-mentees, dri-devel, linux-kernel
Hi,
On Tue, Jul 29, 2025 at 11:18 PM Brigham Campbell
<me@brighamcampbell.com> wrote:
>
> Create mipi_dsi_dcs_read_multi(), which accepts a mipi_dsi_multi_context
> struct for improved error handling and cleaner panel driver code.
>
> Create mipi_dsi_dcs_write_var_seq_multi() and
> mipi_dsi_generic_write_var_seq_multi() macros which allow MIPI panel
> drivers to write non-static data to display controllers.
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Brigham Campbell <me@brighamcampbell.com>
> ---
>
> Doug, you had suggested a slightly different shortlog for this patch. I
> adjusted your suggestion to fit within the canonical recommended
> shortlog length of 50 characters. I understand that the 50 character
> limit isn't a rule as much as it is a guideline, but the current
> shortlog seems to me like a good compromise.
Looks good.
> @@ -431,6 +449,23 @@ void mipi_dsi_dcs_set_tear_off_multi(struct mipi_dsi_multi_context *ctx);
> mipi_dsi_dcs_write_buffer_multi(ctx, d, ARRAY_SIZE(d)); \
> } while (0)
>
> +/**
> + * mipi_dsi_dcs_write_var_seq_multi - transmit a DCS command with non-static
> + * payload
I should have been explicit, but the above "non-static" should also be
"non-constant". ;-)
I could probably fix that when applying, or you could send a v4. Up to you.
Speaking of applying this, I'll be on vacation next week, so I won't
be able to apply the patches until the week after. That will also give
anyone else on the list a chance to comment if they want...
-Doug
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/3] drm: Add MIPI read_multi func and two write macros
2025-07-30 15:56 ` Doug Anderson
@ 2025-07-31 0:49 ` Brigham Campbell
0 siblings, 0 replies; 8+ messages in thread
From: Brigham Campbell @ 2025-07-31 0:49 UTC (permalink / raw)
To: Doug Anderson
Cc: maarten.lankhorst, mripard, tzimmermann, airlied, simona,
linus.walleij, neil.armstrong, jessica.zhang, sam, skhan,
linux-kernel-mentees, dri-devel, linux-kernel
On Wed Jul 30, 2025 at 9:56 AM MDT, Doug Anderson wrote:
>> +/**
>> + * mipi_dsi_dcs_write_var_seq_multi - transmit a DCS command with non-static
>> + * payload
>
> I should have been explicit, but the above "non-static" should also be
> "non-constant". ;-)
>
> I could probably fix that when applying, or you could send a v4. Up to you.
Oops. This obviously needed to change as well, but I tunnel-visioned
hard. I'll go ahead and fix it in v4.
Naturally, I wouldn't be at all opposed to you or any other maintainer
making such a small change to one of my patches as it heads upstream,
but I'd rather not ask you to remember to make that change after a long
vacation and a busy merge window. There's no need for me to add even a
little more cognitive load to your job than what's necessary.
> Speaking of applying this, I'll be on vacation next week, so I won't
> be able to apply the patches until the week after. That will also give
> anyone else on the list a chance to comment if they want...
Awesome! I'll plan to sit tight and act on whatever feedback I get on
v4.
If you happen to be flying over northern Utah to get to your
destination, look out the window. I'll wave as you go by.
Cheers,
Brigham
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/3] drm/panel: novatek-nt35560: Fix invalid return value
2025-07-30 6:17 ` [PATCH v3 1/3] drm/panel: novatek-nt35560: Fix invalid return value Brigham Campbell
@ 2025-08-18 9:24 ` Linus Walleij
0 siblings, 0 replies; 8+ messages in thread
From: Linus Walleij @ 2025-08-18 9:24 UTC (permalink / raw)
To: Brigham Campbell
Cc: maarten.lankhorst, mripard, tzimmermann, airlied, simona,
neil.armstrong, jessica.zhang, sam, dianders, skhan,
linux-kernel-mentees, dri-devel, linux-kernel
On Wed, Jul 30, 2025 at 8:17 AM Brigham Campbell <me@brighamcampbell.com> wrote:
> Fix bug in nt35560_set_brightness() which causes the function to
> erroneously report an error. mipi_dsi_dcs_write() returns either a
> negative value when an error occurred or a positive number of bytes
> written when no error occurred. The buggy code reports an error under
> either condition.
>
> Fixes: 8152c2bfd780 ("drm/panel: Add driver for Sony ACX424AKP panel")
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
> Signed-off-by: Brigham Campbell <me@brighamcampbell.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
(Maybe this is applied already...)
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 3/3] drm/panel: novatek-nt35560: Clean up driver
2025-07-30 6:17 ` [PATCH v3 3/3] drm/panel: novatek-nt35560: Clean up driver Brigham Campbell
@ 2025-08-18 9:25 ` Linus Walleij
0 siblings, 0 replies; 8+ messages in thread
From: Linus Walleij @ 2025-08-18 9:25 UTC (permalink / raw)
To: Brigham Campbell
Cc: maarten.lankhorst, mripard, tzimmermann, airlied, simona,
neil.armstrong, jessica.zhang, sam, dianders, skhan,
linux-kernel-mentees, dri-devel, linux-kernel
On Wed, Jul 30, 2025 at 8:18 AM Brigham Campbell <me@brighamcampbell.com> wrote:
> Update driver to use the "multi" variants of MIPI functions which
> facilitate improved error handling and cleaner driver code.
>
> Remove information from a comment which was made obsolete by commit
> 994ea402c767 ("drm/panel: Rename Sony ACX424 to Novatek NT35560"), which
> determined that this driver supports the Novatek NT35560 panel
> controller.
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Brigham Campbell <me@brighamcampbell.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
(Maybe this is applied already...)
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-08-18 9:25 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-30 6:17 [PATCH v3 0/3] drm/panel: novatek-nt35560: Fix bug and clean up Brigham Campbell
2025-07-30 6:17 ` [PATCH v3 1/3] drm/panel: novatek-nt35560: Fix invalid return value Brigham Campbell
2025-08-18 9:24 ` Linus Walleij
2025-07-30 6:17 ` [PATCH v3 2/3] drm: Add MIPI read_multi func and two write macros Brigham Campbell
2025-07-30 15:56 ` Doug Anderson
2025-07-31 0:49 ` Brigham Campbell
2025-07-30 6:17 ` [PATCH v3 3/3] drm/panel: novatek-nt35560: Clean up driver Brigham Campbell
2025-08-18 9:25 ` Linus Walleij
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).