* [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: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 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: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] 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
* 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
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