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

PATCH New Peter Korsgaard 6 participants 17 messages Oct 13, 2011 – Jan 30, 2012

Subsystems (2)

Framebuffer layer Arm/Microchip (AT91) SoC support

lore

Share & tools

lore

Peter Korsgaard

author

2011-10-13 14:52 UTC #0 lore raw

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

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 <[email protected]>

---
 drivers/video/atmel_lcdfb.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

drivers/video/atmel_lcdfb.c +3 -9

index 7ca3eaf..143f6d9 100644
@@ -418,24 +418,18 @@ static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var,
 		var->red.length = var->green.length = var->blue.length
 			= var->bits_per_pixel;
 		break;
-	case 15:
 	case 16:
 		if (sinfo->lcd_wiring_mode == ATMEL_LCDC_WIRING_RGB) {
 			/* RGB:565 mode */
 			var->red.offset = 11;
 			var->blue.offset = 0;
-			var->green.length = 6;
-		} else if (sinfo->lcd_wiring_mode == ATMEL_LCDC_WIRING_RGB555) {
-			var->red.offset = 10;
-			var->blue.offset = 0;
-			var->green.length = 5;
 		} else {
-			/* BGR:555 mode */
+			/* BGR:565 mode */
 			var->red.offset = 0;
-			var->blue.offset = 10;
-			var->green.length = 5;
+			var->blue.offset = 11;
 		}
 		var->green.offset = 5;
+		var->green.length = 6;
 		var->red.length = var->blue.length = 5;
 		break;
 	case 32:

Signature · 1 line

1.7.6.3

Peter Korsgaard

author

2012-01-05 11:26 UTC #1 lore raw

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

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Peter" == Peter Korsgaard <[email protected]> writes:

Peter> Allow framebuffer to be configured in 16bit mode when panel is Peter> wired in (the default) BGR configuration, and don't claim to Peter> support 15bit input modes, which the LCD controller cannot Peter> handle.

Ping? Nicolas, you added it to your at91-lcd branch - But it doesn't seem to have gone any further.

Peter> Signed-off-by: Peter Korsgaard <[email protected]> Peter> ---

 Peter>  drivers/video/atmel_lcdfb.c |   12 +++---------

Peter> 1 files changed, 3 insertions(+), 9 deletions(-)

Peter> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c Peter> index 7ca3eaf..143f6d9 100644
Peter> --- a/drivers/video/atmel_lcdfb.c
Peter> +++ b/drivers/video/atmel_lcdfb.c
Peter> @@ -418,24 +418,18 @@ static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var, var-> red.length = var->green.length = var->blue.length
Peter> = var->bits_per_pixel;
Peter> break;
Peter> - case 15:

 Peter>  	case 16:
 Peter>  		if (sinfo->lcd_wiring_mode == ATMEL_LCDC_WIRING_RGB) {
 Peter>  			/* RGB:565 mode */

var-> red.offset = 11;
var-> blue.offset = 0;
Peter> - var->green.length = 6;
Peter> - } else if (sinfo->lcd_wiring_mode == ATMEL_LCDC_WIRING_RGB555) {
Peter> - var->red.offset = 10;
Peter> - var->blue.offset = 0;
Peter> - var->green.length = 5;
Peter> } else {
Peter> - /* BGR:555 mode */
Peter> + /* BGR:565 mode */
var-> red.offset = 0;
Peter> - var->blue.offset = 10;
Peter> - var->green.length = 5;
Peter> + var->blue.offset = 11;
Peter> }
var-> green.offset = 5;
Peter> + var->green.length = 6;
var-> red.length = var->blue.length = 5;
Peter> break;
Peter> case 32:
Peter> --
Peter> 1.7.6.3

Signature · 1 line

Bye, Peter Korsgaard

Florian Tobias Schandinat

2012-01-09 02:49 UTC #2 lore raw

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

Hi Peter,

Quoted 9 lines · Peter Korsgaard

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Peter" == Peter Korsgaard <[email protected]> writes:
Peter> Allow framebuffer to be configured in 16bit mode when panel is Peter> wired in (the default) BGR configuration, and don't claim to Peter> support 15bit input modes, which the LCD controller cannot Peter> handle.
Ping? Nicolas, you added it to your at91-lcd branch - But it doesn't seem to have gone any further.

