dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] drm/pl111: Error handling for CMA framebuffer
@ 2018-01-26 13:20 Linus Walleij
  2018-01-26 13:20 ` [PATCH 2/4] drm/pl111: Properly detect the PL110+ variant Linus Walleij
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Linus Walleij @ 2018-01-26 13:20 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Sean Paul, Eric Anholt
  Cc: linux-arm-kernel, dri-devel

When attaching the CMA framebuffer we need to check for
returned error pointers.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/pl111/pl111_drv.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index 101a9c7db6ff..31a0c4268cc6 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -141,6 +141,11 @@ static int pl111_modeset_init(struct drm_device *dev)
 
 	priv->fbdev = drm_fbdev_cma_init(dev, 32,
 					 dev->mode_config.num_connector);
+	if (IS_ERR(priv->fbdev)) {
+		dev_err(dev->dev, "Failed to initialize CMA framebuffer\n");
+		ret = PTR_ERR(priv->fbdev);
+		goto out_bridge;
+	}
 
 	drm_kms_helper_poll_init(dev);
 
-- 
2.14.3

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/4] drm/pl111: Properly detect the PL110+ variant
  2018-01-26 13:20 [PATCH 1/4] drm/pl111: Error handling for CMA framebuffer Linus Walleij
@ 2018-01-26 13:20 ` Linus Walleij
  2018-01-29 23:45   ` Eric Anholt
  2018-01-26 13:20 ` [PATCH 3/4] drm/pl111: Handle the Versatile RGB/BGR565 mode Linus Walleij
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Linus Walleij @ 2018-01-26 13:20 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Sean Paul, Eric Anholt
  Cc: linux-arm-kernel, dri-devel

With a bit of refactoring we can contain the variant data for
the "PL110+" version that is somewhere inbetween PL110 and PL111.
This is used on the ARM Versatile AB and Versatile PB.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/pl111/pl111_drm.h |   2 +
 drivers/gpu/drm/pl111/pl111_drv.c | 106 +++++++++++++++++++++-----------------
 2 files changed, 60 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h
index 440f53ebee8c..9cc0d424ff16 100644
--- a/drivers/gpu/drm/pl111/pl111_drm.h
+++ b/drivers/gpu/drm/pl111/pl111_drm.h
@@ -36,12 +36,14 @@ struct drm_minor;
  * struct pl111_variant_data - encodes IP differences
  * @name: the name of this variant
  * @is_pl110: this is the early PL110 variant
+ * @is_pl110_plus: this is the Versatile Pl110+ variant
  * @formats: array of supported pixel formats on this variant
  * @nformats: the length of the array of supported pixel formats
  */
 struct pl111_variant_data {
 	const char *name;
 	bool is_pl110;
+	bool is_pl110_plus;
 	const u32 *formats;
 	unsigned int nformats;
 };
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index 31a0c4268cc6..8da75089dc59 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -200,12 +200,67 @@ static struct drm_driver pl111_drm_driver = {
 #endif
 };
 
