[PATCH] Add RGB555X and RGB565X formats to pxa-camera

[PATCH] Add RGB555X and RGB565X formats to pxa-camera

[Guennadi Liakhovetski]

On Mon, 3 Aug 2009, Mike Rapoport wrote:

> > 2. Mike, while reviewing this patch I came across code in 
> > pxa_camera_setup_cicr(), introduced by your earlier patch:
> > 
> > 	case V4L2_PIX_FMT_RGB555:
> > 		cicr1 |= CICR1_RGB_BPP_VAL(1) | CICR1_RGBT_CONV_VAL(2) |
> > 			CICR1_TBIT | CICR1_COLOR_SP_VAL(1);
> > 		break;
> > 
> > Why are you enabling the RGB to RGBT conversion here unconditionally? 
> > Generally, what are the advantages of configuring CICR1 for a specific RGB 
> > format compared to using just a raw capture? Do I understand it right, 
> > that ATM we are not using any of those features?
> 
> As far as I remember I've tried to overlay the captured imagery using pxa
> overlay1. Most probably it's left here after those tries.

Mike, could you, please, verify that those bits are indeed unneeded and 
provide patch to remove them?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH] Add RGB555X and RGB565X formats to pxa-camera

[Mike Rapoport]

Hi Guennadi,

Guennadi Liakhovetski wrote:
> On Mon, 3 Aug 2009, Mike Rapoport wrote:
> 
>>> 2. Mike, while reviewing this patch I came across code in 
>>> pxa_camera_setup_cicr(), introduced by your earlier patch:
>>>
>>> 	case V4L2_PIX_FMT_RGB555:
>>> 		cicr1 |= CICR1_RGB_BPP_VAL(1) | CICR1_RGBT_CONV_VAL(2) |
>>> 			CICR1_TBIT | CICR1_COLOR_SP_VAL(1);
>>> 		break;
>>>
>>> Why are you enabling the RGB to RGBT conversion here unconditionally? 
>>> Generally, what are the advantages of configuring CICR1 for a specific RGB 
>>> format compared to using just a raw capture? Do I understand it right, 
>>> that ATM we are not using any of those features?
>> As far as I remember I've tried to overlay the captured imagery using pxa
>> overlay1. Most probably it's left here after those tries.
> 
> Mike, could you, please, verify that those bits are indeed unneeded and 
> provide patch to remove them?

Unfortunately, I don't have the sensor handy at the time :( The one I've used
then is now broken (physically) and there's no replacement for it :(

> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
Sincerely yours,
Mike.

[PATCH] Add RGB555X and RGB565X formats to pxa-camera

[Marek Vasut]