I could also take care of this patch but I'd prefer if it gets an Acked-by of someone at Atmel (or someone else who knows the hardware) as I cannot really tell whether it is correct.

Best regards,

Florian Tobias Schandinat

Quoted 41 lines

Peter> Signed-off-by: Peter Korsgaard <[email protected]> Peter> ---
 Peter>  drivers/video/atmel_lcdfb.c |   12 +++---------
Peter> 1 files changed, 3 insertions(+), 9 deletions(-)
Peter> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c Peter> index 7ca3eaf..143f6d9 100644
Peter> --- a/drivers/video/atmel_lcdfb.c
Peter> +++ b/drivers/video/atmel_lcdfb.c
Peter> @@ -418,24 +418,18 @@ static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var, var-> red.length = var->green.length = var->blue.length
Peter> = var->bits_per_pixel;
Peter> break;
Peter> - case 15:
 Peter>  	case 16:
 Peter>  		if (sinfo->lcd_wiring_mode == ATMEL_LCDC_WIRING_RGB) {
 Peter>  			/* RGB:565 mode */
var-> red.offset = 11;
var-> blue.offset = 0;
Peter> - var->green.length = 6;
Peter> - } else if (sinfo->lcd_wiring_mode == ATMEL_LCDC_WIRING_RGB555) {
Peter> - var->red.offset = 10;
Peter> - var->blue.offset = 0;
Peter> - var->green.length = 5;
Peter> } else {
Peter> - /* BGR:555 mode */
Peter> + /* BGR:565 mode */
var-> red.offset = 0;
Peter> - var->blue.offset = 10;
Peter> - var->green.length = 5;
Peter> + var->blue.offset = 11;
Peter> }
var-> green.offset = 5;
Peter> + var->green.length = 6;
var-> red.length = var->blue.length = 5;
Peter> break;
Peter> case 32:
Peter> --
Peter> 1.7.6.3

Peter Korsgaard

author

2012-01-09 06:15 UTC #3 lore raw

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

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Florian" == Florian Tobias Schandinat <[email protected]> writes:

Hi,

Quoted 3 lines

Quoted 3 lines
Ping? Nicolas, you added it to your at91-lcd branch - But it doesn't seem to have gone any further.

Florian> I could also take care of this patch but I'd prefer if it gets Florian> an Acked-by of someone at Atmel (or someone else who knows the Florian> hardware) as I cannot really tell whether it is correct.

I understand. Nicolas, presumably you're ok with it as you added it to your tree?

Signature · 1 line

Bye, Peter Korsgaard

Nicolas Ferre

2012-01-09 13:36 UTC #4 lore raw

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

In reply to Florian Tobias Schandinat

On 01/09/2012 03:49 AM, Florian Tobias Schandinat :

Quoted 10 lines

Hi Peter,
Quoted 7 lines · Peter Korsgaard
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Peter" == Peter Korsgaard <[email protected]> writes:
Peter> Allow framebuffer to be configured in 16bit mode when panel is Peter> wired in (the default) BGR configuration, and don't claim to Peter> support 15bit input modes, which the LCD controller cannot Peter> handle.

Quoted 6 lines

Quoted 2 lines
Ping? Nicolas, you added it to your at91-lcd branch - But it doesn't seem to have gone any further.
I could also take care of this patch but I'd prefer if it gets an Acked-by of someone at Atmel (or someone else who knows the hardware) as I cannot really tell whether it is correct.

Here is my:

Acked-by Nicolas Ferre <[email protected]>

Thank you Florian for taking care of it.

Best regards,

Quoted 42 lines

Quoted 40 lines
Peter> Signed-off-by: Peter Korsgaard <[email protected]> Peter> ---
 Peter>  drivers/video/atmel_lcdfb.c |   12 +++---------
