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

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

[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 <jacmet@sunsite.dk>
---
 drivers/video/atmel_lcdfb.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 7ca3eaf..143f6d9 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -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:
-- 
1.7.6.3

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

[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 <jacmet@sunsite.dk>
---
 drivers/video/atmel_lcdfb.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 7ca3eaf..143f6d9 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -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:
-- 
1.7.6.3

Re: [PATCH] atmel_lcdfb: support 16bit BGR:565 mode, remove unsupported 15bit modes

[Peter Korsgaard]

>>>>> "Peter" = Peter Korsgaard <jacmet@sunsite.dk> 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 <jacmet@sunsite.dk>
 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

-- 
Bye, Peter Korsgaard

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

[Peter Korsgaard]

>>>>> "Peter" == Peter Korsgaard <jacmet@sunsite.dk> 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 <jacmet@sunsite.dk>
 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

-- 
Bye, Peter Korsgaard

Re: [PATCH] atmel_lcdfb: support 16bit BGR:565 mode, remove unsupported

[Florian Tobias Schandinat]

Hi Peter,

On 01/05/2012 11:26 AM, Peter Korsgaard wrote:
>>>>>> "Peter" = Peter Korsgaard <jacmet@sunsite.dk> 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

> 
>  Peter> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
>  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
> 

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

[Florian Tobias Schandinat]

Hi Peter,

On 01/05/2012 11:26 AM, Peter Korsgaard wrote:
>>>>>> "Peter" == Peter Korsgaard <jacmet@sunsite.dk> 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

> 
>  Peter> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
>  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
> 

Re: [PATCH] atmel_lcdfb: support 16bit BGR:565 mode, remove unsupported 15bit modes

[Peter Korsgaard]

>>>>> "Florian" = Florian Tobias Schandinat <FlorianSchandinat@gmx.de> writes:

Hi,

 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.

 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?

-- 
Bye, Peter Korsgaard

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

[Peter Korsgaard]

>>>>> "Florian" == Florian Tobias Schandinat <FlorianSchandinat@gmx.de> writes:

Hi,

 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.

 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?

-- 
Bye, Peter Korsgaard

Re: [PATCH] atmel_lcdfb: support 16bit BGR:565 mode, remove unsupported

[Nicolas Ferre]

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 <jacmet@sunsite.dk>

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


> ---
>  drivers/video/atmel_lcdfb.c |   12 +++---------
>  1 files changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> index 7ca3eaf..143f6d9 100644
> --- a/drivers/video/atmel_lcdfb.c
> +++ b/drivers/video/atmel_lcdfb.c
> @@ -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;

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


>  		} 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,
-- 
Nicolas Ferre

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

[Nicolas Ferre]

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 <jacmet@sunsite.dk>

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


> ---
>  drivers/video/atmel_lcdfb.c |   12 +++---------
>  1 files changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> index 7ca3eaf..143f6d9 100644
> --- a/drivers/video/atmel_lcdfb.c
> +++ b/drivers/video/atmel_lcdfb.c
> @@ -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;

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


>  		} 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,
-- 
Nicolas Ferre

Re: [PATCH] atmel_lcdfb: support 16bit BGR:565 mode, remove unsupported 15bit modes

[Peter Korsgaard]

>>>>> "Nicolas" = Nicolas Ferre <nicolas.ferre@atmel.com> 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 <jacmet@sunsite.dk>

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

-- 
Bye, Peter Korsgaard

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

[Peter Korsgaard]

>>>>> "Nicolas" == Nicolas Ferre <nicolas.ferre@atmel.com> 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 <jacmet@sunsite.dk>

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

-- 
Bye, Peter Korsgaard

Re: [PATCH] atmel_lcdfb: support 16bit BGR:565 mode, remove

[Russell King - ARM Linux]

On Mon, Jan 09, 2012 at 11:34:25AM +0100, Nicolas Ferre wrote:
> 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 <jacmet@sunsite.dk>
> 
> 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...
> 
> 
> > ---
> >  drivers/video/atmel_lcdfb.c |   12 +++---------
> >  1 files changed, 3 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> > index 7ca3eaf..143f6d9 100644
> > --- a/drivers/video/atmel_lcdfb.c
> > +++ b/drivers/video/atmel_lcdfb.c
> > @@ -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;
> 
> 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.)

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

[Russell King - ARM Linux]

On Mon, Jan 09, 2012 at 11:34:25AM +0100, Nicolas Ferre wrote:
> 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 <jacmet@sunsite.dk>
> 
> 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...
> 
> 
> > ---
> >  drivers/video/atmel_lcdfb.c |   12 +++---------
> >  1 files changed, 3 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> > index 7ca3eaf..143f6d9 100644
> > --- a/drivers/video/atmel_lcdfb.c
> > +++ b/drivers/video/atmel_lcdfb.c
> > @@ -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;
> 
> 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.)

Re: [PATCH] atmel_lcdfb: support 16bit BGR:565 mode, remove unsupported 15bit modes

[Peter Korsgaard]

>>>>> "Russell" = Russell King <- ARM Linux <linux@arm.linux.org.uk>> 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.

-- 
Bye, Peter Korsgaard

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

[Peter Korsgaard]

>>>>> "Russell" == Russell King <- ARM Linux <linux@arm.linux.org.uk>> 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.

-- 
Bye, Peter Korsgaard

Re: [PATCH] atmel_lcdfb: support 16bit BGR:565 mode, remove unsupported

[Nicolas Ferre]

On 01/09/2012 12:13 PM, Peter Korsgaard :
>>>>>> "Nicolas" = Nicolas Ferre <nicolas.ferre@atmel.com> 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 <jacmet@sunsite.dk>
> 
>  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
> 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.

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

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

[Nicolas Ferre]

On 01/09/2012 12:13 PM, Peter Korsgaard :
>>>>>> "Nicolas" == Nicolas Ferre <nicolas.ferre@atmel.com> 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 <jacmet@sunsite.dk>
> 
>  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
> 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.

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

Re: [PATCH] atmel_lcdfb: support 16bit BGR:565 mode, remove unsupported

[Nicolas Ferre]

On 01/09/2012 03:49 AM, Florian Tobias Schandinat :
> Hi Peter,
> 
> On 01/05/2012 11:26 AM, Peter Korsgaard wrote:
>>>>>>> "Peter" = Peter Korsgaard <jacmet@sunsite.dk> 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.

Here is my:

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Thank you Florian for taking care of it.

Best regards,

>>  Peter> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
>>  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
>>
> 
> 


-- 
Nicolas Ferre

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

[Nicolas Ferre]

On 01/09/2012 03:49 AM, Florian Tobias Schandinat :
> Hi Peter,
> 
> On 01/05/2012 11:26 AM, Peter Korsgaard wrote:
>>>>>>> "Peter" == Peter Korsgaard <jacmet@sunsite.dk> 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.

Here is my:

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Thank you Florian for taking care of it.

Best regards,

>>  Peter> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
>>  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
>>
> 
> 


-- 
Nicolas Ferre

Re: [PATCH] atmel_lcdfb: support 16bit BGR:565 mode, remove

[Christian Glindkamp]

On 2012-01-09 12:13, Peter Korsgaard wrote:
> >>>>> "Nicolas" = Nicolas Ferre <nicolas.ferre@atmel.com> 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 <jacmet@sunsite.dk>
> 
>  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
> 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?

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

[Christian Glindkamp]

On 2012-01-09 12:13, Peter Korsgaard wrote:
> >>>>> "Nicolas" == Nicolas Ferre <nicolas.ferre@atmel.com> 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 <jacmet@sunsite.dk>
> 
>  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
> 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?

Re: [PATCH] atmel_lcdfb: support 16bit BGR:565 mode, remove unsupported 15bit modes

[Peter Korsgaard]

>>>>> "Christian" = Christian Glindkamp <christian.glindkamp@taskit.de> writes:

Hi,

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

-- 
Bye, Peter Korsgaard

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

[Peter Korsgaard]

>>>>> "Christian" == Christian Glindkamp <christian.glindkamp@taskit.de> writes:

Hi,

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

-- 
Bye, Peter Korsgaard

Re: [PATCH] atmel_lcdfb: support 16bit BGR:565 mode, remove

[Jamie Lokier]

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

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

[Jamie Lokier]

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

Re: [PATCH] atmel_lcdfb: support 16bit BGR:565 mode, remove unsupported 15bit modes

[Peter Korsgaard]

>>>>> "Jamie" = Jamie Lokier <jamie@shareable.org> 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).

-- 
Bye, Peter Korsgaard

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

[Peter Korsgaard]

>>>>> "Jamie" == Jamie Lokier <jamie@shareable.org> 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).

-- 
Bye, Peter Korsgaard

Re: [PATCH] atmel_lcdfb: support 16bit BGR:565 mode, remove unsupported 15bit modes

[Peter Korsgaard]

>>>>> "Christian" = Christian Glindkamp <christian.glindkamp@taskit.de> writes:

Hi,

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

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?

-- 
Bye, Peter Korsgaard

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

[Peter Korsgaard]

>>>>> "Christian" == Christian Glindkamp <christian.glindkamp@taskit.de> writes:

Hi,

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

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?

-- 
Bye, Peter Korsgaard

Re: [PATCH] atmel_lcdfb: support 16bit BGR:565 mode, remove

[Russell King - ARM Linux]

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

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

[Russell King - ARM Linux]

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

Re: [PATCH] atmel_lcdfb: support 16bit BGR:565 mode, remove unsupported 15bit modes

[Florian Tobias Schandinat]

On 10/13/2011 02:52 PM, Peter Korsgaard wrote:
> 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 <jacmet@sunsite.dk>

Applied.


Thanks,

Florian Tobias Schandinat

> ---
>  drivers/video/atmel_lcdfb.c |   12 +++---------
>  1 files changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> index 7ca3eaf..143f6d9 100644
> --- a/drivers/video/atmel_lcdfb.c
> +++ b/drivers/video/atmel_lcdfb.c
> @@ -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:

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

[Florian Tobias Schandinat]

On 10/13/2011 02:52 PM, Peter Korsgaard wrote:
> 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 <jacmet@sunsite.dk>

Applied.


Thanks,

Florian Tobias Schandinat

> ---
>  drivers/video/atmel_lcdfb.c |   12 +++---------
>  1 files changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> index 7ca3eaf..143f6d9 100644
> --- a/drivers/video/atmel_lcdfb.c
> +++ b/drivers/video/atmel_lcdfb.c
> @@ -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: