public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mt9m111: Fix YUYV format for pxa-camera
@ 2008-10-29 22:25 Antonio Ospite
  2008-10-31 17:21 ` Robert Jarzmik
  0 siblings, 1 reply; 24+ messages in thread
From: Antonio Ospite @ 2008-10-29 22:25 UTC (permalink / raw)
  To: video4linux-list; +Cc: Guennadi Liakhovetski

Use 16 bit depth for YUYV so the pxa-camera image buffer has the correct size,
see the formula:

	*size = icd->width * icd->height *
		((icd->current_fmt->depth + 7) >> 3);

in drivers/media/video/pxa_camera.c: pxa_videobuf_setup().

Don't swap Cb and Cr components, to respect PXA Quick Capture Interface
data format.

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>

---
 drivers/media/video/mt9m111.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index da0b2d5..76fb0cb 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -130,7 +130,7 @@
 	COL_FMT(_name, _depth, _fourcc, V4L2_COLORSPACE_SRGB)
 
 static const struct soc_camera_data_format mt9m111_colour_formats[] = {
-	COL_FMT("YCrYCb 8 bit", 8, V4L2_PIX_FMT_YUYV, V4L2_COLORSPACE_JPEG),
+	COL_FMT("YCrYCb 16 bit", 16, V4L2_PIX_FMT_YUYV, V4L2_COLORSPACE_JPEG),
 	RGB_FMT("RGB 565", 16, V4L2_PIX_FMT_RGB565),
 	RGB_FMT("RGB 555", 16, V4L2_PIX_FMT_RGB555),
 	RGB_FMT("Bayer (sRGB) 10 bit", 10, V4L2_PIX_FMT_SBGGR16),
@@ -864,6 +864,9 @@ static int mt9m111_video_probe(struct soc_camera_device *icd)
 	mt9m111->swap_rgb_even_odd = 1;
 	mt9m111->swap_rgb_red_blue = 1;
 
+	mt9m111->swap_yuv_y_chromas = 1;
+	mt9m111->swap_yuv_cb_cr = 0;
+
 	return 0;
 eisis:
 ei2c:

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH] mt9m111: Fix YUYV format for pxa-camera
  2008-10-29 22:25 [PATCH] mt9m111: Fix YUYV format for pxa-camera Antonio Ospite
@ 2008-10-31 17:21 ` Robert Jarzmik
  2008-10-31 17:40   ` Guennadi Liakhovetski
                     ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Robert Jarzmik @ 2008-10-31 17:21 UTC (permalink / raw)
  To: Antonio Ospite, Guennadi Liakhovetski; +Cc: video4linux-list

Antonio Ospite <ospite@studenti.unina.it> writes:

> Use 16 bit depth for YUYV so the pxa-camera image buffer has the correct size,
> see the formula:
>
> 	*size = icd->width * icd->height *
> 		((icd->current_fmt->depth + 7) >> 3);
>
> in drivers/media/video/pxa_camera.c: pxa_videobuf_setup().
>
> Don't swap Cb and Cr components, to respect PXA Quick Capture Interface
> data format.
>
> Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>

As a side note, I wonder how you found the right swap :
 - I based the code on Intel PXA Developer Manual, table 27-19 (page 1127)
 - and on MT9M111 specification sheet, table 3 (page 14)
My guess is that the PXA manual is wrong somehow ...

Anyway, I'm happy with the patch. Guennadi, could you add this patch to your
queue please ?

--
Robert

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] mt9m111: Fix YUYV format for pxa-camera
  2008-10-31 17:21 ` Robert Jarzmik
@ 2008-10-31 17:40   ` Guennadi Liakhovetski
  2008-10-31 18:04     ` Robert Jarzmik
  2008-10-31 17:45   ` Antonio Ospite
  2008-11-02 21:15   ` Guennadi Liakhovetski
  2 siblings, 1 reply; 24+ messages in thread
From: Guennadi Liakhovetski @ 2008-10-31 17:40 UTC (permalink / raw)
  To: Robert Jarzmik; +Cc: video4linux-list

On Fri, 31 Oct 2008, Robert Jarzmik wrote:

> Antonio Ospite <ospite@studenti.unina.it> writes:
> 
> > Use 16 bit depth for YUYV so the pxa-camera image buffer has the correct size,
> > see the formula:
> >
> > 	*size = icd->width * icd->height *
> > 		((icd->current_fmt->depth + 7) >> 3);
> >
> > in drivers/media/video/pxa_camera.c: pxa_videobuf_setup().
> >
> > Don't swap Cb and Cr components, to respect PXA Quick Capture Interface
> > data format.
> >
> > Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
> Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
> 
> As a side note, I wonder how you found the right swap :
>  - I based the code on Intel PXA Developer Manual, table 27-19 (page 1127)
>  - and on MT9M111 specification sheet, table 3 (page 14)
> My guess is that the PXA manual is wrong somehow ...
> 
> Anyway, I'm happy with the patch. Guennadi, could you add this patch to your
> queue please ?

Do I understand it right, that although this wasn't a regression, this is 
a bug-fix, right? If so, it should go into 2.6.28. I'll take care of it.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] mt9m111: Fix YUYV format for pxa-camera
  2008-10-31 17:21 ` Robert Jarzmik
  2008-10-31 17:40   ` Guennadi Liakhovetski
@ 2008-10-31 17:45   ` Antonio Ospite
  2008-11-02 21:15   ` Guennadi Liakhovetski
  2 siblings, 0 replies; 24+ messages in thread
From: Antonio Ospite @ 2008-10-31 17:45 UTC (permalink / raw)
  To: video4linux-list, Guennadi Liakhovetski


[-- Attachment #1.1: Type: text/plain, Size: 1139 bytes --]

On Fri, 31 Oct 2008 18:21:20 +0100
Robert Jarzmik <robert.jarzmik@free.fr> wrote:

> Antonio Ospite <ospite@studenti.unina.it> writes:
>
> >
> > Don't swap Cb and Cr components, to respect PXA Quick Capture Interface
> > data format.
> >
> > Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
> Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
> 
> As a side note, I wonder how you found the right swap :
>  - I based the code on Intel PXA Developer Manual, table 27-19 (page 1127)
>  - and on MT9M111 specification sheet, table 3 (page 14)
> My guess is that the PXA manual is wrong somehow ...
>

Hi Robert,

I used the old scientific method: trial and error :)
I dumped the data to a file with a test app and then I tried different
combinations converting it from yuyv to rgb.

Regards,
   Antonio Ospite

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

  Web site: http://www.studenti.unina.it/~ospite
Public key: http://www.studenti.unina.it/~ospite/aopubkey.asc

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 164 bytes --]

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] mt9m111: Fix YUYV format for pxa-camera
  2008-10-31 17:40   ` Guennadi Liakhovetski
@ 2008-10-31 18:04     ` Robert Jarzmik
  0 siblings, 0 replies; 24+ messages in thread
From: Robert Jarzmik @ 2008-10-31 18:04 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: video4linux-list

Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:

> Do I understand it right, that although this wasn't a regression, this is 
> a bug-fix, right? If so, it should go into 2.6.28. I'll take care of it.
Yes, a bug fix, and yes, not a regression, just my silly mind trusting the
specifications :)

And yes, should go into 2.6.28-rc series IMO, thanks.

