<div dir="ltr"><div dir="ltr"> </div> <div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Tue, Mar 25, 2025 at 11:34 AM Doug Anderson <<a href="mailto:dianders@chromium.org">dianders@chromium.org</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi, On Mon, Mar 24, 2025 at 1:31 PM Anusha Srivatsa <<a href="mailto:asrivats@redhat.com" target="_blank">asrivats@redhat.com</a>> wrote: > > Move away from using deprecated API and use _multi > variants if available. Use mipi_dsi_msleep() > and mipi_dsi_usleep_range() instead of msleep() > and usleep_range() respectively. > > Used Coccinelle to find the _multi variant APIs, > replacing mpi_dsi_msleep() where necessary and for returning > dsi_ctx.accum_err in these functions. mipi_dsi_dcs_write() > does not have a corresponding _multi() variant. Replacing it with > mipi_dsi_dcs_write_buffer_multi() instead. This change is manual > > @rule_1@ > identifier dsi_var; > identifier r; > identifier func; > type t; > position p; > expression dsi_device; > expression list es; > @@ > t func(...) { > ... > struct mipi_dsi_device *dsi_var = dsi_device; > +struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi_var }; > <+... > ( > -r = mipi_dsi_dcs_nop(dsi_var)@p; > +mipi_dsi_dcs_nop_multi(&dsi_ctx); > | > -r = mipi_dsi_dcs_exit_sleep_mode(dsi_var)@p; > +mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx); > | > -r = mipi_dsi_dcs_enter_sleep_mode(dsi_var)@p; > +mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx); > | > -r = mipi_dsi_dcs_write_buffer(dsi_var,es)@p; > +mipi_dsi_dcs_write_buffer_multi(&dsi_ctx,es); > | > -r = mipi_dsi_dcs_set_display_off(dsi_var,es)@p; > +mipi_dsi_dcs_set_display_off_multi(&dsi_ctx,es); > | > -r = mipi_dsi_compression_mode_ext(dsi_var,es)@p; > +mipi_dsi_compression_mode_ext_multi(&dsi_ctx,es); > | > -r = mipi_dsi_compression_mode(dsi_var,es)@p; > +mipi_dsi_compression_mode_multi(&dsi_ctx,es); > | > -r = mipi_dsi_picture_parameter_set(dsi_var,es)@p; > +mipi_dsi_picture_parameter_set_multi(&dsi_ctx,es); > | > -r = mipi_dsi_dcs_set_display_on(dsi_var,es)@p; > +mipi_dsi_dcs_set_display_on_multi(&dsi_ctx,es); > | > -r = mipi_dsi_dcs_set_tear_on(dsi_var)@p; > +mipi_dsi_dcs_set_tear_on_multi(&dsi_ctx); > | > -r = mipi_dsi_turn_on_peripheral(dsi_var)@p; > +mipi_dsi_turn_on_peripheral_multi(&dsi_ctx); > | > -r = mipi_dsi_dcs_soft_reset(dsi_var)@p; > +mipi_dsi_dcs_soft_reset_multi(&dsi_ctx); > | > -r = mipi_dsi_dcs_set_display_brightness(dsi_var,es)@p; > +mipi_dsi_dcs_set_display_brightness_multi(&dsi_ctx,es); > | > -r = mipi_dsi_dcs_set_pixel_format(dsi_var,es)@p; > +mipi_dsi_dcs_set_pixel_format_multi(&dsi_ctx,es); > | > -r = mipi_dsi_dcs_set_column_address(dsi_var,es)@p; > +mipi_dsi_dcs_set_column_address_multi(&dsi_ctx,es); > | > -r = mipi_dsi_dcs_set_page_address(dsi_var,es)@p; > +mipi_dsi_dcs_set_page_address_multi(&dsi_ctx,es); > | > -r = mipi_dsi_dcs_set_tear_scanline(dsi_var,es)@p; > +mipi_dsi_dcs_set_tear_scanline_multi(&dsi_ctx,es); > ) > -if(r < 0) { > -... > -} > ...+> > } > > @rule_2@ > identifier dsi_var; > identifier r; > identifier func; > type t; > position p; > expression dsi_device; > expression list es; > @@ > t func(...) { > ... > struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi_var }; > <+... > ( > -r = msleep(es)@p; > +r = mipi_dsi_msleep(&dsi_ctx,es); > | > -msleep(es)@p; > +mipi_dsi_msleep(&dsi_ctx,es); > | > -r = usleep_range(es)@p; > +r = mipi_dsi_usleep_range(&dsi_ctx,es); > | > -usleep_range(es)@p; > +mipi_dsi_usleep_range(&dsi_ctx,es); > ) > ...+> > } > > @rule_3@ > identifier dsi_var; > identifier func; > type t; > position p; > expression list es; > @@ > t func(...) { > ... > struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi_var }; > ... > -return 0; > +return dsi_ctx.accum_err; > } This is the exact same script as last time, right? Rather than duplicate it, you can just reference the previous patch that already landed. You'd say something like: The Coccinelle script is the same as the one in commit c8ba07caaecc ("drm/panel/synaptics-r63353: Use _multi variants") </blockquote><div>Hey Doug - Good idea.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> > v2: Use mipi_dsi_dcs_write_buffer_multi() in place of > mipi_dsi_dcs_write(). (Dmitry) > > Cc: Maxime Ripard <<a href="mailto:mripard@kernel.org" target="_blank">mripard@kernel.org</a>> > Cc: Dmitry Baryshkov <<a href="mailto:dmitry.baryshkov@linaro.org" target="_blank">dmitry.baryshkov@linaro.org</a>> > Cc: Tejas Vipin <<a href="mailto:tejasvipin76@gmail.com" target="_blank">tejasvipin76@gmail.com</a>> > Cc: Doug Anderson <<a href="mailto:dianders@chromium.org" target="_blank">dianders@chromium.org</a>> > Signed-off-by: Anusha Srivatsa <<a href="mailto:asrivats@redhat.com" target="_blank">asrivats@redhat.com</a>> > --- > Changes in v2: > - While mipi_dsi_dcs_write() does not have a corresponding _multi() > variant replace it with mipi_dsi_dcs_write_buffer_multi() to have all > APIs following _multi() usage for easier error handling > > - Link to v1: <a href="https://lore.kernel.org/r/20250316-b4-panel-ls043t1le01-v1-1-ee38371b0ba0@redhat.com" rel="noreferrer" target="_blank">https://lore.kernel.org/r/20250316-b4-panel-ls043t1le01-v1-1-ee38371b0ba0@redhat.com</a> > --- > drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 42 ++++++++++--------------- > 1 file changed, 16 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c > index 729cbb0d8403ff7c0c4b9d21774909cc298904a2..e3dc99ff711e388660d6d39251876de8cec50dbc 100644 > --- a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c > +++ b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c > @@ -36,60 +36,50 @@ static inline struct sharp_nt_panel *to_sharp_nt_panel(struct drm_panel *panel) > static int sharp_nt_panel_init(struct sharp_nt_panel *sharp_nt) > { > struct mipi_dsi_device *dsi = sharp_nt->dsi; > - int ret; > + struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi }; > > + static const u8 d[] = { 0xae, 0x03 }; > dsi->mode_flags |= MIPI_DSI_MODE_LPM; > > - ret = mipi_dsi_dcs_exit_sleep_mode(dsi); > - if (ret < 0) > - return ret; > + mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx); > > - msleep(120); > + mipi_dsi_msleep(&dsi_ctx, 120); > > /* Novatek two-lane operation */ > - ret = mipi_dsi_dcs_write(dsi, 0xae, (u8[]){ 0x03 }, 1); > - if (ret < 0) > - return ret; > + mipi_dsi_dcs_write_buffer_multi(&dsi_ctx, d, ARRAY_SIZE(d)); Can't the above just be: mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xae, 0x03); ? </blockquote><div>it can.... definitely simpler. </div><div> </div><div>Anusha </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> -Doug </blockquote></div></div>