[Intel-gfx] [PATCH 00/12] drm: Put drm_display_mode on diet

[Emil Velikov]

On Thu, 20 Feb 2020 at 14:28, Ville Syrjälä
<ville.syrjala at linux.intel.com> wrote:
>
> On Thu, Feb 20, 2020 at 01:21:03PM +0000, Emil Velikov wrote:
> > On Wed, 19 Feb 2020 at 20:35, Ville Syrjala
> > <ville.syrjala at linux.intel.com> wrote:
> > >
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > >
> > > struct drm_display_mode is extremely fat. Put it on diet.
> > >
> > > Some stats for the whole series:
> > >
> > > 64bit sizeof(struct drm_display_mode):
> > > 200 -> 136 bytes (-32%)
> > >
> > > 64bit bloat-o-meter -c drm.ko:
> > > add/remove: 1/0 grow/shrink: 29/47 up/down: 893/-1544 (-651)
> > > Function                                     old     new   delta
> > > ...
> > > Total: Before=189430, After=188779, chg -0.34%
> > > add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
> > > Data                                         old     new   delta
> > > Total: Before=11667, After=11667, chg +0.00%
> > > add/remove: 0/0 grow/shrink: 0/5 up/down: 0/-16896 (-16896)
> > > RO Data                                      old     new   delta
> > > edid_4k_modes                               1000     680    -320
> > > edid_est_modes                              3400    2312   -1088
> > > edid_cea_modes_193                          5400    3672   -1728
> > > drm_dmt_modes                              17600   11968   -5632
> > > edid_cea_modes_1                           25400   17272   -8128
> > > Total: Before=71239, After=54343, chg -23.72%
> > >
> > >
> > > 64bit bloat-o-meter drm.ko:
> > > add/remove: 1/0 grow/shrink: 29/52 up/down: 893/-18440 (-17547)
> > > ...
> > > Total: Before=272336, After=254789, chg -6.44%
> > >
> > >
> > > 32bit sizeof(struct drm_display_mode):
> > > 184 -> 120 bytes (-34%)
> > >
> > > 32bit bloat-o-meter -c drm.ko
> > > add/remove: 1/0 grow/shrink: 19/21 up/down: 743/-1368 (-625)
> > > Function                                     old     new   delta
> > > ...
> > > Total: Before=172359, After=171734, chg -0.36%
> > > add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
> > > Data                                         old     new   delta
> > > Total: Before=4227, After=4227, chg +0.00%
> > > add/remove: 0/0 grow/shrink: 0/5 up/down: 0/-16896 (-16896)
> > > RO Data                                      old     new   delta
> > > edid_4k_modes                                920     600    -320
> > > edid_est_modes                              3128    2040   -1088
> > > edid_cea_modes_193                          4968    3240   -1728
> > > drm_dmt_modes                              16192   10560   -5632
> > > edid_cea_modes_1                           23368   15240   -8128
> > > Total: Before=59230, After=42334, chg -28.53%
> > >
> > > 32bit bloat-o-meter drm.ko:
> > > add/remove: 1/0 grow/shrink: 19/26 up/down: 743/-18264 (-17521)
> > > ...
> > > Total: Before=235816, After=218295, chg -7.43%
> > >
> > >
> > > Some ideas for further reduction:
> > > - Convert mode->name to a pointer (saves 24/28 bytes in the
> > >   struct but would often require a heap alloc for the name (though
> > >   typical mode name is <10 bytes so still overall win perhaps)
> > > - Get rid of mode->name entirely? I guess setcrtc & co. is the only
> > >   place where we have to preserve the user provided name, elsewhere
> > >   could pehaps just generate on demand? Not sure how tricky this
> > >   would get.
> >
> > The series does some great work, with future work reaching the cache
> > line for 64bit.
> > Doing much more than that might be an overkill IMHO.
> >
> > In particular, if we change DRM_DISPLAY_MODE_LEN to 24 we get there,
> > avoiding the heap alloc/calc on demand fun.
> > While also ensuring the name is sufficiently large for the next decade or so.
>
> Unfortunately it's part of the uabi. So can't change it without some
> risk of userspace breakage.
>
Right the define is in the uABI. More importantly userspace can
provide a drm_mode_modeinfo blob, with a name[32], which we cannot
store in the kernel drm_display_mode::name[24].

One might get away with returning -EINVAL if the actual name given by
the user is > 24.
Since you've found a better way, there's on point in risking it.

-Emil