Peter> 1 files changed, 3 insertions(+), 9 deletions(-)
Peter> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c Peter> index 7ca3eaf..143f6d9 100644
Peter> --- a/drivers/video/atmel_lcdfb.c
Peter> +++ b/drivers/video/atmel_lcdfb.c
Peter> @@ -418,24 +418,18 @@ static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var, var-> red.length = var->green.length = var->blue.length
Peter> = var->bits_per_pixel;
Peter> break;
Peter> - case 15:
 Peter>  	case 16:
 Peter>  		if (sinfo->lcd_wiring_mode == ATMEL_LCDC_WIRING_RGB) {
 Peter>  			/* RGB:565 mode */
var-> red.offset = 11;
var-> blue.offset = 0;
Peter> - var->green.length = 6;
Peter> - } else if (sinfo->lcd_wiring_mode == ATMEL_LCDC_WIRING_RGB555) {
Peter> - var->red.offset = 10;
Peter> - var->blue.offset = 0;
Peter> - var->green.length = 5;
Peter> } else {
Peter> - /* BGR:555 mode */
Peter> + /* BGR:565 mode */
var-> red.offset = 0;
Peter> - var->blue.offset = 10;
Peter> - var->green.length = 5;
Peter> + var->blue.offset = 11;
Peter> }
var-> green.offset = 5;
Peter> + var->green.length = 6;
var-> red.length = var->blue.length = 5;
Peter> break;
Peter> case 32:
Peter> --
Peter> 1.7.6.3

Signature · 1 line

Nicolas Ferre

Nicolas Ferre

2012-01-09 10:34 UTC #5 lore raw

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

In reply to Peter Korsgaard

On 10/13/2011 04:52 PM, Peter Korsgaard :

Quoted 5 lines

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 <[email protected]>

Hi Peter,

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

I have a question though about this one...

Quoted 13 lines

---
 drivers/video/atmel_lcdfb.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

drivers/video/atmel_lcdfb.c +0 -1

index 7ca3eaf..143f6d9 100644
@@ -418,24 +418,18 @@ static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var,
 		var->red.length = var->green.length = var->blue.length
 			= var->bits_per_pixel;
 		break;
-	case 15:

drivers/video/atmel_lcdfb.c static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var, · line 431

Quoted 10 lines

drivers/video/atmel_lcdfb.c +0 -5

 	case 16:
 		if (sinfo->lcd_wiring_mode == ATMEL_LCDC_WIRING_RGB) {
 			/* RGB:565 mode */
 			var->red.offset = 11;
 			var->blue.offset = 0;
-			var->green.length = 6;
-		} else if (sinfo->lcd_wiring_mode == ATMEL_LCDC_WIRING_RGB555) {
-			var->red.offset = 10;
-			var->blue.offset = 0;
-			var->green.length = 5;

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

drivers/video/atmel_lcdfb.c static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var, · line 434

Quoted 12 lines

drivers/video/atmel_lcdfb.c +3 -3

 		} else {
-			/* BGR:555 mode */
+			/* BGR:565 mode */
 			var->red.offset = 0;
-			var->blue.offset = 10;
-			var->green.length = 5;
+			var->blue.offset = 11;
 		}
 		var->green.offset = 5;
+		var->green.length = 6;
 		var->red.length = var->blue.length = 5;
 		break;

Maybe we can separate both 15 and 16 cases but I do not know if we can remove completely the 15 RGB555 mode...

Best regards,

Signature · 1 line

Nicolas Ferre

Peter Korsgaard

author

2012-01-09 11:13 UTC #6 lore raw

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

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Nicolas" == Nicolas Ferre <[email protected]> writes:

Nicolas> On 10/13/2011 04:52 PM, Peter Korsgaard :

Quoted 5 lines

Quoted 5 lines
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 <[email protected]>

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...

Quoted 4 lines

drivers/video/atmel_lcdfb.c static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var, · line 431
Quoted 4 lines
drivers/video/atmel_lcdfb.c +0 -4
-		} 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 1 line

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.

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).

I hope this makes it more clear.

Signature · 1 line

Bye, Peter Korsgaard

Nicolas Ferre

2012-01-09 13:32 UTC #7 lore raw

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

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

Quoted 46 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Nicolas" == Nicolas Ferre <[email protected]> writes:
Nicolas> On 10/13/2011 04:52 PM, Peter Korsgaard :
Quoted 5 lines
Quoted 5 lines
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 <[email protected]>
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...
Quoted 4 lines
drivers/video/atmel_lcdfb.c static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var, · line 431
Quoted 4 lines
drivers/video/atmel_lcdfb.c +0 -4
-		} 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 1 line
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.