--
Robert

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] mt9m111: Fix YUYV format for pxa-camera
  2008-10-31 17:21 ` Robert Jarzmik
  2008-10-31 17:40   ` Guennadi Liakhovetski
  2008-10-31 17:45   ` Antonio Ospite
@ 2008-11-02 21:15   ` Guennadi Liakhovetski
  2008-11-03 17:23     ` Robert Jarzmik
  2 siblings, 1 reply; 24+ messages in thread
From: Guennadi Liakhovetski @ 2008-11-02 21:15 UTC (permalink / raw)
  To: Robert Jarzmik; +Cc: video4linux-list

On Fri, 31 Oct 2008, Robert Jarzmik wrote:

> Antonio Ospite <ospite@studenti.unina.it> writes:
> 
> > Use 16 bit depth for YUYV so the pxa-camera image buffer has the correct size,
> > see the formula:
> >
> > 	*size = icd->width * icd->height *
> > 		((icd->current_fmt->depth + 7) >> 3);
> >
> > in drivers/media/video/pxa_camera.c: pxa_videobuf_setup().
> >
> > Don't swap Cb and Cr components, to respect PXA Quick Capture Interface
> > data format.
> >
> > Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
> Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
> 
> As a side note, I wonder how you found the right swap :
>  - I based the code on Intel PXA Developer Manual, table 27-19 (page 1127)
>  - and on MT9M111 specification sheet, table 3 (page 14)
> My guess is that the PXA manual is wrong somehow ...
> 
> Anyway, I'm happy with the patch. Guennadi, could you add this patch to your
> queue please ?

Hm, I am not all that happy about it. The first part is ok - it is indeed 
a 16-bit format. But the second part doesn't seem right. Before you were 
using the non-swapped camera configuration unconditionally. Now you swap 
it - also unconditionally. Whereas under different conditions we need 
different byte order. Here we arrive again at pixel-format negotiation 
between the host and the camera...

Looking in the PXA270 datasheet, I would say, that it supports

V4L2_PIX_FMT_UYVY (Table 27-21, remember pxa270 is little-endian only)
and
V4L2_PIX_FMT_YUV422P (Table 27-20)

Whereas in the driver the former of the two modes is called

V4L2_PIX_FMT_YUYV

... This, I think, is a mistake.

Next, let's see how data is transferred over the 8-bit data bus. The 
PXA270 is expecting the data in the order

UYVY.... (Table 27-19)

MT9M111 can either send UYVY or YUYV depending on the value of bit 1 in 
Output Format Control register(s). Now we know, that 
OUTPUT_FORMAT_CTRL2[1] = 1 matches PXA270 format (UYUV), hence 
OUTPUT_FORMAT_CTRL2[1] = 0 means YUYV, which might be necessary for other 
camera hosts.

So, we have: PXA270 supports V4L2_PIX_FMT_UYVY on input and can convert it 
to either V4L2_PIX_FMT_UYVY or V4L2_PIX_FMT_YUV422P on output.

MT9M111 supports V4L2_PIX_FMT_UYVY and V4L2_PIX_FMT_YUYV (as well as 
V4L2_PIX_FMT_VYUY and V4L2_PIX_FMT_YVYU) on output. Now, PXA270 driver 
should accept user requests for UYVY and YUV422P and request UYUV from the 
camera.

I think, we could just add another format entry to mt9m111 with 
V4L2_PIX_FMT_UYVY and set OUTPUT_FORMAT_CTRL2[1] if _that_ format is 
requested, as opposed to V4L2_PIX_FMT_YUYV. This way mt9m111's behaviour 
will not change, and will even become correct:-) And in pxa driver we 
should check for V4L2_PIX_FMT_UYVY and _not_ V4L2_PIX_FMT_YUYV. And, of 
course, mt9m111 users will have to switch to use V4L2_PIX_FMT_UYVY in 
their user-space applications. Does this sound acceptable?

In the longer run we should do something along the lines:

1. merge .try_bus_param() into .try_fmt_cap() camera host methods - they 
are called only at one place one after another.

2. add a data format table list similar to the one in struct 
soc_camera_device (or identical) to struct soc_camera_host.

3. soc_camera.c should use the list from struct soc_camera_host in calls 
to format_by_fourcc().

4. camera host drivers then decide based upon user request which format to 
request from the camera in calls to .try_fmt_cap() and .set_fmt_cap() 
methods from struct soc_camera_ops.

Only when I'll be able to do this... Comments welcome.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] mt9m111: Fix YUYV format for pxa-camera
  2008-11-02 21:15   ` Guennadi Liakhovetski
@ 2008-11-03 17:23     ` Robert Jarzmik
  2008-11-03 19:09       ` Guennadi Liakhovetski
  0 siblings, 1 reply; 24+ messages in thread
From: Robert Jarzmik @ 2008-11-03 17:23 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: video4linux-list

Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:

> On Fri, 31 Oct 2008, Robert Jarzmik wrote:
>
> Hm, I am not all that happy about it. The first part is ok - it is indeed 
> a 16-bit format. But the second part doesn't seem right. Before you were 
> using the non-swapped camera configuration unconditionally. Now you swap 
> it - also unconditionally. Whereas under different conditions we need 
> different byte order. Here we arrive again at pixel-format negotiation 
> between the host and the camera...
We've already talked about it in August.

I agree the clean fix will be through format negociation. And that clean fix
will impact mt9m111 driver, pxa camera driver, and maybe other camera drivers.

So even if you or I make the clean patch for the 2.6.28-rc branch, this little
fix should go into 2.6.27.n fix branch, as all mt9m111 users are _by now_ on pxa
bus. The clean patch obviously can't, because of impacts on other drivers.

I would strongly encourage you to push the fix into the stable branch, and wait
for my clean patch or make the patch yourself.

> Next, let's see how data is transferred over the 8-bit data bus. The 
> PXA270 is expecting the data in the order
>
> UYVY.... (Table 27-19)
>
> MT9M111 can either send UYVY or YUYV depending on the value of bit 1 in 
> Output Format Control register(s). Now we know, that 
> OUTPUT_FORMAT_CTRL2[1] = 1 matches PXA270 format (UYUV), hence 
> OUTPUT_FORMAT_CTRL2[1] = 0 means YUYV, which might be necessary for other 
> camera hosts.
True. Assuming the mt9m111 specification is correct, table 27-19 is wrong, and
real byte order for pxa is UYUV.

> So, we have: PXA270 supports V4L2_PIX_FMT_UYVY on input and can convert it 
> to either V4L2_PIX_FMT_UYVY or V4L2_PIX_FMT_YUV422P on output.
Yep.

> MT9M111 supports V4L2_PIX_FMT_UYVY and V4L2_PIX_FMT_YUYV (as well as 
> V4L2_PIX_FMT_VYUY and V4L2_PIX_FMT_YVYU) on output. Now, PXA270 driver 
> should accept user requests for UYVY and YUV422P and request UYUV from the 
> camera.
Yep.
>
> I think, we could just add another format entry to mt9m111 with 
> V4L2_PIX_FMT_UYVY and set OUTPUT_FORMAT_CTRL2[1] if _that_ format is 
> requested, as opposed to V4L2_PIX_FMT_YUYV. This way mt9m111's behaviour 
> will not change, and will even become correct:-) And in pxa driver we 
> should check for V4L2_PIX_FMT_UYVY and _not_ V4L2_PIX_FMT_YUYV. And, of 
> course, mt9m111 users will have to switch to use V4L2_PIX_FMT_UYVY in 
> their user-space applications. Does this sound acceptable?
Yes. Even better, mt9m111 should offer :
 - V4L2_PIX_FMT_UYVY
 - V4L2_PIX_FMT_VYUY
 - V4L2_PIX_FMT_YUYV
 - V4L2_PIX_FMT_YVYU
And setup the output format accordingly.

> In the longer run we should do something along the lines:
>
> 1. merge .try_bus_param() into .try_fmt_cap() camera host methods - they 
> are called only at one place one after another.
Agreed.

> 2. add a data format table list similar to the one in struct 
> soc_camera_device (or identical) to struct soc_camera_host.
For negociation. Absolutely.

