[PATCH 00/23] [RFC] drm/format-helper: Introduce drm_pixmap, font support and filling

[Jocelyn Falempe]

Hi Thomas,

Thanks for taking the time to write this series.

I find the drm_pixmap is a nice improvement, and simplifies a bit the code.

Regarding the font blit, I'm not convinced by this approach.
I don't see the benefit of using the same function between fonts and 
framebuffer copy, as it's unlikely to have 16bit or 32bit fonts in the 
kernel, or to have monochrome buffer from userspace. And the big format 
switch will dispatch to different functions, so there is almost no code 
sharing between both paths.

Also for each format supported, you now need 2 functions for 
xrgb8888_to_yyyyy blit, 2 functions for c1_to_yyyyy blit, and 2 
functions for fill_yyyyyy. That doesn't scale well, and there is a lot 
of duplication between those functions.

I prefer the way Noralf Thronnes has done it in [1] and only consider 
the pixel width.

[1] 
https://lore.kernel.org/dri-devel/20190311174218.51899-2-noralf@tronnes.org/

Best regards,

-- 

Jocelyn

On 30/01/2024 10:53, Thomas Zimmermann wrote:
> This RFC patchset implements various features required for DRM panic
> handling [1] and should (for now) be seen in that context.
> 
> Most of all, the patchset replaces struct drm_framebuffer with struct
> drm_pixmap in the format-conversion helpers. DRM pixmap represents a
> source of pixel data for the blitting operations. Patches 1 to 19
> update the interface, implementation and all of the callers. These
> patches could be useful even without DRM panic handling.
> 
> With struct drm_pixmap in place, patches 20 to 22 implement rudimentary
> support for blitting font data. The pixmap refers to a character's glyph,
> which the blit routines write to the destination memory. An example on
> blitting strings is given in patch 20's commit description.
> 
> Patch 23 adds rudimentary support for fill operations. The design is
> based on blitting, but blits the same color into each pixel.
> 
> [1] https://patchwork.freedesktop.org/series/122244/
> 
> Thomas Zimmermann (23):
>    drm/format-helper: Add struct drm_pixmap
>    drm/format-helper: Use struct drm_pixmap for drm_fb_memcpy()
>    drm/format-helper: Streamline drm_fb_xfrm() implementations
>    drm/format-helper: Use struct drm_pixmap internally
>    drm/format-helper: Use struct drm_pixmap for drm_fb_swab()
>    drm/format-helper: Use struct drm_pixmap for
>      drm_fb_xrgb8888_to_rgb332()
>    drm/format-helper: Use struct drm_pixmap for
>      drm_fb_xrgb8888_to_rgb565()
>    drm/format-helper: Use struct drm_pixmap for
>      drm_fb_xrgb8888_to_xrgb1555()
>    drm/format-helper: Use struct drm_pixmap for
>      drm_fb_xrgb8888_to_argb1555()
>    drm/format-helper: Use struct drm_pixmap for
>      drm_fb_xrgb8888_to_rgba5551()
>    drm/format-helper: Use struct drm_pixmap for
>      drm_fb_xrgb8888_to_rgb888()
>    drm/format-helper: Use struct drm_pixmap for
>      drm_fb_xrgb8888_to_argb8888()
>    drm/format-helper: Use struct drm_pixmap for
>      drm_fb_xrgb8888_to_abgr8888()
>    drm/format-helper: Use struct drm_pixmap for
>      drm_fb_xrgb8888_to_xbgr8888()
>    drm/format-helper: Use struct drm_pixmap for
>      drm_fb_xrgb8888_to_xrgb2101010()
>    drm/format-helper: Use struct drm_pixmap for
>      drm_fb_xrgb8888_to_argb2101010()
>    drm/format-helper: Use struct drm_pixmap for
>      drm_fb_xrgb8888_to_gray8()
>    drm/format-helper: Use struct drm_pixmap for drm_fb_xrgb8888_to_mono()
>    drm/format-helper: Use struct drm_pixmap for drm_fb_blit()
>    [DO NOT MERGE] drm/format-helper: Add font-support for DRM pixmap
>    [DO NOT MERGE] drm/format-helper: Add color palette
>    [DO NOT MERGE] drm/format-helper: Support blitting from C1 to XRGB8888
>    [DO NOT MERGE] drm/format-helper: Add drm_fb_fill() to fill screen
>      with color
> 
>   drivers/gpu/drm/ast/ast_mode.c                |   4 +-
>   drivers/gpu/drm/drm_format_helper.c           | 678 ++++++++++++------
>   drivers/gpu/drm/drm_mipi_dbi.c                |   9 +-
>   drivers/gpu/drm/gud/gud_pipe.c                |  24 +-
>   drivers/gpu/drm/hyperv/hyperv_drm_modeset.c   |   4 +-
>   drivers/gpu/drm/mgag200/mgag200_mode.c        |   4 +-
>   drivers/gpu/drm/solomon/ssd130x.c             |  12 +-
>   .../gpu/drm/tests/drm_format_helper_test.c    | 106 ++-
>   drivers/gpu/drm/tiny/cirrus.c                 |   6 +-
>   drivers/gpu/drm/tiny/ofdrm.c                  |   6 +-
>   drivers/gpu/drm/tiny/repaper.c                |   4 +-
>   drivers/gpu/drm/tiny/simpledrm.c              |   6 +-
>   drivers/gpu/drm/tiny/st7586.c                 |   4 +-
>   include/drm/drm_format_helper.h               | 116 ++-
>   14 files changed, 678 insertions(+), 305 deletions(-)
>