Date:	Fri, 31 May 2013 12:31:33 +0200
From:	Sylwester Nawrocki <s.nawrocki@...sung.com>
To:	Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc:	Prabhakar Lad <prabhakar.csengg@...il.com>,
	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>,
	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

On 05/30/2013 05:21 AM, Laurent Pinchart wrote:
> Hi Sylwester,
> 
> On Saturday 25 May 2013 16:11:52 Sylwester Nawrocki wrote:
>> On 05/25/2013 11:17 AM, Prabhakar Lad wrote:
[...]
>>>>>  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.
> 
> I prefer 1. over 2.
> 
>> 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 ?
> 
> My gut feeling is that we should have separate properties for the video sync 
> type and the synchronization signals polarities. We could have a chip that 
> supports sync-on-green on the analog (input) side and outputs separate hsync 
> and vsync signals only on the digital (output) side. There would be no sync-
> on-green polarity in that case.

Yes, agreed. I've had some doubts that using single DT property for defining
really 2 distinct H/W properties like this might not be flexible enough.
The option 1. seems most correct then.

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/

Powered by blists - more mailing lists