[PATCH v2 0/3] video: of: Drive edge selection for sync

[PATCH v2 0/3] video: of: Drive edge selection for sync

[Peter Ujfalusi]

Hi,

Changes since v1:
- separated the code changes from the omap/drm videomode conversion patches
- the DT bindings document is now explicitly states that the drive edge is
  referring to the pixel clock

Since we have several panels under omapdrm/displays/ where the data drive edge
is set to be different then the sync drive edge, the first three patch will add
support to select the sync drive edge via DT.
I was not able to locate the datasheet for all the panels and because the
different edge was used in omapdrm and omapfb for a long time without complains
from users - and they were written this way - I think it is a valid that we can
have panels requiring different edge for data and sync to be driven.

This series will add support for the sync drive edge setting and the omapdrm's
videmode conversion series depends on this change. It is also planned that the
omapdrm is going to move to use the drm/panel in the future and to be able to
convert the panel drivers we will need this feature.

Regards,
Peter
---
Peter Ujfalusi (3):
  dt-bindings: display: display-timing: Add property to configure sync
    drive edge
  video: display_timing: Add flags to select the edge when the sync is
    driven
  video: of: display_timing: Add support for syncclk-active property

 .../devicetree/bindings/display/panel/display-timing.txt         | 8 ++++++++
 drivers/video/of_display_timing.c                                | 9 +++++++++
 include/video/display_timing.h                                   | 4 ++++
 3 files changed, 21 insertions(+)

--
2.10.0

[PATCH v2 1/3] dt-bindings: display: display-timing: Add property to configure sync drive edge

[Peter Ujfalusi]

There are display panels which demands that the sync signal is driven on
different edge than the pixel data.
With the syncclk-active property we can specify the clk edge to be used to
drive the sync signal. When the property is missing it indicates that the
sync is driven on the same edge as the pixel data.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
CC: Rob Herring <robh+dt@kernel.org>
CC: Mark Rutland <mark.rutland@arm.com>
CC: devicetree@vger.kernel.org
---
 .../devicetree/bindings/display/panel/display-timing.txt          | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/panel/display-timing.txt b/Documentation/devicetree/bindings/display/panel/display-timing.txt
index e1d4a0b59612..81a75893d1b8 100644
--- a/Documentation/devicetree/bindings/display/panel/display-timing.txt
+++ b/Documentation/devicetree/bindings/display/panel/display-timing.txt
@@ -32,6 +32,14 @@ optional properties:
 			- active low  = drive pixel data on falling edge/
 					sample data on rising edge
 			- ignored     = ignored
+ - syncclk-active: with
+			- active high = drive sync on rising edge/
+					sample sync on falling edge of pixel
+					clock
+			- active low  = drive sync on falling edge/
+					sample sync on rising edge of pixel
+					clock
+			- omitted     = same configuration as pixelclk-active
  - interlaced (bool): boolean to enable interlaced mode
  - doublescan (bool): boolean to enable doublescan mode
  - doubleclk (bool): boolean to enable doubleclock mode
-- 
2.10.0

[PATCH v2 2/3] video: display_timing: Add flags to select the edge when the sync is driven

[Peter Ujfalusi]

The sync can be - and for some panels it must be - driven on different edge
then the data.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
CC: Rob Herring <robh+dt@kernel.org>
CC: Mark Rutland <mark.rutland@arm.com>
CC: devicetree@vger.kernel.org
---
 include/video/display_timing.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/video/display_timing.h b/include/video/display_timing.h