Dne Po 3. srpna 2009 14:31:12 Guennadi Liakhovetski napsal(a):
> On Mon, 3 Aug 2009, Eric Miao wrote:
> > Marek Vasut wrote:
> > > Hi!
> > >
> > > Eric, would you mind applying ?
> > >
> > > From 4dcbff010e996f4c6e5761b3c19f5d863ab51b39 Mon Sep 17 00:00:00 2001
> > > From: Marek Vasut <marek.vasut@gmail.com>
> > > Date: Mon, 3 Aug 2009 10:27:57 +0200
> > > Subject: [PATCH] Add RGB555X and RGB565X formats to pxa-camera
> > >
> > > Those formats are requiered on widely used OmniVision OV96xx cameras.
> > > Those formats are nothing more then endian-swapped RGB555 and RGB565.
> > >
> > > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> >
> > Acked-by: Eric Miao <eric.y.miao@gmail.com>
> >
> > Guennadi,
> >
> > Would be better if this gets merged by you, thanks.
>
> Indeed it would, and I do have a couple of questions to this and related
> patches:
>
> 1. Marek, you're saying, you need these formats for the OV96xx camera. Yre
> you using the patch from Stefan Herbrechtsmeier to support ov96xx or some
> other?
>
> 2. Mike, while reviewing this patch I came across code in
> pxa_camera_setup_cicr(), introduced by your earlier patch:
>
> 	case V4L2_PIX_FMT_RGB555:
> 		cicr1 |= CICR1_RGB_BPP_VAL(1) | CICR1_RGBT_CONV_VAL(2) |
> 			CICR1_TBIT | CICR1_COLOR_SP_VAL(1);
> 		break;
>
> Why are you enabling the RGB to RGBT conversion here unconditionally?
> Generally, what are the advantages of configuring CICR1 for a specific RGB
> format compared to using just a raw capture? Do I understand it right,
> that ATM we are not using any of those features?
>
> Thanks
> Guennadi
>
> > > ---
> > >  drivers/media/video/pxa_camera.c |    4 ++++
> > >  1 files changed, 4 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/media/video/pxa_camera.c
> > > b/drivers/media/video/pxa_camera.c
> > > index 46e0d8a..de0fc8a 100644
> > > --- a/drivers/media/video/pxa_camera.c
> > > +++ b/drivers/media/video/pxa_camera.c
> > > @@ -1222,6 +1222,8 @@ static int required_buswidth(const struct
> > > soc_camera_data_format *fmt)
> > >  	case V4L2_PIX_FMT_YVYU:
> > >  	case V4L2_PIX_FMT_RGB565:
> > >  	case V4L2_PIX_FMT_RGB555:
> > > +	case V4L2_PIX_FMT_RGB565X:
> > > +	case V4L2_PIX_FMT_RGB555X:
> > >  		return 8;
> > >  	default:
> > >  		return fmt->depth;
> > > @@ -1260,6 +1262,8 @@ static int pxa_camera_get_formats(struct
> > > soc_camera_device *icd, int idx,
> > >  	case V4L2_PIX_FMT_YVYU:
> > >  	case V4L2_PIX_FMT_RGB565:
> > >  	case V4L2_PIX_FMT_RGB555:
> > > +	case V4L2_PIX_FMT_RGB565X:
> > > +	case V4L2_PIX_FMT_RGB555X:
> > >  		formats++;
> > >  		if (xlate) {
> > >  			xlate->host_fmt = icd->formats + idx;
>
> ---
> Guennadi Liakhovetski

What should we do with this patch? Any updates? I spoke to Guennadi and he 
thinks it's not a good idea to apply it (as pxaqci doesnt support those 
formats). But to my understanding, those formats are endian-swapped versions of 
the other ones without X at the end so there shouldnt be a problem with it.

[PATCH] Add RGB555X and RGB565X formats to pxa-camera

[Guennadi Liakhovetski]

On Sat, 5 Sep 2009, Marek Vasut wrote:

> > > >  drivers/media/video/pxa_camera.c |    4 ++++
> > > >  1 files changed, 4 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/drivers/media/video/pxa_camera.c
> > > > b/drivers/media/video/pxa_camera.c
> > > > index 46e0d8a..de0fc8a 100644
> > > > --- a/drivers/media/video/pxa_camera.c
> > > > +++ b/drivers/media/video/pxa_camera.c
> > > > @@ -1222,6 +1222,8 @@ static int required_buswidth(const struct
> > > > soc_camera_data_format *fmt)
> > > >  	case V4L2_PIX_FMT_YVYU:
> > > >  	case V4L2_PIX_FMT_RGB565:
> > > >  	case V4L2_PIX_FMT_RGB555:
> > > > +	case V4L2_PIX_FMT_RGB565X:
> > > > +	case V4L2_PIX_FMT_RGB555X:
> > > >  		return 8;
> > > >  	default:
> > > >  		return fmt->depth;
> > > > @@ -1260,6 +1262,8 @@ static int pxa_camera_get_formats(struct
> > > > soc_camera_device *icd, int idx,
> > > >  	case V4L2_PIX_FMT_YVYU:
> > > >  	case V4L2_PIX_FMT_RGB565:
> > > >  	case V4L2_PIX_FMT_RGB555:
> > > > +	case V4L2_PIX_FMT_RGB565X:
> > > > +	case V4L2_PIX_FMT_RGB555X:
> > > >  		formats++;
> > > >  		if (xlate) {
> > > >  			xlate->host_fmt = icd->formats + idx;
> 
> What should we do with this patch? Any updates? I spoke to Guennadi and he 
> thinks it's not a good idea to apply it (as pxaqci doesnt support those 
> formats). But to my understanding, those formats are endian-swapped versions of 
> the other ones without X at the end so there shouldnt be a problem with it.

Marek, please, look in PXA270 datasheet. To support a specific pixel 
format means, e.g., to be able to process it further, according to this 
format's particular colour component ordering. Process further can mean 
convert to another format, extract various information from the data 
(statistics, etc.)... Now RGB555 looks like (from wikipedia)

15  14  13  12  11  10  09  08  07  06  05  04  03  02  01  00
R4  R3  R2  R1  R0  G4  G3  G2  G1  G1  B4  B3  B2  B1  B1  --

(Actually, I thought bit 15 was unused, but it doesn't matter for this 
discussion.) Now, imagine what happens if you swap the two bytes. I don't 
think the PXA will still be able to meaningfully process that format.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH] Add RGB555X and RGB565X formats to pxa-camera

[Marek Vasut]

Dne So 5. z??? 2009 10:55:55 Guennadi Liakhovetski napsal(a):
> On Sat, 5 Sep 2009, Marek Vasut wrote:
> > > > >  drivers/media/video/pxa_camera.c |    4 ++++
> > > > >  1 files changed, 4 insertions(+), 0 deletions(-)
> > > > >
> > > > > diff --git a/drivers/media/video/pxa_camera.c
> > > > > b/drivers/media/video/pxa_camera.c
> > > > > index 46e0d8a..de0fc8a 100644
> > > > > --- a/drivers/media/video/pxa_camera.c
> > > > > +++ b/drivers/media/video/pxa_camera.c
> > > > > @@ -1222,6 +1222,8 @@ static int required_buswidth(const struct
> > > > > soc_camera_data_format *fmt)
> > > > >  	case V4L2_PIX_FMT_YVYU:
> > > > >  	case V4L2_PIX_FMT_RGB565:
> > > > >  	case V4L2_PIX_FMT_RGB555:
> > > > > +	case V4L2_PIX_FMT_RGB565X:
> > > > > +	case V4L2_PIX_FMT_RGB555X:
> > > > >  		return 8;
> > > > >  	default:
> > > > >  		return fmt->depth;
> > > > > @@ -1260,6 +1262,8 @@ static int pxa_camera_get_formats(struct
> > > > > soc_camera_device *icd, int idx,
> > > > >  	case V4L2_PIX_FMT_YVYU:
> > > > >  	case V4L2_PIX_FMT_RGB565:
> > > > >  	case V4L2_PIX_FMT_RGB555:
> > > > > +	case V4L2_PIX_FMT_RGB565X:
> > > > > +	case V4L2_PIX_FMT_RGB555X:
> > > > >  		formats++;
> > > > >  		if (xlate) {
> > > > >  			xlate->host_fmt = icd->formats + idx;
> >
> > What should we do with this patch? Any updates? I spoke to Guennadi and
> > he thinks it's not a good idea to apply it (as pxaqci doesnt support
> > those formats). But to my understanding, those formats are endian-swapped
> > versions of the other ones without X at the end so there shouldnt be a
> > problem with it.
>
> Marek, please, look in PXA270 datasheet. To support a specific pixel
> format means, e.g., to be able to process it further, according to this
> format's particular colour component ordering. Process further can mean
> convert to another format, extract various information from the data
> (statistics, etc.)... Now RGB555 looks like (from wikipedia)
>
> 15  14  13  12  11  10  09  08  07  06  05  04  03  02  01  00
> R4  R3  R2  R1  R0  G4  G3  G2  G1  G1  B4  B3  B2  B1  B1  --
>
> (Actually, I thought bit 15 was unused, but it doesn't matter for this
> discussion.) Now, imagine what happens if you swap the two bytes. I don't
> think the PXA will still be able to meaningfully process that format.
>

Not on the pxa side, but on the camera side -- Bs and Rs swapped in the diagram 
above.
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/

[PATCH] Add RGB555X and RGB565X formats to pxa-camera

[Guennadi Liakhovetski]

On Sat, 5 Sep 2009, Marek Vasut wrote:

> Dne So 5. z??? 2009 10:55:55 Guennadi Liakhovetski napsal(a):
> > On Sat, 5 Sep 2009, Marek Vasut wrote:
> > > > > >  drivers/media/video/pxa_camera.c |    4 ++++
> > > > > >  1 files changed, 4 insertions(+), 0 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/media/video/pxa_camera.c
> > > > > > b/drivers/media/video/pxa_camera.c
> > > > > > index 46e0d8a..de0fc8a 100644
> > > > > > --- a/drivers/media/video/pxa_camera.c
> > > > > > +++ b/drivers/media/video/pxa_camera.c
> > > > > > @@ -1222,6 +1222,8 @@ static int required_buswidth(const struct
> > > > > > soc_camera_data_format *fmt)
> > > > > >  	case V4L2_PIX_FMT_YVYU:
> > > > > >  	case V4L2_PIX_FMT_RGB565:
> > > > > >  	case V4L2_PIX_FMT_RGB555:
> > > > > > +	case V4L2_PIX_FMT_RGB565X:
> > > > > > +	case V4L2_PIX_FMT_RGB555X:
> > > > > >  		return 8;
> > > > > >  	default:
> > > > > >  		return fmt->depth;
> > > > > > @@ -1260,6 +1262,8 @@ static int pxa_camera_get_formats(struct
> > > > > > soc_camera_device *icd, int idx,
> > > > > >  	case V4L2_PIX_FMT_YVYU:
> > > > > >  	case V4L2_PIX_FMT_RGB565:
> > > > > >  	case V4L2_PIX_FMT_RGB555:
> > > > > > +	case V4L2_PIX_FMT_RGB565X:
> > > > > > +	case V4L2_PIX_FMT_RGB555X:
> > > > > >  		formats++;
> > > > > >  		if (xlate) {
> > > > > >  			xlate->host_fmt = icd->formats + idx;
> > >
> > > What should we do with this patch? Any updates? I spoke to Guennadi and
> > > he thinks it's not a good idea to apply it (as pxaqci doesnt support
> > > those formats). But to my understanding, those formats are endian-swapped
> > > versions of the other ones without X at the end so there shouldnt be a
> > > problem with it.
> >
> > Marek, please, look in PXA270 datasheet. To support a specific pixel
> > format means, e.g., to be able to process it further, according to this
> > format's particular colour component ordering. Process further can mean
> > convert to another format, extract various information from the data
> > (statistics, etc.)... Now RGB555 looks like (from wikipedia)
> >
> > 15  14  13  12  11  10  09  08  07  06  05  04  03  02  01  00
> > R4  R3  R2  R1  R0  G4  G3  G2  G1  G1  B4  B3  B2  B1  B1  --
> >
> > (Actually, I thought bit 15 was unused, but it doesn't matter for this
> > discussion.) Now, imagine what happens if you swap the two bytes. I don't
> > think the PXA will still be able to meaningfully process that format.
> >
> 
> Not on the pxa side, but on the camera side -- Bs and Rs swapped in the diagram 
> above.

And then? Are you trying to tell me, that the PXA then swaps them back?...

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH] Add RGB555X and RGB565X formats to pxa-camera

[Marek Vasut]

Dne So 5. z??? 2009 22:19:42 Guennadi Liakhovetski napsal(a):
> On Sat, 5 Sep 2009, Marek Vasut wrote:
> > Dne So 5. z??? 2009 10:55:55 Guennadi Liakhovetski napsal(a):
> > > On Sat, 5 Sep 2009, Marek Vasut wrote:
> > > > > > >  drivers/media/video/pxa_camera.c |    4 ++++
> > > > > > >  1 files changed, 4 insertions(+), 0 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/media/video/pxa_camera.c
> > > > > > > b/drivers/media/video/pxa_camera.c
> > > > > > > index 46e0d8a..de0fc8a 100644
> > > > > > > --- a/drivers/media/video/pxa_camera.c
> > > > > > > +++ b/drivers/media/video/pxa_camera.c
> > > > > > > @@ -1222,6 +1222,8 @@ static int required_buswidth(const struct
> > > > > > > soc_camera_data_format *fmt)
> > > > > > >  	case V4L2_PIX_FMT_YVYU:
> > > > > > >  	case V4L2_PIX_FMT_RGB565:
> > > > > > >  	case V4L2_PIX_FMT_RGB555:
> > > > > > > +	case V4L2_PIX_FMT_RGB565X:
> > > > > > > +	case V4L2_PIX_FMT_RGB555X:
> > > > > > >  		return 8;
> > > > > > >  	default:
> > > > > > >  		return fmt->depth;
> > > > > > > @@ -1260,6 +1262,8 @@ static int pxa_camera_get_formats(struct
> > > > > > > soc_camera_device *icd, int idx,
> > > > > > >  	case V4L2_PIX_FMT_YVYU:
> > > > > > >  	case V4L2_PIX_FMT_RGB565:
> > > > > > >  	case V4L2_PIX_FMT_RGB555:
> > > > > > > +	case V4L2_PIX_FMT_RGB565X:
> > > > > > > +	case V4L2_PIX_FMT_RGB555X:
> > > > > > >  		formats++;
> > > > > > >  		if (xlate) {
> > > > > > >  			xlate->host_fmt = icd->formats + idx;
> > > >
> > > > What should we do with this patch? Any updates? I spoke to Guennadi
> > > > and he thinks it's not a good idea to apply it (as pxaqci doesnt
> > > > support those formats). But to my understanding, those formats are
> > > > endian-swapped versions of the other ones without X at the end so
> > > > there shouldnt be a problem with it.
> > >
> > > Marek, please, look in PXA270 datasheet. To support a specific pixel
> > > format means, e.g., to be able to process it further, according to this
> > > format's particular colour component ordering. Process further can mean
> > > convert to another format, extract various information from the data
> > > (statistics, etc.)... Now RGB555 looks like (from wikipedia)
> > >
> > > 15  14  13  12  11  10  09  08  07  06  05  04  03  02  01  00
> > > R4  R3  R2  R1  R0  G4  G3  G2  G1  G1  B4  B3  B2  B1  B1  --
> > >
> > > (Actually, I thought bit 15 was unused, but it doesn't matter for this
> > > discussion.) Now, imagine what happens if you swap the two bytes. I
> > > don't think the PXA will still be able to meaningfully process that
> > > format.
> >
> > Not on the pxa side, but on the camera side -- Bs and Rs swapped in the
> > diagram above.
>
> And then? Are you trying to tell me, that the PXA then swaps them back?...

No, the software has to do it then, I'm trying to tell you that it has nothing 
to do with PXA (as PXA really doesnt care if the channel is actually blue or 
red).
>
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/

[PATCH] Add RGB555X and RGB565X formats to pxa-camera

[Guennadi Liakhovetski]

On Sat, 5 Sep 2009, Marek Vasut wrote:

> Dne So 5. z?????? 2009 22:19:42 Guennadi Liakhovetski napsal(a):
> > On Sat, 5 Sep 2009, Marek Vasut wrote:
> > > Dne So 5. z?????? 2009 10:55:55 Guennadi Liakhovetski napsal(a):
> > > >
> > > > Marek, please, look in PXA270 datasheet. To support a specific pixel
> > > > format means, e.g., to be able to process it further, according to this
> > > > format's particular colour component ordering. Process further can mean
> > > > convert to another format, extract various information from the data
> > > > (statistics, etc.)... Now RGB555 looks like (from wikipedia)
> > > >
> > > > 15  14  13  12  11  10  09  08  07  06  05  04  03  02  01  00
> > > > R4  R3  R2  R1  R0  G4  G3  G2  G1  G1  B4  B3  B2  B1  B1  --
> > > >
> > > > (Actually, I thought bit 15 was unused, but it doesn't matter for this
> > > > discussion.) Now, imagine what happens if you swap the two bytes. I
> > > > don't think the PXA will still be able to meaningfully process that
> > > > format.
> > >
> > > Not on the pxa side, but on the camera side -- Bs and Rs swapped in the
> > > diagram above.
> >
> > And then? Are you trying to tell me, that the PXA then swaps them back?...
> 
> No, the software has to do it then, I'm trying to tell you that it has nothing 
> to do with PXA (as PXA really doesnt care if the channel is actually blue or 
> red).

Of course it does. I asked you to swap the above two bytes, you would get 
this:

15  14  13  12  11  10  09  08  07  06  05  04  03  02  01  00
G1  G0  B4  B3  B2  B1  B0  --  R4  R3  R2  R1  R0  G4  G3  G2  

and PXA would still inerpret this as 

R4  R3  R2  R1  R0  G4  G3  G2  G1  G0  B4  B3  B2  B1  B0  --

i.e., it would take bits

R2 R1 R0 G4 G3

for blue, bits

B1 B0 -- R4 R3

for green, and bits

G1 G0 B4 B3 B2

as red. Which, as you see, makes no sense. That's why I'm saying, that it 
doesn't support this format, and we can only pass it through as raw data.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH] Add RGB555X and RGB565X formats to pxa-camera

[Marek Vasut]

Dne Ne 6. z??? 2009 00:05:14 Guennadi Liakhovetski napsal(a):
> On Sat, 5 Sep 2009, Marek Vasut wrote:
> > Dne So 5. z?????? 2009 22:19:42 Guennadi Liakhovetski napsal(a):
> > > On Sat, 5 Sep 2009, Marek Vasut wrote:
> > > > Dne So 5. z?????? 2009 10:55:55 Guennadi Liakhovetski napsal(a):
> > > > > Marek, please, look in PXA270 datasheet. To support a specific
> > > > > pixel format means, e.g., to be able to process it further,
> > > > > according to this format's particular colour component ordering.
> > > > > Process further can mean convert to another format, extract various
> > > > > information from the data (statistics, etc.)... Now RGB555 looks
> > > > > like (from wikipedia)
> > > > >
> > > > > 15  14  13  12  11  10  09  08  07  06  05  04  03  02  01  00
> > > > > R4  R3  R2  R1  R0  G4  G3  G2  G1  G1  B4  B3  B2  B1  B1  --
> > > > >
> > > > > (Actually, I thought bit 15 was unused, but it doesn't matter for
> > > > > this discussion.) Now, imagine what happens if you swap the two
> > > > > bytes. I don't think the PXA will still be able to meaningfully
> > > > > process that format.
> > > >
> > > > Not on the pxa side, but on the camera side -- Bs and Rs swapped in
> > > > the diagram above.
> > >
> > > And then? Are you trying to tell me, that the PXA then swaps them
> > > back?...
> >
> > No, the software has to do it then, I'm trying to tell you that it has
> > nothing to do with PXA (as PXA really doesnt care if the channel is
> > actually blue or red).
>
> Of course it does. I asked you to swap the above two bytes, you would get
> this:
>
> 15  14  13  12  11  10  09  08  07  06  05  04  03  02  01  00
> G1  G0  B4  B3  B2  B1  B0  --  R4  R3  R2  R1  R0  G4  G3  G2
>
> and PXA would still inerpret this as
>
> R4  R3  R2  R1  R0  G4  G3  G2  G1  G0  B4  B3  B2  B1  B0  --
>
> i.e., it would take bits
>
> R2 R1 R0 G4 G3
>
> for blue, bits
>
> B1 B0 -- R4 R3
>
> for green, and bits
>
> G1 G0 B4 B3 B2
>
> as red. Which, as you see, makes no sense. That's why I'm saying, that it
> doesn't support this format, and we can only pass it through as raw data.

Ah damn, I see what you mean. What the camera does is it swaps the RED and BLUE 
channel:
15  14  13  12  11  10  09  08  07  06  05  04  03  02  01  00
B4  B3  B2  B1  B0  G4  G3  G2  G1  G1  R4  R3  R2  R1  R1  --
so it's more a BGR555/565 then. I had to patch fswebcam for this.

Sorry for the confusion.
>
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/

[PATCH] Add RGB555X and RGB565X formats to pxa-camera

[Guennadi Liakhovetski]

On Sun, 6 Sep 2009, Marek Vasut wrote:

> Ah damn, I see what you mean. What the camera does is it swaps the RED and BLUE 
> channel:
> 15  14  13  12  11  10  09  08  07  06  05  04  03  02  01  00
> B4  B3  B2  B1  B0  G4  G3  G2  G1  G1  R4  R3  R2  R1  R1  --
> so it's more a BGR555/565 then. I had to patch fswebcam for this.

Ok, this is, of course, something different. In this case you, probably, 
could deceive the PXA to handle blue as red and the other way round, but 
still, I would prefer not to do that. Hence my suggestion remains - pass 
these formats as raw data.

The only case when you might want to put the PXA into RGB555 mode, while 
feeding BGR555 to it, is you want to use the QCI to set the transparency 
bit for you. But we currently do not support this any way, not in a 
configurable way at least. You would need to implement some sort of a 
"global (one-bit) alpha" control for pxa_camera to use this. Any need for 
this?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH] Add RGB555X and RGB565X formats to pxa-camera

[Marek Vasut]

Dne Ne 6. z??? 2009 18:52:55 Guennadi Liakhovetski napsal(a):
> On Sun, 6 Sep 2009, Marek Vasut wrote:
> > Ah damn, I see what you mean. What the camera does is it swaps the RED
> > and BLUE channel:
> > 15  14  13  12  11  10  09  08  07  06  05  04  03  02  01  00
> > B4  B3  B2  B1  B0  G4  G3  G2  G1  G1  R4  R3  R2  R1  R1  --
> > so it's more a BGR555/565 then. I had to patch fswebcam for this.
>
> Ok, this is, of course, something different. In this case you, probably,
> could deceive the PXA to handle blue as red and the other way round, but
> still, I would prefer not to do that. Hence my suggestion remains - pass
> these formats as raw data.
>
Which is bogus from the camera point of view.

> The only case when you might want to put the PXA into RGB555 mode, while
> feeding BGR555 to it, is you want to use the QCI to set the transparency
> bit for you. But we currently do not support this any way, not in a
> configurable way at least. You would need to implement some sort of a
> "global (one-bit) alpha" control for pxa_camera to use this. Any need for
> this?
>
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/

[PATCH] Add RGB555X and RGB565X formats to pxa-camera

[Guennadi Liakhovetski]

On Sun, 6 Sep 2009, Marek Vasut wrote:

> Dne Ne 6. z??? 2009 18:52:55 Guennadi Liakhovetski napsal(a):
> > On Sun, 6 Sep 2009, Marek Vasut wrote:
> > > Ah damn, I see what you mean. What the camera does is it swaps the RED
> > > and BLUE channel:
> > > 15  14  13  12  11  10  09  08  07  06  05  04  03  02  01  00
> > > B4  B3  B2  B1  B0  G4  G3  G2  G1  G1  R4  R3  R2  R1  R1  --
> > > so it's more a BGR555/565 then. I had to patch fswebcam for this.
> >
> > Ok, this is, of course, something different. In this case you, probably,
> > could deceive the PXA to handle blue as red and the other way round, but
> > still, I would prefer not to do that. Hence my suggestion remains - pass
> > these formats as raw data.
> >
> Which is bogus from the camera point of view.

Not at all. This just means: the subdevice provides a pixel format, that 
the bridge (PXA) knows nothing specific about, but it can just pass it 
one-to-one (as raw data) to the user - don't see anything bogus in this. 
Different bridges have support for different pixel colour formats, but, I 
think, all bridges can pass data as raw (pass-through). Some bridges can 
_only_ do this, so, this is actually the default video-capture mode.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH] Add RGB555X and RGB565X formats to pxa-camera

[Marek Vasut]

Dne Ne 6. z??? 2009 20:15:17 Guennadi Liakhovetski napsal(a):
> On Sun, 6 Sep 2009, Marek Vasut wrote:
> > Dne Ne 6. z??? 2009 18:52:55 Guennadi Liakhovetski napsal(a):
> > > On Sun, 6 Sep 2009, Marek Vasut wrote:
> > > > Ah damn, I see what you mean. What the camera does is it swaps the
> > > > RED and BLUE channel:
> > > > 15  14  13  12  11  10  09  08  07  06  05  04  03  02  01  00
> > > > B4  B3  B2  B1  B0  G4  G3  G2  G1  G1  R4  R3  R2  R1  R1  --
> > > > so it's more a BGR555/565 then. I had to patch fswebcam for this.
> > >
> > > Ok, this is, of course, something different. In this case you,
> > > probably, could deceive the PXA to handle blue as red and the other way
> > > round, but still, I would prefer not to do that. Hence my suggestion
> > > remains - pass these formats as raw data.
> >
> > Which is bogus from the camera point of view.
>
> Not at all. This just means: the subdevice provides a pixel format, that
> the bridge (PXA) knows nothing specific about, but it can just pass it
> one-to-one (as raw data) to the user - don't see anything bogus in this.
> Different bridges have support for different pixel colour formats, but, I
> think, all bridges can pass data as raw (pass-through). Some bridges can
> _only_ do this, so, this is actually the default video-capture mode.

But then you'll have to tell your software how to process the raw data (in what 
format they are). If there was this RGB565X passthrough support, the software 
could at least check if you are not forcing it to process nonsense.
>
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/