+/*
+ * This variant exist in early versions like the ARM Integrator
+ * and this version lacks the 565 and 444 pixel formats.
+ */
+static const u32 pl110_pixel_formats[] = {
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_ARGB8888,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_ABGR1555,
+	DRM_FORMAT_XBGR1555,
+	DRM_FORMAT_ARGB1555,
+	DRM_FORMAT_XRGB1555,
+};
+
+static const struct pl111_variant_data pl110_variant = {
+	.name = "PL110",
+	.is_pl110 = true,
+	.formats = pl110_pixel_formats,
+	.nformats = ARRAY_SIZE(pl110_pixel_formats),
+};
+
+/* This is the in-between PL110+ variant found in the ARM Versatile */
+static const struct pl111_variant_data pl110_plus_variant = {
+	.name = "PL110+",
+	.is_pl110 = true,
+	.is_pl110_plus = true,
+	.formats = pl110_pixel_formats,
+	.nformats = ARRAY_SIZE(pl110_pixel_formats),
+};
+
+/* RealView, Versatile Express etc use this modern variant */
+static const u32 pl111_pixel_formats[] = {
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_ARGB8888,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_BGR565,
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_ABGR1555,
+	DRM_FORMAT_XBGR1555,
+	DRM_FORMAT_ARGB1555,
+	DRM_FORMAT_XRGB1555,
+	DRM_FORMAT_ABGR4444,
+	DRM_FORMAT_XBGR4444,
+	DRM_FORMAT_ARGB4444,
+	DRM_FORMAT_XRGB4444,
+};
+
+static const struct pl111_variant_data pl111_variant = {
+	.name = "PL111",
+	.formats = pl111_pixel_formats,
+	.nformats = ARRAY_SIZE(pl111_pixel_formats),
+};
+
 static int pl111_amba_probe(struct amba_device *amba_dev,
 			    const struct amba_id *id)
 {
 	struct device *dev = &amba_dev->dev;
 	struct pl111_drm_dev_private *priv;
-	struct pl111_variant_data *variant = id->data;
+	const struct pl111_variant_data *variant = id->data;
 	struct drm_device *drm;
 	int ret;
 
@@ -219,7 +274,6 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
 	amba_set_drvdata(amba_dev, drm);
 	priv->drm = drm;
 	drm->dev_private = priv;
-	priv->variant = variant;
 
 	/*
 	 * The PL110 and PL111 variants have two registers
@@ -236,6 +290,7 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
 		 */
 		if (of_machine_is_compatible("arm,versatile-ab") ||
 		    of_machine_is_compatible("arm,versatile-pb")) {
+			variant = &pl110_plus_variant;
 			priv->ienb = CLCD_PL111_IENB;
 			priv->ctrl = CLCD_PL111_CNTL;
 		} else {
@@ -246,6 +301,7 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
 		priv->ienb = CLCD_PL111_IENB;
 		priv->ctrl = CLCD_PL111_CNTL;
 	}
+	priv->variant = variant;
 
 	priv->regs = devm_ioremap_resource(dev, &amba_dev->res);
 	if (IS_ERR(priv->regs)) {
@@ -298,52 +354,6 @@ static int pl111_amba_remove(struct amba_device *amba_dev)
 	return 0;
 }
 
-/*
- * This variant exist in early versions like the ARM Integrator
- * and this version lacks the 565 and 444 pixel formats.
- */
-static const u32 pl110_pixel_formats[] = {
-	DRM_FORMAT_ABGR8888,
-	DRM_FORMAT_XBGR8888,
-	DRM_FORMAT_ARGB8888,
-	DRM_FORMAT_XRGB8888,
-	DRM_FORMAT_ABGR1555,
-	DRM_FORMAT_XBGR1555,
-	DRM_FORMAT_ARGB1555,
-	DRM_FORMAT_XRGB1555,
-};
-
-static const struct pl111_variant_data pl110_variant = {
-	.name = "PL110",
-	.is_pl110 = true,
-	.formats = pl110_pixel_formats,
-	.nformats = ARRAY_SIZE(pl110_pixel_formats),
-};
-
-/* RealView, Versatile Express etc use this modern variant */
-static const u32 pl111_pixel_formats[] = {
-	DRM_FORMAT_ABGR8888,
-	DRM_FORMAT_XBGR8888,
-	DRM_FORMAT_ARGB8888,
-	DRM_FORMAT_XRGB8888,
-	DRM_FORMAT_BGR565,
-	DRM_FORMAT_RGB565,
-	DRM_FORMAT_ABGR1555,
-	DRM_FORMAT_XBGR1555,
-	DRM_FORMAT_ARGB1555,
-	DRM_FORMAT_XRGB1555,
-	DRM_FORMAT_ABGR4444,
-	DRM_FORMAT_XBGR4444,
-	DRM_FORMAT_ARGB4444,
-	DRM_FORMAT_XRGB4444,
-};
-
-static const struct pl111_variant_data pl111_variant = {
-	.name = "PL111",
-	.formats = pl111_pixel_formats,
-	.nformats = ARRAY_SIZE(pl111_pixel_formats),
-};
-
 static const struct amba_id pl111_id_table[] = {
 	{
 		.id = 0x00041110,
-- 
2.14.3

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/4] drm/pl111: Handle the Versatile RGB/BGR565 mode
  2018-01-26 13:20 [PATCH 1/4] drm/pl111: Error handling for CMA framebuffer Linus Walleij
  2018-01-26 13:20 ` [PATCH 2/4] drm/pl111: Properly detect the PL110+ variant Linus Walleij
@ 2018-01-26 13:20 ` Linus Walleij
  2018-01-26 13:20 ` [PATCH 4/4] drm/pl111: Support multiple endpoints on the CLCD Linus Walleij
  2018-01-29 23:39 ` [PATCH 1/4] drm/pl111: Error handling for CMA framebuffer Eric Anholt
  3 siblings, 0 replies; 9+ messages in thread
From: Linus Walleij @ 2018-01-26 13:20 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Sean Paul, Eric Anholt
  Cc: linux-arm-kernel, dri-devel

The ARM Versatile series can do RGB/BGR565 with an external
"PLD" (Programmable Logical Device). However the CLCD does not
have control bits for this, so it needs to be set into the
ordinary 16BPP mode, then the RGB/BGR565 handling of the pixel
data is handled by configuring the PLD through the external
register.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/pl111/pl111_display.c | 15 +++++++++++++--
 drivers/gpu/drm/pl111/pl111_drv.c     |  2 ++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 7fe4040aea46..90946fcd65ef 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -199,10 +199,17 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 		cntl |= CNTL_LCDBPP24 | CNTL_BGR;
 		break;
 	case DRM_FORMAT_BGR565:
-		cntl |= CNTL_LCDBPP16_565;
+		if (priv->variant->is_pl110)
+			cntl |= CNTL_LCDBPP16;
+		else
+			cntl |= CNTL_LCDBPP16_565;
 		break;
 	case DRM_FORMAT_RGB565:
-		cntl |= CNTL_LCDBPP16_565 | CNTL_BGR;
+		if (priv->variant->is_pl110)
+			cntl |= CNTL_LCDBPP16;
+		else
+			cntl |= CNTL_LCDBPP16_565;
+		cntl |= CNTL_BGR;
 		break;
 	case DRM_FORMAT_ABGR1555:
 	case DRM_FORMAT_XBGR1555:
@@ -226,6 +233,10 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 		break;
 	}
 
+	/* The PL110+ does the BGR routing externally */
+	if (priv->variant->is_pl110_plus)
+		cntl &= ~CNTL_BGR;
+
 	/* Power sequence: first enable and chill */
 	writel(cntl, priv->regs + priv->ctrl);
 
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index 8da75089dc59..f1f1b87b0e44 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -209,6 +209,8 @@ static const u32 pl110_pixel_formats[] = {
 	DRM_FORMAT_XBGR8888,
 	DRM_FORMAT_ARGB8888,
 	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_BGR565,
+	DRM_FORMAT_RGB565,
 	DRM_FORMAT_ABGR1555,
 	DRM_FORMAT_XBGR1555,
 	DRM_FORMAT_ARGB1555,
-- 
2.14.3

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/4] drm/pl111: Support multiple endpoints on the CLCD
  2018-01-26 13:20 [PATCH 1/4] drm/pl111: Error handling for CMA framebuffer Linus Walleij
  2018-01-26 13:20 ` [PATCH 2/4] drm/pl111: Properly detect the PL110+ variant Linus Walleij
  2018-01-26 13:20 ` [PATCH 3/4] drm/pl111: Handle the Versatile RGB/BGR565 mode Linus Walleij
@ 2018-01-26 13:20 ` Linus Walleij
  2018-01-29 23:55   ` Eric Anholt
  2018-01-29 23:39 ` [PATCH 1/4] drm/pl111: Error handling for CMA framebuffer Eric Anholt
  3 siblings, 1 reply; 9+ messages in thread
From: Linus Walleij @ 2018-01-26 13:20 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Sean Paul, Eric Anholt
  Cc: linux-arm-kernel, dri-devel

The Versatile PL110 implementations use multiple endpoints:
from the PL111 port, the lines are routed through a PLD,
and from there forked so the same lines go to a VGA DAC and
an external TFT panel connector. This is discrete wireing
so there is no way to turn of one output, i.e. this is
really two endpoints, not two ports.

We model this with multiple endpoints, so we need to loop
over the available endpoints, check for panel or bridge on
each and accumulate the result before continuing.

The code already will give the panel preference over the
bridge, if present, so the output will be sent to the panel
if both a panel and a bridge is present on two endpoints
of the same port.

If they all return -EPROBE_DEFER we return -EPROBE_DEFER
as well.

If just one endpoint is present on the port, the behaviour
is the same as before.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/pl111/pl111_drv.c | 62 +++++++++++++++++++++++++++++++++++----
 1 file changed, 56 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index f1f1b87b0e44..7e9f5efe3274 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -58,6 +58,8 @@
 #include <linux/dma-buf.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_graph.h>
 
 #include <drm/drmP.h>
 #include <drm/drm_atomic_helper.h>
@@ -84,9 +86,13 @@ static int pl111_modeset_init(struct drm_device *dev)
 {
 	struct drm_mode_config *mode_config;
 	struct pl111_drm_dev_private *priv = dev->dev_private;
-	struct drm_panel *panel;
-	struct drm_bridge *bridge;
+	struct device_node *np = dev->dev->of_node;
+	struct device_node *remote;
+	struct drm_panel *panel = NULL;
+	struct drm_bridge *bridge = NULL;
+	bool defer = false;
 	int ret = 0;
+	int i;
 
 	drm_mode_config_init(dev);
 	mode_config = &dev->mode_config;
@@ -96,10 +102,54 @@ static int pl111_modeset_init(struct drm_device *dev)
 	mode_config->min_height = 1;
 	mode_config->max_height = 768;
 
-	ret = drm_of_find_panel_or_bridge(dev->dev->of_node,
-					  0, 0, &panel, &bridge);
-	if (ret && ret != -ENODEV)
-		return ret;
+	i = 0;
+	for_each_endpoint_of_node(np, remote) {
+		struct drm_panel *tmp_panel;
+		struct drm_bridge *tmp_bridge;
+
+		dev_dbg(dev->dev, "checking endpoint %d\n", i);
+
+		ret = drm_of_find_panel_or_bridge(dev->dev->of_node,
+						  0, i,
+						  &tmp_panel,
+						  &tmp_bridge);
+		if (ret) {
+			if (ret == -EPROBE_DEFER) {
+				/*
+				 * Something deferred, but that is often just
+				 * another way of saying -ENODEV, but let's
+				 * cast a vote for later deferral.
+				 */
+				defer = true;
+			} else if (ret != -ENODEV) {
+				/* Continue, maybe something else is working */
+				dev_err(dev->dev,
+					"endpoint %d returns %d\n", i, ret);
+			}
+		}
+
+		if (tmp_panel) {
+			dev_info(dev->dev,
+				 "found panel on endpoint %d\n", i);
+			panel = tmp_panel;
+		}
+		if (tmp_bridge) {
+			dev_info(dev->dev,
+				 "found bridge on endpoint %d\n", i);
+			bridge = tmp_bridge;
+		}
+
+		i++;
+	}
+
+	/*
+	 * If we can't find neither panel nor bridge on any of the
+	 * endpoints, and any of them retured -EPROBE_DEFER, then
+	 * let's defer this driver too.
+	 */
+	if ((!panel && !bridge) && defer)
+		return -EPROBE_DEFER;
+
 	if (panel) {
 		bridge = drm_panel_bridge_add(panel,
 					      DRM_MODE_CONNECTOR_Unknown);
-- 
2.14.3

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] drm/pl111: Error handling for CMA framebuffer
  2018-01-26 13:20 [PATCH 1/4] drm/pl111: Error handling for CMA framebuffer Linus Walleij
                   ` (2 preceding siblings ...)
  2018-01-26 13:20 ` [PATCH 4/4] drm/pl111: Support multiple endpoints on the CLCD Linus Walleij
@ 2018-01-29 23:39 ` Eric Anholt
  3 siblings, 0 replies; 9+ messages in thread
From: Eric Anholt @ 2018-01-29 23:39 UTC (permalink / raw)
  To: Linus Walleij, Daniel Vetter, Jani Nikula, Sean Paul
  Cc: linux-arm-kernel, dri-devel


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

Linus Walleij <linus.walleij@linaro.org> writes:

> When attaching the CMA framebuffer we need to check for
> returned error pointers.

This doesn't seem to be necessary on drm-misc-next.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] drm/pl111: Properly detect the PL110+ variant
  2018-01-26 13:20 ` [PATCH 2/4] drm/pl111: Properly detect the PL110+ variant Linus Walleij
@ 2018-01-29 23:45   ` Eric Anholt
  2018-01-30 15:39     ` Linus Walleij
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Anholt @ 2018-01-29 23:45 UTC (permalink / raw)
  To: Linus Walleij, Daniel Vetter, Jani Nikula, Sean Paul
  Cc: linux-arm-kernel, dri-devel


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

Linus Walleij <linus.walleij@linaro.org> writes:

> With a bit of refactoring we can contain the variant data for
> the "PL110+" version that is somewhere inbetween PL110 and PL111.
> This is used on the ARM Versatile AB and Versatile PB.

Patch 2-3 are:

Reviewed-by: Eric Anholt <eric@anholt.net>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] drm/pl111: Support multiple endpoints on the CLCD
  2018-01-26 13:20 ` [PATCH 4/4] drm/pl111: Support multiple endpoints on the CLCD Linus Walleij
@ 2018-01-29 23:55   ` Eric Anholt
  2018-02-01 12:49     ` Linus Walleij
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Anholt @ 2018-01-29 23:55 UTC (permalink / raw)
  To: Linus Walleij, Daniel Vetter, Jani Nikula, Sean Paul
  Cc: linux-arm-kernel, dri-devel


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

Linus Walleij <linus.walleij@linaro.org> writes:

> The Versatile PL110 implementations use multiple endpoints:
> from the PL111 port, the lines are routed through a PLD,
> and from there forked so the same lines go to a VGA DAC and
> an external TFT panel connector. This is discrete wireing
> so there is no way to turn of one output, i.e. this is
> really two endpoints, not two ports.
>
> We model this with multiple endpoints, so we need to loop
> over the available endpoints, check for panel or bridge on
> each and accumulate the result before continuing.
>
> The code already will give the panel preference over the
> bridge, if present, so the output will be sent to the panel
> if both a panel and a bridge is present on two endpoints
> of the same port.
>
> If they all return -EPROBE_DEFER we return -EPROBE_DEFER
> as well.
>
> If just one endpoint is present on the port, the behaviour
> is the same as before.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Huh, from the binding I would have thought that we were describing
things to the point of the pins coming out of the PLD, not past whatever
splitting comes after that.

I'm also confused how this would work.  You're talking about the DT
looking like:

	clcd@10020000 {
		compatible = "arm,pl111", "arm,primecell";
		reg = <0x10020000 0x1000>;
		interrupt-names = "combined";
		interrupts = <0 44 4>;
		clocks = <&oscclk1>, <&oscclk2>;
		clock-names = "clcdclk", "apb_pclk";
		max-memory-bandwidth = <94371840>; /* Bps, 1024x768@60 16bpp */

		port {
			dac_pads: endpoint1 {
				remote-endpoint = <&vgadac>;
				arm,pl11x,tft-r0g0b0-pads = <0 8 16>;
			};
			tft_pads: endpoint2 {
				remote-endpoint = <&tftpanel>;
				arm,pl11x,tft-r0g0b0-pads = <0 8 16>;
			};
		};

	};


Are you anticipating that a DT would actually connect up to two
endpoints on the CLCD?  How should we resolve the pads property, in that
case?  Is there much point in supporting this, if we don't actually
support panels or bridges on both of the endpoints at once (since we
pick only one to do panel/bridge setup/teardown on)?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] drm/pl111: Properly detect the PL110+ variant
  2018-01-29 23:45   ` Eric Anholt
@ 2018-01-30 15:39     ` Linus Walleij
  0 siblings, 0 replies; 9+ messages in thread
