[PATCH 1/4 v7] drm/bridge: Add bindings for TI THS8134

[PATCH 1/4 v7] drm/bridge: Add bindings for TI THS8134

[Linus Walleij]

This adds device tree bindings for the Texas Instruments
THS8134, THS8134A and THS8134B VGA DACs by extending and
renaming the existing bindings for THS8135.

These DACs are used for the VGA outputs on the ARM reference
designs such as Integrator, Versatile and RealView.

Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
ChangeLog v6->v7:
- No changes, just resending with the series.
ChangeLog v5->v6:
- Fix the more-to-less specific compatible strings.
- Fix some speling.
- Collect Laurent's review tag.
ChangeLog v2->v5:
- Dropped the "ti,ths813x" as it turns out we need precise info
  about the sub-variant anyways as they all very in timings.
- Refine the THS8134 variants, it turns out ths8134, ths8134a
  and ths8134b are three different variants of ths8134.
ChangeLog v1->v2:
- Introduce specific-to-general compatible string:
  compatible = "ti,ths8134a", "ti,ths813x";
  so drivers can handle the whole family the same way.
- Collected Rob's ACK.
---
 .../display/bridge/{ti,ths8135.txt => ti,ths813x.txt}       | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)
 rename Documentation/devicetree/bindings/display/bridge/{ti,ths8135.txt => ti,ths813x.txt} (69%)

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt b/Documentation/devicetree/bindings/display/bridge/ti,ths813x.txt
similarity index 69%
rename from Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
rename to Documentation/devicetree/bindings/display/bridge/ti,ths813x.txt
index 6ec1a880ac18..df3d7c1ac09e 100644
--- a/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
+++ b/Documentation/devicetree/bindings/display/bridge/ti,ths813x.txt
@@ -1,11 +1,16 @@
-THS8135 Video DAC
------------------
+THS8134 and THS8135 Video DAC
+-----------------------------
 
-This is the binding for Texas Instruments THS8135 Video DAC bridge.
+This is the binding for Texas Instruments THS8134, THS8134A, THS8134B and
+THS8135 Video DAC bridges.
 
 Required properties:
 
-- compatible: Must be "ti,ths8135"
+- compatible: Must be one of
+  "ti,ths8134"
+  "ti,ths8134a," "ti,ths8134"
+  "ti,ths8134b", "ti,ths8134"
+  "ti,ths8135"
 
 Required nodes:
 
-- 
2.14.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4 v7] drm/bridge: Add bindings for TI THS8134

[Linus Walleij]

This adds device tree bindings for the Texas Instruments
THS8134, THS8134A and THS8134B VGA DACs by extending and
renaming the existing bindings for THS8135.

These DACs are used for the VGA outputs on the ARM reference
designs such as Integrator, Versatile and RealView.

Cc: devicetree at vger.kernel.org
Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v6->v7:
- No changes, just resending with the series.
ChangeLog v5->v6:
- Fix the more-to-less specific compatible strings.
- Fix some speling.
- Collect Laurent's review tag.
ChangeLog v2->v5:
- Dropped the "ti,ths813x" as it turns out we need precise info
  about the sub-variant anyways as they all very in timings.
- Refine the THS8134 variants, it turns out ths8134, ths8134a
  and ths8134b are three different variants of ths8134.
ChangeLog v1->v2:
- Introduce specific-to-general compatible string:
  compatible = "ti,ths8134a", "ti,ths813x";
  so drivers can handle the whole family the same way.
- Collected Rob's ACK.
---
 .../display/bridge/{ti,ths8135.txt => ti,ths813x.txt}       | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)
 rename Documentation/devicetree/bindings/display/bridge/{ti,ths8135.txt => ti,ths813x.txt} (69%)

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt b/Documentation/devicetree/bindings/display/bridge/ti,ths813x.txt
similarity index 69%
rename from Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
rename to Documentation/devicetree/bindings/display/bridge/ti,ths813x.txt
index 6ec1a880ac18..df3d7c1ac09e 100644
--- a/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
+++ b/Documentation/devicetree/bindings/display/bridge/ti,ths813x.txt
@@ -1,11 +1,16 @@
-THS8135 Video DAC
------------------
+THS8134 and THS8135 Video DAC
+-----------------------------
 
-This is the binding for Texas Instruments THS8135 Video DAC bridge.
+This is the binding for Texas Instruments THS8134, THS8134A, THS8134B and
+THS8135 Video DAC bridges.
 
 Required properties:
 
-- compatible: Must be "ti,ths8135"
+- compatible: Must be one of
+  "ti,ths8134"
+  "ti,ths8134a," "ti,ths8134"
+  "ti,ths8134b", "ti,ths8134"
+  "ti,ths8135"
 
 Required nodes:
 
-- 
2.14.3

[PATCH 2/4 v7] drm/bridge: Provide a way to embed timing info in bridges

[Linus Walleij]

After some discussion and failed patch sets trying to convey
the right timing information between the display engine and
a bridge using the connector, I try instead to use an optional
timing information container in the bridge itself, so that
display engines can retrieve it from any bridge and use it to
determine how to drive outputs.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v6->v7:
- Fix the comment style to use the new inline type of kerneldoc
  for struct members.
- Need an explicit ACK/review by someone on this patch to continue
  with the series...
ChangeLog v5->v6:
- Sort forward struct declarations alphabetically
- Switch to using DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE to indicate
  positive or negatice clock samling edge
ChangeLog ->v5:
- New patch
---
 include/drm/drm_bridge.h | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 682d01ba920c..d3c2eea0bb63 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -29,6 +29,7 @@
 #include <drm/drm_modes.h>
 
 struct drm_bridge;
