| Message ID | 20220131201225.2324984-3-javierm@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | drm/tiny: Add driver for Solomon SSD1307 OLED displays | expand |
Hi Am 31.01.22 um 21:12 schrieb Javier Martinez Canillas: > Add support to convert 8-bit grayscale to reversed monochrome for drivers > that control monochromatic displays, that only have 1 bit per pixel depth. > > This helper function was based on repaper_gray8_to_mono_reversed() from > the drivers/gpu/drm/tiny/repaper.c driver. You could convert repaper to the new helper. > > Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> > --- > > drivers/gpu/drm/drm_format_helper.c | 35 +++++++++++++++++++++++++++++ > include/drm/drm_format_helper.h | 2 ++ > 2 files changed, 37 insertions(+) > > diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c > index 0f28dd2bdd72..bf477c136082 100644 > --- a/drivers/gpu/drm/drm_format_helper.c > +++ b/drivers/gpu/drm/drm_format_helper.c > @@ -584,3 +584,38 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_for > return -EINVAL; > } > EXPORT_SYMBOL(drm_fb_blit_toio); > + > +/** > + * drm_fb_gray8_to_mono_reversed - Convert grayscale to reversed monochrome > + * @dst: reversed monochrome destination buffer > + * @src: 8-bit grayscale source buffer > + * @clip: Clip rectangle area to copy > + * > + * DRM doesn't have native monochrome or grayscale support. > + * Such drivers can announce the commonly supported XR24 format to userspace > + * and use drm_fb_xrgb8888_to_gray8() to convert to grayscale and then this > + * helper function to convert to the native format. > + */ > +void drm_fb_gray8_to_mono_reversed(void *dst, void *src, const struct drm_rect *clip) IMHO it would be better to have a function that converts xrgb8888 to mono and let it handle the intermediate step of gray8. > +{ > + size_t width = drm_rect_width(clip); > + size_t height = drm_rect_width(clip); > + > + u8 *mono = dst, *gray8 = src; > + unsigned int y, xb, i; > + > + for (y = 0; y < height; y++) > + for (xb = 0; xb < width / 8; xb++) { The inner loop should probably go into a separate helper function. See the drm_fb_*_line() helpers in this file At some point, we's want to have a single blit helper that takes source and destination formats/buffers. It would then pick the correct per-line helper for the conversion. So yeah, we'd want something composable. Best regards Thomas > + u8 byte = 0x00; > + > + for (i = 0; i < 8; i++) { > + int x = xb * 8 + i; > + > + byte >>= 1; > + if (gray8[y * width + x] >> 7) > + byte |= BIT(7); > + } > + *mono++ = byte; > + } > +} > +EXPORT_SYMBOL(drm_fb_gray8_to_mono_reversed); > diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h > index b30ed5de0a33..cd4c8b7c78de 100644 > --- a/include/drm/drm_format_helper.h > +++ b/include/drm/drm_format_helper.h > @@ -43,4 +43,6 @@ 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, void *vaddr, const struct drm_rect *clip); > + > #endif /* __LINUX_DRM_FORMAT_HELPER_H */
On Tue, 1 Feb 2022 10:59:43 +0100 Thomas Zimmermann <tzimmermann@suse.de> wrote: > Hi > > Am 31.01.22 um 21:12 schrieb Javier Martinez Canillas: > > Add support to convert 8-bit grayscale to reversed monochrome for drivers > > that control monochromatic displays, that only have 1 bit per pixel depth. > > > > This helper function was based on repaper_gray8_to_mono_reversed() from > > the drivers/gpu/drm/tiny/repaper.c driver. > > You could convert repaper to the new helper. > > > > > Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> > > --- > > > > drivers/gpu/drm/drm_format_helper.c | 35 +++++++++++++++++++++++++++++ > > include/drm/drm_format_helper.h | 2 ++ > > 2 files changed, 37 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c > > index 0f28dd2bdd72..bf477c136082 100644 > > --- a/drivers/gpu/drm/drm_format_helper.c > > +++ b/drivers/gpu/drm/drm_format_helper.c > > @@ -584,3 +584,38 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_for > > return -EINVAL; > > } > > EXPORT_SYMBOL(drm_fb_blit_toio); > > + > > +/** > > + * drm_fb_gray8_to_mono_reversed - Convert grayscale to reversed monochrome > > + * @dst: reversed monochrome destination buffer > > + * @src: 8-bit grayscale source buffer > > + * @clip: Clip rectangle area to copy > > + * > > + * DRM doesn't have native monochrome or grayscale support. > > + * Such drivers can announce the commonly supported XR24 format to userspace > > + * and use drm_fb_xrgb8888_to_gray8() to convert to grayscale and then this > > + * helper function to convert to the native format. > > + */ > > +void drm_fb_gray8_to_mono_reversed(void *dst, void *src, const struct drm_rect *clip) > > IMHO it would be better to have a function that converts xrgb8888 to > mono and let it handle the intermediate step of gray8. > > > +{ > > + size_t width = drm_rect_width(clip); > > + size_t height = drm_rect_width(clip); > > + > > + u8 *mono = dst, *gray8 = src; > > + unsigned int y, xb, i; > > + > > + for (y = 0; y < height; y++) > > + for (xb = 0; xb < width / 8; xb++) { > > The inner loop should probably go into a separate helper function. See > the drm_fb_*_line() helpers in this file > > At some point, we's want to have a single blit helper that takes source > and destination formats/buffers. It would then pick the correct per-line > helper for the conversion. So yeah, we'd want something composable. Btw. VKMS is going to do blending, and it needs to support various formats, so the latest patches from Igor probably do something similar. Thanks, pq > > Best regards > Thomas > > > + u8 byte = 0x00; > > + > > + for (i = 0; i < 8; i++) { > > + int x = xb * 8 + i; > > + > > + byte >>= 1; > > + if (gray8[y * width + x] >> 7) > > + byte |= BIT(7); > > + } > > + *mono++ = byte; > > + } > > +} > > +EXPORT_SYMBOL(drm_fb_gray8_to_mono_reversed);
Hello Thomas, Thanks a lot for your feedback. On 2/1/22 10:59, Thomas Zimmermann wrote: > Hi > > Am 31.01.22 um 21:12 schrieb Javier Martinez Canillas: >> Add support to convert 8-bit grayscale to reversed monochrome for drivers >> that control monochromatic displays, that only have 1 bit per pixel depth. >> >> This helper function was based on repaper_gray8_to_mono_reversed() from >> the drivers/gpu/drm/tiny/repaper.c driver. > > You could convert repaper to the new helper. > Yes, I plan to do it but was out of scope for this patch series. >> >> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> >> --- >> >> drivers/gpu/drm/drm_format_helper.c | 35 +++++++++++++++++++++++++++++ >> include/drm/drm_format_helper.h | 2 ++ >> 2 files changed, 37 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c >> index 0f28dd2bdd72..bf477c136082 100644 >> --- a/drivers/gpu/drm/drm_format_helper.c >> +++ b/drivers/gpu/drm/drm_format_helper.c >> @@ -584,3 +584,38 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_for >> return -EINVAL; >> } >> EXPORT_SYMBOL(drm_fb_blit_toio); >> + >> +/** >> + * drm_fb_gray8_to_mono_reversed - Convert grayscale to reversed monochrome >> + * @dst: reversed monochrome destination buffer >> + * @src: 8-bit grayscale source buffer >> + * @clip: Clip rectangle area to copy >> + * >> + * DRM doesn't have native monochrome or grayscale support. >> + * Such drivers can announce the commonly supported XR24 format to userspace >> + * and use drm_fb_xrgb8888_to_gray8() to convert to grayscale and then this >> + * helper function to convert to the native format. >> + */ >> +void drm_fb_gray8_to_mono_reversed(void *dst, void *src, const struct drm_rect *clip) > > IMHO it would be better to have a function that converts xrgb8888 to > mono and let it handle the intermediate step of gray8. > That's a good idea. I'll add that too. >> +{ >> + size_t width = drm_rect_width(clip); >> + size_t height = drm_rect_width(clip); >> + >> + u8 *mono = dst, *gray8 = src; >> + unsigned int y, xb, i; >> + >> + for (y = 0; y < height; y++) >> + for (xb = 0; xb < width / 8; xb++) { > > The inner loop should probably go into a separate helper function. See > the drm_fb_*_line() helpers in this file > > At some point, we's want to have a single blit helper that takes source > and destination formats/buffers. It would then pick the correct per-line > helper for the conversion. So yeah, we'd want something composable. > Agreed. I'll split that into a separate helper function. Best regards,
Hi Javier, On Mon, Jan 31, 2022 at 9:12 PM Javier Martinez Canillas <javierm@redhat.com> wrote: > Add support to convert 8-bit grayscale to reversed monochrome for drivers > that control monochromatic displays, that only have 1 bit per pixel depth. > > This helper function was based on repaper_gray8_to_mono_reversed() from > the drivers/gpu/drm/tiny/repaper.c driver. > > Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> > --- a/drivers/gpu/drm/drm_format_helper.c > +++ b/drivers/gpu/drm/drm_format_helper.c > @@ -584,3 +584,38 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_for > return -EINVAL; > } > EXPORT_SYMBOL(drm_fb_blit_toio); > + > +/** > + * drm_fb_gray8_to_mono_reversed - Convert grayscale to reversed monochrome > + * @dst: reversed monochrome destination buffer What's the meaning of "reversed"? During the last few days, I've been balancing between (a) "reverse video" and (b) "reverse bit order", but none of them seems to be true. (a) The code maps 0-127 to 0 and 8-255 to 1, which just reduces from 256 to 2 grayscale levels, without inversion. The result is also white-on-black on my ssd130x OLED. (b) On little-endian, the CFB drawops use little-endian bit order, which is what ends up in "byte" in the code below. > + * @src: 8-bit grayscale source buffer > + * @clip: Clip rectangle area to copy > + * > + * DRM doesn't have native monochrome or grayscale support. > + * Such drivers can announce the commonly supported XR24 format to userspace > + * and use drm_fb_xrgb8888_to_gray8() to convert to grayscale and then this > + * helper function to convert to the native format. > + */ > +void drm_fb_gray8_to_mono_reversed(void *dst, void *src, const struct drm_rect *clip) > +{ > + size_t width = drm_rect_width(clip); > + size_t height = drm_rect_width(clip); > + > + u8 *mono = dst, *gray8 = src; > + unsigned int y, xb, i; > + > + for (y = 0; y < height; y++) > + for (xb = 0; xb < width / 8; xb++) { > + u8 byte = 0x00; > + > + for (i = 0; i < 8; i++) { > + int x = xb * 8 + i; > + > + byte >>= 1; > + if (gray8[y * width + x] >> 7) > + byte |= BIT(7); > + } > + *mono++ = byte; > + } > +} Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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
Hello Geert, On 3/14/22 14:40, Geert Uytterhoeven wrote: > Hi Javier, > > On Mon, Jan 31, 2022 at 9:12 PM Javier Martinez Canillas > <javierm@redhat.com> wrote: >> Add support to convert 8-bit grayscale to reversed monochrome for drivers >> that control monochromatic displays, that only have 1 bit per pixel depth. >> >> This helper function was based on repaper_gray8_to_mono_reversed() from >> the drivers/gpu/drm/tiny/repaper.c driver. >> >> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> > >> --- a/drivers/gpu/drm/drm_format_helper.c >> +++ b/drivers/gpu/drm/drm_format_helper.c >> @@ -584,3 +584,38 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_for >> return -EINVAL; >> } >> EXPORT_SYMBOL(drm_fb_blit_toio); >> + >> +/** >> + * drm_fb_gray8_to_mono_reversed - Convert grayscale to reversed monochrome >> + * @dst: reversed monochrome destination buffer > > What's the meaning of "reversed"? Originally I took this helper from the repaper driver and it was called repaper_gray8_to_mono_reversed(), so the naming just got carried over. > During the last few days, I've been balancing between (a) "reverse > video" and (b) "reverse bit order", but none of them seems to be true. > > (a) The code maps 0-127 to 0 and 8-255 to 1, which just reduces from > 256 to 2 grayscale levels, without inversion. The result is also > white-on-black on my ssd130x OLED. > (b) On little-endian, the CFB drawops use little-endian bit order, > which is what ends up in "byte" in the code below. > As far as I understand is (a) from the options since the repaper display uses black-on-white. But as you pointed out the ssd130x OLEDs instead do white-on-black. I should had probably just name the helper drm_fb_gray8_to_monochrome() or maybe drm_fb_gray8_to_gray1() ?
diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index 0f28dd2bdd72..bf477c136082 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -584,3 +584,38 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_for return -EINVAL; } EXPORT_SYMBOL(drm_fb_blit_toio); + +/** + * drm_fb_gray8_to_mono_reversed - Convert grayscale to reversed monochrome + * @dst: reversed monochrome destination buffer + * @src: 8-bit grayscale source buffer + * @clip: Clip rectangle area to copy + * + * DRM doesn't have native monochrome or grayscale support. + * Such drivers can announce the commonly supported XR24 format to userspace + * and use drm_fb_xrgb8888_to_gray8() to convert to grayscale and then this + * helper function to convert to the native format. + */ +void drm_fb_gray8_to_mono_reversed(void *dst, void *src, const struct drm_rect *clip) +{ + size_t width = drm_rect_width(clip); + size_t height = drm_rect_width(clip); + + u8 *mono = dst, *gray8 = src; + unsigned int y, xb, i; + + for (y = 0; y < height; y++) + for (xb = 0; xb < width / 8; xb++) { + u8 byte = 0x00; + + for (i = 0; i < 8; i++) { + int x = xb * 8 + i; + + byte >>= 1; + if (gray8[y * width + x] >> 7) + byte |= BIT(7); + } + *mono++ = byte; + } +} +EXPORT_SYMBOL(drm_fb_gray8_to_mono_reversed); diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h index b30ed5de0a33..cd4c8b7c78de 100644 --- a/include/drm/drm_format_helper.h +++ b/include/drm/drm_format_helper.h @@ -43,4 +43,6 @@ 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, void *vaddr, const struct drm_rect *clip); + #endif /* __LINUX_DRM_FORMAT_HELPER_H */
Add support to convert 8-bit grayscale to reversed monochrome for drivers that control monochromatic displays, that only have 1 bit per pixel depth. This helper function was based on repaper_gray8_to_mono_reversed() from the drivers/gpu/drm/tiny/repaper.c driver. Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> --- drivers/gpu/drm/drm_format_helper.c | 35 +++++++++++++++++++++++++++++ include/drm/drm_format_helper.h | 2 ++ 2 files changed, 37 insertions(+)