| Message ID | 20220315110707.628166-2-geert@linux-m68k.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: Fix monochrome conversion for sdd130x | expand |
Hello Geert, Thanks for your patch. On 3/15/22 12:07, Geert Uytterhoeven wrote: > There is no "reversed" handling in drm_fb_xrgb8888_to_mono_reversed(): > the function just converts from color to grayscale, and reduces the > number of grayscale levels from 256 to 2 (i.e. brightness 0-127 is > mapped to 0, 128-255 to 1). All "reversed" handling is done in the > repaper driver, where this function originated. > > Hence make this clear by renaming drm_fb_xrgb8888_to_mono_reversed() to > drm_fb_xrgb8888_to_mono(), and documenting the black/white pixel > mapping. > > Fixes: bcf8b616deb87941 ("drm/format-helper: Add drm_fb_xrgb8888_to_mono_reversed()") > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > --- As you mentioned the function originally came from the repaper driver (that uses white-on-black) and I wrongly assumed that the destination buffer for the OLED panels were also monochrome reversed. Acked-by: Javier Martinez Canillas <javierm@redhat.com>
On Tue, Mar 15, 2022 at 12:07:03PM +0100, Geert Uytterhoeven wrote: > There is no "reversed" handling in drm_fb_xrgb8888_to_mono_reversed(): > the function just converts from color to grayscale, and reduces the > number of grayscale levels from 256 to 2 (i.e. brightness 0-127 is > mapped to 0, 128-255 to 1). All "reversed" handling is done in the > repaper driver, where this function originated. > > Hence make this clear by renaming drm_fb_xrgb8888_to_mono_reversed() to > drm_fb_xrgb8888_to_mono(), and documenting the black/white pixel > mapping. W/ or w/o the below remark being addressed Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Fixes: bcf8b616deb87941 ("drm/format-helper: Add drm_fb_xrgb8888_to_mono_reversed()") > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > --- > drivers/gpu/drm/drm_format_helper.c | 32 ++++++++++++++--------------- > drivers/gpu/drm/solomon/ssd130x.c | 2 +- > drivers/gpu/drm/tiny/repaper.c | 2 +- > include/drm/drm_format_helper.h | 5 ++--- > 4 files changed, 20 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c > index bc0f49773868a9b0..b68aa857c6514529 100644 > --- a/drivers/gpu/drm/drm_format_helper.c > +++ b/drivers/gpu/drm/drm_format_helper.c > @@ -594,8 +594,8 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_for > } > EXPORT_SYMBOL(drm_fb_blit_toio); > > -static void drm_fb_gray8_to_mono_reversed_line(u8 *dst, const u8 *src, unsigned int pixels, > - unsigned int start_offset, unsigned int end_len) > +static void drm_fb_gray8_to_mono_line(u8 *dst, const u8 *src, unsigned int pixels, > + unsigned int start_offset, unsigned int end_len) > { > unsigned int xb, i; > > @@ -621,8 +621,8 @@ static void drm_fb_gray8_to_mono_reversed_line(u8 *dst, const u8 *src, unsigned > } > > /** > - * drm_fb_xrgb8888_to_mono_reversed - Convert XRGB8888 to reversed monochrome > - * @dst: reversed monochrome destination buffer > + * drm_fb_xrgb8888_to_mono - Convert XRGB8888 to monochrome > + * @dst: monochrome destination buffer (0=black, 1=white) > * @dst_pitch: Number of bytes between two consecutive scanlines within dst > * @src: XRGB8888 source buffer > * @fb: DRM framebuffer > @@ -633,10 +633,10 @@ static void drm_fb_gray8_to_mono_reversed_line(u8 *dst, const u8 *src, unsigned > * 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. > + * then the result is converted from grayscale to monochrome. > */ > -void drm_fb_xrgb8888_to_mono_reversed(void *dst, unsigned int dst_pitch, const void *vaddr, > - const struct drm_framebuffer *fb, const struct drm_rect *clip) > +void drm_fb_xrgb8888_to_mono(void *dst, unsigned int dst_pitch, const void *vaddr, > + const struct drm_framebuffer *fb, const struct drm_rect *clip) > { > unsigned int linepixels = drm_rect_width(clip); > unsigned int lines = clip->y2 - clip->y1; > @@ -652,8 +652,8 @@ void drm_fb_xrgb8888_to_mono_reversed(void *dst, unsigned int dst_pitch, const v > return; > > /* > - * The reversed mono destination buffer contains 1 bit per pixel > - * and destination scanlines have to be in multiple of 8 pixels. > + * The mono destination buffer contains 1 bit per pixel and > + * destination scanlines have to be in multiple of 8 pixels. > */ > if (!dst_pitch) > dst_pitch = DIV_ROUND_UP(linepixels, 8); > @@ -664,9 +664,9 @@ void drm_fb_xrgb8888_to_mono_reversed(void *dst, unsigned int dst_pitch, const v > * The cma memory is write-combined so reads are uncached. > * Speed up by fetching one line at a time. > * > - * Also, format conversion from XR24 to reversed monochrome > - * are done line-by-line but are converted to 8-bit grayscale > - * as an intermediate step. > + * Also, format conversion from XR24 to monochrome are done > + * line-by-line but are converted to 8-bit grayscale as an > + * intermediate step. > * > * Allocate a buffer to be used for both copying from the cma > * memory and to store the intermediate grayscale line pixels. > @@ -683,7 +683,7 @@ void drm_fb_xrgb8888_to_mono_reversed(void *dst, unsigned int dst_pitch, const v > * are not aligned to multiple of 8. > * > * Calculate if the start and end pixels are not aligned and set the > - * offsets for the reversed mono line conversion function to adjust. > + * offsets for the mono line conversion function to adjust. > */ > start_offset = clip->x1 % 8; > end_len = clip->x2 % 8; > @@ -692,12 +692,12 @@ void drm_fb_xrgb8888_to_mono_reversed(void *dst, unsigned int dst_pitch, const v > for (y = 0; y < lines; y++) { > src32 = memcpy(src32, vaddr, len_src32); > drm_fb_xrgb8888_to_gray8_line(gray8, src32, linepixels); > - drm_fb_gray8_to_mono_reversed_line(mono, gray8, dst_pitch, > - start_offset, end_len); > + drm_fb_gray8_to_mono_line(mono, gray8, dst_pitch, start_offset, > + end_len); Can be one line now (definition is already quite behind 80 limit). > vaddr += fb->pitches[0]; > mono += dst_pitch; > } > > kfree(src32); > } > -EXPORT_SYMBOL(drm_fb_xrgb8888_to_mono_reversed); > +EXPORT_SYMBOL(drm_fb_xrgb8888_to_mono); > diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c > index d08d86ef07bcbe7f..caee851efd5726e7 100644 > --- a/drivers/gpu/drm/solomon/ssd130x.c > +++ b/drivers/gpu/drm/solomon/ssd130x.c > @@ -458,7 +458,7 @@ static int ssd130x_fb_blit_rect(struct drm_framebuffer *fb, const struct iosys_m > if (!buf) > return -ENOMEM; > > - drm_fb_xrgb8888_to_mono_reversed(buf, 0, vmap, fb, rect); > + drm_fb_xrgb8888_to_mono(buf, 0, vmap, fb, rect); > > ssd130x_update_rect(ssd130x, buf, rect); > > diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c > index 37b6bb90e46e1fe8..a096fb8b83e99dc8 100644 > --- a/drivers/gpu/drm/tiny/repaper.c > +++ b/drivers/gpu/drm/tiny/repaper.c > @@ -540,7 +540,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb) > if (ret) > goto out_free; > > - drm_fb_xrgb8888_to_mono_reversed(buf, 0, cma_obj->vaddr, fb, &clip); > + drm_fb_xrgb8888_to_mono(buf, 0, cma_obj->vaddr, fb, &clip); > > drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE); > > diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h > index 0b0937c0b2f6324e..55145eca07828321 100644 > --- a/include/drm/drm_format_helper.h > +++ b/include/drm/drm_format_helper.h > @@ -43,8 +43,7 @@ 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_xrgb8888_to_mono_reversed(void *dst, unsigned int dst_pitch, const void *src, > - const struct drm_framebuffer *fb, > - const struct drm_rect *clip); > +void drm_fb_xrgb8888_to_mono(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 */ > -- > 2.25.1 >
Hi Andy, On Tue, Mar 15, 2022 at 2:33 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Tue, Mar 15, 2022 at 12:07:03PM +0100, Geert Uytterhoeven wrote: > > There is no "reversed" handling in drm_fb_xrgb8888_to_mono_reversed(): > > the function just converts from color to grayscale, and reduces the > > number of grayscale levels from 256 to 2 (i.e. brightness 0-127 is > > mapped to 0, 128-255 to 1). All "reversed" handling is done in the > > repaper driver, where this function originated. > > > > Hence make this clear by renaming drm_fb_xrgb8888_to_mono_reversed() to > > drm_fb_xrgb8888_to_mono(), and documenting the black/white pixel > > mapping. > > W/ or w/o the below remark being addressed > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Thanks! > > --- a/drivers/gpu/drm/drm_format_helper.c > > +++ b/drivers/gpu/drm/drm_format_helper.c > > @@ -692,12 +692,12 @@ void drm_fb_xrgb8888_to_mono_reversed(void *dst, unsigned int dst_pitch, const v > > for (y = 0; y < lines; y++) { > > src32 = memcpy(src32, vaddr, len_src32); > > drm_fb_xrgb8888_to_gray8_line(gray8, src32, linepixels); > > - drm_fb_gray8_to_mono_reversed_line(mono, gray8, dst_pitch, > > - start_offset, end_len); > > > + drm_fb_gray8_to_mono_line(mono, gray8, dst_pitch, start_offset, > > + end_len); > > Can be one line now (definition is already quite behind 80 limit). Yeah, but the code isn't. Nevertheless, this will be shortened in a later patch. 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
diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index bc0f49773868a9b0..b68aa857c6514529 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -594,8 +594,8 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_for } EXPORT_SYMBOL(drm_fb_blit_toio); -static void drm_fb_gray8_to_mono_reversed_line(u8 *dst, const u8 *src, unsigned int pixels, - unsigned int start_offset, unsigned int end_len) +static void drm_fb_gray8_to_mono_line(u8 *dst, const u8 *src, unsigned int pixels, + unsigned int start_offset, unsigned int end_len) { unsigned int xb, i; @@ -621,8 +621,8 @@ static void drm_fb_gray8_to_mono_reversed_line(u8 *dst, const u8 *src, unsigned } /** - * drm_fb_xrgb8888_to_mono_reversed - Convert XRGB8888 to reversed monochrome - * @dst: reversed monochrome destination buffer + * drm_fb_xrgb8888_to_mono - Convert XRGB8888 to monochrome + * @dst: monochrome destination buffer (0=black, 1=white) * @dst_pitch: Number of bytes between two consecutive scanlines within dst * @src: XRGB8888 source buffer * @fb: DRM framebuffer @@ -633,10 +633,10 @@ static void drm_fb_gray8_to_mono_reversed_line(u8 *dst, const u8 *src, unsigned * 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. + * then the result is converted from grayscale to monochrome. */ -void drm_fb_xrgb8888_to_mono_reversed(void *dst, unsigned int dst_pitch, const void *vaddr, - const struct drm_framebuffer *fb, const struct drm_rect *clip) +void drm_fb_xrgb8888_to_mono(void *dst, unsigned int dst_pitch, const void *vaddr, + const struct drm_framebuffer *fb, const struct drm_rect *clip) { unsigned int linepixels = drm_rect_width(clip); unsigned int lines = clip->y2 - clip->y1; @@ -652,8 +652,8 @@ void drm_fb_xrgb8888_to_mono_reversed(void *dst, unsigned int dst_pitch, const v return; /* - * The reversed mono destination buffer contains 1 bit per pixel - * and destination scanlines have to be in multiple of 8 pixels. + * The mono destination buffer contains 1 bit per pixel and + * destination scanlines have to be in multiple of 8 pixels. */ if (!dst_pitch) dst_pitch = DIV_ROUND_UP(linepixels, 8); @@ -664,9 +664,9 @@ void drm_fb_xrgb8888_to_mono_reversed(void *dst, unsigned int dst_pitch, const v * The cma memory is write-combined so reads are uncached. * Speed up by fetching one line at a time. * - * Also, format conversion from XR24 to reversed monochrome - * are done line-by-line but are converted to 8-bit grayscale - * as an intermediate step. + * Also, format conversion from XR24 to monochrome are done + * line-by-line but are converted to 8-bit grayscale as an + * intermediate step. * * Allocate a buffer to be used for both copying from the cma * memory and to store the intermediate grayscale line pixels. @@ -683,7 +683,7 @@ void drm_fb_xrgb8888_to_mono_reversed(void *dst, unsigned int dst_pitch, const v * are not aligned to multiple of 8. * * Calculate if the start and end pixels are not aligned and set the - * offsets for the reversed mono line conversion function to adjust. + * offsets for the mono line conversion function to adjust. */ start_offset = clip->x1 % 8; end_len = clip->x2 % 8; @@ -692,12 +692,12 @@ void drm_fb_xrgb8888_to_mono_reversed(void *dst, unsigned int dst_pitch, const v for (y = 0; y < lines; y++) { src32 = memcpy(src32, vaddr, len_src32); drm_fb_xrgb8888_to_gray8_line(gray8, src32, linepixels); - drm_fb_gray8_to_mono_reversed_line(mono, gray8, dst_pitch, - start_offset, end_len); + drm_fb_gray8_to_mono_line(mono, gray8, dst_pitch, start_offset, + end_len); vaddr += fb->pitches[0]; mono += dst_pitch; } kfree(src32); } -EXPORT_SYMBOL(drm_fb_xrgb8888_to_mono_reversed); +EXPORT_SYMBOL(drm_fb_xrgb8888_to_mono); diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c index d08d86ef07bcbe7f..caee851efd5726e7 100644 --- a/drivers/gpu/drm/solomon/ssd130x.c +++ b/drivers/gpu/drm/solomon/ssd130x.c @@ -458,7 +458,7 @@ static int ssd130x_fb_blit_rect(struct drm_framebuffer *fb, const struct iosys_m if (!buf) return -ENOMEM; - drm_fb_xrgb8888_to_mono_reversed(buf, 0, vmap, fb, rect); + drm_fb_xrgb8888_to_mono(buf, 0, vmap, fb, rect); ssd130x_update_rect(ssd130x, buf, rect); diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c index 37b6bb90e46e1fe8..a096fb8b83e99dc8 100644 --- a/drivers/gpu/drm/tiny/repaper.c +++ b/drivers/gpu/drm/tiny/repaper.c @@ -540,7 +540,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb) if (ret) goto out_free; - drm_fb_xrgb8888_to_mono_reversed(buf, 0, cma_obj->vaddr, fb, &clip); + drm_fb_xrgb8888_to_mono(buf, 0, cma_obj->vaddr, fb, &clip); drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE); diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h index 0b0937c0b2f6324e..55145eca07828321 100644 --- a/include/drm/drm_format_helper.h +++ b/include/drm/drm_format_helper.h @@ -43,8 +43,7 @@ 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_xrgb8888_to_mono_reversed(void *dst, unsigned int dst_pitch, const void *src, - const struct drm_framebuffer *fb, - const struct drm_rect *clip); +void drm_fb_xrgb8888_to_mono(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 */
There is no "reversed" handling in drm_fb_xrgb8888_to_mono_reversed(): the function just converts from color to grayscale, and reduces the number of grayscale levels from 256 to 2 (i.e. brightness 0-127 is mapped to 0, 128-255 to 1). All "reversed" handling is done in the repaper driver, where this function originated. Hence make this clear by renaming drm_fb_xrgb8888_to_mono_reversed() to drm_fb_xrgb8888_to_mono(), and documenting the black/white pixel mapping. Fixes: bcf8b616deb87941 ("drm/format-helper: Add drm_fb_xrgb8888_to_mono_reversed()") Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> --- drivers/gpu/drm/drm_format_helper.c | 32 ++++++++++++++--------------- drivers/gpu/drm/solomon/ssd130x.c | 2 +- drivers/gpu/drm/tiny/repaper.c | 2 +- include/drm/drm_format_helper.h | 5 ++--- 4 files changed, 20 insertions(+), 21 deletions(-)