+struct drm_bridge_timings;
 struct drm_panel;
 
 /**
@@ -222,6 +223,35 @@ struct drm_bridge_funcs {
 	void (*enable)(struct drm_bridge *bridge);
 };
 
+/**
+ * struct drm_bridge_timings - timing information for the bridge
+ */
+struct drm_bridge_timings {
+	/**
+	 * @sampling_edge:
+	 *
+	 * 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).
+	 */
+	u32 sampling_edge;
+	/**
+	 * @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;
+};
+
 /**
  * struct drm_bridge - central DRM bridge control structure
  * @dev: DRM device this bridge belongs to
@@ -229,6 +259,8 @@ struct drm_bridge_funcs {
  * @next: the next bridge in the encoder chain
  * @of_node: device node pointer to the bridge
  * @list: to keep track of all added bridges
+ * @timings: the timing specification for the bridge, if any (may
+ * be NULL)
  * @funcs: control functions
  * @driver_private: pointer to the bridge driver's internal context
  */
@@ -240,6 +272,7 @@ struct drm_bridge {
 	struct device_node *of_node;
 #endif
 	struct list_head list;
+	const struct drm_bridge_timings *timings;
 
 	const struct drm_bridge_funcs *funcs;
 	void *driver_private;
-- 
2.14.3

[PATCH 2/4 v7] drm/bridge: Provide a way to embed timing info in bridges

[Linus Walleij]

After some discussion and failed patch sets trying to convey
the right timing information between the display engine and
a bridge using the connector, I try instead to use an optional
timing information container in the bridge itself, so that
display engines can retrieve it from any bridge and use it to
determine how to drive outputs.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v6->v7:
- Fix the comment style to use the new inline type of kerneldoc
  for struct members.
- Need an explicit ACK/review by someone on this patch to continue
  with the series...
ChangeLog v5->v6:
- Sort forward struct declarations alphabetically
- Switch to using DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE to indicate
  positive or negatice clock samling edge
ChangeLog ->v5:
- New patch
---
 include/drm/drm_bridge.h | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 682d01ba920c..d3c2eea0bb63 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -29,6 +29,7 @@
 #include <drm/drm_modes.h>
 
 struct drm_bridge;
+struct drm_bridge_timings;
 struct drm_panel;
 
 /**
@@ -222,6 +223,35 @@ struct drm_bridge_funcs {
 	void (*enable)(struct drm_bridge *bridge);
 };
 
+/**
+ * struct drm_bridge_timings - timing information for the bridge
+ */
+struct drm_bridge_timings {
+	/**
+	 * @sampling_edge:
+	 *
+	 * 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).
+	 */
+	u32 sampling_edge;
+	/**
+	 * @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;
+};
+
 /**
  * struct drm_bridge - central DRM bridge control structure
  * @dev: DRM device this bridge belongs to
@@ -229,6 +259,8 @@ struct drm_bridge_funcs {
  * @next: the next bridge in the encoder chain
  * @of_node: device node pointer to the bridge
  * @list: to keep track of all added bridges
+ * @timings: the timing specification for the bridge, if any (may
+ * be NULL)
  * @funcs: control functions
  * @driver_private: pointer to the bridge driver's internal context
  */
@@ -240,6 +272,7 @@ struct drm_bridge {
 	struct device_node *of_node;
 #endif
 	struct list_head list;
+	const struct drm_bridge_timings *timings;
 
 	const struct drm_bridge_funcs *funcs;
 	void *driver_private;
-- 
2.14.3

[PATCH 3/4 v7] drm/bridge: Add timing support to dumb VGA DAC

[Linus Walleij]

This extends the dumb VGA DAC bridge to handle the THS8134A
and THS8134B VGA DACs in addition to those already handled.

We assign the proper timing data to the pointer inside the
bridge struct so display controllers that need to align their
timings to the bridge can pick it up and work from there.

Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v5->v6:
- Use DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE to indicate
  the sampling edge of the clock signal.
- Skip intermediate variable for timings.
- Leave timings as NULL for really dumb VGA DACs.
- Collect Laurent's Review tag.
ChangeLog v4->v5:
- Rewrite the support using the new concept of defining
  fine-granular sampling (setup+hold) timing definitions
  stored in the bridge timings struct.
ChangeLog v3->v4:
- Actually have the code syntactically correct and compiling :(
  (Kconfig mistake.)
  (...)
  AS      usr/initramfs_data.o
  AR      usr/built-in.o
  CC      drivers/gpu/drm/bridge/dumb-vga-dac.o
  AR      drivers/gpu/drm/bridge/built-in.o
  AR      drivers/gpu/drm/built-in.o
  AR      drivers/gpu/built-in.o
  AR      drivers/built-in.o
  (...)
ChangeLog v2->v3:
- Move const specifier.
- Cut one line of code assigning bus flags.
- Preserve the "ti,ths8135" compatible for elder device trees.
ChangeLog v1->v2:
- Alphabetize includes
- Use a u32 with the bus polarity flags and just encode the
  polarity using the DRM define directly.
- Rename vendor_data to vendor_info.
- Simplify assignment of the flag as it is just a simple
  u32 now.
- Probe all TI variants on the "ti,ths813x" wildcard for now,
  we only need to know that the device is in this family to
  set the clock edge flag right.
---
 drivers/gpu/drm/bridge/dumb-vga-dac.c | 59 +++++++++++++++++++++++++++++++++--
 1 file changed, 56 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index de5e7dee7ad6..498d5948d1a8 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -11,6 +11,7 @@
  */
 
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/of_graph.h>
 #include <linux/regulator/consumer.h>
 
@@ -204,6 +205,7 @@ 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);
 
@@ -222,10 +224,61 @@ static int dumb_vga_remove(struct platform_device *pdev)
 	return 0;
 }
 
+/*
+ * 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 */
+	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.setup_time_ps = 500,
+	.hold_time_ps = 1500,
+};
+
+/*
+ * Information taken from the THS8134, THS8134A, THS8134B datasheet named
+ * "SLVS205D", dated May 1990, revised March 2000.
+ */
+static const struct drm_bridge_timings ti_ths8134_dac_timings = {
+	/* From timing diagram, datasheet page 9 */
+	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	/* From datasheet, page 12 */
+	.setup_time_ps = 3000,
+	/* I guess this means latched input */
+	.hold_time_ps = 0,
+};
+
+/*
+ * Information taken from the THS8135 datasheet named "SLAS343B", dated
+ * May 2001, revised April 2013.
+ */
+static const struct drm_bridge_timings ti_ths8135_dac_timings = {
+	/* From timing diagram, datasheet page 14 */
+	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	/* From datasheet, page 16 */
+	.setup_time_ps = 2000,
+	.hold_time_ps = 500,
+};
+
 static const struct of_device_id dumb_vga_match[] = {
-	{ .compatible = "dumb-vga-dac" },
-	{ .compatible = "adi,adv7123" },
-	{ .compatible = "ti,ths8135" },
+	{
+		.compatible = "dumb-vga-dac",
+		.data = NULL,
+	},
+	{
+		.compatible = "adi,adv7123",
+		.data = &default_dac_timings,
+	},
+	{
+		.compatible = "ti,ths8135",
+		.data = &ti_ths8135_dac_timings,
+	},
+	{
+		.compatible = "ti,ths8134",
+		.data = &ti_ths8134_dac_timings,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, dumb_vga_match);
-- 
2.14.3

[PATCH 3/4 v7] drm/bridge: Add timing support to dumb VGA DAC

[Linus Walleij]

This extends the dumb VGA DAC bridge to handle the THS8134A
and THS8134B VGA DACs in addition to those already handled.

We assign the proper timing data to the pointer inside the
bridge struct so display controllers that need to align their
timings to the bridge can pick it up and work from there.

Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v5->v6:
- Use DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE to indicate
  the sampling edge of the clock signal.
- Skip intermediate variable for timings.
- Leave timings as NULL for really dumb VGA DACs.
- Collect Laurent's Review tag.
ChangeLog v4->v5:
- Rewrite the support using the new concept of defining
  fine-granular sampling (setup+hold) timing definitions
  stored in the bridge timings struct.
ChangeLog v3->v4:
- Actually have the code syntactically correct and compiling :(
  (Kconfig mistake.)
  (...)
  AS      usr/initramfs_data.o
  AR      usr/built-in.o
  CC      drivers/gpu/drm/bridge/dumb-vga-dac.o
  AR      drivers/gpu/drm/bridge/built-in.o
  AR      drivers/gpu/drm/built-in.o
  AR      drivers/gpu/built-in.o
  AR      drivers/built-in.o
  (...)
ChangeLog v2->v3:
- Move const specifier.
- Cut one line of code assigning bus flags.
- Preserve the "ti,ths8135" compatible for elder device trees.
ChangeLog v1->v2:
- Alphabetize includes
- Use a u32 with the bus polarity flags and just encode the
  polarity using the DRM define directly.
- Rename vendor_data to vendor_info.
- Simplify assignment of the flag as it is just a simple
  u32 now.
- Probe all TI variants on the "ti,ths813x" wildcard for now,
  we only need to know that the device is in this family to
  set the clock edge flag right.
---
 drivers/gpu/drm/bridge/dumb-vga-dac.c | 59 +++++++++++++++++++++++++++++++++--
 1 file changed, 56 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index de5e7dee7ad6..498d5948d1a8 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -11,6 +11,7 @@
  */
 
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/of_graph.h>
 #include <linux/regulator/consumer.h>
 
@@ -204,6 +205,7 @@ 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);
 
@@ -222,10 +224,61 @@ static int dumb_vga_remove(struct platform_device *pdev)
 	return 0;
 }
 
+/*
+ * 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 */
+	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.setup_time_ps = 500,
+	.hold_time_ps = 1500,
+};
+
+/*
+ * Information taken from the THS8134, THS8134A, THS8134B datasheet named
+ * "SLVS205D", dated May 1990, revised March 2000.
+ */
+static const struct drm_bridge_timings ti_ths8134_dac_timings = {
+	/* From timing diagram, datasheet page 9 */
+	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	/* From datasheet, page 12 */
+	.setup_time_ps = 3000,
+	/* I guess this means latched input */
+	.hold_time_ps = 0,
+};
+
+/*
+ * Information taken from the THS8135 datasheet named "SLAS343B", dated
+ * May 2001, revised April 2013.
+ */
+static const struct drm_bridge_timings ti_ths8135_dac_timings = {
+	/* From timing diagram, datasheet page 14 */
+	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	/* From datasheet, page 16 */
+	.setup_time_ps = 2000,
+	.hold_time_ps = 500,
+};
+
 static const struct of_device_id dumb_vga_match[] = {
-	{ .compatible = "dumb-vga-dac" },
-	{ .compatible = "adi,adv7123" },
-	{ .compatible = "ti,ths8135" },
+	{
+		.compatible = "dumb-vga-dac",
+		.data = NULL,
+	},
+	{
+		.compatible = "adi,adv7123",
+		.data = &default_dac_timings,
+	},
+	{
+		.compatible = "ti,ths8135",
+		.data = &ti_ths8135_dac_timings,
+	},
+	{
+		.compatible = "ti,ths8134",
+		.data = &ti_ths8134_dac_timings,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, dumb_vga_match);
-- 
2.14.3

[PATCH 4/4 v7] drm/pl111: Support handling bridge timings

[Linus Walleij]

If the bridge has a too strict setup time for the incoming
signals, we may not be fast enough and then we need to
compensate by outputting the signal on the inverse clock
edge so it is for sure stable when the bridge samples it.

Since bridges in difference to panels does not expose their
connectors, make the connector optional in the display
setup code.

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v6->v7:
- Collect Eric's ACK.
ChangeLog v5->v6:
- Collect Laurent's ACK.
ChangeLog v4->v5:
- Use the new bridge timings setup method.
---
 drivers/gpu/drm/pl111/Kconfig         |  1 +
 drivers/gpu/drm/pl111/pl111_display.c | 35 +++++++++++++++++++++++++++++++----
 drivers/gpu/drm/pl111/pl111_drv.c     | 20 +++++++++++---------
 3 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig
index e5e2abd66491..82cb3e60ddc8 100644
--- a/drivers/gpu/drm/pl111/Kconfig
+++ b/drivers/gpu/drm/pl111/Kconfig
@@ -8,6 +8,7 @@ config DRM_PL111
 	select DRM_GEM_CMA_HELPER
 	select DRM_BRIDGE
 	select DRM_PANEL_BRIDGE
+	select DRM_DUMB_VGA_DAC
 	select VT_HW_CONSOLE_BINDING if FRAMEBUFFER_CONSOLE
 	help
 	  Choose this option for DRM support for the PL111 CLCD controller.
diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 06c4bf756b69..7fe4040aea46 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -94,6 +94,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 	const struct drm_display_mode *mode = &cstate->mode;
 	struct drm_framebuffer *fb = plane->state->fb;
 	struct drm_connector *connector = priv->connector;
+	struct drm_bridge *bridge = priv->bridge;
 	u32 cntl;
 	u32 ppl, hsw, hfp, hbp;
 	u32 lpp, vsw, vfp, vbp;
@@ -143,11 +144,37 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 	if (mode->flags & DRM_MODE_FLAG_NVSYNC)
 		tim2 |= TIM2_IVS;
 
-	if (connector->display_info.bus_flags & DRM_BUS_FLAG_DE_LOW)
-		tim2 |= TIM2_IOE;
+	if (connector) {
+		if (connector->display_info.bus_flags & DRM_BUS_FLAG_DE_LOW)
+			tim2 |= TIM2_IOE;
 
-	if (connector->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
-		tim2 |= TIM2_IPC;
+		if (connector->display_info.bus_flags &
+		    DRM_BUS_FLAG_PIXDATA_NEGEDGE)
+			tim2 |= TIM2_IPC;
+	}
+
+	if (bridge) {
+		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.
+		 */
+		if (btimings && btimings->setup_time_ps >= 3000)
+			tim2 ^= TIM2_IPC;
+	}
 
 	tim2 |= cpl << 16;
 	writel(tim2, priv->regs + CLCD_TIM2);
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index 201d57d5cb54..101a9c7db6ff 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -107,11 +107,17 @@ static int pl111_modeset_init(struct drm_device *dev)
 			ret = PTR_ERR(bridge);
 			goto out_config;
 		}
-		/*
-		 * TODO: when we are using a different bridge than a panel
-		 * (such as a dumb VGA connector) we need to devise a different
-		 * method to get the connector out of the bridge.
-		 */
+	} else if (bridge) {
+		dev_info(dev->dev, "Using non-panel bridge\n");
+	} else {
+		dev_err(dev->dev, "No bridge, exiting\n");
+		return -ENODEV;
+	}
+
+	priv->bridge = bridge;
+	if (panel) {
+		priv->panel = panel;
+		priv->connector = panel->connector;
 	}
 
 	ret = pl111_display_init(dev);
@@ -125,10 +131,6 @@ static int pl111_modeset_init(struct drm_device *dev)
 	if (ret)
 		return ret;
 
-	priv->bridge = bridge;
-	priv->panel = panel;
-	priv->connector = panel->connector;
-
 	ret = drm_vblank_init(dev, 1);
 	if (ret != 0) {
 		dev_err(dev->dev, "Failed to init vblank\n");
-- 
2.14.3

[PATCH 4/4 v7] drm/pl111: Support handling bridge timings

[Linus Walleij]

If the bridge has a too strict setup time for the incoming
signals, we may not be fast enough and then we need to
compensate by outputting the signal on the inverse clock
edge so it is for sure stable when the bridge samples it.

Since bridges in difference to panels does not expose their
connectors, make the connector optional in the display
setup code.

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v6->v7:
- Collect Eric's ACK.
ChangeLog v5->v6:
- Collect Laurent's ACK.
ChangeLog v4->v5:
- Use the new bridge timings setup method.
---
 drivers/gpu/drm/pl111/Kconfig         |  1 +
 drivers/gpu/drm/pl111/pl111_display.c | 35 +++++++++++++++++++++++++++++++----
 drivers/gpu/drm/pl111/pl111_drv.c     | 20 +++++++++++---------
 3 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig
index e5e2abd66491..82cb3e60ddc8 100644
--- a/drivers/gpu/drm/pl111/Kconfig
+++ b/drivers/gpu/drm/pl111/Kconfig
@@ -8,6 +8,7 @@ config DRM_PL111
 	select DRM_GEM_CMA_HELPER
 	select DRM_BRIDGE
 	select DRM_PANEL_BRIDGE
+	select DRM_DUMB_VGA_DAC
 	select VT_HW_CONSOLE_BINDING if FRAMEBUFFER_CONSOLE
 	help
 	  Choose this option for DRM support for the PL111 CLCD controller.
diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 06c4bf756b69..7fe4040aea46 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -94,6 +94,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 	const struct drm_display_mode *mode = &cstate->mode;
 	struct drm_framebuffer *fb = plane->state->fb;
 	struct drm_connector *connector = priv->connector;
+	struct drm_bridge *bridge = priv->bridge;
 	u32 cntl;
 	u32 ppl, hsw, hfp, hbp;
 	u32 lpp, vsw, vfp, vbp;
@@ -143,11 +144,37 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 	if (mode->flags & DRM_MODE_FLAG_NVSYNC)
 		tim2 |= TIM2_IVS;
 
-	if (connector->display_info.bus_flags & DRM_BUS_FLAG_DE_LOW)
-		tim2 |= TIM2_IOE;
+	if (connector) {
+		if (connector->display_info.bus_flags & DRM_BUS_FLAG_DE_LOW)
+			tim2 |= TIM2_IOE;
 
-	if (connector->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
-		tim2 |= TIM2_IPC;
+		if (connector->display_info.bus_flags &
+		    DRM_BUS_FLAG_PIXDATA_NEGEDGE)
+			tim2 |= TIM2_IPC;
+	}
+
+	if (bridge) {
+		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.
+		 */
+		if (btimings && btimings->setup_time_ps >= 3000)
+			tim2 ^= TIM2_IPC;
+	}
 
 	tim2 |= cpl << 16;
 	writel(tim2, priv->regs + CLCD_TIM2);
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index 201d57d5cb54..101a9c7db6ff 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -107,11 +107,17 @@ static int pl111_modeset_init(struct drm_device *dev)
 			ret = PTR_ERR(bridge);
 			goto out_config;
 		}
-		/*
-		 * TODO: when we are using a different bridge than a panel
-		 * (such as a dumb VGA connector) we need to devise a different
-		 * method to get the connector out of the bridge.
-		 */
+	} else if (bridge) {
+		dev_info(dev->dev, "Using non-panel bridge\n");
+	} else {
+		dev_err(dev->dev, "No bridge, exiting\n");
+		return -ENODEV;
+	}
+
+	priv->bridge = bridge;
+	if (panel) {
+		priv->panel = panel;
+		priv->connector = panel->connector;
 	}
 
 	ret = pl111_display_init(dev);
@@ -125,10 +131,6 @@ static int pl111_modeset_init(struct drm_device *dev)
 	if (ret)
 		return ret;
 
-	priv->bridge = bridge;
-	priv->panel = panel;
-	priv->connector = panel->connector;
-
 	ret = drm_vblank_init(dev, 1);
 	if (ret != 0) {
 		dev_err(dev->dev, "Failed to init vblank\n");
-- 
2.14.3

Re: [PATCH 1/4 v7] drm/bridge: Add bindings for TI THS8134

[Archit Taneja]

On 01/12/2018 01:18 PM, Linus Walleij wrote:
> This adds device tree bindings for the Texas Instruments
> THS8134, THS8134A and THS8134B VGA DACs by extending and
> renaming the existing bindings for THS8135.
> 
> These DACs are used for the VGA outputs on the ARM reference
> designs such as Integrator, Versatile and RealView.

queued to drm-misc-next.

Thanks,
Archit

> 
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> ChangeLog v6->v7:
> - No changes, just resending with the series.
> ChangeLog v5->v6:
> - Fix the more-to-less specific compatible strings.
> - Fix some speling.
> - Collect Laurent's review tag.
> ChangeLog v2->v5:
> - Dropped the "ti,ths813x" as it turns out we need precise info
>    about the sub-variant anyways as they all very in timings.
> - Refine the THS8134 variants, it turns out ths8134, ths8134a
>    and ths8134b are three different variants of ths8134.
> ChangeLog v1->v2:
> - Introduce specific-to-general compatible string:
>    compatible = "ti,ths8134a", "ti,ths813x";
>    so drivers can handle the whole family the same way.
> - Collected Rob's ACK.
> ---
>   .../display/bridge/{ti,ths8135.txt => ti,ths813x.txt}       | 13 +++++++++----
>   1 file changed, 9 insertions(+), 4 deletions(-)
>   rename Documentation/devicetree/bindings/display/bridge/{ti,ths8135.txt => ti,ths813x.txt} (69%)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt b/Documentation/devicetree/bindings/display/bridge/ti,ths813x.txt
> similarity index 69%
> rename from Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
> rename to Documentation/devicetree/bindings/display/bridge/ti,ths813x.txt
> index 6ec1a880ac18..df3d7c1ac09e 100644
> --- a/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,ths813x.txt
> @@ -1,11 +1,16 @@
> -THS8135 Video DAC
> ------------------
> +THS8134 and THS8135 Video DAC
> +-----------------------------
>   
> -This is the binding for Texas Instruments THS8135 Video DAC bridge.
> +This is the binding for Texas Instruments THS8134, THS8134A, THS8134B and
> +THS8135 Video DAC bridges.
>   
>   Required properties:
>   
> -- compatible: Must be "ti,ths8135"
> +- compatible: Must be one of
> +  "ti,ths8134"
> +  "ti,ths8134a," "ti,ths8134"
> +  "ti,ths8134b", "ti,ths8134"
> +  "ti,ths8135"
>   
>   Required nodes:
>   
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4 v7] drm/bridge: Add bindings for TI THS8134

[Archit Taneja]

On 01/12/2018 01:18 PM, Linus Walleij wrote:
> This adds device tree bindings for the Texas Instruments
> THS8134, THS8134A and THS8134B VGA DACs by extending and
> renaming the existing bindings for THS8135.
> 
> These DACs are used for the VGA outputs on the ARM reference
> designs such as Integrator, Versatile and RealView.

queued to drm-misc-next.

Thanks,
Archit

> 
> Cc: devicetree at vger.kernel.org
> Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Acked-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v6->v7:
> - No changes, just resending with the series.
> ChangeLog v5->v6:
> - Fix the more-to-less specific compatible strings.
> - Fix some speling.
> - Collect Laurent's review tag.
> ChangeLog v2->v5:
> - Dropped the "ti,ths813x" as it turns out we need precise info
>    about the sub-variant anyways as they all very in timings.
> - Refine the THS8134 variants, it turns out ths8134, ths8134a
>    and ths8134b are three different variants of ths8134.
> ChangeLog v1->v2:
> - Introduce specific-to-general compatible string:
>    compatible = "ti,ths8134a", "ti,ths813x";
>    so drivers can handle the whole family the same way.
> - Collected Rob's ACK.
> ---
>   .../display/bridge/{ti,ths8135.txt => ti,ths813x.txt}       | 13 +++++++++----
>   1 file changed, 9 insertions(+), 4 deletions(-)
>   rename Documentation/devicetree/bindings/display/bridge/{ti,ths8135.txt => ti,ths813x.txt} (69%)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt b/Documentation/devicetree/bindings/display/bridge/ti,ths813x.txt
> similarity index 69%
> rename from Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
> rename to Documentation/devicetree/bindings/display/bridge/ti,ths813x.txt
> index 6ec1a880ac18..df3d7c1ac09e 100644
> --- a/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,ths813x.txt
> @@ -1,11 +1,16 @@
> -THS8135 Video DAC
> ------------------
> +THS8134 and THS8135 Video DAC
> +-----------------------------
>   
> -This is the binding for Texas Instruments THS8135 Video DAC bridge.
> +This is the binding for Texas Instruments THS8134, THS8134A, THS8134B and
> +THS8135 Video DAC bridges.
>   
>   Required properties:
>   
> -- compatible: Must be "ti,ths8135"
> +- compatible: Must be one of
> +  "ti,ths8134"
> +  "ti,ths8134a," "ti,ths8134"
> +  "ti,ths8134b", "ti,ths8134"
> +  "ti,ths8135"
>   
>   Required nodes:
>   
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 2/4 v7] drm/bridge: Provide a way to embed timing info in bridges

