Re: [PATCH v2 1/4] drm/format-helper: Add drm_fb_{xrgb8888,gray8}_to_mono_reversed()

[Thomas Zimmermann]

Am 04.02.22 um 16:52 schrieb Thomas Zimmermann:
[...]
> > +/**
> > + * drm_fb_xrgb8888_to_mono_reversed - Convert XRGB8888 to reversed 
> > monochrome
> > + * @dst: reversed monochrome destination buffer
> > + * @dst_pitch: Number of bytes between two consecutive scanlines 
> > within dst
> > + * @src: XRGB8888 source buffer
> > + * @fb: DRM framebuffer
> > + * @clip: Clip rectangle area to copy
> > + *
> > + * DRM doesn't have native monochrome support.
> > + * Such drivers can announce the commonly supported XR24 format to 
> > userspace
> > + * and use this function to convert to the native format.
> > + *
> > + * This function uses drm_fb_xrgb8888_to_gray8() to convert to 
> > grayscale and
> > + * then the result is converted from grayscale to reversed monohrome.
> > + */
> > +void drm_fb_xrgb8888_to_mono_reversed(void *dst, unsigned int 
> > dst_pitch, const void *src,
> > +                      const struct drm_framebuffer *fb,
> > +                      const struct drm_rect *clip)
> > +{
> > +    if (WARN_ON(fb->format->format != DRM_FORMAT_XRGB8888))
> > +        return;
> > +
> > +    if (!dst_pitch)
> > +        dst_pitch = drm_rect_width(clip);
> > +
> > +    drm_fb_xrgb8888_to_gray8(dst, dst_pitch, src, fb, clip);
> > +    drm_fb_gray8_to_mono_reversed(dst, dst_pitch, dst, fb, clip);
>
> Converting from dst into dst can give incorrect results. At some point 
> we probably want to add restrict qualifiers to these pointers, to help 
> the compiler with optimizing.
>
> A better approach here is to pull the per-line conversion from 
> drm_fb_xrgb8888_to_gray8() into a separate helper and implement a 
> line-by-line conversion here. something like this:
>
>    drm_fb_xrgb8888_to_mono_reversed()
>    {
>      char *tmp = kmalloc(size of a single line of gray8)
>
>      for (heigth) {
>         drm_fb_xrgb8888_to_gray8_line(tmp, ..., src, ...);
>         drm_fb_gray8_to_mono_reversed(dst, ..., tmp, ...);

Here, I meant 'drm_fb_gray8_to_mono_reversed_line()'

>         src += fb->pitches[0]
>         dst += dst_pitch;
>      }
>
>      kfree(tmp);
>    }

To elaborate, this is an example of what I meant by composable. In the 
future, we can generalize this function and hand-in 2 function pointers 
the do the conversion with tmp as intermediate buffer. That would work 
for any combination of formats that have a common intermediate format.

> Best regards
> Thomas
>
> > +}
> > +EXPORT_SYMBOL(drm_fb_xrgb8888_to_mono_reversed);
> > diff --git a/include/drm/drm_format_helper.h 
> > b/include/drm/drm_format_helper.h
> > index b30ed5de0a33..85e551a5cbe6 100644
> > --- a/include/drm/drm_format_helper.h
> > +++ b/include/drm/drm_format_helper.h
> > @@ -43,4 +43,11 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned 
> > int dst_pitch, uint32_t dst_for
> >                const void *vmap, const struct drm_framebuffer *fb,
> >                const struct drm_rect *rect);
> > +void drm_fb_gray8_to_mono_reversed(void *dst, unsigned int dst_pitch, 
> > const void *src,
> > +                   const struct drm_rect *clip);
> > +
> > +void drm_fb_xrgb8888_to_mono_reversed(void *dst, unsigned int 
> > dst_pitch, const void *src,
> > +                      const struct drm_framebuffer *fb,
> > +                      const struct drm_rect *clip);
> > +
> >   #endif /* __LINUX_DRM_FORMAT_HELPER_H */

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

Attachment: OpenPGP_signature
Description: OpenPGP digital signature