From: Linus Walleij @ 2018-01-30 15:39 UTC (permalink / raw)
  To: Eric Anholt
  Cc: Linux ARM, Daniel Vetter, Sean Paul, open list:DRM PANEL DRIVERS,
	Jani Nikula

On Tue, Jan 30, 2018 at 12:45 AM, Eric Anholt <eric@anholt.net> wrote:
> Linus Walleij <linus.walleij@linaro.org> writes:
>
>> With a bit of refactoring we can contain the variant data for
>> the "PL110+" version that is somewhere inbetween PL110 and PL111.
>> This is used on the ARM Versatile AB and Versatile PB.
>
> Patch 2-3 are:
>
> Reviewed-by: Eric Anholt <eric@anholt.net>

Thanks! I'll have to respin this patch 2 because it was a bit
to bad at separation of concerns.

Sorry for the fuzz.
Linus Walleii

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] drm/pl111: Support multiple endpoints on the CLCD
  2018-01-29 23:55   ` Eric Anholt
@ 2018-02-01 12:49     ` Linus Walleij
  0 siblings, 0 replies; 9+ messages in thread
From: Linus Walleij @ 2018-02-01 12:49 UTC (permalink / raw)
  To: Eric Anholt; +Cc: Linux ARM, Daniel Vetter, open list:DRM PANEL DRIVERS

