RE: [PATCH V3 06/15] [media] marvell-ccic: add new formats support for marvell-ccic driver
Hi Guennadi,
Below is the update for widthy, widthuv and imgsz_w setting.
>-Original Message-
>From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de]
>Sent: Wednesday, January 02, 2013 12:56 AM
>To: Albert Wang
>Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang
>Subject: Re: [PATCH V3 06/15] [media] marvell-ccic: add new formats support for
>marvell-ccic driver
>
>On Sat, 15 Dec 2012, Albert Wang wrote:
>
>> From: Libin Yang
>>
>> This patch adds the new formats support for marvell-ccic.
>>
>> Signed-off-by: Albert Wang
>> Signed-off-by: Libin Yang
>> ---
>> drivers/media/platform/marvell-ccic/mcam-core.c | 175
>> ++-
>> drivers/media/platform/marvell-ccic/mcam-core.h |6 +
>> 2 files changed, 149 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
>b/drivers/media/platform/marvell-ccic/mcam-core.c
>> index 3cc1d0c..a679917 100755
>> --- a/drivers/media/platform/marvell-ccic/mcam-core.c
>> +++ b/drivers/media/platform/marvell-ccic/mcam-core.c
>
>[snip]
>
>> @@ -658,49 +708,85 @@ static inline void mcam_sg_restart(struct mcam_camera
>> *cam)
>> */
>> static void mcam_ctlr_image(struct mcam_camera *cam)
>> {
>> -int imgsz;
>> struct v4l2_pix_format *fmt = &cam->pix_format;
>> +u32 widthy = 0, widthuv = 0, imgsz_h, imgsz_w;
>> +
>> +cam_dbg(cam, "camera: bytesperline = %d; height = %d\n",
>> +fmt->bytesperline, fmt->sizeimage / fmt->bytesperline);
>> +imgsz_h = (fmt->height << IMGSZ_V_SHIFT) & IMGSZ_V_MASK;
>> +imgsz_w = fmt->bytesperline & IMGSZ_H_MASK;
>> +
>> +switch (fmt->pixelformat) {
>> +case V4L2_PIX_FMT_YUYV:
>> +case V4L2_PIX_FMT_UYVY:
>> +widthy = fmt->width * 2;
>> +widthuv = 0;
>> +break;
>> +case V4L2_PIX_FMT_JPEG:
>> +imgsz_h = (fmt->sizeimage / fmt->bytesperline) << IMGSZ_V_SHIFT;
>> +widthy = fmt->bytesperline;
>> +widthuv = 0;
>> +break;
>> +case V4L2_PIX_FMT_YUV422P:
>> +case V4L2_PIX_FMT_YUV420:
>> +case V4L2_PIX_FMT_YVU420:
>> +imgsz_w = (fmt->bytesperline * 4 / 3) & IMGSZ_H_MASK;
>> +widthy = fmt->width;
>> +widthuv = fmt->width / 2;
>
>I might be wrong, but the above doesn't look right to me. Firstly, YUV422P
>is a 4:2:2 format, whereas YUV420 and YVU420 are 4:2:0 formats, so, I
>would expect calculations for them to differ. Besides, bytesperline * 4 /
>3 doesn't look right for any of them. If this is what I think - total
>number of bytes per line, i.e., sizeimage / height, than shouldn't YAU422P
>have
>+ imgsz_w = fmt->bytesperline & IMGSZ_H_MASK;
>and the other two
>+ imgsz_w = (fmt->bytesperline * 3 / 2) & IMGSZ_H_MASK;
>? But maybe I'm wrong, please, double-check and confirm.
>
First of all, the setting bytesperline in this file is wrong. It should be
like the setting in the mcam-core-soc.c in the later patch. It's my fault
missing modifying the bytesperline when adding the new formats.
Besides the bytesperline in mcam-core-soc.c is a little wrong.
I will update it in the new version of patch.
For imgsz_w setting, this is for the CCIC input data format, which is from
sensor, while 'switch (fmt->pixelformat)' is CCIC output data format.
It seems a little confusing using fmt->pixelformat here. Using
mcam_formats->mbus_code seems more reasonable. Anyway,
each fmt->pixelformat must have one mcam_formats->mbus_code
correspondingly.
Although, our spec says it supports YUV420 input. Our HW team told me
we only use YUV422 and the length is width * 2. So it seems some
mbus_code is wrong set here.
It seems in this case such format will be never used as we can see
ov7670 does not support yuv420.
Regards,
Libin
--
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
RE: [PATCH V3 06/15] [media] marvell-ccic: add new formats support for marvell-ccic driver
Hi Guennadi,
Please see my comments below.
>-Original Message-
>From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de]
>Sent: Wednesday, January 02, 2013 12:56 AM
>To: Albert Wang
>Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang
>Subject: Re: [PATCH V3 06/15] [media] marvell-ccic: add new formats support for
>marvell-ccic driver
>
>On Sat, 15 Dec 2012, Albert Wang wrote:
>
>> From: Libin Yang
>>
>> This patch adds the new formats support for marvell-ccic.
>>
>> Signed-off-by: Albert Wang
>> Signed-off-by: Libin Yang
>> ---
>> drivers/media/platform/marvell-ccic/mcam-core.c | 175
>> ++-
>> drivers/media/platform/marvell-ccic/mcam-core.h |6 +
>> 2 files changed, 149 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
>b/drivers/media/platform/marvell-ccic/mcam-core.c
>> index 3cc1d0c..a679917 100755
>> --- a/drivers/media/platform/marvell-ccic/mcam-core.c
>> +++ b/drivers/media/platform/marvell-ccic/mcam-core.c
>
>[snip]
>
>> @@ -658,49 +708,85 @@ static inline void mcam_sg_restart(struct mcam_camera
>> *cam)
>> */
>> static void mcam_ctlr_image(struct mcam_camera *cam)
>> {
>> -int imgsz;
>> struct v4l2_pix_format *fmt = &cam->pix_format;
>> +u32 widthy = 0, widthuv = 0, imgsz_h, imgsz_w;
>> +
>> +cam_dbg(cam, "camera: bytesperline = %d; height = %d\n",
>> +fmt->bytesperline, fmt->sizeimage / fmt->bytesperline);
>> +imgsz_h = (fmt->height << IMGSZ_V_SHIFT) & IMGSZ_V_MASK;
>> +imgsz_w = fmt->bytesperline & IMGSZ_H_MASK;
>> +
>> +switch (fmt->pixelformat) {
>> +case V4L2_PIX_FMT_YUYV:
>> +case V4L2_PIX_FMT_UYVY:
>> +widthy = fmt->width * 2;
>> +widthuv = 0;
>> +break;
>> +case V4L2_PIX_FMT_JPEG:
>> +imgsz_h = (fmt->sizeimage / fmt->bytesperline) << IMGSZ_V_SHIFT;
>> +widthy = fmt->bytesperline;
>> +widthuv = 0;
>> +break;
>> +case V4L2_PIX_FMT_YUV422P:
>> +case V4L2_PIX_FMT_YUV420:
>> +case V4L2_PIX_FMT_YVU420:
>> +imgsz_w = (fmt->bytesperline * 4 / 3) & IMGSZ_H_MASK;
>> +widthy = fmt->width;
>> +widthuv = fmt->width / 2;
>
>I might be wrong, but the above doesn't look right to me. Firstly, YUV422P
>is a 4:2:2 format, whereas YUV420 and YVU420 are 4:2:0 formats, so, I
>would expect calculations for them to differ. Besides, bytesperline * 4 /
>3 doesn't look right for any of them. If this is what I think - total
>number of bytes per line, i.e., sizeimage / height, than shouldn't YAU422P
>have
>+ imgsz_w = fmt->bytesperline & IMGSZ_H_MASK;
>and the other two
>+ imgsz_w = (fmt->bytesperline * 3 / 2) & IMGSZ_H_MASK;
>? But maybe I'm wrong, please, double-check and confirm.
>
Basically, I agree with you. We are checking with our HW team how the value is
calculated out. And we will update here as soon as we get the answer.
>> +break;
>> +default:
>> +widthy = fmt->bytesperline;
>> +widthuv = 0;
>> +}
>> +
>> +mcam_reg_write_mask(cam, REG_IMGPITCH, widthuv << 16 | widthy,
>> +IMGP_YP_MASK | IMGP_UVP_MASK);
>> +mcam_reg_write(cam, REG_IMGSIZE, imgsz_h | imgsz_w);
>> +mcam_reg_write(cam, REG_IMGOFFSET, 0x0);
>>
>> -imgsz = ((fmt->height << IMGSZ_V_SHIFT) & IMGSZ_V_MASK) |
>> -(fmt->bytesperline & IMGSZ_H_MASK);
>> -mcam_reg_write(cam, REG_IMGSIZE, imgsz);
>> -mcam_reg_write(cam, REG_IMGOFFSET, 0);
>> -/* YPITCH just drops the last two bits */
>> -mcam_reg_write_mask(cam, REG_IMGPITCH, fmt->bytesperline,
>> -IMGP_YP_MASK);
>> /*
>> * Tell the controller about the image format we are using.
>> */
>> -switch (cam->pix_format.pixelformat) {
>> +switch (fmt->pixelformat) {
>> +case V4L2_PIX_FMT_YUV422P:
>> +mcam_reg_write_mask(cam, REG_CTRL0,
>> +C0_DF_YUV | C0_YUV_PLANAR | C0_YUVE_YVYU,
>C0_DF_MASK);
>> +break;
>> +case V4L2_PIX_FMT_YUV420:
>> +case V4L2_PIX_FMT_YVU420:
>> +mcam_reg_write_mask(cam, REG_CTRL0,
>> +C0_DF_YUV | C0_YUV_420PL | C0_YUVE_YVYU, C0_DF_MASK);
>&g
RE: [PATCH V3 06/15] [media] marvell-ccic: add new formats support for marvell-ccic driver
Hi Albert On Tue, 1 Jan 2013, Albert Wang wrote: > >> + case V4L2_PIX_FMT_YUV422P: > >> + case V4L2_PIX_FMT_YUV420: > >> + case V4L2_PIX_FMT_YVU420: > >> + imgsz_w = (fmt->bytesperline * 4 / 3) & IMGSZ_H_MASK; > >> + widthy = fmt->width; > >> + widthuv = fmt->width / 2; > > > >I might be wrong, but the above doesn't look right to me. Firstly, YUV422P > >is a 4:2:2 format, whereas YUV420 and YVU420 are 4:2:0 formats, so, I > >would expect calculations for them to differ. Besides, bytesperline * 4 / > >3 doesn't look right for any of them. If this is what I think - total > >number of bytes per line, i.e., sizeimage / height, than shouldn't YAU422P > >have > >+imgsz_w = fmt->bytesperline & IMGSZ_H_MASK; > >and the other two > >+imgsz_w = (fmt->bytesperline * 3 / 2) & IMGSZ_H_MASK; > >? But maybe I'm wrong, please, double-check and confirm. > > > [Albert Wang] It looks they are both 12 bit planar format, they have same > imgsz_w. > Anyway, we will double check it after back to office. _Both_ YUV420 and YVU420 - yes, but YUV422P is 16-bit. 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH V3 06/15] [media] marvell-ccic: add new formats support for marvell-ccic driver
Hi, Guennadi
>-Original Message-
>From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de]
>Sent: Wednesday, 02 January, 2013 00:56
>To: Albert Wang
>Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang
>Subject: Re: [PATCH V3 06/15] [media] marvell-ccic: add new formats support for
>marvell-ccic driver
>
>On Sat, 15 Dec 2012, Albert Wang wrote:
>
>> From: Libin Yang
>>
>> This patch adds the new formats support for marvell-ccic.
>>
>> Signed-off-by: Albert Wang
>> Signed-off-by: Libin Yang
>> ---
>> drivers/media/platform/marvell-ccic/mcam-core.c | 175
>> ++-
>> drivers/media/platform/marvell-ccic/mcam-core.h |6 +
>> 2 files changed, 149 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
>b/drivers/media/platform/marvell-ccic/mcam-core.c
>> index 3cc1d0c..a679917 100755
>> --- a/drivers/media/platform/marvell-ccic/mcam-core.c
>> +++ b/drivers/media/platform/marvell-ccic/mcam-core.c
>
>[snip]
>
>> @@ -658,49 +708,85 @@ static inline void mcam_sg_restart(struct mcam_camera
>*cam)
>> */
>> static void mcam_ctlr_image(struct mcam_camera *cam)
>> {
>> -int imgsz;
>> struct v4l2_pix_format *fmt = &cam->pix_format;
>> +u32 widthy = 0, widthuv = 0, imgsz_h, imgsz_w;
>> +
>> +cam_dbg(cam, "camera: bytesperline = %d; height = %d\n",
>> +fmt->bytesperline, fmt->sizeimage / fmt->bytesperline);
>> +imgsz_h = (fmt->height << IMGSZ_V_SHIFT) & IMGSZ_V_MASK;
>> +imgsz_w = fmt->bytesperline & IMGSZ_H_MASK;
>> +
>> +switch (fmt->pixelformat) {
>> +case V4L2_PIX_FMT_YUYV:
>> +case V4L2_PIX_FMT_UYVY:
>> +widthy = fmt->width * 2;
>> +widthuv = 0;
>> +break;
>> +case V4L2_PIX_FMT_JPEG:
>> +imgsz_h = (fmt->sizeimage / fmt->bytesperline) << IMGSZ_V_SHIFT;
>> +widthy = fmt->bytesperline;
>> +widthuv = 0;
>> +break;
>> +case V4L2_PIX_FMT_YUV422P:
>> +case V4L2_PIX_FMT_YUV420:
>> +case V4L2_PIX_FMT_YVU420:
>> +imgsz_w = (fmt->bytesperline * 4 / 3) & IMGSZ_H_MASK;
>> +widthy = fmt->width;
>> +widthuv = fmt->width / 2;
>
>I might be wrong, but the above doesn't look right to me. Firstly, YUV422P
>is a 4:2:2 format, whereas YUV420 and YVU420 are 4:2:0 formats, so, I
>would expect calculations for them to differ. Besides, bytesperline * 4 /
>3 doesn't look right for any of them. If this is what I think - total
>number of bytes per line, i.e., sizeimage / height, than shouldn't YAU422P
>have
>+ imgsz_w = fmt->bytesperline & IMGSZ_H_MASK;
>and the other two
>+ imgsz_w = (fmt->bytesperline * 3 / 2) & IMGSZ_H_MASK;
>? But maybe I'm wrong, please, double-check and confirm.
>
[Albert Wang] It looks they are both 12 bit planar format, they have same
imgsz_w.
Anyway, we will double check it after back to office.
>> +break;
>> +default:
>> +widthy = fmt->bytesperline;
>> +widthuv = 0;
>> +}
>> +
>> +mcam_reg_write_mask(cam, REG_IMGPITCH, widthuv << 16 | widthy,
>> +IMGP_YP_MASK | IMGP_UVP_MASK);
>> +mcam_reg_write(cam, REG_IMGSIZE, imgsz_h | imgsz_w);
>> +mcam_reg_write(cam, REG_IMGOFFSET, 0x0);
>>
>> -imgsz = ((fmt->height << IMGSZ_V_SHIFT) & IMGSZ_V_MASK) |
>> -(fmt->bytesperline & IMGSZ_H_MASK);
>> -mcam_reg_write(cam, REG_IMGSIZE, imgsz);
>> -mcam_reg_write(cam, REG_IMGOFFSET, 0);
>> -/* YPITCH just drops the last two bits */
>> -mcam_reg_write_mask(cam, REG_IMGPITCH, fmt->bytesperline,
>> -IMGP_YP_MASK);
>> /*
>> * Tell the controller about the image format we are using.
>> */
>> -switch (cam->pix_format.pixelformat) {
>> +switch (fmt->pixelformat) {
>> +case V4L2_PIX_FMT_YUV422P:
>> +mcam_reg_write_mask(cam, REG_CTRL0,
>> +C0_DF_YUV | C0_YUV_PLANAR | C0_YUVE_YVYU,
>C0_DF_MASK);
>> +break;
>> +case V4L2_PIX_FMT_YUV420:
>> +case V4L2_PIX_FMT_YVU420:
>> +mcam_reg_write_mask(cam, REG_CTRL0,
>> +C0_DF_YUV | C0_YUV_420PL | C0_YUVE_YVYU,
>C0_DF_MASK);
>> +break;
>> case V4L2_PIX_FMT
Re: [PATCH V3 06/15] [media] marvell-ccic: add new formats support for marvell-ccic driver
On Sat, 15 Dec 2012, Albert Wang wrote:
> From: Libin Yang
>
> This patch adds the new formats support for marvell-ccic.
>
> Signed-off-by: Albert Wang
> Signed-off-by: Libin Yang
> ---
> drivers/media/platform/marvell-ccic/mcam-core.c | 175
> ++-
> drivers/media/platform/marvell-ccic/mcam-core.h |6 +
> 2 files changed, 149 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
> b/drivers/media/platform/marvell-ccic/mcam-core.c
> index 3cc1d0c..a679917 100755
> --- a/drivers/media/platform/marvell-ccic/mcam-core.c
> +++ b/drivers/media/platform/marvell-ccic/mcam-core.c
[snip]
> @@ -658,49 +708,85 @@ static inline void mcam_sg_restart(struct mcam_camera
> *cam)
> */
> static void mcam_ctlr_image(struct mcam_camera *cam)
> {
> - int imgsz;
> struct v4l2_pix_format *fmt = &cam->pix_format;
> + u32 widthy = 0, widthuv = 0, imgsz_h, imgsz_w;
> +
> + cam_dbg(cam, "camera: bytesperline = %d; height = %d\n",
> + fmt->bytesperline, fmt->sizeimage / fmt->bytesperline);
> + imgsz_h = (fmt->height << IMGSZ_V_SHIFT) & IMGSZ_V_MASK;
> + imgsz_w = fmt->bytesperline & IMGSZ_H_MASK;
> +
> + switch (fmt->pixelformat) {
> + case V4L2_PIX_FMT_YUYV:
> + case V4L2_PIX_FMT_UYVY:
> + widthy = fmt->width * 2;
> + widthuv = 0;
> + break;
> + case V4L2_PIX_FMT_JPEG:
> + imgsz_h = (fmt->sizeimage / fmt->bytesperline) << IMGSZ_V_SHIFT;
> + widthy = fmt->bytesperline;
> + widthuv = 0;
> + break;
> + case V4L2_PIX_FMT_YUV422P:
> + case V4L2_PIX_FMT_YUV420:
> + case V4L2_PIX_FMT_YVU420:
> + imgsz_w = (fmt->bytesperline * 4 / 3) & IMGSZ_H_MASK;
> + widthy = fmt->width;
> + widthuv = fmt->width / 2;
I might be wrong, but the above doesn't look right to me. Firstly, YUV422P
is a 4:2:2 format, whereas YUV420 and YVU420 are 4:2:0 formats, so, I
would expect calculations for them to differ. Besides, bytesperline * 4 /
3 doesn't look right for any of them. If this is what I think - total
number of bytes per line, i.e., sizeimage / height, than shouldn't YAU422P
have
+ imgsz_w = fmt->bytesperline & IMGSZ_H_MASK;
and the other two
+ imgsz_w = (fmt->bytesperline * 3 / 2) & IMGSZ_H_MASK;
? But maybe I'm wrong, please, double-check and confirm.
> + break;
> + default:
> + widthy = fmt->bytesperline;
> + widthuv = 0;
> + }
> +
> + mcam_reg_write_mask(cam, REG_IMGPITCH, widthuv << 16 | widthy,
> + IMGP_YP_MASK | IMGP_UVP_MASK);
> + mcam_reg_write(cam, REG_IMGSIZE, imgsz_h | imgsz_w);
> + mcam_reg_write(cam, REG_IMGOFFSET, 0x0);
>
> - imgsz = ((fmt->height << IMGSZ_V_SHIFT) & IMGSZ_V_MASK) |
> - (fmt->bytesperline & IMGSZ_H_MASK);
> - mcam_reg_write(cam, REG_IMGSIZE, imgsz);
> - mcam_reg_write(cam, REG_IMGOFFSET, 0);
> - /* YPITCH just drops the last two bits */
> - mcam_reg_write_mask(cam, REG_IMGPITCH, fmt->bytesperline,
> - IMGP_YP_MASK);
> /*
>* Tell the controller about the image format we are using.
>*/
> - switch (cam->pix_format.pixelformat) {
> + switch (fmt->pixelformat) {
> + case V4L2_PIX_FMT_YUV422P:
> + mcam_reg_write_mask(cam, REG_CTRL0,
> + C0_DF_YUV | C0_YUV_PLANAR | C0_YUVE_YVYU, C0_DF_MASK);
> + break;
> + case V4L2_PIX_FMT_YUV420:
> + case V4L2_PIX_FMT_YVU420:
> + mcam_reg_write_mask(cam, REG_CTRL0,
> + C0_DF_YUV | C0_YUV_420PL | C0_YUVE_YVYU, C0_DF_MASK);
> + break;
> case V4L2_PIX_FMT_YUYV:
> - mcam_reg_write_mask(cam, REG_CTRL0,
> - C0_DF_YUV|C0_YUV_PACKED|C0_YUVE_YUYV,
> - C0_DF_MASK);
> - break;
> -
> + mcam_reg_write_mask(cam, REG_CTRL0,
> + C0_DF_YUV | C0_YUV_PACKED | C0_YUVE_UYVY, C0_DF_MASK);
> + break;
> + case V4L2_PIX_FMT_UYVY:
> + mcam_reg_write_mask(cam, REG_CTRL0,
> + C0_DF_YUV | C0_YUV_PACKED | C0_YUVE_YUYV, C0_DF_MASK);
> + break;
> + case V4L2_PIX_FMT_JPEG:
> + mcam_reg_write_mask(cam, REG_CTRL0,
> + C0_DF_YUV | C0_YUV_PACKED | C0_YUVE_YUYV, C0_DF_MASK);
> + break;
> case V4L2_PIX_FMT_RGB444:
> - mcam_reg_write_mask(cam, REG_CTRL0,
> - C0_DF_RGB|C0_RGBF_444|C0_RGB4_XRGB,
> - C0_DF_MASK);
> + mcam_reg_write_mask(cam, REG_CTRL0,
> + C0_DF_RGB | C0_RGBF_444 | C0_RGB4_XRGB, C0_DF_MASK);
> /* Alpha value? */
> - break;
> -
> + break;
> case V4L2_PIX_FMT_RGB565:
> - mcam_reg_write_mask(cam, REG_CTRL0,
> -
RE: [PATCH V3 06/15] [media] marvell-ccic: add new formats support for marvell-ccic driver
Hi, Jonathan
>-Original Message-
>From: Jonathan Corbet [mailto:cor...@lwn.net]
>Sent: Monday, 17 December, 2012 00:17
>To: Albert Wang
>Cc: g.liakhovet...@gmx.de; linux-media@vger.kernel.org; Libin Yang
>Subject: Re: [PATCH V3 06/15] [media] marvell-ccic: add new formats support for
>marvell-ccic driver
>
>On Sat, 15 Dec 2012 17:57:55 +0800
>Albert Wang wrote:
>
>> From: Libin Yang
>>
>> This patch adds the new formats support for marvell-ccic.
>
>Once again, just one second-order comment:
>
>> +static bool mcam_fmt_is_planar(__u32 pfmt)
>> +{
>> +switch (pfmt) {
>> +case V4L2_PIX_FMT_YUV422P:
>> +case V4L2_PIX_FMT_YUV420:
>> +case V4L2_PIX_FMT_YVU420:
>> +return true;
>> +}
>> +return false;
>> +}
>
>This seems like the kind of thing that would be useful in a number of
>places; I'd be tempted to push it up a level and make it available to all
>V4L2 drivers. Of course, that means making it work for *all* formats,
>which would be a pain.
>
>But, then, I can see some potential future pain if somebody adds a new
>format and forgets to tweak this function here. Rather than adding a new
>switch, could you put a "planar" flag into struct mcam_format_struct
>instead? That would help to keep all this information together.
>
[Albert Wang] Yes, it looks make sense, we will think about it in next version.
>jon
Thanks
Albert Wang
86-21-61092656
--
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
Re: [PATCH V3 06/15] [media] marvell-ccic: add new formats support for marvell-ccic driver
On Sat, 15 Dec 2012 17:57:55 +0800
Albert Wang wrote:
> From: Libin Yang
>
> This patch adds the new formats support for marvell-ccic.
Once again, just one second-order comment:
> +static bool mcam_fmt_is_planar(__u32 pfmt)
> +{
> + switch (pfmt) {
> + case V4L2_PIX_FMT_YUV422P:
> + case V4L2_PIX_FMT_YUV420:
> + case V4L2_PIX_FMT_YVU420:
> + return true;
> + }
> + return false;
> +}
This seems like the kind of thing that would be useful in a number of
places; I'd be tempted to push it up a level and make it available to all
V4L2 drivers. Of course, that means making it work for *all* formats,
which would be a pain.
But, then, I can see some potential future pain if somebody adds a new
format and forgets to tweak this function here. Rather than adding a new
switch, could you put a "planar" flag into struct mcam_format_struct
instead? That would help to keep all this information together.
jon
--
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
