Hi Guido,
On Wed, Jun 26, 2024 at 02:25:16PM GMT, Guido Günther wrote:
> [...]
> > - ret = ctx->desc->init_sequence(ctx);
> > - if (ret < 0) {
> > - dev_err(ctx->dev, "Panel init sequence failed: %d\n", ret);
> > - return ret;
> > - }
> > + ctx->desc->init_sequence(&dsi_ctx);
>
> Why no return early here in the error case (same for the other cases
> below) giving us an indication which step went wrong?
Return early is hidden in the wrapped *_multi() function calls.
> >
> > - ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
> > - if (ret < 0) {
> > - dev_err(ctx->dev, "Failed to exit sleep mode: %d\n", ret);
> > - return ret;
> > - }
> > + mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> >
> > /* It takes the controller 120 msec to wake up after sleep. */
> > - msleep(120);
> > + mipi_dsi_msleep(&dsi_ctx, 120);
> >
> > - ret = mipi_dsi_dcs_set_display_on(dsi);
> > - if (ret)
> > - return ret;
> > + mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> >
> > - dev_dbg(ctx->dev, "Panel init sequence done\n");
>
> Would be nice to keep the debug message.
>
> > + if (!dsi_ctx.accum_err)
> > + dev_dbg(ctx->dev, "Panel init sequence done\n");
> >
> > - return 0;
> > + return dsi_ctx.accum_err;
> > }
> >
> > static int st7703_disable(struct drm_panel *panel)
> > {
> > struct st7703 *ctx = panel_to_st7703(panel);
> > struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> > - int ret;
> > + struct mipi_dsi_multi_context dsi_ctx = {.dsi = dsi};
> >
> > - ret = mipi_dsi_dcs_set_display_off(dsi);
> > - if (ret < 0)
> > - dev_err(ctx->dev, "Failed to turn off the display: %d\n", ret);
> > + mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
>
> Similar to the above: it'd be nice to keep the information which step
> failed.
That's done by the _multi() wrappers.
kind regards,
o.