> 3. soc_camera.c should use the list from struct soc_camera_host in calls 
> to format_by_fourcc().
>
> 4. camera host drivers then decide based upon user request which format to 
> request from the camera in calls to .try_fmt_cap() and .set_fmt_cap() 
> methods from struct soc_camera_ops.
Yes.

I'll see what I can do. Meanwhile, please send the patch into the fix 2.6.27 fix branch.

--
Robert

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] mt9m111: Fix YUYV format for pxa-camera
  2008-11-03 17:23     ` Robert Jarzmik
@ 2008-11-03 19:09       ` Guennadi Liakhovetski
  2008-11-03 20:19         ` Robert Jarzmik
  0 siblings, 1 reply; 24+ messages in thread
From: Guennadi Liakhovetski @ 2008-11-03 19:09 UTC (permalink / raw)
  To: Robert Jarzmik; +Cc: video4linux-list

On Mon, 3 Nov 2008, Robert Jarzmik wrote:

> Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:
> 
> > On Fri, 31 Oct 2008, Robert Jarzmik wrote:
> >
> > Hm, I am not all that happy about it. The first part is ok - it is indeed 
> > a 16-bit format. But the second part doesn't seem right. Before you were 
> > using the non-swapped camera configuration unconditionally. Now you swap 
> > it - also unconditionally. Whereas under different conditions we need 
> > different byte order. Here we arrive again at pixel-format negotiation 
> > between the host and the camera...
> We've already talked about it in August.
> 
> I agree the clean fix will be through format negociation. And that clean fix
> will impact mt9m111 driver, pxa camera driver, and maybe other camera drivers.
> 
> So even if you or I make the clean patch for the 2.6.28-rc branch, this little
> fix should go into 2.6.27.n fix branch, as all mt9m111 users are _by now_ on pxa
> bus.

Hm, wondering how you know?:-) I agree most probably there are about 2 
users of this driver in the world:-), but who knows?

I'm adding Mike Rapoport - the author of YCbCr support in pxa-camera 
(sorry, should have done this from the beginning of this thread) to CC to 
ask for his opinion - do we understand it right that pxa270 actually 
supports UYVY and not YUYV? Any objections against fixing this? Here's a 
link to the archived thread: http://marc.info/?t=122531935000001&r=1&w=2

> The clean patch obviously can't, because of impacts on other drivers.
> 
> I would strongly encourage you to push the fix into the stable branch, and wait
> for my clean patch or make the patch yourself.
> 
> > Next, let's see how data is transferred over the 8-bit data bus. The 
> > PXA270 is expecting the data in the order
> >
> > UYVY.... (Table 27-19)
> >
> > MT9M111 can either send UYVY or YUYV depending on the value of bit 1 in 
> > Output Format Control register(s). Now we know, that 
> > OUTPUT_FORMAT_CTRL2[1] = 1 matches PXA270 format (UYUV), hence 
> > OUTPUT_FORMAT_CTRL2[1] = 0 means YUYV, which might be necessary for other 
> > camera hosts.
> True. Assuming the mt9m111 specification is correct, table 27-19 is wrong, and
> real byte order for pxa is UYUV.

I don't see why 27-19 should be wrong - it specifies exactly the byte 
order CbYCrY, i.r., UYVY (I think, this is what you meant by "UYUV".)

> > So, we have: PXA270 supports V4L2_PIX_FMT_UYVY on input and can convert it 
> > to either V4L2_PIX_FMT_UYVY or V4L2_PIX_FMT_YUV422P on output.
> Yep.
> 
> > MT9M111 supports V4L2_PIX_FMT_UYVY and V4L2_PIX_FMT_YUYV (as well as 
> > V4L2_PIX_FMT_VYUY and V4L2_PIX_FMT_YVYU) on output. Now, PXA270 driver 
> > should accept user requests for UYVY and YUV422P and request UYUV from the 
> > camera.
> Yep.
> >
> > I think, we could just add another format entry to mt9m111 with 
> > V4L2_PIX_FMT_UYVY and set OUTPUT_FORMAT_CTRL2[1] if _that_ format is 
> > requested, as opposed to V4L2_PIX_FMT_YUYV. This way mt9m111's behaviour 
> > will not change, and will even become correct:-) And in pxa driver we 
> > should check for V4L2_PIX_FMT_UYVY and _not_ V4L2_PIX_FMT_YUYV. And, of 
> > course, mt9m111 users will have to switch to use V4L2_PIX_FMT_UYVY in 
> > their user-space applications. Does this sound acceptable?
> Yes. Even better, mt9m111 should offer :
>  - V4L2_PIX_FMT_UYVY
>  - V4L2_PIX_FMT_VYUY
>  - V4L2_PIX_FMT_YUYV
>  - V4L2_PIX_FMT_YVYU
> And setup the output format accordingly.

Good, then this is the fix that I'd like to have. It seems pretty simple, 
it will preserve behaviour of mt9m111. It will change the behaviour of 
pxa-camera for the YUYV format, to be precise, it will stop supporting 
this format. So, I would print out a warning, explaining that this format 
is not supported by pxa270 and the user should use UYVY instead. I 
suggested to add only one format to mt9m111 so far, just because this is 
the easiest as a bug-fix. But if you prefer, you can add all four, yes.

> > In the longer run we should do something along the lines:
> >
> > 1. merge .try_bus_param() into .try_fmt_cap() camera host methods - they 
> > are called only at one place one after another.
> Agreed.
> 
> > 2. add a data format table list similar to the one in struct 
> > soc_camera_device (or identical) to struct soc_camera_host.
> For negociation. Absolutely.
> 
> > 3. soc_camera.c should use the list from struct soc_camera_host in calls 
> > to format_by_fourcc().
> >
> > 4. camera host drivers then decide based upon user request which format to 
> > request from the camera in calls to .try_fmt_cap() and .set_fmt_cap() 
> > methods from struct soc_camera_ops.
> Yes.
> 
> I'll see what I can do.

I started working on this, (1) is of course simple, then I fixed one more 
issue with bus-width configuration, and I started (2)...

> Meanwhile, please send the patch into the fix 2.6.27 fix branch.

Still, I would really prefer to see the version described above - add more 
formats to mt9m111 and fix pxa270 to claim the correct format and print a 
warning for YUYV. This shouldn't be much more difficult to make than the 
proposed patch, and it will be correct. I made enough bad experiences with 
"temporary fixes" to try to avoid them as much as possible:-)

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] mt9m111: Fix YUYV format for pxa-camera
  2008-11-03 19:09       ` Guennadi Liakhovetski
@ 2008-11-03 20:19         ` Robert Jarzmik
  2008-11-03 20:52           ` Guennadi Liakhovetski
  0 siblings, 1 reply; 24+ messages in thread
From: Robert Jarzmik @ 2008-11-03 20:19 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: video4linux-list

Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:

> Hm, wondering how you know?:-) I agree most probably there are about 2 
> users of this driver in the world:-), but who knows?
Yes, who knows ...

> I don't see why 27-19 should be wrong - it specifies exactly the byte 
> order CbYCrY, i.r., UYVY (I think, this is what you meant by "UYUV".)
Well, the one working configuration known, Antonio's is :
 - pxa CPU
 - mt9m111, with OUTPUT_FORMAT_CTRL[1]=1

According to the mt9m111 datasheet, table 3, page 14, when OUTPUT_FORMAT_CTRL[1]
= 1 and OUTPUT_FORMAT_CTRL[0] = 0, the MT9M111 sensor outputs data as follows :
 - byte1 = Y1
 - byte2 = Cb1
 - byte3 = Y2
 - byte4 = Cr1
 - and so on ...

According to PXA27x datasheet, table 27-19, input bytes from the sensor should
be :
 - byte1 = Cb1
 - byte2 = Y1
 - byte3 = Cr1
 - byte4 = Y2
 - and so on ...