[PATCH] Add RGB555X and RGB565X formats to pxa-camera

[Guennadi Liakhovetski]

On Mon, 7 Sep 2009, Marek Vasut wrote:

> Dne Ne 6. z?????? 2009 20:15:17 Guennadi Liakhovetski napsal(a):
> > On Sun, 6 Sep 2009, Marek Vasut wrote:
> > > Dne Ne 6. z?????? 2009 18:52:55 Guennadi Liakhovetski napsal(a):
> > > > On Sun, 6 Sep 2009, Marek Vasut wrote:
> > > > > Ah damn, I see what you mean. What the camera does is it swaps the
> > > > > RED and BLUE channel:
> > > > > 15  14  13  12  11  10  09  08  07  06  05  04  03  02  01  00
> > > > > B4  B3  B2  B1  B0  G4  G3  G2  G1  G1  R4  R3  R2  R1  R1  --
> > > > > so it's more a BGR555/565 then. I had to patch fswebcam for this.
> > > >
> > > > Ok, this is, of course, something different. In this case you,
> > > > probably, could deceive the PXA to handle blue as red and the other way
> > > > round, but still, I would prefer not to do that. Hence my suggestion
> > > > remains - pass these formats as raw data.
> > >
> > > Which is bogus from the camera point of view.
> >
> > Not at all. This just means: the subdevice provides a pixel format, that
> > the bridge (PXA) knows nothing specific about, but it can just pass it
> > one-to-one (as raw data) to the user - don't see anything bogus in this.
> > Different bridges have support for different pixel colour formats, but, I
> > think, all bridges can pass data as raw (pass-through). Some bridges can
> > _only_ do this, so, this is actually the default video-capture mode.
> 
> But then you'll have to tell your software how to process the raw data (in what 
> format they are). If there was this RGB565X passthrough support, the software 
> could at least check if you are not forcing it to process nonsense.

