[PATCH V2] media: soc_camera: rcar_vin: Add preliminary R-Car M2 support

[PATCH V2] media: soc_camera: rcar_vin: Add preliminary R-Car M2 support

[Valentine Barshak]

This adds R-Car M2 (R8A7791) VIN support. Both H2 and M2
variants look the same from the driver's point of view,
so use GEN2 id for both.

Changes in V2:
* Used the same (RCAR_GEN2) id for both H2 and M2 variants
  since they are no different from the driver's point of view.

Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
---
 drivers/media/platform/soc_camera/rcar_vin.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
index 6866bb4..3b1c05a 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -106,7 +106,7 @@
 #define VIN_MAX_HEIGHT		2048
 
 enum chip_id {
-	RCAR_H2,
+	RCAR_GEN2,
 	RCAR_H1,
 	RCAR_M1,
 	RCAR_E1,
@@ -302,7 +302,7 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
 		dmr = 0;
 		break;
 	case V4L2_PIX_FMT_RGB32:
-		if (priv->chip = RCAR_H2 || priv->chip = RCAR_H1 ||
+		if (priv->chip = RCAR_GEN2 || priv->chip = RCAR_H1 ||
 		    priv->chip = RCAR_E1) {
 			dmr = VNDMR_EXRGB;
 			break;
@@ -1384,7 +1384,8 @@ static struct soc_camera_host_ops rcar_vin_host_ops = {
 };
 
 static struct platform_device_id rcar_vin_id_table[] = {
-	{ "r8a7790-vin",  RCAR_H2 },
+	{ "r8a7791-vin",  RCAR_GEN2 },
+	{ "r8a7790-vin",  RCAR_GEN2 },
 	{ "r8a7779-vin",  RCAR_H1 },
 	{ "r8a7778-vin",  RCAR_M1 },
 	{ "uPD35004-vin", RCAR_E1 },
-- 
1.8.3.1

Re: [PATCH V2] media: soc_camera: rcar_vin: Add preliminary R-Car M2 support

[Laurent Pinchart]

Hi Valentine,

Thank you for the patch.

On Thursday 26 December 2013 19:31:49 Valentine Barshak wrote:
> This adds R-Car M2 (R8A7791) VIN support. Both H2 and M2
> variants look the same from the driver's point of view,
> so use GEN2 id for both.
> 
> Changes in V2:
> * Used the same (RCAR_GEN2) id for both H2 and M2 variants
>   since they are no different from the driver's point of view.
> 
> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/platform/soc_camera/rcar_vin.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/soc_camera/rcar_vin.c
> b/drivers/media/platform/soc_camera/rcar_vin.c index 6866bb4..3b1c05a
> 100644
> --- a/drivers/media/platform/soc_camera/rcar_vin.c
> +++ b/drivers/media/platform/soc_camera/rcar_vin.c
> @@ -106,7 +106,7 @@
>  #define VIN_MAX_HEIGHT		2048
> 
>  enum chip_id {
> -	RCAR_H2,
> +	RCAR_GEN2,
>  	RCAR_H1,
>  	RCAR_M1,
>  	RCAR_E1,
> @@ -302,7 +302,7 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
>  		dmr = 0;
>  		break;
>  	case V4L2_PIX_FMT_RGB32:
> -		if (priv->chip = RCAR_H2 || priv->chip = RCAR_H1 ||
> +		if (priv->chip = RCAR_GEN2 || priv->chip = RCAR_H1 ||
>  		    priv->chip = RCAR_E1) {
>  			dmr = VNDMR_EXRGB;
>  			break;
> @@ -1384,7 +1384,8 @@ static struct soc_camera_host_ops rcar_vin_host_ops > { };
> 
>  static struct platform_device_id rcar_vin_id_table[] = {
> -	{ "r8a7790-vin",  RCAR_H2 },
> +	{ "r8a7791-vin",  RCAR_GEN2 },
> +	{ "r8a7790-vin",  RCAR_GEN2 },
>  	{ "r8a7779-vin",  RCAR_H1 },
>  	{ "r8a7778-vin",  RCAR_M1 },
>  	{ "uPD35004-vin", RCAR_E1 },
-- 
Regards,

Laurent Pinchart

[PATCH v2] media: soc_camera: rcar_vin: Add r8a7794, r8a7793 device support

[Yoshihiro Kaneko]

From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>

Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
Acked-by: Simon Horman <horms+renesas@verge.net.au>

---

This patch is against master branch of linuxtv.org/media_tree.git.

v2 [Yoshihiro Kaneko]
* Squashed r8a7793 and r8a7794 patches

 drivers/media/platform/soc_camera/rcar_vin.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
index 234cf86..4acae8f 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -1881,6 +1881,8 @@ MODULE_DEVICE_TABLE(of, rcar_vin_of_table);
 #endif
 
 static struct platform_device_id rcar_vin_id_table[] = {
+	{ "r8a7794-vin",  RCAR_GEN2 },
+	{ "r8a7793-vin",  RCAR_GEN2 },
 	{ "r8a7791-vin",  RCAR_GEN2 },
 	{ "r8a7790-vin",  RCAR_GEN2 },
 	{ "r8a7779-vin",  RCAR_H1 },
-- 
1.9.1

Re: [PATCH v2] media: soc_camera: rcar_vin: Add r8a7794, r8a7793 device support

[Laurent Pinchart]

Hi Kaneko-san,

Thank you for the patch.

Could you please also update 
Documentation/devicetree/bindings/media/rcar_vin.txt with the new compatible 
strings ?

On Friday 17 October 2014 16:07:39 Yoshihiro Kaneko wrote:
> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> 
> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> Acked-by: Simon Horman <horms+renesas@verge.net.au>
> 
> ---
> 
> This patch is against master branch of linuxtv.org/media_tree.git.
> 
> v2 [Yoshihiro Kaneko]
> * Squashed r8a7793 and r8a7794 patches
> 
>  drivers/media/platform/soc_camera/rcar_vin.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/media/platform/soc_camera/rcar_vin.c
> b/drivers/media/platform/soc_camera/rcar_vin.c index 234cf86..4acae8f
> 100644
> --- a/drivers/media/platform/soc_camera/rcar_vin.c
> +++ b/drivers/media/platform/soc_camera/rcar_vin.c
> @@ -1881,6 +1881,8 @@ MODULE_DEVICE_TABLE(of, rcar_vin_of_table);
>  #endif
> 
>  static struct platform_device_id rcar_vin_id_table[] = {
> +	{ "r8a7794-vin",  RCAR_GEN2 },
> +	{ "r8a7793-vin",  RCAR_GEN2 },
>  	{ "r8a7791-vin",  RCAR_GEN2 },
>  	{ "r8a7790-vin",  RCAR_GEN2 },
>  	{ "r8a7779-vin",  RCAR_H1 },

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] media: soc_camera: rcar_vin: Add r8a7794, r8a7793 device support

[Simon Horman]

On Fri, Oct 17, 2014 at 11:01:07AM +0300, Laurent Pinchart wrote:
> Hi Kaneko-san,
> 
> Thank you for the patch.
> 
> Could you please also update 
> Documentation/devicetree/bindings/media/rcar_vin.txt with the new compatible 
> strings ?

Hi Laurent,

thanks for pointing that out. It is true that we want DT support for the
new SoCs for this driver and in that case updating the bindings
documentation would be necessary.  However, this patch adds platform device
support.

What I suggest is dropping this patch for now and working on
a replacement that adds DT support only. I do not believe there
are any plans to use a platform device in mainline for this driver on the
new SoCs.

> On Friday 17 October 2014 16:07:39 Yoshihiro Kaneko wrote:
> > From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> > 
> > Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> > Acked-by: Simon Horman <horms+renesas@verge.net.au>
> > 
> > ---
> > 
> > This patch is against master branch of linuxtv.org/media_tree.git.
> > 
> > v2 [Yoshihiro Kaneko]
> > * Squashed r8a7793 and r8a7794 patches
> > 
> >  drivers/media/platform/soc_camera/rcar_vin.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/media/platform/soc_camera/rcar_vin.c
> > b/drivers/media/platform/soc_camera/rcar_vin.c index 234cf86..4acae8f
> > 100644
> > --- a/drivers/media/platform/soc_camera/rcar_vin.c
> > +++ b/drivers/media/platform/soc_camera/rcar_vin.c
> > @@ -1881,6 +1881,8 @@ MODULE_DEVICE_TABLE(of, rcar_vin_of_table);
> >  #endif
> > 
> >  static struct platform_device_id rcar_vin_id_table[] = {
> > +	{ "r8a7794-vin",  RCAR_GEN2 },
> > +	{ "r8a7793-vin",  RCAR_GEN2 },
> >  	{ "r8a7791-vin",  RCAR_GEN2 },
> >  	{ "r8a7790-vin",  RCAR_GEN2 },
> >  	{ "r8a7779-vin",  RCAR_H1 },
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> --
> 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
> 

[PATCH v2] media: soc_camera: rcar_vin: Add BT.709 24-bit RGB888 input support

[Yoshihiro Kaneko]

From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>

Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
---

This patch is against master branch of linuxtv.org/media_tree.git.

v2 [Yoshihiro Kaneko]
* remove unused/useless definition as suggested by Sergei Shtylyov

 drivers/media/platform/soc_camera/rcar_vin.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
index 20defcb..cb5e682 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -74,6 +74,7 @@
 #define VNMC_INF_YUV10_BT656	(2 << 16)
 #define VNMC_INF_YUV10_BT601	(3 << 16)
 #define VNMC_INF_YUV16		(5 << 16)
+#define VNMC_INF_RGB888		(6 << 16)
 #define VNMC_VUP		(1 << 10)
 #define VNMC_IM_ODD		(0 << 3)
 #define VNMC_IM_ODD_EVEN	(1 << 3)
@@ -272,6 +273,10 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
 
 	/* input interface */
 	switch (icd->current_fmt->code) {
+	case V4L2_MBUS_FMT_RGB888_1X24:
+		/* BT.601/BT.709 24-bit RGB-888 */
+		vnmc |= VNMC_INF_RGB888;
+		break;
 	case V4L2_MBUS_FMT_YUYV8_1X16:
 		/* BT.601/BT.1358 16bit YCbCr422 */
 		vnmc |= VNMC_INF_YUV16;
@@ -331,6 +336,9 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
 	if (output_is_yuv)
 		vnmc |= VNMC_BPS;
 
+	if (vnmc & VNMC_INF_RGB888)
+		vnmc ^= VNMC_BPS;
+
 	/* progressive or interlaced mode */
 	interrupts = progressive ? VNIE_FIE | VNIE_EFE : VNIE_EFE;
 
@@ -1013,6 +1021,7 @@ static int rcar_vin_get_formats(struct soc_camera_device *icd, unsigned int idx,
 	case V4L2_MBUS_FMT_YUYV8_1X16:
 	case V4L2_MBUS_FMT_YUYV8_2X8:
 	case V4L2_MBUS_FMT_YUYV10_2X10:
+	case V4L2_MBUS_FMT_RGB888_1X24:
 		if (cam->extra_fmt)
 			break;
 
-- 
1.9.1

Re: [PATCH v2] media: soc_camera: rcar_vin: Add BT.709 24-bit RGB888 input support

[Sergei Shtylyov]

Hello.

On 10/21/2014 9:08 AM, Yoshihiro Kaneko wrote:

> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>

> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> ---

> This patch is against master branch of linuxtv.org/media_tree.git.

> v2 [Yoshihiro Kaneko]
> * remove unused/useless definition as suggested by Sergei Shtylyov

    I didn't say it's useless, I just suspected that you missed the necessary 
test somewhere...

>   drivers/media/platform/soc_camera/rcar_vin.c | 9 +++++++++
>   1 file changed, 9 insertions(+)

> diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
> index 20defcb..cb5e682 100644
> --- a/drivers/media/platform/soc_camera/rcar_vin.c
> +++ b/drivers/media/platform/soc_camera/rcar_vin.c
> @@ -74,6 +74,7 @@
>   #define VNMC_INF_YUV10_BT656	(2 << 16)
>   #define VNMC_INF_YUV10_BT601	(3 << 16)
>   #define VNMC_INF_YUV16		(5 << 16)
> +#define VNMC_INF_RGB888		(6 << 16)
>   #define VNMC_VUP		(1 << 10)
>   #define VNMC_IM_ODD		(0 << 3)
>   #define VNMC_IM_ODD_EVEN	(1 << 3)
[...]
> @@ -331,6 +336,9 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
>   	if (output_is_yuv)
>   		vnmc |= VNMC_BPS;
>
> +	if (vnmc & VNMC_INF_RGB888)
> +		vnmc ^= VNMC_BPS;
> +

    Hm, this also changes the behavior for VNMC_INF_YUV16 and 
VNMC_INF_YUV10_BT{601|656}. Is this actually intended?

[...]

WBR, Sergei

Re: [PATCH v2] media: soc_camera: rcar_vin: Add BT.709 24-bit RGB888 input support

[Yoshihiro Kaneko]

Hello Sergei,

2014-10-21 19:22 GMT+09:00 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>:
> Hello.
>
> On 10/21/2014 9:08 AM, Yoshihiro Kaneko wrote:
>
>> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
>
>
>> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
>> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
>> ---
>
>
>> This patch is against master branch of linuxtv.org/media_tree.git.
>
>
>> v2 [Yoshihiro Kaneko]
>> * remove unused/useless definition as suggested by Sergei Shtylyov
>
>
>    I didn't say it's useless, I just suspected that you missed the necessary
> test somewhere...

Sorry for my inaccurate description.

>
>>   drivers/media/platform/soc_camera/rcar_vin.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>
>
>> diff --git a/drivers/media/platform/soc_camera/rcar_vin.c
>> b/drivers/media/platform/soc_camera/rcar_vin.c
>> index 20defcb..cb5e682 100644
>> --- a/drivers/media/platform/soc_camera/rcar_vin.c
>> +++ b/drivers/media/platform/soc_camera/rcar_vin.c
>> @@ -74,6 +74,7 @@
>>   #define VNMC_INF_YUV10_BT656  (2 << 16)
>>   #define VNMC_INF_YUV10_BT601  (3 << 16)
>>   #define VNMC_INF_YUV16                (5 << 16)
>> +#define VNMC_INF_RGB888                (6 << 16)
>>   #define VNMC_VUP              (1 << 10)
>>   #define VNMC_IM_ODD           (0 << 3)
>>   #define VNMC_IM_ODD_EVEN      (1 << 3)
>
> [...]
>>
>> @@ -331,6 +336,9 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
>>         if (output_is_yuv)
>>                 vnmc |= VNMC_BPS;
>>
>> +       if (vnmc & VNMC_INF_RGB888)
>> +               vnmc ^= VNMC_BPS;
>> +
>
>
>    Hm, this also changes the behavior for VNMC_INF_YUV16 and
> VNMC_INF_YUV10_BT{601|656}. Is this actually intended?

Probably this code is incorrect.
Thank you for your review.

Thanks,
Kaneko

>
> [...]
>
> WBR, Sergei
>

Re: [PATCH v2] media: soc_camera: rcar_vin: Add BT.709 24-bit RGB888 input support

[Simon Horman]

Hi Kaneko-san, Hi Sergei,

On Tue, Oct 21, 2014 at 08:33:52PM +0900, Yoshihiro Kaneko wrote:
> Hello Sergei,
> 
> 2014-10-21 19:22 GMT+09:00 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>:
> > Hello.
> >
> > On 10/21/2014 9:08 AM, Yoshihiro Kaneko wrote:
> >
> >> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> >
> >
> >> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> >> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> >> ---
> >
> >
> >> This patch is against master branch of linuxtv.org/media_tree.git.
> >
> >
> >> v2 [Yoshihiro Kaneko]
> >> * remove unused/useless definition as suggested by Sergei Shtylyov
> >
> >
> >    I didn't say it's useless, I just suspected that you missed the necessary
> > test somewhere...
> 
> Sorry for my inaccurate description.
> 
> >
> >>   drivers/media/platform/soc_camera/rcar_vin.c | 9 +++++++++
> >>   1 file changed, 9 insertions(+)
> >
> >
> >> diff --git a/drivers/media/platform/soc_camera/rcar_vin.c
> >> b/drivers/media/platform/soc_camera/rcar_vin.c
> >> index 20defcb..cb5e682 100644
> >> --- a/drivers/media/platform/soc_camera/rcar_vin.c
> >> +++ b/drivers/media/platform/soc_camera/rcar_vin.c
> >> @@ -74,6 +74,7 @@
> >>   #define VNMC_INF_YUV10_BT656  (2 << 16)
> >>   #define VNMC_INF_YUV10_BT601  (3 << 16)
> >>   #define VNMC_INF_YUV16                (5 << 16)
> >> +#define VNMC_INF_RGB888                (6 << 16)
> >>   #define VNMC_VUP              (1 << 10)
> >>   #define VNMC_IM_ODD           (0 << 3)
> >>   #define VNMC_IM_ODD_EVEN      (1 << 3)
> >
> > [...]
> >>
> >> @@ -331,6 +336,9 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
> >>         if (output_is_yuv)
> >>                 vnmc |= VNMC_BPS;
> >>
> >> +       if (vnmc & VNMC_INF_RGB888)
> >> +               vnmc ^= VNMC_BPS;
> >> +
> >
> >
> >    Hm, this also changes the behavior for VNMC_INF_YUV16 and
> > VNMC_INF_YUV10_BT{601|656}. Is this actually intended?
> 
> Probably this code is incorrect.
> Thank you for your review.

Thanks, I have confirmed with Matsuoka-san that there is a problem here.

He has provided the following fix. Could you see about squashing it into
the above patch and reposting?


From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>

[PATCH] media: soc_camera: rcar_vin: Fix bit field check

Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>

diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
index 013d75c..da62d94 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -94,7 +94,7 @@
 #define VNMC_INF_YUV8_BT601	(1 << 16)
 #define VNMC_INF_YUV16		(5 << 16)
 #define VNMC_INF_RGB888		(6 << 16)
-#define VNMC_INF_RGB_MASK	(6 << 16)
+#define VNMC_INF_MASK		(7 << 16)
 #define VNMC_VUP		(1 << 10)
 #define VNMC_IM_ODD		(0 << 3)
 #define VNMC_IM_ODD_EVEN	(1 << 3)
@@ -675,7 +675,7 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
 	if (output_is_yuv)
 		vnmc |= VNMC_BPS;
 
-	if (vnmc & VNMC_INF_RGB_MASK)
+	if ((vnmc & VNMC_INF_MASK) = VNMC_INF_RGB888)
 		vnmc ^= VNMC_BPS;
 
 	/* progressive or interlaced mode */

Re: [PATCH v2] media: soc_camera: rcar_vin: Add BT.709 24-bit RGB888 input support

[Sergei Shtylyov]

Hello.

On 10/29/2014 7:11 AM, Simon Horman wrote:

>>>> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>

>>>> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
>>>> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
>>>> ---

>>>> This patch is against master branch of linuxtv.org/media_tree.git.

>>>> v2 [Yoshihiro Kaneko]
>>>> * remove unused/useless definition as suggested by Sergei Shtylyov

>>>     I didn't say it's useless, I just suspected that you missed the necessary
>>> test somewhere...

>> Sorry for my inaccurate description.

>>>>    drivers/media/platform/soc_camera/rcar_vin.c | 9 +++++++++
>>>>    1 file changed, 9 insertions(+)

>>>> diff --git a/drivers/media/platform/soc_camera/rcar_vin.c
>>>> b/drivers/media/platform/soc_camera/rcar_vin.c
>>>> index 20defcb..cb5e682 100644
>>>> --- a/drivers/media/platform/soc_camera/rcar_vin.c
>>>> +++ b/drivers/media/platform/soc_camera/rcar_vin.c
>>>> @@ -74,6 +74,7 @@
>>>>    #define VNMC_INF_YUV10_BT656  (2 << 16)
>>>>    #define VNMC_INF_YUV10_BT601  (3 << 16)
>>>>    #define VNMC_INF_YUV16                (5 << 16)
>>>> +#define VNMC_INF_RGB888                (6 << 16)
>>>>    #define VNMC_VUP              (1 << 10)
>>>>    #define VNMC_IM_ODD           (0 << 3)
>>>>    #define VNMC_IM_ODD_EVEN      (1 << 3)

>>> [...]

>>>> @@ -331,6 +336,9 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
>>>>          if (output_is_yuv)
>>>>                  vnmc |= VNMC_BPS;
>>>>
>>>> +       if (vnmc & VNMC_INF_RGB888)
>>>> +               vnmc ^= VNMC_BPS;
>>>> +

>>>     Hm, this also changes the behavior for VNMC_INF_YUV16 and
>>> VNMC_INF_YUV10_BT{601|656}. Is this actually intended?

>> Probably this code is incorrect.
>> Thank you for your review.

> Thanks, I have confirmed with Matsuoka-san that there is a problem here.

> He has provided the following fix. Could you see about squashing it into
> the above patch and reposting?

> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>

> [PATCH] media: soc_camera: rcar_vin: Fix bit field check

> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>

> diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
> index 013d75c..da62d94 100644
> --- a/drivers/media/platform/soc_camera/rcar_vin.c
> +++ b/drivers/media/platform/soc_camera/rcar_vin.c
> @@ -94,7 +94,7 @@
>   #define VNMC_INF_YUV8_BT601	(1 << 16)
>   #define VNMC_INF_YUV16		(5 << 16)
>   #define VNMC_INF_RGB888		(6 << 16)
> -#define VNMC_INF_RGB_MASK	(6 << 16)
> +#define VNMC_INF_MASK		(7 << 16)

    #define it above VNMC_INF_YUV8_BT656 please.

>   #define VNMC_VUP		(1 << 10)
>   #define VNMC_IM_ODD		(0 << 3)
>   #define VNMC_IM_ODD_EVEN	(1 << 3)
> @@ -675,7 +675,7 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
>   	if (output_is_yuv)
>   		vnmc |= VNMC_BPS;
>
> -	if (vnmc & VNMC_INF_RGB_MASK)
> +	if ((vnmc & VNMC_INF_MASK) = VNMC_INF_RGB888)

    Is he sure it shouldn't be (vnmc & VNMC_INF_RGB888) = VNMC_INF_RGB888 to 
also cover 16-bit RGB666 and 12-bit RGB88?

WBR, Sergei

Re: [PATCH v2] media: soc_camera: rcar_vin: Add BT.709 24-bit RGB888 input support

[Simon Horman]

On Wed, Oct 29, 2014 at 02:31:41PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 10/29/2014 7:11 AM, Simon Horman wrote:
> 
> >>>>From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> 
> >>>>Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> >>>>Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> >>>>---
> 
> >>>>This patch is against master branch of linuxtv.org/media_tree.git.
> 
> >>>>v2 [Yoshihiro Kaneko]
> >>>>* remove unused/useless definition as suggested by Sergei Shtylyov
> 
> >>>    I didn't say it's useless, I just suspected that you missed the necessary
> >>>test somewhere...
> 
> >>Sorry for my inaccurate description.
> 
> >>>>   drivers/media/platform/soc_camera/rcar_vin.c | 9 +++++++++
> >>>>   1 file changed, 9 insertions(+)
> 
> >>>>diff --git a/drivers/media/platform/soc_camera/rcar_vin.c
> >>>>b/drivers/media/platform/soc_camera/rcar_vin.c
> >>>>index 20defcb..cb5e682 100644
> >>>>--- a/drivers/media/platform/soc_camera/rcar_vin.c
> >>>>+++ b/drivers/media/platform/soc_camera/rcar_vin.c
> >>>>@@ -74,6 +74,7 @@
> >>>>   #define VNMC_INF_YUV10_BT656  (2 << 16)
> >>>>   #define VNMC_INF_YUV10_BT601  (3 << 16)
> >>>>   #define VNMC_INF_YUV16                (5 << 16)
> >>>>+#define VNMC_INF_RGB888                (6 << 16)
> >>>>   #define VNMC_VUP              (1 << 10)
> >>>>   #define VNMC_IM_ODD           (0 << 3)
> >>>>   #define VNMC_IM_ODD_EVEN      (1 << 3)
> 
> >>>[...]
> 
> >>>>@@ -331,6 +336,9 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
> >>>>         if (output_is_yuv)
> >>>>                 vnmc |= VNMC_BPS;
> >>>>
> >>>>+       if (vnmc & VNMC_INF_RGB888)
> >>>>+               vnmc ^= VNMC_BPS;
> >>>>+
> 
> >>>    Hm, this also changes the behavior for VNMC_INF_YUV16 and
> >>>VNMC_INF_YUV10_BT{601|656}. Is this actually intended?
> 
> >>Probably this code is incorrect.
> >>Thank you for your review.
> 
> >Thanks, I have confirmed with Matsuoka-san that there is a problem here.
> 
> >He has provided the following fix. Could you see about squashing it into
> >the above patch and reposting?
> 
> >From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> 
> >[PATCH] media: soc_camera: rcar_vin: Fix bit field check
> 
> >Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> 
> >diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
> >index 013d75c..da62d94 100644
> >--- a/drivers/media/platform/soc_camera/rcar_vin.c
> >+++ b/drivers/media/platform/soc_camera/rcar_vin.c
> >@@ -94,7 +94,7 @@
> >  #define VNMC_INF_YUV8_BT601	(1 << 16)
> >  #define VNMC_INF_YUV16		(5 << 16)
> >  #define VNMC_INF_RGB888		(6 << 16)
> >-#define VNMC_INF_RGB_MASK	(6 << 16)
> >+#define VNMC_INF_MASK		(7 << 16)
> 
>    #define it above VNMC_INF_YUV8_BT656 please.
> 
> >  #define VNMC_VUP		(1 << 10)
> >  #define VNMC_IM_ODD		(0 << 3)
> >  #define VNMC_IM_ODD_EVEN	(1 << 3)
> >@@ -675,7 +675,7 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
> >  	if (output_is_yuv)
> >  		vnmc |= VNMC_BPS;
> >
> >-	if (vnmc & VNMC_INF_RGB_MASK)
> >+	if ((vnmc & VNMC_INF_MASK) = VNMC_INF_RGB888)
> 
>    Is he sure it shouldn't be (vnmc & VNMC_INF_RGB888) = VNMC_INF_RGB888 to
> also cover 16-bit RGB666 and 12-bit RGB88?

Nice, I think that is a good idea (although the latter formats aren't
supported by the driver yet, right?).

Its somewhat unobvious how that logic works so perhaps we should add a
comment like this

	/* If input and output use the same colorspace, use bypass mode */
	if (output_is_yuv)
		vnmc |= VNMC_BPS;

	/* The above assumes YUV input, toggle BPS for RGB input.
	 * RGB inputs can be detected by checking that the most-significant
	 * two bits of INF are set. This corresponds to the bits
	 * set in VNMC_INF_RGB888. */
	if ((vnmc & VNMC_INF_RGB888)) = VNMC_INF_RGB888)
		vnmc ^= VNMC_BPS;

[PATCH v2] media: soc_camera: rcar_vin: Enable VSYNC field toggle mode

[Yoshihiro Kaneko]

From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>

By applying this patch, it sets to VSYNC field toggle mode not only
at the time of progressive mode but at the time of an interlace mode.

Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
---

This patch is against master branch of linuxtv.org/media_tree.git.

v2 [Yoshihiro Kaneko]
* improve the macro definition for the VLV field

 drivers/media/platform/soc_camera/rcar_vin.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
index 9300076..beaf8e5 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -107,6 +107,7 @@
 #define VNDMR2_VPS		(1 << 30)
 #define VNDMR2_HPS		(1 << 29)
 #define VNDMR2_FTEV		(1 << 17)
+#define VNDMR2_VLV(n)		((n & 0xf) << 12)
 
 #define VIN_MAX_WIDTH		2048
 #define VIN_MAX_HEIGHT		2048
@@ -827,7 +828,7 @@ static int rcar_vin_set_bus_param(struct soc_camera_device *icd)
 	if (ret < 0 && ret != -ENOIOCTLCMD)
 		return ret;
 
-	val = priv->field = V4L2_FIELD_NONE ? VNDMR2_FTEV : 0;
+	val = VNDMR2_FTEV | VNDMR2_VLV(1);
 	if (!(common_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
 		val |= VNDMR2_VPS;
 	if (!(common_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
-- 
1.9.1

Re: [PATCH v2] media: soc_camera: rcar_vin: Enable VSYNC field toggle mode

[Simon Horman]

On Wed, Oct 22, 2014 at 01:05:36PM +0900, Yoshihiro Kaneko wrote:
> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> 
> By applying this patch, it sets to VSYNC field toggle mode not only
> at the time of progressive mode but at the time of an interlace mode.
> 
> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>

Acked-by: Simon Horman <horms+renesas@verge.net.au>

Guennadi, could you consider this patch when you get a chance?

> ---
> 
> This patch is against master branch of linuxtv.org/media_tree.git.
> 
> v2 [Yoshihiro Kaneko]
> * improve the macro definition for the VLV field
> 
>  drivers/media/platform/soc_camera/rcar_vin.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
> index 9300076..beaf8e5 100644
> --- a/drivers/media/platform/soc_camera/rcar_vin.c
> +++ b/drivers/media/platform/soc_camera/rcar_vin.c
> @@ -107,6 +107,7 @@
>  #define VNDMR2_VPS		(1 << 30)
>  #define VNDMR2_HPS		(1 << 29)
>  #define VNDMR2_FTEV		(1 << 17)
> +#define VNDMR2_VLV(n)		((n & 0xf) << 12)
>  
>  #define VIN_MAX_WIDTH		2048
>  #define VIN_MAX_HEIGHT		2048
> @@ -827,7 +828,7 @@ static int rcar_vin_set_bus_param(struct soc_camera_device *icd)
>  	if (ret < 0 && ret != -ENOIOCTLCMD)
>  		return ret;
>  
> -	val = priv->field = V4L2_FIELD_NONE ? VNDMR2_FTEV : 0;
> +	val = VNDMR2_FTEV | VNDMR2_VLV(1);
>  	if (!(common_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
>  		val |= VNDMR2_VPS;
>  	if (!(common_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
> -- 
> 1.9.1
> 

[PATCH v2] media: soc_camera: rcar_vin: Fix wait_for_completion

[Yoshihiro Kaneko]

From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>

When stopping abnormally, a driver can't return from wait_for_completion.
This patch resolved this problem by changing wait_for_completion_timeout
from wait_for_completion.

Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
---

This patch is against master branch of linuxtv.org/media_tree.git.

v2 [Yoshihiro Kaneko]
* remove the original line that I forgot.
* fix an indent to make it easy to read.

 drivers/media/platform/soc_camera/rcar_vin.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
index 279ab9f..e55d7ba 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -135,6 +135,8 @@
 #define VIN_MAX_WIDTH		2048
 #define VIN_MAX_HEIGHT		2048
 
+#define TIMEOUT_MS		100
+
 enum chip_id {
 	RCAR_GEN2,
 	RCAR_H1,
@@ -820,7 +822,10 @@ static void rcar_vin_wait_stop_streaming(struct rcar_vin_priv *priv)
 		if (priv->state = STOPPING) {
 			priv->request_to_stop = true;
 			spin_unlock_irq(&priv->lock);
-			wait_for_completion(&priv->capture_stop);
+			if (!wait_for_completion_timeout(
+					&priv->capture_stop,
+					msecs_to_jiffies(TIMEOUT_MS)))
+				priv->state = STOPPED;
 			spin_lock_irq(&priv->lock);
 		}
 	}
-- 
1.9.1