[PATCH v2 3/4] drm/tinydrm: Add tinydrm_xrgb8888_to_gray8() helper

[Noralf Trønnes]

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.

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.


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