There's no difference for user-space software. It requests BGR555 it gets 
BGR555 back, because that's the information the pxa driver will find in 
this format descriptor: if you take this data and put it in RAM in a 
certain way, you get BGR555.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH] Add RGB555X and RGB565X formats to pxa-camera

[Marek Vasut]

Dne Po 7. z??? 2009 08:22:22 Guennadi Liakhovetski napsal(a):
> On Mon, 7 Sep 2009, Marek Vasut wrote:
> > Dne Ne 6. z?????? 2009 20:15:17 Guennadi Liakhovetski napsal(a):
> > > On Sun, 6 Sep 2009, Marek Vasut wrote:
> > > > Dne Ne 6. z?????? 2009 18:52:55 Guennadi Liakhovetski napsal(a):
> > > > > On Sun, 6 Sep 2009, Marek Vasut wrote:
> > > > > > Ah damn, I see what you mean. What the camera does is it swaps
> > > > > > the RED and BLUE channel:
> > > > > > 15  14  13  12  11  10  09  08  07  06  05  04  03  02  01  00
> > > > > > B4  B3  B2  B1  B0  G4  G3  G2  G1  G1  R4  R3  R2  R1  R1  --
> > > > > > so it's more a BGR555/565 then. I had to patch fswebcam for this.
> > > > >
> > > > > Ok, this is, of course, something different. In this case you,
> > > > > probably, could deceive the PXA to handle blue as red and the other
> > > > > way round, but still, I would prefer not to do that. Hence my
> > > > > suggestion remains - pass these formats as raw data.
> > > >
> > > > Which is bogus from the camera point of view.
> > >
> > > Not at all. This just means: the subdevice provides a pixel format,
> > > that the bridge (PXA) knows nothing specific about, but it can just
> > > pass it one-to-one (as raw data) to the user - don't see anything bogus
> > > in this. Different bridges have support for different pixel colour
> > > formats, but, I think, all bridges can pass data as raw (pass-through).
> > > Some bridges can _only_ do this, so, this is actually the default
> > > video-capture mode.
> >
> > But then you'll have to tell your software how to process the raw data
> > (in what format they are). If there was this RGB565X passthrough support,
> > the software could at least check if you are not forcing it to process
> > nonsense.
>
> There's no difference for user-space software. It requests BGR555 it gets
> BGR555 back, because that's the information the pxa driver will find in
> this format descriptor: if you take this data and put it in RAM in a
> certain way, you get BGR555.

How's it supposed to get BGR555 if the pxa-camera doesnt support that ? Will the 
v4l2 layer convert it or something ?
>
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/

[PATCH] Add RGB555X and RGB565X formats to pxa-camera

[Robert Jarzmik]

Marek Vasut <marek.vasut@gmail.com> writes:

> How's it supposed to get BGR555 if the pxa-camera doesnt support that ? Will the 
> v4l2 layer convert it or something ?

In pxa_camera.c, function pxa_camera_get_formats() :
>         default:
>                 /* Generic pass-through */
>                 formats++;
>                 if (xlate) {
>                         xlate->host_fmt = icd->formats + idx;
>                         xlate->cam_fmt = icd->formats + idx;
>                         xlate->buswidth = icd->formats[idx].depth;
>                         xlate++;
>                         dev_dbg(ici->dev,
>                                 "Providing format %s in pass-through mode\n",
>                                 icd->formats[idx].name);
>                 }
>         }

"Pass-through" means that if a sensors provides a cc, ie. BGR555 for example,
the bridge (pxa_camera) will "forward" to RAM the image in the very same cc
(ie. BGR555). In that case, the bridge is a dummy "sensor to RAM" bus translator
if you prefer.

Marek, you should activate debug trace and watch for yourself. You can trust
Guennadi, when he says it will work, well ... it will work.

If it's out of technical curiousity, check the function above.  If you're even
more curious, there was a thread in linux-media about "RFC: bus configuration
setup for sub-devices", a very interesting one, especially considering the
"pass-through" issue.

Cheers.

--
Robert

[PATCH] Add RGB555X and RGB565X formats to pxa-camera

[Marek Vasut]

Dne Po 7. z??? 2009 12:21:50 Robert Jarzmik napsal(a):
> Marek Vasut <marek.vasut@gmail.com> writes:
> > How's it supposed to get BGR555 if the pxa-camera doesnt support that ?
> > Will the v4l2 layer convert it or something ?
>
> In pxa_camera.c, function pxa_camera_get_formats() :
> >         default:
> >                 /* Generic pass-through */
> >                 formats++;
> >                 if (xlate) {
> >                         xlate->host_fmt = icd->formats + idx;
> >                         xlate->cam_fmt = icd->formats + idx;
> >                         xlate->buswidth = icd->formats[idx].depth;
> >                         xlate++;
> >                         dev_dbg(ici->dev,
> >                                 "Providing format %s in pass-through
> > mode\n", icd->formats[idx].name);
> >                 }
> >         }
>
> "Pass-through" means that if a sensors provides a cc, ie. BGR555 for
> example, the bridge (pxa_camera) will "forward" to RAM the image in the
> very same cc (ie. BGR555). In that case, the bridge is a dummy "sensor to
> RAM" bus translator if you prefer.
>
> Marek, you should activate debug trace and watch for yourself. You can
> trust Guennadi, when he says it will work, well ... it will work.
>
> If it's out of technical curiousity, check the function above.  If you're
> even more curious, there was a thread in linux-media about "RFC: bus
> configuration setup for sub-devices", a very interesting one, especially
> considering the "pass-through" issue.