index 28d9d0d566ca..3d289e990aca 100644
--- a/include/video/display_timing.h
+++ b/include/video/display_timing.h
@@ -28,6 +28,10 @@ enum display_flags {
 	DISPLAY_FLAGS_INTERLACED	= BIT(8),
 	DISPLAY_FLAGS_DOUBLESCAN	= BIT(9),
 	DISPLAY_FLAGS_DOUBLECLK		= BIT(10),
+	/* drive sync on pos. edge */
+	DISPLAY_FLAGS_SYNC_POSEDGE	= BIT(11),
+	/* drive sync on neg. edge */
+	DISPLAY_FLAGS_SYNC_NEGEDGE	= BIT(12),
 };
 
 /*
-- 
2.10.0

[PATCH v2 3/3] video: of: display_timing: Add support for syncclk-active property

[Peter Ujfalusi]

Configure the DISPLAY_FLAGS_SYNC_POSEDGE/NEGEDGE flags according to the
binding document.
If the syncclk-active is present in DT, configure the flags accordingly, if
it is omitted it means that the SYNC edge is following the pixdata
configuration.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
CC: Rob Herring <robh+dt@kernel.org>
CC: Mark Rutland <mark.rutland@arm.com>
CC: devicetree@vger.kernel.org
---
 drivers/video/of_display_timing.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c
index 8a1076beecd3..db992c684f09 100644
--- a/drivers/video/of_display_timing.c
+++ b/drivers/video/of_display_timing.c
@@ -88,6 +88,15 @@ static int of_parse_display_timing(const struct device_node *np,
 		dt->flags |= val ? DISPLAY_FLAGS_PIXDATA_POSEDGE :
 				DISPLAY_FLAGS_PIXDATA_NEGEDGE;
 
+	if (!of_property_read_u32(np, "syncclk-active", &val))
+		dt->flags |= val ? DISPLAY_FLAGS_SYNC_POSEDGE :
+				DISPLAY_FLAGS_SYNC_NEGEDGE;
+	else if (dt->flags & (DISPLAY_FLAGS_PIXDATA_POSEDGE |
+			      DISPLAY_FLAGS_PIXDATA_NEGEDGE))
+		dt->flags |= dt->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE ?
+				DISPLAY_FLAGS_SYNC_POSEDGE :
+				DISPLAY_FLAGS_SYNC_NEGEDGE;
+
 	if (of_property_read_bool(np, "interlaced"))
 		dt->flags |= DISPLAY_FLAGS_INTERLACED;
 	if (of_property_read_bool(np, "doublescan"))
-- 
2.10.0

Re: [PATCH v2 1/3] dt-bindings: display: display-timing: Add property to configure sync drive edge

[Rob Herring]

On Thu, Sep 22, 2016 at 01:35:24PM +0300, Peter Ujfalusi wrote:
> There are display panels which demands that the sync signal is driven on
> different edge than the pixel data.
> With the syncclk-active property we can specify the clk edge to be used to
> drive the sync signal. When the property is missing it indicates that the
> sync is driven on the same edge as the pixel data.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> CC: Rob Herring <robh+dt@kernel.org>
> CC: Mark Rutland <mark.rutland@arm.com>
> CC: devicetree@vger.kernel.org
> ---
>  .../devicetree/bindings/display/panel/display-timing.txt          | 8 ++++++++
>  1 file changed, 8 insertions(+)

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v2 1/3] dt-bindings: display: display-timing: Add property to configure sync drive edge

[Tomi Valkeinen]

[-- Attachment #1.1: Type: text/plain, Size: 1805 bytes --]

On 22/09/16 13:35, Peter Ujfalusi wrote:
> There are display panels which demands that the sync signal is driven on
> different edge than the pixel data.
> With the syncclk-active property we can specify the clk edge to be used to
> drive the sync signal. When the property is missing it indicates that the
> sync is driven on the same edge as the pixel data.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> CC: Rob Herring <robh+dt@kernel.org>
> CC: Mark Rutland <mark.rutland@arm.com>
> CC: devicetree@vger.kernel.org
> ---
>  .../devicetree/bindings/display/panel/display-timing.txt          | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/display-timing.txt b/Documentation/devicetree/bindings/display/panel/display-timing.txt
> index e1d4a0b59612..81a75893d1b8 100644
> --- a/Documentation/devicetree/bindings/display/panel/display-timing.txt
> +++ b/Documentation/devicetree/bindings/display/panel/display-timing.txt
> @@ -32,6 +32,14 @@ optional properties:
>  			- active low  = drive pixel data on falling edge/
>  					sample data on rising edge
>  			- ignored     = ignored
> + - syncclk-active: with
> +			- active high = drive sync on rising edge/
> +					sample sync on falling edge of pixel
> +					clock
> +			- active low  = drive sync on falling edge/
> +					sample sync on rising edge of pixel
> +					clock
> +			- omitted     = same configuration as pixelclk-active

I wonder if the "sample sync on..." should be left out here. It makes
sense for the pixel data, but for sync... Do the panels "sample" it, or
do they trigger on rising/falling edge?

Well, maybe that's a bit on the nitpick side, so:

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 2/3] video: display_timing: Add flags to select the edge when the sync is driven

[Tomi Valkeinen]

[-- Attachment #1.1: Type: text/plain, Size: 1078 bytes --]

On 22/09/16 13:35, Peter Ujfalusi wrote:
> The sync can be - and for some panels it must be - driven on different edge
> then the data.

Well, the "can be" depends on the display controller =).

> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> CC: Rob Herring <robh+dt@kernel.org>
> CC: Mark Rutland <mark.rutland@arm.com>
> CC: devicetree@vger.kernel.org
> ---
>  include/video/display_timing.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/video/display_timing.h b/include/video/display_timing.h
> index 28d9d0d566ca..3d289e990aca 100644
> --- a/include/video/display_timing.h
> +++ b/include/video/display_timing.h
> @@ -28,6 +28,10 @@ enum display_flags {
>  	DISPLAY_FLAGS_INTERLACED	= BIT(8),
>  	DISPLAY_FLAGS_DOUBLESCAN	= BIT(9),
>  	DISPLAY_FLAGS_DOUBLECLK		= BIT(10),
> +	/* drive sync on pos. edge */
> +	DISPLAY_FLAGS_SYNC_POSEDGE	= BIT(11),
> +	/* drive sync on neg. edge */
> +	DISPLAY_FLAGS_SYNC_NEGEDGE	= BIT(12),
>  };
>  
>  /*
> 

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 3/3] video: of: display_timing: Add support for syncclk-active property

[Tomi Valkeinen]

[-- Attachment #1.1: Type: text/plain, Size: 1595 bytes --]

On 22/09/16 13:35, Peter Ujfalusi wrote:
> Configure the DISPLAY_FLAGS_SYNC_POSEDGE/NEGEDGE flags according to the
> binding document.
> If the syncclk-active is present in DT, configure the flags accordingly, if
> it is omitted it means that the SYNC edge is following the pixdata
> configuration.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> CC: Rob Herring <robh+dt@kernel.org>
> CC: Mark Rutland <mark.rutland@arm.com>
> CC: devicetree@vger.kernel.org
> ---
>  drivers/video/of_display_timing.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c
> index 8a1076beecd3..db992c684f09 100644
> --- a/drivers/video/of_display_timing.c
> +++ b/drivers/video/of_display_timing.c
> @@ -88,6 +88,15 @@ static int of_parse_display_timing(const struct device_node *np,
>  		dt->flags |= val ? DISPLAY_FLAGS_PIXDATA_POSEDGE :
>  				DISPLAY_FLAGS_PIXDATA_NEGEDGE;
>  
> +	if (!of_property_read_u32(np, "syncclk-active", &val))
> +		dt->flags |= val ? DISPLAY_FLAGS_SYNC_POSEDGE :
> +				DISPLAY_FLAGS_SYNC_NEGEDGE;
> +	else if (dt->flags & (DISPLAY_FLAGS_PIXDATA_POSEDGE |
> +			      DISPLAY_FLAGS_PIXDATA_NEGEDGE))
> +		dt->flags |= dt->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE ?
> +				DISPLAY_FLAGS_SYNC_POSEDGE :
> +				DISPLAY_FLAGS_SYNC_NEGEDGE;
> +
>  	if (of_property_read_bool(np, "interlaced"))
>  		dt->flags |= DISPLAY_FLAGS_INTERLACED;
>  	if (of_property_read_bool(np, "doublescan"))
> 

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]