From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sergei Shtylyov Date: Fri, 31 Oct 2014 13:40:46 +0000 Subject: Re: [PATCH] media: soc_camera: rcar_vin: Fix alignment of clipping size Message-Id: <5453915E.3020100@cogentembedded.com> List-Id: References: <1414746610-23194-1-git-send-email-ykaneko0929@gmail.com> In-Reply-To: <1414746610-23194-1-git-send-email-ykaneko0929@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Yoshihiro Kaneko , linux-media@vger.kernel.org Cc: Guennadi Liakhovetski , Simon Horman , Magnus Damm , linux-sh@vger.kernel.org Hello. On 10/31/2014 12:10 PM, Yoshihiro Kaneko wrote: > From: Koji Matsuoka > 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 > Signed-off-by: Yoshihiro Kaneko > --- > 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