[v3,for_v3.5] media: mx2_camera: Fix mbus format handling

  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(-)
  

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/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

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/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

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/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

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/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

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/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

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.