Den 28.06.2017 19.12, skrev Daniel Vetter:
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 <nor...@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.

I'm volunteering myself this time :-)
Much better to write a library that smart people will improve upon,
than to put something in tinydrm that nobody looks at.

A couple of questions:

- Can the pages be vmap'ed at object creation time, or should this be
  an explicit function call? I see some drivers being explicit, but
  tinydrm will always need that virtual address.

- Several drivers have cache flags on the gem object used when mmap'ing,
like udl here:

static void update_vm_cache_attr(struct udl_gem_object *obj,
                 struct vm_area_struct *vma)
{
    /* non-cacheable as default. */
    if (obj->flags & UDL_BO_CACHEABLE) {
        vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
    } else if (obj->flags & UDL_BO_WC) {
        vma->vm_page_prot =
            pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
    } else {
        vma->vm_page_prot =
            pgprot_noncached(vm_get_page_prot(vma->vm_flags));
    }
}

Should I do this in the gem shmem library?
drm_driver->gem_create_object can be used by the driver to set the flag.

- And I assume it's better to create a new library instead of
  extending the CMA library? Even though most of the code will be the same.
  Like this: drm_gem_shmem_helper.{c,h} and drm_fb_shmem_helper.{c,h}


Noralf.

-Daniel


Noralf.


Acked-by: Daniel Vetter <daniel.vet...@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to