Hi,
On Sat, Mar 22, 2025 at 10:30 PM Tejas Vipin <tejasvipi...@gmail.com> wrote:
>
> @@ -493,35 +481,28 @@ static int hx8394_enable(struct drm_panel *panel)
> {
> struct hx8394 *ctx = panel_to_hx8394(panel);
> struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> - int ret;
> -
> - ret = ctx->desc->init_sequence(ctx);
> - if (ret) {
> - dev_err(ctx->dev, "Panel init sequence failed: %d\n", ret);
> - return ret;
> - }
> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>
> - ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
> - if (ret) {
> - dev_err(ctx->dev, "Failed to exit sleep mode: %d\n", ret);
> - return ret;
> - }
> + ctx->desc->init_sequence(&dsi_ctx);
>
> + mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> /* Panel is operational 120 msec after reset */
> - msleep(120);
> + mipi_dsi_msleep(&dsi_ctx, 120);
>
> - ret = mipi_dsi_dcs_set_display_on(dsi);
> - if (ret) {
> - dev_err(ctx->dev, "Failed to turn on the display: %d\n", ret);
> + mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> + if (dsi_ctx.accum_err)
> goto sleep_in;
> - }
>
> return 0;
>
> sleep_in:
> + int ret = dsi_ctx.accum_err;
I can't quite remember if kernel conventions allow local variables
defined in the middle of code like you're doing. Maybe safer to define
"int ret" at the top of the function?
> +
> + dsi_ctx.accum_err = 0;
> +
> /* This will probably fail, but let's try orderly power off anyway. */
> - if (!mipi_dsi_dcs_enter_sleep_mode(dsi))
> - msleep(50);
> + mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
> + mipi_dsi_msleep(&dsi_ctx, 50);
>
> return ret;
> }
If I'm reading it right, the above is a slight change in behavior.
Previously if the init_sequence() or mipi_dsi_dcs_exit_sleep_mode()
returned an error then you wouldn't run the
mipi_dsi_dcs_enter_sleep_mode(). Now you will.
If we're certain that this change is correct it should be mentioned in
the commit message. If not then we should make it function the way it
used to.
If we do want to keep it the way you have it, my own personal
preference would also be to rearrange the code and get rid of the
"goto", though I can see some argument for keeping the "goto" to make
it obvious where error handling is... Up to you.