On Tue, Jan 30, 2018 at 12:55 AM, Eric Anholt <eric@anholt.net> wrote:
> Linus Walleij <linus.walleij@linaro.org> writes:
>
>> The Versatile PL110 implementations use multiple endpoints:
>> from the PL111 port, the lines are routed through a PLD,
>> and from there forked so the same lines go to a VGA DAC and
>> an external TFT panel connector. This is discrete wireing
>> so there is no way to turn of one output, i.e. this is
>> really two endpoints, not two ports.
>>
>> We model this with multiple endpoints, so we need to loop
>> over the available endpoints, check for panel or bridge on
>> each and accumulate the result before continuing.
>>
>> The code already will give the panel preference over the
>> bridge, if present, so the output will be sent to the panel
>> if both a panel and a bridge is present on two endpoints
>> of the same port.
>>
>> If they all return -EPROBE_DEFER we return -EPROBE_DEFER
>> as well.
>>
>> If just one endpoint is present on the port, the behaviour
>> is the same as before.
>>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>
> Huh, from the binding I would have thought that we were describing
> things to the point of the pins coming out of the PLD, not past whatever
> splitting comes after that.
>
> I'm also confused how this would work.  You're talking about the DT
> looking like:
>
>         clcd@10020000 {
>                 compatible = "arm,pl111", "arm,primecell";
>                 reg = <0x10020000 0x1000>;
>                 interrupt-names = "combined";
>                 interrupts = <0 44 4>;
>                 clocks = <&oscclk1>, <&oscclk2>;
>                 clock-names = "clcdclk", "apb_pclk";
>                 max-memory-bandwidth = <94371840>; /* Bps, 1024x768@60 16bpp */
>
>                 port {
>                         dac_pads: endpoint1 {
>                                 remote-endpoint = <&vgadac>;
>                                 arm,pl11x,tft-r0g0b0-pads = <0 8 16>;
>                         };
>                         tft_pads: endpoint2 {
>                                 remote-endpoint = <&tftpanel>;
>                                 arm,pl11x,tft-r0g0b0-pads = <0 8 16>;
>                         };
>                 };
>
>         };
>
>
> Are you anticipating that a DT would actually connect up to two
> endpoints on the CLCD?

Yes and I have a patch doing exactly that as well.

The thing is that the ARM Versatile reference design
actually does this. It has no electronic protection for the
VGA DAC hanging off the same lines as the panel, and there
is no way of determining if there is a monitor connected to
the dumb VGA DAC or not.

When the system boots up and the panel comes up (I am just
doing the same as the code in fbdev and let the TFT panel
take precedence) there is some shouting about unsupported
resolution on the connected VGA monitor, of course.

Whether this is good taste in design is another question.

But describing it like this in the device tree is indeed (AFAICT)
describing the hardware as it is soldered up, and
documented in official ARM reference design documentation...

This arrangement can be clearly seen in
ARM DUI 0225D, page 3-41, figure 3-19.
http://infocenter.arm.com/help/topic/com.arm.doc.dui0225d/DUI0225D_versatile_application_baseboard_arm926ej_s_ug.pdf

>  How should we resolve the pads property, in that
> case?  Is there much point in supporting this, if we don't actually
> support panels or bridges on both of the endpoints at once (since we
> pick only one to do panel/bridge setup/teardown on)?

There is no way for software to drive both connected displays
at the same time, like trying to find a least common denominator
or something, it simply has to choose one. It is somewhat natural
to select a connected TFT panel (these need to be consciously
pressed in on top of the board) over the VGA DAC. If I just set
the panel node to "disabled" in the device tree, the VGA comes
up instead. The old fbdev driver also let the panel take
precedence.