[Archit Taneja]

On 01/12/2018 01:18 PM, Linus Walleij wrote:
> After some discussion and failed patch sets trying to convey
> the right timing information between the display engine and
> a bridge using the connector, I try instead to use an optional
> timing information container in the bridge itself, so that
> display engines can retrieve it from any bridge and use it to
> determine how to drive outputs.

queued to drm-misc-next after fixing a minor checkpatch warning
regarding a line exceeding 80 characters.

Thanks,
Archit

> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v6->v7:
> - Fix the comment style to use the new inline type of kerneldoc
>    for struct members.
> - Need an explicit ACK/review by someone on this patch to continue
>    with the series...
> ChangeLog v5->v6:
> - Sort forward struct declarations alphabetically
> - Switch to using DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE to indicate
>    positive or negatice clock samling edge
> ChangeLog ->v5:
> - New patch
> ---
>   include/drm/drm_bridge.h | 33 +++++++++++++++++++++++++++++++++
>   1 file changed, 33 insertions(+)
> 
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 682d01ba920c..d3c2eea0bb63 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -29,6 +29,7 @@
>   #include <drm/drm_modes.h>
>   
>   struct drm_bridge;
> +struct drm_bridge_timings;
>   struct drm_panel;
>   
>   /**
> @@ -222,6 +223,35 @@ struct drm_bridge_funcs {
>   	void (*enable)(struct drm_bridge *bridge);
>   };
>   
> +/**
> + * struct drm_bridge_timings - timing information for the bridge
> + */
> +struct drm_bridge_timings {
> +	/**
> +	 * @sampling_edge:
> +	 *
> +	 * 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).
> +	 */
> +	u32 sampling_edge;
> +	/**
> +	 * @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;
> +};
> +
>   /**
>    * struct drm_bridge - central DRM bridge control structure
>    * @dev: DRM device this bridge belongs to
> @@ -229,6 +259,8 @@ struct drm_bridge_funcs {
>    * @next: the next bridge in the encoder chain
>    * @of_node: device node pointer to the bridge
>    * @list: to keep track of all added bridges
> + * @timings: the timing specification for the bridge, if any (may
> + * be NULL)
>    * @funcs: control functions
>    * @driver_private: pointer to the bridge driver's internal context
>    */
> @@ -240,6 +272,7 @@ struct drm_bridge {
>   	struct device_node *of_node;
>   #endif
>   	struct list_head list;
> +	const struct drm_bridge_timings *timings;
>   
>   	const struct drm_bridge_funcs *funcs;
>   	void *driver_private;
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 2/4 v7] drm/bridge: Provide a way to embed timing info in bridges

[Archit Taneja]

On 01/12/2018 01:18 PM, Linus Walleij wrote:
> After some discussion and failed patch sets trying to convey
> the right timing information between the display engine and
> a bridge using the connector, I try instead to use an optional
> timing information container in the bridge itself, so that
> display engines can retrieve it from any bridge and use it to
> determine how to drive outputs.

queued to drm-misc-next after fixing a minor checkpatch warning
regarding a line exceeding 80 characters.

Thanks,
Archit

> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v6->v7:
> - Fix the comment style to use the new inline type of kerneldoc
>    for struct members.
> - Need an explicit ACK/review by someone on this patch to continue
>    with the series...
> ChangeLog v5->v6:
> - Sort forward struct declarations alphabetically
> - Switch to using DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE to indicate
>    positive or negatice clock samling edge
> ChangeLog ->v5:
> - New patch
> ---
>   include/drm/drm_bridge.h | 33 +++++++++++++++++++++++++++++++++
>   1 file changed, 33 insertions(+)
> 
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 682d01ba920c..d3c2eea0bb63 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -29,6 +29,7 @@
>   #include <drm/drm_modes.h>
>   
>   struct drm_bridge;
> +struct drm_bridge_timings;
>   struct drm_panel;
>   
>   /**
> @@ -222,6 +223,35 @@ struct drm_bridge_funcs {
>   	void (*enable)(struct drm_bridge *bridge);
>   };
>   
> +/**
> + * struct drm_bridge_timings - timing information for the bridge
> + */
> +struct drm_bridge_timings {
> +	/**
> +	 * @sampling_edge:
> +	 *
> +	 * 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).
> +	 */
> +	u32 sampling_edge;
> +	/**
> +	 * @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;
> +};
> +
>   /**
>    * struct drm_bridge - central DRM bridge control structure
>    * @dev: DRM device this bridge belongs to
> @@ -229,6 +259,8 @@ struct drm_bridge_funcs {
>    * @next: the next bridge in the encoder chain
>    * @of_node: device node pointer to the bridge
>    * @list: to keep track of all added bridges
> + * @timings: the timing specification for the bridge, if any (may
> + * be NULL)
>    * @funcs: control functions
>    * @driver_private: pointer to the bridge driver's internal context
>    */
> @@ -240,6 +272,7 @@ struct drm_bridge {
>   	struct device_node *of_node;
>   #endif
>   	struct list_head list;
> +	const struct drm_bridge_timings *timings;
>   
>   	const struct drm_bridge_funcs *funcs;
>   	void *driver_private;
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 3/4 v7] drm/bridge: Add timing support to dumb VGA DAC

[Archit Taneja]

On 01/12/2018 01:18 PM, Linus Walleij wrote:
> This extends the dumb VGA DAC bridge to handle the THS8134A
> and THS8134B VGA DACs in addition to those already handled.
> 
> We assign the proper timing data to the pointer inside the
> bridge struct so display controllers that need to align their
> timings to the bridge can pick it up and work from there.

queued to drm-misc-next.

Thanks,
Archit

> 
> Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v5->v6:
> - Use DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE to indicate
>    the sampling edge of the clock signal.
> - Skip intermediate variable for timings.
> - Leave timings as NULL for really dumb VGA DACs.
> - Collect Laurent's Review tag.
> ChangeLog v4->v5:
> - Rewrite the support using the new concept of defining
>    fine-granular sampling (setup+hold) timing definitions
>    stored in the bridge timings struct.
> ChangeLog v3->v4:
> - Actually have the code syntactically correct and compiling :(
>    (Kconfig mistake.)
>    (...)
>    AS      usr/initramfs_data.o
>    AR      usr/built-in.o
>    CC      drivers/gpu/drm/bridge/dumb-vga-dac.o
>    AR      drivers/gpu/drm/bridge/built-in.o
>    AR      drivers/gpu/drm/built-in.o
>    AR      drivers/gpu/built-in.o
>    AR      drivers/built-in.o
>    (...)
> ChangeLog v2->v3:
> - Move const specifier.
> - Cut one line of code assigning bus flags.
> - Preserve the "ti,ths8135" compatible for elder device trees.
> ChangeLog v1->v2:
> - Alphabetize includes
> - Use a u32 with the bus polarity flags and just encode the
>    polarity using the DRM define directly.
> - Rename vendor_data to vendor_info.
> - Simplify assignment of the flag as it is just a simple
>    u32 now.
> - Probe all TI variants on the "ti,ths813x" wildcard for now,
>    we only need to know that the device is in this family to
>    set the clock edge flag right.
> ---
>   drivers/gpu/drm/bridge/dumb-vga-dac.c | 59 +++++++++++++++++++++++++++++++++--
>   1 file changed, 56 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> index de5e7dee7ad6..498d5948d1a8 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -11,6 +11,7 @@
>    */
>   
>   #include <linux/module.h>
> +#include <linux/of_device.h>
>   #include <linux/of_graph.h>
>   #include <linux/regulator/consumer.h>
>   
> @@ -204,6 +205,7 @@ 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);
>   
> @@ -222,10 +224,61 @@ static int dumb_vga_remove(struct platform_device *pdev)
>   	return 0;
>   }
>   
> +/*
> + * 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 */
> +	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.setup_time_ps = 500,
> +	.hold_time_ps = 1500,
> +};
> +
> +/*
> + * Information taken from the THS8134, THS8134A, THS8134B datasheet named
> + * "SLVS205D", dated May 1990, revised March 2000.
> + */
> +static const struct drm_bridge_timings ti_ths8134_dac_timings = {
> +	/* From timing diagram, datasheet page 9 */
> +	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	/* From datasheet, page 12 */
> +	.setup_time_ps = 3000,
> +	/* I guess this means latched input */
> +	.hold_time_ps = 0,
> +};
> +
> +/*
> + * Information taken from the THS8135 datasheet named "SLAS343B", dated
> + * May 2001, revised April 2013.
> + */
> +static const struct drm_bridge_timings ti_ths8135_dac_timings = {
> +	/* From timing diagram, datasheet page 14 */
> +	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	/* From datasheet, page 16 */
> +	.setup_time_ps = 2000,
> +	.hold_time_ps = 500,
> +};
> +
>   static const struct of_device_id dumb_vga_match[] = {
> -	{ .compatible = "dumb-vga-dac" },
> -	{ .compatible = "adi,adv7123" },
> -	{ .compatible = "ti,ths8135" },
> +	{
> +		.compatible = "dumb-vga-dac",
> +		.data = NULL,
> +	},
> +	{
> +		.compatible = "adi,adv7123",
> +		.data = &default_dac_timings,
> +	},
> +	{
> +		.compatible = "ti,ths8135",
> +		.data = &ti_ths8135_dac_timings,
> +	},
> +	{
> +		.compatible = "ti,ths8134",
> +		.data = &ti_ths8134_dac_timings,
> +	},
>   	{},
>   };
>   MODULE_DEVICE_TABLE(of, dumb_vga_match);
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 3/4 v7] drm/bridge: Add timing support to dumb VGA DAC

