| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 2 May 2024 09:40:16 -0700
From: Doug Anderson <dianders@...omium.org>
To: Linus Walleij <linus.walleij@...aro.org>
Cc: dri-devel@...ts.freedesktop.org, Jani Nikula <jani.nikula@...ux.intel.com>,
Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
Cong Yang <yangcong5@...qin.corp-partner.google.com>, Hsin-Yi Wang <hsinyi@...gle.com>,
Brian Norris <briannorris@...omium.org>, Sam Ravnborg <sam@...nborg.org>,
Neil Armstrong <neil.armstrong@...aro.org>, Javier Martinez Canillas <javierm@...hat.com>,
Joel Selvaraj <jo@...amily.in>, lvzhaoxiong@...qin.corp-partner.google.com,
Daniel Vetter <daniel@...ll.ch>, David Airlie <airlied@...il.com>,
Jessica Zhang <quic_jesszhan@...cinc.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 7/9] drm/panel: boe-tv101wum-nl6: Don't use a table for
initting panels
Hi,
On Thu, May 2, 2024 at 6:42 AM Linus Walleij <linus.walleij@...aro.org> wrote:
>
> On Wed, May 1, 2024 at 5:43 PM Douglas Anderson <dianders@...omium.org> wrote:
>
> > Consensus on the mailing lists is that panels shouldn't use a table of
> > init commands but should instead use init functions. With the recently
> > introduced mipi_dsi_dcs_write_seq_multi() this is not only clean/easy
> > but also saves space. Measuring before/after this change:
> >
> > $ scripts/bloat-o-meter \
> > .../before/panel-boe-tv101wum-nl6.ko \
> > .../after/panel-boe-tv101wum-nl6.ko
> > add/remove: 14/8 grow/shrink: 0/1 up/down: 27062/-31433 (-4371)
> > Function old new delta
> > inx_hj110iz_init - 7040 +7040
> > boe_tv110c9m_init - 6440 +6440
> > boe_init - 5916 +5916
> > starry_qfh032011_53g_init - 1944 +1944
> > starry_himax83102_j02_init - 1228 +1228
> > inx_hj110iz_init.d - 1040 +1040
> > boe_tv110c9m_init.d - 982 +982
> > auo_b101uan08_3_init - 944 +944
> > boe_init.d - 580 +580
> > starry_himax83102_j02_init.d - 512 +512
> > starry_qfh032011_53g_init.d - 180 +180
> > auo_kd101n80_45na_init - 172 +172
> > auo_b101uan08_3_init.d - 82 +82
> > auo_kd101n80_45na_init.d - 2 +2
> > auo_kd101n80_45na_init_cmd 144 - -144
> > boe_panel_prepare 592 440 -152
> > auo_b101uan08_3_init_cmd 1056 - -1056
> > starry_himax83102_j02_init_cmd 1392 - -1392
> > starry_qfh032011_53g_init_cmd 2256 - -2256
> > .compoundliteral 3393 - -3393
> > boe_init_cmd 7008 - -7008
> > boe_tv110c9m_init_cmd 7656 - -7656
> > inx_hj110iz_init_cmd 8376 - -8376
> > Total: Before=37297, After=32926, chg -11.72%
> >
> > Let's do the conversion.
> >
> > Since we're touching all the tables, let's also convert hex numbers to
> > lower case as per kernel conventions.
> >
> > Signed-off-by: Douglas Anderson <dianders@...omium.org>
>
> Wow that's a *VERY* nice patch.
> Reviewed-by: Linus Walleij <linus.walleij@...aro.org>
Thanks!
> The metrics surprisingly reports more compact object code,
> I wasn't expecting this, but it's nice.
I think it has to do with the design of the table structure in this
driver. Each "struct panel_init_cmd" was 24-bytes big. That means that
to represent one 1-byte sequence we needed 24 bytes + 1 bytes cmd + 1
byte seq = 26 bytes. Lots of overhead for 2 bytes of content. The old
table stuff could certainly have been made _a lot_ less overhead, but
since it wasn't then it also wasn't hard to be better than it with it
via the new style.
FWIW, it also wouldn't be terribly hard to save a tiny bit more space
with the new style if we wanted. It gets a little ugly, but it doesn't
affect callers of the macro. Specifically, if you assume people aren't
going to pass more than 256-byte sequences, you could stuff the length
in the data:
#define mipi_dsi_dcs_write_seq_multi(ctx, cmd, seq...) \
- do { \
- static const u8 d[] = { cmd, seq }; \
- mipi_dsi_dcs_write_buffer_multi(ctx, d, ARRAY_SIZE(d)); \
+ do { \
+ static const u8 d[] = { \
+ (sizeof((u8[]){seq})/sizeof(u8)) + 1, cmd, seq }; \
+ mipi_dsi_dcs_write_buffer_multi(ctx, d); \
} while (0)
..and then snag the length out of the first byte of the data in
mipi_dsi_dcs_write_buffer_multi(). If you do this, you actually get
another 10% space savings on this driver. :-P
add/remove: 0/0 grow/shrink: 7/7 up/down: 1140/-4560 (-3420)
Function old new delta
inx_hj110iz_init.d 1040 1385 +345
boe_tv110c9m_init.d 982 1297 +315
boe_init.d 580 870 +290
starry_qfh032011_53g_init.d 180 271 +91
starry_himax83102_j02_init.d 512 568 +56
auo_b101uan08_3_init.d 82 123 +41
auo_kd101n80_45na_init.d 2 4 +2
auo_kd101n80_45na_init 172 164 -8
auo_b101uan08_3_init 944 780 -164
starry_himax83102_j02_init 1228 1004 -224
starry_qfh032011_53g_init 1944 1580 -364
boe_init 5916 4756 -1160
boe_tv110c9m_init 6440 5180 -1260
inx_hj110iz_init 7040 5660 -1380
Total: Before=32906, After=29486, chg -10.39%
I feel like people would balk at that, though...
-Doug
Powered by blists - more mailing lists