* [PATCH 0/2] add more multi functions for streamlined error handling
@ 2024-08-10 4:54 Tejas Vipin
2024-08-10 4:54 ` [PATCH 1/2] drm/mipi-dsi: Add mipi_dsi_dcs_set_tear_scanline_multi Tejas Vipin
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Tejas Vipin @ 2024-08-10 4:54 UTC (permalink / raw)
To: maarten.lankhorst, mripard, tzimmermann, neil.armstrong, quic_jesszhan
Cc: dianders, airlied, daniel, dri-devel, linux-kernel, Tejas Vipin
This patch adds mipi_dsi_dcs_set_tear_scanline_multi to the list of multi
functions and uses it with other multi functions in the jdi-fhd-r63452
panel.
Tejas Vipin (2):
drm/mipi-dsi: Add mipi_dsi_dcs_set_tear_scanline_multi
drm/panel: jdi-fhd-r63452: transition to mipi_dsi wrapped functions
drivers/gpu/drm/drm_mipi_dsi.c | 31 +++++
drivers/gpu/drm/panel/panel-jdi-fhd-r63452.c | 125 ++++++-------------
include/drm/drm_mipi_dsi.h | 2 +
3 files changed, 72 insertions(+), 86 deletions(-)
--
2.46.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] drm/mipi-dsi: Add mipi_dsi_dcs_set_tear_scanline_multi 2024-08-10 4:54 [PATCH 0/2] add more multi functions for streamlined error handling Tejas Vipin @ 2024-08-10 4:54 ` Tejas Vipin 2024-08-12 22:58 ` Doug Anderson 2024-08-10 4:54 ` [PATCH 2/2] drm/panel: jdi-fhd-r63452: transition to mipi_dsi wrapped functions Tejas Vipin 2024-08-12 22:58 ` [PATCH 0/2] add more multi functions for streamlined error handling Doug Anderson 2 siblings, 1 reply; 6+ messages in thread From: Tejas Vipin @ 2024-08-10 4:54 UTC (permalink / raw) To: maarten.lankhorst, mripard, tzimmermann, neil.armstrong, quic_jesszhan Cc: dianders, airlied, daniel, dri-devel, linux-kernel, Tejas Vipin mipi_dsi_dcs_set_tear_scanline_multi can heavily benefit from being converted to a multi style function as it is often called in the context of similar functions. Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com> --- drivers/gpu/drm/drm_mipi_dsi.c | 31 +++++++++++++++++++++++++++++++ include/drm/drm_mipi_dsi.h | 2 ++ 2 files changed, 33 insertions(+) diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c index 8d0a866cf1e0..b7ad18c148c2 100644 --- a/drivers/gpu/drm/drm_mipi_dsi.c +++ b/drivers/gpu/drm/drm_mipi_dsi.c @@ -1339,6 +1339,9 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format); * @dsi: DSI peripheral device * @scanline: scanline to use as trigger * + * This function is deprecated. Use mipi_dsi_dcs_set_tear_scanline_multi() + * instead. + * * Return: 0 on success or a negative error code on failure */ int mipi_dsi_dcs_set_tear_scanline(struct mipi_dsi_device *dsi, u16 scanline) @@ -1833,6 +1836,34 @@ void mipi_dsi_dcs_set_page_address_multi(struct mipi_dsi_multi_context *ctx, } EXPORT_SYMBOL(mipi_dsi_dcs_set_page_address_multi); +/** + * mipi_dsi_dcs_set_tear_scanline_multi() - set the scanline to use as trigger for + * the Tearing Effect output signal of the display module + * @ctx: Context for multiple DSI transactions + * @scanline: scanline to use as trigger + * + * Like mipi_dsi_dcs_set_tear_scanline() but deals with errors in a way that + * makes it convenient to make several calls in a row. + */ +void mipi_dsi_dcs_set_tear_scanline_multi(struct mipi_dsi_multi_context *ctx, + u16 scanline) +{ + struct mipi_dsi_device *dsi = ctx->dsi; + struct device *dev = &dsi->dev; + int ret; + + if (ctx->accum_err) + return; + + ret = mipi_dsi_dcs_set_tear_scanline(dsi, scanline); + if (ret < 0) { + ctx->accum_err = ret; + dev_err(dev, "Failed to set tear scanline: %d\n", + ctx->accum_err); + } +} +EXPORT_SYMBOL(mipi_dsi_dcs_set_tear_scanline_multi); + static int mipi_dsi_drv_probe(struct device *dev) { struct mipi_dsi_driver *drv = to_mipi_dsi_driver(dev->driver); diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h index 602be6ce081a..c823cc13ad1f 100644 --- a/include/drm/drm_mipi_dsi.h +++ b/include/drm/drm_mipi_dsi.h @@ -375,6 +375,8 @@ void mipi_dsi_dcs_set_column_address_multi(struct mipi_dsi_multi_context *ctx, u16 start, u16 end); void mipi_dsi_dcs_set_page_address_multi(struct mipi_dsi_multi_context *ctx, u16 start, u16 end); +void mipi_dsi_dcs_set_tear_scanline_multi(struct mipi_dsi_multi_context *ctx, + u16 scanline); /** * mipi_dsi_generic_write_seq - transmit data using a generic write packet -- 2.46.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] drm/mipi-dsi: Add mipi_dsi_dcs_set_tear_scanline_multi 2024-08-10 4:54 ` [PATCH 1/2] drm/mipi-dsi: Add mipi_dsi_dcs_set_tear_scanline_multi Tejas Vipin @ 2024-08-12 22:58 ` Doug Anderson 0 siblings, 0 replies; 6+ messages in thread From: Doug Anderson @ 2024-08-12 22:58 UTC (permalink / raw) To: Tejas Vipin Cc: maarten.lankhorst, mripard, tzimmermann, neil.armstrong, quic_jesszhan, airlied, daniel, dri-devel, linux-kernel Hi, On Fri, Aug 9, 2024 at 9:55 PM Tejas Vipin <tejasvipin76@gmail.com> wrote: > > mipi_dsi_dcs_set_tear_scanline_multi can heavily benefit from being > converted to a multi style function as it is often called in the context of > similar functions. > > Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com> > --- > drivers/gpu/drm/drm_mipi_dsi.c | 31 +++++++++++++++++++++++++++++++ > include/drm/drm_mipi_dsi.h | 2 ++ > 2 files changed, 33 insertions(+) Reviewed-by: Douglas Anderson <dianders@chromium.org> ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] drm/panel: jdi-fhd-r63452: transition to mipi_dsi wrapped functions 2024-08-10 4:54 [PATCH 0/2] add more multi functions for streamlined error handling Tejas Vipin 2024-08-10 4:54 ` [PATCH 1/2] drm/mipi-dsi: Add mipi_dsi_dcs_set_tear_scanline_multi Tejas Vipin @ 2024-08-10 4:54 ` Tejas Vipin 2024-08-12 22:59 ` Doug Anderson 2024-08-12 22:58 ` [PATCH 0/2] add more multi functions for streamlined error handling Doug Anderson 2 siblings, 1 reply; 6+ messages in thread From: Tejas Vipin @ 2024-08-10 4:54 UTC (permalink / raw) To: maarten.lankhorst, mripard, tzimmermann, neil.armstrong, quic_jesszhan Cc: dianders, airlied, daniel, dri-devel, linux-kernel, Tejas Vipin Changes the jdi-fhd-r63452 panel to use multi style functions for improved error handling. Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com> --- drivers/gpu/drm/panel/panel-jdi-fhd-r63452.c | 125 ++++++------------- 1 file changed, 39 insertions(+), 86 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-jdi-fhd-r63452.c b/drivers/gpu/drm/panel/panel-jdi-fhd-r63452.c index 483dc88d16d8..32a244d4bae7 100644 --- a/drivers/gpu/drm/panel/panel-jdi-fhd-r63452.c +++ b/drivers/gpu/drm/panel/panel-jdi-fhd-r63452.c @@ -41,79 +41,41 @@ static void jdi_fhd_r63452_reset(struct jdi_fhd_r63452 *ctx) static int jdi_fhd_r63452_on(struct jdi_fhd_r63452 *ctx) { struct mipi_dsi_device *dsi = ctx->dsi; - struct device *dev = &dsi->dev; - int ret; + struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi }; dsi->mode_flags |= MIPI_DSI_MODE_LPM; - mipi_dsi_generic_write_seq(dsi, 0xb0, 0x00); - mipi_dsi_generic_write_seq(dsi, 0xd6, 0x01); - mipi_dsi_generic_write_seq(dsi, 0xec, - 0x64, 0xdc, 0xec, 0x3b, 0x52, 0x00, 0x0b, 0x0b, - 0x13, 0x15, 0x68, 0x0b, 0xb5); - mipi_dsi_generic_write_seq(dsi, 0xb0, 0x03); + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb0, 0x00); + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xd6, 0x01); + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xec, + 0x64, 0xdc, 0xec, 0x3b, 0x52, 0x00, 0x0b, 0x0b, + 0x13, 0x15, 0x68, 0x0b, 0xb5); + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb0, 0x03); - ret = mipi_dsi_dcs_set_tear_on(dsi, MIPI_DSI_DCS_TEAR_MODE_VBLANK); - if (ret < 0) { - dev_err(dev, "Failed to set tear on: %d\n", ret); - return ret; - } + mipi_dsi_dcs_set_tear_on_multi(&dsi_ctx, MIPI_DSI_DCS_TEAR_MODE_VBLANK); - mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_SET_ADDRESS_MODE, 0x00); + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_SET_ADDRESS_MODE, 0x00); - ret = mipi_dsi_dcs_set_pixel_format(dsi, 0x77); - if (ret < 0) { - dev_err(dev, "Failed to set pixel format: %d\n", ret); - return ret; - } + mipi_dsi_dcs_set_pixel_format_multi(&dsi_ctx, 0x77); + mipi_dsi_dcs_set_column_address_multi(&dsi_ctx, 0x0000, 0x0437); + mipi_dsi_dcs_set_page_address_multi(&dsi_ctx, 0x0000, 0x077f); + mipi_dsi_dcs_set_tear_scanline_multi(&dsi_ctx, 0x0000); + mipi_dsi_dcs_set_display_brightness_multi(&dsi_ctx, 0x00ff); - ret = mipi_dsi_dcs_set_column_address(dsi, 0x0000, 0x0437); - if (ret < 0) { - dev_err(dev, "Failed to set column address: %d\n", ret); - return ret; - } + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_WRITE_CONTROL_DISPLAY, 0x24); + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_WRITE_POWER_SAVE, 0x00); + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_SET_CABC_MIN_BRIGHTNESS, 0x00); + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x84, 0x00); - ret = mipi_dsi_dcs_set_page_address(dsi, 0x0000, 0x077f); - if (ret < 0) { - dev_err(dev, "Failed to set page address: %d\n", ret); - return ret; - } - - ret = mipi_dsi_dcs_set_tear_scanline(dsi, 0x0000); - if (ret < 0) { - dev_err(dev, "Failed to set tear scanline: %d\n", ret); - return ret; - } + mipi_dsi_dcs_set_display_on_multi(&dsi_ctx); + mipi_dsi_msleep(&dsi_ctx, 20); + mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx); + mipi_dsi_msleep(&dsi_ctx, 80); - ret = mipi_dsi_dcs_set_display_brightness(dsi, 0x00ff); - if (ret < 0) { - dev_err(dev, "Failed to set display brightness: %d\n", ret); - return ret; - } - - mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_WRITE_CONTROL_DISPLAY, 0x24); - mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_WRITE_POWER_SAVE, 0x00); - mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_SET_CABC_MIN_BRIGHTNESS, 0x00); - mipi_dsi_dcs_write_seq(dsi, 0x84, 0x00); - - ret = mipi_dsi_dcs_set_display_on(dsi); - if (ret < 0) { - dev_err(dev, "Failed to set display on: %d\n", ret); - return ret; - } - msleep(20); - - ret = mipi_dsi_dcs_exit_sleep_mode(dsi); - if (ret < 0) { - dev_err(dev, "Failed to exit sleep mode: %d\n", ret); - return ret; - } - msleep(80); - - mipi_dsi_generic_write_seq(dsi, 0xb0, 0x04); - mipi_dsi_dcs_write_seq(dsi, 0x84, 0x00); - mipi_dsi_generic_write_seq(dsi, 0xc8, 0x11); - mipi_dsi_generic_write_seq(dsi, 0xb0, 0x03); + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb0, 0x04); + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x84, 0x00); + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xc8, 0x11); + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb0, 0x03); return 0; } @@ -121,31 +83,22 @@ static int jdi_fhd_r63452_on(struct jdi_fhd_r63452 *ctx) static int jdi_fhd_r63452_off(struct jdi_fhd_r63452 *ctx) { struct mipi_dsi_device *dsi = ctx->dsi; - struct device *dev = &dsi->dev; - int ret; + struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi }; dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; - mipi_dsi_generic_write_seq(dsi, 0xb0, 0x00); - mipi_dsi_generic_write_seq(dsi, 0xd6, 0x01); - mipi_dsi_generic_write_seq(dsi, 0xec, - 0x64, 0xdc, 0xec, 0x3b, 0x52, 0x00, 0x0b, 0x0b, - 0x13, 0x15, 0x68, 0x0b, 0x95); - mipi_dsi_generic_write_seq(dsi, 0xb0, 0x03); - - ret = mipi_dsi_dcs_set_display_off(dsi); - if (ret < 0) { - dev_err(dev, "Failed to set display off: %d\n", ret); - return ret; - } - usleep_range(2000, 3000); - - ret = mipi_dsi_dcs_enter_sleep_mode(dsi); - if (ret < 0) { - dev_err(dev, "Failed to enter sleep mode: %d\n", ret); - return ret; - } - msleep(120); + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb0, 0x00); + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xd6, 0x01); + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xec, + 0x64, 0xdc, 0xec, 0x3b, 0x52, 0x00, 0x0b, 0x0b, + 0x13, 0x15, 0x68, 0x0b, 0x95); + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb0, 0x03); + + mipi_dsi_dcs_set_display_off_multi(&dsi_ctx); + if (!dsi_ctx.accum_err) + usleep_range(2000, 3000); + mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx); + mipi_dsi_msleep(&dsi_ctx, 120); return 0; } -- 2.46.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] drm/panel: jdi-fhd-r63452: transition to mipi_dsi wrapped functions 2024-08-10 4:54 ` [PATCH 2/2] drm/panel: jdi-fhd-r63452: transition to mipi_dsi wrapped functions Tejas Vipin @ 2024-08-12 22:59 ` Doug Anderson 0 siblings, 0 replies; 6+ messages in thread From: Doug Anderson @ 2024-08-12 22:59 UTC (permalink / raw) To: Tejas Vipin Cc: maarten.lankhorst, mripard, tzimmermann, neil.armstrong, quic_jesszhan, airlied, daniel, dri-devel, linux-kernel Hi, On Fri, Aug 9, 2024 at 9:55 PM Tejas Vipin <tejasvipin76@gmail.com> wrote: > > @@ -41,79 +41,41 @@ static void jdi_fhd_r63452_reset(struct jdi_fhd_r63452 *ctx) > static int jdi_fhd_r63452_on(struct jdi_fhd_r63452 *ctx) > { > struct mipi_dsi_device *dsi = ctx->dsi; > - struct device *dev = &dsi->dev; > - int ret; > + struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi }; > > dsi->mode_flags |= MIPI_DSI_MODE_LPM; > > - mipi_dsi_generic_write_seq(dsi, 0xb0, 0x00); > - mipi_dsi_generic_write_seq(dsi, 0xd6, 0x01); > - mipi_dsi_generic_write_seq(dsi, 0xec, > - 0x64, 0xdc, 0xec, 0x3b, 0x52, 0x00, 0x0b, 0x0b, > - 0x13, 0x15, 0x68, 0x0b, 0xb5); > - mipi_dsi_generic_write_seq(dsi, 0xb0, 0x03); > + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb0, 0x00); > + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xd6, 0x01); > + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xec, > + 0x64, 0xdc, 0xec, 0x3b, 0x52, 0x00, 0x0b, 0x0b, > + 0x13, 0x15, 0x68, 0x0b, 0xb5); > + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb0, 0x03); > > - ret = mipi_dsi_dcs_set_tear_on(dsi, MIPI_DSI_DCS_TEAR_MODE_VBLANK); > - if (ret < 0) { > - dev_err(dev, "Failed to set tear on: %d\n", ret); > - return ret; > - } > + mipi_dsi_dcs_set_tear_on_multi(&dsi_ctx, MIPI_DSI_DCS_TEAR_MODE_VBLANK); > > - mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_SET_ADDRESS_MODE, 0x00); > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_SET_ADDRESS_MODE, 0x00); > > - ret = mipi_dsi_dcs_set_pixel_format(dsi, 0x77); > - if (ret < 0) { > - dev_err(dev, "Failed to set pixel format: %d\n", ret); > - return ret; > - } > + mipi_dsi_dcs_set_pixel_format_multi(&dsi_ctx, 0x77); > + mipi_dsi_dcs_set_column_address_multi(&dsi_ctx, 0x0000, 0x0437); > + mipi_dsi_dcs_set_page_address_multi(&dsi_ctx, 0x0000, 0x077f); > + mipi_dsi_dcs_set_tear_scanline_multi(&dsi_ctx, 0x0000); > + mipi_dsi_dcs_set_display_brightness_multi(&dsi_ctx, 0x00ff); > > - ret = mipi_dsi_dcs_set_column_address(dsi, 0x0000, 0x0437); > - if (ret < 0) { > - dev_err(dev, "Failed to set column address: %d\n", ret); > - return ret; > - } > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_WRITE_CONTROL_DISPLAY, 0x24); > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_WRITE_POWER_SAVE, 0x00); > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_SET_CABC_MIN_BRIGHTNESS, 0x00); > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x84, 0x00); > > - ret = mipi_dsi_dcs_set_page_address(dsi, 0x0000, 0x077f); > - if (ret < 0) { > - dev_err(dev, "Failed to set page address: %d\n", ret); > - return ret; > - } > - > - ret = mipi_dsi_dcs_set_tear_scanline(dsi, 0x0000); > - if (ret < 0) { > - dev_err(dev, "Failed to set tear scanline: %d\n", ret); > - return ret; > - } > + mipi_dsi_dcs_set_display_on_multi(&dsi_ctx); > + mipi_dsi_msleep(&dsi_ctx, 20); > + mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx); > + mipi_dsi_msleep(&dsi_ctx, 80); > > - ret = mipi_dsi_dcs_set_display_brightness(dsi, 0x00ff); > - if (ret < 0) { > - dev_err(dev, "Failed to set display brightness: %d\n", ret); > - return ret; > - } > - > - mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_WRITE_CONTROL_DISPLAY, 0x24); > - mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_WRITE_POWER_SAVE, 0x00); > - mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_SET_CABC_MIN_BRIGHTNESS, 0x00); > - mipi_dsi_dcs_write_seq(dsi, 0x84, 0x00); > - > - ret = mipi_dsi_dcs_set_display_on(dsi); > - if (ret < 0) { > - dev_err(dev, "Failed to set display on: %d\n", ret); > - return ret; > - } > - msleep(20); > - > - ret = mipi_dsi_dcs_exit_sleep_mode(dsi); > - if (ret < 0) { > - dev_err(dev, "Failed to exit sleep mode: %d\n", ret); > - return ret; > - } > - msleep(80); > - > - mipi_dsi_generic_write_seq(dsi, 0xb0, 0x04); > - mipi_dsi_dcs_write_seq(dsi, 0x84, 0x00); > - mipi_dsi_generic_write_seq(dsi, 0xc8, 0x11); > - mipi_dsi_generic_write_seq(dsi, 0xb0, 0x03); > + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb0, 0x04); > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x84, 0x00); > + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xc8, 0x11); > + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb0, 0x03); > > return 0; Whoops! Not "return 0". "return dsi_ctx.accum_err". > @@ -121,31 +83,22 @@ static int jdi_fhd_r63452_on(struct jdi_fhd_r63452 *ctx) > static int jdi_fhd_r63452_off(struct jdi_fhd_r63452 *ctx) > { > struct mipi_dsi_device *dsi = ctx->dsi; > - struct device *dev = &dsi->dev; > - int ret; > + struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi }; > > dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; > > - mipi_dsi_generic_write_seq(dsi, 0xb0, 0x00); > - mipi_dsi_generic_write_seq(dsi, 0xd6, 0x01); > - mipi_dsi_generic_write_seq(dsi, 0xec, > - 0x64, 0xdc, 0xec, 0x3b, 0x52, 0x00, 0x0b, 0x0b, > - 0x13, 0x15, 0x68, 0x0b, 0x95); > - mipi_dsi_generic_write_seq(dsi, 0xb0, 0x03); > - > - ret = mipi_dsi_dcs_set_display_off(dsi); > - if (ret < 0) { > - dev_err(dev, "Failed to set display off: %d\n", ret); > - return ret; > - } > - usleep_range(2000, 3000); > - > - ret = mipi_dsi_dcs_enter_sleep_mode(dsi); > - if (ret < 0) { > - dev_err(dev, "Failed to enter sleep mode: %d\n", ret); > - return ret; > - } > - msleep(120); > + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb0, 0x00); > + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xd6, 0x01); > + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xec, > + 0x64, 0xdc, 0xec, 0x3b, 0x52, 0x00, 0x0b, 0x0b, > + 0x13, 0x15, 0x68, 0x0b, 0x95); > + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb0, 0x03); > + > + mipi_dsi_dcs_set_display_off_multi(&dsi_ctx); > + if (!dsi_ctx.accum_err) > + usleep_range(2000, 3000); > + mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx); > + mipi_dsi_msleep(&dsi_ctx, 120); > > return 0; Whoops! Not "return 0". "return dsi_ctx.accum_err". Aside from that, this looks really nice to me. The code is much more succinct and I bet much smaller. FWIW, I won't insist, but I wouldn't object to this patch also fixing the callers of jdi_fhd_r63452_on() and jdi_fhd_r63452_off() so that they no longer print error messages since the _multi functions are always chatty and thus they're just extra double-prints. If you do this, jdi_fhd_r63452_off() could actually be a function that returned "void". ...then you might want to add a comment saying why jdi_fhd_r63452_unprepare() doesn't pass on any errors. That would be something like "NOTE: even if sending one of the poweroff commands failed, we won't return an error here. While the panel won't have been cleanly turned off at least we've asserted the reset signal so it should be safe to power it back on again later". ...or something like that. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] add more multi functions for streamlined error handling 2024-08-10 4:54 [PATCH 0/2] add more multi functions for streamlined error handling Tejas Vipin 2024-08-10 4:54 ` [PATCH 1/2] drm/mipi-dsi: Add mipi_dsi_dcs_set_tear_scanline_multi Tejas Vipin 2024-08-10 4:54 ` [PATCH 2/2] drm/panel: jdi-fhd-r63452: transition to mipi_dsi wrapped functions Tejas Vipin @ 2024-08-12 22:58 ` Doug Anderson 2 siblings, 0 replies; 6+ messages in thread From: Doug Anderson @ 2024-08-12 22:58 UTC (permalink / raw) To: Tejas Vipin Cc: maarten.lankhorst, mripard, tzimmermann, neil.armstrong, quic_jesszhan, airlied, daniel, dri-devel, linux-kernel Hi, On Fri, Aug 9, 2024 at 9:55 PM Tejas Vipin <tejasvipin76@gmail.com> wrote: > > This patch adds mipi_dsi_dcs_set_tear_scanline_multi to the list of multi > functions and uses it with other multi functions in the jdi-fhd-r63452 > panel. > > Tejas Vipin (2): > drm/mipi-dsi: Add mipi_dsi_dcs_set_tear_scanline_multi > drm/panel: jdi-fhd-r63452: transition to mipi_dsi wrapped functions > > drivers/gpu/drm/drm_mipi_dsi.c | 31 +++++ > drivers/gpu/drm/panel/panel-jdi-fhd-r63452.c | 125 ++++++------------- > include/drm/drm_mipi_dsi.h | 2 + > 3 files changed, 72 insertions(+), 86 deletions(-) Not worth spinning just for this, but a few comments: 1. For the cover letter, it's better if you can make the subject more different than the subject of your previous patch series. Comparing this and the previous series you sent out side-by-side: [PATCH 0/2] add more multi functions for streamlined error handling [PATCH v3 0/2] add more multi functions to streamline error handling Maybe this patch's cover letter should have a subject more like: drm/mipi-dsi: convert jdi-fhd-r63452 to mipi_dsi "multi", adding more "multi" ...or something like that. 2. In your cover letter you should note that this series only applies cleanly if you apply it atop your previous series. You should point to it w/ lore links based on the Message-Id, like: https://lore.kernel.org/r/20240806135949.468636-1-tejasvipin76@gmail.com ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-08-12 22:59 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-08-10 4:54 [PATCH 0/2] add more multi functions for streamlined error handling Tejas Vipin 2024-08-10 4:54 ` [PATCH 1/2] drm/mipi-dsi: Add mipi_dsi_dcs_set_tear_scanline_multi Tejas Vipin 2024-08-12 22:58 ` Doug Anderson 2024-08-10 4:54 ` [PATCH 2/2] drm/panel: jdi-fhd-r63452: transition to mipi_dsi wrapped functions Tejas Vipin 2024-08-12 22:59 ` Doug Anderson 2024-08-12 22:58 ` [PATCH 0/2] add more multi functions for streamlined error handling Doug Anderson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®