<div dir="ltr"><div dir="ltr"> </div> <div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Fri, Mar 7, 2025 at 12:47 PM Maxime Ripard <<a href="mailto:mripard@kernel.org">mripard@kernel.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">On Thu, Mar 06, 2025 at 02:12:14PM -0500, Anusha Srivatsa wrote: > On Thu, Mar 6, 2025 at 12:54 PM Doug Anderson <<a href="mailto:dianders@chromium.org" target="_blank">dianders@chromium.org</a>> wrote: > > On Thu, Mar 6, 2025 at 9:20 AM Maxime Ripard <<a href="mailto:mripard@kernel.org" target="_blank">mripard@kernel.org</a>> wrote: > > > > > > On Thu, Mar 06, 2025 at 10:08:24AM -0500, Anusha Srivatsa wrote: > > > > On Thu, Mar 6, 2025 at 4:31 AM Maxime Ripard <<a href="mailto:mripard@kernel.org" target="_blank">mripard@kernel.org</a>> > > wrote: > > > > > > > > > Hi Anusha, > > > > > > > > > > On Wed, Mar 05, 2025 at 07:01:41PM -0500, Anusha Srivatsa 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 multiple occurences. > > > > > > SmPl patch: > > > > > > @rule@ > > > > > > 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 }; > > > > > > <+... > > > > > > ( > > > > > > -mipi_dsi_dcs_write_seq(dsi_var,es)@p; > > > > > > +mipi_dsi_dcs_write_seq_multi(&dsi_ctx,es); > > > > > > | > > > > > > -mipi_dsi_generic_write_seq(dsi_var,es)@p; > > > > > > +mipi_dsi_generic_write_seq_multi(&dsi_ctx,es); > > > > > > | > > > > > > -mipi_dsi_generic_write(dsi_var,es)@p; > > > > > > +mipi_dsi_generic_write_multi(&dsi_ctx,es); > > > > > > | > > > > > > -r = mipi_dsi_dcs_nop(dsi_var)@p; > > > > > > +mipi_dsi_dcs_nop_multi(&dsi_ctx); > > > > > > | > > > > > > ....rest of API > > > > > > .. > > > > > > ) > > > > > > -if(r < 0) { > > > > > > -... > > > > > > -} > > > > > > ...+> > > > > > > > > > > The point of sending a single patch was to review the coccinelle > > script, > > > > > so you must put the entire script you used here. > > > > > > > > I was actually thinking of sending patches per driver this time around > > > > since Tejas also seems to be looking into similar parts....Thoughts? > > > > > > Not really? > > > > > > The point of doing it with one driver was to make sure the coccinelle > > > script was fine before rolling it to other drivers. And actually, it > > > doesn't really matter: the whole point of putting the script in the > > > commit log is to be able to review and document the script you used. If > > > you're not going to put the one you used, it's kind of pointless. > > > > Personally, I don't have any interest in reviewing the coccinelle > > script so I don't need it and, from my point of view, you could just > > remove it from the patch description (or point to it indirectly or > > something). I'll review each patch on its own merits. I am a bit > > curious if you ended up fully generating this patch with a coccinelle > > script or if you used a coccinelle script to start and then had to > > manually tweak the patch after. Actually, I guess I'll take it back. > > If you manage to fully generate conversions for all the panels with a > > single cocinelle script, I would love to take a glance at your full > > script just to satisfy my curiosity for how you handled everything > > properly. :-P > > > > managed to get all conversions other than the usleep_range() and mslee() > because I was trying to replace them with mipi_dsi_* variants only in > functions that I was touching and not each and every occurrence. Didnt > spend enough time to get the script smart enough to do that and did only > usleep() and msleep change manually. Here goes the script: > > @rule_1@ > identifier dsi_var; > expression dsi_device; > expression list es; > @@ > struct mipi_dsi_device *dsi_var = dsi_device; > +struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi_var }; > <+... > -mipi_dsi_dcs_write_seq(dsi_var,es); > +mipi_dsi_dcs_write_seq_multi(&dsi_ctx,es); > ...+> > > //rule_2 > @@ > expression list es; > identifier jdi; > @@ > static int jdi_write_dcdc_registers(struct jdi_panel *jdi) > { > +struct mipi_dsi_multi_context dsi_ctx1 = { .dsi = jdi->link1 }; > +struct mipi_dsi_multi_context dsi_ctx2 = { .dsi = jdi->link2 }; > <+... > -mipi_dsi_generic_write_seq(jdi->link1,es); > +mipi_dsi_generic_write_seq_multi(&dsi_ctx1,es); > -mipi_dsi_generic_write_seq(jdi->link2,es); > +mipi_dsi_generic_write_seq_multi(&dsi_ctx2,es); > ...+> > } > @rule_3@ > 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); > ) You're not matching on msleep because it doesn't return anything, and here you're expecting a return value. You need to make 'r =' optional. </blockquote><div>Yup! works now :)</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"> Maxime </blockquote></div></div>