Re: mxsfb: DATA_FORMAT_24_BIT flag outputs invalid colours

2013-06-07 Thread maxime.rip...@free-electrons.com
On Fri, Jun 07, 2013 at 09:28:32AM +0200, Hector Palacios wrote:
> I wasn't sure that everybody involved agreed with the patch.
> @Juergen, would the patch break your platform?
> Additionally, the guys from Crystalfontz didn't comment on it, but
> their platform is also using a 18bit data bus width and 32bpp.

We (Free Electrons) are doing the bring up for the Crystalfontz boards,
so it's fine :)

Thanks!
Maxime
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: mxsfb: DATA_FORMAT_24_BIT flag outputs invalid colours

2013-06-07 Thread Juergen Beisert
Hi Hector,

On 07.06.2013 09:28, Hector Palacios wrote:
> On 06/07/2013 09:21 AM, maxime.rip...@free-electrons.com wrote:
>> Hi Hector,
>>
>> On Fri, May 24, 2013 at 03:33:19PM +0200, Juergen Beisert wrote:
>>> Someone told me, Qt5 cannot handle this special RGB666 mode (even not the
>>> def_rgb666_shift memory layout mentioned above). My test are based on Qt4.
>>> Qt5 needs a regular RGB888 mode, which should silently be converted
>>> internally to RGB666 in the hardware.
>>>
>>> So, your patch to always use the RGB888 memory layout seems to be the right
>>> way to go.
>>
>> Do you plan on submitting this patch? (Or did you already submit it and
>> I overlooked it?)
> 
> I wasn't sure that everybody involved agreed with the patch.
> @Juergen, would the patch break your platform?

No, we need to switch to this data format here, too.

> Additionally, the guys from Crystalfontz didn't comment on it, but their 
> platform is 
> also using a 18bit data bus width and 32bpp.
> 
> If no-one is against I'll be glad to submit it.

You can have my Acked-by: Juergen Beisert  for this patch.

Regards,
Juergen

-- 
Pengutronix e.K.  | Juergen Beisert |
Linux Solutions for Science and Industry  | Phone: +49-5121-206917-5128 |
Peiner Str. 6-8, 31137 Hildesheim, Germany| Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: mxsfb: DATA_FORMAT_24_BIT flag outputs invalid colours

2013-06-07 Thread Hector Palacios

Hi Maxime,

On 06/07/2013 09:21 AM, maxime.rip...@free-electrons.com wrote:

Hi Hector,

On Fri, May 24, 2013 at 03:33:19PM +0200, Juergen Beisert wrote:

Someone told me, Qt5 cannot handle this special RGB666 mode (even not the
def_rgb666_shift memory layout mentioned above). My test are based on Qt4.
Qt5 needs a regular RGB888 mode, which should silently be converted
internally to RGB666 in the hardware.

So, your patch to always use the RGB888 memory layout seems to be the right
way to go.


Do you plan on submitting this patch? (Or did you already submit it and
I overlooked it?)


I wasn't sure that everybody involved agreed with the patch.
@Juergen, would the patch break your platform?
Additionally, the guys from Crystalfontz didn't comment on it, but their platform is 
also using a 18bit data bus width and 32bpp.


If no-one is against I'll be glad to submit it.

Best regards,
--
Hector Palacios
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: mxsfb: DATA_FORMAT_24_BIT flag outputs invalid colours

2013-06-07 Thread maxime.rip...@free-electrons.com
Hi Hector,

On Fri, May 24, 2013 at 03:33:19PM +0200, Juergen Beisert wrote:
> Someone told me, Qt5 cannot handle this special RGB666 mode (even not the 
> def_rgb666_shift memory layout mentioned above). My test are based on Qt4. 
> Qt5 needs a regular RGB888 mode, which should silently be converted 
> internally to RGB666 in the hardware.
> 
> So, your patch to always use the RGB888 memory layout seems to be the right 
> way to go.

Do you plan on submitting this patch? (Or did you already submit it and
I overlooked it?)

Thanks,
Maxime
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: mxsfb: DATA_FORMAT_24_BIT flag outputs invalid colours

2013-05-24 Thread Juergen Beisert
Hi Hector,

