[stm32f4-ltdc] this fixes ltdc_get_rgb888_from_rgb565 return value type

[h2obrain]

... and adds ltdc_get_rgb565_from_rgb888

This small fix renames the misleading rgb888 argument to rgb565 and changes the return value type to uint32_t since 3*8 bits do not fit into 16 bits.

Additionally I added the opposite function.

(This bug was probably introduced during the conversion from macro to static inline..)

[schodet]

It does not seems to work, for example if blue is 31: (31 << 8) / 31 = 256, that's too much by 1.
In the other function if blue is 255 : (255 * 31) >> 8 => 30, that's too low by 1.
Did I miss something?

[ChuckM]

Seems a bit like you are moving the bits more than needed, have you tried something like

    return (uint16_t)(
            ((rgb888 & 0xf80000) >> 16)  | /* grab upper 5 bits of red)
            ((rgb888 & 0xfc00) >> 5) | /* grab 6 bits of green */
            ((rgb888 & 0xf8) >> 3); /* grab 5 bits of blue */
    );

[h2obrain]

okay, I did some simulation for possible conversion functions in python

def rgb888_from_rgb565_old(rgb565) :
    return (((((rgb565) & 0xF800) >> (11-8))/31)<<16) \
         | (((((rgb565) & 0x07E0) <<  (8-5))/63)<<8)  \
         | (((((rgb565) & 0x001F) <<  (8-0))/31)<<0)

def rgb888_from_rgb565_standard(rgb565) :
    return (((((rgb565 & 0xF800) >> 11) * 255 + 15) / 31) << 16) \
         | (((((rgb565 & 0x07E0) >>  5) * 255 + 31) / 63) << 8)  \
         | (((((rgb565 & 0x001F) >>  0) * 255 + 15) / 31) << 0)

def rgb888_from_rgb565_simplest(rgb565) :
    return ((rgb565 & 0xF800) << (16 - 11 + 8 - 5)) \
         | ((rgb565 & 0x07E0) << (8  -  5 + 8 - 6)) \
         | ((rgb565 & 0x001F) << (0  -  0 + 8 - 5))

def rgb888_from_rgb565_old_fixed(rgb565) :
    r = ((((rgb565) & 0xF800) >> (11-8))/31)
    g = ((((rgb565) & 0x07E0) <<  (8-5))/63)
    b = ((((rgb565) & 0x001F) <<  (8-0))/31)

    if (r==256) : r = 255;
    if (g==256) : g = 255;
    if (b==256) : b = 255;

    return (r<<16) | (g<<8) | b

def rgb565_from_rgb888_simplest(rgb888) :
    return  ((rgb888 & 0xf80000) >> (16 - 11 + (8-5))) \
          | ((rgb888 & 0x00fc00) >> (8  -  5 + (8-6))) \
          | ((rgb888 & 0x0000f8) >> (0  -  0 + (8-5)))

def test_conversion(rgb565) :
    print "##################################################"
    print "conversion of 0x%04X" %rgb565
    print "0x%06X old"           %rgb888_from_rgb565_old(rgb565)
    print "0x%06X standard"      %rgb888_from_rgb565_standard(rgb565)
    print "0x%06X simplest"      %rgb888_from_rgb565_simplest(rgb565)
    print "0x%06X old fixed"     %rgb888_from_rgb565_old_fixed(rgb565)
    print
    rgb888 = rgb888_from_rgb565_standard(rgb565)
    print "conversion of 0x%06X" %rgb888
    print "0x%04X simplest"      %rgb565_from_rgb888_simplest(rgb888)
    print

# run tests
test_conversion(0x01ad)
test_conversion(0)
test_conversion(0xffff)

results :

##################################################
conversion of 0x01AD
0x00346B old
0x00356B standard
0x003468 simplest
0x00346B old fixed

conversion of 0x00356B
0x01AD simplest

##################################################
conversion of 0x0000
0x000000 old
0x000000 standard
0x000000 simplest
0x000000 old fixed

conversion of 0x000000
0x0000 simplest

##################################################
conversion of 0xFFFF
0x1010100 old
0xFFFFFF standard
0xF8FCF8 simplest
0xFFFFFF old fixed

conversion of 0xFFFFFF
0xFFFF simplest

the fixed PR uses rgb888_from_rgb565_old_fixed as rgb565-to-rgb888 function and rgb565_from_rgb888_simplest as rgb888-to-rgb565 function. to avoid future 'fixes' i also added the more standard rgb565-to-rgb888 conversion function rgb888_from_rgb565_standard

[karlp]

to or from?

[h2obrain]

ups, i meant from
this function could be dropped to avoid confusion

[karlp]

Is it immediately clear to the user what they should use in all cases? Would it perhaps make more sense to drop these from the headers and include them in some example code instead? Where it can be more clear what version you're using where and why?

[h2obrain]

Set the color key

Use case 1 - Select a color-key in rgb565

• Get rgb888 using ltdc_get_rgb888_from_rgb565 to set color key. (The hardware will convert this automatically to rgb565 (or active color format))
• Fill the buffer with the selected color-key

Use case 2 - Select a color-key in rgb888

• Set color key (The hardware will convert this automatically to rgb565 (or active color format))
• Get rgb565 using ltdc_get_rgb565_from_rgb888
• Fill the buffer with the converted color-key

When a user wants to convert between color formats, he needs to use the same conversion the hardware does otherwise he will be in trouble. If there is a simpler way to do this than using the provided, I am open to suggestions :). (Drawing a single pixel buffer or so using DMA2D seems like an overkill to me, but in general a conversion done by the hardware itself would be appreciated..)

[karlp]

was anything wrong with this? I realise it fell through the floor, I've never had any devices with screens to even try this, but it doesn't look like anythign was actually wrong with this?

[h2obrain]

Ups.. I did some spring cleaning and did not realise this was still open..
I think this function is useful to have, since you need to set the color keys in rgb888 format.
The current function is broken, so I would fix or delete it.