[PATCH v2 0/8] drm/bridge: add bus flag support

[PATCH v2 0/8] drm/bridge: add bus flag support

[Stefan Agner]

This v2 shifted a bit more to rework bridge timing support. While
my use case mainly cares about complete bus flag support, it seems
to me that bus timings have ended up to be more complex than
necessary. Patch 2/3 are an attempt to simplify bus timings. This
also aligns bridge timings with how display timings are specified
today.

@Linus Walleij, Laurent Pinchart: Since you have been involved in
the initial bus timings discussion, I would like to have your Ack
on at least patch 2/3... If we want to keep the setup/hold timings,
the patchset should also work without those two patches.

--
Stefan

Changes in v2:
- Rename bus_flags to simple_bus_flags
- Reword dt-bindings for de-active
- Add patch 2/3 which attempts to simplify bridge timings

Stefan Agner (8):
  drm/bridge: use bus flags in bridge timings
  drm/pl111: simplify bridge timing support
  drm/bridge: simplify bridge timing info
  drm/bridge: allow to specify data-enable polarity
  dt-bindings: display: add data-enable polarity property
  drm/imx: support handling bridge timings bus flags
  ARM: dts: imx6qdl-apalis: add VGA support
  ARM: dts: imx6qdl-apalis: add GPIO I2C node for DDC

 .../bindings/display/bridge/dumb-vga-dac.txt  |  2 +
 arch/arm/boot/dts/imx6q-apalis-eval.dts       | 28 ++++++++
 arch/arm/boot/dts/imx6qdl-apalis.dtsi         | 72 +++++++++++++++++++
 drivers/gpu/drm/bridge/dumb-vga-dac.c         | 45 +++++++-----
 drivers/gpu/drm/imx/parallel-display.c        |  3 +
 drivers/gpu/drm/pl111/pl111_display.c         | 22 ++----
 include/drm/drm_bridge.h                      | 25 ++-----
 7 files changed, 142 insertions(+), 55 deletions(-)

-- 
2.18.0

[PATCH v2 0/8] drm/bridge: add bus flag support

[Stefan Agner]

This v2 shifted a bit more to rework bridge timing support. While
my use case mainly cares about complete bus flag support, it seems
to me that bus timings have ended up to be more complex than
necessary. Patch 2/3 are an attempt to simplify bus timings. This
also aligns bridge timings with how display timings are specified
today.

@Linus Walleij, Laurent Pinchart: Since you have been involved in
the initial bus timings discussion, I would like to have your Ack
on at least patch 2/3... If we want to keep the setup/hold timings,
the patchset should also work without those two patches.

--
Stefan

Changes in v2:
- Rename bus_flags to simple_bus_flags
- Reword dt-bindings for de-active
- Add patch 2/3 which attempts to simplify bridge timings

Stefan Agner (8):
  drm/bridge: use bus flags in bridge timings
  drm/pl111: simplify bridge timing support
  drm/bridge: simplify bridge timing info
  drm/bridge: allow to specify data-enable polarity
  dt-bindings: display: add data-enable polarity property
  drm/imx: support handling bridge timings bus flags
  ARM: dts: imx6qdl-apalis: add VGA support
  ARM: dts: imx6qdl-apalis: add GPIO I2C node for DDC

 .../bindings/display/bridge/dumb-vga-dac.txt  |  2 +
 arch/arm/boot/dts/imx6q-apalis-eval.dts       | 28 ++++++++
 arch/arm/boot/dts/imx6qdl-apalis.dtsi         | 72 +++++++++++++++++++
 drivers/gpu/drm/bridge/dumb-vga-dac.c         | 45 +++++++-----
 drivers/gpu/drm/imx/parallel-display.c        |  3 +
 drivers/gpu/drm/pl111/pl111_display.c         | 22 ++----
 include/drm/drm_bridge.h                      | 25 ++-----
 7 files changed, 142 insertions(+), 55 deletions(-)

-- 
2.18.0

[PATCH v2 1/8] drm/bridge: use bus flags in bridge timings

[Stefan Agner]

The DRM bus flags conveys additional information on pixel data on
the bus. All currently available bus flags might be of interest for
a bridge. In the case at hand a dumb VGA bridge needs a specific
data enable polarity (DRM_BUS_FLAG_DE_LOW).

Replace the sampling_edge field with input_bus_flags and allow all
currently documented bus flags.

This changes the perspective from sampling side to the driving
side for the currently supported flags. We assume that the sampling
edge is always the opposite of the driving edge (hence we need to
invert the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE flags). This is an
assumption we already make for displays. For all we know it is a
safe assumption for bridges too.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/bridge/dumb-vga-dac.c |  6 +++---
 include/drm/drm_bridge.h              | 11 +++++------
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index 9b706789a341..d5aa0f931ef2 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
  */
 static const struct drm_bridge_timings default_dac_timings = {
 	/* Timing specifications, datasheet page 7 */
-	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
 	.setup_time_ps = 500,
 	.hold_time_ps = 1500,
 };
@@ -245,7 +245,7 @@ static const struct drm_bridge_timings default_dac_timings = {
  */
 static const struct drm_bridge_timings ti_ths8134_dac_timings = {
 	/* From timing diagram, datasheet page 9 */
-	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
 	/* From datasheet, page 12 */
 	.setup_time_ps = 3000,
 	/* I guess this means latched input */
@@ -258,7 +258,7 @@ static const struct drm_bridge_timings ti_ths8134_dac_timings = {
  */
 static const struct drm_bridge_timings ti_ths8135_dac_timings = {
 	/* From timing diagram, datasheet page 14 */
-	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
 	/* From datasheet, page 16 */
 	.setup_time_ps = 2000,
 	.hold_time_ps = 500,
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index bd850747ce54..45e90f4b46c3 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -244,14 +244,13 @@ struct drm_bridge_funcs {
  */
 struct drm_bridge_timings {
 	/**
-	 * @sampling_edge:
+	 * @input_bus_flags:
 	 *
-	 * Tells whether the bridge samples the digital input signal
-	 * from the display engine on the positive or negative edge of the
-	 * clock, this should reuse the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE
-	 * bitwise flags from the DRM connector (bit 2 and 3 valid).
+	 * Additional settings this bridge requires for the pixel data on
+	 * the input bus (e.g. pixel signal polarity). See also
+	 * &drm_display_info->bus_flags.
 	 */
-	u32 sampling_edge;
+	u32 input_bus_flags;
 	/**
 	 * @setup_time_ps:
 	 *
-- 
2.18.0

[PATCH v2 1/8] drm/bridge: use bus flags in bridge timings

[Stefan Agner]

The DRM bus flags conveys additional information on pixel data on
the bus. All currently available bus flags might be of interest for
a bridge. In the case at hand a dumb VGA bridge needs a specific
data enable polarity (DRM_BUS_FLAG_DE_LOW).

Replace the sampling_edge field with input_bus_flags and allow all
currently documented bus flags.

This changes the perspective from sampling side to the driving
side for the currently supported flags. We assume that the sampling
edge is always the opposite of the driving edge (hence we need to
invert the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE flags). This is an
assumption we already make for displays. For all we know it is a
safe assumption for bridges too.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/bridge/dumb-vga-dac.c |  6 +++---
 include/drm/drm_bridge.h              | 11 +++++------
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index 9b706789a341..d5aa0f931ef2 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
  */
 static const struct drm_bridge_timings default_dac_timings = {
 	/* Timing specifications, datasheet page 7 */
-	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
 	.setup_time_ps = 500,
 	.hold_time_ps = 1500,
 };
@@ -245,7 +245,7 @@ static const struct drm_bridge_timings default_dac_timings = {
  */
 static const struct drm_bridge_timings ti_ths8134_dac_timings = {
 	/* From timing diagram, datasheet page 9 */
-	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
 	/* From datasheet, page 12 */
 	.setup_time_ps = 3000,
 	/* I guess this means latched input */
@@ -258,7 +258,7 @@ static const struct drm_bridge_timings ti_ths8134_dac_timings = {
  */
 static const struct drm_bridge_timings ti_ths8135_dac_timings = {
 	/* From timing diagram, datasheet page 14 */
-	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
 	/* From datasheet, page 16 */
 	.setup_time_ps = 2000,
 	.hold_time_ps = 500,
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index bd850747ce54..45e90f4b46c3 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -244,14 +244,13 @@ struct drm_bridge_funcs {
  */
 struct drm_bridge_timings {
 	/**
-	 * @sampling_edge:
+	 * @input_bus_flags:
 	 *
-	 * Tells whether the bridge samples the digital input signal
-	 * from the display engine on the positive or negative edge of the
-	 * clock, this should reuse the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE
-	 * bitwise flags from the DRM connector (bit 2 and 3 valid).
+	 * Additional settings this bridge requires for the pixel data on
+	 * the input bus (e.g. pixel signal polarity). See also
+	 * &drm_display_info->bus_flags.
 	 */
-	u32 sampling_edge;
+	u32 input_bus_flags;
 	/**
 	 * @setup_time_ps:
 	 *
-- 
2.18.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/8] drm/bridge: use bus flags in bridge timings

[Stefan Agner]

The DRM bus flags conveys additional information on pixel data on
the bus. All currently available bus flags might be of interest for
a bridge. In the case at hand a dumb VGA bridge needs a specific
data enable polarity (DRM_BUS_FLAG_DE_LOW).

Replace the sampling_edge field with input_bus_flags and allow all
currently documented bus flags.

This changes the perspective from sampling side to the driving
side for the currently supported flags. We assume that the sampling
edge is always the opposite of the driving edge (hence we need to
invert the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE flags). This is an
assumption we already make for displays. For all we know it is a
safe assumption for bridges too.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/bridge/dumb-vga-dac.c |  6 +++---
 include/drm/drm_bridge.h              | 11 +++++------
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index 9b706789a341..d5aa0f931ef2 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
  */
 static const struct drm_bridge_timings default_dac_timings = {
 	/* Timing specifications, datasheet page 7 */
-	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
 	.setup_time_ps = 500,
 	.hold_time_ps = 1500,
 };
@@ -245,7 +245,7 @@ static const struct drm_bridge_timings default_dac_timings = {
  */
 static const struct drm_bridge_timings ti_ths8134_dac_timings = {
 	/* From timing diagram, datasheet page 9 */
-	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
 	/* From datasheet, page 12 */
 	.setup_time_ps = 3000,
 	/* I guess this means latched input */
@@ -258,7 +258,7 @@ static const struct drm_bridge_timings ti_ths8134_dac_timings = {
  */
 static const struct drm_bridge_timings ti_ths8135_dac_timings = {
 	/* From timing diagram, datasheet page 14 */
-	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
 	/* From datasheet, page 16 */
 	.setup_time_ps = 2000,
 	.hold_time_ps = 500,
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index bd850747ce54..45e90f4b46c3 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -244,14 +244,13 @@ struct drm_bridge_funcs {
  */
 struct drm_bridge_timings {
 	/**
-	 * @sampling_edge:
+	 * @input_bus_flags:
 	 *
-	 * Tells whether the bridge samples the digital input signal
-	 * from the display engine on the positive or negative edge of the
-	 * clock, this should reuse the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE
-	 * bitwise flags from the DRM connector (bit 2 and 3 valid).
+	 * Additional settings this bridge requires for the pixel data on
+	 * the input bus (e.g. pixel signal polarity). See also
+	 * &drm_display_info->bus_flags.
 	 */
-	u32 sampling_edge;
+	u32 input_bus_flags;
 	/**
 	 * @setup_time_ps:
 	 *
-- 
2.18.0

[PATCH v2 2/8] drm/pl111: simplify bridge timing support

[Stefan Agner]

Simplify bridge timing support by only supporting pixel clock
polarity. This aligns pixel clock polarity handling for bridges
with displays.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/pl111/pl111_display.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 754f6b25f265..31eb62e4476f 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -196,23 +196,13 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 		const struct drm_bridge_timings *btimings = bridge->timings;
 
 		/*
-		 * Here is when things get really fun. Sometimes the bridge
-		 * timings are such that the signal out from PL11x is not
-		 * stable before the receiving bridge (such as a dumb VGA DAC
-		 * or similar) samples it. If that happens, we compensate by
-		 * the only method we have: output the data on the opposite
-		 * edge of the clock so it is for sure stable when it gets
-		 * sampled.
-		 *
-		 * The PL111 manual does not contain proper timining diagrams
-		 * or data for these details, but we know from experiments
-		 * that the setup time is more than 3000 picoseconds (3 ns).
-		 * If we have a bridge that requires the signal to be stable
-		 * earlier than 3000 ps before the clock pulse, we have to
-		 * output the data on the opposite edge to avoid flicker.
+		 * Use LCD Timing 2 Register Invert Pixel Clock (IPC) bit
+		 * to make sure to drive data on falling edge if requested
+		 * by bridge.
 		 */
-		if (btimings && btimings->setup_time_ps >= 3000)
-			tim2 ^= TIM2_IPC;
+		if (btimings && btimings->input_bus_flags &
+		    DRM_BUS_FLAG_PIXDATA_NEGEDGE)
+			tim2 |= TIM2_IPC;
 	}
 
 	tim2 |= cpl << 16;
-- 
2.18.0

[PATCH v2 2/8] drm/pl111: simplify bridge timing support

[Stefan Agner]

Simplify bridge timing support by only supporting pixel clock
polarity. This aligns pixel clock polarity handling for bridges
with displays.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/pl111/pl111_display.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 754f6b25f265..31eb62e4476f 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -196,23 +196,13 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 		const struct drm_bridge_timings *btimings = bridge->timings;
 
 		/*
-		 * Here is when things get really fun. Sometimes the bridge
-		 * timings are such that the signal out from PL11x is not
-		 * stable before the receiving bridge (such as a dumb VGA DAC
-		 * or similar) samples it. If that happens, we compensate by
-		 * the only method we have: output the data on the opposite
-		 * edge of the clock so it is for sure stable when it gets
-		 * sampled.
-		 *
-		 * The PL111 manual does not contain proper timining diagrams
-		 * or data for these details, but we know from experiments
-		 * that the setup time is more than 3000 picoseconds (3 ns).
-		 * If we have a bridge that requires the signal to be stable
-		 * earlier than 3000 ps before the clock pulse, we have to
-		 * output the data on the opposite edge to avoid flicker.
+		 * Use LCD Timing 2 Register Invert Pixel Clock (IPC) bit
+		 * to make sure to drive data on falling edge if requested
+		 * by bridge.
 		 */
-		if (btimings && btimings->setup_time_ps >= 3000)
-			tim2 ^= TIM2_IPC;
+		if (btimings && btimings->input_bus_flags &
+		    DRM_BUS_FLAG_PIXDATA_NEGEDGE)
+			tim2 |= TIM2_IPC;
 	}
 
 	tim2 |= cpl << 16;
-- 
2.18.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/8] drm/pl111: simplify bridge timing support

[Stefan Agner]

Simplify bridge timing support by only supporting pixel clock
polarity. This aligns pixel clock polarity handling for bridges
with displays.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/pl111/pl111_display.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 754f6b25f265..31eb62e4476f 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -196,23 +196,13 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 		const struct drm_bridge_timings *btimings = bridge->timings;
 
 		/*
-		 * Here is when things get really fun. Sometimes the bridge
-		 * timings are such that the signal out from PL11x is not
-		 * stable before the receiving bridge (such as a dumb VGA DAC
-		 * or similar) samples it. If that happens, we compensate by
-		 * the only method we have: output the data on the opposite
-		 * edge of the clock so it is for sure stable when it gets
-		 * sampled.
-		 *
-		 * The PL111 manual does not contain proper timining diagrams
-		 * or data for these details, but we know from experiments
-		 * that the setup time is more than 3000 picoseconds (3 ns).
-		 * If we have a bridge that requires the signal to be stable
-		 * earlier than 3000 ps before the clock pulse, we have to
-		 * output the data on the opposite edge to avoid flicker.
+		 * Use LCD Timing 2 Register Invert Pixel Clock (IPC) bit
+		 * to make sure to drive data on falling edge if requested
+		 * by bridge.
 		 */
-		if (btimings && btimings->setup_time_ps >= 3000)
-			tim2 ^= TIM2_IPC;
+		if (btimings && btimings->input_bus_flags &
+		    DRM_BUS_FLAG_PIXDATA_NEGEDGE)
+			tim2 |= TIM2_IPC;
 	}
 
 	tim2 |= cpl << 16;
-- 
2.18.0

[PATCH v2 3/8] drm/bridge: simplify bridge timing info

[Stefan Agner]

Bridges are typically connected to a parallel display signal with
pixel clock, sync signals and data lines. Parallel display signals
are also used in lower-end embedded display panels. For parallel
display panels we currently do not specify setup/hold times. From
discussions on the mailing list it seems not convincing that this
is currently really required for bridges either.

Remove setup/hold timings again to better align timing information
of displays and briges.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/bridge/dumb-vga-dac.c | 17 +++++------------
 include/drm/drm_bridge.h              | 14 --------------
 2 files changed, 5 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index d5aa0f931ef2..b2309ad228cf 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -229,14 +229,14 @@ static int dumb_vga_remove(struct platform_device *pdev)
 /*
  * We assume the ADV7123 DAC is the "default" for historical reasons
  * Information taken from the ADV7123 datasheet, revision D.
- * NOTE: the ADV7123EP seems to have other timings and need a new timings
- * set if used.
  */
 static const struct drm_bridge_timings default_dac_timings = {
-	/* Timing specifications, datasheet page 7 */
+	/*
+	 * From Timing diagram, datasheet page 7. The bridge samples
+	 * on pixel clocks positive edge, hence the display controller
+	 * should drive signals on the negative edge.
+	 */
 	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
-	.setup_time_ps = 500,
-	.hold_time_ps = 1500,
 };
 
 /*
@@ -246,10 +246,6 @@ static const struct drm_bridge_timings default_dac_timings = {
 static const struct drm_bridge_timings ti_ths8134_dac_timings = {
 	/* From timing diagram, datasheet page 9 */
 	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
-	/* From datasheet, page 12 */
-	.setup_time_ps = 3000,
-	/* I guess this means latched input */
-	.hold_time_ps = 0,
 };
 
 /*
@@ -259,9 +255,6 @@ static const struct drm_bridge_timings ti_ths8134_dac_timings = {
 static const struct drm_bridge_timings ti_ths8135_dac_timings = {
 	/* From timing diagram, datasheet page 14 */
 	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
-	/* From datasheet, page 16 */
-	.setup_time_ps = 2000,
-	.hold_time_ps = 500,
 };
 
 static const struct of_device_id dumb_vga_match[] = {
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 45e90f4b46c3..1a1d08350eaf 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -251,20 +251,6 @@ struct drm_bridge_timings {
 	 * &drm_display_info->bus_flags.
 	 */
 	u32 input_bus_flags;
-	/**
-	 * @setup_time_ps:
-	 *
-	 * Defines the time in picoseconds the input data lines must be
-	 * stable before the clock edge.
-	 */
-	u32 setup_time_ps;
-	/**
-	 * @hold_time_ps:
-	 *
-	 * Defines the time in picoseconds taken for the bridge to sample the
-	 * input signal after the clock edge.
-	 */
-	u32 hold_time_ps;
 };
 
 /**
-- 
2.18.0

[PATCH v2 3/8] drm/bridge: simplify bridge timing info

[Stefan Agner]

Bridges are typically connected to a parallel display signal with
pixel clock, sync signals and data lines. Parallel display signals
are also used in lower-end embedded display panels. For parallel
display panels we currently do not specify setup/hold times. From
discussions on the mailing list it seems not convincing that this
is currently really required for bridges either.

Remove setup/hold timings again to better align timing information
of displays and briges.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/bridge/dumb-vga-dac.c | 17 +++++------------
 include/drm/drm_bridge.h              | 14 --------------
 2 files changed, 5 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index d5aa0f931ef2..b2309ad228cf 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -229,14 +229,14 @@ static int dumb_vga_remove(struct platform_device *pdev)
 /*
  * We assume the ADV7123 DAC is the "default" for historical reasons
  * Information taken from the ADV7123 datasheet, revision D.
- * NOTE: the ADV7123EP seems to have other timings and need a new timings
- * set if used.
  */
 static const struct drm_bridge_timings default_dac_timings = {
-	/* Timing specifications, datasheet page 7 */
+	/*
+	 * From Timing diagram, datasheet page 7. The bridge samples
+	 * on pixel clocks positive edge, hence the display controller
+	 * should drive signals on the negative edge.
+	 */
 	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
-	.setup_time_ps = 500,
-	.hold_time_ps = 1500,
 };
 
 /*
@@ -246,10 +246,6 @@ static const struct drm_bridge_timings default_dac_timings = {
 static const struct drm_bridge_timings ti_ths8134_dac_timings = {
 	/* From timing diagram, datasheet page 9 */
 	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
-	/* From datasheet, page 12 */
-	.setup_time_ps = 3000,
-	/* I guess this means latched input */
-	.hold_time_ps = 0,
 };
 
 /*
@@ -259,9 +255,6 @@ static const struct drm_bridge_timings ti_ths8134_dac_timings = {
 static const struct drm_bridge_timings ti_ths8135_dac_timings = {
 	/* From timing diagram, datasheet page 14 */
 	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
-	/* From datasheet, page 16 */
-	.setup_time_ps = 2000,
-	.hold_time_ps = 500,
 };
 
 static const struct of_device_id dumb_vga_match[] = {
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 45e90f4b46c3..1a1d08350eaf 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -251,20 +251,6 @@ struct drm_bridge_timings {
 	 * &drm_display_info->bus_flags.
 	 */
 	u32 input_bus_flags;
-	/**
-	 * @setup_time_ps:
-	 *
-	 * Defines the time in picoseconds the input data lines must be
-	 * stable before the clock edge.
-	 */
-	u32 setup_time_ps;
-	/**
-	 * @hold_time_ps:
-	 *
-	 * Defines the time in picoseconds taken for the bridge to sample the
-	 * input signal after the clock edge.
-	 */
-	u32 hold_time_ps;
 };
 
 /**
-- 
2.18.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 3/8] drm/bridge: simplify bridge timing info

[Stefan Agner]

Bridges are typically connected to a parallel display signal with
pixel clock, sync signals and data lines. Parallel display signals
are also used in lower-end embedded display panels. For parallel
display panels we currently do not specify setup/hold times. From
discussions on the mailing list it seems not convincing that this
is currently really required for bridges either.

Remove setup/hold timings again to better align timing information
of displays and briges.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/bridge/dumb-vga-dac.c | 17 +++++------------
 include/drm/drm_bridge.h              | 14 --------------
 2 files changed, 5 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index d5aa0f931ef2..b2309ad228cf 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -229,14 +229,14 @@ static int dumb_vga_remove(struct platform_device *pdev)
 /*
  * We assume the ADV7123 DAC is the "default" for historical reasons
  * Information taken from the ADV7123 datasheet, revision D.
- * NOTE: the ADV7123EP seems to have other timings and need a new timings
- * set if used.
  */
 static const struct drm_bridge_timings default_dac_timings = {
-	/* Timing specifications, datasheet page 7 */
+	/*
+	 * From Timing diagram, datasheet page 7. The bridge samples
+	 * on pixel clocks positive edge, hence the display controller
+	 * should drive signals on the negative edge.
+	 */
 	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
-	.setup_time_ps = 500,
-	.hold_time_ps = 1500,
 };
 
 /*
@@ -246,10 +246,6 @@ static const struct drm_bridge_timings default_dac_timings = {
 static const struct drm_bridge_timings ti_ths8134_dac_timings = {
 	/* From timing diagram, datasheet page 9 */
 	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
-	/* From datasheet, page 12 */
-	.setup_time_ps = 3000,
-	/* I guess this means latched input */
-	.hold_time_ps = 0,
 };
 
 /*
@@ -259,9 +255,6 @@ static const struct drm_bridge_timings ti_ths8134_dac_timings = {
 static const struct drm_bridge_timings ti_ths8135_dac_timings = {
 	/* From timing diagram, datasheet page 14 */
 	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
-	/* From datasheet, page 16 */
-	.setup_time_ps = 2000,
-	.hold_time_ps = 500,
 };
 
 static const struct of_device_id dumb_vga_match[] = {
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 45e90f4b46c3..1a1d08350eaf 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -251,20 +251,6 @@ struct drm_bridge_timings {
 	 * &drm_display_info->bus_flags.
 	 */
 	u32 input_bus_flags;
-	/**
-	 * @setup_time_ps:
-	 *
-	 * Defines the time in picoseconds the input data lines must be
-	 * stable before the clock edge.
-	 */
-	u32 setup_time_ps;
-	/**
-	 * @hold_time_ps:
-	 *
-	 * Defines the time in picoseconds taken for the bridge to sample the
-	 * input signal after the clock edge.
-	 */
-	u32 hold_time_ps;
 };
 
 /**
-- 
2.18.0

[PATCH v2 4/8] drm/bridge: allow to specify data-enable polarity

[Stefan Agner]

Support boards with a passive RGB to VGA bridge which require a low
active data-enable polarity.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/bridge/dumb-vga-dac.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index b2309ad228cf..de001e322ba8 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -179,6 +179,7 @@ static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
 static int dumb_vga_probe(struct platform_device *pdev)
 {
 	struct dumb_vga *vga;
+	u32 de;
 
 	vga = devm_kzalloc(&pdev->dev, sizeof(*vga), GFP_KERNEL);
 	if (!vga)
@@ -194,6 +195,23 @@ static int dumb_vga_probe(struct platform_device *pdev)
 		dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
 	}
 
+	vga->bridge.funcs = &dumb_vga_bridge_funcs;
+	vga->bridge.of_node = pdev->dev.of_node;
+	vga->bridge.timings = of_device_get_match_data(&pdev->dev);
+
+	if (!vga->bridge.timings &&
+	    !of_property_read_u32(pdev->dev.of_node, "de-active", &de)) {
+		struct drm_bridge_timings *timings;
+
+		timings = devm_kzalloc(&pdev->dev, sizeof(*timings), GFP_KERNEL);
+		if (!timings)
+			return -ENOMEM;
+
+		timings->input_bus_flags = de ? DRM_BUS_FLAG_DE_HIGH :
+						DRM_BUS_FLAG_DE_LOW;
+		vga->bridge.timings = timings;
+	}
+
 	vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
 	if (IS_ERR(vga->ddc)) {
 		if (PTR_ERR(vga->ddc) == -ENODEV) {
@@ -205,10 +223,6 @@ static int dumb_vga_probe(struct platform_device *pdev)
 		}
 	}
 
-	vga->bridge.funcs = &dumb_vga_bridge_funcs;
-	vga->bridge.of_node = pdev->dev.of_node;
-	vga->bridge.timings = of_device_get_match_data(&pdev->dev);
-
 	drm_bridge_add(&vga->bridge);
 
 	return 0;
-- 
2.18.0

[PATCH v2 4/8] drm/bridge: allow to specify data-enable polarity

[Stefan Agner]

Support boards with a passive RGB to VGA bridge which require a low
active data-enable polarity.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/bridge/dumb-vga-dac.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index b2309ad228cf..de001e322ba8 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -179,6 +179,7 @@ static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
 static int dumb_vga_probe(struct platform_device *pdev)
 {
 	struct dumb_vga *vga;
+	u32 de;
 
 	vga = devm_kzalloc(&pdev->dev, sizeof(*vga), GFP_KERNEL);
 	if (!vga)
@@ -194,6 +195,23 @@ static int dumb_vga_probe(struct platform_device *pdev)
 		dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
 	}
 
+	vga->bridge.funcs = &dumb_vga_bridge_funcs;
+	vga->bridge.of_node = pdev->dev.of_node;
+	vga->bridge.timings = of_device_get_match_data(&pdev->dev);
+
+	if (!vga->bridge.timings &&
+	    !of_property_read_u32(pdev->dev.of_node, "de-active", &de)) {
+		struct drm_bridge_timings *timings;
+
+		timings = devm_kzalloc(&pdev->dev, sizeof(*timings), GFP_KERNEL);
+		if (!timings)
+			return -ENOMEM;
+
+		timings->input_bus_flags = de ? DRM_BUS_FLAG_DE_HIGH :
+						DRM_BUS_FLAG_DE_LOW;
+		vga->bridge.timings = timings;
+	}
+
 	vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
 	if (IS_ERR(vga->ddc)) {
 		if (PTR_ERR(vga->ddc) == -ENODEV) {
@@ -205,10 +223,6 @@ static int dumb_vga_probe(struct platform_device *pdev)
 		}
 	}
 
-	vga->bridge.funcs = &dumb_vga_bridge_funcs;
-	vga->bridge.of_node = pdev->dev.of_node;
-	vga->bridge.timings = of_device_get_match_data(&pdev->dev);
-
 	drm_bridge_add(&vga->bridge);
 
 	return 0;
-- 
2.18.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 4/8] drm/bridge: allow to specify data-enable polarity

[Stefan Agner]

Support boards with a passive RGB to VGA bridge which require a low
active data-enable polarity.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/bridge/dumb-vga-dac.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index b2309ad228cf..de001e322ba8 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -179,6 +179,7 @@ static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
 static int dumb_vga_probe(struct platform_device *pdev)
 {
 	struct dumb_vga *vga;
+	u32 de;
 
 	vga = devm_kzalloc(&pdev->dev, sizeof(*vga), GFP_KERNEL);
 	if (!vga)
@@ -194,6 +195,23 @@ static int dumb_vga_probe(struct platform_device *pdev)
 		dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
 	}
 
+	vga->bridge.funcs = &dumb_vga_bridge_funcs;
+	vga->bridge.of_node = pdev->dev.of_node;
+	vga->bridge.timings = of_device_get_match_data(&pdev->dev);
+
+	if (!vga->bridge.timings &&
+	    !of_property_read_u32(pdev->dev.of_node, "de-active", &de)) {
+		struct drm_bridge_timings *timings;
+
+		timings = devm_kzalloc(&pdev->dev, sizeof(*timings), GFP_KERNEL);
+		if (!timings)
+			return -ENOMEM;
+
+		timings->input_bus_flags = de ? DRM_BUS_FLAG_DE_HIGH :
+						DRM_BUS_FLAG_DE_LOW;
+		vga->bridge.timings = timings;
+	}
+
 	vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
 	if (IS_ERR(vga->ddc)) {
 		if (PTR_ERR(vga->ddc) == -ENODEV) {
@@ -205,10 +223,6 @@ static int dumb_vga_probe(struct platform_device *pdev)
 		}
 	}
 
-	vga->bridge.funcs = &dumb_vga_bridge_funcs;
-	vga->bridge.of_node = pdev->dev.of_node;
-	vga->bridge.timings = of_device_get_match_data(&pdev->dev);
-
 	drm_bridge_add(&vga->bridge);
 
 	return 0;
-- 
2.18.0

[PATCH v2 5/8] dt-bindings: display: add data-enable polarity property

[Stefan Agner]

Allow to specify the data-enable polarity required by a dumb VGA
DAC converting parallel RGB to VGA.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 .../devicetree/bindings/display/bridge/dumb-vga-dac.txt         | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
index 164cbb15f04c..727111ade203 100644
--- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
+++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
@@ -18,6 +18,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
 
 Optional properties:
 - vdd-supply: Power supply for DAC
+- de-active: Polarity of the data enable signal. 0 for active low, 1 for
+  active high, unset for system-specific defaults.
 
 Example
 -------
-- 
2.18.0

[PATCH v2 5/8] dt-bindings: display: add data-enable polarity property

[Stefan Agner]

Allow to specify the data-enable polarity required by a dumb VGA
DAC converting parallel RGB to VGA.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 .../devicetree/bindings/display/bridge/dumb-vga-dac.txt         | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
index 164cbb15f04c..727111ade203 100644
--- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
+++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
@@ -18,6 +18,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
 
 Optional properties:
 - vdd-supply: Power supply for DAC
+- de-active: Polarity of the data enable signal. 0 for active low, 1 for
+  active high, unset for system-specific defaults.
 
 Example
 -------
-- 
2.18.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 5/8] dt-bindings: display: add data-enable polarity property

[Stefan Agner]

Allow to specify the data-enable polarity required by a dumb VGA
DAC converting parallel RGB to VGA.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 .../devicetree/bindings/display/bridge/dumb-vga-dac.txt         | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
index 164cbb15f04c..727111ade203 100644
--- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
+++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
@@ -18,6 +18,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
 
 Optional properties:
 - vdd-supply: Power supply for DAC
+- de-active: Polarity of the data enable signal. 0 for active low, 1 for
+  active high, unset for system-specific defaults.
 
 Example
 -------
-- 
2.18.0

[PATCH v2 6/8] drm/imx: support handling bridge timings bus flags

[Stefan Agner]

A bridge might require specific settings for the pixel data on
the bus. Copy the bus flags from the bridge timings if a bridge
is in use.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/imx/parallel-display.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
index aefd04e18f93..7798a0621df7 100644
--- a/drivers/gpu/drm/imx/parallel-display.c
+++ b/drivers/gpu/drm/imx/parallel-display.c
@@ -239,6 +239,9 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data)
 	if (ret && ret != -ENODEV)
 		return ret;
 
+	if (imxpd->bridge && imxpd->bridge->timings)
+		imxpd->bus_flags = imxpd->bridge->timings->input_bus_flags;
+
 	imxpd->dev = dev;
 
 	ret = imx_pd_register(drm, imxpd);
-- 
2.18.0

[PATCH v2 6/8] drm/imx: support handling bridge timings bus flags

[Stefan Agner]

A bridge might require specific settings for the pixel data on
the bus. Copy the bus flags from the bridge timings if a bridge
is in use.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/imx/parallel-display.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
index aefd04e18f93..7798a0621df7 100644
--- a/drivers/gpu/drm/imx/parallel-display.c
+++ b/drivers/gpu/drm/imx/parallel-display.c
@@ -239,6 +239,9 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data)
 	if (ret && ret != -ENODEV)
 		return ret;
 
+	if (imxpd->bridge && imxpd->bridge->timings)
+		imxpd->bus_flags = imxpd->bridge->timings->input_bus_flags;
+
 	imxpd->dev = dev;
 
 	ret = imx_pd_register(drm, imxpd);
-- 
2.18.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 6/8] drm/imx: support handling bridge timings bus flags

[Stefan Agner]

A bridge might require specific settings for the pixel data on
the bus. Copy the bus flags from the bridge timings if a bridge
is in use.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/imx/parallel-display.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
index aefd04e18f93..7798a0621df7 100644
--- a/drivers/gpu/drm/imx/parallel-display.c
+++ b/drivers/gpu/drm/imx/parallel-display.c
@@ -239,6 +239,9 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data)
 	if (ret && ret != -ENODEV)
 		return ret;
 
+	if (imxpd->bridge && imxpd->bridge->timings)
+		imxpd->bus_flags = imxpd->bridge->timings->input_bus_flags;
+
 	imxpd->dev = dev;
 
 	ret = imx_pd_register(drm, imxpd);
-- 
2.18.0

[PATCH v2 7/8] ARM: dts: imx6qdl-apalis: add VGA support

[Stefan Agner]

The Apalis iMX6 modules use a simple DAC using resistor ladders
to convert parallel RGB to VGA. Add support for this output using
the dumb VGA driver.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 arch/arm/boot/dts/imx6q-apalis-eval.dts | 28 +++++++++++++
 arch/arm/boot/dts/imx6qdl-apalis.dtsi   | 52 +++++++++++++++++++++++++
 2 files changed, 80 insertions(+)

diff --git a/arch/arm/boot/dts/imx6q-apalis-eval.dts b/arch/arm/boot/dts/imx6q-apalis-eval.dts
index 707ac9a46115..ae07b3a105ef 100644
--- a/arch/arm/boot/dts/imx6q-apalis-eval.dts
+++ b/arch/arm/boot/dts/imx6q-apalis-eval.dts
@@ -140,6 +140,16 @@
 		regulator-max-microvolt = <3300000>;
 		regulator-always-on;
 	};
+
+	vga {
+		compatible = "vga-connector";
+
+		port {
+			vga_con_in: endpoint {
+				remote-endpoint = <&vga_bridge_out>;
+			};
+		};
+	};
 };
 
 &backlight {
@@ -281,6 +291,24 @@
 	status = "okay";
 };
 
+&vgabridge {
+	status = "okay";
+
+	ports {
+		port@1 {
+			reg = <1>;
+
+			vga_bridge_out: endpoint {
+				remote-endpoint = <&vga_con_in>;
+			};
+		};
+	};
+};
+
+&vgadisplay {
+	status = "okay";
+};
+
 &iomuxc {
 	/*
 	 * Mux the Apalis GPIOs
diff --git a/arch/arm/boot/dts/imx6qdl-apalis.dtsi b/arch/arm/boot/dts/imx6qdl-apalis.dtsi
index 05f07ea3e8c8..e8d0407e3e18 100644
--- a/arch/arm/boot/dts/imx6qdl-apalis.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-apalis.dtsi
@@ -138,6 +138,54 @@
 		spdif-out;
 		status = "disabled";
 	};
+
+	vgabridge: bridge {
+		compatible = "dumb-vga-dac";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		de-active = <0>;
+		status = "disabled";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+
+				vga_bridge_in: endpoint {
+					remote-endpoint = <&vga_display_out>;
+				};
+			};
+		};
+	};
+
+	vgadisplay: display@di0 {
+		compatible = "fsl,imx-parallel-display";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		interface-pix-fmt = "rgb565";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_ipu2_vdac>;
+		status = "disabled";
+
+		port@0 {
+			reg = <0>;
+
+			vga_display_in: endpoint {
+				remote-endpoint = <&ipu2_di0_disp0>;
+			};
+		};
+
+		port@1 {
+			reg = <1>;
+
+			vga_display_out: endpoint {
+				remote-endpoint = <&vga_bridge_in>;
+			};
+		};
+	};
+
 };
 
 &audmux {
@@ -373,6 +421,10 @@
 	status = "disabled";
 };
 
+&ipu2_di0_disp0 {
+	remote-endpoint = <&vga_display_in>;
+};
+
 &pwm1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_pwm1>;
-- 
2.18.0

[PATCH v2 7/8] ARM: dts: imx6qdl-apalis: add VGA support

[Stefan Agner]

The Apalis iMX6 modules use a simple DAC using resistor ladders
to convert parallel RGB to VGA. Add support for this output using
the dumb VGA driver.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 arch/arm/boot/dts/imx6q-apalis-eval.dts | 28 +++++++++++++
 arch/arm/boot/dts/imx6qdl-apalis.dtsi   | 52 +++++++++++++++++++++++++
 2 files changed, 80 insertions(+)

diff --git a/arch/arm/boot/dts/imx6q-apalis-eval.dts b/arch/arm/boot/dts/imx6q-apalis-eval.dts
index 707ac9a46115..ae07b3a105ef 100644
--- a/arch/arm/boot/dts/imx6q-apalis-eval.dts
+++ b/arch/arm/boot/dts/imx6q-apalis-eval.dts
@@ -140,6 +140,16 @@
 		regulator-max-microvolt = <3300000>;
 		regulator-always-on;
 	};
+
+	vga {
+		compatible = "vga-connector";
+
+		port {
+			vga_con_in: endpoint {
+				remote-endpoint = <&vga_bridge_out>;
+			};
+		};
+	};
 };
 
 &backlight {
@@ -281,6 +291,24 @@
 	status = "okay";
 };
 
+&vgabridge {
+	status = "okay";
+
+	ports {
+		port@1 {
+			reg = <1>;
+
+			vga_bridge_out: endpoint {
+				remote-endpoint = <&vga_con_in>;
+			};
+		};
+	};
+};
+
+&vgadisplay {
+	status = "okay";
+};
+
 &iomuxc {
 	/*
 	 * Mux the Apalis GPIOs
diff --git a/arch/arm/boot/dts/imx6qdl-apalis.dtsi b/arch/arm/boot/dts/imx6qdl-apalis.dtsi
index 05f07ea3e8c8..e8d0407e3e18 100644
--- a/arch/arm/boot/dts/imx6qdl-apalis.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-apalis.dtsi
@@ -138,6 +138,54 @@
 		spdif-out;
 		status = "disabled";
 	};
+
+	vgabridge: bridge {
+		compatible = "dumb-vga-dac";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		de-active = <0>;
+		status = "disabled";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+
+				vga_bridge_in: endpoint {
+					remote-endpoint = <&vga_display_out>;
+				};
+			};
+		};
+	};
+
+	vgadisplay: display@di0 {
+		compatible = "fsl,imx-parallel-display";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		interface-pix-fmt = "rgb565";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_ipu2_vdac>;
+		status = "disabled";
+
+		port@0 {
+			reg = <0>;
+
+			vga_display_in: endpoint {
+				remote-endpoint = <&ipu2_di0_disp0>;
+			};
+		};
+
+		port@1 {
+			reg = <1>;
+
+			vga_display_out: endpoint {
+				remote-endpoint = <&vga_bridge_in>;
+			};
+		};
+	};
+
 };
 
 &audmux {
@@ -373,6 +421,10 @@
 	status = "disabled";
 };
 
+&ipu2_di0_disp0 {
+	remote-endpoint = <&vga_display_in>;
+};
+
 &pwm1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_pwm1>;
-- 
2.18.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 7/8] ARM: dts: imx6qdl-apalis: add VGA support

[Stefan Agner]

The Apalis iMX6 modules use a simple DAC using resistor ladders
to convert parallel RGB to VGA. Add support for this output using
the dumb VGA driver.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 arch/arm/boot/dts/imx6q-apalis-eval.dts | 28 +++++++++++++
 arch/arm/boot/dts/imx6qdl-apalis.dtsi   | 52 +++++++++++++++++++++++++
 2 files changed, 80 insertions(+)

diff --git a/arch/arm/boot/dts/imx6q-apalis-eval.dts b/arch/arm/boot/dts/imx6q-apalis-eval.dts
index 707ac9a46115..ae07b3a105ef 100644
--- a/arch/arm/boot/dts/imx6q-apalis-eval.dts
+++ b/arch/arm/boot/dts/imx6q-apalis-eval.dts
@@ -140,6 +140,16 @@
 		regulator-max-microvolt = <3300000>;
 		regulator-always-on;
 	};
+
+	vga {
+		compatible = "vga-connector";
+
+		port {
+			vga_con_in: endpoint {
+				remote-endpoint = <&vga_bridge_out>;
+			};
+		};
+	};
 };
 
 &backlight {
@@ -281,6 +291,24 @@
 	status = "okay";
 };
 
+&vgabridge {
+	status = "okay";
+
+	ports {
+		port at 1 {
+			reg = <1>;
+
+			vga_bridge_out: endpoint {
+				remote-endpoint = <&vga_con_in>;
+			};
+		};
+	};
+};
+
+&vgadisplay {
+	status = "okay";
+};
+
 &iomuxc {
 	/*
 	 * Mux the Apalis GPIOs
diff --git a/arch/arm/boot/dts/imx6qdl-apalis.dtsi b/arch/arm/boot/dts/imx6qdl-apalis.dtsi
index 05f07ea3e8c8..e8d0407e3e18 100644
--- a/arch/arm/boot/dts/imx6qdl-apalis.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-apalis.dtsi
@@ -138,6 +138,54 @@
 		spdif-out;
 		status = "disabled";
 	};
+
+	vgabridge: bridge {
+		compatible = "dumb-vga-dac";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		de-active = <0>;
+		status = "disabled";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port at 0 {
+				reg = <0>;
+
+				vga_bridge_in: endpoint {
+					remote-endpoint = <&vga_display_out>;
+				};
+			};
+		};
+	};
+
+	vgadisplay: display at di0 {
+		compatible = "fsl,imx-parallel-display";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		interface-pix-fmt = "rgb565";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_ipu2_vdac>;
+		status = "disabled";
+
+		port at 0 {
+			reg = <0>;
+
+			vga_display_in: endpoint {
+				remote-endpoint = <&ipu2_di0_disp0>;
+			};
+		};
+
+		port at 1 {
+			reg = <1>;
+
+			vga_display_out: endpoint {
+				remote-endpoint = <&vga_bridge_in>;
+			};
+		};
+	};
+
 };
 
 &audmux {
@@ -373,6 +421,10 @@
 	status = "disabled";
 };
 
+&ipu2_di0_disp0 {
+	remote-endpoint = <&vga_display_in>;
+};
+
 &pwm1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_pwm1>;
-- 
2.18.0

[PATCH v2 8/8] ARM: dts: imx6qdl-apalis: add GPIO I2C node for DDC

[Stefan Agner]

Currently, the DDC signals are controlled by the DWC HDMI I2C
master to be used for HDMI (DVI on the Apalis Evaluation Board).
However, the Apalis Evaluation board also allows to route the Apalis
DDC I2C to the LVDS or the VGA connector. By default, the signal
is routed to DVI (HDMI), and therefor the current default settings
are sensible.

To ease customization and to prepare for carrier boards with other
needs, add a GPIO I2C DDC node.

E.g. to reroute the Apalis DDC to the VGA connector on the Apalis
Evaluation Board, the following changes can be used:

vga {
	...
	ddc-i2c-bus = <&i2cddc>;
};

&hdmi {
	/delete-property/ pinctrl-0;
};

&i2cddc {
	status = "okay";
};

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 arch/arm/boot/dts/imx6qdl-apalis.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl-apalis.dtsi b/arch/arm/boot/dts/imx6qdl-apalis.dtsi
index e8d0407e3e18..8340391796df 100644
--- a/arch/arm/boot/dts/imx6qdl-apalis.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-apalis.dtsi
@@ -61,6 +61,18 @@
 		status = "disabled";
 	};
 
+	/* DDC_I2C: I2C2_SDA/SCL on MXM3 205/207 */
+	i2cddc: i2c@0 {
+		compatible = "i2c-gpio";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_i2c_ddc>;
+		gpios = <&gpio3 16 GPIO_ACTIVE_HIGH /* sda */
+			 &gpio2 30 GPIO_ACTIVE_HIGH /* scl */
+			>;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		status = "disabled";
+	};
+
 	reg_module_3v3: regulator-module-3v3 {
 		compatible = "regulator-fixed";
 		regulator-name = "+V3.3";
@@ -688,6 +700,14 @@
 		>;
 	};
 
+	pinctrl_i2c_ddc: gpioi2cddcgrp {
+		fsl,pins = <
+			/* DDC bitbang */
+			MX6QDL_PAD_EIM_EB2__GPIO2_IO30 0x1b0b0
+			MX6QDL_PAD_EIM_D16__GPIO3_IO16 0x1b0b0
+		>;
+	};
+
 	pinctrl_i2c1: i2c1grp {
 		fsl,pins = <
 			MX6QDL_PAD_CSI0_DAT8__I2C1_SDA 0x4001b8b1
-- 
2.18.0

[PATCH v2 8/8] ARM: dts: imx6qdl-apalis: add GPIO I2C node for DDC

[Stefan Agner]

Currently, the DDC signals are controlled by the DWC HDMI I2C
master to be used for HDMI (DVI on the Apalis Evaluation Board).
However, the Apalis Evaluation board also allows to route the Apalis
DDC I2C to the LVDS or the VGA connector. By default, the signal
is routed to DVI (HDMI), and therefor the current default settings
are sensible.

To ease customization and to prepare for carrier boards with other
needs, add a GPIO I2C DDC node.

E.g. to reroute the Apalis DDC to the VGA connector on the Apalis
Evaluation Board, the following changes can be used:

vga {
	...
	ddc-i2c-bus = <&i2cddc>;
};

&hdmi {
	/delete-property/ pinctrl-0;
};

&i2cddc {
	status = "okay";
};

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 arch/arm/boot/dts/imx6qdl-apalis.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl-apalis.dtsi b/arch/arm/boot/dts/imx6qdl-apalis.dtsi
index e8d0407e3e18..8340391796df 100644
--- a/arch/arm/boot/dts/imx6qdl-apalis.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-apalis.dtsi
@@ -61,6 +61,18 @@
 		status = "disabled";
 	};
 
+	/* DDC_I2C: I2C2_SDA/SCL on MXM3 205/207 */
+	i2cddc: i2c at 0 {
+		compatible = "i2c-gpio";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_i2c_ddc>;
+		gpios = <&gpio3 16 GPIO_ACTIVE_HIGH /* sda */
+			 &gpio2 30 GPIO_ACTIVE_HIGH /* scl */
+			>;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		status = "disabled";
+	};
+
 	reg_module_3v3: regulator-module-3v3 {
 		compatible = "regulator-fixed";
 		regulator-name = "+V3.3";
@@ -688,6 +700,14 @@
 		>;
 	};
 
+	pinctrl_i2c_ddc: gpioi2cddcgrp {
+		fsl,pins = <
+			/* DDC bitbang */
+			MX6QDL_PAD_EIM_EB2__GPIO2_IO30 0x1b0b0
+			MX6QDL_PAD_EIM_D16__GPIO3_IO16 0x1b0b0
+		>;
+	};
+
 	pinctrl_i2c1: i2c1grp {
 		fsl,pins = <
 			MX6QDL_PAD_CSI0_DAT8__I2C1_SDA 0x4001b8b1
-- 
2.18.0

Re: [PATCH v2 6/8] drm/imx: support handling bridge timings bus flags

[Philipp Zabel]

On Wed, 2018-09-12 at 11:32 -0700, Stefan Agner wrote:
> A bridge might require specific settings for the pixel data on
> the bus.

On which bus? The bridge has input and output.

> Copy the bus flags from the bridge timings if a bridge
> is in use.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/gpu/drm/imx/parallel-display.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
> index aefd04e18f93..7798a0621df7 100644
> --- a/drivers/gpu/drm/imx/parallel-display.c
> +++ b/drivers/gpu/drm/imx/parallel-display.c
> @@ -239,6 +239,9 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data)
>  	if (ret && ret != -ENODEV)
>  		return ret;
>  
> +	if (imxpd->bridge && imxpd->bridge->timings)
> +		imxpd->bus_flags = imxpd->bridge->timings->input_bus_flags;

Oh, ok. I'd also specify input bus in the commit message.

> +
>  	imxpd->dev = dev;
>  
>  	ret = imx_pd_register(drm, imxpd);

regards
Philipp

Re: [PATCH v2 6/8] drm/imx: support handling bridge timings bus flags

[Philipp Zabel]

On Wed, 2018-09-12 at 11:32 -0700, Stefan Agner wrote:
> A bridge might require specific settings for the pixel data on
> the bus.

On which bus? The bridge has input and output.

> Copy the bus flags from the bridge timings if a bridge
> is in use.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/gpu/drm/imx/parallel-display.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
> index aefd04e18f93..7798a0621df7 100644
> --- a/drivers/gpu/drm/imx/parallel-display.c
> +++ b/drivers/gpu/drm/imx/parallel-display.c
> @@ -239,6 +239,9 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data)
>  	if (ret && ret != -ENODEV)
>  		return ret;
>  
> +	if (imxpd->bridge && imxpd->bridge->timings)
> +		imxpd->bus_flags = imxpd->bridge->timings->input_bus_flags;

Oh, ok. I'd also specify input bus in the commit message.

> +
>  	imxpd->dev = dev;
>  
>  	ret = imx_pd_register(drm, imxpd);

regards
Philipp
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 6/8] drm/imx: support handling bridge timings bus flags

[Philipp Zabel]

On Wed, 2018-09-12 at 11:32 -0700, Stefan Agner wrote:
> A bridge might require specific settings for the pixel data on
> the bus.

On which bus? The bridge has input and output.

> Copy the bus flags from the bridge timings if a bridge
> is in use.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/gpu/drm/imx/parallel-display.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
> index aefd04e18f93..7798a0621df7 100644
> --- a/drivers/gpu/drm/imx/parallel-display.c
> +++ b/drivers/gpu/drm/imx/parallel-display.c
> @@ -239,6 +239,9 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data)
>  	if (ret && ret != -ENODEV)
>  		return ret;
>  
> +	if (imxpd->bridge && imxpd->bridge->timings)
> +		imxpd->bus_flags = imxpd->bridge->timings->input_bus_flags;

Oh, ok. I'd also specify input bus in the commit message.

> +
>  	imxpd->dev = dev;
>  
>  	ret = imx_pd_register(drm, imxpd);

regards
Philipp

Re: [PATCH v2 6/8] drm/imx: support handling bridge timings bus flags

[Stefan Agner]

On 13.09.2018 01:38, Philipp Zabel wrote:
> On Wed, 2018-09-12 at 11:32 -0700, Stefan Agner wrote:
>> A bridge might require specific settings for the pixel data on
>> the bus.
> 
> On which bus? The bridge has input and output.
> 
>> Copy the bus flags from the bridge timings if a bridge
>> is in use.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  drivers/gpu/drm/imx/parallel-display.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
>> index aefd04e18f93..7798a0621df7 100644
>> --- a/drivers/gpu/drm/imx/parallel-display.c
>> +++ b/drivers/gpu/drm/imx/parallel-display.c
>> @@ -239,6 +239,9 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data)
>>  	if (ret && ret != -ENODEV)
>>  		return ret;
>>
>> +	if (imxpd->bridge && imxpd->bridge->timings)
>> +		imxpd->bus_flags = imxpd->bridge->timings->input_bus_flags;
> 
> Oh, ok. I'd also specify input bus in the commit message.
> 

Good point, will change in v3 to:

"A bridge might require specific settings for the pixel data on
its input bus. Those are specified through optional bus timings.
Copy the bridges input bus flags as to the imxpd bus flags."

--
Stefan

>> +
>>  	imxpd->dev = dev;
>>
>>  	ret = imx_pd_register(drm, imxpd);
> 
> regards
> Philipp

Re: [PATCH v2 6/8] drm/imx: support handling bridge timings bus flags

[Stefan Agner]

On 13.09.2018 01:38, Philipp Zabel wrote:
> On Wed, 2018-09-12 at 11:32 -0700, Stefan Agner wrote:
>> A bridge might require specific settings for the pixel data on
>> the bus.
> 
> On which bus? The bridge has input and output.
> 
>> Copy the bus flags from the bridge timings if a bridge
>> is in use.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  drivers/gpu/drm/imx/parallel-display.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
>> index aefd04e18f93..7798a0621df7 100644
>> --- a/drivers/gpu/drm/imx/parallel-display.c
>> +++ b/drivers/gpu/drm/imx/parallel-display.c
>> @@ -239,6 +239,9 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data)
>>  	if (ret && ret != -ENODEV)
>>  		return ret;
>>
>> +	if (imxpd->bridge && imxpd->bridge->timings)
>> +		imxpd->bus_flags = imxpd->bridge->timings->input_bus_flags;
> 
> Oh, ok. I'd also specify input bus in the commit message.
> 

Good point, will change in v3 to:

"A bridge might require specific settings for the pixel data on
its input bus. Those are specified through optional bus timings.
Copy the bridges input bus flags as to the imxpd bus flags."

--
Stefan

>> +
>>  	imxpd->dev = dev;
>>
>>  	ret = imx_pd_register(drm, imxpd);
> 
> regards
> Philipp
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 6/8] drm/imx: support handling bridge timings bus flags

[Stefan Agner]

On 13.09.2018 01:38, Philipp Zabel wrote:
> On Wed, 2018-09-12 at 11:32 -0700, Stefan Agner wrote:
>> A bridge might require specific settings for the pixel data on
>> the bus.
> 
> On which bus? The bridge has input and output.
> 
>> Copy the bus flags from the bridge timings if a bridge
>> is in use.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  drivers/gpu/drm/imx/parallel-display.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
>> index aefd04e18f93..7798a0621df7 100644
>> --- a/drivers/gpu/drm/imx/parallel-display.c
>> +++ b/drivers/gpu/drm/imx/parallel-display.c
>> @@ -239,6 +239,9 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data)
>>  	if (ret && ret != -ENODEV)
>>  		return ret;
>>
>> +	if (imxpd->bridge && imxpd->bridge->timings)
>> +		imxpd->bus_flags = imxpd->bridge->timings->input_bus_flags;
> 
> Oh, ok. I'd also specify input bus in the commit message.
> 

Good point, will change in v3 to:

"A bridge might require specific settings for the pixel data on
its input bus. Those are specified through optional bus timings.
Copy the bridges input bus flags as to the imxpd bus flags."

--
Stefan

>> +
>>  	imxpd->dev = dev;
>>
>>  	ret = imx_pd_register(drm, imxpd);
> 
> regards
> Philipp

Re: [PATCH v2 0/8] drm/bridge: add bus flag support

[Linus Walleij]

On Wed, Sep 12, 2018 at 8:32 PM Stefan Agner <stefan@agner.ch> wrote:

> @Linus Walleij, Laurent Pinchart: Since you have been involved in
> the initial bus timings discussion, I would like to have your Ack
> on at least patch 2/3... If we want to keep the setup/hold timings,
> the patchset should also work without those two patches.

AFAICT this ends up with the PL111 code identical to what I
proposed in my first patch set to support the latching properly,
so I am pretty much happy as long as my hardware does not
flicker. (Rough consensus and running code.) I'm not very invested
in either idea to represent these timings, so
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v2 0/8] drm/bridge: add bus flag support

[Linus Walleij]

On Wed, Sep 12, 2018 at 8:32 PM Stefan Agner <stefan@agner.ch> wrote:

> @Linus Walleij, Laurent Pinchart: Since you have been involved in
> the initial bus timings discussion, I would like to have your Ack
> on at least patch 2/3... If we want to keep the setup/hold timings,
> the patchset should also work without those two patches.

AFAICT this ends up with the PL111 code identical to what I
proposed in my first patch set to support the latching properly,
so I am pretty much happy as long as my hardware does not
flicker. (Rough consensus and running code.) I'm not very invested
in either idea to represent these timings, so
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v2 0/8] drm/bridge: add bus flag support

[Linus Walleij]

On Wed, Sep 12, 2018 at 8:32 PM Stefan Agner <stefan@agner.ch> wrote:

> @Linus Walleij, Laurent Pinchart: Since you have been involved in
> the initial bus timings discussion, I would like to have your Ack
> on at least patch 2/3... If we want to keep the setup/hold timings,
> the patchset should also work without those two patches.

AFAICT this ends up with the PL111 code identical to what I
proposed in my first patch set to support the latching properly,
so I am pretty much happy as long as my hardware does not
flicker. (Rough consensus and running code.) I'm not very invested
in either idea to represent these timings, so
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v2 1/8] drm/bridge: use bus flags in bridge timings

[Laurent Pinchart]

Hi Stefan,

Thankk you for the patch.

On Wednesday, 12 September 2018 21:32:15 EEST Stefan Agner wrote:
> The DRM bus flags conveys additional information on pixel data on
> the bus. All currently available bus flags might be of interest for
> a bridge. In the case at hand a dumb VGA bridge needs a specific
> data enable polarity (DRM_BUS_FLAG_DE_LOW).
> 
> Replace the sampling_edge field with input_bus_flags and allow all
> currently documented bus flags.
> 
> This changes the perspective from sampling side to the driving
> side for the currently supported flags. We assume that the sampling
> edge is always the opposite of the driving edge (hence we need to
> invert the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE flags). This is an
> assumption we already make for displays. For all we know it is a
> safe assumption for bridges too.

Please don't, that will get confusing very quickly. Flag names such as 
DRM_BUS_FLAG_PIXDATA_NEGEDGE don't mention sampling or driving. There's only a 
small comment next to their definition, which will easily be overlooked. I'd 
rather update the definition to cover both sampling and driving, and document 
the input_bus_flags field to explain that all flags refer to sampling.

> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/gpu/drm/bridge/dumb-vga-dac.c |  6 +++---
>  include/drm/drm_bridge.h              | 11 +++++------
>  2 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> b/drivers/gpu/drm/bridge/dumb-vga-dac.c index 9b706789a341..d5aa0f931ef2
> 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
> */
>  static const struct drm_bridge_timings default_dac_timings = {
>  	/* Timing specifications, datasheet page 7 */
> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
>  	.setup_time_ps = 500,
>  	.hold_time_ps = 1500,
>  };
> @@ -245,7 +245,7 @@ static const struct drm_bridge_timings
> default_dac_timings = { */
>  static const struct drm_bridge_timings ti_ths8134_dac_timings = {
>  	/* From timing diagram, datasheet page 9 */
> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
>  	/* From datasheet, page 12 */
>  	.setup_time_ps = 3000,
>  	/* I guess this means latched input */
> @@ -258,7 +258,7 @@ static const struct drm_bridge_timings
> ti_ths8134_dac_timings = { */
>  static const struct drm_bridge_timings ti_ths8135_dac_timings = {
>  	/* From timing diagram, datasheet page 14 */
> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
>  	/* From datasheet, page 16 */
>  	.setup_time_ps = 2000,
>  	.hold_time_ps = 500,
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index bd850747ce54..45e90f4b46c3 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -244,14 +244,13 @@ struct drm_bridge_funcs {
>   */
>  struct drm_bridge_timings {
>  	/**
> -	 * @sampling_edge:
> +	 * @input_bus_flags:
>  	 *
> -	 * Tells whether the bridge samples the digital input signal
> -	 * from the display engine on the positive or negative edge of the
> -	 * clock, this should reuse the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE
> -	 * bitwise flags from the DRM connector (bit 2 and 3 valid).
> +	 * Additional settings this bridge requires for the pixel data on
> +	 * the input bus (e.g. pixel signal polarity). See also
> +	 * &drm_display_info->bus_flags.
>  	 */
> -	u32 sampling_edge;
> +	u32 input_bus_flags;
>  	/**
>  	 * @setup_time_ps:
>  	 *

-- 
Regards,

Laurent Pinchart

[PATCH v2 1/8] drm/bridge: use bus flags in bridge timings

[Laurent Pinchart]

Hi Stefan,

Thankk you for the patch.

On Wednesday, 12 September 2018 21:32:15 EEST Stefan Agner wrote:
> The DRM bus flags conveys additional information on pixel data on
> the bus. All currently available bus flags might be of interest for
> a bridge. In the case at hand a dumb VGA bridge needs a specific
> data enable polarity (DRM_BUS_FLAG_DE_LOW).
> 
> Replace the sampling_edge field with input_bus_flags and allow all
> currently documented bus flags.
> 
> This changes the perspective from sampling side to the driving
> side for the currently supported flags. We assume that the sampling
> edge is always the opposite of the driving edge (hence we need to
> invert the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE flags). This is an
> assumption we already make for displays. For all we know it is a
> safe assumption for bridges too.

Please don't, that will get confusing very quickly. Flag names such as 
DRM_BUS_FLAG_PIXDATA_NEGEDGE don't mention sampling or driving. There's only a 
small comment next to their definition, which will easily be overlooked. I'd 
rather update the definition to cover both sampling and driving, and document 
the input_bus_flags field to explain that all flags refer to sampling.

> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/gpu/drm/bridge/dumb-vga-dac.c |  6 +++---
>  include/drm/drm_bridge.h              | 11 +++++------
>  2 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> b/drivers/gpu/drm/bridge/dumb-vga-dac.c index 9b706789a341..d5aa0f931ef2
> 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
> */
>  static const struct drm_bridge_timings default_dac_timings = {
>  	/* Timing specifications, datasheet page 7 */
> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
>  	.setup_time_ps = 500,
>  	.hold_time_ps = 1500,
>  };
> @@ -245,7 +245,7 @@ static const struct drm_bridge_timings
> default_dac_timings = { */
>  static const struct drm_bridge_timings ti_ths8134_dac_timings = {
>  	/* From timing diagram, datasheet page 9 */
> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
>  	/* From datasheet, page 12 */
>  	.setup_time_ps = 3000,
>  	/* I guess this means latched input */
> @@ -258,7 +258,7 @@ static const struct drm_bridge_timings
> ti_ths8134_dac_timings = { */
>  static const struct drm_bridge_timings ti_ths8135_dac_timings = {
>  	/* From timing diagram, datasheet page 14 */
> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
>  	/* From datasheet, page 16 */
>  	.setup_time_ps = 2000,
>  	.hold_time_ps = 500,
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index bd850747ce54..45e90f4b46c3 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -244,14 +244,13 @@ struct drm_bridge_funcs {
>   */
>  struct drm_bridge_timings {
>  	/**
> -	 * @sampling_edge:
> +	 * @input_bus_flags:
>  	 *
> -	 * Tells whether the bridge samples the digital input signal
> -	 * from the display engine on the positive or negative edge of the
> -	 * clock, this should reuse the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE
> -	 * bitwise flags from the DRM connector (bit 2 and 3 valid).
> +	 * Additional settings this bridge requires for the pixel data on
> +	 * the input bus (e.g. pixel signal polarity). See also
> +	 * &drm_display_info->bus_flags.
>  	 */
> -	u32 sampling_edge;
> +	u32 input_bus_flags;
>  	/**
>  	 * @setup_time_ps:
>  	 *

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 3/8] drm/bridge: simplify bridge timing info

[Laurent Pinchart]

Hi Stefan,

On Wednesday, 12 September 2018 21:32:17 EEST Stefan Agner wrote:
> Bridges are typically connected to a parallel display signal with
> pixel clock, sync signals and data lines. Parallel display signals
> are also used in lower-end embedded display panels. For parallel
> display panels we currently do not specify setup/hold times. From
> discussions on the mailing list it seems not convincing that this
> is currently really required for bridges either.
> 
> Remove setup/hold timings again to better align timing information
> of displays and briges.

The setup and hold timings were the result of a long discussion with Linux 
Walleij, who was confronted with a system that didn't work properly unless he 
flipped the clock polarity. His initial patch contradicted the bridge 
datasheet, and after investigating we found out that timings played a major 
role.

In a nutshell, given the setup and hold time requirements, the polarity on the 
driving side depends on the pixel clock frequency. As the frequency increases, 
when the half clock period reaches the setup time, you can't latch on the 
opposite edge anymore or you will violate the setup time. The component 
connected to the bridge thus needs to select a driving edge based on the 
sampling edge of the bridge, on the bridge's setup time requirement, and on 
the pixel clock frequency.

> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/gpu/drm/bridge/dumb-vga-dac.c | 17 +++++------------
>  include/drm/drm_bridge.h              | 14 --------------
>  2 files changed, 5 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> b/drivers/gpu/drm/bridge/dumb-vga-dac.c index d5aa0f931ef2..b2309ad228cf
> 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -229,14 +229,14 @@ static int dumb_vga_remove(struct platform_device
> *pdev) /*
>   * We assume the ADV7123 DAC is the "default" for historical reasons
>   * Information taken from the ADV7123 datasheet, revision D.
> - * NOTE: the ADV7123EP seems to have other timings and need a new timings
> - * set if used.
>   */
>  static const struct drm_bridge_timings default_dac_timings = {
> -	/* Timing specifications, datasheet page 7 */
> +	/*
> +	 * From Timing diagram, datasheet page 7. The bridge samples
> +	 * on pixel clocks positive edge, hence the display controller
> +	 * should drive signals on the negative edge.
> +	 */
>  	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> -	.setup_time_ps = 500,
> -	.hold_time_ps = 1500,
>  };
> 
>  /*
> @@ -246,10 +246,6 @@ static const struct drm_bridge_timings
> default_dac_timings = { static const struct drm_bridge_timings
> ti_ths8134_dac_timings = { /* From timing diagram, datasheet page 9 */
>  	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> -	/* From datasheet, page 12 */
> -	.setup_time_ps = 3000,
> -	/* I guess this means latched input */
> -	.hold_time_ps = 0,
>  };
> 
>  /*
> @@ -259,9 +255,6 @@ static const struct drm_bridge_timings
> ti_ths8134_dac_timings = { static const struct drm_bridge_timings
> ti_ths8135_dac_timings = { /* From timing diagram, datasheet page 14 */
>  	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> -	/* From datasheet, page 16 */
> -	.setup_time_ps = 2000,
> -	.hold_time_ps = 500,
>  };
> 
>  static const struct of_device_id dumb_vga_match[] = {
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 45e90f4b46c3..1a1d08350eaf 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -251,20 +251,6 @@ struct drm_bridge_timings {
>  	 * &drm_display_info->bus_flags.
>  	 */
>  	u32 input_bus_flags;
> -	/**
> -	 * @setup_time_ps:
> -	 *
> -	 * Defines the time in picoseconds the input data lines must be
> -	 * stable before the clock edge.
> -	 */
> -	u32 setup_time_ps;
> -	/**
> -	 * @hold_time_ps:
> -	 *
> -	 * Defines the time in picoseconds taken for the bridge to sample the
> -	 * input signal after the clock edge.
> -	 */
> -	u32 hold_time_ps;
>  };
> 
>  /**


-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 3/8] drm/bridge: simplify bridge timing info

[Laurent Pinchart]

Hi Stefan,

On Wednesday, 12 September 2018 21:32:17 EEST Stefan Agner wrote:
> Bridges are typically connected to a parallel display signal with
> pixel clock, sync signals and data lines. Parallel display signals
> are also used in lower-end embedded display panels. For parallel
> display panels we currently do not specify setup/hold times. From
> discussions on the mailing list it seems not convincing that this
> is currently really required for bridges either.
> 
> Remove setup/hold timings again to better align timing information
> of displays and briges.

The setup and hold timings were the result of a long discussion with Linux 
Walleij, who was confronted with a system that didn't work properly unless he 
flipped the clock polarity. His initial patch contradicted the bridge 
datasheet, and after investigating we found out that timings played a major 
role.

In a nutshell, given the setup and hold time requirements, the polarity on the 
driving side depends on the pixel clock frequency. As the frequency increases, 
when the half clock period reaches the setup time, you can't latch on the 
opposite edge anymore or you will violate the setup time. The component 
connected to the bridge thus needs to select a driving edge based on the 
sampling edge of the bridge, on the bridge's setup time requirement, and on 
the pixel clock frequency.

> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/gpu/drm/bridge/dumb-vga-dac.c | 17 +++++------------
>  include/drm/drm_bridge.h              | 14 --------------
>  2 files changed, 5 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> b/drivers/gpu/drm/bridge/dumb-vga-dac.c index d5aa0f931ef2..b2309ad228cf
> 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -229,14 +229,14 @@ static int dumb_vga_remove(struct platform_device
> *pdev) /*
>   * We assume the ADV7123 DAC is the "default" for historical reasons
>   * Information taken from the ADV7123 datasheet, revision D.
> - * NOTE: the ADV7123EP seems to have other timings and need a new timings
> - * set if used.
>   */
>  static const struct drm_bridge_timings default_dac_timings = {
> -	/* Timing specifications, datasheet page 7 */
> +	/*
> +	 * From Timing diagram, datasheet page 7. The bridge samples
> +	 * on pixel clocks positive edge, hence the display controller
> +	 * should drive signals on the negative edge.
> +	 */
>  	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> -	.setup_time_ps = 500,
> -	.hold_time_ps = 1500,
>  };
> 
>  /*
> @@ -246,10 +246,6 @@ static const struct drm_bridge_timings
> default_dac_timings = { static const struct drm_bridge_timings
> ti_ths8134_dac_timings = { /* From timing diagram, datasheet page 9 */
>  	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> -	/* From datasheet, page 12 */
> -	.setup_time_ps = 3000,
> -	/* I guess this means latched input */
> -	.hold_time_ps = 0,
>  };
> 
>  /*
> @@ -259,9 +255,6 @@ static const struct drm_bridge_timings
> ti_ths8134_dac_timings = { static const struct drm_bridge_timings
> ti_ths8135_dac_timings = { /* From timing diagram, datasheet page 14 */
>  	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> -	/* From datasheet, page 16 */
> -	.setup_time_ps = 2000,
> -	.hold_time_ps = 500,
>  };
> 
>  static const struct of_device_id dumb_vga_match[] = {
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 45e90f4b46c3..1a1d08350eaf 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -251,20 +251,6 @@ struct drm_bridge_timings {
>  	 * &drm_display_info->bus_flags.
>  	 */
>  	u32 input_bus_flags;
> -	/**
> -	 * @setup_time_ps:
> -	 *
> -	 * Defines the time in picoseconds the input data lines must be
> -	 * stable before the clock edge.
> -	 */
> -	u32 setup_time_ps;
> -	/**
> -	 * @hold_time_ps:
> -	 *
> -	 * Defines the time in picoseconds taken for the bridge to sample the
> -	 * input signal after the clock edge.
> -	 */
> -	u32 hold_time_ps;
>  };
> 
>  /**


-- 
Regards,

Laurent Pinchart



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 3/8] drm/bridge: simplify bridge timing info

[Laurent Pinchart]

Hi Stefan,

On Wednesday, 12 September 2018 21:32:17 EEST Stefan Agner wrote:
> Bridges are typically connected to a parallel display signal with
> pixel clock, sync signals and data lines. Parallel display signals
> are also used in lower-end embedded display panels. For parallel
> display panels we currently do not specify setup/hold times. From
> discussions on the mailing list it seems not convincing that this
> is currently really required for bridges either.
> 
> Remove setup/hold timings again to better align timing information
> of displays and briges.

The setup and hold timings were the result of a long discussion with Linux 
Walleij, who was confronted with a system that didn't work properly unless he 
flipped the clock polarity. His initial patch contradicted the bridge 
datasheet, and after investigating we found out that timings played a major 
role.

In a nutshell, given the setup and hold time requirements, the polarity on the 
driving side depends on the pixel clock frequency. As the frequency increases, 
when the half clock period reaches the setup time, you can't latch on the 
opposite edge anymore or you will violate the setup time. The component 
connected to the bridge thus needs to select a driving edge based on the 
sampling edge of the bridge, on the bridge's setup time requirement, and on 
the pixel clock frequency.

> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/gpu/drm/bridge/dumb-vga-dac.c | 17 +++++------------
>  include/drm/drm_bridge.h              | 14 --------------
>  2 files changed, 5 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> b/drivers/gpu/drm/bridge/dumb-vga-dac.c index d5aa0f931ef2..b2309ad228cf
> 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -229,14 +229,14 @@ static int dumb_vga_remove(struct platform_device
> *pdev) /*
>   * We assume the ADV7123 DAC is the "default" for historical reasons
>   * Information taken from the ADV7123 datasheet, revision D.
> - * NOTE: the ADV7123EP seems to have other timings and need a new timings
> - * set if used.
>   */
>  static const struct drm_bridge_timings default_dac_timings = {
> -	/* Timing specifications, datasheet page 7 */
> +	/*
> +	 * From Timing diagram, datasheet page 7. The bridge samples
> +	 * on pixel clocks positive edge, hence the display controller
> +	 * should drive signals on the negative edge.
> +	 */
>  	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> -	.setup_time_ps = 500,
> -	.hold_time_ps = 1500,
>  };
> 
>  /*
> @@ -246,10 +246,6 @@ static const struct drm_bridge_timings
> default_dac_timings = { static const struct drm_bridge_timings
> ti_ths8134_dac_timings = { /* From timing diagram, datasheet page 9 */
>  	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> -	/* From datasheet, page 12 */
> -	.setup_time_ps = 3000,
> -	/* I guess this means latched input */
> -	.hold_time_ps = 0,
>  };
> 
>  /*
> @@ -259,9 +255,6 @@ static const struct drm_bridge_timings
> ti_ths8134_dac_timings = { static const struct drm_bridge_timings
> ti_ths8135_dac_timings = { /* From timing diagram, datasheet page 14 */
>  	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> -	/* From datasheet, page 16 */
> -	.setup_time_ps = 2000,
> -	.hold_time_ps = 500,
>  };
> 
>  static const struct of_device_id dumb_vga_match[] = {
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 45e90f4b46c3..1a1d08350eaf 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -251,20 +251,6 @@ struct drm_bridge_timings {
>  	 * &drm_display_info->bus_flags.
>  	 */
>  	u32 input_bus_flags;
> -	/**
> -	 * @setup_time_ps:
> -	 *
> -	 * Defines the time in picoseconds the input data lines must be
> -	 * stable before the clock edge.
> -	 */
> -	u32 setup_time_ps;
> -	/**
> -	 * @hold_time_ps:
> -	 *
> -	 * Defines the time in picoseconds taken for the bridge to sample the
> -	 * input signal after the clock edge.
> -	 */
> -	u32 hold_time_ps;
>  };
> 
>  /**


-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 0/8] drm/bridge: add bus flag support

[Laurent Pinchart]

Hello,

On Friday, 14 September 2018 12:07:03 EEST Linus Walleij wrote:
> On Wed, Sep 12, 2018 at 8:32 PM Stefan Agner <stefan@agner.ch> wrote:
> > @Linus Walleij, Laurent Pinchart: Since you have been involved in
> > the initial bus timings discussion, I would like to have your Ack
> > on at least patch 2/3... If we want to keep the setup/hold timings,
> > the patchset should also work without those two patches.
> 
> AFAICT this ends up with the PL111 code identical to what I
> proposed in my first patch set to support the latching properly,
> so I am pretty much happy as long as my hardware does not
> flicker. (Rough consensus and running code.) I'm not very invested
> in either idea to represent these timings, so
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

I've just replied to patch 3/8 with an explanation of why we have added this 
particular API, and why we need to keep it.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 0/8] drm/bridge: add bus flag support

[Laurent Pinchart]

Hello,

On Friday, 14 September 2018 12:07:03 EEST Linus Walleij wrote:
> On Wed, Sep 12, 2018 at 8:32 PM Stefan Agner <stefan@agner.ch> wrote:
> > @Linus Walleij, Laurent Pinchart: Since you have been involved in
> > the initial bus timings discussion, I would like to have your Ack
> > on at least patch 2/3... If we want to keep the setup/hold timings,
> > the patchset should also work without those two patches.
> 
> AFAICT this ends up with the PL111 code identical to what I
> proposed in my first patch set to support the latching properly,
> so I am pretty much happy as long as my hardware does not
> flicker. (Rough consensus and running code.) I'm not very invested
> in either idea to represent these timings, so
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

I've just replied to patch 3/8 with an explanation of why we have added this 
particular API, and why we need to keep it.

-- 
Regards,

Laurent Pinchart



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 0/8] drm/bridge: add bus flag support

[Laurent Pinchart]

Hello,

On Friday, 14 September 2018 12:07:03 EEST Linus Walleij wrote:
> On Wed, Sep 12, 2018 at 8:32 PM Stefan Agner <stefan@agner.ch> wrote:
> > @Linus Walleij, Laurent Pinchart: Since you have been involved in
> > the initial bus timings discussion, I would like to have your Ack
> > on at least patch 2/3... If we want to keep the setup/hold timings,
> > the patchset should also work without those two patches.
> 
> AFAICT this ends up with the PL111 code identical to what I
> proposed in my first patch set to support the latching properly,
> so I am pretty much happy as long as my hardware does not
> flicker. (Rough consensus and running code.) I'm not very invested
> in either idea to represent these timings, so
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

I've just replied to patch 3/8 with an explanation of why we have added this 
particular API, and why we need to keep it.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 6/8] drm/imx: support handling bridge timings bus flags

[Laurent Pinchart]

Hi Stefan,

Thank you for the patch.

On Wednesday, 12 September 2018 21:32:20 EEST Stefan Agner wrote:
> A bridge might require specific settings for the pixel data on
> the bus. Copy the bus flags from the bridge timings if a bridge
> is in use.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/gpu/drm/imx/parallel-display.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/imx/parallel-display.c
> b/drivers/gpu/drm/imx/parallel-display.c index aefd04e18f93..7798a0621df7
> 100644
> --- a/drivers/gpu/drm/imx/parallel-display.c
> +++ b/drivers/gpu/drm/imx/parallel-display.c
> @@ -239,6 +239,9 @@ static int imx_pd_bind(struct device *dev, struct device
> *master, void *data) if (ret && ret != -ENODEV)
>  		return ret;
> 
> +	if (imxpd->bridge && imxpd->bridge->timings)
> +		imxpd->bus_flags = imxpd->bridge->timings->input_bus_flags;

As explained in different replies in this mail thread (and in v1), we need 
something more complex than this.

How about creating a helper function that would take a sampling edge, setup 
and hold times, clock frequency and internal delay on the driving side, and 
return which edge to drive the data on ? I agree with you that the logic is 
complex, so we shouldn't duplicate it in multiple drivers. If you submit such 
a patch I'll implement support for configuring the clock edge in the R-Car DU 
driver and test it with the ADV7123.

>  	imxpd->dev = dev;
> 
>  	ret = imx_pd_register(drm, imxpd);

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 6/8] drm/imx: support handling bridge timings bus flags

[Laurent Pinchart]

Hi Stefan,

Thank you for the patch.

On Wednesday, 12 September 2018 21:32:20 EEST Stefan Agner wrote:
> A bridge might require specific settings for the pixel data on
> the bus. Copy the bus flags from the bridge timings if a bridge
> is in use.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/gpu/drm/imx/parallel-display.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/imx/parallel-display.c
> b/drivers/gpu/drm/imx/parallel-display.c index aefd04e18f93..7798a0621df7
> 100644
> --- a/drivers/gpu/drm/imx/parallel-display.c
> +++ b/drivers/gpu/drm/imx/parallel-display.c
> @@ -239,6 +239,9 @@ static int imx_pd_bind(struct device *dev, struct device
> *master, void *data) if (ret && ret != -ENODEV)
>  		return ret;
> 
> +	if (imxpd->bridge && imxpd->bridge->timings)
> +		imxpd->bus_flags = imxpd->bridge->timings->input_bus_flags;

As explained in different replies in this mail thread (and in v1), we need 
something more complex than this.

How about creating a helper function that would take a sampling edge, setup 
and hold times, clock frequency and internal delay on the driving side, and 
return which edge to drive the data on ? I agree with you that the logic is 
complex, so we shouldn't duplicate it in multiple drivers. If you submit such 
a patch I'll implement support for configuring the clock edge in the R-Car DU 
driver and test it with the ADV7123.

>  	imxpd->dev = dev;
> 
>  	ret = imx_pd_register(drm, imxpd);

-- 
Regards,

Laurent Pinchart



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 6/8] drm/imx: support handling bridge timings bus flags

[Laurent Pinchart]

Hi Stefan,

Thank you for the patch.

On Wednesday, 12 September 2018 21:32:20 EEST Stefan Agner wrote:
> A bridge might require specific settings for the pixel data on
> the bus. Copy the bus flags from the bridge timings if a bridge
> is in use.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/gpu/drm/imx/parallel-display.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/imx/parallel-display.c
> b/drivers/gpu/drm/imx/parallel-display.c index aefd04e18f93..7798a0621df7
> 100644
> --- a/drivers/gpu/drm/imx/parallel-display.c
> +++ b/drivers/gpu/drm/imx/parallel-display.c
> @@ -239,6 +239,9 @@ static int imx_pd_bind(struct device *dev, struct device
> *master, void *data) if (ret && ret != -ENODEV)
>  		return ret;
> 
> +	if (imxpd->bridge && imxpd->bridge->timings)
> +		imxpd->bus_flags = imxpd->bridge->timings->input_bus_flags;

As explained in different replies in this mail thread (and in v1), we need 
something more complex than this.

How about creating a helper function that would take a sampling edge, setup 
and hold times, clock frequency and internal delay on the driving side, and 
return which edge to drive the data on ? I agree with you that the logic is 
complex, so we shouldn't duplicate it in multiple drivers. If you submit such 
a patch I'll implement support for configuring the clock edge in the R-Car DU 
driver and test it with the ADV7123.

>  	imxpd->dev = dev;
> 
>  	ret = imx_pd_register(drm, imxpd);

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/8] drm/bridge: use bus flags in bridge timings

[Stefan Agner]

On 14.09.2018 02:23, Laurent Pinchart wrote:
> Hi Stefan,
> 
> Thankk you for the patch.
> 
> On Wednesday, 12 September 2018 21:32:15 EEST Stefan Agner wrote:
>> The DRM bus flags conveys additional information on pixel data on
>> the bus. All currently available bus flags might be of interest for
>> a bridge. In the case at hand a dumb VGA bridge needs a specific
>> data enable polarity (DRM_BUS_FLAG_DE_LOW).
>>
>> Replace the sampling_edge field with input_bus_flags and allow all
>> currently documented bus flags.
>>
>> This changes the perspective from sampling side to the driving
>> side for the currently supported flags. We assume that the sampling
>> edge is always the opposite of the driving edge (hence we need to
>> invert the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE flags). This is an
>> assumption we already make for displays. For all we know it is a
>> safe assumption for bridges too.
> 
> Please don't, that will get confusing very quickly. Flag names such as 
> DRM_BUS_FLAG_PIXDATA_NEGEDGE don't mention sampling or driving. There's only a 
> small comment next to their definition, which will easily be overlooked. I'd 
> rather update the definition to cover both sampling and driving, and document 
> the input_bus_flags field to explain that all flags refer to sampling.
> 

There is history to that, and I'd really rather prefer to keep that similar across the kernel. There is already precedence in the kernel, the display timings (which is a consumer) defines it from the driving perspective too (see DISPLAY_FLAGS_PIXDATA_POSEDGE).

That is why I introduced the bus flags using the same perspective.

If we _really_ want it from sampling side, we should create new defines which are explicit about that, e.g.: DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE.

But again, since even the display flags use the driving perspective, I'd rather prefer to have it that way also for bridges too.

--
Stefan


>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  drivers/gpu/drm/bridge/dumb-vga-dac.c |  6 +++---
>>  include/drm/drm_bridge.h              | 11 +++++------
>>  2 files changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> b/drivers/gpu/drm/bridge/dumb-vga-dac.c index 9b706789a341..d5aa0f931ef2
>> 100644
>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> @@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
>> */
>>  static const struct drm_bridge_timings default_dac_timings = {
>>  	/* Timing specifications, datasheet page 7 */
>> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
>> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
>>  	.setup_time_ps = 500,
>>  	.hold_time_ps = 1500,
>>  };
>> @@ -245,7 +245,7 @@ static const struct drm_bridge_timings
>> default_dac_timings = { */
>>  static const struct drm_bridge_timings ti_ths8134_dac_timings = {
>>  	/* From timing diagram, datasheet page 9 */
>> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
>> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
>>  	/* From datasheet, page 12 */
>>  	.setup_time_ps = 3000,
>>  	/* I guess this means latched input */
>> @@ -258,7 +258,7 @@ static const struct drm_bridge_timings
>> ti_ths8134_dac_timings = { */
>>  static const struct drm_bridge_timings ti_ths8135_dac_timings = {
>>  	/* From timing diagram, datasheet page 14 */
>> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
>> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
>>  	/* From datasheet, page 16 */
>>  	.setup_time_ps = 2000,
>>  	.hold_time_ps = 500,
>> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
>> index bd850747ce54..45e90f4b46c3 100644
>> --- a/include/drm/drm_bridge.h
>> +++ b/include/drm/drm_bridge.h
>> @@ -244,14 +244,13 @@ struct drm_bridge_funcs {
>>   */
>>  struct drm_bridge_timings {
>>  	/**
>> -	 * @sampling_edge:
>> +	 * @input_bus_flags:
>>  	 *
>> -	 * Tells whether the bridge samples the digital input signal
>> -	 * from the display engine on the positive or negative edge of the
>> -	 * clock, this should reuse the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE
>> -	 * bitwise flags from the DRM connector (bit 2 and 3 valid).
>> +	 * Additional settings this bridge requires for the pixel data on
>> +	 * the input bus (e.g. pixel signal polarity). See also
>> +	 * &drm_display_info->bus_flags.
>>  	 */
>> -	u32 sampling_edge;
>> +	u32 input_bus_flags;
>>  	/**
>>  	 * @setup_time_ps:
>>  	 *

Re: [PATCH v2 1/8] drm/bridge: use bus flags in bridge timings

[Stefan Agner]

On 14.09.2018 02:23, Laurent Pinchart wrote:
> Hi Stefan,
> 
> Thankk you for the patch.
> 
> On Wednesday, 12 September 2018 21:32:15 EEST Stefan Agner wrote:
>> The DRM bus flags conveys additional information on pixel data on
>> the bus. All currently available bus flags might be of interest for
>> a bridge. In the case at hand a dumb VGA bridge needs a specific
>> data enable polarity (DRM_BUS_FLAG_DE_LOW).
>>
>> Replace the sampling_edge field with input_bus_flags and allow all
>> currently documented bus flags.
>>
>> This changes the perspective from sampling side to the driving
>> side for the currently supported flags. We assume that the sampling
>> edge is always the opposite of the driving edge (hence we need to
>> invert the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE flags). This is an
>> assumption we already make for displays. For all we know it is a
>> safe assumption for bridges too.
> 
> Please don't, that will get confusing very quickly. Flag names such as 
> DRM_BUS_FLAG_PIXDATA_NEGEDGE don't mention sampling or driving. There's only a 
> small comment next to their definition, which will easily be overlooked. I'd 
> rather update the definition to cover both sampling and driving, and document 
> the input_bus_flags field to explain that all flags refer to sampling.
> 

There is history to that, and I'd really rather prefer to keep that similar across the kernel. There is already precedence in the kernel, the display timings (which is a consumer) defines it from the driving perspective too (see DISPLAY_FLAGS_PIXDATA_POSEDGE).

That is why I introduced the bus flags using the same perspective.

If we _really_ want it from sampling side, we should create new defines which are explicit about that, e.g.: DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE.

But again, since even the display flags use the driving perspective, I'd rather prefer to have it that way also for bridges too.

--
Stefan


>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  drivers/gpu/drm/bridge/dumb-vga-dac.c |  6 +++---
>>  include/drm/drm_bridge.h              | 11 +++++------
>>  2 files changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> b/drivers/gpu/drm/bridge/dumb-vga-dac.c index 9b706789a341..d5aa0f931ef2
>> 100644
>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> @@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
>> */
>>  static const struct drm_bridge_timings default_dac_timings = {
>>  	/* Timing specifications, datasheet page 7 */
>> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
>> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
>>  	.setup_time_ps = 500,
>>  	.hold_time_ps = 1500,
>>  };
>> @@ -245,7 +245,7 @@ static const struct drm_bridge_timings
>> default_dac_timings = { */
>>  static const struct drm_bridge_timings ti_ths8134_dac_timings = {
>>  	/* From timing diagram, datasheet page 9 */
>> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
>> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
>>  	/* From datasheet, page 12 */
>>  	.setup_time_ps = 3000,
>>  	/* I guess this means latched input */
>> @@ -258,7 +258,7 @@ static const struct drm_bridge_timings
>> ti_ths8134_dac_timings = { */
>>  static const struct drm_bridge_timings ti_ths8135_dac_timings = {
>>  	/* From timing diagram, datasheet page 14 */
>> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
>> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
>>  	/* From datasheet, page 16 */
>>  	.setup_time_ps = 2000,
>>  	.hold_time_ps = 500,
>> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
>> index bd850747ce54..45e90f4b46c3 100644
>> --- a/include/drm/drm_bridge.h
>> +++ b/include/drm/drm_bridge.h
>> @@ -244,14 +244,13 @@ struct drm_bridge_funcs {
>>   */
>>  struct drm_bridge_timings {
>>  	/**
>> -	 * @sampling_edge:
>> +	 * @input_bus_flags:
>>  	 *
>> -	 * Tells whether the bridge samples the digital input signal
>> -	 * from the display engine on the positive or negative edge of the
>> -	 * clock, this should reuse the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE
>> -	 * bitwise flags from the DRM connector (bit 2 and 3 valid).
>> +	 * Additional settings this bridge requires for the pixel data on
>> +	 * the input bus (e.g. pixel signal polarity). See also
>> +	 * &drm_display_info->bus_flags.
>>  	 */
>> -	u32 sampling_edge;
>> +	u32 input_bus_flags;
>>  	/**
>>  	 * @setup_time_ps:
>>  	 *
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/8] drm/bridge: use bus flags in bridge timings

[Stefan Agner]

On 14.09.2018 02:23, Laurent Pinchart wrote:
> Hi Stefan,
> 
> Thankk you for the patch.
> 
> On Wednesday, 12 September 2018 21:32:15 EEST Stefan Agner wrote:
>> The DRM bus flags conveys additional information on pixel data on
>> the bus. All currently available bus flags might be of interest for
>> a bridge. In the case at hand a dumb VGA bridge needs a specific
>> data enable polarity (DRM_BUS_FLAG_DE_LOW).
>>
>> Replace the sampling_edge field with input_bus_flags and allow all
>> currently documented bus flags.
>>
>> This changes the perspective from sampling side to the driving
>> side for the currently supported flags. We assume that the sampling
>> edge is always the opposite of the driving edge (hence we need to
>> invert the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE flags). This is an
>> assumption we already make for displays. For all we know it is a
>> safe assumption for bridges too.
> 
> Please don't, that will get confusing very quickly. Flag names such as 
> DRM_BUS_FLAG_PIXDATA_NEGEDGE don't mention sampling or driving. There's only a 
> small comment next to their definition, which will easily be overlooked. I'd 
> rather update the definition to cover both sampling and driving, and document 
> the input_bus_flags field to explain that all flags refer to sampling.
> 

There is history to that, and I'd really rather prefer to keep that similar across the kernel. There is already precedence in the kernel, the display timings (which is a consumer) defines it from the driving perspective too (see DISPLAY_FLAGS_PIXDATA_POSEDGE).

That is why I introduced the bus flags using the same perspective.

If we _really_ want it from sampling side, we should create new defines which are explicit about that, e.g.: DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE.

But again, since even the display flags use the driving perspective, I'd rather prefer to have it that way also for bridges too.

--
Stefan


>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  drivers/gpu/drm/bridge/dumb-vga-dac.c |  6 +++---
>>  include/drm/drm_bridge.h              | 11 +++++------
>>  2 files changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> b/drivers/gpu/drm/bridge/dumb-vga-dac.c index 9b706789a341..d5aa0f931ef2
>> 100644
>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> @@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
>> */
>>  static const struct drm_bridge_timings default_dac_timings = {
>>  	/* Timing specifications, datasheet page 7 */
>> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
>> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
>>  	.setup_time_ps = 500,
>>  	.hold_time_ps = 1500,
>>  };
>> @@ -245,7 +245,7 @@ static const struct drm_bridge_timings
>> default_dac_timings = { */
>>  static const struct drm_bridge_timings ti_ths8134_dac_timings = {
>>  	/* From timing diagram, datasheet page 9 */
>> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
>> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
>>  	/* From datasheet, page 12 */
>>  	.setup_time_ps = 3000,
>>  	/* I guess this means latched input */
>> @@ -258,7 +258,7 @@ static const struct drm_bridge_timings
>> ti_ths8134_dac_timings = { */
>>  static const struct drm_bridge_timings ti_ths8135_dac_timings = {
>>  	/* From timing diagram, datasheet page 14 */
>> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
>> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
>>  	/* From datasheet, page 16 */
>>  	.setup_time_ps = 2000,
>>  	.hold_time_ps = 500,
>> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
>> index bd850747ce54..45e90f4b46c3 100644
>> --- a/include/drm/drm_bridge.h
>> +++ b/include/drm/drm_bridge.h
>> @@ -244,14 +244,13 @@ struct drm_bridge_funcs {
>>   */
>>  struct drm_bridge_timings {
>>  	/**
>> -	 * @sampling_edge:
>> +	 * @input_bus_flags:
>>  	 *
>> -	 * Tells whether the bridge samples the digital input signal
>> -	 * from the display engine on the positive or negative edge of the
>> -	 * clock, this should reuse the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE
>> -	 * bitwise flags from the DRM connector (bit 2 and 3 valid).
>> +	 * Additional settings this bridge requires for the pixel data on
>> +	 * the input bus (e.g. pixel signal polarity). See also
>> +	 * &drm_display_info->bus_flags.
>>  	 */
>> -	u32 sampling_edge;
>> +	u32 input_bus_flags;
>>  	/**
>>  	 * @setup_time_ps:
>>  	 *

Re: [PATCH v2 1/8] drm/bridge: use bus flags in bridge timings

[Laurent Pinchart]

Hi Stefan,

On Tuesday, 18 September 2018 21:14:22 EEST Stefan Agner wrote:
> On 14.09.2018 02:23, Laurent Pinchart wrote:
> > On Wednesday, 12 September 2018 21:32:15 EEST Stefan Agner wrote:
> >> The DRM bus flags conveys additional information on pixel data on
> >> the bus. All currently available bus flags might be of interest for
> >> a bridge. In the case at hand a dumb VGA bridge needs a specific
> >> data enable polarity (DRM_BUS_FLAG_DE_LOW).
> >> 
> >> Replace the sampling_edge field with input_bus_flags and allow all
> >> currently documented bus flags.
> >> 
> >> This changes the perspective from sampling side to the driving
> >> side for the currently supported flags. We assume that the sampling
> >> edge is always the opposite of the driving edge (hence we need to
> >> invert the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE flags). This is an
> >> assumption we already make for displays. For all we know it is a
> >> safe assumption for bridges too.
> > 
> > Please don't, that will get confusing very quickly. Flag names such as
> > DRM_BUS_FLAG_PIXDATA_NEGEDGE don't mention sampling or driving. There's
> > only a small comment next to their definition, which will easily be
> > overlooked. I'd rather update the definition to cover both sampling and
> > driving, and document the input_bus_flags field to explain that all flags
> > refer to sampling.
> 
> There is history to that, and I'd really rather prefer to keep that similar
> across the kernel. There is already precedence in the kernel, the display
> timings (which is a consumer) defines it from the driving perspective too
> (see DISPLAY_FLAGS_PIXDATA_POSEDGE).
> 
> That is why I introduced the bus flags using the same perspective.
> 
> If we _really_ want it from sampling side, we should create new defines
> which are explicit about that, e.g.: DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE.

I'm not opposed to that.

> But again, since even the display flags use the driving perspective, I'd
> rather prefer to have it that way also for bridges too.

But look at the following diff:

-	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,

The first line makes it very explicit that the data signals are sampled on the 
rising edge. The second line reads to me as sampling on the negative edge, as 
it reports input information from a bridge perspective. I'll have to read the 
documentation to get it right, and I'm sure I and other will overlook it from 
time to time.

The following would be much more explicit:

	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLING_POSEDGE,

and we could define macros as follows:

/*
 * Don't use those two flags directly, use the DRM_BUS_FLAG_PIXDATA_DRIVE_*
 * and DRM_BUS_FLAG_PIXDATA_SAMPLE_* variants to qualify the flags explicitly.
 * The DRM_BUS_FLAG_PIXDATA_SAMPLE_* flags are defined as the opposite of the
 * DRM_BUS_FLAG_PIXDATA_DRIVE_* flags to make code simpler, as signals are
 * usually to be sampled on the opposite edge of the driving edge.
 */
#define DRM_BUS_FLAG_PIXDATA_POSEDGE    (1<<2)
#define DRM_BUS_FLAG_PIXDATA_NEGEDGE    (1<<3)

/* Drive data on rising edge */
#define DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE    DRM_BUS_FLAG_PIXDATA_POSEDGE
/* Drive data on falling edge */
#define DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE    DRM_BUS_FLAG_PIXDATA_NEGEDGE
/* Sample data on rising edge */
#define DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE   DRM_BUS_FLAG_PIXDATA_NEGEDGE
/* Sample data on falling edge */
#define DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE   DRM_BUS_FLAG_PIXDATA_POSEDGE

I'll submit a patch series.

> >> Signed-off-by: Stefan Agner <stefan@agner.ch>
> >> ---
> >> 
> >>  drivers/gpu/drm/bridge/dumb-vga-dac.c |  6 +++---
> >>  include/drm/drm_bridge.h              | 11 +++++------
> >>  2 files changed, 8 insertions(+), 9 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> >> b/drivers/gpu/drm/bridge/dumb-vga-dac.c index 9b706789a341..d5aa0f931ef2
> >> 100644
> >> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> >> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> >> @@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device
> >> *pdev)
> >>  */
> >>  static const struct drm_bridge_timings default_dac_timings = {
> >>  	/* Timing specifications, datasheet page 7 */
> >> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> >> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> >>  	.setup_time_ps = 500,
> >>  	.hold_time_ps = 1500,
> >>  };
> >> @@ -245,7 +245,7 @@ static const struct drm_bridge_timings
> >> default_dac_timings = {
> >>  */
> >>  static const struct drm_bridge_timings ti_ths8134_dac_timings = {
> >>  	/* From timing diagram, datasheet page 9 */
> >> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> >> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> >>  	/* From datasheet, page 12 */
> >>  	.setup_time_ps = 3000,
> >>  	/* I guess this means latched input */
> >> @@ -258,7 +258,7 @@ static const struct drm_bridge_timings
> >> ti_ths8134_dac_timings = {
> >>  */
> >>  static const struct drm_bridge_timings ti_ths8135_dac_timings = {
> >>  	/* From timing diagram, datasheet page 14 */
> >> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> >> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> >>  	/* From datasheet, page 16 */
> >>  	.setup_time_ps = 2000,
> >>  	.hold_time_ps = 500,
> >> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> >> index bd850747ce54..45e90f4b46c3 100644
> >> --- a/include/drm/drm_bridge.h
> >> +++ b/include/drm/drm_bridge.h
> >> @@ -244,14 +244,13 @@ struct drm_bridge_funcs {
> >>   */
> >>  struct drm_bridge_timings {
> >>  	/**
> >> -	 * @sampling_edge:
> >> +	 * @input_bus_flags:
> >>  	 *
> >> -	 * Tells whether the bridge samples the digital input signal
> >> -	 * from the display engine on the positive or negative edge of the
> >> -	 * clock, this should reuse the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE
> >> -	 * bitwise flags from the DRM connector (bit 2 and 3 valid).
> >> +	 * Additional settings this bridge requires for the pixel data on
> >> +	 * the input bus (e.g. pixel signal polarity). See also
> >> +	 * &drm_display_info->bus_flags.
> >>  	 */
> >> -	u32 sampling_edge;
> >> +	u32 input_bus_flags;
> >> 
> >>  	/**
> >>  	 * @setup_time_ps:
> >>  	 *

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/8] drm/bridge: use bus flags in bridge timings

[Laurent Pinchart]

Hi Stefan,

On Tuesday, 18 September 2018 21:14:22 EEST Stefan Agner wrote:
> On 14.09.2018 02:23, Laurent Pinchart wrote:
> > On Wednesday, 12 September 2018 21:32:15 EEST Stefan Agner wrote:
> >> The DRM bus flags conveys additional information on pixel data on
> >> the bus. All currently available bus flags might be of interest for
> >> a bridge. In the case at hand a dumb VGA bridge needs a specific
> >> data enable polarity (DRM_BUS_FLAG_DE_LOW).
> >> 
> >> Replace the sampling_edge field with input_bus_flags and allow all
> >> currently documented bus flags.
> >> 
> >> This changes the perspective from sampling side to the driving
> >> side for the currently supported flags. We assume that the sampling
> >> edge is always the opposite of the driving edge (hence we need to
> >> invert the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE flags). This is an
> >> assumption we already make for displays. For all we know it is a
> >> safe assumption for bridges too.
> > 
> > Please don't, that will get confusing very quickly. Flag names such as
> > DRM_BUS_FLAG_PIXDATA_NEGEDGE don't mention sampling or driving. There's
> > only a small comment next to their definition, which will easily be
> > overlooked. I'd rather update the definition to cover both sampling and
> > driving, and document the input_bus_flags field to explain that all flags
> > refer to sampling.
> 
> There is history to that, and I'd really rather prefer to keep that similar
> across the kernel. There is already precedence in the kernel, the display
> timings (which is a consumer) defines it from the driving perspective too
> (see DISPLAY_FLAGS_PIXDATA_POSEDGE).
> 
> That is why I introduced the bus flags using the same perspective.
> 
> If we _really_ want it from sampling side, we should create new defines
> which are explicit about that, e.g.: DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE.

I'm not opposed to that.

> But again, since even the display flags use the driving perspective, I'd
> rather prefer to have it that way also for bridges too.

But look at the following diff:

-	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,

The first line makes it very explicit that the data signals are sampled on the 
rising edge. The second line reads to me as sampling on the negative edge, as 
it reports input information from a bridge perspective. I'll have to read the 
documentation to get it right, and I'm sure I and other will overlook it from 
time to time.

The following would be much more explicit:

	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLING_POSEDGE,

and we could define macros as follows:

/*
 * Don't use those two flags directly, use the DRM_BUS_FLAG_PIXDATA_DRIVE_*
 * and DRM_BUS_FLAG_PIXDATA_SAMPLE_* variants to qualify the flags explicitly.
 * The DRM_BUS_FLAG_PIXDATA_SAMPLE_* flags are defined as the opposite of the
 * DRM_BUS_FLAG_PIXDATA_DRIVE_* flags to make code simpler, as signals are
 * usually to be sampled on the opposite edge of the driving edge.
 */
#define DRM_BUS_FLAG_PIXDATA_POSEDGE    (1<<2)
#define DRM_BUS_FLAG_PIXDATA_NEGEDGE    (1<<3)

/* Drive data on rising edge */
#define DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE    DRM_BUS_FLAG_PIXDATA_POSEDGE
/* Drive data on falling edge */
#define DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE    DRM_BUS_FLAG_PIXDATA_NEGEDGE
/* Sample data on rising edge */
#define DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE   DRM_BUS_FLAG_PIXDATA_NEGEDGE
/* Sample data on falling edge */
#define DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE   DRM_BUS_FLAG_PIXDATA_POSEDGE

I'll submit a patch series.

> >> Signed-off-by: Stefan Agner <stefan@agner.ch>
> >> ---
> >> 
> >>  drivers/gpu/drm/bridge/dumb-vga-dac.c |  6 +++---
> >>  include/drm/drm_bridge.h              | 11 +++++------
> >>  2 files changed, 8 insertions(+), 9 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> >> b/drivers/gpu/drm/bridge/dumb-vga-dac.c index 9b706789a341..d5aa0f931ef2
> >> 100644
> >> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> >> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> >> @@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device
> >> *pdev)
> >>  */
> >>  static const struct drm_bridge_timings default_dac_timings = {
> >>  	/* Timing specifications, datasheet page 7 */
> >> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> >> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> >>  	.setup_time_ps = 500,
> >>  	.hold_time_ps = 1500,
> >>  };
> >> @@ -245,7 +245,7 @@ static const struct drm_bridge_timings
> >> default_dac_timings = {
> >>  */
> >>  static const struct drm_bridge_timings ti_ths8134_dac_timings = {
> >>  	/* From timing diagram, datasheet page 9 */
> >> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> >> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> >>  	/* From datasheet, page 12 */
> >>  	.setup_time_ps = 3000,
> >>  	/* I guess this means latched input */
> >> @@ -258,7 +258,7 @@ static const struct drm_bridge_timings
> >> ti_ths8134_dac_timings = {
> >>  */
> >>  static const struct drm_bridge_timings ti_ths8135_dac_timings = {
> >>  	/* From timing diagram, datasheet page 14 */
> >> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> >> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> >>  	/* From datasheet, page 16 */
> >>  	.setup_time_ps = 2000,
> >>  	.hold_time_ps = 500,
> >> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> >> index bd850747ce54..45e90f4b46c3 100644
> >> --- a/include/drm/drm_bridge.h
> >> +++ b/include/drm/drm_bridge.h
> >> @@ -244,14 +244,13 @@ struct drm_bridge_funcs {
> >>   */
> >>  struct drm_bridge_timings {
> >>  	/**
> >> -	 * @sampling_edge:
> >> +	 * @input_bus_flags:
> >>  	 *
> >> -	 * Tells whether the bridge samples the digital input signal
> >> -	 * from the display engine on the positive or negative edge of the
> >> -	 * clock, this should reuse the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE
> >> -	 * bitwise flags from the DRM connector (bit 2 and 3 valid).
> >> +	 * Additional settings this bridge requires for the pixel data on
> >> +	 * the input bus (e.g. pixel signal polarity). See also
> >> +	 * &drm_display_info->bus_flags.
> >>  	 */
> >> -	u32 sampling_edge;
> >> +	u32 input_bus_flags;
> >> 
> >>  	/**
> >>  	 * @setup_time_ps:
> >>  	 *

-- 
Regards,

Laurent Pinchart



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/8] drm/bridge: use bus flags in bridge timings

[Laurent Pinchart]

Hi Stefan,

On Tuesday, 18 September 2018 21:14:22 EEST Stefan Agner wrote:
> On 14.09.2018 02:23, Laurent Pinchart wrote:
> > On Wednesday, 12 September 2018 21:32:15 EEST Stefan Agner wrote:
> >> The DRM bus flags conveys additional information on pixel data on
> >> the bus. All currently available bus flags might be of interest for
> >> a bridge. In the case at hand a dumb VGA bridge needs a specific
> >> data enable polarity (DRM_BUS_FLAG_DE_LOW).
> >> 
> >> Replace the sampling_edge field with input_bus_flags and allow all
> >> currently documented bus flags.
> >> 
> >> This changes the perspective from sampling side to the driving
> >> side for the currently supported flags. We assume that the sampling
> >> edge is always the opposite of the driving edge (hence we need to
> >> invert the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE flags). This is an
> >> assumption we already make for displays. For all we know it is a
> >> safe assumption for bridges too.
> > 
> > Please don't, that will get confusing very quickly. Flag names such as
> > DRM_BUS_FLAG_PIXDATA_NEGEDGE don't mention sampling or driving. There's
> > only a small comment next to their definition, which will easily be
> > overlooked. I'd rather update the definition to cover both sampling and
> > driving, and document the input_bus_flags field to explain that all flags
> > refer to sampling.
> 
> There is history to that, and I'd really rather prefer to keep that similar
> across the kernel. There is already precedence in the kernel, the display
> timings (which is a consumer) defines it from the driving perspective too
> (see DISPLAY_FLAGS_PIXDATA_POSEDGE).
> 
> That is why I introduced the bus flags using the same perspective.
> 
> If we _really_ want it from sampling side, we should create new defines
> which are explicit about that, e.g.: DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE.

I'm not opposed to that.

> But again, since even the display flags use the driving perspective, I'd
> rather prefer to have it that way also for bridges too.

But look at the following diff:

-	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,

The first line makes it very explicit that the data signals are sampled on the 
rising edge. The second line reads to me as sampling on the negative edge, as 
it reports input information from a bridge perspective. I'll have to read the 
documentation to get it right, and I'm sure I and other will overlook it from 
time to time.

The following would be much more explicit:

	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLING_POSEDGE,

and we could define macros as follows:

/*
 * Don't use those two flags directly, use the DRM_BUS_FLAG_PIXDATA_DRIVE_*
 * and DRM_BUS_FLAG_PIXDATA_SAMPLE_* variants to qualify the flags explicitly.
 * The DRM_BUS_FLAG_PIXDATA_SAMPLE_* flags are defined as the opposite of the
 * DRM_BUS_FLAG_PIXDATA_DRIVE_* flags to make code simpler, as signals are
 * usually to be sampled on the opposite edge of the driving edge.
 */
#define DRM_BUS_FLAG_PIXDATA_POSEDGE    (1<<2)
#define DRM_BUS_FLAG_PIXDATA_NEGEDGE    (1<<3)

/* Drive data on rising edge */
#define DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE    DRM_BUS_FLAG_PIXDATA_POSEDGE
/* Drive data on falling edge */
#define DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE    DRM_BUS_FLAG_PIXDATA_NEGEDGE
/* Sample data on rising edge */
#define DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE   DRM_BUS_FLAG_PIXDATA_NEGEDGE
/* Sample data on falling edge */
#define DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE   DRM_BUS_FLAG_PIXDATA_POSEDGE

I'll submit a patch series.

> >> Signed-off-by: Stefan Agner <stefan@agner.ch>
> >> ---
> >> 
> >>  drivers/gpu/drm/bridge/dumb-vga-dac.c |  6 +++---
> >>  include/drm/drm_bridge.h              | 11 +++++------
> >>  2 files changed, 8 insertions(+), 9 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> >> b/drivers/gpu/drm/bridge/dumb-vga-dac.c index 9b706789a341..d5aa0f931ef2
> >> 100644
> >> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> >> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> >> @@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device
> >> *pdev)
> >>  */
> >>  static const struct drm_bridge_timings default_dac_timings = {
> >>  	/* Timing specifications, datasheet page 7 */
> >> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> >> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> >>  	.setup_time_ps = 500,
> >>  	.hold_time_ps = 1500,
> >>  };
> >> @@ -245,7 +245,7 @@ static const struct drm_bridge_timings
> >> default_dac_timings = {
> >>  */
> >>  static const struct drm_bridge_timings ti_ths8134_dac_timings = {
> >>  	/* From timing diagram, datasheet page 9 */
> >> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> >> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> >>  	/* From datasheet, page 12 */
> >>  	.setup_time_ps = 3000,
> >>  	/* I guess this means latched input */
> >> @@ -258,7 +258,7 @@ static const struct drm_bridge_timings
> >> ti_ths8134_dac_timings = {
> >>  */
> >>  static const struct drm_bridge_timings ti_ths8135_dac_timings = {
> >>  	/* From timing diagram, datasheet page 14 */
> >> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> >> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> >>  	/* From datasheet, page 16 */
> >>  	.setup_time_ps = 2000,
> >>  	.hold_time_ps = 500,
> >> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> >> index bd850747ce54..45e90f4b46c3 100644
> >> --- a/include/drm/drm_bridge.h
> >> +++ b/include/drm/drm_bridge.h
> >> @@ -244,14 +244,13 @@ struct drm_bridge_funcs {
> >>   */
> >>  struct drm_bridge_timings {
> >>  	/**
> >> -	 * @sampling_edge:
> >> +	 * @input_bus_flags:
> >>  	 *
> >> -	 * Tells whether the bridge samples the digital input signal
> >> -	 * from the display engine on the positive or negative edge of the
> >> -	 * clock, this should reuse the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE
> >> -	 * bitwise flags from the DRM connector (bit 2 and 3 valid).
> >> +	 * Additional settings this bridge requires for the pixel data on
> >> +	 * the input bus (e.g. pixel signal polarity). See also
> >> +	 * &drm_display_info->bus_flags.
> >>  	 */
> >> -	u32 sampling_edge;
> >> +	u32 input_bus_flags;
> >> 
> >>  	/**
> >>  	 * @setup_time_ps:
> >>  	 *

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 5/8] dt-bindings: display: add data-enable polarity property

[Rob Herring]

On Wed, 12 Sep 2018 11:32:19 -0700, Stefan Agner wrote:
> Allow to specify the data-enable polarity required by a dumb VGA
> DAC converting parallel RGB to VGA.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  .../devicetree/bindings/display/bridge/dumb-vga-dac.txt         | 2 ++
>  1 file changed, 2 insertions(+)
> 

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

Re: [PATCH v2 5/8] dt-bindings: display: add data-enable polarity property

[Rob Herring]

On Wed, 12 Sep 2018 11:32:19 -0700, Stefan Agner wrote:
> Allow to specify the data-enable polarity required by a dumb VGA
> DAC converting parallel RGB to VGA.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  .../devicetree/bindings/display/bridge/dumb-vga-dac.txt         | 2 ++
>  1 file changed, 2 insertions(+)
> 

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

[PATCH v2 5/8] dt-bindings: display: add data-enable polarity property

[Rob Herring]

On Wed, 12 Sep 2018 11:32:19 -0700, Stefan Agner wrote:
> Allow to specify the data-enable polarity required by a dumb VGA
> DAC converting parallel RGB to VGA.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  .../devicetree/bindings/display/bridge/dumb-vga-dac.txt         | 2 ++
>  1 file changed, 2 insertions(+)
> 

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