Do you see the discrepancy now ? Either pxa27x datasheet of mt9m111 datasheet is
wrong, or else Antonio's setup wouldn't work.

> Good, then this is the fix that I'd like to have. It seems pretty simple, 
> it will preserve behaviour of mt9m111. It will change the behaviour of 
> pxa-camera for the YUYV format, to be precise, it will stop supporting 
> this format. So, I would print out a warning, explaining that this format 
> is not supported by pxa270 and the user should use UYVY instead. I 
> suggested to add only one format to mt9m111 so far, just because this is 
> the easiest as a bug-fix. But if you prefer, you can add all four, yes.
Right, so be it for the 4 formats.

> Still, I would really prefer to see the version described above - add more 
> formats to mt9m111 and fix pxa270 to claim the correct format and print a 
> warning for YUYV. This shouldn't be much more difficult to make than the 
> proposed patch, and it will be correct. I made enough bad experiences with 
> "temporary fixes" to try to avoid them as much as possible:-)

If you wish so. I'll watch you pushing the fix in the stable branch to see how
you convince people that changing the pxa camera driver is only a "fix" and not
an evolution :) Too bad for a quick working fix.

Cheers.

--
Robert

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] mt9m111: Fix YUYV format for pxa-camera
  2008-11-03 20:19         ` Robert Jarzmik
@ 2008-11-03 20:52           ` Guennadi Liakhovetski
  2008-11-03 22:26             ` Guennadi Liakhovetski
  0 siblings, 1 reply; 24+ messages in thread
From: Guennadi Liakhovetski @ 2008-11-03 20:52 UTC (permalink / raw)
  To: Robert Jarzmik; +Cc: video4linux-list

On Mon, 3 Nov 2008, Robert Jarzmik wrote:

> Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:
> 
> > Hm, wondering how you know?:-) I agree most probably there are about 2 
> > users of this driver in the world:-), but who knows?
> Yes, who knows ...
> 
> > I don't see why 27-19 should be wrong - it specifies exactly the byte 
> > order CbYCrY, i.r., UYVY (I think, this is what you meant by "UYUV".)
> Well, the one working configuration known, Antonio's is :
>  - pxa CPU
>  - mt9m111, with OUTPUT_FORMAT_CTRL[1]=1
> 
> According to the mt9m111 datasheet, table 3, page 14, when OUTPUT_FORMAT_CTRL[1]
> = 1 and OUTPUT_FORMAT_CTRL[0] = 0, the MT9M111 sensor outputs data as follows :
>  - byte1 = Y1
>  - byte2 = Cb1
>  - byte3 = Y2
>  - byte4 = Cr1
>  - and so on ...
> 
> According to PXA27x datasheet, table 27-19, input bytes from the sensor should
> be :
>  - byte1 = Cb1
>  - byte2 = Y1
>  - byte3 = Cr1
>  - byte4 = Y2
>  - and so on ...
> 
> Do you see the discrepancy now ? Either pxa27x datasheet of mt9m111 datasheet is
> wrong, or else Antonio's setup wouldn't work.

Yes, now I see it. Then we have to figure out which one is wrong... Or 
just decide as it best suits us. Would be interesting to know how the data 
is stored in RAM in the packed format - CbYCrY as is claimed in 27-21 or 
YCbYCr? Antonio, did you test in planar or in packed mode? Maybe you could 
test the packed mode and look at the data? One could also try to read data 
out of the FIFO, but that's too much work:-) I think, it would suffice to 
test in packed mode and use some standard application and see with what 
format it will produce the correct picture.

In fact, I think, it might even be, that both datasheets are correct and 
the testing was wrong. In packed mode pxa270 probably just pipes the data 
through one-to-one. So, if both datasheets are correct you would get UYUV 
in application buffers. Now, if you tell the application, that it's YUYV 
the picture will be wrong of course. And if you just swap the bytes _at_ 
_the_ _sensor_ the picture will be right, even though the pxa is only 
documented to work in UYUV mode. In packed mode it probably just doesn't 
care. Or am I missing something? Antonio, how did you test - in packed or 
planar mode and what format have you configured at the application level?

> > Good, then this is the fix that I'd like to have. It seems pretty simple, 
> > it will preserve behaviour of mt9m111. It will change the behaviour of 
> > pxa-camera for the YUYV format, to be precise, it will stop supporting 
> > this format. So, I would print out a warning, explaining that this format 
> > is not supported by pxa270 and the user should use UYVY instead. I 
> > suggested to add only one format to mt9m111 so far, just because this is 
> > the easiest as a bug-fix. But if you prefer, you can add all four, yes.
> Right, so be it for the 4 formats.
> 
> > Still, I would really prefer to see the version described above - add more 
> > formats to mt9m111 and fix pxa270 to claim the correct format and print a 
> > warning for YUYV. This shouldn't be much more difficult to make than the 
> > proposed patch, and it will be correct. I made enough bad experiences with 
> > "temporary fixes" to try to avoid them as much as possible:-)
> 
> If you wish so. I'll watch you pushing the fix in the stable branch to see how
> you convince people that changing the pxa camera driver is only a "fix" and not
> an evolution :) Too bad for a quick working fix.

Well, I am not at all sure we need it for 2.6.27-stable. This is not a 
regression.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] mt9m111: Fix YUYV format for pxa-camera
  2008-11-03 20:52           ` Guennadi Liakhovetski
@ 2008-11-03 22:26             ` Guennadi Liakhovetski
  2008-11-03 22:31               ` Robert Jarzmik
  0 siblings, 1 reply; 24+ messages in thread
From: Guennadi Liakhovetski @ 2008-11-03 22:26 UTC (permalink / raw)
  To: Robert Jarzmik; +Cc: video4linux-list

On Mon, 3 Nov 2008, Guennadi Liakhovetski wrote:

> In fact, I think, it might even be, that both datasheets are correct and 
> the testing was wrong. In packed mode pxa270 probably just pipes the data 
> through one-to-one. So, if both datasheets are correct you would get UYUV 
> in application buffers. Now, if you tell the application, that it's YUYV 
> the picture will be wrong of course. And if you just swap the bytes _at_ 
> _the_ _sensor_ the picture will be right, even though the pxa is only 
> documented to work in UYUV mode. In packed mode it probably just doesn't 
> care. Or am I missing something? Antonio, how did you test - in packed or 
> planar mode and what format have you configured at the application level?

Ok, just thinking one step further - Antonio most certainly was testing 
V4L2_PIX_FMT_YUYV, i.e., packed with his application, any other YCbCr 
format would be rejected by mt9m111 and YUYV _is_ packed. So, I think this 
is indeed the case - there are mo errors in datasheets, we just named the 
formats wrongly in pxa-camera and mt9m111 drivers.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] mt9m111: Fix YUYV format for pxa-camera
  2008-11-03 22:26             ` Guennadi Liakhovetski
@ 2008-11-03 22:31               ` Robert Jarzmik
  2008-11-04 14:42                 ` Guennadi Liakhovetski
  0 siblings, 1 reply; 24+ messages in thread
From: Robert Jarzmik @ 2008-11-03 22:31 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: video4linux-list

Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:

> Ok, just thinking one step further - Antonio most certainly was testing 
> V4L2_PIX_FMT_YUYV, i.e., packed with his application, any other YCbCr 
> format would be rejected by mt9m111 and YUYV _is_ packed. So, I think this 
> is indeed the case - there are mo errors in datasheets, we just named the 
> formats wrongly in pxa-camera and mt9m111 drivers.

I don't agree. This has nothing to do with naming, this has to do with byte
order on qif bus and out of mt9m111 sensor.

But you can change my mind : just tell me where my thinking was
wrong in the previous mail where I stated bytes order (out of mt9m111 and in pxa
qif bus).

--
Robert

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] mt9m111: Fix YUYV format for pxa-camera
  2008-11-03 22:31               ` Robert Jarzmik