So the patch checks if there is both a panel and a DAC
connected and selects the panel over the VGA if and only
if a panel is connected.

I guess we need to pick up the pads of the endpoint that
we end up connecting to a bridge and handle that from there.

The pads are only used in Versatile Express and Nomadik,
I haven't gotten to those yet, but the Versatile Express has
only DVI VGA out, and the Nomadik has only a TFT panel,
not this duality, so those designs will not be ambigous about
what pads to use.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-02-01 12:49 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-26 13:20 [PATCH 1/4] drm/pl111: Error handling for CMA framebuffer Linus Walleij
2018-01-26 13:20 ` [PATCH 2/4] drm/pl111: Properly detect the PL110+ variant Linus Walleij
2018-01-29 23:45   ` Eric Anholt
2018-01-30 15:39     ` Linus Walleij
2018-01-26 13:20 ` [PATCH 3/4] drm/pl111: Handle the Versatile RGB/BGR565 mode Linus Walleij
2018-01-26 13:20 ` [PATCH 4/4] drm/pl111: Support multiple endpoints on the CLCD Linus Walleij
2018-01-29 23:55   ` Eric Anholt
2018-02-01 12:49     ` Linus Walleij
2018-01-29 23:39 ` [PATCH 1/4] drm/pl111: Error handling for CMA framebuffer Eric Anholt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).