[Archit Taneja]

On 01/12/2018 01:18 PM, Linus Walleij wrote:
> This extends the dumb VGA DAC bridge to handle the THS8134A
> and THS8134B VGA DACs in addition to those already handled.
> 
> We assign the proper timing data to the pointer inside the
> bridge struct so display controllers that need to align their
> timings to the bridge can pick it up and work from there.

queued to drm-misc-next.

Thanks,
Archit

> 
> Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v5->v6:
> - Use DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE to indicate
>    the sampling edge of the clock signal.
> - Skip intermediate variable for timings.
> - Leave timings as NULL for really dumb VGA DACs.
> - Collect Laurent's Review tag.
> ChangeLog v4->v5:
> - Rewrite the support using the new concept of defining
>    fine-granular sampling (setup+hold) timing definitions
>    stored in the bridge timings struct.
> ChangeLog v3->v4:
> - Actually have the code syntactically correct and compiling :(
>    (Kconfig mistake.)
>    (...)
>    AS      usr/initramfs_data.o
>    AR      usr/built-in.o
>    CC      drivers/gpu/drm/bridge/dumb-vga-dac.o
>    AR      drivers/gpu/drm/bridge/built-in.o
>    AR      drivers/gpu/drm/built-in.o
>    AR      drivers/gpu/built-in.o
>    AR      drivers/built-in.o
>    (...)
> ChangeLog v2->v3:
> - Move const specifier.
> - Cut one line of code assigning bus flags.
> - Preserve the "ti,ths8135" compatible for elder device trees.
> ChangeLog v1->v2:
> - Alphabetize includes
> - Use a u32 with the bus polarity flags and just encode the
>    polarity using the DRM define directly.
> - Rename vendor_data to vendor_info.
> - Simplify assignment of the flag as it is just a simple
>    u32 now.
> - Probe all TI variants on the "ti,ths813x" wildcard for now,
>    we only need to know that the device is in this family to
>    set the clock edge flag right.
> ---
>   drivers/gpu/drm/bridge/dumb-vga-dac.c | 59 +++++++++++++++++++++++++++++++++--
>   1 file changed, 56 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> index de5e7dee7ad6..498d5948d1a8 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -11,6 +11,7 @@
>    */
>   
>   #include <linux/module.h>
> +#include <linux/of_device.h>
>   #include <linux/of_graph.h>
>   #include <linux/regulator/consumer.h>
>   
> @@ -204,6 +205,7 @@ 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);
>   
> @@ -222,10 +224,61 @@ static int dumb_vga_remove(struct platform_device *pdev)
>   	return 0;
>   }
>   
> +/*
> + * 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 */
> +	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.setup_time_ps = 500,
> +	.hold_time_ps = 1500,
> +};
> +
> +/*
> + * Information taken from the THS8134, THS8134A, THS8134B datasheet named
> + * "SLVS205D", dated May 1990, revised March 2000.
> + */
> +static const struct drm_bridge_timings ti_ths8134_dac_timings = {
> +	/* From timing diagram, datasheet page 9 */
> +	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	/* From datasheet, page 12 */
> +	.setup_time_ps = 3000,
> +	/* I guess this means latched input */
> +	.hold_time_ps = 0,
> +};
> +
> +/*
> + * Information taken from the THS8135 datasheet named "SLAS343B", dated
> + * May 2001, revised April 2013.
> + */
> +static const struct drm_bridge_timings ti_ths8135_dac_timings = {
> +	/* From timing diagram, datasheet page 14 */
> +	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	/* From datasheet, page 16 */
> +	.setup_time_ps = 2000,
> +	.hold_time_ps = 500,
> +};
> +
>   static const struct of_device_id dumb_vga_match[] = {
> -	{ .compatible = "dumb-vga-dac" },
> -	{ .compatible = "adi,adv7123" },
> -	{ .compatible = "ti,ths8135" },
> +	{
> +		.compatible = "dumb-vga-dac",
> +		.data = NULL,
> +	},
> +	{
> +		.compatible = "adi,adv7123",
> +		.data = &default_dac_timings,
> +	},
> +	{
> +		.compatible = "ti,ths8135",
> +		.data = &ti_ths8135_dac_timings,
> +	},
> +	{
> +		.compatible = "ti,ths8134",
> +		.data = &ti_ths8134_dac_timings,
> +	},
>   	{},
>   };
>   MODULE_DEVICE_TABLE(of, dumb_vga_match);
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 4/4 v7] drm/pl111: Support handling bridge timings

[Archit Taneja]

On 01/12/2018 01:18 PM, Linus Walleij wrote:
> If the bridge has a too strict setup time for the incoming
> signals, we may not be fast enough and then we need to
> compensate by outputting the signal on the inverse clock
> edge so it is for sure stable when the bridge samples it.
> 
> Since bridges in difference to panels does not expose their
> connectors, make the connector optional in the display
> setup code.
> 

Since Eric has already Ack'ed it, queued to drm-misc-next so that
the commits stay together.

Thanks,
Archit

> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Eric Anholt <eric@anholt.net>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v6->v7:
> - Collect Eric's ACK.
> ChangeLog v5->v6:
> - Collect Laurent's ACK.
> ChangeLog v4->v5:
> - Use the new bridge timings setup method.
> ---
>   drivers/gpu/drm/pl111/Kconfig         |  1 +
>   drivers/gpu/drm/pl111/pl111_display.c | 35 +++++++++++++++++++++++++++++++----
>   drivers/gpu/drm/pl111/pl111_drv.c     | 20 +++++++++++---------
>   3 files changed, 43 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig
> index e5e2abd66491..82cb3e60ddc8 100644
> --- a/drivers/gpu/drm/pl111/Kconfig
> +++ b/drivers/gpu/drm/pl111/Kconfig
> @@ -8,6 +8,7 @@ config DRM_PL111
>   	select DRM_GEM_CMA_HELPER
>   	select DRM_BRIDGE
>   	select DRM_PANEL_BRIDGE
> +	select DRM_DUMB_VGA_DAC
>   	select VT_HW_CONSOLE_BINDING if FRAMEBUFFER_CONSOLE
>   	help
>   	  Choose this option for DRM support for the PL111 CLCD controller.
> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> index 06c4bf756b69..7fe4040aea46 100644
> --- a/drivers/gpu/drm/pl111/pl111_display.c
> +++ b/drivers/gpu/drm/pl111/pl111_display.c
> @@ -94,6 +94,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
>   	const struct drm_display_mode *mode = &cstate->mode;
>   	struct drm_framebuffer *fb = plane->state->fb;
>   	struct drm_connector *connector = priv->connector;
> +	struct drm_bridge *bridge = priv->bridge;
>   	u32 cntl;
>   	u32 ppl, hsw, hfp, hbp;
>   	u32 lpp, vsw, vfp, vbp;
> @@ -143,11 +144,37 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
>   	if (mode->flags & DRM_MODE_FLAG_NVSYNC)
>   		tim2 |= TIM2_IVS;
>   
> -	if (connector->display_info.bus_flags & DRM_BUS_FLAG_DE_LOW)
> -		tim2 |= TIM2_IOE;
> +	if (connector) {
> +		if (connector->display_info.bus_flags & DRM_BUS_FLAG_DE_LOW)
> +			tim2 |= TIM2_IOE;
>   
> -	if (connector->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
> -		tim2 |= TIM2_IPC;
> +		if (connector->display_info.bus_flags &
> +		    DRM_BUS_FLAG_PIXDATA_NEGEDGE)
> +			tim2 |= TIM2_IPC;
> +	}
> +
> +	if (bridge) {
> +		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.
> +		 */
> +		if (btimings && btimings->setup_time_ps >= 3000)
> +			tim2 ^= TIM2_IPC;
> +	}
>   
>   	tim2 |= cpl << 16;
>   	writel(tim2, priv->regs + CLCD_TIM2);
> diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
> index 201d57d5cb54..101a9c7db6ff 100644
> --- a/drivers/gpu/drm/pl111/pl111_drv.c
> +++ b/drivers/gpu/drm/pl111/pl111_drv.c
> @@ -107,11 +107,17 @@ static int pl111_modeset_init(struct drm_device *dev)
>   			ret = PTR_ERR(bridge);
>   			goto out_config;
>   		}
> -		/*
> -		 * TODO: when we are using a different bridge than a panel
> -		 * (such as a dumb VGA connector) we need to devise a different
> -		 * method to get the connector out of the bridge.
> -		 */
> +	} else if (bridge) {
> +		dev_info(dev->dev, "Using non-panel bridge\n");
> +	} else {
> +		dev_err(dev->dev, "No bridge, exiting\n");
> +		return -ENODEV;
> +	}
> +
> +	priv->bridge = bridge;
> +	if (panel) {
> +		priv->panel = panel;
> +		priv->connector = panel->connector;
>   	}
>   
>   	ret = pl111_display_init(dev);
> @@ -125,10 +131,6 @@ static int pl111_modeset_init(struct drm_device *dev)
>   	if (ret)
>   		return ret;
>   
> -	priv->bridge = bridge;
> -	priv->panel = panel;
> -	priv->connector = panel->connector;
> -
>   	ret = drm_vblank_init(dev, 1);
>   	if (ret != 0) {
>   		dev_err(dev->dev, "Failed to init vblank\n");
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 4/4 v7] drm/pl111: Support handling bridge timings

[Archit Taneja]

On 01/12/2018 01:18 PM, Linus Walleij wrote:
> If the bridge has a too strict setup time for the incoming
> signals, we may not be fast enough and then we need to
> compensate by outputting the signal on the inverse clock
> edge so it is for sure stable when the bridge samples it.
> 
> Since bridges in difference to panels does not expose their
> connectors, make the connector optional in the display
> setup code.
> 

Since Eric has already Ack'ed it, queued to drm-misc-next so that
the commits stay together.

Thanks,
Archit

> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Eric Anholt <eric@anholt.net>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v6->v7:
> - Collect Eric's ACK.
> ChangeLog v5->v6:
> - Collect Laurent's ACK.
> ChangeLog v4->v5:
> - Use the new bridge timings setup method.
> ---
>   drivers/gpu/drm/pl111/Kconfig         |  1 +
>   drivers/gpu/drm/pl111/pl111_display.c | 35 +++++++++++++++++++++++++++++++----
>   drivers/gpu/drm/pl111/pl111_drv.c     | 20 +++++++++++---------
>   3 files changed, 43 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig
> index e5e2abd66491..82cb3e60ddc8 100644
> --- a/drivers/gpu/drm/pl111/Kconfig
> +++ b/drivers/gpu/drm/pl111/Kconfig
> @@ -8,6 +8,7 @@ config DRM_PL111
>   	select DRM_GEM_CMA_HELPER
>   	select DRM_BRIDGE
>   	select DRM_PANEL_BRIDGE
> +	select DRM_DUMB_VGA_DAC
>   	select VT_HW_CONSOLE_BINDING if FRAMEBUFFER_CONSOLE
>   	help
>   	  Choose this option for DRM support for the PL111 CLCD controller.
> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> index 06c4bf756b69..7fe4040aea46 100644
> --- a/drivers/gpu/drm/pl111/pl111_display.c
> +++ b/drivers/gpu/drm/pl111/pl111_display.c
> @@ -94,6 +94,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
>   	const struct drm_display_mode *mode = &cstate->mode;
>   	struct drm_framebuffer *fb = plane->state->fb;
>   	struct drm_connector *connector = priv->connector;
> +	struct drm_bridge *bridge = priv->bridge;
>   	u32 cntl;
>   	u32 ppl, hsw, hfp, hbp;
>   	u32 lpp, vsw, vfp, vbp;
> @@ -143,11 +144,37 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
>   	if (mode->flags & DRM_MODE_FLAG_NVSYNC)
>   		tim2 |= TIM2_IVS;
>   
> -	if (connector->display_info.bus_flags & DRM_BUS_FLAG_DE_LOW)
> -		tim2 |= TIM2_IOE;
> +	if (connector) {
> +		if (connector->display_info.bus_flags & DRM_BUS_FLAG_DE_LOW)
> +			tim2 |= TIM2_IOE;
>   
> -	if (connector->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
> -		tim2 |= TIM2_IPC;
> +		if (connector->display_info.bus_flags &
> +		    DRM_BUS_FLAG_PIXDATA_NEGEDGE)
> +			tim2 |= TIM2_IPC;
> +	}
> +
> +	if (bridge) {
> +		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.
> +		 */
> +		if (btimings && btimings->setup_time_ps >= 3000)
> +			tim2 ^= TIM2_IPC;
> +	}
>   
>   	tim2 |= cpl << 16;
>   	writel(tim2, priv->regs + CLCD_TIM2);
> diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
> index 201d57d5cb54..101a9c7db6ff 100644
> --- a/drivers/gpu/drm/pl111/pl111_drv.c
> +++ b/drivers/gpu/drm/pl111/pl111_drv.c
> @@ -107,11 +107,17 @@ static int pl111_modeset_init(struct drm_device *dev)
>   			ret = PTR_ERR(bridge);
>   			goto out_config;
>   		}
> -		/*
> -		 * TODO: when we are using a different bridge than a panel
> -		 * (such as a dumb VGA connector) we need to devise a different
> -		 * method to get the connector out of the bridge.
> -		 */
> +	} else if (bridge) {
> +		dev_info(dev->dev, "Using non-panel bridge\n");
> +	} else {
> +		dev_err(dev->dev, "No bridge, exiting\n");
> +		return -ENODEV;
> +	}
> +
> +	priv->bridge = bridge;
> +	if (panel) {
> +		priv->panel = panel;
> +		priv->connector = panel->connector;
>   	}
>   
>   	ret = pl111_display_init(dev);
> @@ -125,10 +131,6 @@ static int pl111_modeset_init(struct drm_device *dev)
>   	if (ret)
>   		return ret;
>   
> -	priv->bridge = bridge;
> -	priv->panel = panel;
> -	priv->connector = panel->connector;
> -
>   	ret = drm_vblank_init(dev, 1);
>   	if (ret != 0) {
>   		dev_err(dev->dev, "Failed to init vblank\n");
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 4/4 v7] drm/pl111: Support handling bridge timings

[Linus Walleij]

On Fri, Jan 12, 2018 at 10:25 AM, Archit Taneja <architt@codeaurora.org> wrote:
> On 01/12/2018 01:18 PM, Linus Walleij wrote:
>>
>> If the bridge has a too strict setup time for the incoming
>> signals, we may not be fast enough and then we need to
>> compensate by outputting the signal on the inverse clock
>> edge so it is for sure stable when the bridge samples it.
>>
>> Since bridges in difference to panels does not expose their
>> connectors, make the connector optional in the display
>> setup code.
>>
>
> Since Eric has already Ack'ed it, queued to drm-misc-next so that
> the commits stay together.

Thanks man!

I have commit access but this is fine too, it's a bit like an
definitive external ACK :)

Yours,
Linus Walleij

Re: [PATCH 4/4 v7] drm/pl111: Support handling bridge timings

[Linus Walleij]

On Fri, Jan 12, 2018 at 10:25 AM, Archit Taneja <architt@codeaurora.org> wrote:
> On 01/12/2018 01:18 PM, Linus Walleij wrote:
>>
>> If the bridge has a too strict setup time for the incoming
>> signals, we may not be fast enough and then we need to
>> compensate by outputting the signal on the inverse clock
>> edge so it is for sure stable when the bridge samples it.
>>
>> Since bridges in difference to panels does not expose their
>> connectors, make the connector optional in the display
>> setup code.
>>
>
> Since Eric has already Ack'ed it, queued to drm-misc-next so that
> the commits stay together.

