[PATCH 2/8] drm/fb-helper: Add support for DRM_FORMAT_C[124]

[Geert Uytterhoeven]

Hi Thomas,

On Fri, Feb 18, 2022 at 9:14 AM Thomas Zimmermann <tzimmermann at suse.de> wrote:
> Am 17.02.22 um 17:12 schrieb Geert Uytterhoeven:
> > On Thu, Feb 17, 2022 at 3:57 PM Thomas Zimmermann <tzimmermann at suse.de> wrote:
> >> Am 15.02.22 um 17:52 schrieb Geert Uytterhoeven:
> >>> Add support for color-indexed frame buffer formats with two, four, and
> >>> sixteen colors to the DRM framebuffer helper functions:
> >>>     1. Add support for depths 1/2/4 to the damage helper,
> >>>     2. For color-indexed modes, the length of the color bitfields must be
> >>>        set to the color depth, else the logo code may pick a logo with too
> >>>        many colors.  Drop the incorrect DAC width comment, which
> >>>        originates from the i915 driver.
> >>>     3. Accept C[124] modes when validating or filling in struct
> >>>        fb_var_screeninfo, and  use the correct number of bits per pixel.
> >>>     4. Set the visual to FB_VISUAL_PSEUDOCOLOR for all supported
> >>>        color-indexed modes.
> >>>
> >>> Signed-off-by: Geert Uytterhoeven <geert at linux-m68k.org>
> >
> >>> --- a/drivers/gpu/drm/drm_fb_helper.c
> >>> +++ b/drivers/gpu/drm/drm_fb_helper.c
> >>> @@ -376,12 +376,34 @@ static void drm_fb_helper_damage_blit_real(struct drm_fb_helper *fb_helper,
> >>>                                           struct iosys_map *dst)
> >>>    {
> >>>        struct drm_framebuffer *fb = fb_helper->fb;
> >>> -     unsigned int cpp = fb->format->cpp[0];
> >>> -     size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
> >>> -     void *src = fb_helper->fbdev->screen_buffer + offset;
> >>> -     size_t len = (clip->x2 - clip->x1) * cpp;
> >>> +     size_t offset = clip->y1 * fb->pitches[0];
> >>> +     size_t len = clip->x2 - clip->x1;
> >>>        unsigned int y;
> >>> +     void *src;
> >>>
> >>> +     switch (fb->format->depth) {
> >>
> >> The depth field is deprecated. It's probably better to use
> >> fb->format->format and test against 4CC codes.
> >
> > The reason I checked for depth instead of a 4CC code is that the only
> > thing that matters here is the number of bits per pixel.  Hence this
> > function won't need any changes to support R1, R2, R4, and D1 later.
> > When we get here, we already know that we are using a format that
> > is supported by the fbdev helper code, and thus passed the 4CC
> > checks elsewhere.
>
> At some point, we will probably have to change several of these tests to
> 4cc. C8 and RGB332 both have 8-bit depth/bpp; same for C4 and RGB121; or
> whatever low-color formats we also want to add.
>
> It's not a blocker now, but maybe something to keep in mind.
>
> >
> > Alternatively, we could introduce drm_format_info_bpp() earlier in
> > the series, and use that?
>
> Having a helper for this might indeed be useful. We use depth for the
> number of color bits and bpp for the number of bits in he pixel.  That's
> important for XRGB8888, where depth is 24, or XRGB555 where depth is 15.
>
> If that makes sense, maybe have a helper for depth and one for bpp, even
> if they return the same value in most of the cases.

The helper for bpp is introduced in "[PATCH 3/8] drm/fourcc: Add
drm_format_info_bpp() helper".
I don't think we need a helper for depth, there's already the .depth
field.  It might be deprecated, but it's still used?
Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds