[PATCH v2 3/4] drm/tinydrm: Add tinydrm_xrgb8888_to_gray8() helper
Daniel Vetter
daniel at ffwll.ch
Wed Jun 28 17:12:32 UTC 2017
On Wed, Jun 28, 2017 at 04:26:23PM +0200, Noralf Trønnes wrote:
>
> Den 26.06.2017 11.43, skrev Daniel Vetter:
> > On Thu, Jun 08, 2017 at 05:14:34PM +0200, Noralf Trønnes wrote:
> > > Drm has no monochrome or greyscale support so add a conversion
> > > from the common format XR24.
> > >
> > > Also reorder includes into the common order.
> > >
> > > Signed-off-by: Noralf Trønnes <noralf at tronnes.org>
> > > ---
> > > drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 74 +++++++++++++++++++++++++-
> > > include/drm/tinydrm/tinydrm-helpers.h | 1 +
> > > 2 files changed, 73 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> > > index d4cda33..75808bb 100644
> > > --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> > > +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> > > @@ -7,13 +7,15 @@
> > > * (at your option) any later version.
> > > */
> > > -#include <drm/tinydrm/tinydrm.h>
> > > -#include <drm/tinydrm/tinydrm-helpers.h>
> > > #include <linux/backlight.h>
> > > +#include <linux/dma-buf.h>
> > > #include <linux/pm.h>
> > > #include <linux/spi/spi.h>
> > > #include <linux/swab.h>
> > > +#include <drm/tinydrm/tinydrm.h>
> > > +#include <drm/tinydrm/tinydrm-helpers.h>
> > > +
> > > static unsigned int spi_max;
> > > module_param(spi_max, uint, 0400);
> > > MODULE_PARM_DESC(spi_max, "Set a lower SPI max transfer size");
> > > @@ -181,6 +183,74 @@ void tinydrm_xrgb8888_to_rgb565(u16 *dst, void *vaddr,
> > > EXPORT_SYMBOL(tinydrm_xrgb8888_to_rgb565);
> > > /**
> > > + * tinydrm_xrgb8888_to_gray8 - Convert XRGB8888 to grayscale
> > > + * @dst: 8-bit grayscale destination buffer
> > > + * @fb: DRM framebuffer
> > > + *
> > > + * Drm doesn't have native monochrome or grayscale support.
> > > + * Such drivers can announce the commonly supported XR24 format to userspace
> > > + * and use this function to convert to the native format.
> > > + *
> > > + * Monochrome drivers will use the most significant bit,
> > > + * where 1 means foreground color and 0 background color.
> > > + *
> > > + * ITU BT.601 is used for the RGB -> luma (brightness) conversion.
> > > + *
> > > + * Returns:
> > > + * Zero on success, negative error code on failure.
> > > + */
> > > +int tinydrm_xrgb8888_to_gray8(u8 *dst, struct drm_framebuffer *fb)
> > I think it's ok to use this for backwards compat reasons with userspace
> > that simply expects xrgb8888, but adding monochrome fourcc codes to drm
> > isn't hard, and I think would be better to do that here.
> >
> > There's also my comment from earlier that too much of tinydrm is in
> > tinydrm, and maybe we should move some of it out (we already have
> > tinydrm_xrgb8888_to_rgb565). But looks ok otherwise.
>
> I'm not sure that tinydrm_xrgb8888_to_rgb565 and tinydrm_xrgb8888_to_gray8
> does fit in the core as-is, because they work around the fact that
> cma memory has uncached reads by buffering one pixel line at a time.
>
> I have been aware that the cma library isn't really a prefect match for
> tinydrm because of this (but it was easy to use). For tiny displays this
> isn't really a performance problem to speak of, but I see now that it
> limits which gpus that I can import buffers from, since cma requires
> continous memory. This is probably the reason the Grain Media driver
> couldn't use tinydrm or that it needed that shmem thing.
Hm, might be good to port tinydrm to plain gem shmem objects? We have a
bunch of gem shmem based simple drivers know, so if helpers aren't as
comprehensive as with cma that can be fixed (and I'm trying other driver
submitters to volunteer for that already).
> I'll see if I can use gem/prime code from vgem coupled with prime import
> code from udl. The spi core claims to do dma transfers on vmalloc
> addreses, so it seems possible.
Still feeling guilty for volunteering you for everything, but would be
awesome if you can make it happen. I think there's other drivers in-flight
(or even merged) which could benefit from parts of this at least.
-Daniel
>
>
> Noralf.
>
>
> > Acked-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> >
> > Daniel
> >
> >
> > > +{
> > > + struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
> > > + struct dma_buf_attachment *import_attach = cma_obj->base.import_attach;
> > > + unsigned int x, y, pitch = fb->pitches[0];
> > > + int ret = 0;
> > > + void *buf;
> > > + u32 *src;
> > > +
> > > + if (WARN_ON(fb->format->format != DRM_FORMAT_XRGB8888))
> > > + return -EINVAL;
> > > + /*
> > > + * The cma memory is write-combined so reads are uncached.
> > > + * Speed up by fetching one line at a time.
> > > + */
> > > + buf = kmalloc(pitch, GFP_KERNEL);
> > > + if (!buf)
> > > + return -ENOMEM;
> > > +
> > > + if (import_attach) {
> > > + ret = dma_buf_begin_cpu_access(import_attach->dmabuf,
> > > + DMA_FROM_DEVICE);
> > > + if (ret)
> > > + goto err_free;
> > > + }
> > > +
> > > + for (y = 0; y < fb->height; y++) {
> > > + src = cma_obj->vaddr + (y * pitch);
> > > + memcpy(buf, src, pitch);
> > > + src = buf;
> > > + for (x = 0; x < fb->width; x++) {
> > > + u8 r = (*src & 0x00ff0000) >> 16;
> > > + u8 g = (*src & 0x0000ff00) >> 8;
> > > + u8 b = *src & 0x000000ff;
> > > +
> > > + /* ITU BT.601: Y = 0.299 R + 0.587 G + 0.114 B */
> > > + *dst++ = (3 * r + 6 * g + b) / 10;
> > > + src++;
> > > + }
> > > + }
> > > +
> > > + if (import_attach)
> > > + ret = dma_buf_end_cpu_access(import_attach->dmabuf,
> > > + DMA_FROM_DEVICE);
> > > +err_free:
> > > + kfree(buf);
> > > +
> > > + return ret;
> > > +}
> > > +EXPORT_SYMBOL(tinydrm_xrgb8888_to_gray8);
> > > +
> > > +/**
> > > * tinydrm_of_find_backlight - Find backlight device in device-tree
> > > * @dev: Device
> > > *
> > > diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h
> > > index 9b9b6cf..a6c387f 100644
> > > --- a/include/drm/tinydrm/tinydrm-helpers.h
> > > +++ b/include/drm/tinydrm/tinydrm-helpers.h
> > > @@ -43,6 +43,7 @@ void tinydrm_swab16(u16 *dst, void *vaddr, struct drm_framebuffer *fb,
> > > void tinydrm_xrgb8888_to_rgb565(u16 *dst, void *vaddr,
> > > struct drm_framebuffer *fb,
> > > struct drm_clip_rect *clip, bool swap);
> > > +int tinydrm_xrgb8888_to_gray8(u8 *dst, struct drm_framebuffer *fb);
> > > struct backlight_device *tinydrm_of_find_backlight(struct device *dev);
> > > int tinydrm_enable_backlight(struct backlight_device *backlight);
> > > --
> > > 2.7.4
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the dri-devel
mailing list