Juergen Beisert wrote:
> Hector Palacios wrote:
> > On 05/24/2013 12:28 PM, Juergen Beisert wrote:
> > > Hector Palacios wrote:
> > >> On 05/23/2013 03:31 PM, Juergen Beisert wrote:
> > >>> maxime.rip...@free-electrons.com wrote:
> >  On Thu, May 23, 2013 at 01:55:28PM +0200, Hector Palacios wrote:
> > > I'm using an i.MX28 based board with lcd connected with 18bits data
> > > bus. My platform uses 32 bits per pixel:
> > >
> > >   mxsfb_pdata.default_bpp = 32;
> > >   mxsfb_pdata.ld_intf_width = STMLCDIF_18BIT;
> > >
> > > With these settings the mxsfb.c driver sets flag DATA_FORMAT_24_BIT
> > > at HW_LCDIF_CTRL register in function mxsfb_set_par():
> > >
> > >   case 32:
> > >   dev_dbg(&host->pdev->dev, "Setting up RGB888/666 
> > > mode\n");
> > >   ctrl |= CTRL_SET_WORD_LENGTH(3);
> > >   switch (host->ld_intf_width) {
> > >   case STMLCDIF_8BIT:
> > >   dev_dbg(&host->pdev->dev,
> > >   "Unsupported LCD bus width 
> > > mapping\n");
> > >   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;
> > >   }
> > >
> > > According to the manual, this flag does:
> > >   0x0: ALL_24_BITS_VALID: Data input to the block is in 24 bpp
> > > format, such that all RGB 888 data is contained in 24 bits.
> > >   0x1: DROP_UPPER_2_BITS_PER_BYTE — Data input to the block is
> > > actually RGB 18 bpp, but there is 1 colour per byte, hence the
> > > upper 2 bits in each byte do not contain any useful data, and
> > > should be dropped.
> > >
> > > The setting of this flag is producing bad colours with true colour
> > > images (i.e. the Linux penguin is displayed ok, but QT applications
> > > or images displayed with fbv are not).
> > > I believe the setting of this flag is not correct (after all, if my
> > > bpp is 32, then all 24bit colours are useful and dropping the upper
> > > 2 bits is a bad idea).
> > > If I don't set it, then true colour images are displayed correctly.
> > > The only problem is that the Linux penguin is displayed much darker
> > > than usual (correct colours, but darker). Perhaps the 224 colour
> > > format of this image justifies it?
> > >
> > > I noticed the cfa10049 platform also uses the same configuration
> > > (18 bits data bus and 32bpp) and was wondering if true colour
> > > images are correctly displayed in this platform with this flag set
> > > (for example with fbv application [1]).
> > 
> >  I had the exact same problem, and suggested the exact same solution
> >  a few weeks back.
> > 
> >  https://patchwork.kernel.org/patch/2470441/
> > 
> >  The conclusion of that discussion what that the userspace
> >  applications were not honouring the bitfield correctly set by the
> >  mxsfb driver, and as such, it was not a bug in the driver.
> > 
> >  While this is correct, I wonder, now that since we had that same
> >  problem in a very short amount of time, if we couldn't set this
> >  behaviour dependant of some (dt? kernel argument?) property so that
> >  one could customise it anyway he want.
> > 
> >  Maxime
> > >>>
> > >>> i.MX2[3|8]LCD1   LCD2   LCD3
> > >>> 24bit  18bit  18bit
> > >>> 
> > >>> LCD_D0 B0 B0 --
> > >>> LCD_D1 B1 B1 --
> > >>> LCD_D2 B2 B2 B0
> > >>> LCD_D3 B3 B3 B1
> > >>> LCD_D4 B4 B4 B2
> > >>> LCD_D5 B5 B5 B3
> > >>> LCD_D6 B6 G0 B4
> > >>> LCD_D7 B7 G1 B5
> > >>>
> > >>> LCD_D8 G0 G2 --
> > >>> LCD_D9 G1 G3 --
> > >>> LCD_D10G2 G4 G0
> > >>> LCD_D11G3 G5 G1
> > >>> LCD_D12G4 R0 G2
> > >>> LCD_D13G5 R1 G3
> > >>> LCD_D14G6 R2 G4
> > >>> LCD_D15G7 R3 G5
> > >>>
> > >>> LCD_D16R0 R4 --
> > >>> LCD_D17R1 R5 --
> > >>> LCD_D18R2R0
> > >>> LCD_D19   

Re: mxsfb: DATA_FORMAT_24_BIT flag outputs invalid colours

2013-05-24 Thread Juergen Beisert
Hector Palacios wrote:
> Hi Juergen,
>
> On 05/24/2013 12:28 PM, Juergen Beisert wrote:
> > Hector Palacios wrote:
> >> Hi Juergen,
> >>
> >> On 05/23/2013 03:31 PM, Juergen Beisert wrote:
> >>> Hi Maxime,
> >>>
> >>> maxime.rip...@free-electrons.com wrote:
>  On Thu, May 23, 2013 at 01:55:28PM +0200, Hector Palacios wrote:
> > I'm using an i.MX28 based board with lcd connected with 18bits data
> > bus. My platform uses 32 bits per pixel:
> >
> > mxsfb_pdata.default_bpp = 32;
> > mxsfb_pdata.ld_intf_width = STMLCDIF_18BIT;
> >
> > With these settings the mxsfb.c driver sets flag DATA_FORMAT_24_BIT
> > at HW_LCDIF_CTRL register in function mxsfb_set_par():
> >
> > case 32:
> > dev_dbg(&host->pdev->dev, "Setting up RGB888/666 
> > mode\n");
> > ctrl |= CTRL_SET_WORD_LENGTH(3);
> > switch (host->ld_intf_width) {
> > case STMLCDIF_8BIT:
> > dev_dbg(&host->pdev->dev,
> > "Unsupported LCD bus width 
> > mapping\n");
> > 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;
> > }
> >
> > According to the manual, this flag does:
> > 0x0: ALL_24_BITS_VALID: Data input to the block is in 24 bpp
> > format, such that all RGB 888 data is contained in 24 bits.
> > 0x1: DROP_UPPER_2_BITS_PER_BYTE — Data input to the block is
> > actually RGB 18 bpp, but there is 1 colour per byte, hence the upper
> > 2 bits in each byte do not contain any useful data, and should be
> > dropped.
> >
> > The setting of this flag is producing bad colours with true colour
> > images (i.e. the Linux penguin is displayed ok, but QT applications
> > or images displayed with fbv are not).
> > I believe the setting of this flag is not correct (after all, if my
> > bpp is 32, then all 24bit colours are useful and dropping the upper
> > 2 bits is a bad idea).
> > If I don't set it, then true colour images are displayed correctly.
> > The only problem is that the Linux penguin is displayed much darker
> > than usual (correct colours, but darker). Perhaps the 224 colour
> > format of this image justifies it?
> >
> > I noticed the cfa10049 platform also uses the same configuration (18
> > bits data bus and 32bpp) and was wondering if true colour images are
> > correctly displayed in this platform with this flag set (for example
> > with fbv application [1]).
> 
>  I had the exact same problem, and suggested the exact same solution a
>  few weeks back.
> 
>  https://patchwork.kernel.org/patch/2470441/
> 
>  The conclusion of that discussion what that the userspace applications
>  were not honouring the bitfield correctly set by the mxsfb driver, and
>  as such, it was not a bug in the driver.
> 
>  While this is correct, I wonder, now that since we had that same
>  problem in a very short amount of time, if we couldn't set this
>  behaviour dependant of some (dt? kernel argument?) property so that
>  one could customise it anyway he want.
> 
>  Maxime
> >>>
> >>> i.MX2[3|8]LCD1   LCD2   LCD3
> >>> 24bit  18bit  18bit
> >>> 
> >>> LCD_D0 B0 B0 --
> >>> LCD_D1 B1 B1 --
> >>> LCD_D2 B2 B2 B0
> >>> LCD_D3 B3 B3 B1
> >>> LCD_D4 B4 B4 B2
> >>> LCD_D5 B5 B5 B3
> >>> LCD_D6 B6 G0 B4
> >>> LCD_D7 B7 G1 B5
> >>>
> >>> LCD_D8 G0 G2 --
> >>> LCD_D9 G1 G3 --
> >>> LCD_D10G2 G4 G0
> >>> LCD_D11G3 G5 G1
> >>> LCD_D12G4 R0 G2
> >>> LCD_D13G5 R1 G3
> >>> LCD_D14G6 R2 G4
> >>> LCD_D15G7 R3 G5
> >>>
> >>> LCD_D16R0 R4 --
> >>> LCD_D17R1 R5 --
> >>> LCD_D18R2R0
> >>> LCD_D19R3R1
> >>> LCD_D20R4R2
> >>> LCD_D21R5R3
> >>> LCD_

Re: mxsfb: DATA_FORMAT_24_BIT flag outputs invalid colours

2013-05-24 Thread Hector Palacios

Hi Juergen,

On 05/24/2013 12:28 PM, Juergen Beisert wrote:

Hector Palacios wrote:

Hi Juergen,

On 05/23/2013 03:31 PM, Juergen Beisert wrote:

Hi Maxime,

maxime.rip...@free-electrons.com wrote:

On Thu, May 23, 2013 at 01:55:28PM +0200, Hector Palacios wrote:

I'm using an i.MX28 based board with lcd connected with 18bits data
bus. My platform uses 32 bits per pixel:

mxsfb_pdata.default_bpp = 32;
mxsfb_pdata.ld_intf_width = STMLCDIF_18BIT;

With these settings the mxsfb.c driver sets flag DATA_FORMAT_24_BIT
at HW_LCDIF_CTRL register in function mxsfb_set_par():

case 32:
dev_dbg(&host->pdev->dev, "Setting up RGB888/666 mode\n");
ctrl |= CTRL_SET_WORD_LENGTH(3);
switch (host->ld_intf_width) {
case STMLCDIF_8BIT:
dev_dbg(&host->pdev->dev,
"Unsupported LCD bus width mapping\n");
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;
}

According to the manual, this flag does:
0x0: ALL_24_BITS_VALID: Data input to the block is in 24 bpp
format, such that all RGB 888 data is contained in 24 bits.
0x1: DROP_UPPER_2_BITS_PER_BYTE — Data input to the block is
actually RGB 18 bpp, but there is 1 colour per byte, hence the upper
2 bits in each byte do not contain any useful data, and should be
dropped.

The setting of this flag is producing bad colours with true colour
images (i.e. the Linux penguin is displayed ok, but QT applications
or images displayed with fbv are not).
I believe the setting of this flag is not correct (after all, if my
bpp is 32, then all 24bit colours are useful and dropping the upper
2 bits is a bad idea).
If I don't set it, then true colour images are displayed correctly.
The only problem is that the Linux penguin is displayed much darker
than usual (correct colours, but darker). Perhaps the 224 colour
format of this image justifies it?

I noticed the cfa10049 platform also uses the same configuration (18
bits data bus and 32bpp) and was wondering if true colour images are
correctly displayed in this platform with this flag set (for example
with fbv application [1]).


I had the exact same problem, and suggested the exact same solution a
few weeks back.

https://patchwork.kernel.org/patch/2470441/

The conclusion of that discussion what that the userspace applications
were not honouring the bitfield correctly set by the mxsfb driver, and
as such, it was not a bug in the driver.

While this is correct, I wonder, now that since we had that same problem
in a very short amount of time, if we couldn't set this behaviour
dependant of some (dt? kernel argument?) property so that one could
customise it anyway he want.

Maxime


i.MX2[3|8]LCD1   LCD2   LCD3
24bit  18bit  18bit

LCD_D0 B0 B0 --
LCD_D1 B1 B1 --
LCD_D2 B2 B2 B0
LCD_D3 B3 B3 B1
LCD_D4 B4 B4 B2
LCD_D5 B5 B5 B3
LCD_D6 B6 G0 B4
LCD_D7 B7 G1 B5

LCD_D8 G0 G2 --
LCD_D9 G1 G3 --
LCD_D10G2 G4 G0
LCD_D11G3 G5 G1
LCD_D12G4 R0 G2
LCD_D13G5 R1 G3
LCD_D14G6 R2 G4
LCD_D15G7 R3 G5

LCD_D16R0 R4 --
LCD_D17R1 R5 --
LCD_D18R2R0
LCD_D19R3R1
LCD_D20R4R2
LCD_D21R5R3
LCD_D22R6R4
LCD_D23R7R5

Is your display connected like LCD2 or LCD3? LCD3 must still handled like
a 24 bit display shown in LCD1, while only the LCD2-case is the "24 bit
to 18 bit mapping" case.

At least my current tests with an i.MX23 and a connection like LCD2 are
working here with a Qt application. Qt honours the pixel bitfield
description. And I'm using the "bits-per-pixel = <32>" and "bus-width =
<18>" entries in the device tree.


I have a 24bit LCD display but my connection to it is done at 18bits data
width. Represented below as LCD4.
NOTE: In my LCD4 column, notation Rx/Gx/Bx represent the color bit in
memory as well as the display data line.
Since we use 32bpp each channel has 8 bits (R7..R0, et

Re: mxsfb: DATA_FORMAT_24_BIT flag outputs invalid colours

2013-05-24 Thread Juergen Beisert
Hector Palacios wrote:
> Hi Juergen,
>
> On 05/23/2013 03:31 PM, Juergen Beisert wrote:
> > Hi Maxime,
> >
> > maxime.rip...@free-electrons.com wrote:
> >> On Thu, May 23, 2013 at 01:55:28PM +0200, Hector Palacios wrote:
> >>> I'm using an i.MX28 based board with lcd connected with 18bits data
> >>> bus. My platform uses 32 bits per pixel:
> >>>
> >>>   mxsfb_pdata.default_bpp = 32;
> >>>   mxsfb_pdata.ld_intf_width = STMLCDIF_18BIT;
> >>>
> >>> With these settings the mxsfb.c driver sets flag DATA_FORMAT_24_BIT
> >>> at HW_LCDIF_CTRL register in function mxsfb_set_par():
> >>>
> >>>   case 32:
> >>>   dev_dbg(&host->pdev->dev, "Setting up RGB888/666 mode\n");
> >>>   ctrl |= CTRL_SET_WORD_LENGTH(3);
> >>>   switch (host->ld_intf_width) {
> >>>   case STMLCDIF_8BIT:
> >>>   dev_dbg(&host->pdev->dev,
> >>>   "Unsupported LCD bus width mapping\n");
> >>>   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;
> >>>   }
> >>>
> >>> According to the manual, this flag does:
> >>>   0x0: ALL_24_BITS_VALID: Data input to the block is in 24 bpp
> >>> format, such that all RGB 888 data is contained in 24 bits.
> >>>   0x1: DROP_UPPER_2_BITS_PER_BYTE — Data input to the block is
> >>> actually RGB 18 bpp, but there is 1 colour per byte, hence the upper
> >>> 2 bits in each byte do not contain any useful data, and should be
> >>> dropped.
> >>>
> >>> The setting of this flag is producing bad colours with true colour
> >>> images (i.e. the Linux penguin is displayed ok, but QT applications
> >>> or images displayed with fbv are not).
> >>> I believe the setting of this flag is not correct (after all, if my
> >>> bpp is 32, then all 24bit colours are useful and dropping the upper
> >>> 2 bits is a bad idea).
> >>> If I don't set it, then true colour images are displayed correctly.
> >>> The only problem is that the Linux penguin is displayed much darker
> >>> than usual (correct colours, but darker). Perhaps the 224 colour
> >>> format of this image justifies it?
> >>>
> >>> I noticed the cfa10049 platform also uses the same configuration (18
> >>> bits data bus and 32bpp) and was wondering if true colour images are
> >>> correctly displayed in this platform with this flag set (for example
> >>> with fbv application [1]).
> >>
> >> I had the exact same problem, and suggested the exact same solution a
> >> few weeks back.
> >>
> >> https://patchwork.kernel.org/patch/2470441/
> >>
> >> The conclusion of that discussion what that the userspace applications
> >> were not honouring the bitfield correctly set by the mxsfb driver, and
> >> as such, it was not a bug in the driver.
> >>
> >> While this is correct, I wonder, now that since we had that same problem
> >> in a very short amount of time, if we couldn't set this behaviour
> >> dependant of some (dt? kernel argument?) property so that one could
> >> customise it anyway he want.
> >>
> >> Maxime
> >
> > i.MX2[3|8]LCD1   LCD2   LCD3
> >24bit  18bit  18bit
> > 
> > LCD_D0 B0 B0 --
> > LCD_D1 B1 B1 --
> > LCD_D2 B2 B2 B0
> > LCD_D3 B3 B3 B1
> > LCD_D4 B4 B4 B2
> > LCD_D5 B5 B5 B3
> > LCD_D6 B6 G0 B4
> > LCD_D7 B7 G1 B5
> >
> > LCD_D8 G0 G2 --
> > LCD_D9 G1 G3 --
> > LCD_D10G2 G4 G0
> > LCD_D11G3 G5 G1
> > LCD_D12G4 R0 G2
> > LCD_D13G5 R1 G3
> > LCD_D14G6 R2 G4
> > LCD_D15G7 R3 G5
> >
> > LCD_D16R0 R4 --
> > LCD_D17R1 R5 --
> > LCD_D18R2R0
> > LCD_D19R3R1
> > LCD_D20R4R2
> > LCD_D21R5R3
> > LCD_D22R6R4
> > LCD_D23R7R5
> >
> > Is your display connected like LCD2 or LCD3? LCD3 must still handled like
> > a 24 bit display shown in LCD1, while only the LCD2-case is the "24 bit
> > to 18 bit mapping" case.
> >
> > At least my current tests with an i.MX23 and a connection like LCD2 are
> > working here with a Qt application. Qt honours the pixel bitfield
> > description. And I'm using the "bits-pe

Re: mxsfb: DATA_FORMAT_24_BIT flag outputs invalid colours

2013-05-24 Thread maxime.rip...@free-electrons.com
Hi Juergen,

On Thu, May 23, 2013 at 03:31:31PM +0200, Juergen Beisert wrote:
> Hi Maxime,
> 
> maxime.rip...@free-electrons.com wrote:
> > On Thu, May 23, 2013 at 01:55:28PM +0200, Hector Palacios wrote:
> > > I'm using an i.MX28 based board with lcd connected with 18bits data bus.
> > > My platform uses 32 bits per pixel:
> > >
> > >   mxsfb_pdata.default_bpp = 32;
> > >   mxsfb_pdata.ld_intf_width = STMLCDIF_18BIT;
> > >
> > > With these settings the mxsfb.c driver sets flag DATA_FORMAT_24_BIT
> > > at HW_LCDIF_CTRL register in function mxsfb_set_par():
> > >
> > >   case 32:
> > >   dev_dbg(&host->pdev->dev, "Setting up RGB888/666 mode\n");
> > >   ctrl |= CTRL_SET_WORD_LENGTH(3);
> > >   switch (host->ld_intf_width) {
> > >   case STMLCDIF_8BIT:
> > >   dev_dbg(&host->pdev->dev,
> > >   "Unsupported LCD bus width mapping\n");
> > >   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;
> > >   }
> > >
> > > According to the manual, this flag does:
> > >   0x0: ALL_24_BITS_VALID: Data input to the block is in 24 bpp
> > > format, such that all RGB 888 data is contained in 24 bits.
> > >   0x1: DROP_UPPER_2_BITS_PER_BYTE — Data input to the block is
> > > actually RGB 18 bpp, but there is 1 colour per byte, hence the upper
> > > 2 bits in each byte do not contain any useful data, and should be
> > > dropped.
> > >
> > > The setting of this flag is producing bad colours with true colour
> > > images (i.e. the Linux penguin is displayed ok, but QT applications
> > > or images displayed with fbv are not).
> > > I believe the setting of this flag is not correct (after all, if my
> > > bpp is 32, then all 24bit colours are useful and dropping the upper
> > > 2 bits is a bad idea).
> > > If I don't set it, then true colour images are displayed correctly.
> > > The only problem is that the Linux penguin is displayed much darker
> > > than usual (correct colours, but darker). Perhaps the 224 colour
> > > format of this image justifies it?
> > >
> > > I noticed the cfa10049 platform also uses the same configuration (18
> > > bits data bus and 32bpp) and was wondering if true colour images are
> > > correctly displayed in this platform with this flag set (for example
> > > with fbv application [1]).
> >
> > I had the exact same problem, and suggested the exact same solution a
> > few weeks back.
> >
> > https://patchwork.kernel.org/patch/2470441/
> >
> > The conclusion of that discussion what that the userspace applications
> > were not honouring the bitfield correctly set by the mxsfb driver, and
> > as such, it was not a bug in the driver.
> >
> > While this is correct, I wonder, now that since we had that same problem
> > in a very short amount of time, if we couldn't set this behaviour
> > dependant of some (dt? kernel argument?) property so that one could
> > customise it anyway he want.
> >
> > Maxime
> 
> i.MX2[3|8]LCD1   LCD2   LCD3
>   24bit  18bit  18bit
> 
> LCD_D0 B0 B0 --
> LCD_D1 B1 B1 --
> LCD_D2 B2 B2 B0
> LCD_D3 B3 B3 B1
> LCD_D4 B4 B4 B2
> LCD_D5 B5 B5 B3
> LCD_D6 B6 G0 B4
> LCD_D7 B7 G1 B5
> 
> LCD_D8 G0 G2 --
> LCD_D9 G1 G3 --
> LCD_D10G2 G4 G0
> LCD_D11G3 G5 G1
> LCD_D12G4 R0 G2
> LCD_D13G5 R1 G3
> LCD_D14G6 R2 G4
> LCD_D15G7 R3 G5
> 
> LCD_D16R0 R4 --
> LCD_D17R1 R5 --
> LCD_D18R2R0
> LCD_D19R3R1
> LCD_D20R4R2
> LCD_D21R5R3
> LCD_D22R6R4
> LCD_D23R7R5
> 
> Is your display connected like LCD2 or LCD3? LCD3 must still handled like a 
> 24 
> bit display shown in LCD1, while only the LCD2-case is the "24 bit to 18 bit 
> mapping" case.
> 
> At least my current tests with an i.MX23 and a connection like LCD2 are 
> working here with a Qt application. Qt honours the pixel bitfield 
> description. And I'm using the "bits-per-pixel = <32>" and "bus-width = <18>" 
> entries in the device tree.

I'm in the second case, so

Re: mxsfb: DATA_FORMAT_24_BIT flag outputs invalid colours

2013-05-23 Thread Hector Palacios

Hi Juergen,

On 05/23/2013 03:31 PM, Juergen Beisert wrote:

Hi Maxime,

maxime.rip...@free-electrons.com wrote:

On Thu, May 23, 2013 at 01:55:28PM +0200, Hector Palacios wrote:

I'm using an i.MX28 based board with lcd connected with 18bits data bus.
My platform uses 32 bits per pixel:

mxsfb_pdata.default_bpp = 32;
mxsfb_pdata.ld_intf_width = STMLCDIF_18BIT;

With these settings the mxsfb.c driver sets flag DATA_FORMAT_24_BIT
at HW_LCDIF_CTRL register in function mxsfb_set_par():

case 32:
dev_dbg(&host->pdev->dev, "Setting up RGB888/666 mode\n");
ctrl |= CTRL_SET_WORD_LENGTH(3);
switch (host->ld_intf_width) {
case STMLCDIF_8BIT:
dev_dbg(&host->pdev->dev,
"Unsupported LCD bus width mapping\n");
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;
}

According to the manual, this flag does:
0x0: ALL_24_BITS_VALID: Data input to the block is in 24 bpp
format, such that all RGB 888 data is contained in 24 bits.
0x1: DROP_UPPER_2_BITS_PER_BYTE — Data input to the block is
actually RGB 18 bpp, but there is 1 colour per byte, hence the upper
2 bits in each byte do not contain any useful data, and should be
dropped.

The setting of this flag is producing bad colours with true colour
images (i.e. the Linux penguin is displayed ok, but QT applications
or images displayed with fbv are not).
I believe the setting of this flag is not correct (after all, if my
bpp is 32, then all 24bit colours are useful and dropping the upper
2 bits is a bad idea).
If I don't set it, then true colour images are displayed correctly.
The only problem is that the Linux penguin is displayed much darker
than usual (correct colours, but darker). Perhaps the 224 colour
format of this image justifies it?

I noticed the cfa10049 platform also uses the same configuration (18
bits data bus and 32bpp) and was wondering if true colour images are
correctly displayed in this platform with this flag set (for example
with fbv application [1]).


I had the exact same problem, and suggested the exact same solution a
few weeks back.

https://patchwork.kernel.org/patch/2470441/

The conclusion of that discussion what that the userspace applications
were not honouring the bitfield correctly set by the mxsfb driver, and
as such, it was not a bug in the driver.

While this is correct, I wonder, now that since we had that same problem
in a very short amount of time, if we couldn't set this behaviour
dependant of some (dt? kernel argument?) property so that one could
customise it anyway he want.

Maxime


i.MX2[3|8]LCD1   LCD2   LCD3
   24bit  18bit  18bit

LCD_D0 B0 B0 --
LCD_D1 B1 B1 --
LCD_D2 B2 B2 B0
LCD_D3 B3 B3 B1
LCD_D4 B4 B4 B2
LCD_D5 B5 B5 B3
LCD_D6 B6 G0 B4
LCD_D7 B7 G1 B5

LCD_D8 G0 G2 --
LCD_D9 G1 G3 --
LCD_D10G2 G4 G0
LCD_D11G3 G5 G1
LCD_D12G4 R0 G2
LCD_D13G5 R1 G3
LCD_D14G6 R2 G4
LCD_D15G7 R3 G5

LCD_D16R0 R4 --
LCD_D17R1 R5 --
LCD_D18R2R0
LCD_D19R3R1
LCD_D20R4R2
LCD_D21R5R3
LCD_D22R6R4
LCD_D23R7R5

Is your display connected like LCD2 or LCD3? LCD3 must still handled like a 24
bit display shown in LCD1, while only the LCD2-case is the "24 bit to 18 bit
mapping" case.

At least my current tests with an i.MX23 and a connection like LCD2 are
working here with a Qt application. Qt honours the pixel bitfield
description. And I'm using the "bits-per-pixel = <32>" and "bus-width = <18>"
entries in the device tree.


I have a 24bit LCD display but my connection to it is done at 18bits data width.
Represented below as LCD4.
NOTE: In my LCD4 column, notation Rx/Gx/Bx represent the color bit in memory as well 
as the display data line.

Since we use 32bpp each channel has 8 bits (R7..R0, etc.).
I understand that you have an 18bit display and that your notation in LCD2 colu

Re: mxsfb: DATA_FORMAT_24_BIT flag outputs invalid colours

2013-05-23 Thread Juergen Beisert
Hi Maxime,

maxime.rip...@free-electrons.com wrote:
> On Thu, May 23, 2013 at 01:55:28PM +0200, Hector Palacios wrote:
> > I'm using an i.MX28 based board with lcd connected with 18bits data bus.
> > My platform uses 32 bits per pixel:
> >
> > mxsfb_pdata.default_bpp = 32;
> > mxsfb_pdata.ld_intf_width = STMLCDIF_18BIT;
> >
> > With these settings the mxsfb.c driver sets flag DATA_FORMAT_24_BIT
> > at HW_LCDIF_CTRL register in function mxsfb_set_par():
> >
> > case 32:
> > dev_dbg(&host->pdev->dev, "Setting up RGB888/666 mode\n");
> > ctrl |= CTRL_SET_WORD_LENGTH(3);
> > switch (host->ld_intf_width) {
> > case STMLCDIF_8BIT:
> > dev_dbg(&host->pdev->dev,
> > "Unsupported LCD bus width mapping\n");
> > 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;
> > }
> >
> > According to the manual, this flag does:
> > 0x0: ALL_24_BITS_VALID: Data input to the block is in 24 bpp
> > format, such that all RGB 888 data is contained in 24 bits.
> > 0x1: DROP_UPPER_2_BITS_PER_BYTE — Data input to the block is
> > actually RGB 18 bpp, but there is 1 colour per byte, hence the upper
> > 2 bits in each byte do not contain any useful data, and should be
> > dropped.
> >
> > The setting of this flag is producing bad colours with true colour
> > images (i.e. the Linux penguin is displayed ok, but QT applications
> > or images displayed with fbv are not).
> > I believe the setting of this flag is not correct (after all, if my
> > bpp is 32, then all 24bit colours are useful and dropping the upper
> > 2 bits is a bad idea).
> > If I don't set it, then true colour images are displayed correctly.
> > The only problem is that the Linux penguin is displayed much darker
> > than usual (correct colours, but darker). Perhaps the 224 colour
> > format of this image justifies it?
> >
> > I noticed the cfa10049 platform also uses the same configuration (18
> > bits data bus and 32bpp) and was wondering if true colour images are
> > correctly displayed in this platform with this flag set (for example
> > with fbv application [1]).
>
> I had the exact same problem, and suggested the exact same solution a
> few weeks back.
>
> https://patchwork.kernel.org/patch/2470441/
>
> The conclusion of that discussion what that the userspace applications
> were not honouring the bitfield correctly set by the mxsfb driver, and
> as such, it was not a bug in the driver.
>
> While this is correct, I wonder, now that since we had that same problem
> in a very short amount of time, if we couldn't set this behaviour
> dependant of some (dt? kernel argument?) property so that one could
> customise it anyway he want.
>
> Maxime

i.MX2[3|8]LCD1   LCD2   LCD3
  24bit  18bit  18bit

LCD_D0 B0 B0 --
LCD_D1 B1 B1 --
LCD_D2 B2 B2 B0
LCD_D3 B3 B3 B1
LCD_D4 B4 B4 B2
LCD_D5 B5 B5 B3
LCD_D6 B6 G0 B4
LCD_D7 B7 G1 B5

LCD_D8 G0 G2 --
LCD_D9 G1 G3 --
LCD_D10G2 G4 G0
LCD_D11G3 G5 G1
LCD_D12G4 R0 G2
LCD_D13G5 R1 G3
LCD_D14G6 R2 G4
LCD_D15G7 R3 G5

LCD_D16R0 R4 --
LCD_D17R1 R5 --
LCD_D18R2R0
LCD_D19R3R1
LCD_D20R4R2
LCD_D21R5R3
LCD_D22R6R4
LCD_D23R7R5

Is your display connected like LCD2 or LCD3? LCD3 must still handled like a 24 
bit display shown in LCD1, while only the LCD2-case is the "24 bit to 18 bit 
mapping" case.

At least my current tests with an i.MX23 and a connection like LCD2 are 
working here with a Qt application. Qt honours the pixel bitfield 
description. And I'm using the "bits-per-pixel = <32>" and "bus-width = <18>" 
entries in the device tree.

Regards,
Juergen

-- 
Pengutronix e.K.  | Juergen Beisert |
Linux Solutions for Science and Industry  | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ma

Re: mxsfb: DATA_FORMAT_24_BIT flag outputs invalid colours

2013-05-23 Thread maxime.rip...@free-electrons.com
Hi Hector,

On Thu, May 23, 2013 at 01:55:28PM +0200, Hector Palacios wrote:
> Hello,
> 
> I'm using an i.MX28 based board with lcd connected with 18bits data bus.
> My platform uses 32 bits per pixel:
> 
>   mxsfb_pdata.default_bpp = 32;
>   mxsfb_pdata.ld_intf_width = STMLCDIF_18BIT;
> 
> With these settings the mxsfb.c driver sets flag DATA_FORMAT_24_BIT
> at HW_LCDIF_CTRL register in function mxsfb_set_par():
> 
>   case 32:
>   dev_dbg(&host->pdev->dev, "Setting up RGB888/666 mode\n");
>   ctrl |= CTRL_SET_WORD_LENGTH(3);
>   switch (host->ld_intf_width) {
>   case STMLCDIF_8BIT:
>   dev_dbg(&host->pdev->dev,
>   "Unsupported LCD bus width mapping\n");
>   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;
>   }
> 
> According to the manual, this flag does:
>   0x0: ALL_24_BITS_VALID: Data input to the block is in 24 bpp
> format, such that all RGB 888 data is contained in 24 bits.
>   0x1: DROP_UPPER_2_BITS_PER_BYTE — Data input to the block is
> actually RGB 18 bpp, but there is 1 colour per byte, hence the upper
> 2 bits in each byte do not contain any useful data, and should be
> dropped.
> 
> The setting of this flag is producing bad colours with true colour
> images (i.e. the Linux penguin is displayed ok, but QT applications
> or images displayed with fbv are not).
> I believe the setting of this flag is not correct (after all, if my
> bpp is 32, then all 24bit colours are useful and dropping the upper
> 2 bits is a bad idea).
> If I don't set it, then true colour images are displayed correctly.
> The only problem is that the Linux penguin is displayed much darker
> than usual (correct colours, but darker). Perhaps the 224 colour
> format of this image justifies it?
> 
> I noticed the cfa10049 platform also uses the same configuration (18
> bits data bus and 32bpp) and was wondering if true colour images are
> correctly displayed in this platform with this flag set (for example
> with fbv application [1]).

I had the exact same problem, and suggested the exact same solution a
few weeks back.

https://patchwork.kernel.org/patch/2470441/

The conclusion of that discussion what that the userspace applications
were not honouring the bitfield correctly set by the mxsfb driver, and
as such, it was not a bug in the driver.

While this is correct, I wonder, now that since we had that same problem
in a very short amount of time, if we couldn't set this behaviour
dependant of some (dt? kernel argument?) property so that one could
customise it anyway he want.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/