atmel_lcdfb: support 16bit BGR:565 mode, remove unsupported 15bit modes

quoted

quoted

quoted

quoted

"Nicolas" = Nicolas Ferre [off-list ref] writes:

 Nicolas> On 10/13/2011 04:52 PM, Peter Korsgaard :
 >> Allow framebuffer to be configured in 16bit mode when panel is wired in
 >> (the default) BGR configuration, and don't claim to support 15bit input
 >> modes, which the LCD controller cannot handle.
 >> 
 >> Signed-off-by: Peter Korsgaard [off-list ref]

 Nicolas> Hi Peter,

 Nicolas> Sorry for not having more responsive concerning the two
 Nicolas> patches that you posted about atmel_lcdfb driver.

No problem.

 Nicolas> I have a question though about this one...

 >> -		} else if (sinfo->lcd_wiring_mode = ATMEL_LCDC_WIRING_RGB555) {
 >> -			var->red.offset = 10;
 >> -			var->blue.offset = 0;
 >> -			var->green.length = 5;

 Nicolas> Maybe I have missed something but I do not know why you are removing
 Nicolas> this part of the configuration? A board at least is using this wiring
 Nicolas> mode...

Because it is arguable wrong as far as I understand the HW.
There's two parts here:

- 1: Format of framebuffer memory
- 2: Wiring of LCD (RGB/BGR order and number of bits)

From the datasheet, the following framebuffer formats are supported:

1, 2, 4, 8 bits per pixel (palletized), 16, 24 bits per pixel
(non-palletized) for TFT.

So it doesn't really support RGB555 mode. The controller reads up to
32bit of framebuffer data and outputs 24bit on the LCD pins. You CAN
wire up a RGB555 panel by just skipping the LSB green of a RGB565

22 more lines

On 01/09/2012 12:13 PM, Peter Korsgaard :

quoted

quoted

quoted

quoted

quoted

"Nicolas" = Nicolas Ferre [off-list ref] writes:

 Nicolas> On 10/13/2011 04:52 PM, Peter Korsgaard :
 >> Allow framebuffer to be configured in 16bit mode when panel is wired in
 >> (the default) BGR configuration, and don't claim to support 15bit input
 >> modes, which the LCD controller cannot handle.
 >> 
 >> Signed-off-by: Peter Korsgaard [off-list ref]

 Nicolas> Hi Peter,

 Nicolas> Sorry for not having more responsive concerning the two
 Nicolas> patches that you posted about atmel_lcdfb driver.

No problem.

 Nicolas> I have a question though about this one...

 >> -		} else if (sinfo->lcd_wiring_mode = ATMEL_LCDC_WIRING_RGB555) {
 >> -			var->red.offset = 10;
 >> -			var->blue.offset = 0;
 >> -			var->green.length = 5;

 Nicolas> Maybe I have missed something but I do not know why you are removing
 Nicolas> this part of the configuration? A board at least is using this wiring
 Nicolas> mode...

Because it is arguable wrong as far as I understand the HW.
There's two parts here:

- 1: Format of framebuffer memory
- 2: Wiring of LCD (RGB/BGR order and number of bits)

quoted

From the datasheet, the following framebuffer formats are supported:

1, 2, 4, 8 bits per pixel (palletized), 16, 24 bits per pixel
(non-palletized) for TFT.

So it doesn't really support RGB555 mode. The controller reads up to
32bit of framebuffer data and outputs 24bit on the LCD pins. You CAN

8 more quoted lines

Yes. Thanks to Russell and you for the explanation.

I acknowledge your two patches right now.

So in conclusion, I think we should just have:

- ATMEL_LCDC_WIRING_RGB
- ATMEL_LCDC_WIRING_BGR
- ATMEL_LCDC_WIRING_RGB565
- ATMEL_LCDC_WIRING_BGR565

These simply define framebuffer bit order (RGB/BGR) and preferred
default bit depth (16/24).

Maybe we can think about a patch that removes the RGB555 variable from
the header (include/video/atmel_lcdc.h) and the board file that is using
it (board-neocore926.c).

Best regards,
-- 
Nicolas Ferre

On 2012-01-09 12:13, Peter Korsgaard wrote:

quoted

quoted

quoted

quoted

quoted

"Nicolas" = Nicolas Ferre [off-list ref] writes:

 Nicolas> On 10/13/2011 04:52 PM, Peter Korsgaard :
 >> Allow framebuffer to be configured in 16bit mode when panel is wired in
 >> (the default) BGR configuration, and don't claim to support 15bit input
 >> modes, which the LCD controller cannot handle.
 >> 
 >> Signed-off-by: Peter Korsgaard [off-list ref]

 Nicolas> Hi Peter,

 Nicolas> Sorry for not having more responsive concerning the two
 Nicolas> patches that you posted about atmel_lcdfb driver.

No problem.

 Nicolas> I have a question though about this one...

 >> -		} else if (sinfo->lcd_wiring_mode = ATMEL_LCDC_WIRING_RGB555) {
 >> -			var->red.offset = 10;
 >> -			var->blue.offset = 0;
 >> -			var->green.length = 5;

 Nicolas> Maybe I have missed something but I do not know why you are removing
 Nicolas> this part of the configuration? A board at least is using this wiring
 Nicolas> mode...

Because it is arguable wrong as far as I understand the HW.
There's two parts here:

- 1: Format of framebuffer memory
- 2: Wiring of LCD (RGB/BGR order and number of bits)

From the datasheet, the following framebuffer formats are supported:

1, 2, 4, 8 bits per pixel (palletized), 16, 24 bits per pixel
(non-palletized) for TFT.

So it doesn't really support RGB555 mode. The controller reads up to
32bit of framebuffer data and outputs 24bit on the LCD pins. You CAN

2 more quoted lines

But the AT91SAM9261/AT91SAM9263 do not have a native RGB565 format if it
is configured for 16bit (so it does not read 32bit and output 24bit but
just 16bit) but uses BGR555 with an additional intensity bit in the MSB
like the palette where you also kept the BGR555 format. How can you get
correct colors on these processors if this code above is removed?

Peter Korsgaard wrote:

Because it is arguable wrong as far as I understand the HW.
There's two parts here:

- 1: Format of framebuffer memory
- 2: Wiring of LCD (RGB/BGR order and number of bits)

quoted

From the datasheet, the following framebuffer formats are supported:

1, 2, 4, 8 bits per pixel (palletized), 16, 24 bits per pixel
(non-palletized) for TFT.

So it doesn't really support RGB555 mode. The controller reads up to
32bit of framebuffer data and outputs 24bit on the LCD pins. You CAN
wire up a RGB555 panel by just skipping the LSB green of a RGB565
wiring, but that is independent of the framebufffer format. The
controller/driver doesn't do any RGB/BGR swapping, so the RBG/BGR wiring
settings are just used as a software hint (in FBIOGET_VSCREENINFO) about
the meaning of the individual bits of a pixel in the framebuffer.

Similar you can connect a 12bit 4:4:4 panel by just connecting it to the
MSB LCD pins.

If you're connecting an RGB555 or RGB444 panel, don't you want the
software using the framebuffer to know this so it will dither appropriately?

E.g. gradients look "bandy" if drawn in RGB565 then green's LSB is dropped.

-- Jamie

On Tue, Jan 10, 2012 at 02:02:18PM +0000, Jamie Lokier wrote:

If you're connecting an RGB555 or RGB444 panel, don't you want the
software using the framebuffer to know this so it will dither appropriately?

E.g. gradients look "bandy" if drawn in RGB565 then green's LSB is dropped.

This is why you use the bitfield stuff to tell userspace what the
actual properties of the framebuffer are.

(However, there are some userspace programs which ignore that
information and think they know better than the kernel about how a
frame buffer is organised - which is a constant pain in the butt when
you have a display which isn't BGR.  But that's really a userspace
bug in those silly programs.)