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

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1: Type: text/plain, Size: 3788 bytes --]


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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]