@ 2008-11-04 14:42                 ` Guennadi Liakhovetski
  2008-11-04 17:21                   ` Antonio Ospite
  2008-11-04 21:57                   ` Robert Jarzmik
  0 siblings, 2 replies; 24+ messages in thread
From: Guennadi Liakhovetski @ 2008-11-04 14:42 UTC (permalink / raw)
  To: Robert Jarzmik; +Cc: video4linux-list

On Mon, 3 Nov 2008, Robert Jarzmik wrote:

> Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:
> 
> > Ok, just thinking one step further - Antonio most certainly was testing 
> > V4L2_PIX_FMT_YUYV, i.e., packed with his application, any other YCbCr 
> > format would be rejected by mt9m111 and YUYV _is_ packed. So, I think this 
> > is indeed the case - there are mo errors in datasheets, we just named the 
> > formats wrongly in pxa-camera and mt9m111 drivers.
> 
> I don't agree. This has nothing to do with naming, this has to do with byte
> order on qif bus and out of mt9m111 sensor.

We agree, that YCbCr _in_ _memory_ format as defined in pxa270 datasheet 
table 27-21 is UYVY, right?

To get that byte-order in memory data should appear on the camera bus as 
specified in Table 27-19. This order is also the default order for 
mt9m111. So, I think, it is reasonable to expect, that when a user 
application requests a UYVY format, we have to configure the camera to its 
defaults and the PXA will work as documented.

Instead, this configuration in the current mainline state is called YUYV, 
so, we provide data in UYUV format to an application, requesting YUYV. 
Then, of course, corrupted image result as in Antonio's test.

Hence, the first thing we shouldn't lie to applications - the format we 
currently provide is UYUV and this is how we should advertise it. That's 
why it _is_ a naming issue.

And, according to PXA documentation, pxa270 doesn't support any other 
byte-order variants on the camera bus, so, in principle one could stop 
here. Note, I think, this restriction is imposed to make image 
post-processing possible (see 7.4.9.2)

Next, what we observe, I think, is that in this mode pxa acts just in a 
pass-through mode with 16-bit pixels packing bytes as they arrive in the 
FIFO in RAM buffers. So, if we don't use post-processing, we can (ab)use 
this mode for other 16-bit YCbCr formats, e.g., YUYV. For this we leave 
PXA as it is, and just configure the sensor to provide YUYV. This is what 
essentially Antonio's patch does. In this sense it is "correct" - mt9m111 
is indeed configured for YUYV and it is the only YCbCr format it 
advertises, and pxa pretends to support YUYV. But, that's exactly why I am 
not quite happy about it - we abandon mt9m111's default UYUV format and 
switch it unconditionally to YUYV and we leave PXA270 lying about its 
supported pixel format. Instead, extending mt9m111 to claim support for 
all 4 formats, switching between them dynamically, and fixing pxa-camera 
to support all these four formats, and providing a comment, that we just 
use PXA270's UYUV as 16-bit pass-through, is a more complete fix and, 
probably, would have taken less time than this discussion:-)

> But you can change my mind : just tell me where my thinking was
> wrong in the previous mail where I stated bytes order (out of mt9m111 and in pxa
> qif bus).

Let's see if I managed...

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] mt9m111: Fix YUYV format for pxa-camera
  2008-11-04 14:42                 ` Guennadi Liakhovetski
@ 2008-11-04 17:21                   ` Antonio Ospite
  2008-11-04 21:57                   ` Robert Jarzmik
  1 sibling, 0 replies; 24+ messages in thread
From: Antonio Ospite @ 2008-11-04 17:21 UTC (permalink / raw)
  To: video4linux-list


[-- Attachment #1.1: Type: text/plain, Size: 3865 bytes --]

On Tue, 4 Nov 2008 15:42:59 +0100 (CET)
Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:

> On Mon, 3 Nov 2008, Robert Jarzmik wrote:
> 
> > Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:
> > 
> > > Ok, just thinking one step further - Antonio most certainly was testing 
> > > V4L2_PIX_FMT_YUYV, i.e., packed with his application, any other YCbCr 
> > > format would be rejected by mt9m111 and YUYV _is_ packed. So, I think this 
> > > is indeed the case - there are mo errors in datasheets, we just named the 
> > > formats wrongly in pxa-camera and mt9m111 drivers.
> > 
> > I don't agree. This has nothing to do with naming, this has to do with byte
> > order on qif bus and out of mt9m111 sensor.
>

Indeed I only played with packet formats.

> We agree, that YCbCr _in_ _memory_ format as defined in pxa270 datasheet 
> table 27-21 is UYVY, right?
> 
> To get that byte-order in memory data should appear on the camera bus as 
> specified in Table 27-19. This order is also the default order for 
> mt9m111. So, I think, it is reasonable to expect, that when a user 
> application requests a UYVY format, we have to configure the camera to its 
> defaults and the PXA will work as documented.
> 
> Instead, this configuration in the current mainline state is called YUYV, 
> so, we provide data in UYUV format to an application, requesting YUYV. 
> Then, of course, corrupted image result as in Antonio's test.
> 
> Hence, the first thing we shouldn't lie to applications - the format we 
> currently provide is UYUV and this is how we should advertise it. That's 
> why it _is_ a naming issue.
> 
> And, according to PXA documentation, pxa270 doesn't support any other 
> byte-order variants on the camera bus, so, in principle one could stop 
> here. Note, I think, this restriction is imposed to make image 
> post-processing possible (see 7.4.9.2)
> 
> Next, what we observe, I think, is that in this mode pxa acts just in a 
> pass-through mode with 16-bit pixels packing bytes as they arrive in the 
> FIFO in RAM buffers. So, if we don't use post-processing, we can (ab)use 
> this mode for other 16-bit YCbCr formats, e.g., YUYV. For this we leave 
> PXA as it is, and just configure the sensor to provide YUYV. This is what 
> essentially Antonio's patch does. In this sense it is "correct" - mt9m111 
> is indeed configured for YUYV and it is the only YCbCr format it 
> advertises, and pxa pretends to support YUYV. But, that's exactly why I am 
> not quite happy about it - we abandon mt9m111's default UYUV format and 
> switch it unconditionally to YUYV and we leave PXA270 lying about its 
> supported pixel format. Instead, extending mt9m111 to claim support for 
> all 4 formats, switching between them dynamically, and fixing pxa-camera 
> to support all these four formats, and providing a comment, that we just 
> use PXA270's UYUV as 16-bit pass-through, is a more complete fix and, 
> probably, would have taken less time than this discussion:-)
> 
> > But you can change my mind : just tell me where my thinking was
> > wrong in the previous mail where I stated bytes order (out of mt9m111 and in pxa
> > qif bus).
> 
> Let's see if I managed...
> 
> Thanks
> Guennadi

Very interesting discussion especially from a new learner point of view.
Of course I aimed at the correct end result not at the correct solution
as you stated.

I'll be very happy to promptly test the "complete" fix as soon as you
have something cooked.

Regards,
   Antonio Ospite

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

  Web site: http://www.studenti.unina.it/~ospite
Public key: http://www.studenti.unina.it/~ospite/aopubkey.asc

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 164 bytes --]

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] mt9m111: Fix YUYV format for pxa-camera
  2008-11-04 14:42                 ` Guennadi Liakhovetski
  2008-11-04 17:21                   ` Antonio Ospite
@ 2008-11-04 21:57                   ` Robert Jarzmik
  2008-11-04 21:59                     ` [PATCH] Add new pixel format VYUY 16 bits wide Robert Jarzmik
  1 sibling, 1 reply; 24+ messages in thread
From: Robert Jarzmik @ 2008-11-04 21:57 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: video4linux-list

Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:

