| Message ID | 1338543105-20322-1-git-send-email-javier.martin@vista-silicon.com |
|---|---|
| State | New |
| Headers | show |
Hi, can this patch be applied please? It solves a BUG for 3.5. Guennadi, Fabio, could you give me an ack for this? Regards.
On 6 July 2012 13:00, javier Martin <javier.martin@vista-silicon.com> wrote: > Hi, > can this patch be applied please? > > It solves a BUG for 3.5. Guennadi, Fabio, could you give me an ack for this? > > Regards. But it should be applied after this one to preserve bisectability: http://patchwork.linuxtv.org/patch/10483/ So I'd better send a new series to clarify the order.
On Fri, 6 Jul 2012, javier Martin wrote: > Hi, > can this patch be applied please? > > It solves a BUG for 3.5. Guennadi, Fabio, could you give me an ack for this? Sorry? This patch has been applied and proven to break more, than it fixed, so, it has been reverted. Am I missing something? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
Hi Guennadi, On 6 July 2012 13:09, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > On Fri, 6 Jul 2012, javier Martin wrote: > >> Hi, >> can this patch be applied please? >> >> It solves a BUG for 3.5. Guennadi, Fabio, could you give me an ack for this? > > Sorry? This patch has been applied and proven to break more, than it > fixed, so, it has been reverted. Am I missing something? Patch v1 was the version that broke pass-through mode (which nobody seems to be using/testing). It was applied, then it was reverted as you requested in [1]. Then I sent v2 that didn't break pass-through but was invalid too because of a merge conflict [2]. Finally, this is v3 which has the pass-through problem and the merge problem fixed. It is currently marked as "Under review" and should be applied as a fix to 3.5. It can be applied safely since the patch I stated previously is already in 3.5-rc5 [4] (it was applied through the imx tree). [1] http://patchwork.linuxtv.org/patch/11504/ [2] http://patchwork.linuxtv.org/patch/11558/ [3] http://patchwork.linuxtv.org/patch/11559/ [4] http://patchwork.linuxtv.org/patch/10483/ -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com
On Fri, 6 Jul 2012, javier Martin wrote: > Hi Guennadi, > > On 6 July 2012 13:09, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > > On Fri, 6 Jul 2012, javier Martin wrote: > > > >> Hi, > >> can this patch be applied please? > >> > >> It solves a BUG for 3.5. Guennadi, Fabio, could you give me an ack for this? > > > > Sorry? This patch has been applied and proven to break more, than it > > fixed, so, it has been reverted. Am I missing something? > > Patch v1 was the version that broke pass-through mode (which nobody > seems to be using/testing). It was applied, then it was reverted as > you requested in [1]. > > Then I sent v2 that didn't break pass-through but was invalid too > because of a merge conflict [2]. > > Finally, this is v3 which has the pass-through problem and the merge > problem fixed. It is currently marked as "Under review" and should be > applied as a fix to 3.5. Ah, ok, then, don't you think, that expecting your patch to be applied within 4 minutes of its submission is a bit... overoptimistic? Because it's 4 minutes after your original patch, that you've sent your "reminder"... Thanks Guennadi > It can be applied safely since the patch I stated previously is > already in 3.5-rc5 [4] (it was applied through the imx tree). > > [1] http://patchwork.linuxtv.org/patch/11504/ > [2] http://patchwork.linuxtv.org/patch/11558/ > [3] http://patchwork.linuxtv.org/patch/11559/ > [4] http://patchwork.linuxtv.org/patch/10483/ > -- > Javier Martin > Vista Silicon S.L. > CDTUC - FASE C - Oficina S-345 > Avda de los Castros s/n > 39005- Santander. Cantabria. Spain > +34 942 25 32 60 > www.vista-silicon.com > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
Hi Guennadi, On 6 July 2012 13:39, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > On Fri, 6 Jul 2012, javier Martin wrote: > >> Hi Guennadi, >> >> On 6 July 2012 13:09, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: >> > On Fri, 6 Jul 2012, javier Martin wrote: >> > >> >> Hi, >> >> can this patch be applied please? >> >> >> >> It solves a BUG for 3.5. Guennadi, Fabio, could you give me an ack for this? >> > >> > Sorry? This patch has been applied and proven to break more, than it >> > fixed, so, it has been reverted. Am I missing something? >> >> Patch v1 was the version that broke pass-through mode (which nobody >> seems to be using/testing). It was applied, then it was reverted as >> you requested in [1]. >> >> Then I sent v2 that didn't break pass-through but was invalid too >> because of a merge conflict [2]. >> >> Finally, this is v3 which has the pass-through problem and the merge >> problem fixed. It is currently marked as "Under review" and should be >> applied as a fix to 3.5. > > Ah, ok, then, don't you think, that expecting your patch to be applied > within 4 minutes of its submission is a bit... overoptimistic? Because > it's 4 minutes after your original patch, that you've sent your > "reminder"... This patch was sent on '2012-06-01 09:31:45', which is more than a month ago. Look at patchwork: http://patchwork.linuxtv.org/patch/11559/ I think that a month is a reasonable period to send a reminder and I didn't mean to offend anyone with it. Regards.
On Fri, 6 Jul 2012, javier Martin wrote: > Hi Guennadi, > > On 6 July 2012 13:39, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > > On Fri, 6 Jul 2012, javier Martin wrote: > > > >> Hi Guennadi, > >> > >> On 6 July 2012 13:09, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > >> > On Fri, 6 Jul 2012, javier Martin wrote: > >> > > >> >> Hi, > >> >> can this patch be applied please? > >> >> > >> >> It solves a BUG for 3.5. Guennadi, Fabio, could you give me an ack for this? > >> > > >> > Sorry? This patch has been applied and proven to break more, than it > >> > fixed, so, it has been reverted. Am I missing something? > >> > >> Patch v1 was the version that broke pass-through mode (which nobody > >> seems to be using/testing). It was applied, then it was reverted as > >> you requested in [1]. > >> > >> Then I sent v2 that didn't break pass-through but was invalid too > >> because of a merge conflict [2]. > >> > >> Finally, this is v3 which has the pass-through problem and the merge > >> problem fixed. It is currently marked as "Under review" and should be > >> applied as a fix to 3.5. > > > > Ah, ok, then, don't you think, that expecting your patch to be applied > > within 4 minutes of its submission is a bit... overoptimistic? Because > > it's 4 minutes after your original patch, that you've sent your > > "reminder"... > > This patch was sent on '2012-06-01 09:31:45', which is more than a > month ago. Look at patchwork: > http://patchwork.linuxtv.org/patch/11559/ > > I think that a month is a reasonable period to send a reminder and I > didn't mean to offend anyone with it. Hrm, right, sorry. Must have been blind. I've looked at v3 of your patch, I've got one more question to it, expect a reply in a few minutes. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
Hi Javier Thanks for the patch, and sorry for delay. I was away first 10 days of June and still haven't come round to cleaning up my todo list since then... On Fri, 1 Jun 2012, Javier Martin wrote: > Remove MX2_CAMERA_SWAP16 and MX2_CAMERA_PACK_DIR_MSB flags > so that the driver can negotiate with the attached sensor > whether the mbus format needs convertion from UYUV to YUYV > or not. > > Signed-off-by: Javier Martin <javier.martin@vista-silicon.com> > --- > Fix pass-through mode as requested by Guennadi. > Also a merge conflict has been addressed. > > This patch should be applied to for_v3.5 since Guennadi > has requested Mauro to remove the old version: > > [PATCH] Revert "[media] media: mx2_camera: Fix mbus format handling" > > This patch is part of the following series: > > media: tvp5150: Fix mbus format. > i.MX27: visstrim_m10: Remove use of MX2_CAMERA_SWAP16. > media: mx2_camera: Fix mbus format handling. > --- > arch/arm/plat-mxc/include/mach/mx2_cam.h | 2 - > drivers/media/video/mx2_camera.c | 50 +++++++++++++++++++++++++++--- > 2 files changed, 45 insertions(+), 7 deletions(-) > > diff --git a/arch/arm/plat-mxc/include/mach/mx2_cam.h b/arch/arm/plat-mxc/include/mach/mx2_cam.h > index 3c080a3..7ded6f1 100644 > --- a/arch/arm/plat-mxc/include/mach/mx2_cam.h > +++ b/arch/arm/plat-mxc/include/mach/mx2_cam.h > @@ -23,7 +23,6 @@ > #ifndef __MACH_MX2_CAM_H_ > #define __MACH_MX2_CAM_H_ > > -#define MX2_CAMERA_SWAP16 (1 << 0) > #define MX2_CAMERA_EXT_VSYNC (1 << 1) > #define MX2_CAMERA_CCIR (1 << 2) > #define MX2_CAMERA_CCIR_INTERLACE (1 << 3) > @@ -31,7 +30,6 @@ > #define MX2_CAMERA_GATED_CLOCK (1 << 5) > #define MX2_CAMERA_INV_DATA (1 << 6) > #define MX2_CAMERA_PCLK_SAMPLE_RISING (1 << 7) > -#define MX2_CAMERA_PACK_DIR_MSB (1 << 8) > > /** > * struct mx2_camera_platform_data - optional platform data for mx2_camera > diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c > index 18afaee..b30ebe5 100644 > --- a/drivers/media/video/mx2_camera.c > +++ b/drivers/media/video/mx2_camera.c > @@ -344,6 +344,19 @@ static struct mx2_fmt_cfg mx27_emma_prp_table[] = { > PRP_INTR_CH2OVF, > } > }, > + { > + .in_fmt = V4L2_MBUS_FMT_UYVY8_2X8, > + .out_fmt = V4L2_PIX_FMT_YUV420, > + .cfg = { > + .channel = 2, > + .in_fmt = PRP_CNTL_DATA_IN_YUV422, > + .out_fmt = PRP_CNTL_CH2_OUT_YUV420, > + .src_pixel = 0x22000888, /* YUV422 (YUYV) */ > + .irq_flags = PRP_INTR_RDERR | PRP_INTR_CH2WERR | > + PRP_INTR_CH2FC | PRP_INTR_LBOVF | > + PRP_INTR_CH2OVF, > + } > + }, IIUC, this adds one more conversion from V4L2_MBUS_FMT_UYVY8_2X8 to V4L2_PIX_FMT_YUV420. > }; > > static struct mx2_fmt_cfg *mx27_emma_prp_get_format( > @@ -980,6 +993,8 @@ static int mx2_camera_set_bus_param(struct soc_camera_device *icd) > struct soc_camera_host *ici = to_soc_camera_host(icd->parent); > struct mx2_camera_dev *pcdev = ici->priv; > struct v4l2_mbus_config cfg = {.type = V4L2_MBUS_PARALLEL,}; > + const struct soc_camera_format_xlate *xlate; > + u32 pixfmt = icd->current_fmt->host_fmt->fourcc; > unsigned long common_flags; > int ret; > int bytesperline; > @@ -1024,14 +1039,28 @@ static int mx2_camera_set_bus_param(struct soc_camera_device *icd) > return ret; > } > > + xlate = soc_camera_xlate_by_fourcc(icd, pixfmt); > + if (!xlate) { > + dev_warn(icd->parent, "Format %x not found\n", pixfmt); > + return -EINVAL; > + } > + > + if (xlate->code == V4L2_MBUS_FMT_YUYV8_2X8) { > + csicr1 |= CSICR1_PACK_DIR; > + csicr1 &= ~CSICR1_SWAP16_EN; > + dev_dbg(icd->parent, "already yuyv format, don't convert\n"); > + } else if (xlate->code == V4L2_MBUS_FMT_UYVY8_2X8) { > + csicr1 &= ~CSICR1_PACK_DIR; > + csicr1 |= CSICR1_SWAP16_EN; > + dev_dbg(icd->parent, "convert uyvy mbus format into yuyv\n"); > + } > + > if (common_flags & V4L2_MBUS_PCLK_SAMPLE_RISING) > csicr1 |= CSICR1_REDGE; > if (common_flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH) > csicr1 |= CSICR1_SOF_POL; > if (common_flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH) > csicr1 |= CSICR1_HSYNC_POL; > - if (pcdev->platform_flags & MX2_CAMERA_SWAP16) > - csicr1 |= CSICR1_SWAP16_EN; > if (pcdev->platform_flags & MX2_CAMERA_EXT_VSYNC) > csicr1 |= CSICR1_EXT_VSYNC; > if (pcdev->platform_flags & MX2_CAMERA_CCIR) > @@ -1042,8 +1071,6 @@ static int mx2_camera_set_bus_param(struct soc_camera_device *icd) > csicr1 |= CSICR1_GCLK_MODE; > if (pcdev->platform_flags & MX2_CAMERA_INV_DATA) > csicr1 |= CSICR1_INV_DATA; > - if (pcdev->platform_flags & MX2_CAMERA_PACK_DIR_MSB) > - csicr1 |= CSICR1_PACK_DIR; > > pcdev->csicr1 = csicr1; > > @@ -1118,7 +1145,8 @@ static int mx2_camera_get_formats(struct soc_camera_device *icd, > return 0; > } > > - if (code == V4L2_MBUS_FMT_YUYV8_2X8) { > + if (code == V4L2_MBUS_FMT_YUYV8_2X8 || > + code == V4L2_MBUS_FMT_UYVY8_2X8) { This tells us, that from V4L2_MBUS_FMT_UYVY8_2X8 we also can get V4L2_PIX_FMT_YUV420 - as provided by the mbus_fmt[] table in soc_mediabus.c, this translation implements your above addition to the mx27_emma_prp_table[] table. > formats++; > if (xlate) { > /* > @@ -1134,6 +1162,18 @@ static int mx2_camera_get_formats(struct soc_camera_device *icd, > } > } > > + if (code == V4L2_MBUS_FMT_UYVY8_2X8) { > + formats++; > + if (xlate) { > + xlate->host_fmt = > + soc_mbus_get_fmtdesc(V4L2_MBUS_FMT_YUYV8_2X8); > + xlate->code = code; > + dev_dbg(dev, "Providing host format %s for sensor code %d\n", > + xlate->host_fmt->name, code); > + xlate++; > + } > + } This is telling us, that V4L2_MBUS_FMT_UYVY8_2X8 can also be converted to V4L2_PIX_FMT_YUYV. Since there is no explicit entry in mx27_emma_prp_table[] for this conversion, it will also be handled by the top 1-to-1 entry. > + > /* Generic pass-trough */ > formats++; > if (xlate) { And the pass-through adds a third conversion for V4L2_MBUS_FMT_UYVY8_2X8 - to V4L2_PIX_FMT_UYVY, which is served by the first generic 1-to-1 entry in mx27_emma_prp_table[]. So, maybe the above is correct, just wanted to make sure once more: is this really what you were trying to achieve? In case of the V4L2_MBUS_FMT_UYVY8_2X8 format you can produce 3 output formats, of which these two: V4L2_MBUS_FMT_YUYV8_2X8 and V4L2_PIX_FMT_UYVY are produced by the same pass-through entry of the mx27_emma_prp_table[] table. The difference between those two formats is only produced in mx2_camera_set_bus_param() in the way you set CSICR1 PACK_DIR and SWAP16_EN flags? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
hmm... sorry again. It is my fault, that I left this patch without attention for full 5 weeks, but I still don't have a sufficiently good feeling about it. Look here: On Fri, 6 Jul 2012, Guennadi Liakhovetski wrote: > Hi Javier > > Thanks for the patch, and sorry for delay. I was away first 10 days of > June and still haven't come round to cleaning up my todo list since > then... > > On Fri, 1 Jun 2012, Javier Martin wrote: [snip] > > @@ -1024,14 +1039,28 @@ static int mx2_camera_set_bus_param(struct soc_camera_device *icd) > > return ret; > > } > > > > + xlate = soc_camera_xlate_by_fourcc(icd, pixfmt); > > + if (!xlate) { > > + dev_warn(icd->parent, "Format %x not found\n", pixfmt); > > + return -EINVAL; > > + } > > + > > + if (xlate->code == V4L2_MBUS_FMT_YUYV8_2X8) { > > + csicr1 |= CSICR1_PACK_DIR; > > + csicr1 &= ~CSICR1_SWAP16_EN; > > + dev_dbg(icd->parent, "already yuyv format, don't convert\n"); > > + } else if (xlate->code == V4L2_MBUS_FMT_UYVY8_2X8) { > > + csicr1 &= ~CSICR1_PACK_DIR; > > + csicr1 |= CSICR1_SWAP16_EN; > > + dev_dbg(icd->parent, "convert uyvy mbus format into yuyv\n"); > > + } This doesn't look right. From V4L2_MBUS_FMT_YUYV8_2X8 you can produce two output formats: V4L2_PIX_FMT_YUV420 and V4L2_PIX_FMT_YUYV For both of them you set CSICR1_PACK_DIR, which wasn't the default before? Next for V4L2_MBUS_FMT_UYVY8_2X8. From this one you can produce 3 formats: V4L2_PIX_FMT_YUV420, V4L2_PIX_FMT_YUYV and V4L2_PIX_FMT_UYVY For all 3 of them you now set CSICR1_SWAP16_EN. Are you sure all the above is correct? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
On 6 July 2012 14:55, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > Hi Javier > > Thanks for the patch, and sorry for delay. I was away first 10 days of > June and still haven't come round to cleaning up my todo list since > then... > > On Fri, 1 Jun 2012, Javier Martin wrote: > >> Remove MX2_CAMERA_SWAP16 and MX2_CAMERA_PACK_DIR_MSB flags >> so that the driver can negotiate with the attached sensor >> whether the mbus format needs convertion from UYUV to YUYV >> or not. >> >> Signed-off-by: Javier Martin <javier.martin@vista-silicon.com> >> --- >> Fix pass-through mode as requested by Guennadi. >> Also a merge conflict has been addressed. >> >> This patch should be applied to for_v3.5 since Guennadi >> has requested Mauro to remove the old version: >> >> [PATCH] Revert "[media] media: mx2_camera: Fix mbus format handling" >> >> This patch is part of the following series: >> >> media: tvp5150: Fix mbus format. >> i.MX27: visstrim_m10: Remove use of MX2_CAMERA_SWAP16. >> media: mx2_camera: Fix mbus format handling. >> --- >> arch/arm/plat-mxc/include/mach/mx2_cam.h | 2 - >> drivers/media/video/mx2_camera.c | 50 +++++++++++++++++++++++++++--- >> 2 files changed, 45 insertions(+), 7 deletions(-) >> >> diff --git a/arch/arm/plat-mxc/include/mach/mx2_cam.h b/arch/arm/plat-mxc/include/mach/mx2_cam.h >> index 3c080a3..7ded6f1 100644 >> --- a/arch/arm/plat-mxc/include/mach/mx2_cam.h >> +++ b/arch/arm/plat-mxc/include/mach/mx2_cam.h >> @@ -23,7 +23,6 @@ >> #ifndef __MACH_MX2_CAM_H_ >> #define __MACH_MX2_CAM_H_ >> >> -#define MX2_CAMERA_SWAP16 (1 << 0) >> #define MX2_CAMERA_EXT_VSYNC (1 << 1) >> #define MX2_CAMERA_CCIR (1 << 2) >> #define MX2_CAMERA_CCIR_INTERLACE (1 << 3) >> @@ -31,7 +30,6 @@ >> #define MX2_CAMERA_GATED_CLOCK (1 << 5) >> #define MX2_CAMERA_INV_DATA (1 << 6) >> #define MX2_CAMERA_PCLK_SAMPLE_RISING (1 << 7) >> -#define MX2_CAMERA_PACK_DIR_MSB (1 << 8) >> >> /** >> * struct mx2_camera_platform_data - optional platform data for mx2_camera >> diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c >> index 18afaee..b30ebe5 100644 >> --- a/drivers/media/video/mx2_camera.c >> +++ b/drivers/media/video/mx2_camera.c >> @@ -344,6 +344,19 @@ static struct mx2_fmt_cfg mx27_emma_prp_table[] = { >> PRP_INTR_CH2OVF, >> } >> }, >> + { >> + .in_fmt = V4L2_MBUS_FMT_UYVY8_2X8, >> + .out_fmt = V4L2_PIX_FMT_YUV420, >> + .cfg = { >> + .channel = 2, >> + .in_fmt = PRP_CNTL_DATA_IN_YUV422, >> + .out_fmt = PRP_CNTL_CH2_OUT_YUV420, >> + .src_pixel = 0x22000888, /* YUV422 (YUYV) */ >> + .irq_flags = PRP_INTR_RDERR | PRP_INTR_CH2WERR | >> + PRP_INTR_CH2FC | PRP_INTR_LBOVF | >> + PRP_INTR_CH2OVF, >> + } >> + }, > > IIUC, this adds one more conversion from V4L2_MBUS_FMT_UYVY8_2X8 to > V4L2_PIX_FMT_YUV420. Yes, that's exactly what this does. >> }; >> >> static struct mx2_fmt_cfg *mx27_emma_prp_get_format( >> @@ -980,6 +993,8 @@ static int mx2_camera_set_bus_param(struct soc_camera_device *icd) >> struct soc_camera_host *ici = to_soc_camera_host(icd->parent); >> struct mx2_camera_dev *pcdev = ici->priv; >> struct v4l2_mbus_config cfg = {.type = V4L2_MBUS_PARALLEL,}; >> + const struct soc_camera_format_xlate *xlate; >> + u32 pixfmt = icd->current_fmt->host_fmt->fourcc; >> unsigned long common_flags; >> int ret; >> int bytesperline; >> @@ -1024,14 +1039,28 @@ static int mx2_camera_set_bus_param(struct soc_camera_device *icd) >> return ret; >> } >> >> + xlate = soc_camera_xlate_by_fourcc(icd, pixfmt); >> + if (!xlate) { >> + dev_warn(icd->parent, "Format %x not found\n", pixfmt); >> + return -EINVAL; >> + } >> + >> + if (xlate->code == V4L2_MBUS_FMT_YUYV8_2X8) { >> + csicr1 |= CSICR1_PACK_DIR; >> + csicr1 &= ~CSICR1_SWAP16_EN; >> + dev_dbg(icd->parent, "already yuyv format, don't convert\n"); >> + } else if (xlate->code == V4L2_MBUS_FMT_UYVY8_2X8) { >> + csicr1 &= ~CSICR1_PACK_DIR; >> + csicr1 |= CSICR1_SWAP16_EN; >> + dev_dbg(icd->parent, "convert uyvy mbus format into yuyv\n"); >> + } >> + >> if (common_flags & V4L2_MBUS_PCLK_SAMPLE_RISING) >> csicr1 |= CSICR1_REDGE; >> if (common_flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH) >> csicr1 |= CSICR1_SOF_POL; >> if (common_flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH) >> csicr1 |= CSICR1_HSYNC_POL; >> - if (pcdev->platform_flags & MX2_CAMERA_SWAP16) >> - csicr1 |= CSICR1_SWAP16_EN; >> if (pcdev->platform_flags & MX2_CAMERA_EXT_VSYNC) >> csicr1 |= CSICR1_EXT_VSYNC; >> if (pcdev->platform_flags & MX2_CAMERA_CCIR) >> @@ -1042,8 +1071,6 @@ static int mx2_camera_set_bus_param(struct soc_camera_device *icd) >> csicr1 |= CSICR1_GCLK_MODE; >> if (pcdev->platform_flags & MX2_CAMERA_INV_DATA) >> csicr1 |= CSICR1_INV_DATA; >> - if (pcdev->platform_flags & MX2_CAMERA_PACK_DIR_MSB) >> - csicr1 |= CSICR1_PACK_DIR; >> >> pcdev->csicr1 = csicr1; >> >> @@ -1118,7 +1145,8 @@ static int mx2_camera_get_formats(struct soc_camera_device *icd, >> return 0; >> } >> >> - if (code == V4L2_MBUS_FMT_YUYV8_2X8) { >> + if (code == V4L2_MBUS_FMT_YUYV8_2X8 || >> + code == V4L2_MBUS_FMT_UYVY8_2X8) { > > This tells us, that from V4L2_MBUS_FMT_UYVY8_2X8 we also can get > V4L2_PIX_FMT_YUV420 - as provided by the mbus_fmt[] table in > soc_mediabus.c, this translation implements your above addition to the > mx27_emma_prp_table[] table. You are right. >> formats++; >> if (xlate) { >> /* >> @@ -1134,6 +1162,18 @@ static int mx2_camera_get_formats(struct soc_camera_device *icd, >> } >> } >> >> + if (code == V4L2_MBUS_FMT_UYVY8_2X8) { >> + formats++; >> + if (xlate) { >> + xlate->host_fmt = >> + soc_mbus_get_fmtdesc(V4L2_MBUS_FMT_YUYV8_2X8); >> + xlate->code = code; >> + dev_dbg(dev, "Providing host format %s for sensor code %d\n", >> + xlate->host_fmt->name, code); >> + xlate++; >> + } >> + } > > This is telling us, that V4L2_MBUS_FMT_UYVY8_2X8 can also be converted to > V4L2_PIX_FMT_YUYV. Since there is no explicit entry in > mx27_emma_prp_table[] for this conversion, it will also be handled by the > top 1-to-1 entry. Correct. >> + >> /* Generic pass-trough */ >> formats++; >> if (xlate) { > > And the pass-through adds a third conversion for V4L2_MBUS_FMT_UYVY8_2X8 - > to V4L2_PIX_FMT_UYVY, which is served by the first generic 1-to-1 entry in > mx27_emma_prp_table[]. With pass-through you can always get at the output the same format as at the input. > So, maybe the above is correct, just wanted to make sure once more: is > this really what you were trying to achieve? In case of the > V4L2_MBUS_FMT_UYVY8_2X8 format you can produce 3 output formats, of which > these two: > > V4L2_MBUS_FMT_YUYV8_2X8 and > V4L2_PIX_FMT_UYVY > > are produced by the same pass-through entry of the mx27_emma_prp_table[] > table. The difference between those two formats is only produced in > mx2_camera_set_bus_param() in the way you set CSICR1 PACK_DIR and > SWAP16_EN flags? Yes.
On 6 July 2012 17:11, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > hmm... sorry again. It is my fault, that I left this patch without > attention for full 5 weeks, but I still don't have a sufficiently good > feeling about it. Look here: > > On Fri, 6 Jul 2012, Guennadi Liakhovetski wrote: > >> Hi Javier >> >> Thanks for the patch, and sorry for delay. I was away first 10 days of >> June and still haven't come round to cleaning up my todo list since >> then... >> >> On Fri, 1 Jun 2012, Javier Martin wrote: > > [snip] > >> > @@ -1024,14 +1039,28 @@ static int mx2_camera_set_bus_param(struct soc_camera_device *icd) >> > return ret; >> > } >> > >> > + xlate = soc_camera_xlate_by_fourcc(icd, pixfmt); >> > + if (!xlate) { >> > + dev_warn(icd->parent, "Format %x not found\n", pixfmt); >> > + return -EINVAL; >> > + } >> > + >> > + if (xlate->code == V4L2_MBUS_FMT_YUYV8_2X8) { >> > + csicr1 |= CSICR1_PACK_DIR; >> > + csicr1 &= ~CSICR1_SWAP16_EN; >> > + dev_dbg(icd->parent, "already yuyv format, don't convert\n"); >> > + } else if (xlate->code == V4L2_MBUS_FMT_UYVY8_2X8) { >> > + csicr1 &= ~CSICR1_PACK_DIR; >> > + csicr1 |= CSICR1_SWAP16_EN; >> > + dev_dbg(icd->parent, "convert uyvy mbus format into yuyv\n"); >> > + } > > This doesn't look right. From V4L2_MBUS_FMT_YUYV8_2X8 you can produce two > output formats: > > V4L2_PIX_FMT_YUV420 and > V4L2_PIX_FMT_YUYV > > For both of them you set CSICR1_PACK_DIR, which wasn't the default before? > Next for V4L2_MBUS_FMT_UYVY8_2X8. From this one you can produce 3 formats: > > V4L2_PIX_FMT_YUV420, > V4L2_PIX_FMT_YUYV and > V4L2_PIX_FMT_UYVY > > For all 3 of them you now set CSICR1_SWAP16_EN. Are you sure all the above > is correct? No, there's just one thing wrong. With this patch, pass-through mode for V4L2_MBUS_FMT_UYVY8_2X8 won't work, since I always convert it to YUYV. Let me send a new version of the patch to address this problem. Regards.
diff --git a/arch/arm/plat-mxc/include/mach/mx2_cam.h b/arch/arm/plat-mxc/include/mach/mx2_cam.h index 3c080a3..7ded6f1 100644 --- a/arch/arm/plat-mxc/include/mach/mx2_cam.h +++ b/arch/arm/plat-mxc/include/mach/mx2_cam.h @@ -23,7 +23,6 @@ #ifndef __MACH_MX2_CAM_H_ #define __MACH_MX2_CAM_H_ -#define MX2_CAMERA_SWAP16 (1 << 0) #define MX2_CAMERA_EXT_VSYNC (1 << 1) #define MX2_CAMERA_CCIR (1 << 2) #define MX2_CAMERA_CCIR_INTERLACE (1 << 3) @@ -31,7 +30,6 @@ #define MX2_CAMERA_GATED_CLOCK (1 << 5) #define MX2_CAMERA_INV_DATA (1 << 6) #define MX2_CAMERA_PCLK_SAMPLE_RISING (1 << 7) -#define MX2_CAMERA_PACK_DIR_MSB (1 << 8) /** * struct mx2_camera_platform_data - optional platform data for mx2_camera diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c index 18afaee..b30ebe5 100644 --- a/drivers/media/video/mx2_camera.c +++ b/drivers/media/video/mx2_camera.c @@ -344,6 +344,19 @@ static struct mx2_fmt_cfg mx27_emma_prp_table[] = { PRP_INTR_CH2OVF, } }, + { + .in_fmt = V4L2_MBUS_FMT_UYVY8_2X8, + .out_fmt = V4L2_PIX_FMT_YUV420, + .cfg = { + .channel = 2, + .in_fmt = PRP_CNTL_DATA_IN_YUV422, + .out_fmt = PRP_CNTL_CH2_OUT_YUV420, + .src_pixel = 0x22000888, /* YUV422 (YUYV) */ + .irq_flags = PRP_INTR_RDERR | PRP_INTR_CH2WERR | + PRP_INTR_CH2FC | PRP_INTR_LBOVF | + PRP_INTR_CH2OVF, + } + }, }; static struct mx2_fmt_cfg *mx27_emma_prp_get_format( @@ -980,6 +993,8 @@ static int mx2_camera_set_bus_param(struct soc_camera_device *icd) struct soc_camera_host *ici = to_soc_camera_host(icd->parent); struct mx2_camera_dev *pcdev = ici->priv; struct v4l2_mbus_config cfg = {.type = V4L2_MBUS_PARALLEL,}; + const struct soc_camera_format_xlate *xlate; + u32 pixfmt = icd->current_fmt->host_fmt->fourcc; unsigned long common_flags; int ret; int bytesperline; @@ -1024,14 +1039,28 @@ static int mx2_camera_set_bus_param(struct soc_camera_device *icd) return ret; } + xlate = soc_camera_xlate_by_fourcc(icd, pixfmt); + if (!xlate) { + dev_warn(icd->parent, "Format %x not found\n", pixfmt); + return -EINVAL; + } + + if (xlate->code == V4L2_MBUS_FMT_YUYV8_2X8) { + csicr1 |= CSICR1_PACK_DIR; + csicr1 &= ~CSICR1_SWAP16_EN; + dev_dbg(icd->parent, "already yuyv format, don't convert\n"); + } else if (xlate->code == V4L2_MBUS_FMT_UYVY8_2X8) { + csicr1 &= ~CSICR1_PACK_DIR; + csicr1 |= CSICR1_SWAP16_EN; + dev_dbg(icd->parent, "convert uyvy mbus format into yuyv\n"); + } + if (common_flags & V4L2_MBUS_PCLK_SAMPLE_RISING) csicr1 |= CSICR1_REDGE; if (common_flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH) csicr1 |= CSICR1_SOF_POL; if (common_flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH) csicr1 |= CSICR1_HSYNC_POL; - if (pcdev->platform_flags & MX2_CAMERA_SWAP16) - csicr1 |= CSICR1_SWAP16_EN; if (pcdev->platform_flags & MX2_CAMERA_EXT_VSYNC) csicr1 |= CSICR1_EXT_VSYNC; if (pcdev->platform_flags & MX2_CAMERA_CCIR) @@ -1042,8 +1071,6 @@ static int mx2_camera_set_bus_param(struct soc_camera_device *icd) csicr1 |= CSICR1_GCLK_MODE; if (pcdev->platform_flags & MX2_CAMERA_INV_DATA) csicr1 |= CSICR1_INV_DATA; - if (pcdev->platform_flags & MX2_CAMERA_PACK_DIR_MSB) - csicr1 |= CSICR1_PACK_DIR; pcdev->csicr1 = csicr1; @@ -1118,7 +1145,8 @@ static int mx2_camera_get_formats(struct soc_camera_device *icd, return 0; } - if (code == V4L2_MBUS_FMT_YUYV8_2X8) { + if (code == V4L2_MBUS_FMT_YUYV8_2X8 || + code == V4L2_MBUS_FMT_UYVY8_2X8) { formats++; if (xlate) { /* @@ -1134,6 +1162,18 @@ static int mx2_camera_get_formats(struct soc_camera_device *icd, } } + if (code == V4L2_MBUS_FMT_UYVY8_2X8) { + formats++; + if (xlate) { + xlate->host_fmt = + soc_mbus_get_fmtdesc(V4L2_MBUS_FMT_YUYV8_2X8); + xlate->code = code; + dev_dbg(dev, "Providing host format %s for sensor code %d\n", + xlate->host_fmt->name, code); + xlate++; + } + } + /* Generic pass-trough */ formats++; if (xlate) {
Remove MX2_CAMERA_SWAP16 and MX2_CAMERA_PACK_DIR_MSB flags so that the driver can negotiate with the attached sensor whether the mbus format needs convertion from UYUV to YUYV or not. Signed-off-by: Javier Martin <javier.martin@vista-silicon.com> --- Fix pass-through mode as requested by Guennadi. Also a merge conflict has been addressed. This patch should be applied to for_v3.5 since Guennadi has requested Mauro to remove the old version: [PATCH] Revert "[media] media: mx2_camera: Fix mbus format handling" This patch is part of the following series: media: tvp5150: Fix mbus format. i.MX27: visstrim_m10: Remove use of MX2_CAMERA_SWAP16. media: mx2_camera: Fix mbus format handling. --- arch/arm/plat-mxc/include/mach/mx2_cam.h | 2 - drivers/media/video/mx2_camera.c | 50 +++++++++++++++++++++++++++--- 2 files changed, 45 insertions(+), 7 deletions(-)