drivers/gpu/drm/panel/panel-novatek-nt35950.c | 214 ++++++------------ 1 file changed, 70 insertions(+), 144 deletions(-)
Changes the novatek-nt35950 panel to use multi style functions for
improved error handling.
Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
---
drivers/gpu/drm/panel/panel-novatek-nt35950.c | 214 ++++++------------
1 file changed, 70 insertions(+), 144 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35950.c b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
index 028fdac293f7..fa4db7a3bc25 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt35950.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
@@ -100,106 +100,89 @@ static void nt35950_reset(struct nt35950 *nt)
/*
* nt35950_set_cmd2_page - Select manufacturer control (CMD2) page
+ * @dsi_ctx: context for mipi_dsi functions
* @nt: Main driver structure
* @page: Page number (0-7)
- *
- * Return: Number of transferred bytes or negative number on error
*/
-static int nt35950_set_cmd2_page(struct nt35950 *nt, u8 page)
+static void nt35950_set_cmd2_page(struct mipi_dsi_multi_context *dsi_ctx,
+ struct nt35950 *nt, u8 page)
{
const u8 mauc_cmd2_page[] = { MCS_CMD_MAUCCTR, 0x55, 0xaa, 0x52,
0x08, page };
- int ret;
- ret = mipi_dsi_dcs_write_buffer(nt->dsi[0], mauc_cmd2_page,
+ mipi_dsi_dcs_write_buffer_multi(dsi_ctx, mauc_cmd2_page,
ARRAY_SIZE(mauc_cmd2_page));
- if (ret < 0)
- return ret;
+ if (dsi_ctx->accum_err)
+ return;
nt->last_page = page;
- return 0;
}
/*
* nt35950_set_data_compression - Set data compression mode
+ * @dsi_ctx: context for mipi_dsi functions
* @nt: Main driver structure
* @comp_mode: Compression mode
- *
- * Return: Number of transferred bytes or negative number on error
*/
-static int nt35950_set_data_compression(struct nt35950 *nt, u8 comp_mode)
+static void nt35950_set_data_compression(struct mipi_dsi_multi_context *dsi_ctx,
+ struct nt35950 *nt, u8 comp_mode)
{
u8 cmd_data_compression[] = { MCS_PARAM_DATA_COMPRESSION, comp_mode };
u8 cmd_vesa_dsc_on[] = { MCS_PARAM_VESA_DSC_ON, !!comp_mode };
u8 cmd_vesa_dsc_setting[] = { MCS_PARAM_VESA_DSC_SETTING, 0x03 };
u8 last_page = nt->last_page;
- int ret;
/* Set CMD2 Page 0 if we're not there yet */
- if (last_page != 0) {
- ret = nt35950_set_cmd2_page(nt, 0);
- if (ret < 0)
- return ret;
- }
+ if (last_page != 0)
+ nt35950_set_cmd2_page(dsi_ctx, nt, 0);
- ret = mipi_dsi_dcs_write_buffer(nt->dsi[0], cmd_data_compression,
+ mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd_data_compression,
ARRAY_SIZE(cmd_data_compression));
- if (ret < 0)
- return ret;
-
- ret = mipi_dsi_dcs_write_buffer(nt->dsi[0], cmd_vesa_dsc_on,
+ mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd_vesa_dsc_on,
ARRAY_SIZE(cmd_vesa_dsc_on));
- if (ret < 0)
- return ret;
/* Set the vesa dsc setting on Page 4 */
- ret = nt35950_set_cmd2_page(nt, 4);
- if (ret < 0)
- return ret;
+ nt35950_set_cmd2_page(dsi_ctx, nt, 4);
/* Display Stream Compression setting, always 0x03 */
- ret = mipi_dsi_dcs_write_buffer(nt->dsi[0], cmd_vesa_dsc_setting,
+ mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd_vesa_dsc_setting,
ARRAY_SIZE(cmd_vesa_dsc_setting));
- if (ret < 0)
- return ret;
/* Get back to the previously set page */
- return nt35950_set_cmd2_page(nt, last_page);
+ nt35950_set_cmd2_page(dsi_ctx, nt, last_page);
}
/*
* nt35950_set_scaler - Enable/disable resolution upscaling
- * @nt: Main driver structure
+ * @dsi_ctx: context for mipi_dsi functions
* @scale_up: Scale up function control
- *
- * Return: Number of transferred bytes or negative number on error
*/
-static int nt35950_set_scaler(struct nt35950 *nt, u8 scale_up)
+static void nt35950_set_scaler(struct mipi_dsi_multi_context *dsi_ctx,
+ u8 scale_up)
{
u8 cmd_scaler[] = { MCS_PARAM_SCALER_FUNCTION, scale_up };
- return mipi_dsi_dcs_write_buffer(nt->dsi[0], cmd_scaler,
- ARRAY_SIZE(cmd_scaler));
+ mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd_scaler,
+ ARRAY_SIZE(cmd_scaler));
}
/*
* nt35950_set_scale_mode - Resolution upscaling mode
- * @nt: Main driver structure
+ * @dsi_ctx: context for mipi_dsi functions
* @mode: Scaler mode (MCS_DATA_COMPRESSION_*)
- *
- * Return: Number of transferred bytes or negative number on error
*/
-static int nt35950_set_scale_mode(struct nt35950 *nt, u8 mode)
+static void nt35950_set_scale_mode(struct mipi_dsi_multi_context *dsi_ctx,
+ u8 mode)
{
u8 cmd_scaler[] = { MCS_PARAM_SCALEUP_MODE, mode };
- return mipi_dsi_dcs_write_buffer(nt->dsi[0], cmd_scaler,
- ARRAY_SIZE(cmd_scaler));
+ mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd_scaler,
+ ARRAY_SIZE(cmd_scaler));
}
/*
* nt35950_inject_black_image - Display a completely black image
- * @nt: Main driver structure
+ * @dsi_ctx: context for mipi_dsi functions
*
* After IC setup, the attached panel may show random data
* due to driveric behavior changes (resolution, compression,
@@ -208,43 +191,34 @@ static int nt35950_set_scale_mode(struct nt35950 *nt, u8 mode)
* the display.
* It makes sense to push a black image before sending the sleep-out
* and display-on commands.
- *
- * Return: Number of transferred bytes or negative number on error
*/
-static int nt35950_inject_black_image(struct nt35950 *nt)
+static void nt35950_inject_black_image(struct mipi_dsi_multi_context *dsi_ctx)
{
const u8 cmd0_black_img[] = { 0x6f, 0x01 };
const u8 cmd1_black_img[] = { 0xf3, 0x10 };
u8 cmd_test[] = { 0xff, 0xaa, 0x55, 0xa5, 0x80 };
- int ret;
/* Enable test command */
- ret = mipi_dsi_dcs_write_buffer(nt->dsi[0], cmd_test, ARRAY_SIZE(cmd_test));
- if (ret < 0)
- return ret;
+ mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd_test, ARRAY_SIZE(cmd_test));
/* Send a black image */
- ret = mipi_dsi_dcs_write_buffer(nt->dsi[0], cmd0_black_img,
+ mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd0_black_img,
ARRAY_SIZE(cmd0_black_img));
- if (ret < 0)
- return ret;
- ret = mipi_dsi_dcs_write_buffer(nt->dsi[0], cmd1_black_img,
+ mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd1_black_img,
ARRAY_SIZE(cmd1_black_img));
- if (ret < 0)
- return ret;
/* Disable test command */
cmd_test[ARRAY_SIZE(cmd_test) - 1] = 0x00;
- return mipi_dsi_dcs_write_buffer(nt->dsi[0], cmd_test, ARRAY_SIZE(cmd_test));
+ mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd_test, ARRAY_SIZE(cmd_test));
}
/*
* nt35950_set_dispout - Set Display Output register parameters
* @nt: Main driver structure
- *
- * Return: Number of transferred bytes or negative number on error
+ * @dsi_ctx: context for mipi_dsi functions
*/
-static int nt35950_set_dispout(struct nt35950 *nt)
+static void nt35950_set_dispout(struct mipi_dsi_multi_context *dsi_ctx,
+ struct nt35950 *nt)
{
u8 cmd_dispout[] = { MCS_PARAM_DISP_OUTPUT_CTRL, 0x00 };
const struct nt35950_panel_mode *mode_data = nt->desc->mode_data;
@@ -254,8 +228,8 @@ static int nt35950_set_dispout(struct nt35950 *nt)
if (mode_data[nt->cur_mode].enable_sram)
cmd_dispout[1] |= MCS_DISP_OUT_SRAM_EN;
- return mipi_dsi_dcs_write_buffer(nt->dsi[0], cmd_dispout,
- ARRAY_SIZE(cmd_dispout));
+ mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd_dispout,
+ ARRAY_SIZE(cmd_dispout));
}
static int nt35950_get_current_mode(struct nt35950 *nt)
@@ -284,109 +258,68 @@ static int nt35950_on(struct nt35950 *nt)
{
const struct nt35950_panel_mode *mode_data = nt->desc->mode_data;
struct mipi_dsi_device *dsi = nt->dsi[0];
- struct device *dev = &dsi->dev;
- int ret;
+ struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
nt->cur_mode = nt35950_get_current_mode(nt);
nt->dsi[0]->mode_flags |= MIPI_DSI_MODE_LPM;
nt->dsi[1]->mode_flags |= MIPI_DSI_MODE_LPM;
- ret = nt35950_set_cmd2_page(nt, 0);
- if (ret < 0)
- return ret;
-
- ret = nt35950_set_data_compression(nt, mode_data[nt->cur_mode].compression);
- if (ret < 0)
- return ret;
-
- ret = nt35950_set_scale_mode(nt, mode_data[nt->cur_mode].scaler_mode);
- if (ret < 0)
- return ret;
-
- ret = nt35950_set_scaler(nt, mode_data[nt->cur_mode].scaler_on);
- if (ret < 0)
- return ret;
+ nt35950_set_cmd2_page(&dsi_ctx, nt, 0);
+ nt35950_set_data_compression(&dsi_ctx, nt, mode_data[nt->cur_mode].compression);
+ nt35950_set_scale_mode(&dsi_ctx, mode_data[nt->cur_mode].scaler_mode);
+ nt35950_set_scaler(&dsi_ctx, mode_data[nt->cur_mode].scaler_on);
+ nt35950_set_dispout(&dsi_ctx, nt);
- ret = nt35950_set_dispout(nt);
- if (ret < 0)
- return ret;
-
- 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;
- }
-
- ret = mipi_dsi_dcs_set_tear_scanline(dsi, 0);
- if (ret < 0) {
- dev_err(dev, "Failed to set tear scanline: %d\n", ret);
- return ret;
- }
+ mipi_dsi_dcs_set_tear_on_multi(&dsi_ctx, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
+ mipi_dsi_dcs_set_tear_scanline_multi(&dsi_ctx, 0);
/* CMD2 Page 1 */
- ret = nt35950_set_cmd2_page(nt, 1);
- if (ret < 0)
- return ret;
+ nt35950_set_cmd2_page(&dsi_ctx, nt, 1);
/* Unknown command */
- mipi_dsi_dcs_write_seq(dsi, 0xd4, 0x88, 0x88);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xd4, 0x88, 0x88);
/* CMD2 Page 7 */
- ret = nt35950_set_cmd2_page(nt, 7);
- if (ret < 0)
- return ret;
+ nt35950_set_cmd2_page(&dsi_ctx, nt, 7);
/* Enable SubPixel Rendering */
- mipi_dsi_dcs_write_seq(dsi, MCS_PARAM_SPR_EN, 0x01);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MCS_PARAM_SPR_EN, 0x01);
/* SPR Mode: YYG Rainbow-RGB */
- mipi_dsi_dcs_write_seq(dsi, MCS_PARAM_SPR_MODE, MCS_SPR_MODE_YYG_RAINBOW_RGB);
+ mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MCS_PARAM_SPR_MODE,
+ MCS_SPR_MODE_YYG_RAINBOW_RGB);
/* CMD3 */
- ret = nt35950_inject_black_image(nt);
- if (ret < 0)
- return ret;
+ nt35950_inject_black_image(&dsi_ctx);
+ mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+ mipi_dsi_msleep(&dsi_ctx, 120);
- ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
- if (ret < 0)
- return ret;
- msleep(120);
-
- ret = mipi_dsi_dcs_set_display_on(dsi);
- if (ret < 0)
- return ret;
- msleep(120);
+ mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
+ mipi_dsi_msleep(&dsi_ctx, 120);
nt->dsi[0]->mode_flags &= ~MIPI_DSI_MODE_LPM;
nt->dsi[1]->mode_flags &= ~MIPI_DSI_MODE_LPM;
- return 0;
+ return dsi_ctx.accum_err;
}
static int nt35950_off(struct nt35950 *nt)
{
- struct device *dev = &nt->dsi[0]->dev;
- int ret;
+ struct mipi_dsi_device *dsi = nt->dsi[0];
+ struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
- ret = mipi_dsi_dcs_set_display_off(nt->dsi[0]);
- if (ret < 0) {
- dev_err(dev, "Failed to set display off: %d\n", ret);
- goto set_lpm;
- }
- usleep_range(10000, 11000);
+ mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
+ mipi_dsi_usleep_range(&dsi_ctx, 10000, 11000);
- ret = mipi_dsi_dcs_enter_sleep_mode(nt->dsi[0]);
- if (ret < 0) {
- dev_err(dev, "Failed to enter sleep mode: %d\n", ret);
- goto set_lpm;
- }
- msleep(150);
+ mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
+ mipi_dsi_msleep(&dsi_ctx, 150);
-set_lpm:
- nt->dsi[0]->mode_flags |= MIPI_DSI_MODE_LPM;
- nt->dsi[1]->mode_flags |= MIPI_DSI_MODE_LPM;
+ if (dsi_ctx.accum_err) {
+ nt->dsi[0]->mode_flags |= MIPI_DSI_MODE_LPM;
+ nt->dsi[1]->mode_flags |= MIPI_DSI_MODE_LPM;
+ }
- return 0;
+ return dsi_ctx.accum_err;
}
static int nt35950_sharp_init_vregs(struct nt35950 *nt, struct device *dev)
@@ -427,7 +360,6 @@ static int nt35950_sharp_init_vregs(struct nt35950 *nt, struct device *dev)
static int nt35950_prepare(struct drm_panel *panel)
{
struct nt35950 *nt = to_nt35950(panel);
- struct device *dev = &nt->dsi[0]->dev;
int ret;
ret = regulator_enable(nt->vregs[0].consumer);
@@ -452,10 +384,8 @@ static int nt35950_prepare(struct drm_panel *panel)
nt35950_reset(nt);
ret = nt35950_on(nt);
- if (ret < 0) {
- dev_err(dev, "Failed to initialize panel: %d\n", ret);
+ if (ret < 0)
goto end;
- }
end:
if (ret < 0) {
@@ -469,12 +399,8 @@ static int nt35950_prepare(struct drm_panel *panel)
static int nt35950_unprepare(struct drm_panel *panel)
{
struct nt35950 *nt = to_nt35950(panel);
- struct device *dev = &nt->dsi[0]->dev;
- int ret;
- ret = nt35950_off(nt);
- if (ret < 0)
- dev_err(dev, "Failed to deinitialize panel: %d\n", ret);
+ nt35950_off(nt);
gpiod_set_value_cansleep(nt->reset_gpio, 0);
regulator_bulk_disable(ARRAY_SIZE(nt->vregs), nt->vregs);
--
2.46.0Hi,
On Sat, Aug 24, 2024 at 1:44 AM Tejas Vipin <tejasvipin76@gmail.com> wrote:
>
> +static void nt35950_set_cmd2_page(struct mipi_dsi_multi_context *dsi_ctx,
> + struct nt35950 *nt, u8 page)
> {
> const u8 mauc_cmd2_page[] = { MCS_CMD_MAUCCTR, 0x55, 0xaa, 0x52,
> 0x08, page };
> - int ret;
>
> - ret = mipi_dsi_dcs_write_buffer(nt->dsi[0], mauc_cmd2_page,
> + mipi_dsi_dcs_write_buffer_multi(dsi_ctx, mauc_cmd2_page,
> ARRAY_SIZE(mauc_cmd2_page));
> - if (ret < 0)
> - return ret;
> + if (dsi_ctx->accum_err)
> + return;
>
> nt->last_page = page;
> - return 0;
nit: It's a style choice, but I personally would have changed the above to just:
if (!dsi_ctx->accum_err)
nt->last_page = page;
> @@ -284,109 +258,68 @@ static int nt35950_on(struct nt35950 *nt)
> {
> const struct nt35950_panel_mode *mode_data = nt->desc->mode_data;
> struct mipi_dsi_device *dsi = nt->dsi[0];
> - struct device *dev = &dsi->dev;
> - int ret;
> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>
> nt->cur_mode = nt35950_get_current_mode(nt);
> nt->dsi[0]->mode_flags |= MIPI_DSI_MODE_LPM;
> nt->dsi[1]->mode_flags |= MIPI_DSI_MODE_LPM;
>
> - ret = nt35950_set_cmd2_page(nt, 0);
> - if (ret < 0)
> - return ret;
> -
> - ret = nt35950_set_data_compression(nt, mode_data[nt->cur_mode].compression);
> - if (ret < 0)
> - return ret;
> -
> - ret = nt35950_set_scale_mode(nt, mode_data[nt->cur_mode].scaler_mode);
> - if (ret < 0)
> - return ret;
> -
> - ret = nt35950_set_scaler(nt, mode_data[nt->cur_mode].scaler_on);
> - if (ret < 0)
> - return ret;
> + nt35950_set_cmd2_page(&dsi_ctx, nt, 0);
> + nt35950_set_data_compression(&dsi_ctx, nt, mode_data[nt->cur_mode].compression);
> + nt35950_set_scale_mode(&dsi_ctx, mode_data[nt->cur_mode].scaler_mode);
> + nt35950_set_scaler(&dsi_ctx, mode_data[nt->cur_mode].scaler_on);
> + nt35950_set_dispout(&dsi_ctx, nt);
>
> - ret = nt35950_set_dispout(nt);
> - if (ret < 0)
> - return ret;
> -
> - 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;
> - }
> -
> - ret = mipi_dsi_dcs_set_tear_scanline(dsi, 0);
> - if (ret < 0) {
> - dev_err(dev, "Failed to set tear scanline: %d\n", ret);
> - return ret;
> - }
> + mipi_dsi_dcs_set_tear_on_multi(&dsi_ctx, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
> + mipi_dsi_dcs_set_tear_scanline_multi(&dsi_ctx, 0);
>
> /* CMD2 Page 1 */
> - ret = nt35950_set_cmd2_page(nt, 1);
> - if (ret < 0)
> - return ret;
> + nt35950_set_cmd2_page(&dsi_ctx, nt, 1);
>
> /* Unknown command */
> - mipi_dsi_dcs_write_seq(dsi, 0xd4, 0x88, 0x88);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xd4, 0x88, 0x88);
>
> /* CMD2 Page 7 */
> - ret = nt35950_set_cmd2_page(nt, 7);
> - if (ret < 0)
> - return ret;
> + nt35950_set_cmd2_page(&dsi_ctx, nt, 7);
>
> /* Enable SubPixel Rendering */
> - mipi_dsi_dcs_write_seq(dsi, MCS_PARAM_SPR_EN, 0x01);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MCS_PARAM_SPR_EN, 0x01);
>
> /* SPR Mode: YYG Rainbow-RGB */
> - mipi_dsi_dcs_write_seq(dsi, MCS_PARAM_SPR_MODE, MCS_SPR_MODE_YYG_RAINBOW_RGB);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MCS_PARAM_SPR_MODE,
> + MCS_SPR_MODE_YYG_RAINBOW_RGB);
>
> /* CMD3 */
> - ret = nt35950_inject_black_image(nt);
> - if (ret < 0)
> - return ret;
> + nt35950_inject_black_image(&dsi_ctx);
> + mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> + mipi_dsi_msleep(&dsi_ctx, 120);
>
> - ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
> - if (ret < 0)
> - return ret;
> - msleep(120);
> -
> - ret = mipi_dsi_dcs_set_display_on(dsi);
> - if (ret < 0)
> - return ret;
> - msleep(120);
> + mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> + mipi_dsi_msleep(&dsi_ctx, 120);
>
> nt->dsi[0]->mode_flags &= ~MIPI_DSI_MODE_LPM;
> nt->dsi[1]->mode_flags &= ~MIPI_DSI_MODE_LPM;
>
> - return 0;
> + return dsi_ctx.accum_err;
I think you only want to clear "MIPI_DSI_MODE_LPM" if there was no
error, right? That matches the old code. So you'd want:
if (dsi_ctx.accum_err)
return dsi_ctx.accum_err;
nt->dsi[0]->mode_flags &= ~MIPI_DSI_MODE_LPM;
nt->dsi[1]->mode_flags &= ~MIPI_DSI_MODE_LPM;
return 0;
> static int nt35950_off(struct nt35950 *nt)
> {
> - struct device *dev = &nt->dsi[0]->dev;
> - int ret;
> + struct mipi_dsi_device *dsi = nt->dsi[0];
> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>
> - ret = mipi_dsi_dcs_set_display_off(nt->dsi[0]);
> - if (ret < 0) {
> - dev_err(dev, "Failed to set display off: %d\n", ret);
> - goto set_lpm;
> - }
> - usleep_range(10000, 11000);
> + mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> + mipi_dsi_usleep_range(&dsi_ctx, 10000, 11000);
>
> - ret = mipi_dsi_dcs_enter_sleep_mode(nt->dsi[0]);
> - if (ret < 0) {
> - dev_err(dev, "Failed to enter sleep mode: %d\n", ret);
> - goto set_lpm;
> - }
> - msleep(150);
> + mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
> + mipi_dsi_msleep(&dsi_ctx, 150);
>
> -set_lpm:
> - nt->dsi[0]->mode_flags |= MIPI_DSI_MODE_LPM;
> - nt->dsi[1]->mode_flags |= MIPI_DSI_MODE_LPM;
> + if (dsi_ctx.accum_err) {
> + nt->dsi[0]->mode_flags |= MIPI_DSI_MODE_LPM;
> + nt->dsi[1]->mode_flags |= MIPI_DSI_MODE_LPM;
> + }
I'm pretty sure you want to set MIPI_DSI_MODE_LPM _unconditionally,
right? The old code would set it in both error and non-error cases I
think.
> @@ -452,10 +384,8 @@ static int nt35950_prepare(struct drm_panel *panel)
> nt35950_reset(nt);
>
> ret = nt35950_on(nt);
> - if (ret < 0) {
> - dev_err(dev, "Failed to initialize panel: %d\n", ret);
> + if (ret < 0)
> goto end;
> - }
I'd just get rid of the "if" test since "end" is next anyway. Given
that there's no error message it seems silly to have the "if" test
now.
> @@ -469,12 +399,8 @@ static int nt35950_prepare(struct drm_panel *panel)
> static int nt35950_unprepare(struct drm_panel *panel)
> {
> struct nt35950 *nt = to_nt35950(panel);
> - struct device *dev = &nt->dsi[0]->dev;
> - int ret;
>
> - ret = nt35950_off(nt);
> - if (ret < 0)
> - dev_err(dev, "Failed to deinitialize panel: %d\n", ret);
> + nt35950_off(nt);
This is the only caller of nt35950_off() and you no longer check the
return code, so you can just make nt35950_off() void, right?
On 24/08/2024 10:44, Tejas Vipin wrote:
> Changes the novatek-nt35950 panel to use multi style functions for
> improved error handling.
>
> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
> ---
> drivers/gpu/drm/panel/panel-novatek-nt35950.c | 214 ++++++------------
> 1 file changed, 70 insertions(+), 144 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35950.c b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
> index 028fdac293f7..fa4db7a3bc25 100644
> --- a/drivers/gpu/drm/panel/panel-novatek-nt35950.c
> +++ b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
> @@ -100,106 +100,89 @@ static void nt35950_reset(struct nt35950 *nt)
>
> /*
> * nt35950_set_cmd2_page - Select manufacturer control (CMD2) page
> + * @dsi_ctx: context for mipi_dsi functions
> * @nt: Main driver structure
> * @page: Page number (0-7)
> - *
> - * Return: Number of transferred bytes or negative number on error
> */
> -static int nt35950_set_cmd2_page(struct nt35950 *nt, u8 page)
> +static void nt35950_set_cmd2_page(struct mipi_dsi_multi_context *dsi_ctx,
> + struct nt35950 *nt, u8 page)
> {
> const u8 mauc_cmd2_page[] = { MCS_CMD_MAUCCTR, 0x55, 0xaa, 0x52,
> 0x08, page };
> - int ret;
>
> - ret = mipi_dsi_dcs_write_buffer(nt->dsi[0], mauc_cmd2_page,
> + mipi_dsi_dcs_write_buffer_multi(dsi_ctx, mauc_cmd2_page,
> ARRAY_SIZE(mauc_cmd2_page));
> - if (ret < 0)
> - return ret;
> + if (dsi_ctx->accum_err)
> + return;
>
> nt->last_page = page;
> - return 0;
> }
>
> /*
> * nt35950_set_data_compression - Set data compression mode
> + * @dsi_ctx: context for mipi_dsi functions
> * @nt: Main driver structure
> * @comp_mode: Compression mode
> - *
> - * Return: Number of transferred bytes or negative number on error
> */
> -static int nt35950_set_data_compression(struct nt35950 *nt, u8 comp_mode)
> +static void nt35950_set_data_compression(struct mipi_dsi_multi_context *dsi_ctx,
> + struct nt35950 *nt, u8 comp_mode)
> {
> u8 cmd_data_compression[] = { MCS_PARAM_DATA_COMPRESSION, comp_mode };
> u8 cmd_vesa_dsc_on[] = { MCS_PARAM_VESA_DSC_ON, !!comp_mode };
> u8 cmd_vesa_dsc_setting[] = { MCS_PARAM_VESA_DSC_SETTING, 0x03 };
> u8 last_page = nt->last_page;
> - int ret;
>
> /* Set CMD2 Page 0 if we're not there yet */
> - if (last_page != 0) {
> - ret = nt35950_set_cmd2_page(nt, 0);
> - if (ret < 0)
> - return ret;
> - }
> + if (last_page != 0)
> + nt35950_set_cmd2_page(dsi_ctx, nt, 0);
>
> - ret = mipi_dsi_dcs_write_buffer(nt->dsi[0], cmd_data_compression,
> + mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd_data_compression,
> ARRAY_SIZE(cmd_data_compression));
> - if (ret < 0)
> - return ret;
> -
> - ret = mipi_dsi_dcs_write_buffer(nt->dsi[0], cmd_vesa_dsc_on,
> + mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd_vesa_dsc_on,
> ARRAY_SIZE(cmd_vesa_dsc_on));
> - if (ret < 0)
> - return ret;
>
> /* Set the vesa dsc setting on Page 4 */
> - ret = nt35950_set_cmd2_page(nt, 4);
> - if (ret < 0)
> - return ret;
> + nt35950_set_cmd2_page(dsi_ctx, nt, 4);
>
> /* Display Stream Compression setting, always 0x03 */
> - ret = mipi_dsi_dcs_write_buffer(nt->dsi[0], cmd_vesa_dsc_setting,
> + mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd_vesa_dsc_setting,
> ARRAY_SIZE(cmd_vesa_dsc_setting));
> - if (ret < 0)
> - return ret;
>
> /* Get back to the previously set page */
> - return nt35950_set_cmd2_page(nt, last_page);
> + nt35950_set_cmd2_page(dsi_ctx, nt, last_page);
> }
>
> /*
> * nt35950_set_scaler - Enable/disable resolution upscaling
> - * @nt: Main driver structure
> + * @dsi_ctx: context for mipi_dsi functions
> * @scale_up: Scale up function control
> - *
> - * Return: Number of transferred bytes or negative number on error
> */
> -static int nt35950_set_scaler(struct nt35950 *nt, u8 scale_up)
> +static void nt35950_set_scaler(struct mipi_dsi_multi_context *dsi_ctx,
> + u8 scale_up)
> {
> u8 cmd_scaler[] = { MCS_PARAM_SCALER_FUNCTION, scale_up };
>
> - return mipi_dsi_dcs_write_buffer(nt->dsi[0], cmd_scaler,
> - ARRAY_SIZE(cmd_scaler));
> + mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd_scaler,
> + ARRAY_SIZE(cmd_scaler));
> }
>
> /*
> * nt35950_set_scale_mode - Resolution upscaling mode
> - * @nt: Main driver structure
> + * @dsi_ctx: context for mipi_dsi functions
> * @mode: Scaler mode (MCS_DATA_COMPRESSION_*)
> - *
> - * Return: Number of transferred bytes or negative number on error
> */
> -static int nt35950_set_scale_mode(struct nt35950 *nt, u8 mode)
> +static void nt35950_set_scale_mode(struct mipi_dsi_multi_context *dsi_ctx,
> + u8 mode)
> {
> u8 cmd_scaler[] = { MCS_PARAM_SCALEUP_MODE, mode };
>
> - return mipi_dsi_dcs_write_buffer(nt->dsi[0], cmd_scaler,
> - ARRAY_SIZE(cmd_scaler));
> + mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd_scaler,
> + ARRAY_SIZE(cmd_scaler));
> }
>
> /*
> * nt35950_inject_black_image - Display a completely black image
> - * @nt: Main driver structure
> + * @dsi_ctx: context for mipi_dsi functions
> *
> * After IC setup, the attached panel may show random data
> * due to driveric behavior changes (resolution, compression,
> @@ -208,43 +191,34 @@ static int nt35950_set_scale_mode(struct nt35950 *nt, u8 mode)
> * the display.
> * It makes sense to push a black image before sending the sleep-out
> * and display-on commands.
> - *
> - * Return: Number of transferred bytes or negative number on error
> */
> -static int nt35950_inject_black_image(struct nt35950 *nt)
> +static void nt35950_inject_black_image(struct mipi_dsi_multi_context *dsi_ctx)
> {
> const u8 cmd0_black_img[] = { 0x6f, 0x01 };
> const u8 cmd1_black_img[] = { 0xf3, 0x10 };
> u8 cmd_test[] = { 0xff, 0xaa, 0x55, 0xa5, 0x80 };
> - int ret;
>
> /* Enable test command */
> - ret = mipi_dsi_dcs_write_buffer(nt->dsi[0], cmd_test, ARRAY_SIZE(cmd_test));
> - if (ret < 0)
> - return ret;
> + mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd_test, ARRAY_SIZE(cmd_test));
>
> /* Send a black image */
> - ret = mipi_dsi_dcs_write_buffer(nt->dsi[0], cmd0_black_img,
> + mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd0_black_img,
> ARRAY_SIZE(cmd0_black_img));
> - if (ret < 0)
> - return ret;
> - ret = mipi_dsi_dcs_write_buffer(nt->dsi[0], cmd1_black_img,
> + mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd1_black_img,
> ARRAY_SIZE(cmd1_black_img));
> - if (ret < 0)
> - return ret;
>
> /* Disable test command */
> cmd_test[ARRAY_SIZE(cmd_test) - 1] = 0x00;
> - return mipi_dsi_dcs_write_buffer(nt->dsi[0], cmd_test, ARRAY_SIZE(cmd_test));
> + mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd_test, ARRAY_SIZE(cmd_test));
> }
>
> /*
> * nt35950_set_dispout - Set Display Output register parameters
> * @nt: Main driver structure
> - *
> - * Return: Number of transferred bytes or negative number on error
> + * @dsi_ctx: context for mipi_dsi functions
> */
> -static int nt35950_set_dispout(struct nt35950 *nt)
> +static void nt35950_set_dispout(struct mipi_dsi_multi_context *dsi_ctx,
> + struct nt35950 *nt)
> {
> u8 cmd_dispout[] = { MCS_PARAM_DISP_OUTPUT_CTRL, 0x00 };
> const struct nt35950_panel_mode *mode_data = nt->desc->mode_data;
> @@ -254,8 +228,8 @@ static int nt35950_set_dispout(struct nt35950 *nt)
> if (mode_data[nt->cur_mode].enable_sram)
> cmd_dispout[1] |= MCS_DISP_OUT_SRAM_EN;
>
> - return mipi_dsi_dcs_write_buffer(nt->dsi[0], cmd_dispout,
> - ARRAY_SIZE(cmd_dispout));
> + mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd_dispout,
> + ARRAY_SIZE(cmd_dispout));
> }
>
> static int nt35950_get_current_mode(struct nt35950 *nt)
> @@ -284,109 +258,68 @@ static int nt35950_on(struct nt35950 *nt)
> {
> const struct nt35950_panel_mode *mode_data = nt->desc->mode_data;
> struct mipi_dsi_device *dsi = nt->dsi[0];
> - struct device *dev = &dsi->dev;
> - int ret;
> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>
> nt->cur_mode = nt35950_get_current_mode(nt);
> nt->dsi[0]->mode_flags |= MIPI_DSI_MODE_LPM;
> nt->dsi[1]->mode_flags |= MIPI_DSI_MODE_LPM;
>
> - ret = nt35950_set_cmd2_page(nt, 0);
> - if (ret < 0)
> - return ret;
> -
> - ret = nt35950_set_data_compression(nt, mode_data[nt->cur_mode].compression);
> - if (ret < 0)
> - return ret;
> -
> - ret = nt35950_set_scale_mode(nt, mode_data[nt->cur_mode].scaler_mode);
> - if (ret < 0)
> - return ret;
> -
> - ret = nt35950_set_scaler(nt, mode_data[nt->cur_mode].scaler_on);
> - if (ret < 0)
> - return ret;
> + nt35950_set_cmd2_page(&dsi_ctx, nt, 0);
> + nt35950_set_data_compression(&dsi_ctx, nt, mode_data[nt->cur_mode].compression);
> + nt35950_set_scale_mode(&dsi_ctx, mode_data[nt->cur_mode].scaler_mode);
> + nt35950_set_scaler(&dsi_ctx, mode_data[nt->cur_mode].scaler_on);
> + nt35950_set_dispout(&dsi_ctx, nt);
>
> - ret = nt35950_set_dispout(nt);
> - if (ret < 0)
> - return ret;
> -
> - 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;
> - }
> -
> - ret = mipi_dsi_dcs_set_tear_scanline(dsi, 0);
> - if (ret < 0) {
> - dev_err(dev, "Failed to set tear scanline: %d\n", ret);
> - return ret;
> - }
> + mipi_dsi_dcs_set_tear_on_multi(&dsi_ctx, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
> + mipi_dsi_dcs_set_tear_scanline_multi(&dsi_ctx, 0);
>
> /* CMD2 Page 1 */
> - ret = nt35950_set_cmd2_page(nt, 1);
> - if (ret < 0)
> - return ret;
> + nt35950_set_cmd2_page(&dsi_ctx, nt, 1);
>
> /* Unknown command */
> - mipi_dsi_dcs_write_seq(dsi, 0xd4, 0x88, 0x88);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xd4, 0x88, 0x88);
>
> /* CMD2 Page 7 */
> - ret = nt35950_set_cmd2_page(nt, 7);
> - if (ret < 0)
> - return ret;
> + nt35950_set_cmd2_page(&dsi_ctx, nt, 7);
>
> /* Enable SubPixel Rendering */
> - mipi_dsi_dcs_write_seq(dsi, MCS_PARAM_SPR_EN, 0x01);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MCS_PARAM_SPR_EN, 0x01);
>
> /* SPR Mode: YYG Rainbow-RGB */
> - mipi_dsi_dcs_write_seq(dsi, MCS_PARAM_SPR_MODE, MCS_SPR_MODE_YYG_RAINBOW_RGB);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MCS_PARAM_SPR_MODE,
> + MCS_SPR_MODE_YYG_RAINBOW_RGB);
>
> /* CMD3 */
> - ret = nt35950_inject_black_image(nt);
> - if (ret < 0)
> - return ret;
> + nt35950_inject_black_image(&dsi_ctx);
> + mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> + mipi_dsi_msleep(&dsi_ctx, 120);
>
> - ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
> - if (ret < 0)
> - return ret;
> - msleep(120);
> -
> - ret = mipi_dsi_dcs_set_display_on(dsi);
> - if (ret < 0)
> - return ret;
> - msleep(120);
> + mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> + mipi_dsi_msleep(&dsi_ctx, 120);
>
> nt->dsi[0]->mode_flags &= ~MIPI_DSI_MODE_LPM;
> nt->dsi[1]->mode_flags &= ~MIPI_DSI_MODE_LPM;
>
> - return 0;
> + return dsi_ctx.accum_err;
> }
>
> static int nt35950_off(struct nt35950 *nt)
> {
> - struct device *dev = &nt->dsi[0]->dev;
> - int ret;
> + struct mipi_dsi_device *dsi = nt->dsi[0];
> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>
> - ret = mipi_dsi_dcs_set_display_off(nt->dsi[0]);
> - if (ret < 0) {
> - dev_err(dev, "Failed to set display off: %d\n", ret);
> - goto set_lpm;
> - }
> - usleep_range(10000, 11000);
> + mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> + mipi_dsi_usleep_range(&dsi_ctx, 10000, 11000);
>
> - ret = mipi_dsi_dcs_enter_sleep_mode(nt->dsi[0]);
> - if (ret < 0) {
> - dev_err(dev, "Failed to enter sleep mode: %d\n", ret);
> - goto set_lpm;
> - }
> - msleep(150);
> + mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
> + mipi_dsi_msleep(&dsi_ctx, 150);
>
> -set_lpm:
> - nt->dsi[0]->mode_flags |= MIPI_DSI_MODE_LPM;
> - nt->dsi[1]->mode_flags |= MIPI_DSI_MODE_LPM;
> + if (dsi_ctx.accum_err) {
> + nt->dsi[0]->mode_flags |= MIPI_DSI_MODE_LPM;
> + nt->dsi[1]->mode_flags |= MIPI_DSI_MODE_LPM;
> + }
>
> - return 0;
> + return dsi_ctx.accum_err;
> }
>
> static int nt35950_sharp_init_vregs(struct nt35950 *nt, struct device *dev)
> @@ -427,7 +360,6 @@ static int nt35950_sharp_init_vregs(struct nt35950 *nt, struct device *dev)
> static int nt35950_prepare(struct drm_panel *panel)
> {
> struct nt35950 *nt = to_nt35950(panel);
> - struct device *dev = &nt->dsi[0]->dev;
> int ret;
>
> ret = regulator_enable(nt->vregs[0].consumer);
> @@ -452,10 +384,8 @@ static int nt35950_prepare(struct drm_panel *panel)
> nt35950_reset(nt);
>
> ret = nt35950_on(nt);
> - if (ret < 0) {
> - dev_err(dev, "Failed to initialize panel: %d\n", ret);
> + if (ret < 0)
> goto end;
> - }
>
> end:
> if (ret < 0) {
> @@ -469,12 +399,8 @@ static int nt35950_prepare(struct drm_panel *panel)
> static int nt35950_unprepare(struct drm_panel *panel)
> {
> struct nt35950 *nt = to_nt35950(panel);
> - struct device *dev = &nt->dsi[0]->dev;
> - int ret;
>
> - ret = nt35950_off(nt);
> - if (ret < 0)
> - dev_err(dev, "Failed to deinitialize panel: %d\n", ret);
> + nt35950_off(nt);
>
> gpiod_set_value_cansleep(nt->reset_gpio, 0);
> regulator_bulk_disable(ARRAY_SIZE(nt->vregs), nt->vregs);
Looks fine!
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
© 2016 - 2025 Red Hat, Inc.