Re: [PATCHv4][ 3/7] staging: imx-drm: Add RGB666 support for parallel display.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi Russell,

On Wednesday 13 November 2013 19:12:30 Russell King - ARM Linux wrote:
> On Wed, Nov 13, 2013 at 11:43:44AM -0700, Troy Kisky wrote:
> > On 11/13/2013 2:23 AM, Denis Carikli wrote:
> >>   +	/* rgb666 */
> >> 
> >> +	ipu_dc_map_clear(priv, IPU_DC_MAP_RGB666);
> >> +	ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 2, 17, 0xfc); /* red */
> >> +	ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 1, 11, 0xfc); /* green */
> >> +	ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 0, 5, 0xfc); /* blue */
> >> +
> >>   	return 0;
> >>   }
> > 
> > Since,  rgb24 and bgr24 reverse the byte numbers
> > /* rgb24 */
> >         ipu_dc_map_clear(priv, IPU_DC_MAP_RGB24);
> >         ipu_dc_map_config(priv, IPU_DC_MAP_RGB24, 0, 7, 0xff); /* blue */
> >         ipu_dc_map_config(priv, IPU_DC_MAP_RGB24, 1, 15, 0xff); /* green
> >         */
> >         ipu_dc_map_config(priv, IPU_DC_MAP_RGB24, 2, 23, 0xff); /* red */
> > 
> > /* bgr24 */
> >         ipu_dc_map_clear(priv, IPU_DC_MAP_BGR24);
> >         ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 2, 7, 0xff); /* red */
> >         ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 1, 15, 0xff); /* green
> >         */
> >         ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 0, 23, 0xff); /* blue */
> > 
> > Shouldn't rgb666 and bgr666 do the same?
> > Currently we have,
> > 
> > /* bgr666 */
> >         ipu_dc_map_clear(priv, IPU_DC_MAP_BGR666);
> >         ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 0, 5, 0xfc); /* blue */
> >         ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 1, 11, 0xfc); /*
> > green */
> >         ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 17, 0xfc); /* red */
> 
> Yes, I concur - this doesn't make sense to me.  BGR666 would mean in
> memory:
> 
>         1    11
>         7    21    65    0
>         BBBBBBGGGGGGRRRRRR
> 
> which reflects the same order for "RGB24" above.

Beside component order and number of bits per component, an in-memory RGB 
format also defines the memory endianness and, for formats that don't span an 
interger number of bytes, the left or right alignment.

BGR666 is currently defined in V4L2 as

Byte 0                         1                        2
Bit  7  6  5  4  3  2  1  0    7  6  5  4  3  2  1  0   7  6  5  4  3  2  1  0

     b5 b4 b3 b2 b1 b0 g5 g4  g3 g2 g1 g0 r5 r4 r3 r2  r1 r0  -  -  -  -  -  -

(see the *second* table in http://linuxtv.org/downloads/v4l-dvb-apis/packed-rgb.html)

I would thus expect RGB666 to be

Byte 0                         1                        2
Bit  7  6  5  4  3  2  1  0    7  6  5  4  3  2  1  0   7  6  5  4  3  2  1  0

     r5 r4 r3 r2 r1 r0 g5 g4  g3 g2 g1 g0 b5 b4 b3 b2  b1 b0  -  -  -  -  -  -

We can also define right-aligned formats if needed.

> > Where I'd expect to see
> > /* bgr666 */
> > 
> >         ipu_dc_map_clear(priv, IPU_DC_MAP_BGR666);
> >         ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 0, 17, 0xfc); /* blue
> >         */
> >         ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 1, 11, 0xfc); /*
> > green */
> >         ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 5, 0xfc); /* red */
> 
> So this makes sense to me.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux