Den 10.03.2021 05.55, skrev Peter Stuge:
> Noralf Trønnes wrote:
>>> Depending on how long it takes for the DMA mask dependency patch to show
>>> up in drm-misc-next, I will either publish a new version or apply the
>>> current and provide patches with the necessary fixes.
>>
>> In case I apply this version, are you happy enough with it that you want
>> to give an ack or r-b?
>
> I've now tested R1 and RGB111 and I think I've found two more things:
>
> I didn't receive the expected bits/bytes for RGB111 on the bulk endpoint,
> I think because of how components were extracted in gud_xrgb8888_to_color().
>
> Changing to the following gets me the expected (X R1 G1 B1 X R2 G2 B2) bytes:
>
> r = (*pix32 >> 8) & 0xff;
> g = (*pix32 >> 16) & 0xff;
> b = (*pix32++ >> 24) & 0xff;
>
We're accessing the whole word here through pix32, no byte access, so
endianess doesn't come into play. This change will flip r and b, which
gives: XRGB8888 -> XBGR1111
BGR is a common thing on controllers, are you sure yours are set to RGB
and not BGR?
And the 0xff masking isn't necessary since we're assigning to a byte, right?
I haven't got a native R1G1B1 display so I have emulated and I do get
the expected colors. This is the conversion function I use on the device
which I think is correct:
static size_t rgb111_to_rgb565(uint16_t *dst, uint8_t *src,
uint16_t src_width, uint16_t src_height)
{
uint8_t rgb111, val = 0;
size_t len = 0;
for (uint16_t y = 0; y < src_height; y++) {
for (uint16_t x = 0; x < src_width; x++) {
if (!(x % 2))
val = *src++;
rgb111 = val >> 4;
*dst++ = ((rgb111 & 0x04) << 13) | ((rgb111 & 0x02) << 9) |
((rgb111 & 0x01) << 4);
len += sizeof(*dst);
val <<= 4;
}
}
return len;
}
>
> Then, gud_xrgb8888_to_color() and maybe even gud_xrgb8888_to_r124() seem
> to be host endian dependent, at least because of that pix32, but maybe more?
> I don't know whether drm guarantees "native" XRGB byte sequence in memory,
> then I guess the pix32 is okay? Please take a look?
>
>
> Finally my very last ask: Please consider renaming GUD_PIXEL_FORMAT_RGB111
> to GUD_PIXEL_FORMAT_XRGB1111?
>
It has crossed my mind, I'll change it.
Noralf.
