Re: [PATCH] media: soc_camera: rcar_vin: Fix alignment of clipping size
Hi, On Fri, Oct 31, 2014 at 04:40:46PM +0300, Sergei Shtylyov wrote: Hello. On 10/31/2014 12:10 PM, Yoshihiro Kaneko wrote: From: Koji Matsuoka koji.matsuoka...@renesas.com Since the Start Line Pre-Clip register, the Start Pixel Pre-Clip register and the End Line Post-Clip register do not have restriction Hm, my R-Car H1 manual says to specify an even number for the Start Pixel Pre-Clip Register. Thanks. I have confirmed with Matsuoka-san that you are correct. It seems to me that the following portion of the patch should be removed accordingly. --- a/drivers/media/platform/soc_camera/rcar_vin.c +++ b/drivers/media/platform/soc_camera/rcar_vin.c @@ -1558,8 +1558,8 @@ static int rcar_vin_set_crop(struct soc_camera_device *icd, cam-width = mf.width; cam-height = mf.height; - cam-vin_left = rect-left ~1; - cam-vin_top = rect-top ~1; + cam-vin_left = rect-left; + cam-vin_top = rect-top; /* Use VIN cropping to crop to the new window. */ ret = rcar_vin_set_rect(icd); of H/W to a setting value, the processing of alignment is unnecessary. This patch corrects so that processing of alignment is not performed. However, the End Pixel Post-Clip register has restriction of H/W which must be an even number value at the time of the output of YCbCr-422 format. By this patch, the processing of alignment to an even number value is added on the above-mentioned conditions. Well, the R-Car H1/M1A manuals don't specify such restriction. Thanks, I have confirmed with Matsuoka-san that the above text is somewhat misleading. And I think it should read something more like this: In the case where YCbCr-422 is used and (End pixel post clip) - (Start pixel post clip) is odd one will be added by the hardware to round it up to an even number. I see this in section 26.2.11 Video n End Pixel Post-Clip Register (VnEPPoC) if the R-Car Gen 2 manual. The variable set to a register is as follows. - Start Line Pre-Clip register cam-vin_top - Start Pixel Pre-Clip register cam-vin_left - End Line Post-Clip register pix-height - End Pixel Post-Clip register pix-width Signed-off-by: Koji Matsuoka koji.matsuoka...@renesas.com Signed-off-by: Yoshihiro Kaneko ykaneko0...@gmail.com --- drivers/media/platform/soc_camera/rcar_vin.c | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c index d3d2f7d..1934e15 100644 --- a/drivers/media/platform/soc_camera/rcar_vin.c +++ b/drivers/media/platform/soc_camera/rcar_vin.c [...] @@ -1761,8 +1761,18 @@ static int rcar_vin_try_fmt(struct soc_camera_device *icd, } /* FIXME: calculate using depth and bus width */ -v4l_bound_align_image(pix-width, 2, VIN_MAX_WIDTH, 1, - pix-height, 4, VIN_MAX_HEIGHT, 2, 0); +/* + * When performing a YCbCr-422 format output, even if it performs + * odd number clipping by pixel post clip processing, it is outputted s/outputted/output/ -- it's an irregular verb. + * to a memory per even pixels. + */ +if ((pixfmt == V4L2_PIX_FMT_NV16) || (pixfmt == V4L2_PIX_FMT_YUYV) || +(pixfmt == V4L2_PIX_FMT_UYVY)) This is certainly asking to be a *switch* statement instead... +v4l_bound_align_image(pix-width, 5, VIN_MAX_WIDTH, 1, + pix-height, 2, VIN_MAX_HEIGHT, 0, 0); +else +v4l_bound_align_image(pix-width, 5, VIN_MAX_WIDTH, 0, + pix-height, 2, VIN_MAX_HEIGHT, 0, 0); WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-sh in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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
[PATCH] media: soc_camera: rcar_vin: Fix alignment of clipping size
From: Koji Matsuoka koji.matsuoka...@renesas.com Since the Start Line Pre-Clip register, the Start Pixel Pre-Clip register and the End Line Post-Clip register do not have restriction of H/W to a setting value, the processing of alignment is unnecessary. This patch corrects so that processing of alignment is not performed. However, the End Pixel Post-Clip register has restriction of H/W which must be an even number value at the time of the output of YCbCr-422 format. By this patch, the processing of alignment to an even number value is added on the above-mentioned conditions. The variable set to a register is as follows. - Start Line Pre-Clip register cam-vin_top - Start Pixel Pre-Clip register cam-vin_left - End Line Post-Clip register pix-height - End Pixel Post-Clip register pix-width Signed-off-by: Koji Matsuoka koji.matsuoka...@renesas.com Signed-off-by: Yoshihiro Kaneko ykaneko0...@gmail.com --- drivers/media/platform/soc_camera/rcar_vin.c | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c index d3d2f7d..1934e15 100644 --- a/drivers/media/platform/soc_camera/rcar_vin.c +++ b/drivers/media/platform/soc_camera/rcar_vin.c @@ -1558,8 +1558,8 @@ static int rcar_vin_set_crop(struct soc_camera_device *icd, cam-width = mf.width; cam-height = mf.height; - cam-vin_left = rect-left ~1; - cam-vin_top = rect-top ~1; + cam-vin_left = rect-left; + cam-vin_top = rect-top; /* Use VIN cropping to crop to the new window. */ ret = rcar_vin_set_rect(icd); @@ -1761,8 +1761,18 @@ static int rcar_vin_try_fmt(struct soc_camera_device *icd, } /* FIXME: calculate using depth and bus width */ - v4l_bound_align_image(pix-width, 2, VIN_MAX_WIDTH, 1, - pix-height, 4, VIN_MAX_HEIGHT, 2, 0); + /* +* When performing a YCbCr-422 format output, even if it performs +* odd number clipping by pixel post clip processing, it is outputted +* to a memory per even pixels. +*/ + if ((pixfmt == V4L2_PIX_FMT_NV16) || (pixfmt == V4L2_PIX_FMT_YUYV) || + (pixfmt == V4L2_PIX_FMT_UYVY)) + v4l_bound_align_image(pix-width, 5, VIN_MAX_WIDTH, 1, + pix-height, 2, VIN_MAX_HEIGHT, 0, 0); + else + v4l_bound_align_image(pix-width, 5, VIN_MAX_WIDTH, 0, + pix-height, 2, VIN_MAX_HEIGHT, 0, 0); width = pix-width; height = pix-height; -- 1.9.1 -- 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] media: soc_camera: rcar_vin: Fix alignment of clipping size
Hello. On 10/31/2014 12:10 PM, Yoshihiro Kaneko wrote: From: Koji Matsuoka koji.matsuoka...@renesas.com Since the Start Line Pre-Clip register, the Start Pixel Pre-Clip register and the End Line Post-Clip register do not have restriction Hm, my R-Car H1 manual says to specify an even number for the Start Pixel Pre-Clip Register. of H/W to a setting value, the processing of alignment is unnecessary. This patch corrects so that processing of alignment is not performed. However, the End Pixel Post-Clip register has restriction of H/W which must be an even number value at the time of the output of YCbCr-422 format. By this patch, the processing of alignment to an even number value is added on the above-mentioned conditions. Well, the R-Car H1/M1A manuals don't specify such restriction. The variable set to a register is as follows. - Start Line Pre-Clip register cam-vin_top - Start Pixel Pre-Clip register cam-vin_left - End Line Post-Clip register pix-height - End Pixel Post-Clip register pix-width Signed-off-by: Koji Matsuoka koji.matsuoka...@renesas.com Signed-off-by: Yoshihiro Kaneko ykaneko0...@gmail.com --- drivers/media/platform/soc_camera/rcar_vin.c | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c index d3d2f7d..1934e15 100644 --- a/drivers/media/platform/soc_camera/rcar_vin.c +++ b/drivers/media/platform/soc_camera/rcar_vin.c [...] @@ -1761,8 +1761,18 @@ static int rcar_vin_try_fmt(struct soc_camera_device *icd, } /* FIXME: calculate using depth and bus width */ - v4l_bound_align_image(pix-width, 2, VIN_MAX_WIDTH, 1, - pix-height, 4, VIN_MAX_HEIGHT, 2, 0); + /* +* When performing a YCbCr-422 format output, even if it performs +* odd number clipping by pixel post clip processing, it is outputted s/outputted/output/ -- it's an irregular verb. +* to a memory per even pixels. +*/ + if ((pixfmt == V4L2_PIX_FMT_NV16) || (pixfmt == V4L2_PIX_FMT_YUYV) || + (pixfmt == V4L2_PIX_FMT_UYVY)) This is certainly asking to be a *switch* statement instead... + v4l_bound_align_image(pix-width, 5, VIN_MAX_WIDTH, 1, + pix-height, 2, VIN_MAX_HEIGHT, 0, 0); + else + v4l_bound_align_image(pix-width, 5, VIN_MAX_WIDTH, 0, + pix-height, 2, VIN_MAX_HEIGHT, 0, 0); WBR, Sergei -- 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
