Discussion:
[PATCH] media: soc_camera: rcar_vin: Enable VSYNC field toggle mode
(too old to reply)
Yoshihiro Kaneko
2014-10-14 06:25:56 UTC
Permalink
Raw Message
From: Koji Matsuoka <***@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 <***@renesas.com>
Signed-off-by: Yoshihiro Kaneko <***@gmail.com>
---

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

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 5196c81..bf97ed6 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -108,6 +108,7 @@
#define VNDMR2_VPS (1 << 30)
#define VNDMR2_HPS (1 << 29)
#define VNDMR2_FTEV (1 << 17)
+#define VNDMR2_VLV_1 (1 << 12)

#define VIN_MAX_WIDTH 2048
#define VIN_MAX_HEIGHT 2048
@@ -828,7 +829,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

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Simon Horman
2014-10-16 05:25:32 UTC
Permalink
Raw Message
[CC Mauro Carvalho Chehab]

On Tue, Oct 14, 2014 at 03:25:56PM +0900, Yoshihiro Kaneko wrote:
> From: Koji Matsuoka <***@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 <***@renesas.com>
> Signed-off-by: Yoshihiro Kaneko <***@gmail.com>

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

If the series needs reposting to a different CC list -
e.g. including Mauro - please let Kaneko-san or myself know.

> ---
>
> This patch is against master branch of linuxtv.org/media_tree.git.
>
> 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 5196c81..bf97ed6 100644
> --- a/drivers/media/platform/soc_camera/rcar_vin.c
> +++ b/drivers/media/platform/soc_camera/rcar_vin.c
> @@ -108,6 +108,7 @@
> #define VNDMR2_VPS (1 << 30)
> #define VNDMR2_HPS (1 << 29)
> #define VNDMR2_FTEV (1 << 17)
> +#define VNDMR2_VLV_1 (1 << 12)
>
> #define VIN_MAX_WIDTH 2048
> #define VIN_MAX_HEIGHT 2048
> @@ -828,7 +829,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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to ***@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Sergei Shtylyov
2014-10-18 15:01:56 UTC
Permalink
Raw Message
Hello.

On 10/14/2014 10:25 AM, Yoshihiro Kaneko wrote:

> From: Koji Matsuoka <***@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 <***@renesas.com>
> Signed-off-by: Yoshihiro Kaneko <***@gmail.com>
> ---

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

> 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 5196c81..bf97ed6 100644
> --- a/drivers/media/platform/soc_camera/rcar_vin.c
> +++ b/drivers/media/platform/soc_camera/rcar_vin.c
> @@ -108,6 +108,7 @@
> #define VNDMR2_VPS (1 << 30)
> #define VNDMR2_HPS (1 << 29)
> #define VNDMR2_FTEV (1 << 17)
> +#define VNDMR2_VLV_1 (1 << 12)

Please instead do:

#define VNDMR2_VLV(n) ((n & 0xf) << 12)

WBR, Sergei
Yoshihiro Kaneko
2014-10-21 03:30:29 UTC
Permalink
Raw Message
Hello Sergei,

Thank you for your comments.

2014-10-19 0:01 GMT+09:00 Sergei Shtylyov <***@cogentembedded.com>:
> Hello.
>
> On 10/14/2014 10:25 AM, Yoshihiro Kaneko wrote:
>
>> From: Koji Matsuoka <***@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 <***@renesas.com>
>> Signed-off-by: Yoshihiro Kaneko <***@gmail.com>
>> ---
>
>
>> This patch is against master branch of linuxtv.org/media_tree.git.
>
>
>> 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 5196c81..bf97ed6 100644
>> --- a/drivers/media/platform/soc_camera/rcar_vin.c
>> +++ b/drivers/media/platform/soc_camera/rcar_vin.c
>> @@ -108,6 +108,7 @@
>> #define VNDMR2_VPS (1 << 30)
>> #define VNDMR2_HPS (1 << 29)
>> #define VNDMR2_FTEV (1 << 17)
>> +#define VNDMR2_VLV_1 (1 << 12)
>
>
> Please instead do:
>
> #define VNDMR2_VLV(n) ((n & 0xf) << 12)

It's unclear to me why the style of the new #define should differ
from those of the existing ones.

Thanks,
Kaneko

>
> WBR, Sergei
>
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Geert Uytterhoeven
2014-10-21 07:09:43 UTC
Permalink
Raw Message
Hi Kaneko-san,

On Tue, Oct 21, 2014 at 5:30 AM, Yoshihiro Kaneko <***@gmail.com> wrote:
>>> --- a/drivers/media/platform/soc_camera/rcar_vin.c
>>> +++ b/drivers/media/platform/soc_camera/rcar_vin.c
>>> @@ -108,6 +108,7 @@
>>> #define VNDMR2_VPS (1 << 30)
>>> #define VNDMR2_HPS (1 << 29)
>>> #define VNDMR2_FTEV (1 << 17)
>>> +#define VNDMR2_VLV_1 (1 << 12)
>>
>> Please instead do:
>>
>> #define VNDMR2_VLV(n) ((n & 0xf) << 12)
>
> It's unclear to me why the style of the new #define should differ
> from those of the existing ones.

I think Sergey wants to say that unlike for the other fields, there are
multiple possible values for the VLV field.

By providing the single macro definition

#define VNDMR2_VLV(n) ((n & 0xf) << 12)

you can easily provide a way to set any of VNDMR2_VLV_n.

I hope this explanation makes it clearer.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ***@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Yoshihiro Kaneko
2014-10-22 03:17:04 UTC
Permalink
Raw Message
Hi Geert-san,

2014-10-21 16:09 GMT+09:00 Geert Uytterhoeven <***@linux-m68k.org>:
> Hi Kaneko-san,
>
> On Tue, Oct 21, 2014 at 5:30 AM, Yoshihiro Kaneko <***@gmail.com> wrote:
>>>> --- a/drivers/media/platform/soc_camera/rcar_vin.c
>>>> +++ b/drivers/media/platform/soc_camera/rcar_vin.c
>>>> @@ -108,6 +108,7 @@
>>>> #define VNDMR2_VPS (1 << 30)
>>>> #define VNDMR2_HPS (1 << 29)
>>>> #define VNDMR2_FTEV (1 << 17)
>>>> +#define VNDMR2_VLV_1 (1 << 12)
>>>
>>> Please instead do:
>>>
>>> #define VNDMR2_VLV(n) ((n & 0xf) << 12)
>>
>> It's unclear to me why the style of the new #define should differ
>> from those of the existing ones.
>
> I think Sergey wants to say that unlike for the other fields, there are
> multiple possible values for the VLV field.
>
> By providing the single macro definition
>
> #define VNDMR2_VLV(n) ((n & 0xf) << 12)
>
> you can easily provide a way to set any of VNDMR2_VLV_n.
>
> I hope this explanation makes it clearer.

Thank you for the clarification!
I'll update this patch sooner.

Thanks,
Kaneko

>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ***@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Loading...