Hi, On Sun, May 17, 2026 at 8:43 PM Chintan Patel <[email protected]> wrote: > > @@ -55,13 +51,9 @@ struct nt36672a_panel_desc { > enum mipi_dsi_pixel_format format; > unsigned int lanes; > > - unsigned int num_on_cmds_1; > - const struct nt36672a_panel_cmd *on_cmds_1; > - unsigned int num_on_cmds_2; > - const struct nt36672a_panel_cmd *on_cmds_2; > - > - unsigned int num_off_cmds; > - const struct nt36672a_panel_cmd *off_cmds; > + void (*on_init_1)(struct mipi_dsi_multi_context *dsi_ctx); > + void (*on_init_2)(struct mipi_dsi_multi_context *dsi_ctx); > + void (*off_init)(struct mipi_dsi_multi_context *dsi_ctx);
nit: The name "off_init" sounds a little strange to me. Maybe the three functions could be: send_init_cmds_1 send_init_cmds_2 send_deinit_cmds ...or something like that? > + mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0xFF, 0x22); nit: I wouldn't block landing the patch for it, but since we're touching all this code it would be nice to make things lowercase hex. 0xff instead of 0xFF. That's the kernel standard, even if there are some drivers that violate it. > + for (reg = 0x02; reg <= 0x10; reg++) > + mipi_dsi_dcs_write_var_seq_multi(dsi_ctx, reg, 0x40); So I was curious about if the "for" loops helped out. They seem to. Overall, your change increased the size of the driver by quite a bit, but I guess that's the price we pay since (when we talked about this in the past) people really didn't like the table-based approach. Old driver vs. with your changes: $ ./scripts/bloat-o-meter panel-novatek-nt36672a-old.ko panel-novatek-nt36672a-new.ko add/remove: 9/3 grow/shrink: 2/5 up/down: 10430/-1712 (8718) Function old new delta .Ltmp1 1292 4652 +3360 $x 1392 4752 +3360 tianma_fhd_video_on_init_1 - 3148 +3148 tianma_fhd_video_on_init_1.d - 252 +252 tianma_fhd_video_on_init_2 - 144 +144 tianma_fhd_video_off_init - 124 +124 tianma_fhd_video_on_init_2.d - 10 +10 tianma_fhd_video_off_init.d - 8 +8 .Ltmp8 - 8 +8 .Ltmp7 - 8 +8 .Ltmp6 - 8 +8 tianma_fhd_video_off_cmds 8 - -8 tianma_fhd_video_on_cmds_2 10 - -10 tianma_fhd_video_panel_desc 88 64 -24 nt36672a_panel_unprepare 244 212 -32 nt36672a_panel_prepare 340 284 -56 $d 3555 3331 -224 tianma_fhd_video_on_cmds_1 562 - -562 .Ltmp3 804 8 -796 Total: Before=11206, After=19924, chg +77.80% ...but if we hadn't done the "for" loops, it would have been much worse: dianders@dianders:v6.6 ((8953acf077cf...))$ ./scripts/bloat-o-meter panel-novatek-nt36672a-old.ko panel-novatek-nt36672a-new-noloops.ko add/remove: 9/3 grow/shrink: 4/4 up/down: 18380/-1488 (16892) Function old new delta .Ltmp1 1292 7164 +5872 $x 1392 7264 +5872 tianma_fhd_video_on_init_1 - 5664 +5664 tianma_fhd_video_on_init_1.d - 562 +562 tianma_fhd_video_on_init_2 - 144 +144 tianma_fhd_video_off_init - 124 +124 $d 3555 3649 +94 tianma_fhd_video_on_init_2.d - 10 +10 tianma_fhd_video_off_init.d - 8 +8 .Ltmp8 - 8 +8 .Ltmp7 - 8 +8 .Ltmp6 - 8 +8 __UNIQUE_ID_modinfo_411 79 85 +6 tianma_fhd_video_off_cmds 8 - -8 tianma_fhd_video_on_cmds_2 10 - -10 tianma_fhd_video_panel_desc 88 64 -24 nt36672a_panel_unprepare 244 212 -32 nt36672a_panel_prepare 340 284 -56 tianma_fhd_video_on_cmds_1 562 - -562 .Ltmp3 804 8 -796 Total: Before=11206, After=28098, chg +150.74% ...and even the shortest "for" loop you added made a difference. ;-) So I'm good with it, though I'd prefer that you send a new version (if possible) since the "off_init" really bothers me for some reason. ;-P I'll also note that I did some hacky testing to confirm that your "for" loops do indeed create the same set of commands as we had before your patch. In any case: Reviewed-by: Douglas Anderson <[email protected]>
