Re: [PATCH 1/5] drm/tinydrm: Add tinydrm_rgb565_buf_copy()

["Noralf Trønnes" <noralf@tronnes.org>]

Den 12.03.2017 19.00, skrev Daniel Vetter:
> On Sat, Mar 11, 2017 at 10:35:32PM +0100, Noralf Trønnes wrote:
>> Add tinydrm_rgb565_buf_copy() function that copies buffer rectangle to
>> destination buffer and also handles XRGB8888 and byte swap conversions.
>> Useful for displays that only support RGB565 and can do partial updates.
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>   drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 56 +++++++++++++++++++++++++-
>>   include/drm/tinydrm/tinydrm-helpers.h          |  2 +
>>   2 files changed, 56 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..e639453 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,56 @@ void tinydrm_xrgb8888_to_rgb565(u16 *dst, void *vaddr,
>>   EXPORT_SYMBOL(tinydrm_xrgb8888_to_rgb565);
> So I noticed that we already have the xrgb8888 to rgb565 function, so I'm
> a bit late on this, but: DRM doesn't do format conversions, with the
> single exception that the legacy cursor interface is specced to be
> argb8888.
>
> Imo this should be removed (and preferrably before we ship tinydrm in a
> stable kernel). Why did you add it?

I added it from the start because plymouth can only do xrgb8888 and I
thought that this was probably the format that most apps/libs/tools
supported, ensuring that tinydrm would work with everything. But I was
aware that this was the kernel patching up userspace, so I knew that it
might be shot down.

But after your comment, I thought that this was in the clear:
https://lists.freedesktop.org/archives/dri-devel/2017-January/130551.html

 > > +EXPORT_SYMBOL(tinydrm_xrgb8888_to_rgb565);
 >
 > I wonder whether the above would make sense in drm core as some kind 
of fb
 > helpers. But we can do that once there's a clear need.

I can make a patch that removes this format conversion.

Noralf.

> -Daniel

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