From: Doug Anderson <dianders@chromium.org>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: dri-devel@lists.freedesktop.org,
Jani Nikula <jani.nikula@linux.intel.com>,
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
Cong Yang <yangcong5@huaqin.corp-partner.google.com>,
Hsin-Yi Wang <hsinyi@google.com>,
Brian Norris <briannorris@chromium.org>,
Sam Ravnborg <sam@ravnborg.org>,
Neil Armstrong <neil.armstrong@linaro.org>,
Javier Martinez Canillas <javierm@redhat.com>,
Joel Selvaraj <jo@jsfamily.in>,
lvzhaoxiong@huaqin.corp-partner.google.com,
Daniel Vetter <daniel@ffwll.ch>,
David Airlie <airlied@gmail.com>,
Jessica Zhang <quic_jesszhan@quicinc.com>,
Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
Maxime Ripard <mripard@kernel.org>,
Thomas Zimmermann <tzimmermann@suse.de>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 7/9] drm/panel: boe-tv101wum-nl6: Don't use a table for initting panels
Date: Thu, 2 May 2024 09:40:16 -0700 [thread overview]
Message-ID: <CAD=FV=U59+au4Sfi5xdxmCAEaAVq7YguM2FjkyF+OYX16ydW4w@mail.gmail.com> (raw)
In-Reply-To: <CACRpkdYiND3uLAbFqyGEYgi5+ycOTYoncmSYGTsYtTZ7Ox=4DQ@mail.gmail.com>
Hi,
On Thu, May 2, 2024 at 6:42 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Wed, May 1, 2024 at 5:43 PM Douglas Anderson <dianders@chromium.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@chromium.org>
>
> Wow that's a *VERY* nice patch.
> Reviewed-by: Linus Walleij <linus.walleij@linaro.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
next prev parent reply other threads:[~2024-05-02 16:40 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-01 15:41 [PATCH v3 0/9] drm/mipi-dsi: Reduce bloat and add funcs for cleaner init seqs Douglas Anderson
2024-05-01 15:41 ` [PATCH v3 1/9] drm/mipi-dsi: Fix theoretical int overflow in mipi_dsi_dcs_write_seq() Douglas Anderson
2024-05-02 7:26 ` Neil Armstrong
2024-05-02 8:15 ` Linus Walleij
2024-05-01 15:41 ` [PATCH v3 2/9] drm/mipi-dsi: Fix theoretical int overflow in mipi_dsi_generic_write_seq() Douglas Anderson
2024-05-02 7:26 ` Neil Armstrong
2024-05-02 8:15 ` Linus Walleij
2024-05-01 15:41 ` [PATCH v3 3/9] drm/mipi-dsi: mipi_dsi_*_write functions don't need to ratelimit prints Douglas Anderson
2024-05-02 7:27 ` Neil Armstrong
2024-05-02 8:16 ` Linus Walleij
2024-05-01 15:41 ` [PATCH v3 4/9] drm/mipi-dsi: Reduce driver bloat of mipi_dsi_*_write_seq() Douglas Anderson
2024-05-02 7:27 ` Neil Armstrong
2024-05-02 8:23 ` Linus Walleij
2024-05-02 16:59 ` Doug Anderson
2024-05-01 15:41 ` [PATCH v3 5/9] drm/mipi-dsi: Introduce mipi_dsi_*_write_seq_multi() Douglas Anderson
2024-05-02 7:28 ` Neil Armstrong
2024-05-06 6:22 ` Linus Walleij
2024-05-01 15:41 ` [PATCH v3 6/9] drm/panel: novatek-nt36672e: Switch to mipi_dsi_dcs_write_seq_multi() Douglas Anderson
2024-05-01 15:41 ` [PATCH v3 7/9] drm/panel: boe-tv101wum-nl6: Don't use a table for initting panels Douglas Anderson
2024-05-02 13:41 ` Linus Walleij
2024-05-02 16:40 ` Doug Anderson [this message]
2024-05-01 15:41 ` [PATCH v3 8/9] drm/panel: ili9882t: " Douglas Anderson
2024-05-06 6:19 ` Linus Walleij
2024-05-01 15:41 ` [PATCH v3 9/9] drm/panel: innolux-p079zca: " Douglas Anderson
2024-05-03 16:32 ` Doug Anderson
2024-05-06 6:45 ` Linus Walleij
2024-05-02 7:48 ` [PATCH v3 0/9] drm/mipi-dsi: Reduce bloat and add funcs for cleaner init seqs Neil Armstrong
2024-05-02 14:27 ` Doug Anderson
2024-05-02 16:05 ` neil.armstrong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAD=FV=U59+au4Sfi5xdxmCAEaAVq7YguM2FjkyF+OYX16ydW4w@mail.gmail.com' \
--to=dianders@chromium.org \
--cc=airlied@gmail.com \
--cc=briannorris@chromium.org \
--cc=daniel@ffwll.ch \
--cc=dmitry.baryshkov@linaro.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=hsinyi@google.com \
--cc=jani.nikula@linux.intel.com \
--cc=javierm@redhat.com \
--cc=jo@jsfamily.in \
--cc=linus.walleij@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lvzhaoxiong@huaqin.corp-partner.google.com \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mripard@kernel.org \
--cc=neil.armstrong@linaro.org \
--cc=quic_jesszhan@quicinc.com \
--cc=sam@ravnborg.org \
--cc=tzimmermann@suse.de \
--cc=yangcong5@huaqin.corp-partner.google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).