<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Thu, Mar 6, 2025 at 12:54 PM Doug Anderson <<a href="mailto:dianders@chromium.org">dianders@chromium.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br>
<br>
On Thu, Mar 6, 2025 at 9:20 AM Maxime Ripard <<a href="mailto:mripard@kernel.org" target="_blank">mripard@kernel.org</a>> wrote:<br>
><br>
> On Thu, Mar 06, 2025 at 10:08:24AM -0500, Anusha Srivatsa wrote:<br>
> > On Thu, Mar 6, 2025 at 4:31 AM Maxime Ripard <<a href="mailto:mripard@kernel.org" target="_blank">mripard@kernel.org</a>> wrote:<br>
> ><br>
> > > Hi Anusha,<br>
> > ><br>
> > > On Wed, Mar 05, 2025 at 07:01:41PM -0500, Anusha Srivatsa wrote:<br>
> > > > Move away from using deprecated API and use _multi<br>
> > > > variants if available. Use mipi_dsi_msleep()<br>
> > > > and mipi_dsi_usleep_range() instead of msleep()<br>
> > > > and usleep_range() respectively.<br>
> > > ><br>
> > > > Used Coccinelle to find the multiple occurences.<br>
> > > > SmPl patch:<br>
> > > > @rule@<br>
> > > > identifier dsi_var;<br>
> > > > identifier r;<br>
> > > > identifier func;<br>
> > > > type t;<br>
> > > > position p;<br>
> > > > expression dsi_device;<br>
> > > > expression list es;<br>
> > > > @@<br>
> > > > t func(...) {<br>
> > > > ...<br>
> > > > struct mipi_dsi_device *dsi_var = dsi_device;<br>
> > > > +struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi_var };<br>
> > > > <+...<br>
> > > > (<br>
> > > > -mipi_dsi_dcs_write_seq(dsi_var,es)@p;<br>
> > > > +mipi_dsi_dcs_write_seq_multi(&dsi_ctx,es);<br>
> > > > |<br>
> > > > -mipi_dsi_generic_write_seq(dsi_var,es)@p;<br>
> > > > +mipi_dsi_generic_write_seq_multi(&dsi_ctx,es);<br>
> > > > |<br>
> > > > -mipi_dsi_generic_write(dsi_var,es)@p;<br>
> > > > +mipi_dsi_generic_write_multi(&dsi_ctx,es);<br>
> > > > |<br>
> > > > -r = mipi_dsi_dcs_nop(dsi_var)@p;<br>
> > > > +mipi_dsi_dcs_nop_multi(&dsi_ctx);<br>
> > > > |<br>
> > > > ....rest of API<br>
> > > > ..<br>
> > > > )<br>
> > > > -if(r < 0) {<br>
> > > > -...<br>
> > > > -}<br>
> > > > ...+><br>
> > ><br>
> > > The point of sending a single patch was to review the coccinelle script,<br>
> > > so you must put the entire script you used here.<br>
> ><br>
> > I was actually thinking of sending patches per driver this time around<br>
> > since Tejas also seems to be looking into similar parts....Thoughts?<br>
><br>
> Not really?<br>
><br>
> The point of doing it with one driver was to make sure the coccinelle<br>
> script was fine before rolling it to other drivers. And actually, it<br>
> doesn't really matter: the whole point of putting the script in the<br>
> commit log is to be able to review and document the script you used. If<br>
> you're not going to put the one you used, it's kind of pointless.<br>
<br>
Personally, I don't have any interest in reviewing the coccinelle<br>
script so I don't need it and, from my point of view, you could just<br>
remove it from the patch description (or point to it indirectly or<br>
something). I'll review each patch on its own merits. I am a bit<br>
curious if you ended up fully generating this patch with a coccinelle<br>
script or if you used a coccinelle script to start and then had to<br>
manually tweak the patch after. Actually, I guess I'll take it back.<br>
If you manage to fully generate conversions for all the panels with a<br>
single cocinelle script, I would love to take a glance at your full<br>
script just to satisfy my curiosity for how you handled everything<br>
properly. :-P<br></blockquote><div><br></div><div>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:</div><div><br></div><div>@rule_1@<br>identifier dsi_var;<br>expression dsi_device;<br>expression list es;<br>@@<br>struct mipi_dsi_device *dsi_var = dsi_device;<br>+struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi_var };<br><+...<br>-mipi_dsi_dcs_write_seq(dsi_var,es);<br>+mipi_dsi_dcs_write_seq_multi(&dsi_ctx,es);<br>...+><br><br>//rule_2<br>@@<br>expression list es;<br>identifier jdi;<br>@@<br>static int jdi_write_dcdc_registers(struct jdi_panel *jdi)<br>{<br>+struct mipi_dsi_multi_context dsi_ctx1 = { .dsi = jdi->link1 };<br>+struct mipi_dsi_multi_context dsi_ctx2 = { .dsi = jdi->link2 };<br><+...<br>-mipi_dsi_generic_write_seq(jdi->link1,es);<br>+mipi_dsi_generic_write_seq_multi(&dsi_ctx1,es);<br>-mipi_dsi_generic_write_seq(jdi->link2,es);<br>+mipi_dsi_generic_write_seq_multi(&dsi_ctx2,es);<br>...+><br>}<br>@rule_3@<br>identifier dsi_var;<br>identifier r;<br>identifier func;<br>type t;<br>position p;<br>expression dsi_device;<br>expression list es;<br>@@<br>t func(...) {<br>...<br>struct mipi_dsi_device *dsi_var = dsi_device;<br>+struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi_var };<br><+...<br>(<br>-r = mipi_dsi_dcs_nop(dsi_var)@p;<br>+mipi_dsi_dcs_nop_multi(&dsi_ctx);<br>|<br>-r = mipi_dsi_dcs_exit_sleep_mode(dsi_var)@p;<br>+mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);<br>|<br>-r = mipi_dsi_dcs_enter_sleep_mode(dsi_var)@p;<br>+mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);<br>|<br>|<br>-r = mipi_dsi_dcs_write_buffer(dsi_var,es)@p;<br>+mipi_dsi_dcs_write_buffer_multi(&dsi_ctx,es);<br>|<br>-r = mipi_dsi_dcs_set_display_off(dsi_var,es)@p;<br>+mipi_dsi_dcs_set_display_off_multi(&dsi_ctx,es);<br>|<br>-r = mipi_dsi_compression_mode_ext(dsi_var,es)@p;<br>+mipi_dsi_compression_mode_ext_multi(&dsi_ctx,es);<br>|<br>-r = mipi_dsi_compression_mode(dsi_var,es)@p;<br>+mipi_dsi_compression_mode_multi(&dsi_ctx,es);<br>|<br>-r = mipi_dsi_picture_parameter_set(dsi_var,es)@p;<br>+mipi_dsi_picture_parameter_set_multi(&dsi_ctx,es);<br>|<br>-r = mipi_dsi_dcs_set_display_on(dsi_var,es)@p;<br>+mipi_dsi_dcs_set_display_on_multi(&dsi_ctx,es);<br>|<br>-r = mipi_dsi_dcs_set_tear_on(dsi_var)@p;<br>+mipi_dsi_dcs_set_tear_on_multi(&dsi_ctx);<br>|<br>-r = mipi_dsi_turn_on_peripheral(dsi_var)@p;<br>+mipi_dsi_turn_on_peripheral_multi(&dsi_ctx);<br>|<br>-r = mipi_dsi_dcs_soft_reset(dsi_var)@p;<br>+mipi_dsi_dcs_soft_reset_multi(&dsi_ctx);<br>|<br>-r = mipi_dsi_dcs_set_display_brightness(dsi_var,es)@p;<br>+mipi_dsi_dcs_set_display_brightness_multi(&dsi_ctx,es);<br>|<br>-r = mipi_dsi_dcs_set_pixel_format(dsi_var,es)@p;<br>+mipi_dsi_dcs_set_pixel_format_multi(&dsi_ctx,es);<br>|<br>-r = mipi_dsi_dcs_set_column_address(dsi_var,es)@p;<br>+mipi_dsi_dcs_set_column_address_multi(&dsi_ctx,es);<br>|<br>-r = mipi_dsi_dcs_set_page_address(dsi_var,es)@p;<br>+mipi_dsi_dcs_set_page_address_multi(&dsi_ctx,es);<br>|<br>-r = mipi_dsi_dcs_set_tear_scanline(dsi_var,es)@p;<br>+mipi_dsi_dcs_set_tear_scanline_multi(&dsi_ctx,es);<br>)<br>-if(r < 0) {<br>-...<br>-}<br>...+><br>}<br></div><div><br></div><div>Thanks,</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<br>
<br>
</blockquote></div></div>