Hi Javier,

Thanks for the patch.

On Wednesday 11 July 2012 09:56:49 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.

The commit message doesn't really match the content of the patch anymore, as 
you don't remove the MX2_CAMERA_SWAP16 and MX2_CAMERA_PACK_DIR_MSB flags but 
just stop using them.

Could you please fix the commit message, and submit a patch that removes the 
flag from arch/arm/plat-mxc/include/mach/mx2_cam.h for v3.6 ?

Please don't forget to add your SoB line.

> ---
>  drivers/media/video/mx2_camera.c |   28 +++++++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/video/mx2_camera.c
> b/drivers/media/video/mx2_camera.c index 11a9353..0f01e7b 100644
> --- a/drivers/media/video/mx2_camera.c
> +++ b/drivers/media/video/mx2_camera.c
> @@ -118,6 +118,8 @@
>  #define CSISR_ECC_INT                (1 << 1)
>  #define CSISR_DRDY           (1 << 0)
> 
> +#define CSICR1_FMT_MASK       (CSICR1_PACK_DIR | CSICR1_SWAP16_EN)
> +
>  #define CSICR1                       0x00
>  #define CSICR2                       0x04
>  #define CSISR                        (cpu_is_mx27() ? 0x08 : 0x18)
> @@ -230,6 +232,7 @@ struct mx2_prp_cfg {
>       u32 src_pixel;
>       u32 ch1_pixel;
>       u32 irq_flags;
> +     u32 csicr1;
>  };
> 
>  /* prp resizing parameters */
> @@ -330,6 +333,7 @@ static struct mx2_fmt_cfg mx27_emma_prp_table[] = {
>                       .ch1_pixel      = 0x2ca00565, /* RGB565 */
>                       .irq_flags      = PRP_INTR_RDERR | PRP_INTR_CH1WERR |
>                                               PRP_INTR_CH1FC | PRP_INTR_LBOVF,
> +                     .csicr1         = 0,
>               }
>       },
>       {
> @@ -343,6 +347,21 @@ static struct mx2_fmt_cfg mx27_emma_prp_table[] = {
>                       .irq_flags      = PRP_INTR_RDERR | PRP_INTR_CH2WERR |
>                                       PRP_INTR_CH2FC | PRP_INTR_LBOVF |
>                                       PRP_INTR_CH2OVF,
> +                     .csicr1         = CSICR1_PACK_DIR,
> +             }
> +     },
> +     {
> +             .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,
> +                     .csicr1         = CSICR1_SWAP16_EN,
>               }

Have you tested this patch with both YUYV8_2X8 and UYVY8_2X8 ? I'm not 
familiar with the hardware, so I can't really comment on this specific hunk. 
Knowing that it has been tested would be enough for me to ack the patch (after 
fixing the commit message of course).

>       },
>  };
> @@ -1018,14 +1037,14 @@ static int mx2_camera_set_bus_param(struct
> soc_camera_device *icd) return ret;
>       }
> 
> +     csicr1 = (csicr1 & ~CSICR1_FMT_MASK) | pcdev->emma_prp->cfg.csicr1;
> +
>       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;

This is a completely different issue (and thus v3.6 material, if needed), but 
can common_flags change between invocations ? If so you should clear the 
CSICR1_* flags before conditionally setting them.

> -     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)
> @@ -1036,8 +1055,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;
> 
> @@ -1112,7 +1129,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) {
>                       /*
-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to