[PATCH] media: i.MX27 camera: fix picture source width

[PATCH] media: i.MX27 camera: fix picture source width

[Christoph Fritz]

While using a mt9m001 (monochrome) camera the final output falsely gets
horizontally divided into two pictures.

The issue was git bisected to commit f410991dcf1f

  |  [media] i.MX27 camera: add support for YUV420 format
  |
  |  This patch uses channel 2 of the eMMa-PrP to convert
  |  format provided by the sensor to YUV420.
  |
  |  This format is very useful since it is used by the
  |  internal H.264 encoder.

It sets PICTURE_X_SIZE in register PRP_SRC_FRAME_SIZE to its full width
while before that commit it was divided by two:

-   writel(((bytesperline >> 1) << 16) | icd->user_height,
+           writel((icd->user_width << 16) | icd->user_height,
                    pcdev->base_emma + PRP_SRC_FRAME_SIZE);

i.mx27 reference manual (41.6.12 PrP Source Frame Size Register) says:

    PICTURE_X_SIZE. These bits set the frame width to be
    processed in number of pixels. In YUV 4:2:0 mode, Cb and
    Cr widths are taken as PICTURE_X_SIZE/2 pixels.  In YUV
    4:2:0 mode, this value should be a multiple of 8-pixels.
    In other modes (RGB, YUV 4:2:2 and YUV 4:4:4) it should
    be a multiple of 4 pixels.

This patch reverts to PICTURE_X_SIZE/2 for channel 1.

Tested on Kernel 3.4, merged to 3.8rc.

Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
---
 drivers/media/platform/soc_camera/mx2_camera.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/soc_camera/mx2_camera.c b/drivers/media/platform/soc_camera/mx2_camera.c
index 8bda2c9..795bd3f 100644
--- a/drivers/media/platform/soc_camera/mx2_camera.c
+++ b/drivers/media/platform/soc_camera/mx2_camera.c
@@ -778,11 +778,11 @@ static void mx27_camera_emma_buf_init(struct soc_camera_device *icd,
 	struct mx2_camera_dev *pcdev = ici->priv;
 	struct mx2_fmt_cfg *prp = pcdev->emma_prp;
 
-	writel((pcdev->s_width << 16) | pcdev->s_height,
-	       pcdev->base_emma + PRP_SRC_FRAME_SIZE);
 	writel(prp->cfg.src_pixel,
 	       pcdev->base_emma + PRP_SRC_PIXEL_FORMAT_CNTL);
 	if (prp->cfg.channel == 1) {
+		writel(((bytesperline >> 1) << 16) | pcdev->s_height,
+			pcdev->base_emma + PRP_SRC_FRAME_SIZE);
 		writel((icd->user_width << 16) | icd->user_height,
 			pcdev->base_emma + PRP_CH1_OUT_IMAGE_SIZE);
 		writel(bytesperline,
@@ -790,6 +790,8 @@ static void mx27_camera_emma_buf_init(struct soc_camera_device *icd,
 		writel(prp->cfg.ch1_pixel,
 			pcdev->base_emma + PRP_CH1_PIXEL_FORMAT_CNTL);
 	} else { /* channel 2 */
+		writel((pcdev->s_width << 16) | pcdev->s_height,
+			pcdev->base_emma + PRP_SRC_FRAME_SIZE);
 		writel((icd->user_width << 16) | icd->user_height,
 			pcdev->base_emma + PRP_CH2_OUT_IMAGE_SIZE);
 	}
-- 
1.7.10.4

Re: [PATCH] media: i.MX27 camera: fix picture source width

[Greg KH]

On Fri, Feb 15, 2013 at 06:08:41PM +0100, Christoph Fritz wrote:
> While using a mt9m001 (monochrome) camera the final output falsely gets
> horizontally divided into two pictures.
> 
> The issue was git bisected to commit f410991dcf1f
> 
>   |  [media] i.MX27 camera: add support for YUV420 format
>   |
>   |  This patch uses channel 2 of the eMMa-PrP to convert
>   |  format provided by the sensor to YUV420.
>   |
>   |  This format is very useful since it is used by the
>   |  internal H.264 encoder.
> 
> It sets PICTURE_X_SIZE in register PRP_SRC_FRAME_SIZE to its full width
> while before that commit it was divided by two:
> 
> -   writel(((bytesperline >> 1) << 16) | icd->user_height,
> +           writel((icd->user_width << 16) | icd->user_height,
>                     pcdev->base_emma + PRP_SRC_FRAME_SIZE);
> 
> i.mx27 reference manual (41.6.12 PrP Source Frame Size Register) says:
> 
>     PICTURE_X_SIZE. These bits set the frame width to be
>     processed in number of pixels. In YUV 4:2:0 mode, Cb and
>     Cr widths are taken as PICTURE_X_SIZE/2 pixels.  In YUV
>     4:2:0 mode, this value should be a multiple of 8-pixels.
>     In other modes (RGB, YUV 4:2:2 and YUV 4:4:4) it should
>     be a multiple of 4 pixels.
> 
> This patch reverts to PICTURE_X_SIZE/2 for channel 1.
> 
> Tested on Kernel 3.4, merged to 3.8rc.
> 
> Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
> ---
>  drivers/media/platform/soc_camera/mx2_camera.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.

</formletter>

[PATCH v2] media: i.MX27 camera: fix picture source width

[Christoph Fritz]

While using a mt9m001 (monochrome) camera the final output falsely gets
horizontally divided into two pictures.

The issue was git bisected to commit f410991dcf1f

  |  [media] i.MX27 camera: add support for YUV420 format
  |
  |  This patch uses channel 2 of the eMMa-PrP to convert
  |  format provided by the sensor to YUV420.
  |
  |  This format is very useful since it is used by the
  |  internal H.264 encoder.

It sets PICTURE_X_SIZE in register PRP_SRC_FRAME_SIZE to its full width
while before that commit it was divided by two:

-   writel(((bytesperline >> 1) << 16) | icd->user_height,
+           writel((icd->user_width << 16) | icd->user_height,
                    pcdev->base_emma + PRP_SRC_FRAME_SIZE);

i.mx27 reference manual (41.6.12 PrP Source Frame Size Register) says:

    PICTURE_X_SIZE. These bits set the frame width to be
    processed in number of pixels. In YUV 4:2:0 mode, Cb and
    Cr widths are taken as PICTURE_X_SIZE/2 pixels.  In YUV
    4:2:0 mode, this value should be a multiple of 8-pixels.
    In other modes (RGB, YUV 4:2:2 and YUV 4:4:4) it should
    be a multiple of 4 pixels.

This patch reverts to PICTURE_X_SIZE/2 for channel 1.

Tested on Kernel 3.4, merged to 3.8rc.

Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
---
 drivers/media/platform/soc_camera/mx2_camera.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/soc_camera/mx2_camera.c b/drivers/media/platform/soc_camera/mx2_camera.c
index 8bda2c9..795bd3f 100644
--- a/drivers/media/platform/soc_camera/mx2_camera.c
+++ b/drivers/media/platform/soc_camera/mx2_camera.c
@@ -778,11 +778,11 @@ static void mx27_camera_emma_buf_init(struct soc_camera_device *icd,
 	struct mx2_camera_dev *pcdev = ici->priv;
 	struct mx2_fmt_cfg *prp = pcdev->emma_prp;
 
-	writel((pcdev->s_width << 16) | pcdev->s_height,
-	       pcdev->base_emma + PRP_SRC_FRAME_SIZE);
 	writel(prp->cfg.src_pixel,
 	       pcdev->base_emma + PRP_SRC_PIXEL_FORMAT_CNTL);
 	if (prp->cfg.channel == 1) {
+		writel(((bytesperline >> 1) << 16) | pcdev->s_height,
+			pcdev->base_emma + PRP_SRC_FRAME_SIZE);
 		writel((icd->user_width << 16) | icd->user_height,
 			pcdev->base_emma + PRP_CH1_OUT_IMAGE_SIZE);
 		writel(bytesperline,
@@ -790,6 +790,8 @@ static void mx27_camera_emma_buf_init(struct soc_camera_device *icd,
 		writel(prp->cfg.ch1_pixel,
 			pcdev->base_emma + PRP_CH1_PIXEL_FORMAT_CNTL);
 	} else { /* channel 2 */
+		writel((pcdev->s_width << 16) | pcdev->s_height,
+			pcdev->base_emma + PRP_SRC_FRAME_SIZE);
 		writel((icd->user_width << 16) | icd->user_height,
 			pcdev->base_emma + PRP_CH2_OUT_IMAGE_SIZE);
 	}
-- 
1.7.10.4

Re: [PATCH v2] media: i.MX27 camera: fix picture source width

[Guennadi Liakhovetski]

(Javier's opinion requested)

I'm no expert in i.MX27 hardware, would be great to have an ack from 
someone, intensively working in this area. Javier, what do you think? Is 
this really correct always for channel 1, or should this calculation 
depend on the pixel format?

Thanks
Guennadi

On Sat, 16 Feb 2013, Christoph Fritz wrote:

> While using a mt9m001 (monochrome) camera the final output falsely gets
> horizontally divided into two pictures.
> 
> The issue was git bisected to commit f410991dcf1f
> 
>   |  [media] i.MX27 camera: add support for YUV420 format
>   |
>   |  This patch uses channel 2 of the eMMa-PrP to convert
>   |  format provided by the sensor to YUV420.
>   |
>   |  This format is very useful since it is used by the
>   |  internal H.264 encoder.
> 
> It sets PICTURE_X_SIZE in register PRP_SRC_FRAME_SIZE to its full width
> while before that commit it was divided by two:
> 
> -   writel(((bytesperline >> 1) << 16) | icd->user_height,
> +           writel((icd->user_width << 16) | icd->user_height,
>                     pcdev->base_emma + PRP_SRC_FRAME_SIZE);
> 
> i.mx27 reference manual (41.6.12 PrP Source Frame Size Register) says:
> 
>     PICTURE_X_SIZE. These bits set the frame width to be
>     processed in number of pixels. In YUV 4:2:0 mode, Cb and
>     Cr widths are taken as PICTURE_X_SIZE/2 pixels.  In YUV
>     4:2:0 mode, this value should be a multiple of 8-pixels.
>     In other modes (RGB, YUV 4:2:2 and YUV 4:4:4) it should
>     be a multiple of 4 pixels.
> 
> This patch reverts to PICTURE_X_SIZE/2 for channel 1.
> 
> Tested on Kernel 3.4, merged to 3.8rc.
> 
> Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
> ---
>  drivers/media/platform/soc_camera/mx2_camera.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/soc_camera/mx2_camera.c b/drivers/media/platform/soc_camera/mx2_camera.c
> index 8bda2c9..795bd3f 100644
> --- a/drivers/media/platform/soc_camera/mx2_camera.c
> +++ b/drivers/media/platform/soc_camera/mx2_camera.c
> @@ -778,11 +778,11 @@ static void mx27_camera_emma_buf_init(struct soc_camera_device *icd,
>  	struct mx2_camera_dev *pcdev = ici->priv;
>  	struct mx2_fmt_cfg *prp = pcdev->emma_prp;
>  
> -	writel((pcdev->s_width << 16) | pcdev->s_height,
> -	       pcdev->base_emma + PRP_SRC_FRAME_SIZE);
>  	writel(prp->cfg.src_pixel,
>  	       pcdev->base_emma + PRP_SRC_PIXEL_FORMAT_CNTL);
>  	if (prp->cfg.channel == 1) {
> +		writel(((bytesperline >> 1) << 16) | pcdev->s_height,
> +			pcdev->base_emma + PRP_SRC_FRAME_SIZE);
>  		writel((icd->user_width << 16) | icd->user_height,
>  			pcdev->base_emma + PRP_CH1_OUT_IMAGE_SIZE);
>  		writel(bytesperline,
> @@ -790,6 +790,8 @@ static void mx27_camera_emma_buf_init(struct soc_camera_device *icd,
>  		writel(prp->cfg.ch1_pixel,
>  			pcdev->base_emma + PRP_CH1_PIXEL_FORMAT_CNTL);
>  	} else { /* channel 2 */
> +		writel((pcdev->s_width << 16) | pcdev->s_height,
> +			pcdev->base_emma + PRP_SRC_FRAME_SIZE);
>  		writel((icd->user_width << 16) | icd->user_height,
>  			pcdev->base_emma + PRP_CH2_OUT_IMAGE_SIZE);
>  	}
> -- 
> 1.7.10.4
> 
> 
> 

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

Re: [PATCH v2] media: i.MX27 camera: fix picture source width

[javier Martin]

Hi,
sorry for the long delay. I missed this one.

On 5 March 2013 18:56, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> (Javier's opinion requested)
>
> I'm no expert in i.MX27 hardware, would be great to have an ack from
> someone, intensively working in this area. Javier, what do you think? Is
> this really correct always for channel 1, or should this calculation
> depend on the pixel format?
>
> Thanks
> Guennadi
>
> On Sat, 16 Feb 2013, Christoph Fritz wrote:
>
>> While using a mt9m001 (monochrome) camera the final output falsely gets
>> horizontally divided into two pictures.
>>
>> The issue was git bisected to commit f410991dcf1f
>>
>>   |  [media] i.MX27 camera: add support for YUV420 format
>>   |
>>   |  This patch uses channel 2 of the eMMa-PrP to convert
>>   |  format provided by the sensor to YUV420.
>>   |
>>   |  This format is very useful since it is used by the
>>   |  internal H.264 encoder.
>>
>> It sets PICTURE_X_SIZE in register PRP_SRC_FRAME_SIZE to its full width
>> while before that commit it was divided by two:
>>
>> -   writel(((bytesperline >> 1) << 16) | icd->user_height,
>> +           writel((icd->user_width << 16) | icd->user_height,
>>                     pcdev->base_emma + PRP_SRC_FRAME_SIZE);
>>
>> i.mx27 reference manual (41.6.12 PrP Source Frame Size Register) says:
>>
>>     PICTURE_X_SIZE. These bits set the frame width to be
>>     processed in number of pixels. In YUV 4:2:0 mode, Cb and
>>     Cr widths are taken as PICTURE_X_SIZE/2 pixels.  In YUV
>>     4:2:0 mode, this value should be a multiple of 8-pixels.
>>     In other modes (RGB, YUV 4:2:2 and YUV 4:4:4) it should
>>     be a multiple of 4 pixels.

Note that, according to the description in the datasheet,
PICTURE_X_SIZE is specified in pixels, not bytes. So, it is not
dependant on the format used.

>> This patch reverts to PICTURE_X_SIZE/2 for channel 1.
>>
>> Tested on Kernel 3.4, merged to 3.8rc.
>>
>> Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
>> ---
>>  drivers/media/platform/soc_camera/mx2_camera.c |    6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/soc_camera/mx2_camera.c b/drivers/media/platform/soc_camera/mx2_camera.c
>> index 8bda2c9..795bd3f 100644
>> --- a/drivers/media/platform/soc_camera/mx2_camera.c
>> +++ b/drivers/media/platform/soc_camera/mx2_camera.c
>> @@ -778,11 +778,11 @@ static void mx27_camera_emma_buf_init(struct soc_camera_device *icd,
>>       struct mx2_camera_dev *pcdev = ici->priv;
>>       struct mx2_fmt_cfg *prp = pcdev->emma_prp;
>>
>> -     writel((pcdev->s_width << 16) | pcdev->s_height,
>> -            pcdev->base_emma + PRP_SRC_FRAME_SIZE);
>>       writel(prp->cfg.src_pixel,
>>              pcdev->base_emma + PRP_SRC_PIXEL_FORMAT_CNTL);
>>       if (prp->cfg.channel == 1) {
>> +             writel(((bytesperline >> 1) << 16) | pcdev->s_height,
>> +                     pcdev->base_emma + PRP_SRC_FRAME_SIZE);

If you use bytesperline here you are making this operation dependant
on the mbus format.
You should use s_width instead and there is no reason to divide it by
2 since it is already in pixels, as well as PRP_SRC_FRAME_SIZE.

>>               writel((icd->user_width << 16) | icd->user_height,
>>                       pcdev->base_emma + PRP_CH1_OUT_IMAGE_SIZE);
>>               writel(bytesperline,
>> @@ -790,6 +790,8 @@ static void mx27_camera_emma_buf_init(struct soc_camera_device *icd,
>>               writel(prp->cfg.ch1_pixel,
>>                       pcdev->base_emma + PRP_CH1_PIXEL_FORMAT_CNTL);
>>       } else { /* channel 2 */
>> +             writel((pcdev->s_width << 16) | pcdev->s_height,
>> +                     pcdev->base_emma + PRP_SRC_FRAME_SIZE);
>>               writel((icd->user_width << 16) | icd->user_height,
>>                       pcdev->base_emma + PRP_CH2_OUT_IMAGE_SIZE);
>>       }
>> --
>> 1.7.10.4

We are using channel 1 here daily for capturing YUV422 and have not
experience the problem you point out.

What mbus format are you using? Could you please check if the s_width
value that your sensor mt9m001 returns is correct? Remember it should
be in pixels, not in bytes.


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

Re: [PATCH v2] media: i.MX27 camera: fix picture source width

[Guennadi Liakhovetski]

Hi Javier

On Thu, 7 Mar 2013, javier Martin wrote:

> Hi,
> sorry for the long delay. I missed this one.
> 
> On 5 March 2013 18:56, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> > (Javier's opinion requested)
> >
> > I'm no expert in i.MX27 hardware, would be great to have an ack from
> > someone, intensively working in this area. Javier, what do you think? Is
> > this really correct always for channel 1, or should this calculation
> > depend on the pixel format?
> >
> > Thanks
> > Guennadi
> >
> > On Sat, 16 Feb 2013, Christoph Fritz wrote:
> >
> >> While using a mt9m001 (monochrome) camera the final output falsely gets
> >> horizontally divided into two pictures.
> >>
> >> The issue was git bisected to commit f410991dcf1f
> >>
> >>   |  [media] i.MX27 camera: add support for YUV420 format
> >>   |
> >>   |  This patch uses channel 2 of the eMMa-PrP to convert
> >>   |  format provided by the sensor to YUV420.
> >>   |
> >>   |  This format is very useful since it is used by the
> >>   |  internal H.264 encoder.
> >>
> >> It sets PICTURE_X_SIZE in register PRP_SRC_FRAME_SIZE to its full width
> >> while before that commit it was divided by two:
> >>
> >> -   writel(((bytesperline >> 1) << 16) | icd->user_height,
> >> +           writel((icd->user_width << 16) | icd->user_height,
> >>                     pcdev->base_emma + PRP_SRC_FRAME_SIZE);
> >>
> >> i.mx27 reference manual (41.6.12 PrP Source Frame Size Register) says:
> >>
> >>     PICTURE_X_SIZE. These bits set the frame width to be
> >>     processed in number of pixels. In YUV 4:2:0 mode, Cb and
> >>     Cr widths are taken as PICTURE_X_SIZE/2 pixels.  In YUV
> >>     4:2:0 mode, this value should be a multiple of 8-pixels.
> >>     In other modes (RGB, YUV 4:2:2 and YUV 4:4:4) it should
> >>     be a multiple of 4 pixels.
> 
> Note that, according to the description in the datasheet,
> PICTURE_X_SIZE is specified in pixels, not bytes. So, it is not
> dependant on the format used.

Yes, looks like it should operate in pixels, not bytes.

> >> This patch reverts to PICTURE_X_SIZE/2 for channel 1.
> >>
> >> Tested on Kernel 3.4, merged to 3.8rc.
> >>
> >> Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
> >> ---
> >>  drivers/media/platform/soc_camera/mx2_camera.c |    6 ++++--
> >>  1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/media/platform/soc_camera/mx2_camera.c b/drivers/media/platform/soc_camera/mx2_camera.c
> >> index 8bda2c9..795bd3f 100644
> >> --- a/drivers/media/platform/soc_camera/mx2_camera.c
> >> +++ b/drivers/media/platform/soc_camera/mx2_camera.c
> >> @@ -778,11 +778,11 @@ static void mx27_camera_emma_buf_init(struct soc_camera_device *icd,
> >>       struct mx2_camera_dev *pcdev = ici->priv;
> >>       struct mx2_fmt_cfg *prp = pcdev->emma_prp;
> >>
> >> -     writel((pcdev->s_width << 16) | pcdev->s_height,
> >> -            pcdev->base_emma + PRP_SRC_FRAME_SIZE);
> >>       writel(prp->cfg.src_pixel,
> >>              pcdev->base_emma + PRP_SRC_PIXEL_FORMAT_CNTL);
> >>       if (prp->cfg.channel == 1) {
> >> +             writel(((bytesperline >> 1) << 16) | pcdev->s_height,
> >> +                     pcdev->base_emma + PRP_SRC_FRAME_SIZE);
> 
> If you use bytesperline here you are making this operation dependant
> on the mbus format.
> You should use s_width instead and there is no reason to divide it by
> 2 since it is already in pixels, as well as PRP_SRC_FRAME_SIZE.
> 
> >>               writel((icd->user_width << 16) | icd->user_height,
> >>                       pcdev->base_emma + PRP_CH1_OUT_IMAGE_SIZE);
> >>               writel(bytesperline,
> >> @@ -790,6 +790,8 @@ static void mx27_camera_emma_buf_init(struct soc_camera_device *icd,
> >>               writel(prp->cfg.ch1_pixel,
> >>                       pcdev->base_emma + PRP_CH1_PIXEL_FORMAT_CNTL);
> >>       } else { /* channel 2 */
> >> +             writel((pcdev->s_width << 16) | pcdev->s_height,
> >> +                     pcdev->base_emma + PRP_SRC_FRAME_SIZE);
> >>               writel((icd->user_width << 16) | icd->user_height,
> >>                       pcdev->base_emma + PRP_CH2_OUT_IMAGE_SIZE);
> >>       }
> >> --
> >> 1.7.10.4
> 
> We are using channel 1 here daily for capturing YUV422 and have not
> experience the problem you point out.
> 
> What mbus format are you using? Could you please check if the s_width
> value that your sensor mt9m001 returns is correct? Remember it should
> be in pixels, not in bytes.

Thanks for looking at this. But here's my question: for a pass-through 
mode mx2_camera uses a 16-bpp (RGB565) format. But what if it's actually 
an 8-bpp format, don't you then have to adjust line-width register 
settings? If you don't do that, the camera interface would expect a double 
number of bytes per line, so, it could get confused by HSYNC coming after 
half-line?

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

Re: [PATCH v2] media: i.MX27 camera: fix picture source width

[Christoph Fritz]

On Tue, 2013-03-12 at 08:58 +0100, Guennadi Liakhovetski wrote:
> On Thu, 7 Mar 2013, javier Martin wrote: 

> > What mbus format are you using? Could you please check if the s_width
> > value that your sensor mt9m001 returns is correct? Remember it should
> > be in pixels, not in bytes.
> 
> Thanks for looking at this. But here's my question: for a pass-through 
> mode mx2_camera uses a 16-bpp (RGB565) format. But what if it's actually 
> an 8-bpp format, don't you then have to adjust line-width register 
> settings? If you don't do that, the camera interface would expect a double 
> number of bytes per line, so, it could get confused by HSYNC coming after 
> half-line?

To emphasize this: I'm using here a mt9m001 (monochrome) camera with an
8-bpp format.

Thanks
 -- Christoph

Re: [PATCH v2] media: i.MX27 camera: fix picture source width

[javier Martin]

Hi Guernnadi, Christoph,

On 12 March 2013 09:25, Christoph Fritz <chf.fritz@googlemail.com> wrote:
> On Tue, 2013-03-12 at 08:58 +0100, Guennadi Liakhovetski wrote:
>> On Thu, 7 Mar 2013, javier Martin wrote:
>
>> > What mbus format are you using? Could you please check if the s_width
>> > value that your sensor mt9m001 returns is correct? Remember it should
>> > be in pixels, not in bytes.
>>
>> Thanks for looking at this. But here's my question: for a pass-through
>> mode mx2_camera uses a 16-bpp (RGB565) format. But what if it's actually
>> an 8-bpp format, don't you then have to adjust line-width register
>> settings? If you don't do that, the camera interface would expect a double
>> number of bytes per line, so, it could get confused by HSYNC coming after
>> half-line?

You are right.

> To emphasize this: I'm using here a mt9m001 (monochrome) camera with an
> 8-bpp format.

Ok, now that makes sense.

Then, what you should do is apply your patch conditionally so that you
don't break other working cases:
- Channel 1 is being used.
- Channel 1 is in pass-through mode.
- The sensor uses an 8-bpp format.

Regards.
-- 
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

Re: [PATCH v2] media: i.MX27 camera: fix picture source width

[Guennadi Liakhovetski]

On Tue, 12 Mar 2013, javier Martin wrote:

> Hi Guernnadi, Christoph,
> 
> On 12 March 2013 09:25, Christoph Fritz <chf.fritz@googlemail.com> wrote:
> > On Tue, 2013-03-12 at 08:58 +0100, Guennadi Liakhovetski wrote:
> >> On Thu, 7 Mar 2013, javier Martin wrote:
> >
> >> > What mbus format are you using? Could you please check if the s_width
> >> > value that your sensor mt9m001 returns is correct? Remember it should
> >> > be in pixels, not in bytes.
> >>
> >> Thanks for looking at this. But here's my question: for a pass-through
> >> mode mx2_camera uses a 16-bpp (RGB565) format. But what if it's actually
> >> an 8-bpp format, don't you then have to adjust line-width register
> >> settings? If you don't do that, the camera interface would expect a double
> >> number of bytes per line, so, it could get confused by HSYNC coming after
> >> half-line?
> 
> You are right.
> 
> > To emphasize this: I'm using here a mt9m001 (monochrome) camera with an
> > 8-bpp format.
> 
> Ok, now that makes sense.
> 
> Then, what you should do is apply your patch conditionally so that you
> don't break other working cases:
> - Channel 1 is being used.
> - Channel 1 is in pass-through mode.

which would be

	if (!prp->in_fmt && !prp->out_fmt)

> - The sensor uses an 8-bpp format.

No, the format in unimportant - you pretend to use a 16-bit format, so, 
your "simulated" line is always bytesperline / 2 pseudo-pixels long. 
Christoph, in your next comment please add a comment something like

	/*
	 * In pass-through we configure EMMA with a 16-bpp format,
	 * set the line-width accordingly.
	 */

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

Re: [PATCH v2] media: i.MX27 camera: fix picture source width

[javier Martin]

On 12 March 2013 10:39, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> On Tue, 12 Mar 2013, javier Martin wrote:
>
>> Hi Guernnadi, Christoph,
>>
>> On 12 March 2013 09:25, Christoph Fritz <chf.fritz@googlemail.com> wrote:
>> > On Tue, 2013-03-12 at 08:58 +0100, Guennadi Liakhovetski wrote:
>> >> On Thu, 7 Mar 2013, javier Martin wrote:
>> >
>> >> > What mbus format are you using? Could you please check if the s_width
>> >> > value that your sensor mt9m001 returns is correct? Remember it should
>> >> > be in pixels, not in bytes.
>> >>
>> >> Thanks for looking at this. But here's my question: for a pass-through
>> >> mode mx2_camera uses a 16-bpp (RGB565) format. But what if it's actually
>> >> an 8-bpp format, don't you then have to adjust line-width register
>> >> settings? If you don't do that, the camera interface would expect a double
>> >> number of bytes per line, so, it could get confused by HSYNC coming after
>> >> half-line?
>>
>> You are right.
>>
>> > To emphasize this: I'm using here a mt9m001 (monochrome) camera with an
>> > 8-bpp format.
>>
>> Ok, now that makes sense.
>>
>> Then, what you should do is apply your patch conditionally so that you
>> don't break other working cases:
>> - Channel 1 is being used.
>> - Channel 1 is in pass-through mode.
>
> which would be
>
>         if (!prp->in_fmt && !prp->out_fmt)

Yes, that would do.

>> - The sensor uses an 8-bpp format.
>
> No, the format in unimportant - you pretend to use a 16-bit format, so,
> your "simulated" line is always bytesperline / 2 pseudo-pixels long.

OK.

> Christoph, in your next comment please add a comment something like
>
>         /*
>          * In pass-through we configure EMMA with a 16-bpp format,
>          * set the line-width accordingly.
>          */




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