| Message ID | 20240810045404.188146-3-tejasvipin76@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add more multi functions for streamlined error handling | expand |
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.
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; }
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(-)