> probably, would have taken less time than this discussion:-)
Right. Here follows my patches, in the next mails.

> Let's see if I managed...
Nope. But then, I don't want to waste anymore of both our time on it. Let's
proceed on patches, we'll see what the users will say.

--
Robert

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH] Add new pixel format VYUY 16 bits wide.
  2008-11-04 21:57                   ` Robert Jarzmik
@ 2008-11-04 21:59                     ` Robert Jarzmik
  2008-11-04 21:59                       ` [PATCH] mt9m111: add all yuv format combinations Robert Jarzmik
                                         ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Robert Jarzmik @ 2008-11-04 21:59 UTC (permalink / raw)
  To: g.liakhovetski; +Cc: video4linux-list

There were already 3 YUV formats defined :
 - YUYV
 - YVYU
 - UYVY
The only left combination is VYUY, which is added in this
patch.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 include/linux/videodev2.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 4669d7e..ec311d4 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -293,6 +293,7 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_YVU420  v4l2_fourcc('Y', 'V', '1', '2') /* 12  YVU 4:2:0     */
 #define V4L2_PIX_FMT_YUYV    v4l2_fourcc('Y', 'U', 'Y', 'V') /* 16  YUV 4:2:2     */
 #define V4L2_PIX_FMT_UYVY    v4l2_fourcc('U', 'Y', 'V', 'Y') /* 16  YUV 4:2:2     */
+#define V4L2_PIX_FMT_VYUY    v4l2_fourcc('V', 'Y', 'U', 'Y') /* 16  YUV 4:2:2     */
 #define V4L2_PIX_FMT_YUV422P v4l2_fourcc('4', '2', '2', 'P') /* 16  YVU422 planar */
 #define V4L2_PIX_FMT_YUV411P v4l2_fourcc('4', '1', '1', 'P') /* 16  YVU411 planar */
 #define V4L2_PIX_FMT_Y41P    v4l2_fourcc('Y', '4', '1', 'P') /* 12  YUV 4:1:1     */
-- 
1.5.6.5

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH] mt9m111: add all yuv format combinations.
  2008-11-04 21:59                     ` [PATCH] Add new pixel format VYUY 16 bits wide Robert Jarzmik
@ 2008-11-04 21:59                       ` Robert Jarzmik
  2008-11-04 22:40                         ` Guennadi Liakhovetski
  2008-12-01 13:59                         ` Guennadi Liakhovetski
  2008-11-04 22:43                       ` [PATCH] Add new pixel format VYUY 16 bits wide Guennadi Liakhovetski
  2008-11-05  7:19                       ` Hans Verkuil
  2 siblings, 2 replies; 24+ messages in thread
From: Robert Jarzmik @ 2008-11-04 21:59 UTC (permalink / raw)
  To: g.liakhovetski; +Cc: video4linux-list

The Micron mt9m111 offers 4 byte orders for YCbCr
output. This patchs adds all possible outputs capabilities
to the mt9m111 driver.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/media/video/mt9m111.c |   24 +++++++++++++++++++++++-
 1 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index da0b2d5..9b9b377 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -128,9 +128,14 @@
 	.colorspace = _colorspace }
 #define RGB_FMT(_name, _depth, _fourcc) \
 	COL_FMT(_name, _depth, _fourcc, V4L2_COLORSPACE_SRGB)
+#define JPG_FMT(_name, _depth, _fourcc) \
+	COL_FMT(_name, _depth, _fourcc, V4L2_COLORSPACE_JPEG)
 
 static const struct soc_camera_data_format mt9m111_colour_formats[] = {
-	COL_FMT("YCrYCb 8 bit", 8, V4L2_PIX_FMT_YUYV, V4L2_COLORSPACE_JPEG),
+	JPG_FMT("CbYCrY 16 bit", 16, V4L2_PIX_FMT_UYVY),
+	JPG_FMT("CrYCbY 16 bit", 16, V4L2_PIX_FMT_VYUY),
+	JPG_FMT("YCbYCr 16 bit", 16, V4L2_PIX_FMT_YUYV),
+	JPG_FMT("YCrYCb 16 bit", 16, V4L2_PIX_FMT_YVYU),
 	RGB_FMT("RGB 565", 16, V4L2_PIX_FMT_RGB565),
 	RGB_FMT("RGB 555", 16, V4L2_PIX_FMT_RGB555),
 	RGB_FMT("Bayer (sRGB) 10 bit", 10, V4L2_PIX_FMT_SBGGR16),
@@ -438,7 +443,24 @@ static int mt9m111_set_pixfmt(struct soc_camera_device *icd, u32 pixfmt)
 	case V4L2_PIX_FMT_RGB565:
 		ret = mt9m111_setfmt_rgb565(icd);
 		break;
+	case V4L2_PIX_FMT_UYVY:
+		mt9m111->swap_yuv_y_chromas = 0;
+		mt9m111->swap_yuv_cb_cr = 0;
+		ret = mt9m111_setfmt_yuv(icd);
+		break;
+	case V4L2_PIX_FMT_VYUY:
+		mt9m111->swap_yuv_y_chromas = 0;
+		mt9m111->swap_yuv_cb_cr = 1;
+		ret = mt9m111_setfmt_yuv(icd);
+		break;
 	case V4L2_PIX_FMT_YUYV:
+		mt9m111->swap_yuv_y_chromas = 1;
+		mt9m111->swap_yuv_cb_cr = 0;
+		ret = mt9m111_setfmt_yuv(icd);
+		break;
+	case V4L2_PIX_FMT_YVYU:
+		mt9m111->swap_yuv_y_chromas = 1;
+		mt9m111->swap_yuv_cb_cr = 1;
 		ret = mt9m111_setfmt_yuv(icd);
 		break;
 	default:
-- 
1.5.6.5

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH] mt9m111: add all yuv format combinations.
  2008-11-04 21:59                       ` [PATCH] mt9m111: add all yuv format combinations Robert Jarzmik