That one should work for RGB565X ? (the piece of code you posted ?) It's 
interesting it didnt work for me ... ok, I'll take it it works then, whatever. 
I'll test it again when I have time.
>
> Cheers.
>
> --
> Robert

[PATCH] Add RGB555X and RGB565X formats to pxa-camera

[Guennadi Liakhovetski]

On Mon, 7 Sep 2009, Marek Vasut wrote:

> Dne Po 7. z??? 2009 12:21:50 Robert Jarzmik napsal(a):
> > Marek Vasut <marek.vasut@gmail.com> writes:
> > > How's it supposed to get BGR555 if the pxa-camera doesnt support that ?
> > > Will the v4l2 layer convert it or something ?
> >
> > In pxa_camera.c, function pxa_camera_get_formats() :
> > >         default:
> > >                 /* Generic pass-through */
> > >                 formats++;
> > >                 if (xlate) {
> > >                         xlate->host_fmt = icd->formats + idx;
> > >                         xlate->cam_fmt = icd->formats + idx;
> > >                         xlate->buswidth = icd->formats[idx].depth;
> > >                         xlate++;
> > >                         dev_dbg(ici->dev,
> > >                                 "Providing format %s in pass-through
> > > mode\n", icd->formats[idx].name);
> > >                 }
> > >         }
> >
> > "Pass-through" means that if a sensors provides a cc, ie. BGR555 for
> > example, the bridge (pxa_camera) will "forward" to RAM the image in the
> > very same cc (ie. BGR555). In that case, the bridge is a dummy "sensor to
> > RAM" bus translator if you prefer.
> >
> > Marek, you should activate debug trace and watch for yourself. You can
> > trust Guennadi, when he says it will work, well ... it will work.
> >
> > If it's out of technical curiousity, check the function above.  If you're
> > even more curious, there was a thread in linux-media about "RFC: bus
> > configuration setup for sub-devices", a very interesting one, especially
> > considering the "pass-through" issue.
> 
> That one should work for RGB565X ? (the piece of code you posted ?) It's 
> interesting it didnt work for me ... ok, I'll take it it works then, whatever. 
> I'll test it again when I have time.

No, that one cannot work with rgb565x or any other format, not supported 
by pxa directly and not complying to bus-width = bit-depth. But it will 
work with the new imagebus API. Either see my explanations in a previous 
mail, or check the patches I posted recently to the list.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH] Add RGB555X and RGB565X formats to pxa-camera

[Jamie Lokier]

Guennadi Liakhovetski wrote:
 Now RGB555 looks like (from wikipedia)
> 
> 15  14  13  12  11  10  09  08  07  06  05  04  03  02  01  00
> R4  R3  R2  R1  R0  G4  G3  G2  G1  G1  B4  B3  B2  B1  B1  --
> 
> (Actually, I thought bit 15 was unused, but it doesn't matter for this 
> discussion.)

You are right: sometimes RGB555 means bit 15 is unused.  Wikipedia
isn't always right.

-- Jamie