On 10 May 2014 00:35, Peter Crosthwaite <[email protected]> wrote: > On Fri, May 9, 2014 at 4:46 AM, Peter Maydell <[email protected]> > wrote: >> The pxa2xx palette entry "16bpp plus transparency" format is >> xxxxxxxTRRRRR000GGGGGG00BBBBB000, and "18bpp plus transparency" is >> xxxxxxxTRRRRRR00GGGGGG00BBBBBB00. >> >> Correct errors in the code for reading these and converting >> them to the internal format. In particular, the buggy code >> was attempting to mask out bit 24 of a uint16_t, which >> Coverity spotted as an error. >> >> Signed-off-by: Peter Maydell <[email protected]> >> --- >> hw/display/pxa2xx_lcd.c | 14 +++++++------- >> 1 file changed, 7 insertions(+), 7 deletions(-) >> >> diff --git a/hw/display/pxa2xx_lcd.c b/hw/display/pxa2xx_lcd.c >> index 09cdf17..fce013d 100644 >> --- a/hw/display/pxa2xx_lcd.c >> +++ b/hw/display/pxa2xx_lcd.c >> @@ -620,13 +620,13 @@ static void pxa2xx_palette_parse(PXA2xxLCDState *s, >> int ch, int bpp) >> src += 2; >> break; >> case 1: /* 16 bpp plus transparency */ >> - alpha = *(uint16_t *) src & (1 << 24); >> + alpha = *(uint32_t *) src & (1 << 24); >> if (s->control[0] & LCCR0_CMS) >> - r = g = b = *(uint16_t *) src & 0xff; >> + r = g = b = *(uint32_t *) src & 0xff; >> else { >> - r = (*(uint16_t *) src & 0xf800) >> 8; >> - g = (*(uint16_t *) src & 0x07e0) >> 3; >> - b = (*(uint16_t *) src & 0x001f) << 3; >> + r = (*(uint32_t *) src & 0x7c0000) >> 15; > > 16BPP format (pasted from above with byte spacing) > xxxxxxxT RRRRR000 GGGGGG00 BBBBB000 > > So shouldn't r be 0xf80000 >> 16?
Yep, you're right. My first attempt at this I read the wrong diagram from the manual (the framebuffer pixel formats are different from the palette formats) and failed to spot I hadn't corrected the code completely. Since this is the only patch in this set that needs a respin, and they're all independent fixes, I'll put the other 7 into target-arm.next and just resend this. thanks -- PMM