Thanks man!

I have commit access but this is fine too, it's a bit like an
definitive external ACK :)

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

[PATCH 2/4 v7] drm/bridge: Provide a way to embed timing info in bridges

[Laurent Pinchart]

Hi Linus,

Thank you for the patch.

On Friday, 12 January 2018 09:48:52 EET Linus Walleij wrote:
> After some discussion and failed patch sets trying to convey
> the right timing information between the display engine and
> a bridge using the connector, I try instead to use an optional
> timing information container in the bridge itself, so that
> display engines can retrieve it from any bridge and use it to
> determine how to drive outputs.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> ChangeLog v6->v7:
> - Fix the comment style to use the new inline type of kerneldoc
>   for struct members.
> - Need an explicit ACK/review by someone on this patch to continue
>   with the series...
> ChangeLog v5->v6:
> - Sort forward struct declarations alphabetically
> - Switch to using DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE to indicate
>   positive or negatice clock samling edge
> ChangeLog ->v5:
> - New patch
> ---
>  include/drm/drm_bridge.h | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 682d01ba920c..d3c2eea0bb63 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -29,6 +29,7 @@
>  #include <drm/drm_modes.h>
> 
>  struct drm_bridge;
> +struct drm_bridge_timings;
>  struct drm_panel;
> 
>  /**
> @@ -222,6 +223,35 @@ struct drm_bridge_funcs {
>  	void (*enable)(struct drm_bridge *bridge);
>  };
> 
> +/**
> + * struct drm_bridge_timings - timing information for the bridge
> + */
> +struct drm_bridge_timings {
> +	/**
> +	 * @sampling_edge:
> +	 *
> +	 * 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).
> +	 */
> +	u32 sampling_edge;
> +	/**
> +	 * @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;
> +};
> +
>  /**
>   * struct drm_bridge - central DRM bridge control structure
>   * @dev: DRM device this bridge belongs to
> @@ -229,6 +259,8 @@ struct drm_bridge_funcs {
>   * @next: the next bridge in the encoder chain
>   * @of_node: device node pointer to the bridge
>   * @list: to keep track of all added bridges
> + * @timings: the timing specification for the bridge, if any (may
> + * be NULL)
>   * @funcs: control functions
>   * @driver_private: pointer to the bridge driver's internal context
>   */
> @@ -240,6 +272,7 @@ struct drm_bridge {
>  	struct device_node *of_node;
>  #endif
>  	struct list_head list;
> +	const struct drm_bridge_timings *timings;
> 
>  	const struct drm_bridge_funcs *funcs;
>  	void *driver_private;


-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/4 v7] drm/bridge: Provide a way to embed timing info in bridges

[Laurent Pinchart]

Hi Linus,

Thank you for the patch.

On Friday, 12 January 2018 09:48:52 EET Linus Walleij wrote:
> After some discussion and failed patch sets trying to convey
> the right timing information between the display engine and
> a bridge using the connector, I try instead to use an optional
> timing information container in the bridge itself, so that
> display engines can retrieve it from any bridge and use it to
> determine how to drive outputs.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> ChangeLog v6->v7:
> - Fix the comment style to use the new inline type of kerneldoc
>   for struct members.
> - Need an explicit ACK/review by someone on this patch to continue
>   with the series...
> ChangeLog v5->v6:
> - Sort forward struct declarations alphabetically
> - Switch to using DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE to indicate
>   positive or negatice clock samling edge
> ChangeLog ->v5:
> - New patch
> ---
>  include/drm/drm_bridge.h | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 682d01ba920c..d3c2eea0bb63 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -29,6 +29,7 @@
>  #include <drm/drm_modes.h>
> 
>  struct drm_bridge;
> +struct drm_bridge_timings;
>  struct drm_panel;
> 
>  /**
> @@ -222,6 +223,35 @@ struct drm_bridge_funcs {
>  	void (*enable)(struct drm_bridge *bridge);
>  };
> 
> +/**
> + * struct drm_bridge_timings - timing information for the bridge
> + */
> +struct drm_bridge_timings {
> +	/**
> +	 * @sampling_edge:
> +	 *
> +	 * 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).
> +	 */
> +	u32 sampling_edge;
> +	/**
> +	 * @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;
> +};
> +
>  /**
>   * struct drm_bridge - central DRM bridge control structure
>   * @dev: DRM device this bridge belongs to
> @@ -229,6 +259,8 @@ struct drm_bridge_funcs {
>   * @next: the next bridge in the encoder chain
>   * @of_node: device node pointer to the bridge
>   * @list: to keep track of all added bridges
> + * @timings: the timing specification for the bridge, if any (may
> + * be NULL)
>   * @funcs: control functions
>   * @driver_private: pointer to the bridge driver's internal context
>   */
> @@ -240,6 +272,7 @@ struct drm_bridge {
>  	struct device_node *of_node;
>  #endif
>  	struct list_head list;
> +	const struct drm_bridge_timings *timings;
> 
>  	const struct drm_bridge_funcs *funcs;
>  	void *driver_private;


-- 
Regards,

Laurent Pinchart

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