linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] media: soc_camera: rcar_vin: Fix alignment of clipping size
@ 2014-10-31  9:10 Yoshihiro Kaneko
  2014-10-31 13:40 ` Sergei Shtylyov
  0 siblings, 1 reply; 3+ messages in thread
From: Yoshihiro Kaneko @ 2014-10-31  9:10 UTC (permalink / raw)
  To: linux-media; +Cc: Guennadi Liakhovetski, Simon Horman, Magnus Damm, linux-sh

From: Koji Matsuoka <koji.matsuoka.xm@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.xm@renesas.com>
Signed-off-by: Yoshihiro Kaneko <ykaneko0929@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


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] media: soc_camera: rcar_vin: Fix alignment of clipping size
  2014-10-31  9:10 [PATCH] media: soc_camera: rcar_vin: Fix alignment of clipping size Yoshihiro Kaneko
@ 2014-10-31 13:40 ` Sergei Shtylyov
  2014-11-20  1:45   ` Simon Horman
  0 siblings, 1 reply; 3+ messages in thread
From: Sergei Shtylyov @ 2014-10-31 13:40 UTC (permalink / raw)
  To: Yoshihiro Kaneko, linux-media
  Cc: Guennadi Liakhovetski, Simon Horman, Magnus Damm, linux-sh

Hello.

On 10/31/2014 12:10 PM, Yoshihiro Kaneko wrote:

> From: Koji Matsuoka <koji.matsuoka.xm@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.xm@renesas.com>
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@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


^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] media: soc_camera: rcar_vin: Fix alignment of clipping size
  2014-10-31 13:40 ` Sergei Shtylyov
@ 2014-11-20  1:45   ` Simon Horman
  0 siblings, 0 replies; 3+ messages in thread
From: Simon Horman @ 2014-11-20  1:45 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Yoshihiro Kaneko, linux-media, Guennadi Liakhovetski,
	Magnus Damm, linux-sh

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.xm@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.xm@renesas.com>
> >Signed-off-by: Yoshihiro Kaneko <ykaneko0929@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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-11-20  1:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-31  9:10 [PATCH] media: soc_camera: rcar_vin: Fix alignment of clipping size Yoshihiro Kaneko
2014-10-31 13:40 ` Sergei Shtylyov
2014-11-20  1:45   ` Simon Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).