@ 2008-11-04 22:40                         ` Guennadi Liakhovetski
  2008-11-05 22:04                           ` Robert Jarzmik
  2008-12-01 13:59                         ` Guennadi Liakhovetski
  1 sibling, 1 reply; 24+ messages in thread
From: Guennadi Liakhovetski @ 2008-11-04 22:40 UTC (permalink / raw)
  To: Robert Jarzmik; +Cc: video4linux-list

On Tue, 4 Nov 2008, Robert Jarzmik wrote:

Re-added Antonio and Mike to cc.

> The Micron mt9m111 offers 4 byte orders for YCbCr
> output. This patchs adds all possible outputs capabilities
> to the mt9m111 driver.
> 
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>

Ok, yes, this is exactly what I wanted to see. Have you also tested it? Or 
do we have to ask Antonio to test it? Next time, when you send several 
patches of which some depend on others, please put them in a patch-series 
- if this patch is applied first mt9m111 will not compile. I will take 
them in the right order this time and I _think_ this should be enough to 
also guarantee, that they go upstream in the same order.

Would you also like to make the third patch - updating pxa-camera with the 
three further YCbCr formats and adding a comment, that although the docs 
only claim support for UYUV the others can be used too, as long as we 
don't use post-processing. Can you also test other formats?

Thanks
Guennadi

> ---
>  drivers/media/video/mt9m111.c |   24 +++++++++++++++++++++++-
>  1 files changed, 23 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
> index da0b2d5..9b9b377 100644
> --- a/drivers/media/video/mt9m111.c
> +++ b/drivers/media/video/mt9m111.c
> @@ -128,9 +128,14 @@
>  	.colorspace = _colorspace }
>  #define RGB_FMT(_name, _depth, _fourcc) \
>  	COL_FMT(_name, _depth, _fourcc, V4L2_COLORSPACE_SRGB)
> +#define JPG_FMT(_name, _depth, _fourcc) \
> +	COL_FMT(_name, _depth, _fourcc, V4L2_COLORSPACE_JPEG)
>  
>  static const struct soc_camera_data_format mt9m111_colour_formats[] = {
> -	COL_FMT("YCrYCb 8 bit", 8, V4L2_PIX_FMT_YUYV, V4L2_COLORSPACE_JPEG),
> +	JPG_FMT("CbYCrY 16 bit", 16, V4L2_PIX_FMT_UYVY),
> +	JPG_FMT("CrYCbY 16 bit", 16, V4L2_PIX_FMT_VYUY),
> +	JPG_FMT("YCbYCr 16 bit", 16, V4L2_PIX_FMT_YUYV),
> +	JPG_FMT("YCrYCb 16 bit", 16, V4L2_PIX_FMT_YVYU),
>  	RGB_FMT("RGB 565", 16, V4L2_PIX_FMT_RGB565),
>  	RGB_FMT("RGB 555", 16, V4L2_PIX_FMT_RGB555),
>  	RGB_FMT("Bayer (sRGB) 10 bit", 10, V4L2_PIX_FMT_SBGGR16),
> @@ -438,7 +443,24 @@ static int mt9m111_set_pixfmt(struct soc_camera_device *icd, u32 pixfmt)
>  	case V4L2_PIX_FMT_RGB565:
>  		ret = mt9m111_setfmt_rgb565(icd);
>  		break;
> +	case V4L2_PIX_FMT_UYVY:
> +		mt9m111->swap_yuv_y_chromas = 0;
> +		mt9m111->swap_yuv_cb_cr = 0;
> +		ret = mt9m111_setfmt_yuv(icd);
> +		break;
> +	case V4L2_PIX_FMT_VYUY:
> +		mt9m111->swap_yuv_y_chromas = 0;
> +		mt9m111->swap_yuv_cb_cr = 1;
> +		ret = mt9m111_setfmt_yuv(icd);
> +		break;
>  	case V4L2_PIX_FMT_YUYV:
> +		mt9m111->swap_yuv_y_chromas = 1;
> +		mt9m111->swap_yuv_cb_cr = 0;
> +		ret = mt9m111_setfmt_yuv(icd);
> +		break;
> +	case V4L2_PIX_FMT_YVYU:
> +		mt9m111->swap_yuv_y_chromas = 1;
> +		mt9m111->swap_yuv_cb_cr = 1;
>  		ret = mt9m111_setfmt_yuv(icd);
>  		break;
>  	default:
> -- 
> 1.5.6.5
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Add new pixel format VYUY 16 bits wide.
  2008-11-04 21:59                     ` [PATCH] Add new pixel format VYUY 16 bits wide Robert Jarzmik
  2008-11-04 21:59                       ` [PATCH] mt9m111: add all yuv format combinations Robert Jarzmik
@ 2008-11-04 22:43                       ` Guennadi Liakhovetski
  2008-11-05  7:19                       ` Hans Verkuil
  2 siblings, 0 replies; 24+ messages in thread
From: Guennadi Liakhovetski @ 2008-11-04 22:43 UTC (permalink / raw)
  To: Robert Jarzmik; +Cc: video4linux-list, Michael Schimek

On Tue, 4 Nov 2008, Robert Jarzmik wrote:

> There were already 3 YUV formats defined :
>  - YUYV
>  - YVYU
>  - UYVY
> The only left combination is VYUY, which is added in this
> patch.
> 
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>

We have to ask the fourcc maintainer: Michael? Would you accept this?

Thanks
Guennadi

> ---
>  include/linux/videodev2.h |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 4669d7e..ec311d4 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -293,6 +293,7 @@ struct v4l2_pix_format {
>  #define V4L2_PIX_FMT_YVU420  v4l2_fourcc('Y', 'V', '1', '2') /* 12  YVU 4:2:0     */
>  #define V4L2_PIX_FMT_YUYV    v4l2_fourcc('Y', 'U', 'Y', 'V') /* 16  YUV 4:2:2     */
>  #define V4L2_PIX_FMT_UYVY    v4l2_fourcc('U', 'Y', 'V', 'Y') /* 16  YUV 4:2:2     */
> +#define V4L2_PIX_FMT_VYUY    v4l2_fourcc('V', 'Y', 'U', 'Y') /* 16  YUV 4:2:2     */
>  #define V4L2_PIX_FMT_YUV422P v4l2_fourcc('4', '2', '2', 'P') /* 16  YVU422 planar */
>  #define V4L2_PIX_FMT_YUV411P v4l2_fourcc('4', '1', '1', 'P') /* 16  YVU411 planar */
>  #define V4L2_PIX_FMT_Y41P    v4l2_fourcc('Y', '4', '1', 'P') /* 12  YUV 4:1:1     */
> -- 
> 1.5.6.5
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Add new pixel format VYUY 16 bits wide.
  2008-11-04 21:59                     ` [PATCH] Add new pixel format VYUY 16 bits wide Robert Jarzmik
  2008-11-04 21:59                       ` [PATCH] mt9m111: add all yuv format combinations Robert Jarzmik
  2008-11-04 22:43                       ` [PATCH] Add new pixel format VYUY 16 bits wide Guennadi Liakhovetski
@ 2008-11-05  7:19                       ` Hans Verkuil
  2 siblings, 0 replies; 24+ messages in thread
From: Hans Verkuil @ 2008-11-05  7:19 UTC (permalink / raw)
  To: video4linux-list; +Cc: g.liakhovetski

On Tuesday 04 November 2008 22:59:37 Robert Jarzmik wrote:
> There were already 3 YUV formats defined :
>  - YUYV
>  - YVYU
>  - UYVY
> The only left combination is VYUY, which is added in this
> patch.
> 
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>

It's fine by me, but since you are making a change anyway, can you move the
V4L2_PIX_FMT_YVYU define up and put it after V4L2_PIX_FMT_YUYV? Then all
four combinations are together.

Reviewed-by: Hans Verkuil <hverkuil@xs4all.nl>

Regards,

	Hans

> ---
>  include/linux/videodev2.h |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 4669d7e..ec311d4 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -293,6 +293,7 @@ struct v4l2_pix_format {
>  #define V4L2_PIX_FMT_YVU420  v4l2_fourcc('Y', 'V', '1', '2') /* 12  YVU 4:2:0     */
>  #define V4L2_PIX_FMT_YUYV    v4l2_fourcc('Y', 'U', 'Y', 'V') /* 16  YUV 4:2:2     */
>  #define V4L2_PIX_FMT_UYVY    v4l2_fourcc('U', 'Y', 'V', 'Y') /* 16  YUV 4:2:2     */
> +#define V4L2_PIX_FMT_VYUY    v4l2_fourcc('V', 'Y', 'U', 'Y') /* 16  YUV 4:2:2     */
>  #define V4L2_PIX_FMT_YUV422P v4l2_fourcc('4', '2', '2', 'P') /* 16  YVU422 planar */
>  #define V4L2_PIX_FMT_YUV411P v4l2_fourcc('4', '1', '1', 'P') /* 16  YVU411 planar */
>  #define V4L2_PIX_FMT_Y41P    v4l2_fourcc('Y', '4', '1', 'P') /* 12  YUV 4:1:1     */
> -- 
> 1.5.6.5
> 
> --
> video4linux-list mailing list
> Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
> https://www.redhat.com/mailman/listinfo/video4linux-list
> 
> 


--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] mt9m111: add all yuv format combinations.
  2008-11-04 22:40                         ` Guennadi Liakhovetski
@ 2008-11-05 22:04                           ` Robert Jarzmik
  2008-11-05 22:37                             ` Guennadi Liakhovetski
  0 siblings, 1 reply; 24+ messages in thread
From: Robert Jarzmik @ 2008-11-05 22:04 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: video4linux-list

Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:

> Would you also like to make the third patch - updating pxa-camera with the 
> three further YCbCr formats and adding a comment, that although the docs 
> only claim support for UYUV the others can be used too, as long as we 
> don't use post-processing. Can you also test other formats?
Yes, give me a couple of days. Y*Y* and *Y*Y are tested already, I just want to
try the planar format, and I'll post.

--
Robert

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] mt9m111: add all yuv format combinations.
  2008-11-05 22:04                           ` Robert Jarzmik
@ 2008-11-05 22:37                             ` Guennadi Liakhovetski
  0 siblings, 0 replies; 24+ messages in thread
From: Guennadi Liakhovetski @ 2008-11-05 22:37 UTC (permalink / raw)
  To: Robert Jarzmik; +Cc: video4linux-list

On Wed, 5 Nov 2008, Robert Jarzmik wrote:

> Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:
> 
> > Would you also like to make the third patch - updating pxa-camera with the 
> > three further YCbCr formats and adding a comment, that although the docs 
> > only claim support for UYUV the others can be used too, as long as we 
> > don't use post-processing. Can you also test other formats?
> Yes, give me a couple of days. Y*Y* and *Y*Y are tested already, I just want to
> try the planar format, and I'll post.

Great, thanks! Unfortunately, all I have here to test is monochrome and 
Bayer.

Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] mt9m111: add all yuv format combinations.
  2008-11-04 21:59                       ` [PATCH] mt9m111: add all yuv format combinations Robert Jarzmik
  2008-11-04 22:40                         ` Guennadi Liakhovetski
@ 2008-12-01 13:59                         ` Guennadi Liakhovetski
  2008-12-01 18:30                           ` Robert Jarzmik
  1 sibling, 1 reply; 24+ messages in thread
From: Guennadi Liakhovetski @ 2008-12-01 13:59 UTC (permalink / raw)
  To: Robert Jarzmik; +Cc: video4linux-list

On Tue, 4 Nov 2008, Robert Jarzmik wrote:

> The Micron mt9m111 offers 4 byte orders for YCbCr
> output. This patchs adds all possible outputs capabilities
> to the mt9m111 driver.
> 
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>

Robert, could you please confirm, that this patch is still correct?

Thanks
Guennadi

> ---
>  drivers/media/video/mt9m111.c |   24 +++++++++++++++++++++++-
>  1 files changed, 23 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
> index da0b2d5..9b9b377 100644
> --- a/drivers/media/video/mt9m111.c
> +++ b/drivers/media/video/mt9m111.c
> @@ -128,9 +128,14 @@
>  	.colorspace = _colorspace }
>  #define RGB_FMT(_name, _depth, _fourcc) \
>  	COL_FMT(_name, _depth, _fourcc, V4L2_COLORSPACE_SRGB)
> +#define JPG_FMT(_name, _depth, _fourcc) \
> +	COL_FMT(_name, _depth, _fourcc, V4L2_COLORSPACE_JPEG)
>  
>  static const struct soc_camera_data_format mt9m111_colour_formats[] = {
> -	COL_FMT("YCrYCb 8 bit", 8, V4L2_PIX_FMT_YUYV, V4L2_COLORSPACE_JPEG),
> +	JPG_FMT("CbYCrY 16 bit", 16, V4L2_PIX_FMT_UYVY),
> +	JPG_FMT("CrYCbY 16 bit", 16, V4L2_PIX_FMT_VYUY),
> +	JPG_FMT("YCbYCr 16 bit", 16, V4L2_PIX_FMT_YUYV),
> +	JPG_FMT("YCrYCb 16 bit", 16, V4L2_PIX_FMT_YVYU),
>  	RGB_FMT("RGB 565", 16, V4L2_PIX_FMT_RGB565),
>  	RGB_FMT("RGB 555", 16, V4L2_PIX_FMT_RGB555),
>  	RGB_FMT("Bayer (sRGB) 10 bit", 10, V4L2_PIX_FMT_SBGGR16),
> @@ -438,7 +443,24 @@ static int mt9m111_set_pixfmt(struct soc_camera_device *icd, u32 pixfmt)
>  	case V4L2_PIX_FMT_RGB565:
>  		ret = mt9m111_setfmt_rgb565(icd);
>  		break;
> +	case V4L2_PIX_FMT_UYVY:
> +		mt9m111->swap_yuv_y_chromas = 0;
> +		mt9m111->swap_yuv_cb_cr = 0;
> +		ret = mt9m111_setfmt_yuv(icd);
> +		break;
> +	case V4L2_PIX_FMT_VYUY:
> +		mt9m111->swap_yuv_y_chromas = 0;
> +		mt9m111->swap_yuv_cb_cr = 1;
> +		ret = mt9m111_setfmt_yuv(icd);
> +		break;
>  	case V4L2_PIX_FMT_YUYV:
> +		mt9m111->swap_yuv_y_chromas = 1;
> +		mt9m111->swap_yuv_cb_cr = 0;
> +		ret = mt9m111_setfmt_yuv(icd);
> +		break;
> +	case V4L2_PIX_FMT_YVYU:
> +		mt9m111->swap_yuv_y_chromas = 1;
> +		mt9m111->swap_yuv_cb_cr = 1;
>  		ret = mt9m111_setfmt_yuv(icd);
>  		break;
>  	default:
> -- 
> 1.5.6.5
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] mt9m111: add all yuv format combinations.
  2008-12-01 13:59                         ` Guennadi Liakhovetski
@ 2008-12-01 18:30                           ` Robert Jarzmik
  0 siblings, 0 replies; 24+ messages in thread
From: Robert Jarzmik @ 2008-12-01 18:30 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: video4linux-list

Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:

> On Tue, 4 Nov 2008, Robert Jarzmik wrote:
>
>> The Micron mt9m111 offers 4 byte orders for YCbCr
>> output. This patchs adds all possible outputs capabilities
>> to the mt9m111 driver.
>> 
>> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
>
> Robert, could you please confirm, that this patch is still correct?
Yes, confirmed.

I'm using it for some time now, without modification.

--
Robert

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2008-12-01 18:31 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-10-29 22:25 [PATCH] mt9m111: Fix YUYV format for pxa-camera Antonio Ospite
2008-10-31 17:21 ` Robert Jarzmik
2008-10-31 17:40   ` Guennadi Liakhovetski
2008-10-31 18:04     ` Robert Jarzmik
2008-10-31 17:45   ` Antonio Ospite
2008-11-02 21:15   ` Guennadi Liakhovetski
2008-11-03 17:23     ` Robert Jarzmik
2008-11-03 19:09       ` Guennadi Liakhovetski
2008-11-03 20:19         ` Robert Jarzmik
2008-11-03 20:52           ` Guennadi Liakhovetski
2008-11-03 22:26             ` Guennadi Liakhovetski
2008-11-03 22:31               ` Robert Jarzmik
2008-11-04 14:42                 ` Guennadi Liakhovetski
2008-11-04 17:21                   ` Antonio Ospite
2008-11-04 21:57                   ` Robert Jarzmik
2008-11-04 21:59                     ` [PATCH] Add new pixel format VYUY 16 bits wide Robert Jarzmik
2008-11-04 21:59                       ` [PATCH] mt9m111: add all yuv format combinations Robert Jarzmik
2008-11-04 22:40                         ` Guennadi Liakhovetski
2008-11-05 22:04                           ` Robert Jarzmik
2008-11-05 22:37                             ` Guennadi Liakhovetski
2008-12-01 13:59                         ` Guennadi Liakhovetski
2008-12-01 18:30                           ` Robert Jarzmik
2008-11-04 22:43                       ` [PATCH] Add new pixel format VYUY 16 bits wide Guennadi Liakhovetski
2008-11-05  7:19                       ` Hans Verkuil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox