diff mbox

video: mxsfb: Fix colors display on lower color depth

Message ID 1366277032-671-1-git-send-email-maxime.ripard@free-electrons.com (mailing list archive)
State New, archived
Headers show

Commit Message

Maxime Ripard April 18, 2013, 9:23 a.m. UTC
The current code always registers as a 32 bits display, and uses the
hardware to drop the MSB of each color to abjust to the interface width
used by the panel.

This results on 18 bits (and probably 16 bits display as well) in colors
being displayed poorly, because the MSB are obviously the most important
bits for each color definition.

The default controller behaviour when using an interface width smaller
than the color depth is to drop the LSBs of each color, which makes more
sense because you lose the least important part of the color definition.

So, to fix the colors display, just get back to the default controller
behaviour.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/video/mxsfb.c |    5 -----
 1 file changed, 5 deletions(-)

Comments

Marek Vasut April 18, 2013, 2 p.m. UTC | #1
Dear Maxime Ripard,

> The current code always registers as a 32 bits display, and uses the
> hardware to drop the MSB of each color to abjust to the interface width
> used by the panel.
> 
> This results on 18 bits (and probably 16 bits display as well) in colors
> being displayed poorly, because the MSB are obviously the most important
> bits for each color definition.
> 
> The default controller behaviour when using an interface width smaller
> than the color depth is to drop the LSBs of each color, which makes more
> sense because you lose the least important part of the color definition.
> 
> So, to fix the colors display, just get back to the default controller
> behaviour.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Isn't this what M28EVK is doing already? This was last tested on 3.9-rc3 I 
think:

bits-per-pixel = <16>;
bus-width = <18>;

But it's also true we have all 24 data lines routed to the LCD.

Best regards,
Marek Vasut
Shawn Guo April 18, 2013, 2:48 p.m. UTC | #2
Copy Sascha.

Shawn

On Thu, Apr 18, 2013 at 11:23:52AM +0200, Maxime Ripard wrote:
> The current code always registers as a 32 bits display, and uses the
> hardware to drop the MSB of each color to abjust to the interface width
> used by the panel.
> 
> This results on 18 bits (and probably 16 bits display as well) in colors
> being displayed poorly, because the MSB are obviously the most important
> bits for each color definition.
> 
> The default controller behaviour when using an interface width smaller
> than the color depth is to drop the LSBs of each color, which makes more
> sense because you lose the least important part of the color definition.
> 
> So, to fix the colors display, just get back to the default controller
> behaviour.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/video/mxsfb.c |    5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
> index 755556c..2cfaf8b 100644
> --- a/drivers/video/mxsfb.c
> +++ b/drivers/video/mxsfb.c
> @@ -424,11 +424,6 @@ static int mxsfb_set_par(struct fb_info *fb_info)
>  			return -EINVAL;
>  		case STMLCDIF_16BIT:
>  		case STMLCDIF_18BIT:
> -			/* 24 bit to 18 bit mapping */
> -			ctrl |= CTRL_DF24; /* ignore the upper 2 bits in
> -					    *  each colour component
> -					    */
> -			break;
>  		case STMLCDIF_24BIT:
>  			/* real 24 bit */
>  			break;
> -- 
> 1.7.10.4
>
Maxime Ripard April 22, 2013, 8:07 a.m. UTC | #3
Hi Shawn,

Le 18/04/2013 16:48, Shawn Guo a écrit :
> Copy Sascha.

Ah, yes, sorry, I somehow got confused and thought you were the one that
wrote the driver.

I'll resend the patch.

Maxime
diff mbox

Patch

diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
index 755556c..2cfaf8b 100644
--- a/drivers/video/mxsfb.c
+++ b/drivers/video/mxsfb.c
@@ -424,11 +424,6 @@  static int mxsfb_set_par(struct fb_info *fb_info)
 			return -EINVAL;
 		case STMLCDIF_16BIT:
 		case STMLCDIF_18BIT:
-			/* 24 bit to 18 bit mapping */
-			ctrl |= CTRL_DF24; /* ignore the upper 2 bits in
-					    *  each colour component
-					    */
-			break;
 		case STMLCDIF_24BIT:
 			/* real 24 bit */
 			break;