linux-kernel - Re: [PATCH RFC v2] media: OF: add sync-on-green endpoint property

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 24 May 2013 13:11:35 +0200
From:	Sylwester Nawrocki <sylvester.nawrocki@...il.com>
To:	Lad Prabhakar <prabhakar.csengg@...il.com>
CC:	LMML <linux-media@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	DLOS <davinci-linux-open-source@...ux.davincidsp.com>,
	Mauro Carvalho Chehab <mchehab@...hat.com>,
	Hans Verkuil <hans.verkuil@...co.com>,
	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	Guennadi Liakhovetski <g.liakhovetski@....de>,
	Sakari Ailus <sakari.ailus@....fi>,
	Grant Likely <grant.likely@...retlab.ca>,
	Rob Herring <rob.herring@...xeda.com>,
	Rob Landley <rob@...dley.net>,
	devicetree-discuss@...ts.ozlabs.org, linux-doc@...r.kernel.org,
	Kyungmin Park <kyungmin.park@...sung.com>
Subject: Re: [PATCH RFC v2] media: OF: add sync-on-green endpoint property

Prabhakar,

On 05/16/2013 03:18 PM, Lad Prabhakar wrote:
> From: Lad, Prabhakar <prabhakar.csengg@...il.com>
>
> This patch adds "sync-on-green" property as part of
> endpoint properties and also support to parse them in the parser.

> --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> @@ -101,6 +101,8 @@ Optional endpoint properties
>     array contains only one entry.
>   - clock-noncontinuous: a boolean property to allow MIPI CSI-2 non-continuous
>     clock mode.
> +-sync-on-green: a boolean property indicating to sync with the green signal in
> + RGB.

Are you sure this is what you need for the TVP7002 chip ?

I think we should differentiate between analog and digital signals and 
the related
device's configuration. AFAIU for the analog part there can be various video
sychronisation methods, i.e. ways in which the synchronisation signals are
transmitted along side the video component (RGB or luma/chroma) signals. 
According
to [1] (presumably not most reliable source of information) there are 
following
methods of transmitting sync signals:

  - Separate sync
  - Composite sync
  - Sync-on-green (SOG)
  - Sync-on-luminance
  - Sync-on-composite

And all these seem to refer to analog video signal.

 From looking at Figure 8 "TVP7002 Application Example" in the TVP7002's 
datasheet
([2], p. 52) and your initial TVP7002 patches it looks like what you 
want is to
specify polarity of the SOGOUT signal, so the processor that receives 
this signal
can properly interpret it, is it correct ?

If so then wouldn't it be more appropriate to define e.g. 'sog-active' 
property
and media bus flags:
	V4L2_MBUS_SYNC_ON_GREEN_ACTIVE_LOW
	V4L2_MBUS_SYNC_ON_GREEN_ACTIVE_HIGH
?

And for synchronisation method on the analog part we could perhaps define
'component-sync' or similar property that would enumerate all possible
synchronisation methods. We might as well use separate boolean properties,
but I'm a bit concerned about the increasing number of properties that need
to be parsed for each parallel video bus "endpoint".

Anyway I'm not sure if we would already have use cases for the 
'component-sync'
property.

[1] http://en.wikipedia.org/wiki/Component_video_sync
[2] 
http://www.ti.com/general/docs/lit/getliterature.tsp?genericPartNumber=tvp7002&fileType=pdf

>   Example
> diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
> index aa59639..b51d61f 100644
> --- a/drivers/media/v4l2-core/v4l2-of.c
> +++ b/drivers/media/v4l2-core/v4l2-of.c
> @@ -100,6 +100,9 @@ static void v4l2_of_parse_parallel_bus(const struct device_node *node,
>   	if (!of_property_read_u32(node, "data-shift",&v))
>   		bus->data_shift = v;
>
> +	if (of_get_property(node, "sync-on-green",&v))
> +		flags |= V4L2_MBUS_SYNC_ON_GREEN;
> +
>   	bus->flags = flags;
>
>   }
> diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
> index 83ae07e..cf2c66f9 100644
> --- a/include/media/v4l2-mediabus.h
> +++ b/include/media/v4l2-mediabus.h
> @@ -40,6 +40,7 @@
>   #define V4L2_MBUS_FIELD_EVEN_HIGH		(1<<  10)
>   /* FIELD = 1/0 - Field1 (odd)/Field2 (even) */
>   #define V4L2_MBUS_FIELD_EVEN_LOW		(1<<  11)
> +#define V4L2_MBUS_SYNC_ON_GREEN		(1<<  12)

--
Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/