Hi,

On 05/25/2013 11:17 AM, Prabhakar Lad wrote:

quoted

 From looking at Figure 8 "TVP7002 Application Example" in the TVP7002's

quoted

 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 ?

Yes

quoted

quoted

 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
 ?

Agreed I'll add these flags.

quoted

quoted

 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".

I am not clear on it can please elaborate more on this.

I thought about two possible options:

1. single property 'component-sync' or 'video-sync' that would have values:

#define VIDEO_SEPARATE_SYNC	0x01
#define VIDEO_COMPOSITE_SYNC	0x02
#define VIDEO_SYNC_ON_COMPOSITE	0x04
#define VIDEO_SYNC_ON_GREEN	0x08
#define VIDEO_SYNC_ON_LUMINANCE	0x10

And we could put these definitions into a separate header, e.g.
<dt-bindings/video-interfaces.h>

Then in a device tree source file one could have, e.g.

video-sync = <VIDEO_SYNC_ON_GREEN>;


2. Separate boolean property for each video sync type, e.g.

	"video-composite-sync"
	"video-sync-on-composite"
	"video-sync-on-green"
	"video-sync-on-luminance"

Separate sync, with separate VSYNC, HSYNC lines, would be the default, when
none of the above is specified and 'vsync-active', 'hsync-active' properties
are present.

However, I suppose the better would be to deduce the video synchronisation
method from the sync signal polarity flags. Then, for instance, when an
endpoint node contains "composite-sync-active" property the parser would
determine the "composite sync" synchronisation type is used.

Thus it might make sense to have only following integer properties (added
as needed):

composite-sync-active
sync-on-green-active
sync-on-comp-active
sync-on-luma-active

This would allow to specify polarity of each signal and at the same time
the parsing code could derive synchronisation type. A new field could be
added to struct v4l2_of_parallel_bus, e.g. sync_type and it would be filled
within v4l2_of_parse_endpoint().

What do you think ?


Thanks,
Sylwester