Quoted 2 lines

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

Yes. Thanks to Russell and you for the explanation.

I acknowledge your two patches right now.

Quoted 9 lines

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,

Signature · 1 line

Nicolas Ferre

Christian Glindkamp

2012-01-10 13:01 UTC #8 lore raw

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

In reply to Peter Korsgaard

Quoted 35 lines · Peter Korsgaard

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Nicolas" == Nicolas Ferre <[email protected]> writes:
Nicolas> On 10/13/2011 04:52 PM, Peter Korsgaard :
Quoted 5 lines
Quoted 5 lines
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 <[email protected]>
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...
Quoted 4 lines
drivers/video/atmel_lcdfb.c static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var, · line 431
Quoted 4 lines
drivers/video/atmel_lcdfb.c +0 -4
-		} 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:

Quoted 7 lines

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.

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

author

2012-01-10 13:17 UTC #9 lore raw

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

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Christian" == Christian Glindkamp <[email protected]> writes:

Hi,

Quoted 4 lines

Quoted 4 lines
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.

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

Ahh, I wasn't aware of that (I'm using 9g45). I guess we need to do something similar to what I did for the palette handling:

https://github.com/at91linux/linux-at91/commit/7a256fc44c1892ad3166363ba309d9996a49e7b8

E.G. keep the RBG555/BGR555 format for the old devices, and my proposed change for the new ones. I'll cook something up.

Signature · 1 line

Bye, Peter Korsgaard

Peter Korsgaard

author

2012-01-10 16:42 UTC #10 lore raw

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

In reply to Christian Glindkamp

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Christian" == Christian Glindkamp <[email protected]> writes:

Hi,

Quoted 4 lines

Quoted 4 lines
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.

I now had a look at the 9261/9263 datasheets, and don't find any explicit mention of the bit layout of the 16bit/24bit modes. Just to be completely sure:

- 16bit mode is IBGR-1555 (independently of little/big/wince mode?)
- 24bit is BGR-888 (packed) or XBGR-8888 (unpacked)

Right?

Signature · 1 line

Bye, Peter Korsgaard

Jamie Lokier

2012-01-10 14:02 UTC #11 lore raw

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

In reply to Peter Korsgaard

Quoted 19 lines · Peter Korsgaard

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 1 line
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.

Quoted 2 lines

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

Peter Korsgaard

author

2012-01-10 14:19 UTC #12 lore raw

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

Quoted 4 lines

Quoted 4 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Jamie" == Jamie Lokier <[email protected]> writes:
Similar you can connect a 12bit 4:4:4 panel by just connecting it to the MSB LCD pins.

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

Potentially. This is similar to the issues with cheap 18bit laptop LCDs. We somewhat do this (E.G. default to 565 mode if a 16bit panel is used), but as the hardware doesn't support anything bwteen 8bit paletted and RGB565 modes, there's not much else we can do (fbdev doesn't provide any info about the panel specs).

Signature · 1 line

Bye, Peter Korsgaard

Russell King - ARM Linux

2012-01-10 21:06 UTC #13 lore raw

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

In reply to Jamie Lokier

Quoted 4 lines · Jamie Lokier

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.)

Russell King - ARM Linux

2012-01-09 12:13 UTC #14 lore raw

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

In reply to Nicolas Ferre

Quoted 35 lines · Nicolas Ferre

On 10/13/2011 04:52 PM, Peter Korsgaard :
Quoted 5 lines
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 <[email protected]>
Hi Peter,
Sorry for not having more responsive concerning the two patches that you posted about atmel_lcdfb driver.
I have a question though about this one...
Quoted 19 lines
---
 drivers/video/atmel_lcdfb.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)
drivers/video/atmel_lcdfb.c +0 -2
index 7ca3eaf..143f6d9 100644
@@ -418,24 +418,18 @@ static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var,
 		var->red.length = var->green.length = var->blue.length
 			= var->bits_per_pixel;
 		break;
-	case 15:
 	case 16:
 		if (sinfo->lcd_wiring_mode == ATMEL_LCDC_WIRING_RGB) {
 			/* RGB:565 mode */
 			var->red.offset = 11;
 			var->blue.offset = 0;
-			var->green.length = 6;

Quoted 8 lines

drivers/video/atmel_lcdfb.c static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var, · line 431
Quoted 4 lines
drivers/video/atmel_lcdfb.c +0 -4
-		} else if (sinfo->lcd_wiring_mode == ATMEL_LCDC_WIRING_RGB555) {
-			var->red.offset = 10;
-			var->blue.offset = 0;
-			var->green.length = 5;
Maybe I have missed something but I do not know why you are removing this part of the configuration? A board at least is using this wiring mode...

Oh, this old gem.

var->bits_per_pixel represents the number of bits in the framebuffer per pixel, and '15' for this means that your frame buffer is organized as:

3 3 2 2 2 2 2 2 2 2 2 2 1 1 1 1 1 1 1 1 1 1
1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
----+-----------------------------+-----------------------------+
A | B | C |
----+-----------------------------+-----------------------------+

whereas what you actually have is:

3 3 2 2 2 2 2 2 2 2 2 2 1 1 1 1 1 1 1 1 1 1
1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0

+-------------------------------+-------------------------------+
|            B                  |              C                |
+-------------------------------+-------------------------------+

and one bit in each pixel remains unused.

This is because bytes_per_line = xres * 8 / bits_per_pixel.

You describe the RGB555 vs RGB565 via the bitfield values in var->red, var->green and var->blue. So, RGB(or BGR)565 has var->green.length with 6, and RGB(or BGR)555 has this as 5. In both cases, bits_per_pixel is 16 as that's the width of a pixel in the frame buffer.

If the bitfield can't be interpreted by the driver as identifying one of these combinations, then you're supposed to chose one (and in the case where it's fixed by hardware, you set the bitfields according to what the hardware supports.)

Peter Korsgaard

author

2012-01-09 12:20 UTC #15 lore raw

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

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Russell" == Russell King <- ARM Linux <[email protected]>> writes:

Hi,

Russell> You describe the RGB555 vs RGB565 via the bitfield values in Russell> var->red, var-> green and var->blue. So, RGB(or BGR)565 has Russell> var->green.length with 6, and RGB(or BGR)555 has this as 5. Russell> In both cases, bits_per_pixel is 16 as that's the width of a Russell> pixel in the frame buffer.

Russell> If the bitfield can't be interpreted by the driver as Russell> identifying one of these combinations, then you're supposed to Russell> chose one (and in the case where it's fixed by hardware, you Russell> set the bitfields according to what the hardware supports.)

Indeed, and as the hardware can only interprete 1/2/4/8 paletted and 16/24 RGB, we shouldn't claim to support 15bit.

Signature · 1 line

Bye, Peter Korsgaard

Florian Tobias Schandinat

Applied

2012-01-30 05:12 UTC #16 lore raw

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

In reply to Peter Korsgaard

Quoted 5 lines · 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 <[email protected]>

Applied.

Thanks,

Florian Tobias Schandinat

Quoted 36 lines

---
 drivers/video/atmel_lcdfb.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

drivers/video/atmel_lcdfb.c +3 -9

index 7ca3eaf..143f6d9 100644
@@ -418,24 +418,18 @@ static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var,
 		var->red.length = var->green.length = var->blue.length
 			= var->bits_per_pixel;
 		break;
-	case 15:
 	case 16:
 		if (sinfo->lcd_wiring_mode == ATMEL_LCDC_WIRING_RGB) {
 			/* RGB:565 mode */
 			var->red.offset = 11;
 			var->blue.offset = 0;
-			var->green.length = 6;
-		} else if (sinfo->lcd_wiring_mode == ATMEL_LCDC_WIRING_RGB555) {
-			var->red.offset = 10;
-			var->blue.offset = 0;
-			var->green.length = 5;
 		} else {
-			/* BGR:555 mode */
+			/* BGR:565 mode */
 			var->red.offset = 0;
-			var->blue.offset = 10;
-			var->green.length = 5;
+			var->blue.offset = 11;
 		}
 		var->green.offset = 5;
+		var->green.length = 6;
 		var->red.length = var->blue.length = 5;
 		break;
 	case 32: