All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] First set of PL111 enhancements
@ 2017-08-30 18:07 ` Linus Walleij
  0 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2017-08-30 18:07 UTC (permalink / raw)
  To: linux-arm-kernel

Hi folks,

here is the first few patches to enhance the PL111 with the following
objectives:

- Consolidation - use the panel bridge, which I figured out after some
  tinkering (yes I will fix TVE200 too).
- Handle the PL110 variant
- Handle the power-on delay documented in both manuals
- Add callbacks to handle subvariants, and a scratch implementation
  of the Versatile extra register (no Nomadik yet).

Next steps will be (I guess)

- Make the driver also use the dumb VGA connector bridge for the platforms
  that simply connects the PL11x to a DAC to VGA
- Proper panel driver for the Versatile board
- Nomadik and its panel
- When things work, stepwise move the ARM platforms using this over to
  the DRM PL111 driver

Stuff that gets properly reviewed I can queue on drm-misc myself these days.

Linus Walleij (7):
  drm/pl111: Cleanup local header file
  drm/pl111: Add all registers to debugfs
  drm/pl111: Replace custom connector with panel bridge
  drm/pl111: Enable PL110 variant
  drm/pl111: Insert delay before powering up PL11x
  drm/pl111: Add optional variant display en/disable callbacks
  drm/pl111: Add handling of Versatile platforms

 drivers/gpu/drm/pl111/Kconfig           |   3 +-
 drivers/gpu/drm/pl111/Makefile          |   4 +-
 drivers/gpu/drm/pl111/pl111_connector.c | 127 ---------------
 drivers/gpu/drm/pl111/pl111_debugfs.c   |   6 +
 drivers/gpu/drm/pl111/pl111_display.c   |  82 +++++-----
 drivers/gpu/drm/pl111/pl111_drm.h       |  38 +++--
 drivers/gpu/drm/pl111/pl111_drv.c       | 151 +++++++++++++++---
 drivers/gpu/drm/pl111/pl111_versatile.c | 270 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/pl111/pl111_versatile.h |   9 ++
 9 files changed, 491 insertions(+), 199 deletions(-)
 delete mode 100644 drivers/gpu/drm/pl111/pl111_connector.c
 create mode 100644 drivers/gpu/drm/pl111/pl111_versatile.c
 create mode 100644 drivers/gpu/drm/pl111/pl111_versatile.h

-- 
2.13.5

^ permalink raw reply	[flat|nested] 32+ messages in thread
* [PATCH 0/7] First set of PL111 enhancements
@ 2017-08-30 18:07 ` Linus Walleij
  0 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2017-08-30 18:07 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Sean Paul, Eric Anholt
  Cc: linux-arm-kernel, dri-devel

Hi folks,

here is the first few patches to enhance the PL111 with the following
objectives:

- Consolidation - use the panel bridge, which I figured out after some
  tinkering (yes I will fix TVE200 too).
- Handle the PL110 variant
- Handle the power-on delay documented in both manuals
- Add callbacks to handle subvariants, and a scratch implementation
  of the Versatile extra register (no Nomadik yet).

Next steps will be (I guess)

- Make the driver also use the dumb VGA connector bridge for the platforms
  that simply connects the PL11x to a DAC to VGA
- Proper panel driver for the Versatile board
- Nomadik and its panel
- When things work, stepwise move the ARM platforms using this over to
  the DRM PL111 driver

Stuff that gets properly reviewed I can queue on drm-misc myself these days.

Linus Walleij (7):
  drm/pl111: Cleanup local header file
  drm/pl111: Add all registers to debugfs
  drm/pl111: Replace custom connector with panel bridge
  drm/pl111: Enable PL110 variant
  drm/pl111: Insert delay before powering up PL11x
  drm/pl111: Add optional variant display en/disable callbacks
  drm/pl111: Add handling of Versatile platforms

 drivers/gpu/drm/pl111/Kconfig           |   3 +-
 drivers/gpu/drm/pl111/Makefile          |   4 +-
 drivers/gpu/drm/pl111/pl111_connector.c | 127 ---------------
 drivers/gpu/drm/pl111/pl111_debugfs.c   |   6 +
 drivers/gpu/drm/pl111/pl111_display.c   |  82 +++++-----
 drivers/gpu/drm/pl111/pl111_drm.h       |  38 +++--
 drivers/gpu/drm/pl111/pl111_drv.c       | 151 +++++++++++++++---
 drivers/gpu/drm/pl111/pl111_versatile.c | 270 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/pl111/pl111_versatile.h |   9 ++
 9 files changed, 491 insertions(+), 199 deletions(-)
 delete mode 100644 drivers/gpu/drm/pl111/pl111_connector.c
 create mode 100644 drivers/gpu/drm/pl111/pl111_versatile.c
 create mode 100644 drivers/gpu/drm/pl111/pl111_versatile.h

-- 
2.13.5

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

^ permalink raw reply	[flat|nested] 32+ messages in thread
* [PATCH 1/7] drm/pl111: Cleanup local header file
  2017-08-30 18:07 ` Linus Walleij
@ 2017-08-30 18:07   ` Linus Walleij
  -1 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2017-08-30 18:07 UTC (permalink / raw)
  To: linux-arm-kernel

The header file contains prototypes for two nonexisting
functions. Get rid of them.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/pl111/pl111_drm.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h
index 5c685bfc8fdc..a97f303f6833 100644
--- a/drivers/gpu/drm/pl111/pl111_drm.h
+++ b/drivers/gpu/drm/pl111/pl111_drm.h
@@ -58,10 +58,6 @@ int pl111_enable_vblank(struct drm_device *drm, unsigned int crtc);
 void pl111_disable_vblank(struct drm_device *drm, unsigned int crtc);
 irqreturn_t pl111_irq(int irq, void *data);
 int pl111_connector_init(struct drm_device *dev);
-int pl111_encoder_init(struct drm_device *dev);
-int pl111_dumb_create(struct drm_file *file_priv,
-		      struct drm_device *dev,
-		      struct drm_mode_create_dumb *args);
 int pl111_debugfs_init(struct drm_minor *minor);
 
 #endif /* _PL111_DRM_H_ */
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH 1/7] drm/pl111: Cleanup local header file
@ 2017-08-30 18:07   ` Linus Walleij
  0 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2017-08-30 18:07 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Sean Paul, Eric Anholt
  Cc: linux-arm-kernel, dri-devel

The header file contains prototypes for two nonexisting
functions. Get rid of them.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/pl111/pl111_drm.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h
index 5c685bfc8fdc..a97f303f6833 100644
--- a/drivers/gpu/drm/pl111/pl111_drm.h
+++ b/drivers/gpu/drm/pl111/pl111_drm.h
@@ -58,10 +58,6 @@ int pl111_enable_vblank(struct drm_device *drm, unsigned int crtc);
 void pl111_disable_vblank(struct drm_device *drm, unsigned int crtc);
 irqreturn_t pl111_irq(int irq, void *data);
 int pl111_connector_init(struct drm_device *dev);
-int pl111_encoder_init(struct drm_device *dev);
-int pl111_dumb_create(struct drm_file *file_priv,
-		      struct drm_device *dev,
-		      struct drm_mode_create_dumb *args);
 int pl111_debugfs_init(struct drm_minor *minor);
 
 #endif /* _PL111_DRM_H_ */
-- 
2.13.5

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

^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH 2/7] drm/pl111: Add all registers to debugfs
  2017-08-30 18:07 ` Linus Walleij
@ 2017-08-30 18:07   ` Linus Walleij
  -1 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2017-08-30 18:07 UTC (permalink / raw)
  To: linux-arm-kernel

This adds all the main control registers to the debugfs
register file. This was helpful for my debugging so it will
likely help others as well.

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

diff --git a/drivers/gpu/drm/pl111/pl111_debugfs.c b/drivers/gpu/drm/pl111/pl111_debugfs.c
index 0d9dee199b2c..7ddc7e3b9e7d 100644
--- a/drivers/gpu/drm/pl111/pl111_debugfs.c
+++ b/drivers/gpu/drm/pl111/pl111_debugfs.c
@@ -22,8 +22,14 @@ static const struct {
 	REGDEF(CLCD_TIM2),
 	REGDEF(CLCD_TIM3),
 	REGDEF(CLCD_UBAS),
+	REGDEF(CLCD_LBAS),
 	REGDEF(CLCD_PL111_CNTL),
 	REGDEF(CLCD_PL111_IENB),
+	REGDEF(CLCD_PL111_RIS),
+	REGDEF(CLCD_PL111_MIS),
+	REGDEF(CLCD_PL111_ICR),
+	REGDEF(CLCD_PL111_UCUR),
+	REGDEF(CLCD_PL111_LCUR),
 };
 
 int pl111_debugfs_regs(struct seq_file *m, void *unused)
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH 2/7] drm/pl111: Add all registers to debugfs
@ 2017-08-30 18:07   ` Linus Walleij
  0 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2017-08-30 18:07 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Sean Paul, Eric Anholt
  Cc: linux-arm-kernel, dri-devel

This adds all the main control registers to the debugfs
register file. This was helpful for my debugging so it will
likely help others as well.

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

diff --git a/drivers/gpu/drm/pl111/pl111_debugfs.c b/drivers/gpu/drm/pl111/pl111_debugfs.c
index 0d9dee199b2c..7ddc7e3b9e7d 100644
--- a/drivers/gpu/drm/pl111/pl111_debugfs.c
+++ b/drivers/gpu/drm/pl111/pl111_debugfs.c
@@ -22,8 +22,14 @@ static const struct {
 	REGDEF(CLCD_TIM2),
 	REGDEF(CLCD_TIM3),
 	REGDEF(CLCD_UBAS),
+	REGDEF(CLCD_LBAS),
 	REGDEF(CLCD_PL111_CNTL),
 	REGDEF(CLCD_PL111_IENB),
+	REGDEF(CLCD_PL111_RIS),
+	REGDEF(CLCD_PL111_MIS),
+	REGDEF(CLCD_PL111_ICR),
+	REGDEF(CLCD_PL111_UCUR),
+	REGDEF(CLCD_PL111_LCUR),
 };
 
 int pl111_debugfs_regs(struct seq_file *m, void *unused)
-- 
2.13.5

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

^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH 3/7] drm/pl111: Replace custom connector with panel bridge
  2017-08-30 18:07 ` Linus Walleij
@ 2017-08-30 18:07   ` Linus Walleij
  -1 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2017-08-30 18:07 UTC (permalink / raw)
  To: linux-arm-kernel

This replaces the custom connector in the PL111 with the
panel bridge helper.

This works nicely for all standard panels, but since there
are several PL11x-based systems that will need to use the dumb
VGA connector bridge we use drm_of_find_panel_or_bridge()
and make some headroom for dealing with bridges that are
not panels as well, and drop a TODO in the code.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/pl111/Kconfig           |   3 +-
 drivers/gpu/drm/pl111/Makefile          |   3 +-
 drivers/gpu/drm/pl111/pl111_connector.c | 127 --------------------------------
 drivers/gpu/drm/pl111/pl111_display.c   |  15 ++--
 drivers/gpu/drm/pl111/pl111_drm.h       |  18 ++---
 drivers/gpu/drm/pl111/pl111_drv.c       |  62 +++++++++++-----
 6 files changed, 64 insertions(+), 164 deletions(-)
 delete mode 100644 drivers/gpu/drm/pl111/pl111_connector.c

diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig
index bbfba87cd1a8..e5e2abd66491 100644
--- a/drivers/gpu/drm/pl111/Kconfig
+++ b/drivers/gpu/drm/pl111/Kconfig
@@ -6,7 +6,8 @@ config DRM_PL111
 	select DRM_KMS_HELPER
 	select DRM_KMS_CMA_HELPER
 	select DRM_GEM_CMA_HELPER
-	select DRM_PANEL
+	select DRM_BRIDGE
+	select DRM_PANEL_BRIDGE
 	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/Makefile b/drivers/gpu/drm/pl111/Makefile
index 59483d610ef5..c5f8f9684848 100644
--- a/drivers/gpu/drm/pl111/Makefile
+++ b/drivers/gpu/drm/pl111/Makefile
@@ -1,5 +1,4 @@
-pl111_drm-y +=	pl111_connector.o \
-		pl111_display.o \
+pl111_drm-y +=	pl111_display.o \
 		pl111_drv.o
 
 pl111_drm-$(CONFIG_DEBUG_FS) += pl111_debugfs.o
diff --git a/drivers/gpu/drm/pl111/pl111_connector.c b/drivers/gpu/drm/pl111/pl111_connector.c
deleted file mode 100644
index 3f213d7e7692..000000000000
--- a/drivers/gpu/drm/pl111/pl111_connector.c
+++ /dev/null
@@ -1,127 +0,0 @@
-/*
- * (C) COPYRIGHT 2012-2013 ARM Limited. All rights reserved.
- *
- * Parts of this file were based on sources as follows:
- *
- * Copyright (c) 2006-2008 Intel Corporation
- * Copyright (c) 2007 Dave Airlie <airlied@linux.ie>
- * Copyright (C) 2011 Texas Instruments
- *
- * This program is free software and is provided to you under the terms of the
- * GNU General Public License version 2 as published by the Free Software
- * Foundation, and any use by you of this program is subject to the terms of
- * such GNU licence.
- *
- */
-
-/**
- * pl111_drm_connector.c
- * Implementation of the connector functions for PL111 DRM
- */
-#include <linux/amba/clcd-regs.h>
-#include <linux/version.h>
-#include <linux/shmem_fs.h>
-#include <linux/dma-buf.h>
-
-#include <drm/drmP.h>
-#include <drm/drm_atomic_helper.h>
-#include <drm/drm_crtc_helper.h>
-#include <drm/drm_of.h>
-#include <drm/drm_panel.h>
-
-#include "pl111_drm.h"
-
-static void pl111_connector_destroy(struct drm_connector *connector)
-{
-	struct pl111_drm_connector *pl111_connector =
-		to_pl111_connector(connector);
-
-	if (pl111_connector->panel)
-		drm_panel_detach(pl111_connector->panel);
-
-	drm_connector_unregister(connector);
-	drm_connector_cleanup(connector);
-}
-
-static enum drm_connector_status pl111_connector_detect(struct drm_connector
-							*connector, bool force)
-{
-	struct pl111_drm_connector *pl111_connector =
-		to_pl111_connector(connector);
-
-	return (pl111_connector->panel ?
-		connector_status_connected :
-		connector_status_disconnected);
-}
-
-static int pl111_connector_helper_get_modes(struct drm_connector *connector)
-{
-	struct pl111_drm_connector *pl111_connector =
-		to_pl111_connector(connector);
-
-	if (!pl111_connector->panel)
-		return 0;
-
-	return drm_panel_get_modes(pl111_connector->panel);
-}
-
-const struct drm_connector_funcs connector_funcs = {
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = pl111_connector_destroy,
-	.detect = pl111_connector_detect,
-	.dpms = drm_atomic_helper_connector_dpms,
-	.reset = drm_atomic_helper_connector_reset,
-	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
-	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-
-const struct drm_connector_helper_funcs connector_helper_funcs = {
-	.get_modes = pl111_connector_helper_get_modes,
-};
-
-/* Walks the OF graph to find the panel node and then asks DRM to look
- * up the panel.
- */
-static struct drm_panel *pl111_get_panel(struct device *dev)
-{
-	struct device_node *endpoint, *panel_node;
-	struct device_node *np = dev->of_node;
-	struct drm_panel *panel;
-
-	endpoint = of_graph_get_next_endpoint(np, NULL);
-	if (!endpoint) {
-		dev_err(dev, "no endpoint to fetch panel\n");
-		return NULL;
-	}
-
-	/* don't proceed if we have an endpoint but no panel_node tied to it */
-	panel_node = of_graph_get_remote_port_parent(endpoint);
-	of_node_put(endpoint);
-	if (!panel_node) {
-		dev_err(dev, "no valid panel node\n");
-		return NULL;
-	}
-
-	panel = of_drm_find_panel(panel_node);
-	of_node_put(panel_node);
-
-	return panel;
-}
-
-int pl111_connector_init(struct drm_device *dev)
-{
-	struct pl111_drm_dev_private *priv = dev->dev_private;
-	struct pl111_drm_connector *pl111_connector = &priv->connector;
-	struct drm_connector *connector = &pl111_connector->connector;
-
-	drm_connector_init(dev, connector, &connector_funcs,
-			   DRM_MODE_CONNECTOR_DPI);
-	drm_connector_helper_add(connector, &connector_helper_funcs);
-
-	pl111_connector->panel = pl111_get_panel(dev->dev);
-	if (pl111_connector->panel)
-		drm_panel_attach(pl111_connector->panel, connector);
-
-	return 0;
-}
-
diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index c6ca4f1bbd49..ef86ef60aed1 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -93,7 +93,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 	struct pl111_drm_dev_private *priv = drm->dev_private;
 	const struct drm_display_mode *mode = &cstate->mode;
 	struct drm_framebuffer *fb = plane->state->fb;
-	struct drm_connector *connector = &priv->connector.connector;
+	struct drm_connector *connector = priv->connector;
 	u32 cntl;
 	u32 ppl, hsw, hfp, hbp;
 	u32 lpp, vsw, vfp, vbp;
@@ -155,7 +155,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 
 	writel(0, priv->regs + CLCD_TIM3);
 
-	drm_panel_prepare(priv->connector.panel);
+	drm_panel_prepare(priv->panel);
 
 	/* Enable and Power Up */
 	cntl = CNTL_LCDEN | CNTL_LCDTFT | CNTL_LCDPWR | CNTL_LCDVCOMP(1);
@@ -203,7 +203,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 
 	writel(cntl, priv->regs + CLCD_PL111_CNTL);
 
-	drm_panel_enable(priv->connector.panel);
+	drm_panel_enable(priv->panel);
 
 	drm_crtc_vblank_on(crtc);
 }
@@ -216,12 +216,12 @@ void pl111_display_disable(struct drm_simple_display_pipe *pipe)
 
 	drm_crtc_vblank_off(crtc);
 
-	drm_panel_disable(priv->connector.panel);
+	drm_panel_disable(priv->panel);
 
 	/* Disable and Power Down */
 	writel(0, priv->regs + CLCD_PL111_CNTL);
 
-	drm_panel_unprepare(priv->connector.panel);
+	drm_panel_unprepare(priv->panel);
 
 	clk_disable_unprepare(priv->clk);
 }
@@ -457,9 +457,12 @@ int pl111_display_init(struct drm_device *drm)
 	ret = drm_simple_display_pipe_init(drm, &priv->pipe,
 					   &pl111_display_funcs,
 					   formats, ARRAY_SIZE(formats),
-					   &priv->connector.connector);
+					   priv->connector);
 	if (ret)
 		return ret;
 
+	/* We need the encoder to attach the bridge */
+	priv->encoder = &priv->pipe.encoder;
+
 	return 0;
 }
diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h
index a97f303f6833..8804af0f8997 100644
--- a/drivers/gpu/drm/pl111/pl111_drm.h
+++ b/drivers/gpu/drm/pl111/pl111_drm.h
@@ -21,21 +21,23 @@
 
 #include <drm/drm_gem.h>
 #include <drm/drm_simple_kms_helper.h>
+#include <drm/drm_connector.h>
+#include <drm/drm_encoder.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_bridge.h>
 #include <linux/clk-provider.h>
 
 #define CLCD_IRQ_NEXTBASE_UPDATE BIT(2)
 
 struct drm_minor;
 
-struct pl111_drm_connector {
-	struct drm_connector connector;
-	struct drm_panel *panel;
-};
-
 struct pl111_drm_dev_private {
 	struct drm_device *drm;
 
-	struct pl111_drm_connector connector;
+	struct drm_connector *connector;
+	struct drm_encoder *encoder;
+	struct drm_panel *panel;
+	struct drm_bridge *bridge;
 	struct drm_simple_display_pipe pipe;
 	struct drm_fbdev_cma *fbdev;
 
@@ -50,14 +52,10 @@ struct pl111_drm_dev_private {
 	spinlock_t tim2_lock;
 };
 
-#define to_pl111_connector(x) \
-	container_of(x, struct pl111_drm_connector, connector)
-
 int pl111_display_init(struct drm_device *dev);
 int pl111_enable_vblank(struct drm_device *drm, unsigned int crtc);
 void pl111_disable_vblank(struct drm_device *drm, unsigned int crtc);
 irqreturn_t pl111_irq(int irq, void *data);
-int pl111_connector_init(struct drm_device *dev);
 int pl111_debugfs_init(struct drm_minor *minor);
 
 #endif /* _PL111_DRM_H_ */
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index ac8771be70b0..e66cbf202e17 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -67,6 +67,9 @@
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_of.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_panel.h>
 
 #include "pl111_drm.h"
 
@@ -82,6 +85,8 @@ 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;
 	int ret = 0;
 
 	drm_mode_config_init(dev);
@@ -92,34 +97,46 @@ static int pl111_modeset_init(struct drm_device *dev)
 	mode_config->min_height = 1;
 	mode_config->max_height = 768;
 
-	ret = pl111_connector_init(dev);
-	if (ret) {
-		dev_err(dev->dev, "Failed to create pl111_drm_connector\n");
-		goto out_config;
-	}
-
-	/* Don't actually attach if we didn't find a drm_panel
-	 * attached to us.  This will allow a kernel to include both
-	 * the fbdev pl111 driver and this one, and choose between
-	 * them based on which subsystem has support for the panel.
-	 */
-	if (!priv->connector.panel) {
-		dev_info(dev->dev,
-			 "Disabling due to lack of DRM panel device.\n");
-		ret = -ENODEV;
-		goto out_config;
+	ret = drm_of_find_panel_or_bridge(dev->dev->of_node,
+					  0, 0, &panel, &bridge);
+	if (ret && ret != -ENODEV)
+		return ret;
+	if (panel) {
+		bridge = drm_panel_bridge_add(panel,
+					      DRM_MODE_CONNECTOR_Unknown);
+		if (IS_ERR(bridge)) {
+			ret = PTR_ERR(bridge);
+			goto out_config;
+		}
 	}
 
 	ret = pl111_display_init(dev);
 	if (ret != 0) {
 		dev_err(dev->dev, "Failed to init display\n");
-		goto out_config;
+		goto out_bridge;
+	}
+
+	if (bridge) {
+		ret = drm_bridge_attach(priv->encoder, bridge, NULL);
+		if (ret)
+			goto out_bridge;
+	}
+
+	/*
+	 * 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.
+	 */
+	if (panel) {
+		priv->panel = panel;
+		priv->connector = panel->connector;
 	}
+	priv->bridge = bridge;
 
 	ret = drm_vblank_init(dev, 1);
 	if (ret != 0) {
 		dev_err(dev->dev, "Failed to init vblank\n");
-		goto out_config;
+		goto out_bridge;
 	}
 
 	drm_mode_config_reset(dev);
@@ -131,6 +148,11 @@ static int pl111_modeset_init(struct drm_device *dev)
 
 	goto finish;
 
+out_bridge:
+	if (panel)
+		drm_panel_bridge_remove(bridge);
+	else
+		drm_bridge_remove(bridge);
 out_config:
 	drm_mode_config_cleanup(dev);
 finish:
@@ -237,6 +259,10 @@ static int pl111_amba_remove(struct amba_device *amba_dev)
 	drm_dev_unregister(drm);
 	if (priv->fbdev)
 		drm_fbdev_cma_fini(priv->fbdev);
+	if (priv->panel)
+		drm_panel_bridge_remove(priv->bridge);
+	else
+		drm_bridge_remove(priv->bridge);
 	drm_mode_config_cleanup(drm);
 	drm_dev_unref(drm);
 
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH 3/7] drm/pl111: Replace custom connector with panel bridge
@ 2017-08-30 18:07   ` Linus Walleij
  0 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2017-08-30 18:07 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Sean Paul, Eric Anholt
  Cc: linux-arm-kernel, dri-devel

This replaces the custom connector in the PL111 with the
panel bridge helper.

This works nicely for all standard panels, but since there
are several PL11x-based systems that will need to use the dumb
VGA connector bridge we use drm_of_find_panel_or_bridge()
and make some headroom for dealing with bridges that are
not panels as well, and drop a TODO in the code.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/pl111/Kconfig           |   3 +-
 drivers/gpu/drm/pl111/Makefile          |   3 +-
 drivers/gpu/drm/pl111/pl111_connector.c | 127 --------------------------------
 drivers/gpu/drm/pl111/pl111_display.c   |  15 ++--
 drivers/gpu/drm/pl111/pl111_drm.h       |  18 ++---
 drivers/gpu/drm/pl111/pl111_drv.c       |  62 +++++++++++-----
 6 files changed, 64 insertions(+), 164 deletions(-)
 delete mode 100644 drivers/gpu/drm/pl111/pl111_connector.c

diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig
index bbfba87cd1a8..e5e2abd66491 100644
--- a/drivers/gpu/drm/pl111/Kconfig
+++ b/drivers/gpu/drm/pl111/Kconfig
@@ -6,7 +6,8 @@ config DRM_PL111
 	select DRM_KMS_HELPER
 	select DRM_KMS_CMA_HELPER
 	select DRM_GEM_CMA_HELPER
-	select DRM_PANEL
+	select DRM_BRIDGE
+	select DRM_PANEL_BRIDGE
 	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/Makefile b/drivers/gpu/drm/pl111/Makefile
index 59483d610ef5..c5f8f9684848 100644
--- a/drivers/gpu/drm/pl111/Makefile
+++ b/drivers/gpu/drm/pl111/Makefile
@@ -1,5 +1,4 @@
-pl111_drm-y +=	pl111_connector.o \
-		pl111_display.o \
+pl111_drm-y +=	pl111_display.o \
 		pl111_drv.o
 
 pl111_drm-$(CONFIG_DEBUG_FS) += pl111_debugfs.o
diff --git a/drivers/gpu/drm/pl111/pl111_connector.c b/drivers/gpu/drm/pl111/pl111_connector.c
deleted file mode 100644
index 3f213d7e7692..000000000000
--- a/drivers/gpu/drm/pl111/pl111_connector.c
+++ /dev/null
@@ -1,127 +0,0 @@
-/*
- * (C) COPYRIGHT 2012-2013 ARM Limited. All rights reserved.
- *
- * Parts of this file were based on sources as follows:
- *
- * Copyright (c) 2006-2008 Intel Corporation
- * Copyright (c) 2007 Dave Airlie <airlied@linux.ie>
- * Copyright (C) 2011 Texas Instruments
- *
- * This program is free software and is provided to you under the terms of the
- * GNU General Public License version 2 as published by the Free Software
- * Foundation, and any use by you of this program is subject to the terms of
- * such GNU licence.
- *
- */
-
-/**
- * pl111_drm_connector.c
- * Implementation of the connector functions for PL111 DRM
- */
-#include <linux/amba/clcd-regs.h>
-#include <linux/version.h>
-#include <linux/shmem_fs.h>
-#include <linux/dma-buf.h>
-
-#include <drm/drmP.h>
-#include <drm/drm_atomic_helper.h>
-#include <drm/drm_crtc_helper.h>
-#include <drm/drm_of.h>
-#include <drm/drm_panel.h>
-
-#include "pl111_drm.h"
-
-static void pl111_connector_destroy(struct drm_connector *connector)
-{
-	struct pl111_drm_connector *pl111_connector =
-		to_pl111_connector(connector);
-
-	if (pl111_connector->panel)
-		drm_panel_detach(pl111_connector->panel);
-
-	drm_connector_unregister(connector);
-	drm_connector_cleanup(connector);
-}
-
-static enum drm_connector_status pl111_connector_detect(struct drm_connector
-							*connector, bool force)
-{
-	struct pl111_drm_connector *pl111_connector =
-		to_pl111_connector(connector);
-
-	return (pl111_connector->panel ?
-		connector_status_connected :
-		connector_status_disconnected);
-}
-
-static int pl111_connector_helper_get_modes(struct drm_connector *connector)
-{
-	struct pl111_drm_connector *pl111_connector =
-		to_pl111_connector(connector);
-
-	if (!pl111_connector->panel)
-		return 0;
-
-	return drm_panel_get_modes(pl111_connector->panel);
-}
-
-const struct drm_connector_funcs connector_funcs = {
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = pl111_connector_destroy,
-	.detect = pl111_connector_detect,
-	.dpms = drm_atomic_helper_connector_dpms,
-	.reset = drm_atomic_helper_connector_reset,
-	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
-	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-
-const struct drm_connector_helper_funcs connector_helper_funcs = {
-	.get_modes = pl111_connector_helper_get_modes,
-};
-
-/* Walks the OF graph to find the panel node and then asks DRM to look
- * up the panel.
- */
-static struct drm_panel *pl111_get_panel(struct device *dev)
-{
-	struct device_node *endpoint, *panel_node;
-	struct device_node *np = dev->of_node;
-	struct drm_panel *panel;
-
-	endpoint = of_graph_get_next_endpoint(np, NULL);
-	if (!endpoint) {
-		dev_err(dev, "no endpoint to fetch panel\n");
-		return NULL;
-	}
-
-	/* don't proceed if we have an endpoint but no panel_node tied to it */
-	panel_node = of_graph_get_remote_port_parent(endpoint);
-	of_node_put(endpoint);
-	if (!panel_node) {
-		dev_err(dev, "no valid panel node\n");
-		return NULL;
-	}
-
-	panel = of_drm_find_panel(panel_node);
-	of_node_put(panel_node);
-
-	return panel;
-}
-
-int pl111_connector_init(struct drm_device *dev)
-{
-	struct pl111_drm_dev_private *priv = dev->dev_private;
-	struct pl111_drm_connector *pl111_connector = &priv->connector;
-	struct drm_connector *connector = &pl111_connector->connector;
-
-	drm_connector_init(dev, connector, &connector_funcs,
-			   DRM_MODE_CONNECTOR_DPI);
-	drm_connector_helper_add(connector, &connector_helper_funcs);
-
-	pl111_connector->panel = pl111_get_panel(dev->dev);
-	if (pl111_connector->panel)
-		drm_panel_attach(pl111_connector->panel, connector);
-
-	return 0;
-}
-
diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index c6ca4f1bbd49..ef86ef60aed1 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -93,7 +93,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 	struct pl111_drm_dev_private *priv = drm->dev_private;
 	const struct drm_display_mode *mode = &cstate->mode;
 	struct drm_framebuffer *fb = plane->state->fb;
-	struct drm_connector *connector = &priv->connector.connector;
+	struct drm_connector *connector = priv->connector;
 	u32 cntl;
 	u32 ppl, hsw, hfp, hbp;
 	u32 lpp, vsw, vfp, vbp;
@@ -155,7 +155,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 
 	writel(0, priv->regs + CLCD_TIM3);
 
-	drm_panel_prepare(priv->connector.panel);
+	drm_panel_prepare(priv->panel);
 
 	/* Enable and Power Up */
 	cntl = CNTL_LCDEN | CNTL_LCDTFT | CNTL_LCDPWR | CNTL_LCDVCOMP(1);
@@ -203,7 +203,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 
 	writel(cntl, priv->regs + CLCD_PL111_CNTL);
 
-	drm_panel_enable(priv->connector.panel);
+	drm_panel_enable(priv->panel);
 
 	drm_crtc_vblank_on(crtc);
 }
@@ -216,12 +216,12 @@ void pl111_display_disable(struct drm_simple_display_pipe *pipe)
 
 	drm_crtc_vblank_off(crtc);
 
-	drm_panel_disable(priv->connector.panel);
+	drm_panel_disable(priv->panel);
 
 	/* Disable and Power Down */
 	writel(0, priv->regs + CLCD_PL111_CNTL);
 
-	drm_panel_unprepare(priv->connector.panel);
+	drm_panel_unprepare(priv->panel);
 
 	clk_disable_unprepare(priv->clk);
 }
@@ -457,9 +457,12 @@ int pl111_display_init(struct drm_device *drm)
 	ret = drm_simple_display_pipe_init(drm, &priv->pipe,
 					   &pl111_display_funcs,
 					   formats, ARRAY_SIZE(formats),
-					   &priv->connector.connector);
+					   priv->connector);
 	if (ret)
 		return ret;
 
+	/* We need the encoder to attach the bridge */
+	priv->encoder = &priv->pipe.encoder;
+
 	return 0;
 }
diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h
index a97f303f6833..8804af0f8997 100644
--- a/drivers/gpu/drm/pl111/pl111_drm.h
+++ b/drivers/gpu/drm/pl111/pl111_drm.h
@@ -21,21 +21,23 @@
 
 #include <drm/drm_gem.h>
 #include <drm/drm_simple_kms_helper.h>
+#include <drm/drm_connector.h>
+#include <drm/drm_encoder.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_bridge.h>
 #include <linux/clk-provider.h>
 
 #define CLCD_IRQ_NEXTBASE_UPDATE BIT(2)
 
 struct drm_minor;
 
-struct pl111_drm_connector {
-	struct drm_connector connector;
-	struct drm_panel *panel;
-};
-
 struct pl111_drm_dev_private {
 	struct drm_device *drm;
 
-	struct pl111_drm_connector connector;
+	struct drm_connector *connector;
+	struct drm_encoder *encoder;
+	struct drm_panel *panel;
+	struct drm_bridge *bridge;
 	struct drm_simple_display_pipe pipe;
 	struct drm_fbdev_cma *fbdev;
 
@@ -50,14 +52,10 @@ struct pl111_drm_dev_private {
 	spinlock_t tim2_lock;
 };
 
-#define to_pl111_connector(x) \
-	container_of(x, struct pl111_drm_connector, connector)
-
 int pl111_display_init(struct drm_device *dev);
 int pl111_enable_vblank(struct drm_device *drm, unsigned int crtc);
 void pl111_disable_vblank(struct drm_device *drm, unsigned int crtc);
 irqreturn_t pl111_irq(int irq, void *data);
-int pl111_connector_init(struct drm_device *dev);
 int pl111_debugfs_init(struct drm_minor *minor);
 
 #endif /* _PL111_DRM_H_ */
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index ac8771be70b0..e66cbf202e17 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -67,6 +67,9 @@
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_of.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_panel.h>
 
 #include "pl111_drm.h"
 
@@ -82,6 +85,8 @@ 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;
 	int ret = 0;
 
 	drm_mode_config_init(dev);
@@ -92,34 +97,46 @@ static int pl111_modeset_init(struct drm_device *dev)
 	mode_config->min_height = 1;
 	mode_config->max_height = 768;
 
-	ret = pl111_connector_init(dev);
-	if (ret) {
-		dev_err(dev->dev, "Failed to create pl111_drm_connector\n");
-		goto out_config;
-	}
-
-	/* Don't actually attach if we didn't find a drm_panel
-	 * attached to us.  This will allow a kernel to include both
-	 * the fbdev pl111 driver and this one, and choose between
-	 * them based on which subsystem has support for the panel.
-	 */
-	if (!priv->connector.panel) {
-		dev_info(dev->dev,
-			 "Disabling due to lack of DRM panel device.\n");
-		ret = -ENODEV;
-		goto out_config;
+	ret = drm_of_find_panel_or_bridge(dev->dev->of_node,
+					  0, 0, &panel, &bridge);
+	if (ret && ret != -ENODEV)
+		return ret;
+	if (panel) {
+		bridge = drm_panel_bridge_add(panel,
+					      DRM_MODE_CONNECTOR_Unknown);
+		if (IS_ERR(bridge)) {
+			ret = PTR_ERR(bridge);
+			goto out_config;
+		}
 	}
 
 	ret = pl111_display_init(dev);
 	if (ret != 0) {
 		dev_err(dev->dev, "Failed to init display\n");
-		goto out_config;
+		goto out_bridge;
+	}
+
+	if (bridge) {
+		ret = drm_bridge_attach(priv->encoder, bridge, NULL);
+		if (ret)
+			goto out_bridge;
+	}
+
+	/*
+	 * 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.
+	 */
+	if (panel) {
+		priv->panel = panel;
+		priv->connector = panel->connector;
 	}
+	priv->bridge = bridge;
 
 	ret = drm_vblank_init(dev, 1);
 	if (ret != 0) {
 		dev_err(dev->dev, "Failed to init vblank\n");
-		goto out_config;
+		goto out_bridge;
 	}
 
 	drm_mode_config_reset(dev);
@@ -131,6 +148,11 @@ static int pl111_modeset_init(struct drm_device *dev)
 
 	goto finish;
 
+out_bridge:
+	if (panel)
+		drm_panel_bridge_remove(bridge);
+	else
+		drm_bridge_remove(bridge);
 out_config:
 	drm_mode_config_cleanup(dev);
 finish:
@@ -237,6 +259,10 @@ static int pl111_amba_remove(struct amba_device *amba_dev)
 	drm_dev_unregister(drm);
 	if (priv->fbdev)
 		drm_fbdev_cma_fini(priv->fbdev);
+	if (priv->panel)
+		drm_panel_bridge_remove(priv->bridge);
+	else
+		drm_bridge_remove(priv->bridge);
 	drm_mode_config_cleanup(drm);
 	drm_dev_unref(drm);
 
-- 
2.13.5

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

^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH 4/7] drm/pl111: Enable PL110 variant
  2017-08-30 18:07 ` Linus Walleij
@ 2017-08-30 18:07   ` Linus Walleij
  -1 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2017-08-30 18:07 UTC (permalink / raw)
  To: linux-arm-kernel

We detect and enable the use of the PL110 variant, an earlier
incarnation of PL111. The only real difference is that the
control and interrupt enable registers have swapped place.
The Versatile AB and Versatile PB have a variant inbetween
PL110 and PL111, it is PL110 but they have already swapped the
two registers so those two need a bit of special handling.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/pl111/pl111_display.c | 27 +++--------
 drivers/gpu/drm/pl111/pl111_drm.h     | 17 +++++++
 drivers/gpu/drm/pl111/pl111_drv.c     | 84 ++++++++++++++++++++++++++++++++++-
 3 files changed, 105 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index ef86ef60aed1..6447f36c243a 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -201,7 +201,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 		break;
 	}
 
-	writel(cntl, priv->regs + CLCD_PL111_CNTL);
+	writel(cntl, priv->regs + priv->ctrl);
 
 	drm_panel_enable(priv->panel);
 
@@ -219,7 +219,7 @@ void pl111_display_disable(struct drm_simple_display_pipe *pipe)
 	drm_panel_disable(priv->panel);
 
 	/* Disable and Power Down */
-	writel(0, priv->regs + CLCD_PL111_CNTL);
+	writel(0, priv->regs + priv->ctrl);
 
 	drm_panel_unprepare(priv->panel);
 
@@ -259,7 +259,7 @@ int pl111_enable_vblank(struct drm_device *drm, unsigned int crtc)
 {
 	struct pl111_drm_dev_private *priv = drm->dev_private;
 
-	writel(CLCD_IRQ_NEXTBASE_UPDATE, priv->regs + CLCD_PL111_IENB);
+	writel(CLCD_IRQ_NEXTBASE_UPDATE, priv->regs + priv->ienb);
 
 	return 0;
 }
@@ -268,7 +268,7 @@ void pl111_disable_vblank(struct drm_device *drm, unsigned int crtc)
 {
 	struct pl111_drm_dev_private *priv = drm->dev_private;
 
-	writel(0, priv->regs + CLCD_PL111_IENB);
+	writel(0, priv->regs + priv->ienb);
 }
 
 static int pl111_display_prepare_fb(struct drm_simple_display_pipe *pipe,
@@ -412,22 +412,6 @@ int pl111_display_init(struct drm_device *drm)
 	struct device_node *endpoint;
 	u32 tft_r0b0g0[3];
 	int ret;
-	static const u32 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,
-	};
 
 	endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
 	if (!endpoint)
@@ -456,7 +440,8 @@ int pl111_display_init(struct drm_device *drm)
 
 	ret = drm_simple_display_pipe_init(drm, &priv->pipe,
 					   &pl111_display_funcs,
-					   formats, ARRAY_SIZE(formats),
+					   priv->variant->formats,
+					   priv->variant->nformats,
 					   priv->connector);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h
index 8804af0f8997..b316a8a0fbc0 100644
--- a/drivers/gpu/drm/pl111/pl111_drm.h
+++ b/drivers/gpu/drm/pl111/pl111_drm.h
@@ -31,6 +31,20 @@
 
 struct drm_minor;
 
+/**
+ * struct pl111_variant_data - encodes IP differences
+ * @name: the name of this variant
+ * @is_pl110: this is the early 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;
+	const u32 *formats;
+	unsigned int nformats;
+};
+
 struct pl111_drm_dev_private {
 	struct drm_device *drm;
 
@@ -42,6 +56,8 @@ struct pl111_drm_dev_private {
 	struct drm_fbdev_cma *fbdev;
 
 	void *regs;
+	u32 ienb;
+	u32 ctrl;
 	/* The pixel clock (a reference to our clock divider off of CLCDCLK). */
 	struct clk *clk;
 	/* pl111's internal clock divider. */
@@ -50,6 +66,7 @@ struct pl111_drm_dev_private {
 	 * subsystem and pl111_display_enable().
 	 */
 	spinlock_t tim2_lock;
+	struct pl111_variant_data *variant;
 };
 
 int pl111_display_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index e66cbf202e17..f6863c0fb809 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -206,6 +206,7 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
 {
 	struct device *dev = &amba_dev->dev;
 	struct pl111_drm_dev_private *priv;
+	struct pl111_variant_data *variant = id->data;
 	struct drm_device *drm;
 	int ret;
 
@@ -219,6 +220,33 @@ 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
+	 * swapped: interrupt enable and control. For this reason
+	 * we use offsets that we can change per variant.
+	 */
+	if (variant->is_pl110) {
+		/*
+		 * The ARM Versatile boards are even more special:
+		 * their PrimeCell ID say they are PL110 but the
+		 * control and interrupt enable registers are anyway
+		 * swapped to the PL111 order so they are not following
+		 * the PL110 datasheet.
+		 */
+		if (of_machine_is_compatible("arm,versatile-ab") ||
+		    of_machine_is_compatible("arm,versatile-pb")) {
+			priv->ienb = CLCD_PL111_IENB;
+			priv->ctrl = CLCD_PL111_CNTL;
+		} else {
+			priv->ienb = CLCD_PL110_IENB;
+			priv->ctrl = CLCD_PL110_CNTL;
+		}
+	} else {
+		priv->ienb = CLCD_PL111_IENB;
+		priv->ctrl = CLCD_PL111_CNTL;
+	}
 
 	priv->regs = devm_ioremap_resource(dev, &amba_dev->res);
 	if (IS_ERR(priv->regs)) {
@@ -227,10 +255,10 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
 	}
 
 	/* turn off interrupts before requesting the irq */
-	writel(0, priv->regs + CLCD_PL111_IENB);
+	writel(0, priv->regs + priv->ienb);
 
 	ret = devm_request_irq(dev, amba_dev->irq[0], pl111_irq, 0,
-			       "pl111", priv);
+			       variant->name, priv);
 	if (ret != 0) {
 		dev_err(dev, "%s failed irq %d\n", __func__, ret);
 		return ret;
@@ -269,10 +297,62 @@ 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,
+};
+
+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,
+};
+
+struct pl111_variant_data pl111_variant = {
+	.name = "PL111",
+	.formats = pl111_pixel_formats,
+	.nformats = ARRAY_SIZE(pl111_pixel_formats),
+};
+
 static struct amba_id pl111_id_table[] = {
 	{
+		.id = 0x00041110,
+		.mask = 0x000fffff,
+		.data = &pl110_variant,
+	},
+	{
 		.id = 0x00041111,
 		.mask = 0x000fffff,
+		.data = &pl111_variant,
 	},
 	{0, 0},
 };
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH 4/7] drm/pl111: Enable PL110 variant
@ 2017-08-30 18:07   ` Linus Walleij
  0 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2017-08-30 18:07 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Sean Paul, Eric Anholt
  Cc: linux-arm-kernel, dri-devel

We detect and enable the use of the PL110 variant, an earlier
incarnation of PL111. The only real difference is that the
control and interrupt enable registers have swapped place.
The Versatile AB and Versatile PB have a variant inbetween
PL110 and PL111, it is PL110 but they have already swapped the
two registers so those two need a bit of special handling.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/pl111/pl111_display.c | 27 +++--------
 drivers/gpu/drm/pl111/pl111_drm.h     | 17 +++++++
 drivers/gpu/drm/pl111/pl111_drv.c     | 84 ++++++++++++++++++++++++++++++++++-
 3 files changed, 105 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index ef86ef60aed1..6447f36c243a 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -201,7 +201,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 		break;
 	}
 
-	writel(cntl, priv->regs + CLCD_PL111_CNTL);
+	writel(cntl, priv->regs + priv->ctrl);
 
 	drm_panel_enable(priv->panel);
 
@@ -219,7 +219,7 @@ void pl111_display_disable(struct drm_simple_display_pipe *pipe)
 	drm_panel_disable(priv->panel);
 
 	/* Disable and Power Down */
-	writel(0, priv->regs + CLCD_PL111_CNTL);
+	writel(0, priv->regs + priv->ctrl);
 
 	drm_panel_unprepare(priv->panel);
 
@@ -259,7 +259,7 @@ int pl111_enable_vblank(struct drm_device *drm, unsigned int crtc)
 {
 	struct pl111_drm_dev_private *priv = drm->dev_private;
 
-	writel(CLCD_IRQ_NEXTBASE_UPDATE, priv->regs + CLCD_PL111_IENB);
+	writel(CLCD_IRQ_NEXTBASE_UPDATE, priv->regs + priv->ienb);
 
 	return 0;
 }
@@ -268,7 +268,7 @@ void pl111_disable_vblank(struct drm_device *drm, unsigned int crtc)
 {
 	struct pl111_drm_dev_private *priv = drm->dev_private;
 
-	writel(0, priv->regs + CLCD_PL111_IENB);
+	writel(0, priv->regs + priv->ienb);
 }
 
 static int pl111_display_prepare_fb(struct drm_simple_display_pipe *pipe,
@@ -412,22 +412,6 @@ int pl111_display_init(struct drm_device *drm)
 	struct device_node *endpoint;
 	u32 tft_r0b0g0[3];
 	int ret;
-	static const u32 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,
-	};
 
 	endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
 	if (!endpoint)
@@ -456,7 +440,8 @@ int pl111_display_init(struct drm_device *drm)
 
 	ret = drm_simple_display_pipe_init(drm, &priv->pipe,
 					   &pl111_display_funcs,
-					   formats, ARRAY_SIZE(formats),
+					   priv->variant->formats,
+					   priv->variant->nformats,
 					   priv->connector);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h
index 8804af0f8997..b316a8a0fbc0 100644
--- a/drivers/gpu/drm/pl111/pl111_drm.h
+++ b/drivers/gpu/drm/pl111/pl111_drm.h
@@ -31,6 +31,20 @@
 
 struct drm_minor;
 
+/**
+ * struct pl111_variant_data - encodes IP differences
+ * @name: the name of this variant
+ * @is_pl110: this is the early 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;
+	const u32 *formats;
+	unsigned int nformats;
+};
+
 struct pl111_drm_dev_private {
 	struct drm_device *drm;
 
@@ -42,6 +56,8 @@ struct pl111_drm_dev_private {
 	struct drm_fbdev_cma *fbdev;
 
 	void *regs;
+	u32 ienb;
+	u32 ctrl;
 	/* The pixel clock (a reference to our clock divider off of CLCDCLK). */
 	struct clk *clk;
 	/* pl111's internal clock divider. */
@@ -50,6 +66,7 @@ struct pl111_drm_dev_private {
 	 * subsystem and pl111_display_enable().
 	 */
 	spinlock_t tim2_lock;
+	struct pl111_variant_data *variant;
 };
 
 int pl111_display_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index e66cbf202e17..f6863c0fb809 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -206,6 +206,7 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
 {
 	struct device *dev = &amba_dev->dev;
 	struct pl111_drm_dev_private *priv;
+	struct pl111_variant_data *variant = id->data;
 	struct drm_device *drm;
 	int ret;
 
@@ -219,6 +220,33 @@ 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
+	 * swapped: interrupt enable and control. For this reason
+	 * we use offsets that we can change per variant.
+	 */
+	if (variant->is_pl110) {
+		/*
+		 * The ARM Versatile boards are even more special:
+		 * their PrimeCell ID say they are PL110 but the
+		 * control and interrupt enable registers are anyway
+		 * swapped to the PL111 order so they are not following
+		 * the PL110 datasheet.
+		 */
+		if (of_machine_is_compatible("arm,versatile-ab") ||
+		    of_machine_is_compatible("arm,versatile-pb")) {
+			priv->ienb = CLCD_PL111_IENB;
+			priv->ctrl = CLCD_PL111_CNTL;
+		} else {
+			priv->ienb = CLCD_PL110_IENB;
+			priv->ctrl = CLCD_PL110_CNTL;
+		}
+	} else {
+		priv->ienb = CLCD_PL111_IENB;
+		priv->ctrl = CLCD_PL111_CNTL;
+	}
 
 	priv->regs = devm_ioremap_resource(dev, &amba_dev->res);
 	if (IS_ERR(priv->regs)) {
@@ -227,10 +255,10 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
 	}
 
 	/* turn off interrupts before requesting the irq */
-	writel(0, priv->regs + CLCD_PL111_IENB);
+	writel(0, priv->regs + priv->ienb);
 
 	ret = devm_request_irq(dev, amba_dev->irq[0], pl111_irq, 0,
-			       "pl111", priv);
+			       variant->name, priv);
 	if (ret != 0) {
 		dev_err(dev, "%s failed irq %d\n", __func__, ret);
 		return ret;
@@ -269,10 +297,62 @@ 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,
+};
+
+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,
+};
+
+struct pl111_variant_data pl111_variant = {
+	.name = "PL111",
+	.formats = pl111_pixel_formats,
+	.nformats = ARRAY_SIZE(pl111_pixel_formats),
+};
+
 static struct amba_id pl111_id_table[] = {
 	{
+		.id = 0x00041110,
+		.mask = 0x000fffff,
+		.data = &pl110_variant,
+	},
+	{
 		.id = 0x00041111,
 		.mask = 0x000fffff,
+		.data = &pl111_variant,
 	},
 	{0, 0},
 };
-- 
2.13.5

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

^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH 5/7] drm/pl111: Insert delay before powering up PL11x
  2017-08-30 18:07 ` Linus Walleij
@ 2017-08-30 18:07   ` Linus Walleij
  -1 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2017-08-30 18:07 UTC (permalink / raw)
  To: linux-arm-kernel

The old codebase has a delay between enabling and powering up the
PL11x.

According to the manual for PL110, ARM DDI 0161E page 1-5 and
the PL111 manual ARM DDI 0293C page 1-6, the power sequence should
be such that once Vdd is stable (which we assume it is at boot)
LCDEN is enabled first and then CLPOWER should be enabled
"after the signals have stabilized" and this is said to
be display-dependent. The old codebase uses 20ms.

The delay construction in the old code is not to be trusted: it
was likely assuming a certain hard-coded display (such as the
Versatile PROSPECTOR display) and the PL11x blocks are used in
several designs with different displays. Instead rely on the
display driver to provide the proper delay in response to
the drm_panel_prepare() and drm_panel_enable() as well as
the drm_panel_disable() and drm_panel_unprepare() calls,
but make sure to set the LCDEN before these calls and then
CLPOWER after calling them (and the reverse for disabling).

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

diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 6447f36c243a..39106068b158 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -157,8 +157,8 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 
 	drm_panel_prepare(priv->panel);
 
-	/* Enable and Power Up */
-	cntl = CNTL_LCDEN | CNTL_LCDTFT | CNTL_LCDPWR | CNTL_LCDVCOMP(1);
+	/* Hard-code TFT panel */
+	cntl = CNTL_LCDEN | CNTL_LCDTFT | CNTL_LCDVCOMP(1);
 
 	/* Note that the the hardware's format reader takes 'r' from
 	 * the low bit, while DRM formats list channels from high bit
@@ -201,10 +201,19 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 		break;
 	}
 
+	/* Power sequence: first enable and chill */
 	writel(cntl, priv->regs + priv->ctrl);
 
+	/*
+	 * We expect these calls to enable and stabilize the contrast
+	 * voltage Vee as stipulated by the manual
+	 */
 	drm_panel_enable(priv->panel);
 
+	/* Power Up */
+	cntl |= CNTL_LCDPWR;
+	writel(cntl, priv->regs + priv->ctrl);
+
 	drm_crtc_vblank_on(crtc);
 }
 
@@ -213,16 +222,27 @@ void pl111_display_disable(struct drm_simple_display_pipe *pipe)
 	struct drm_crtc *crtc = &pipe->crtc;
 	struct drm_device *drm = crtc->dev;
 	struct pl111_drm_dev_private *priv = drm->dev_private;
+	u32 cntl;
 
 	drm_crtc_vblank_off(crtc);
 
+	/* Power Down */
+	cntl = readl(priv->regs + priv->ctrl);
+	if (cntl & CNTL_LCDPWR) {
+		cntl &= ~CNTL_LCDPWR;
+		writel(cntl, priv->regs + priv->ctrl);
+	}
+
+	/*
+	 * We expect these calls to disable the contrast voltage Vee as
+	 * stipulated by the manual
+	 */
 	drm_panel_disable(priv->panel);
+	drm_panel_unprepare(priv->panel);
 
-	/* Disable and Power Down */
+	/* Disable */
 	writel(0, priv->regs + priv->ctrl);
 
-	drm_panel_unprepare(priv->panel);
-
 	clk_disable_unprepare(priv->clk);
 }
 
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH 5/7] drm/pl111: Insert delay before powering up PL11x
@ 2017-08-30 18:07   ` Linus Walleij
  0 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2017-08-30 18:07 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Sean Paul, Eric Anholt
  Cc: linux-arm-kernel, dri-devel

The old codebase has a delay between enabling and powering up the
PL11x.

According to the manual for PL110, ARM DDI 0161E page 1-5 and
the PL111 manual ARM DDI 0293C page 1-6, the power sequence should
be such that once Vdd is stable (which we assume it is at boot)
LCDEN is enabled first and then CLPOWER should be enabled
"after the signals have stabilized" and this is said to
be display-dependent. The old codebase uses 20ms.

The delay construction in the old code is not to be trusted: it
was likely assuming a certain hard-coded display (such as the
Versatile PROSPECTOR display) and the PL11x blocks are used in
several designs with different displays. Instead rely on the
display driver to provide the proper delay in response to
the drm_panel_prepare() and drm_panel_enable() as well as
the drm_panel_disable() and drm_panel_unprepare() calls,
but make sure to set the LCDEN before these calls and then
CLPOWER after calling them (and the reverse for disabling).

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

diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 6447f36c243a..39106068b158 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -157,8 +157,8 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 
 	drm_panel_prepare(priv->panel);
 
-	/* Enable and Power Up */
-	cntl = CNTL_LCDEN | CNTL_LCDTFT | CNTL_LCDPWR | CNTL_LCDVCOMP(1);
+	/* Hard-code TFT panel */
+	cntl = CNTL_LCDEN | CNTL_LCDTFT | CNTL_LCDVCOMP(1);
 
 	/* Note that the the hardware's format reader takes 'r' from
 	 * the low bit, while DRM formats list channels from high bit
@@ -201,10 +201,19 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 		break;
 	}
 
+	/* Power sequence: first enable and chill */
 	writel(cntl, priv->regs + priv->ctrl);
 
+	/*
+	 * We expect these calls to enable and stabilize the contrast
+	 * voltage Vee as stipulated by the manual
+	 */
 	drm_panel_enable(priv->panel);
 
+	/* Power Up */
+	cntl |= CNTL_LCDPWR;
+	writel(cntl, priv->regs + priv->ctrl);
+
 	drm_crtc_vblank_on(crtc);
 }
 
@@ -213,16 +222,27 @@ void pl111_display_disable(struct drm_simple_display_pipe *pipe)
 	struct drm_crtc *crtc = &pipe->crtc;
 	struct drm_device *drm = crtc->dev;
 	struct pl111_drm_dev_private *priv = drm->dev_private;
+	u32 cntl;
 
 	drm_crtc_vblank_off(crtc);
 
+	/* Power Down */
+	cntl = readl(priv->regs + priv->ctrl);
+	if (cntl & CNTL_LCDPWR) {
+		cntl &= ~CNTL_LCDPWR;
+		writel(cntl, priv->regs + priv->ctrl);
+	}
+
+	/*
+	 * We expect these calls to disable the contrast voltage Vee as
+	 * stipulated by the manual
+	 */
 	drm_panel_disable(priv->panel);
+	drm_panel_unprepare(priv->panel);
 
-	/* Disable and Power Down */
+	/* Disable */
 	writel(0, priv->regs + priv->ctrl);
 
-	drm_panel_unprepare(priv->panel);
-
 	clk_disable_unprepare(priv->clk);
 }
 
-- 
2.13.5

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

^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH 6/7] drm/pl111: Add optional variant display en/disable callbacks
  2017-08-30 18:07 ` Linus Walleij
@ 2017-08-30 18:07   ` Linus Walleij
  -1 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2017-08-30 18:07 UTC (permalink / raw)
  To: linux-arm-kernel

The silcon and components around the PL111 may require some
variants to perform special set-up of the display. Add two
callbacks to manage this.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/pl111/pl111_display.c | 7 +++++++
 drivers/gpu/drm/pl111/pl111_drm.h     | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 39106068b158..37f409867934 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -208,6 +208,9 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 	 * We expect these calls to enable and stabilize the contrast
 	 * voltage Vee as stipulated by the manual
 	 */
+	if (priv->variant_display_enable)
+		priv->variant_display_enable(drm, fb->format->format);
+
 	drm_panel_enable(priv->panel);
 
 	/* Power Up */
@@ -238,6 +241,10 @@ void pl111_display_disable(struct drm_simple_display_pipe *pipe)
 	 * stipulated by the manual
 	 */
 	drm_panel_disable(priv->panel);
+
+	if (priv->variant_display_disable)
+		priv->variant_display_disable(drm);
+
 	drm_panel_unprepare(priv->panel);
 
 	/* Disable */
diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h
index b316a8a0fbc0..a5368d36e9f5 100644
--- a/drivers/gpu/drm/pl111/pl111_drm.h
+++ b/drivers/gpu/drm/pl111/pl111_drm.h
@@ -26,6 +26,7 @@
 #include <drm/drm_panel.h>
 #include <drm/drm_bridge.h>
 #include <linux/clk-provider.h>
+#include <linux/interrupt.h>
 
 #define CLCD_IRQ_NEXTBASE_UPDATE BIT(2)
 
@@ -67,6 +68,8 @@ struct pl111_drm_dev_private {
 	 */
 	spinlock_t tim2_lock;
 	struct pl111_variant_data *variant;
+	void (*variant_display_enable) (struct drm_device *drm, u32 format);
+	void (*variant_display_disable) (struct drm_device *drm);
 };
 
 int pl111_display_init(struct drm_device *dev);
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH 6/7] drm/pl111: Add optional variant display en/disable callbacks
@ 2017-08-30 18:07   ` Linus Walleij
  0 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2017-08-30 18:07 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Sean Paul, Eric Anholt
  Cc: linux-arm-kernel, dri-devel

The silcon and components around the PL111 may require some
variants to perform special set-up of the display. Add two
callbacks to manage this.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/pl111/pl111_display.c | 7 +++++++
 drivers/gpu/drm/pl111/pl111_drm.h     | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 39106068b158..37f409867934 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -208,6 +208,9 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 	 * We expect these calls to enable and stabilize the contrast
 	 * voltage Vee as stipulated by the manual
 	 */
+	if (priv->variant_display_enable)
+		priv->variant_display_enable(drm, fb->format->format);
+
 	drm_panel_enable(priv->panel);
 
 	/* Power Up */
@@ -238,6 +241,10 @@ void pl111_display_disable(struct drm_simple_display_pipe *pipe)
 	 * stipulated by the manual
 	 */
 	drm_panel_disable(priv->panel);
+
+	if (priv->variant_display_disable)
+		priv->variant_display_disable(drm);
+
 	drm_panel_unprepare(priv->panel);
 
 	/* Disable */
diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h
index b316a8a0fbc0..a5368d36e9f5 100644
--- a/drivers/gpu/drm/pl111/pl111_drm.h
+++ b/drivers/gpu/drm/pl111/pl111_drm.h
@@ -26,6 +26,7 @@
 #include <drm/drm_panel.h>
 #include <drm/drm_bridge.h>
 #include <linux/clk-provider.h>
+#include <linux/interrupt.h>
 
 #define CLCD_IRQ_NEXTBASE_UPDATE BIT(2)
 
@@ -67,6 +68,8 @@ struct pl111_drm_dev_private {
 	 */
 	spinlock_t tim2_lock;
 	struct pl111_variant_data *variant;
+	void (*variant_display_enable) (struct drm_device *drm, u32 format);
+	void (*variant_display_disable) (struct drm_device *drm);
 };
 
 int pl111_display_init(struct drm_device *dev);
-- 
2.13.5

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

^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH 7/7] drm/pl111: Add handling of Versatile platforms
  2017-08-30 18:07 ` Linus Walleij
@ 2017-08-30 18:07   ` Linus Walleij
  -1 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2017-08-30 18:07 UTC (permalink / raw)
  To: linux-arm-kernel

The ARM reference designs in the Versatile family: Integrator,
Versatile and RealView can make use of the new DRM driver as well.
We just need to create a bit of platform-specific code for them
that we isolate to its own file.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/pl111/Makefile          |   1 +
 drivers/gpu/drm/pl111/pl111_display.c   |   7 -
 drivers/gpu/drm/pl111/pl111_drv.c       |   5 +
 drivers/gpu/drm/pl111/pl111_versatile.c | 270 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/pl111/pl111_versatile.h |   9 ++
 5 files changed, 285 insertions(+), 7 deletions(-)
 create mode 100644 drivers/gpu/drm/pl111/pl111_versatile.c
 create mode 100644 drivers/gpu/drm/pl111/pl111_versatile.h

diff --git a/drivers/gpu/drm/pl111/Makefile b/drivers/gpu/drm/pl111/Makefile
index c5f8f9684848..fce1453a93e1 100644
--- a/drivers/gpu/drm/pl111/Makefile
+++ b/drivers/gpu/drm/pl111/Makefile
@@ -1,4 +1,5 @@
 pl111_drm-y +=	pl111_display.o \
+		pl111_versatile.o \
 		pl111_drv.o
 
 pl111_drm-$(CONFIG_DEBUG_FS) += pl111_debugfs.o
diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 37f409867934..f7b043f4fed6 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -454,13 +454,6 @@ int pl111_display_init(struct drm_device *drm)
 	}
 	of_node_put(endpoint);
 
-	if (tft_r0b0g0[0] != 0 ||
-	    tft_r0b0g0[1] != 8 ||
-	    tft_r0b0g0[2] != 16) {
-		dev_err(dev, "arm,pl11x,tft-r0g0b0-pads != [0,8,16] not yet supported\n");
-		return -EINVAL;
-	}
-
 	ret = pl111_init_clock_divider(drm);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index f6863c0fb809..b1759004269c 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -72,6 +72,7 @@
 #include <drm/drm_panel.h>
 
 #include "pl111_drm.h"
+#include "pl111_versatile.h"
 
 #define DRIVER_DESC      "DRM module for PL111"
 
@@ -264,6 +265,10 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
 		return ret;
 	}
 
+	ret = pl111_versatile_init(dev, priv);
+	if (ret)
+		goto dev_unref;
+
 	ret = pl111_modeset_init(drm);
 	if (ret != 0)
 		goto dev_unref;
diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c
new file mode 100644
index 000000000000..97d4af6925a3
--- /dev/null
+++ b/drivers/gpu/drm/pl111/pl111_versatile.c
@@ -0,0 +1,270 @@
+#include <linux/device.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+#include <linux/bitops.h>
+#include <linux/module.h>
+#include <drm/drmP.h>
+#include "pl111_versatile.h"
+#include "pl111_drm.h"
+
+static struct regmap *versatile_syscon_map;
+
+/*
+ * We detect the different syscon types from the compatible strings.
+ */
+enum versatile_clcd {
+	INTEGRATOR_CLCD_CM,
+	VERSATILE_CLCD,
+	REALVIEW_CLCD_EB,
+	REALVIEW_CLCD_PB1176,
+	REALVIEW_CLCD_PB11MP,
+	REALVIEW_CLCD_PBA8,
+	REALVIEW_CLCD_PBX,
+};
+
+static const struct of_device_id versatile_clcd_of_match[] = {
+	{
+		.compatible = "arm,core-module-integrator",
+		.data = (void *)INTEGRATOR_CLCD_CM,
+	},
+	{
+		.compatible = "arm,versatile-sysreg",
+		.data = (void *)VERSATILE_CLCD,
+	},
+	{
+		.compatible = "arm,realview-eb-syscon",
+		.data = (void *)REALVIEW_CLCD_EB,
+	},
+	{
+		.compatible = "arm,realview-pb1176-syscon",
+		.data = (void *)REALVIEW_CLCD_PB1176,
+	},
+	{
+		.compatible = "arm,realview-pb11mp-syscon",
+		.data = (void *)REALVIEW_CLCD_PB11MP,
+	},
+	{
+		.compatible = "arm,realview-pba8-syscon",
+		.data = (void *)REALVIEW_CLCD_PBA8,
+	},
+	{
+		.compatible = "arm,realview-pbx-syscon",
+		.data = (void *)REALVIEW_CLCD_PBX,
+	},
+	{},
+};
+
+/*
+ * Core module CLCD control on the Integrator/CP, bits
+ * 8 thru 19 of the CM_CONTROL register controls a bunch
+ * of CLCD settings.
+ */
+#define INTEGRATOR_HDR_CTRL_OFFSET	0x0C
+#define INTEGRATOR_CLCD_LCDBIASEN	BIT(8)
+#define INTEGRATOR_CLCD_LCDBIASUP	BIT(9)
+#define INTEGRATOR_CLCD_LCDBIASDN	BIT(10)
+/* Bits 11,12,13 controls the LCD type */
+#define INTEGRATOR_CLCD_LCDMUX_MASK	(BIT(11)|BIT(12)|BIT(13))
+#define INTEGRATOR_CLCD_LCDMUX_LCD24	BIT(11)
+#define INTEGRATOR_CLCD_LCDMUX_VGA565	BIT(12)
+#define INTEGRATOR_CLCD_LCDMUX_SHARP	(BIT(11)|BIT(12))
+#define INTEGRATOR_CLCD_LCDMUX_VGA555	BIT(13)
+#define INTEGRATOR_CLCD_LCDMUX_VGA24	(BIT(11)|BIT(12)|BIT(13))
+#define INTEGRATOR_CLCD_LCD0_EN		BIT(14)
+#define INTEGRATOR_CLCD_LCD1_EN		BIT(15)
+/* R/L flip on Sharp */
+#define INTEGRATOR_CLCD_LCD_STATIC1	BIT(16)
+/* U/D flip on Sharp */
+#define INTEGRATOR_CLCD_LCD_STATIC2	BIT(17)
+/* No connection on Sharp */
+#define INTEGRATOR_CLCD_LCD_STATIC	BIT(18)
+/* 0 = 24bit VGA, 1 = 18bit VGA */
+#define INTEGRATOR_CLCD_LCD_N24BITEN	BIT(19)
+
+#define INTEGRATOR_CLCD_MASK		(INTEGRATOR_CLCD_LCDBIASEN | \
+					 INTEGRATOR_CLCD_LCDBIASUP | \
+					 INTEGRATOR_CLCD_LCDBIASDN | \
+					 INTEGRATOR_CLCD_LCDMUX_MASK | \
+					 INTEGRATOR_CLCD_LCD0_EN | \
+					 INTEGRATOR_CLCD_LCD1_EN | \
+					 INTEGRATOR_CLCD_LCD_STATIC1 | \
+					 INTEGRATOR_CLCD_LCD_STATIC2 | \
+					 INTEGRATOR_CLCD_LCD_STATIC | \
+					 INTEGRATOR_CLCD_LCD_N24BITEN)
+
+static void pl111_integrator_enable(struct drm_device *drm, u32 format)
+{
+	u32 val;
+
+	dev_info(drm->dev, "enable Integrator CLCD connectors\n");
+
+	/* FIXME: really needed? */
+	val = INTEGRATOR_CLCD_LCD_STATIC1 | INTEGRATOR_CLCD_LCD_STATIC2 |
+		INTEGRATOR_CLCD_LCD0_EN | INTEGRATOR_CLCD_LCD1_EN;
+
+	switch (format) {
+	case DRM_FORMAT_XBGR8888:
+	case DRM_FORMAT_XRGB8888:
+		break;
+	case DRM_FORMAT_BGR565:
+	case DRM_FORMAT_RGB565:
+		/* truecolor RGB565 */
+		val |= INTEGRATOR_CLCD_LCDMUX_VGA565;
+		break;
+	case DRM_FORMAT_XBGR1555:
+	case DRM_FORMAT_XRGB1555:
+		/* Pseudocolor, RGB555, BGR555 */
+		val |= INTEGRATOR_CLCD_LCDMUX_VGA555;
+		break;
+	default:
+		dev_err(drm->dev, "unhandled format on Integrator 0x%08x\n",
+			format);
+		break;
+	}
+
+	regmap_update_bits(versatile_syscon_map,
+			   INTEGRATOR_HDR_CTRL_OFFSET,
+			   INTEGRATOR_CLCD_MASK,
+			   val);
+}
+
+/*
+ * This configuration register in the Versatile and RealView
+ * family is uniformly present but appears more and more
+ * unutilized starting with the RealView series.
+ */
+#define SYS_CLCD			0x50
+#define SYS_CLCD_MODE_MASK		(BIT(0)|BIT(1))
+#define SYS_CLCD_MODE_888		0
+#define SYS_CLCD_MODE_5551		BIT(0)
+#define SYS_CLCD_MODE_565_R_LSB		BIT(1)
+#define SYS_CLCD_MODE_565_B_LSB		(BIT(0)|BIT(1))
+#define SYS_CLCD_CONNECTOR_MASK		(BIT(2)|BIT(3)|BIT(4)|BIT(5))
+#define SYS_CLCD_NLCDIOON		BIT(2)
+#define SYS_CLCD_VDDPOSSWITCH		BIT(3)
+#define SYS_CLCD_PWR3V5SWITCH		BIT(4)
+#define SYS_CLCD_VDDNEGSWITCH		BIT(5)
+
+static void pl111_versatile_disable(struct drm_device *drm)
+{
+	dev_info(drm->dev, "disable Versatile CLCD connectors\n");
+	regmap_update_bits(versatile_syscon_map,
+			   SYS_CLCD,
+			   SYS_CLCD_CONNECTOR_MASK,
+			   0);
+}
+
+static void pl111_versatile_enable(struct drm_device *drm, u32 format)
+{
+	u32 val = 0;
+
+	dev_info(drm->dev, "enable Versatile CLCD connectors\n");
+
+	switch (format) {
+	case DRM_FORMAT_ABGR8888:
+	case DRM_FORMAT_XBGR8888:
+	case DRM_FORMAT_ARGB8888:
+	case DRM_FORMAT_XRGB8888:
+		val |= SYS_CLCD_MODE_888;
+		break;
+	case DRM_FORMAT_BGR565:
+		val |= SYS_CLCD_MODE_565_R_LSB;
+		break;
+	case DRM_FORMAT_RGB565:
+		val |= SYS_CLCD_MODE_565_B_LSB;
+		break;
+	case DRM_FORMAT_ABGR1555:
+	case DRM_FORMAT_XBGR1555:
+	case DRM_FORMAT_ARGB1555:
+	case DRM_FORMAT_XRGB1555:
+		val |= SYS_CLCD_MODE_5551;
+		break;
+	default:
+		dev_err(drm->dev, "unhandled format on Versatile 0x%08x\n",
+			format);
+		break;
+	}
+
+	/* Set up the MUX */
+	regmap_update_bits(versatile_syscon_map,
+			   SYS_CLCD,
+			   SYS_CLCD_MODE_MASK,
+			   val);
+
+	/* Then enable the display */
+	regmap_update_bits(versatile_syscon_map,
+			   SYS_CLCD,
+			   SYS_CLCD_CONNECTOR_MASK,
+			   SYS_CLCD_NLCDIOON | SYS_CLCD_PWR3V5SWITCH);
+}
+
+static void pl111_realview_clcd_disable(struct drm_device *drm)
+{
+	dev_info(drm->dev, "disable RealView CLCD connectors\n");
+	regmap_update_bits(versatile_syscon_map,
+			   SYS_CLCD,
+			   SYS_CLCD_CONNECTOR_MASK,
+			   0);
+}
+
+static void pl111_realview_clcd_enable(struct drm_device *drm, u32 format)
+{
+	dev_info(drm->dev, "enable RealView CLCD connectors\n");
+	regmap_update_bits(versatile_syscon_map,
+			   SYS_CLCD,
+			   SYS_CLCD_CONNECTOR_MASK,
+			   SYS_CLCD_NLCDIOON | SYS_CLCD_PWR3V5SWITCH);
+}
+
+int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
+{
+	const struct of_device_id *clcd_id;
+	enum versatile_clcd versatile_clcd_type;
+	struct device_node *np;
+	struct regmap *map;
+
+	np = of_find_matching_node_and_match(NULL, versatile_clcd_of_match,
+					     &clcd_id);
+	if (!np) {
+		/* Non-ARM reference designs, just bail out */
+		return 0;
+	}
+	versatile_clcd_type = (enum versatile_clcd)clcd_id->data;
+
+	map = syscon_node_to_regmap(np);
+	if (IS_ERR(map)) {
+		dev_err(dev, "no Versatile syscon regmap\n");
+		return PTR_ERR(map);
+	}
+
+	switch (versatile_clcd_type) {
+	case INTEGRATOR_CLCD_CM:
+		versatile_syscon_map = map;
+		priv->variant_display_enable = pl111_integrator_enable;
+		dev_info(dev, "set up callbacks for Integrator PL110\n");
+		break;
+	case VERSATILE_CLCD:
+		versatile_syscon_map = map;
+		priv->variant_display_enable = pl111_versatile_enable;
+		priv->variant_display_disable = pl111_versatile_disable;
+		dev_info(dev, "set up callbacks for Versatile PL110+\n");
+		break;
+	case REALVIEW_CLCD_EB:
+	case REALVIEW_CLCD_PB1176:
+	case REALVIEW_CLCD_PB11MP:
+	case REALVIEW_CLCD_PBA8:
+	case REALVIEW_CLCD_PBX:
+		versatile_syscon_map = map;
+		priv->variant_display_enable = pl111_realview_clcd_enable;
+		priv->variant_display_disable = pl111_realview_clcd_disable;
+		dev_info(dev, "set up callbacks for RealView PL111\n");
+		break;
+	default:
+		dev_info(dev, "unknown Versatile system controller\n");
+		break;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pl111_versatile_init);
diff --git a/drivers/gpu/drm/pl111/pl111_versatile.h b/drivers/gpu/drm/pl111/pl111_versatile.h
new file mode 100644
index 000000000000..41aa6d969dc6
--- /dev/null
+++ b/drivers/gpu/drm/pl111/pl111_versatile.h
@@ -0,0 +1,9 @@
+#include <linux/device.h>
+#include "pl111_drm.h"
+
+#ifndef PL111_VERSATILE_H
+#define PL111_VERSATILE_H
+
+int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv);
+
+#endif
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH 7/7] drm/pl111: Add handling of Versatile platforms
@ 2017-08-30 18:07   ` Linus Walleij
  0 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2017-08-30 18:07 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Sean Paul, Eric Anholt
  Cc: linux-arm-kernel, dri-devel

The ARM reference designs in the Versatile family: Integrator,
Versatile and RealView can make use of the new DRM driver as well.
We just need to create a bit of platform-specific code for them
that we isolate to its own file.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/pl111/Makefile          |   1 +
 drivers/gpu/drm/pl111/pl111_display.c   |   7 -
 drivers/gpu/drm/pl111/pl111_drv.c       |   5 +
 drivers/gpu/drm/pl111/pl111_versatile.c | 270 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/pl111/pl111_versatile.h |   9 ++
 5 files changed, 285 insertions(+), 7 deletions(-)
 create mode 100644 drivers/gpu/drm/pl111/pl111_versatile.c
 create mode 100644 drivers/gpu/drm/pl111/pl111_versatile.h

diff --git a/drivers/gpu/drm/pl111/Makefile b/drivers/gpu/drm/pl111/Makefile
index c5f8f9684848..fce1453a93e1 100644
--- a/drivers/gpu/drm/pl111/Makefile
+++ b/drivers/gpu/drm/pl111/Makefile
@@ -1,4 +1,5 @@
 pl111_drm-y +=	pl111_display.o \
+		pl111_versatile.o \
 		pl111_drv.o
 
 pl111_drm-$(CONFIG_DEBUG_FS) += pl111_debugfs.o
diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 37f409867934..f7b043f4fed6 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -454,13 +454,6 @@ int pl111_display_init(struct drm_device *drm)
 	}
 	of_node_put(endpoint);
 
-	if (tft_r0b0g0[0] != 0 ||
-	    tft_r0b0g0[1] != 8 ||
-	    tft_r0b0g0[2] != 16) {
-		dev_err(dev, "arm,pl11x,tft-r0g0b0-pads != [0,8,16] not yet supported\n");
-		return -EINVAL;
-	}
-
 	ret = pl111_init_clock_divider(drm);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index f6863c0fb809..b1759004269c 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -72,6 +72,7 @@
 #include <drm/drm_panel.h>
 
 #include "pl111_drm.h"
+#include "pl111_versatile.h"
 
 #define DRIVER_DESC      "DRM module for PL111"
 
@@ -264,6 +265,10 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
 		return ret;
 	}
 
+	ret = pl111_versatile_init(dev, priv);
+	if (ret)
+		goto dev_unref;
+
 	ret = pl111_modeset_init(drm);
 	if (ret != 0)
 		goto dev_unref;
diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c
new file mode 100644
index 000000000000..97d4af6925a3
--- /dev/null
+++ b/drivers/gpu/drm/pl111/pl111_versatile.c
@@ -0,0 +1,270 @@
+#include <linux/device.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+#include <linux/bitops.h>
+#include <linux/module.h>
+#include <drm/drmP.h>
+#include "pl111_versatile.h"
+#include "pl111_drm.h"
+
+static struct regmap *versatile_syscon_map;
+
+/*
+ * We detect the different syscon types from the compatible strings.
+ */
+enum versatile_clcd {
+	INTEGRATOR_CLCD_CM,
+	VERSATILE_CLCD,
+	REALVIEW_CLCD_EB,
+	REALVIEW_CLCD_PB1176,
+	REALVIEW_CLCD_PB11MP,
+	REALVIEW_CLCD_PBA8,
+	REALVIEW_CLCD_PBX,
+};
+
+static const struct of_device_id versatile_clcd_of_match[] = {
+	{
+		.compatible = "arm,core-module-integrator",
+		.data = (void *)INTEGRATOR_CLCD_CM,
+	},
+	{
+		.compatible = "arm,versatile-sysreg",
+		.data = (void *)VERSATILE_CLCD,
+	},
+	{
+		.compatible = "arm,realview-eb-syscon",
+		.data = (void *)REALVIEW_CLCD_EB,
+	},
+	{
+		.compatible = "arm,realview-pb1176-syscon",
+		.data = (void *)REALVIEW_CLCD_PB1176,
+	},
+	{
+		.compatible = "arm,realview-pb11mp-syscon",
+		.data = (void *)REALVIEW_CLCD_PB11MP,
+	},
+	{
+		.compatible = "arm,realview-pba8-syscon",
+		.data = (void *)REALVIEW_CLCD_PBA8,
+	},
+	{
+		.compatible = "arm,realview-pbx-syscon",
+		.data = (void *)REALVIEW_CLCD_PBX,
+	},
+	{},
+};
+
+/*
+ * Core module CLCD control on the Integrator/CP, bits
+ * 8 thru 19 of the CM_CONTROL register controls a bunch
+ * of CLCD settings.
+ */
+#define INTEGRATOR_HDR_CTRL_OFFSET	0x0C
+#define INTEGRATOR_CLCD_LCDBIASEN	BIT(8)
+#define INTEGRATOR_CLCD_LCDBIASUP	BIT(9)
+#define INTEGRATOR_CLCD_LCDBIASDN	BIT(10)
+/* Bits 11,12,13 controls the LCD type */
+#define INTEGRATOR_CLCD_LCDMUX_MASK	(BIT(11)|BIT(12)|BIT(13))
+#define INTEGRATOR_CLCD_LCDMUX_LCD24	BIT(11)
+#define INTEGRATOR_CLCD_LCDMUX_VGA565	BIT(12)
+#define INTEGRATOR_CLCD_LCDMUX_SHARP	(BIT(11)|BIT(12))
+#define INTEGRATOR_CLCD_LCDMUX_VGA555	BIT(13)
+#define INTEGRATOR_CLCD_LCDMUX_VGA24	(BIT(11)|BIT(12)|BIT(13))
+#define INTEGRATOR_CLCD_LCD0_EN		BIT(14)
+#define INTEGRATOR_CLCD_LCD1_EN		BIT(15)
+/* R/L flip on Sharp */
+#define INTEGRATOR_CLCD_LCD_STATIC1	BIT(16)
+/* U/D flip on Sharp */
+#define INTEGRATOR_CLCD_LCD_STATIC2	BIT(17)
+/* No connection on Sharp */
+#define INTEGRATOR_CLCD_LCD_STATIC	BIT(18)
+/* 0 = 24bit VGA, 1 = 18bit VGA */
+#define INTEGRATOR_CLCD_LCD_N24BITEN	BIT(19)
+
+#define INTEGRATOR_CLCD_MASK		(INTEGRATOR_CLCD_LCDBIASEN | \
+					 INTEGRATOR_CLCD_LCDBIASUP | \
+					 INTEGRATOR_CLCD_LCDBIASDN | \
+					 INTEGRATOR_CLCD_LCDMUX_MASK | \
+					 INTEGRATOR_CLCD_LCD0_EN | \
+					 INTEGRATOR_CLCD_LCD1_EN | \
+					 INTEGRATOR_CLCD_LCD_STATIC1 | \
+					 INTEGRATOR_CLCD_LCD_STATIC2 | \
+					 INTEGRATOR_CLCD_LCD_STATIC | \
+					 INTEGRATOR_CLCD_LCD_N24BITEN)
+
+static void pl111_integrator_enable(struct drm_device *drm, u32 format)
+{
+	u32 val;
+
+	dev_info(drm->dev, "enable Integrator CLCD connectors\n");
+
+	/* FIXME: really needed? */
+	val = INTEGRATOR_CLCD_LCD_STATIC1 | INTEGRATOR_CLCD_LCD_STATIC2 |
+		INTEGRATOR_CLCD_LCD0_EN | INTEGRATOR_CLCD_LCD1_EN;
+
+	switch (format) {
+	case DRM_FORMAT_XBGR8888:
+	case DRM_FORMAT_XRGB8888:
+		break;
+	case DRM_FORMAT_BGR565:
+	case DRM_FORMAT_RGB565:
+		/* truecolor RGB565 */
+		val |= INTEGRATOR_CLCD_LCDMUX_VGA565;
+		break;
+	case DRM_FORMAT_XBGR1555:
+	case DRM_FORMAT_XRGB1555:
+		/* Pseudocolor, RGB555, BGR555 */
+		val |= INTEGRATOR_CLCD_LCDMUX_VGA555;
+		break;
+	default:
+		dev_err(drm->dev, "unhandled format on Integrator 0x%08x\n",
+			format);
+		break;
+	}
+
+	regmap_update_bits(versatile_syscon_map,
+			   INTEGRATOR_HDR_CTRL_OFFSET,
+			   INTEGRATOR_CLCD_MASK,
+			   val);
+}
+
+/*
+ * This configuration register in the Versatile and RealView
+ * family is uniformly present but appears more and more
+ * unutilized starting with the RealView series.
+ */
+#define SYS_CLCD			0x50
+#define SYS_CLCD_MODE_MASK		(BIT(0)|BIT(1))
+#define SYS_CLCD_MODE_888		0
+#define SYS_CLCD_MODE_5551		BIT(0)
+#define SYS_CLCD_MODE_565_R_LSB		BIT(1)
+#define SYS_CLCD_MODE_565_B_LSB		(BIT(0)|BIT(1))
+#define SYS_CLCD_CONNECTOR_MASK		(BIT(2)|BIT(3)|BIT(4)|BIT(5))
+#define SYS_CLCD_NLCDIOON		BIT(2)
+#define SYS_CLCD_VDDPOSSWITCH		BIT(3)
+#define SYS_CLCD_PWR3V5SWITCH		BIT(4)
+#define SYS_CLCD_VDDNEGSWITCH		BIT(5)
+
+static void pl111_versatile_disable(struct drm_device *drm)
+{
+	dev_info(drm->dev, "disable Versatile CLCD connectors\n");
+	regmap_update_bits(versatile_syscon_map,
+			   SYS_CLCD,
+			   SYS_CLCD_CONNECTOR_MASK,
+			   0);
+}
+
+static void pl111_versatile_enable(struct drm_device *drm, u32 format)
+{
+	u32 val = 0;
+
+	dev_info(drm->dev, "enable Versatile CLCD connectors\n");
+
+	switch (format) {
+	case DRM_FORMAT_ABGR8888:
+	case DRM_FORMAT_XBGR8888:
+	case DRM_FORMAT_ARGB8888:
+	case DRM_FORMAT_XRGB8888:
+		val |= SYS_CLCD_MODE_888;
+		break;
+	case DRM_FORMAT_BGR565:
+		val |= SYS_CLCD_MODE_565_R_LSB;
+		break;
+	case DRM_FORMAT_RGB565:
+		val |= SYS_CLCD_MODE_565_B_LSB;
+		break;
+	case DRM_FORMAT_ABGR1555:
+	case DRM_FORMAT_XBGR1555:
+	case DRM_FORMAT_ARGB1555:
+	case DRM_FORMAT_XRGB1555:
+		val |= SYS_CLCD_MODE_5551;
+		break;
+	default:
+		dev_err(drm->dev, "unhandled format on Versatile 0x%08x\n",
+			format);
+		break;
+	}
+
+	/* Set up the MUX */
+	regmap_update_bits(versatile_syscon_map,
+			   SYS_CLCD,
+			   SYS_CLCD_MODE_MASK,
+			   val);
+
+	/* Then enable the display */
+	regmap_update_bits(versatile_syscon_map,
+			   SYS_CLCD,
+			   SYS_CLCD_CONNECTOR_MASK,
+			   SYS_CLCD_NLCDIOON | SYS_CLCD_PWR3V5SWITCH);
+}
+
+static void pl111_realview_clcd_disable(struct drm_device *drm)
+{
+	dev_info(drm->dev, "disable RealView CLCD connectors\n");
+	regmap_update_bits(versatile_syscon_map,
+			   SYS_CLCD,
+			   SYS_CLCD_CONNECTOR_MASK,
+			   0);
+}
+
+static void pl111_realview_clcd_enable(struct drm_device *drm, u32 format)
+{
+	dev_info(drm->dev, "enable RealView CLCD connectors\n");
+	regmap_update_bits(versatile_syscon_map,
+			   SYS_CLCD,
+			   SYS_CLCD_CONNECTOR_MASK,
+			   SYS_CLCD_NLCDIOON | SYS_CLCD_PWR3V5SWITCH);
+}
+
+int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
+{
+	const struct of_device_id *clcd_id;
+	enum versatile_clcd versatile_clcd_type;
+	struct device_node *np;
+	struct regmap *map;
+
+	np = of_find_matching_node_and_match(NULL, versatile_clcd_of_match,
+					     &clcd_id);
+	if (!np) {
+		/* Non-ARM reference designs, just bail out */
+		return 0;
+	}
+	versatile_clcd_type = (enum versatile_clcd)clcd_id->data;
+
+	map = syscon_node_to_regmap(np);
+	if (IS_ERR(map)) {
+		dev_err(dev, "no Versatile syscon regmap\n");
+		return PTR_ERR(map);
+	}
+
+	switch (versatile_clcd_type) {
+	case INTEGRATOR_CLCD_CM:
+		versatile_syscon_map = map;
+		priv->variant_display_enable = pl111_integrator_enable;
+		dev_info(dev, "set up callbacks for Integrator PL110\n");
+		break;
+	case VERSATILE_CLCD:
+		versatile_syscon_map = map;
+		priv->variant_display_enable = pl111_versatile_enable;
+		priv->variant_display_disable = pl111_versatile_disable;
+		dev_info(dev, "set up callbacks for Versatile PL110+\n");
+		break;
+	case REALVIEW_CLCD_EB:
+	case REALVIEW_CLCD_PB1176:
+	case REALVIEW_CLCD_PB11MP:
+	case REALVIEW_CLCD_PBA8:
+	case REALVIEW_CLCD_PBX:
+		versatile_syscon_map = map;
+		priv->variant_display_enable = pl111_realview_clcd_enable;
+		priv->variant_display_disable = pl111_realview_clcd_disable;
+		dev_info(dev, "set up callbacks for RealView PL111\n");
+		break;
+	default:
+		dev_info(dev, "unknown Versatile system controller\n");
+		break;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pl111_versatile_init);
diff --git a/drivers/gpu/drm/pl111/pl111_versatile.h b/drivers/gpu/drm/pl111/pl111_versatile.h
new file mode 100644
index 000000000000..41aa6d969dc6
--- /dev/null
+++ b/drivers/gpu/drm/pl111/pl111_versatile.h
@@ -0,0 +1,9 @@
+#include <linux/device.h>
+#include "pl111_drm.h"
+
+#ifndef PL111_VERSATILE_H
+#define PL111_VERSATILE_H
+
+int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv);
+
+#endif
-- 
2.13.5

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

^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH 4/7] drm/pl111: Enable PL110 variant
  2017-08-30 18:07   ` Linus Walleij
@ 2017-08-31 10:19     ` Eric Engestrom
  -1 siblings, 0 replies; 32+ messages in thread
From: Eric Engestrom @ 2017-08-31 10:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday, 2017-08-30 20:07:08 +0200, Linus Walleij wrote:
> We detect and enable the use of the PL110 variant, an earlier
> incarnation of PL111. The only real difference is that the
> control and interrupt enable registers have swapped place.
> The Versatile AB and Versatile PB have a variant inbetween
> PL110 and PL111, it is PL110 but they have already swapped the
> two registers so those two need a bit of special handling.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/gpu/drm/pl111/pl111_display.c | 27 +++--------
>  drivers/gpu/drm/pl111/pl111_drm.h     | 17 +++++++
>  drivers/gpu/drm/pl111/pl111_drv.c     | 84 ++++++++++++++++++++++++++++++++++-
>  3 files changed, 105 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> index ef86ef60aed1..6447f36c243a 100644
> --- a/drivers/gpu/drm/pl111/pl111_display.c
> +++ b/drivers/gpu/drm/pl111/pl111_display.c
> @@ -201,7 +201,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
>  		break;
>  	}
>  
> -	writel(cntl, priv->regs + CLCD_PL111_CNTL);
> +	writel(cntl, priv->regs + priv->ctrl);
>  
>  	drm_panel_enable(priv->panel);
>  
> @@ -219,7 +219,7 @@ void pl111_display_disable(struct drm_simple_display_pipe *pipe)
>  	drm_panel_disable(priv->panel);
>  
>  	/* Disable and Power Down */
> -	writel(0, priv->regs + CLCD_PL111_CNTL);
> +	writel(0, priv->regs + priv->ctrl);
>  
>  	drm_panel_unprepare(priv->panel);
>  
> @@ -259,7 +259,7 @@ int pl111_enable_vblank(struct drm_device *drm, unsigned int crtc)
>  {
>  	struct pl111_drm_dev_private *priv = drm->dev_private;
>  
> -	writel(CLCD_IRQ_NEXTBASE_UPDATE, priv->regs + CLCD_PL111_IENB);
> +	writel(CLCD_IRQ_NEXTBASE_UPDATE, priv->regs + priv->ienb);
>  
>  	return 0;
>  }
> @@ -268,7 +268,7 @@ void pl111_disable_vblank(struct drm_device *drm, unsigned int crtc)
>  {
>  	struct pl111_drm_dev_private *priv = drm->dev_private;
>  
> -	writel(0, priv->regs + CLCD_PL111_IENB);
> +	writel(0, priv->regs + priv->ienb);
>  }
>  
>  static int pl111_display_prepare_fb(struct drm_simple_display_pipe *pipe,
> @@ -412,22 +412,6 @@ int pl111_display_init(struct drm_device *drm)
>  	struct device_node *endpoint;
>  	u32 tft_r0b0g0[3];
>  	int ret;
> -	static const u32 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,
> -	};
>  
>  	endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
>  	if (!endpoint)
> @@ -456,7 +440,8 @@ int pl111_display_init(struct drm_device *drm)
>  
>  	ret = drm_simple_display_pipe_init(drm, &priv->pipe,
>  					   &pl111_display_funcs,
> -					   formats, ARRAY_SIZE(formats),
> +					   priv->variant->formats,
> +					   priv->variant->nformats,
>  					   priv->connector);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h
> index 8804af0f8997..b316a8a0fbc0 100644
> --- a/drivers/gpu/drm/pl111/pl111_drm.h
> +++ b/drivers/gpu/drm/pl111/pl111_drm.h
> @@ -31,6 +31,20 @@
>  
>  struct drm_minor;
>  
> +/**
> + * struct pl111_variant_data - encodes IP differences
> + * @name: the name of this variant
> + * @is_pl110: this is the early 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;
> +	const u32 *formats;
> +	unsigned int nformats;
> +};
> +
>  struct pl111_drm_dev_private {
>  	struct drm_device *drm;
>  
> @@ -42,6 +56,8 @@ struct pl111_drm_dev_private {
>  	struct drm_fbdev_cma *fbdev;
>  
>  	void *regs;
> +	u32 ienb;
> +	u32 ctrl;
>  	/* The pixel clock (a reference to our clock divider off of CLCDCLK). */
>  	struct clk *clk;
>  	/* pl111's internal clock divider. */
> @@ -50,6 +66,7 @@ struct pl111_drm_dev_private {
>  	 * subsystem and pl111_display_enable().
>  	 */
>  	spinlock_t tim2_lock;
> +	struct pl111_variant_data *variant;
>  };
>  
>  int pl111_display_init(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
> index e66cbf202e17..f6863c0fb809 100644
> --- a/drivers/gpu/drm/pl111/pl111_drv.c
> +++ b/drivers/gpu/drm/pl111/pl111_drv.c
> @@ -206,6 +206,7 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
>  {
>  	struct device *dev = &amba_dev->dev;
>  	struct pl111_drm_dev_private *priv;
> +	struct pl111_variant_data *variant = id->data;
>  	struct drm_device *drm;
>  	int ret;
>  
> @@ -219,6 +220,33 @@ 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
> +	 * swapped: interrupt enable and control. For this reason
> +	 * we use offsets that we can change per variant.
> +	 */
> +	if (variant->is_pl110) {
> +		/*
> +		 * The ARM Versatile boards are even more special:
> +		 * their PrimeCell ID say they are PL110 but the
> +		 * control and interrupt enable registers are anyway
> +		 * swapped to the PL111 order so they are not following
> +		 * the PL110 datasheet.
> +		 */
> +		if (of_machine_is_compatible("arm,versatile-ab") ||
> +		    of_machine_is_compatible("arm,versatile-pb")) {
> +			priv->ienb = CLCD_PL111_IENB;
> +			priv->ctrl = CLCD_PL111_CNTL;
> +		} else {
> +			priv->ienb = CLCD_PL110_IENB;
> +			priv->ctrl = CLCD_PL110_CNTL;
> +		}
> +	} else {
> +		priv->ienb = CLCD_PL111_IENB;
> +		priv->ctrl = CLCD_PL111_CNTL;
> +	}
>  
>  	priv->regs = devm_ioremap_resource(dev, &amba_dev->res);
>  	if (IS_ERR(priv->regs)) {
> @@ -227,10 +255,10 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
>  	}
>  
>  	/* turn off interrupts before requesting the irq */
> -	writel(0, priv->regs + CLCD_PL111_IENB);
> +	writel(0, priv->regs + priv->ienb);
>  
>  	ret = devm_request_irq(dev, amba_dev->irq[0], pl111_irq, 0,
> -			       "pl111", priv);
> +			       variant->name, priv);
>  	if (ret != 0) {
>  		dev_err(dev, "%s failed irq %d\n", __func__, ret);
>  		return ret;
> @@ -269,10 +297,62 @@ 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,
> +};
> +
> +struct pl111_variant_data pl110_variant = {

I think this (and `pl111_variant` below) can be `static const`, right?

> +	.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,
> +};
> +
> +struct pl111_variant_data pl111_variant = {
> +	.name = "PL111",
> +	.formats = pl111_pixel_formats,
> +	.nformats = ARRAY_SIZE(pl111_pixel_formats),
> +};
> +
>  static struct amba_id pl111_id_table[] = {

Not 100% sure on this one, but I think it can also be `const`, as
amba_lookup() and pl111_amba_probe() are the only users I could find and
both take a `const struct amba_id *` (and pl111_amba_probe() doesn't
even use it).

(Just a couple drive-by comments, I don't know enough for an actual
review, sorry...)

>  	{
> +		.id = 0x00041110,
> +		.mask = 0x000fffff,
> +		.data = &pl110_variant,
> +	},
> +	{
>  		.id = 0x00041111,
>  		.mask = 0x000fffff,
> +		.data = &pl111_variant,
>  	},
>  	{0, 0},
>  };
> -- 
> 2.13.5
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH 4/7] drm/pl111: Enable PL110 variant
@ 2017-08-31 10:19     ` Eric Engestrom
  0 siblings, 0 replies; 32+ messages in thread
From: Eric Engestrom @ 2017-08-31 10:19 UTC (permalink / raw)
  To: Linus Walleij; +Cc: dri-devel, Daniel Vetter, linux-arm-kernel

On Wednesday, 2017-08-30 20:07:08 +0200, Linus Walleij wrote:
> We detect and enable the use of the PL110 variant, an earlier
> incarnation of PL111. The only real difference is that the
> control and interrupt enable registers have swapped place.
> The Versatile AB and Versatile PB have a variant inbetween
> PL110 and PL111, it is PL110 but they have already swapped the
> two registers so those two need a bit of special handling.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/gpu/drm/pl111/pl111_display.c | 27 +++--------
>  drivers/gpu/drm/pl111/pl111_drm.h     | 17 +++++++
>  drivers/gpu/drm/pl111/pl111_drv.c     | 84 ++++++++++++++++++++++++++++++++++-
>  3 files changed, 105 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> index ef86ef60aed1..6447f36c243a 100644
> --- a/drivers/gpu/drm/pl111/pl111_display.c
> +++ b/drivers/gpu/drm/pl111/pl111_display.c
> @@ -201,7 +201,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
>  		break;
>  	}
>  
> -	writel(cntl, priv->regs + CLCD_PL111_CNTL);
> +	writel(cntl, priv->regs + priv->ctrl);
>  
>  	drm_panel_enable(priv->panel);
>  
> @@ -219,7 +219,7 @@ void pl111_display_disable(struct drm_simple_display_pipe *pipe)
>  	drm_panel_disable(priv->panel);
>  
>  	/* Disable and Power Down */
> -	writel(0, priv->regs + CLCD_PL111_CNTL);
> +	writel(0, priv->regs + priv->ctrl);
>  
>  	drm_panel_unprepare(priv->panel);
>  
> @@ -259,7 +259,7 @@ int pl111_enable_vblank(struct drm_device *drm, unsigned int crtc)
>  {
>  	struct pl111_drm_dev_private *priv = drm->dev_private;
>  
> -	writel(CLCD_IRQ_NEXTBASE_UPDATE, priv->regs + CLCD_PL111_IENB);
> +	writel(CLCD_IRQ_NEXTBASE_UPDATE, priv->regs + priv->ienb);
>  
>  	return 0;
>  }
> @@ -268,7 +268,7 @@ void pl111_disable_vblank(struct drm_device *drm, unsigned int crtc)
>  {
>  	struct pl111_drm_dev_private *priv = drm->dev_private;
>  
> -	writel(0, priv->regs + CLCD_PL111_IENB);
> +	writel(0, priv->regs + priv->ienb);
>  }
>  
>  static int pl111_display_prepare_fb(struct drm_simple_display_pipe *pipe,
> @@ -412,22 +412,6 @@ int pl111_display_init(struct drm_device *drm)
>  	struct device_node *endpoint;
>  	u32 tft_r0b0g0[3];
>  	int ret;
> -	static const u32 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,
> -	};
>  
>  	endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
>  	if (!endpoint)
> @@ -456,7 +440,8 @@ int pl111_display_init(struct drm_device *drm)
>  
>  	ret = drm_simple_display_pipe_init(drm, &priv->pipe,
>  					   &pl111_display_funcs,
> -					   formats, ARRAY_SIZE(formats),
> +					   priv->variant->formats,
> +					   priv->variant->nformats,
>  					   priv->connector);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h
> index 8804af0f8997..b316a8a0fbc0 100644
> --- a/drivers/gpu/drm/pl111/pl111_drm.h
> +++ b/drivers/gpu/drm/pl111/pl111_drm.h
> @@ -31,6 +31,20 @@
>  
>  struct drm_minor;
>  
> +/**
> + * struct pl111_variant_data - encodes IP differences
> + * @name: the name of this variant
> + * @is_pl110: this is the early 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;
> +	const u32 *formats;
> +	unsigned int nformats;
> +};
> +
>  struct pl111_drm_dev_private {
>  	struct drm_device *drm;
>  
> @@ -42,6 +56,8 @@ struct pl111_drm_dev_private {
>  	struct drm_fbdev_cma *fbdev;
>  
>  	void *regs;
> +	u32 ienb;
> +	u32 ctrl;
>  	/* The pixel clock (a reference to our clock divider off of CLCDCLK). */
>  	struct clk *clk;
>  	/* pl111's internal clock divider. */
> @@ -50,6 +66,7 @@ struct pl111_drm_dev_private {
>  	 * subsystem and pl111_display_enable().
>  	 */
>  	spinlock_t tim2_lock;
> +	struct pl111_variant_data *variant;
>  };
>  
>  int pl111_display_init(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
> index e66cbf202e17..f6863c0fb809 100644
> --- a/drivers/gpu/drm/pl111/pl111_drv.c
> +++ b/drivers/gpu/drm/pl111/pl111_drv.c
> @@ -206,6 +206,7 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
>  {
>  	struct device *dev = &amba_dev->dev;
>  	struct pl111_drm_dev_private *priv;
> +	struct pl111_variant_data *variant = id->data;
>  	struct drm_device *drm;
>  	int ret;
>  
> @@ -219,6 +220,33 @@ 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
> +	 * swapped: interrupt enable and control. For this reason
> +	 * we use offsets that we can change per variant.
> +	 */
> +	if (variant->is_pl110) {
> +		/*
> +		 * The ARM Versatile boards are even more special:
> +		 * their PrimeCell ID say they are PL110 but the
> +		 * control and interrupt enable registers are anyway
> +		 * swapped to the PL111 order so they are not following
> +		 * the PL110 datasheet.
> +		 */
> +		if (of_machine_is_compatible("arm,versatile-ab") ||
> +		    of_machine_is_compatible("arm,versatile-pb")) {
> +			priv->ienb = CLCD_PL111_IENB;
> +			priv->ctrl = CLCD_PL111_CNTL;
> +		} else {
> +			priv->ienb = CLCD_PL110_IENB;
> +			priv->ctrl = CLCD_PL110_CNTL;
> +		}
> +	} else {
> +		priv->ienb = CLCD_PL111_IENB;
> +		priv->ctrl = CLCD_PL111_CNTL;
> +	}
>  
>  	priv->regs = devm_ioremap_resource(dev, &amba_dev->res);
>  	if (IS_ERR(priv->regs)) {
> @@ -227,10 +255,10 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
>  	}
>  
>  	/* turn off interrupts before requesting the irq */
> -	writel(0, priv->regs + CLCD_PL111_IENB);
> +	writel(0, priv->regs + priv->ienb);
>  
>  	ret = devm_request_irq(dev, amba_dev->irq[0], pl111_irq, 0,
> -			       "pl111", priv);
> +			       variant->name, priv);
>  	if (ret != 0) {
>  		dev_err(dev, "%s failed irq %d\n", __func__, ret);
>  		return ret;
> @@ -269,10 +297,62 @@ 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,
> +};
> +
> +struct pl111_variant_data pl110_variant = {

I think this (and `pl111_variant` below) can be `static const`, right?

> +	.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,
> +};
> +
> +struct pl111_variant_data pl111_variant = {
> +	.name = "PL111",
> +	.formats = pl111_pixel_formats,
> +	.nformats = ARRAY_SIZE(pl111_pixel_formats),
> +};
> +
>  static struct amba_id pl111_id_table[] = {

Not 100% sure on this one, but I think it can also be `const`, as
amba_lookup() and pl111_amba_probe() are the only users I could find and
both take a `const struct amba_id *` (and pl111_amba_probe() doesn't
even use it).

(Just a couple drive-by comments, I don't know enough for an actual
review, sorry...)

>  	{
> +		.id = 0x00041110,
> +		.mask = 0x000fffff,
> +		.data = &pl110_variant,
> +	},
> +	{
>  		.id = 0x00041111,
>  		.mask = 0x000fffff,
> +		.data = &pl111_variant,
>  	},
>  	{0, 0},
>  };
> -- 
> 2.13.5
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread
* [PATCH 3/7] drm/pl111: Replace custom connector with panel bridge
  2017-08-30 18:07   ` Linus Walleij
@ 2017-08-31 17:38     ` Eric Anholt
  -1 siblings, 0 replies; 32+ messages in thread
From: Eric Anholt @ 2017-08-31 17:38 UTC (permalink / raw)
  To: linux-arm-kernel

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

> This replaces the custom connector in the PL111 with the
> panel bridge helper.
>
> This works nicely for all standard panels, but since there
> are several PL11x-based systems that will need to use the dumb
> VGA connector bridge we use drm_of_find_panel_or_bridge()
> and make some headroom for dealing with bridges that are
> not panels as well, and drop a TODO in the code.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> index c6ca4f1bbd49..ef86ef60aed1 100644
> --- a/drivers/gpu/drm/pl111/pl111_display.c
> +++ b/drivers/gpu/drm/pl111/pl111_display.c
> @@ -93,7 +93,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
>  	struct pl111_drm_dev_private *priv = drm->dev_private;
>  	const struct drm_display_mode *mode = &cstate->mode;
>  	struct drm_framebuffer *fb = plane->state->fb;
> -	struct drm_connector *connector = &priv->connector.connector;
> +	struct drm_connector *connector = priv->connector;
>  	u32 cntl;
>  	u32 ppl, hsw, hfp, hbp;
>  	u32 lpp, vsw, vfp, vbp;
> @@ -155,7 +155,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
>  
>  	writel(0, priv->regs + CLCD_TIM3);
>  
> -	drm_panel_prepare(priv->connector.panel);
> +	drm_panel_prepare(priv->panel);

If we're moving to panel-bridge, then we should drop our manual panel
prepare/enable/disable calls -- the panel-bridge will have already
panel_prepare()d before our encoder's enable, and will panel_enable()
after we finish.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170831/a301ad01/attachment.sig>

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH 3/7] drm/pl111: Replace custom connector with panel bridge
@ 2017-08-31 17:38     ` Eric Anholt
  0 siblings, 0 replies; 32+ messages in thread
From: Eric Anholt @ 2017-08-31 17:38 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: 1651 bytes --]

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

> This replaces the custom connector in the PL111 with the
> panel bridge helper.
>
> This works nicely for all standard panels, but since there
> are several PL11x-based systems that will need to use the dumb
> VGA connector bridge we use drm_of_find_panel_or_bridge()
> and make some headroom for dealing with bridges that are
> not panels as well, and drop a TODO in the code.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> index c6ca4f1bbd49..ef86ef60aed1 100644
> --- a/drivers/gpu/drm/pl111/pl111_display.c
> +++ b/drivers/gpu/drm/pl111/pl111_display.c
> @@ -93,7 +93,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
>  	struct pl111_drm_dev_private *priv = drm->dev_private;
>  	const struct drm_display_mode *mode = &cstate->mode;
>  	struct drm_framebuffer *fb = plane->state->fb;
> -	struct drm_connector *connector = &priv->connector.connector;
> +	struct drm_connector *connector = priv->connector;
>  	u32 cntl;
>  	u32 ppl, hsw, hfp, hbp;
>  	u32 lpp, vsw, vfp, vbp;
> @@ -155,7 +155,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
>  
>  	writel(0, priv->regs + CLCD_TIM3);
>  
> -	drm_panel_prepare(priv->connector.panel);
> +	drm_panel_prepare(priv->panel);

If we're moving to panel-bridge, then we should drop our manual panel
prepare/enable/disable calls -- the panel-bridge will have already
panel_prepare()d before our encoder's enable, and will panel_enable()
after we finish.

[-- 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] 32+ messages in thread
* [PATCH 2/7] drm/pl111: Add all registers to debugfs
  2017-08-30 18:07   ` Linus Walleij
@ 2017-08-31 17:40     ` Eric Anholt
  -1 siblings, 0 replies; 32+ messages in thread
From: Eric Anholt @ 2017-08-31 17:40 UTC (permalink / raw)
  To: linux-arm-kernel

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

> This adds all the main control registers to the debugfs
> register file. This was helpful for my debugging so it will
> likely help others as well.

This and the previous patch are:

Reviewed-by: Eric Anholt <eric@anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170831/92b3e77e/attachment-0001.sig>

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH 2/7] drm/pl111: Add all registers to debugfs
@ 2017-08-31 17:40     ` Eric Anholt
  0 siblings, 0 replies; 32+ messages in thread
From: Eric Anholt @ 2017-08-31 17:40 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: 278 bytes --]

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

> This adds all the main control registers to the debugfs
> register file. This was helpful for my debugging so it will
> likely help others as well.

This and the previous patch 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] 32+ messages in thread
* [PATCH 4/7] drm/pl111: Enable PL110 variant
  2017-08-30 18:07   ` Linus Walleij
@ 2017-08-31 17:46     ` Eric Anholt
  -1 siblings, 0 replies; 32+ messages in thread
From: Eric Anholt @ 2017-08-31 17:46 UTC (permalink / raw)
  To: linux-arm-kernel

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

> We detect and enable the use of the PL110 variant, an earlier
> incarnation of PL111. The only real difference is that the
> control and interrupt enable registers have swapped place.
> The Versatile AB and Versatile PB have a variant inbetween
> PL110 and PL111, it is PL110 but they have already swapped the
> two registers so those two need a bit of special handling.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

With the other Eric's little const fixes,

Reviewed-by: Eric Anholt <eric@anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170831/2778e092/attachment.sig>

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH 4/7] drm/pl111: Enable PL110 variant
@ 2017-08-31 17:46     ` Eric Anholt
  0 siblings, 0 replies; 32+ messages in thread
From: Eric Anholt @ 2017-08-31 17:46 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: 570 bytes --]

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

> We detect and enable the use of the PL110 variant, an earlier
> incarnation of PL111. The only real difference is that the
> control and interrupt enable registers have swapped place.
> The Versatile AB and Versatile PB have a variant inbetween
> PL110 and PL111, it is PL110 but they have already swapped the
> two registers so those two need a bit of special handling.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

With the other Eric's little const fixes,

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] 32+ messages in thread
* [PATCH 7/7] drm/pl111: Add handling of Versatile platforms
  2017-08-30 18:07   ` Linus Walleij
@ 2017-08-31 17:58     ` Eric Anholt
  -1 siblings, 0 replies; 32+ messages in thread
From: Eric Anholt @ 2017-08-31 17:58 UTC (permalink / raw)
  To: linux-arm-kernel

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

> The ARM reference designs in the Versatile family: Integrator,
> Versatile and RealView can make use of the new DRM driver as well.
> We just need to create a bit of platform-specific code for them
> that we isolate to its own file.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/gpu/drm/pl111/Makefile          |   1 +
>  drivers/gpu/drm/pl111/pl111_display.c   |   7 -
>  drivers/gpu/drm/pl111/pl111_drv.c       |   5 +
>  drivers/gpu/drm/pl111/pl111_versatile.c | 270 ++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/pl111/pl111_versatile.h |   9 ++
>  5 files changed, 285 insertions(+), 7 deletions(-)
>  create mode 100644 drivers/gpu/drm/pl111/pl111_versatile.c
>  create mode 100644 drivers/gpu/drm/pl111/pl111_versatile.h
>
> diff --git a/drivers/gpu/drm/pl111/Makefile b/drivers/gpu/drm/pl111/Makefile
> index c5f8f9684848..fce1453a93e1 100644
> --- a/drivers/gpu/drm/pl111/Makefile
> +++ b/drivers/gpu/drm/pl111/Makefile
> @@ -1,4 +1,5 @@
>  pl111_drm-y +=	pl111_display.o \
> +		pl111_versatile.o \
>  		pl111_drv.o
>  
>  pl111_drm-$(CONFIG_DEBUG_FS) += pl111_debugfs.o
> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> index 37f409867934..f7b043f4fed6 100644
> --- a/drivers/gpu/drm/pl111/pl111_display.c
> +++ b/drivers/gpu/drm/pl111/pl111_display.c
> @@ -454,13 +454,6 @@ int pl111_display_init(struct drm_device *drm)
>  	}
>  	of_node_put(endpoint);
>  
> -	if (tft_r0b0g0[0] != 0 ||
> -	    tft_r0b0g0[1] != 8 ||
> -	    tft_r0b0g0[2] != 16) {
> -		dev_err(dev, "arm,pl11x,tft-r0g0b0-pads != [0,8,16] not yet supported\n");
> -		return -EINVAL;
> -	}

I had a note in the DOC section of pl111_drv.c about needing to filter
available formats based on this property.  Could you update that with
some explanation of the new state of things?  (Do we actually need to
filter the formats?)

I haven't verified that we get the same pin routing setup as the fbdev
driver did for these platforms, but even if they don't match yet this
seems like an excellent start and we can make sure we sort them out as
we add panel drivers.

I've verified that my Cygnus board continues working with your series.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170831/7da51f1b/attachment.sig>

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH 7/7] drm/pl111: Add handling of Versatile platforms
@ 2017-08-31 17:58     ` Eric Anholt
  0 siblings, 0 replies; 32+ messages in thread
From: Eric Anholt @ 2017-08-31 17:58 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: 2294 bytes --]

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

> The ARM reference designs in the Versatile family: Integrator,
> Versatile and RealView can make use of the new DRM driver as well.
> We just need to create a bit of platform-specific code for them
> that we isolate to its own file.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/gpu/drm/pl111/Makefile          |   1 +
>  drivers/gpu/drm/pl111/pl111_display.c   |   7 -
>  drivers/gpu/drm/pl111/pl111_drv.c       |   5 +
>  drivers/gpu/drm/pl111/pl111_versatile.c | 270 ++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/pl111/pl111_versatile.h |   9 ++
>  5 files changed, 285 insertions(+), 7 deletions(-)
>  create mode 100644 drivers/gpu/drm/pl111/pl111_versatile.c
>  create mode 100644 drivers/gpu/drm/pl111/pl111_versatile.h
>
> diff --git a/drivers/gpu/drm/pl111/Makefile b/drivers/gpu/drm/pl111/Makefile
> index c5f8f9684848..fce1453a93e1 100644
> --- a/drivers/gpu/drm/pl111/Makefile
> +++ b/drivers/gpu/drm/pl111/Makefile
> @@ -1,4 +1,5 @@
>  pl111_drm-y +=	pl111_display.o \
> +		pl111_versatile.o \
>  		pl111_drv.o
>  
>  pl111_drm-$(CONFIG_DEBUG_FS) += pl111_debugfs.o
> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> index 37f409867934..f7b043f4fed6 100644
> --- a/drivers/gpu/drm/pl111/pl111_display.c
> +++ b/drivers/gpu/drm/pl111/pl111_display.c
> @@ -454,13 +454,6 @@ int pl111_display_init(struct drm_device *drm)
>  	}
>  	of_node_put(endpoint);
>  
> -	if (tft_r0b0g0[0] != 0 ||
> -	    tft_r0b0g0[1] != 8 ||
> -	    tft_r0b0g0[2] != 16) {
> -		dev_err(dev, "arm,pl11x,tft-r0g0b0-pads != [0,8,16] not yet supported\n");
> -		return -EINVAL;
> -	}

I had a note in the DOC section of pl111_drv.c about needing to filter
available formats based on this property.  Could you update that with
some explanation of the new state of things?  (Do we actually need to
filter the formats?)

I haven't verified that we get the same pin routing setup as the fbdev
driver did for these platforms, but even if they don't match yet this
seems like an excellent start and we can make sure we sort them out as
we add panel drivers.

I've verified that my Cygnus board continues working with your series.

[-- 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] 32+ messages in thread
* [PATCH 4/7] drm/pl111: Enable PL110 variant
  2017-08-31 10:19     ` Eric Engestrom
@ 2017-09-01  8:23       ` Emil Velikov
  -1 siblings, 0 replies; 32+ messages in thread
From: Emil Velikov @ 2017-09-01  8:23 UTC (permalink / raw)
  To: linux-arm-kernel

On 31 August 2017 at 11:19, Eric Engestrom <eric.engestrom@imgtec.com> wrote:
> On Wednesday, 2017-08-30 20:07:08 +0200, Linus Walleij wrote:
>> We detect and enable the use of the PL110 variant, an earlier
>> incarnation of PL111. The only real difference is that the
>> control and interrupt enable registers have swapped place.
>> The Versatile AB and Versatile PB have a variant inbetween
>> PL110 and PL111, it is PL110 but they have already swapped the
>> two registers so those two need a bit of special handling.
>>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>> ---
>>  drivers/gpu/drm/pl111/pl111_display.c | 27 +++--------
>>  drivers/gpu/drm/pl111/pl111_drm.h     | 17 +++++++
>>  drivers/gpu/drm/pl111/pl111_drv.c     | 84 ++++++++++++++++++++++++++++++++++-
>>  3 files changed, 105 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
>> index ef86ef60aed1..6447f36c243a 100644
>> --- a/drivers/gpu/drm/pl111/pl111_display.c
>> +++ b/drivers/gpu/drm/pl111/pl111_display.c
>> @@ -201,7 +201,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
>>               break;
>>       }
>>
>> -     writel(cntl, priv->regs + CLCD_PL111_CNTL);
>> +     writel(cntl, priv->regs + priv->ctrl);
>>
>>       drm_panel_enable(priv->panel);
>>
>> @@ -219,7 +219,7 @@ void pl111_display_disable(struct drm_simple_display_pipe *pipe)
>>       drm_panel_disable(priv->panel);
>>
>>       /* Disable and Power Down */
>> -     writel(0, priv->regs + CLCD_PL111_CNTL);
>> +     writel(0, priv->regs + priv->ctrl);
>>
>>       drm_panel_unprepare(priv->panel);
>>
>> @@ -259,7 +259,7 @@ int pl111_enable_vblank(struct drm_device *drm, unsigned int crtc)
>>  {
>>       struct pl111_drm_dev_private *priv = drm->dev_private;
>>
>> -     writel(CLCD_IRQ_NEXTBASE_UPDATE, priv->regs + CLCD_PL111_IENB);
>> +     writel(CLCD_IRQ_NEXTBASE_UPDATE, priv->regs + priv->ienb);
>>
>>       return 0;
>>  }
>> @@ -268,7 +268,7 @@ void pl111_disable_vblank(struct drm_device *drm, unsigned int crtc)
>>  {
>>       struct pl111_drm_dev_private *priv = drm->dev_private;
>>
>> -     writel(0, priv->regs + CLCD_PL111_IENB);
>> +     writel(0, priv->regs + priv->ienb);
>>  }
>>
>>  static int pl111_display_prepare_fb(struct drm_simple_display_pipe *pipe,
>> @@ -412,22 +412,6 @@ int pl111_display_init(struct drm_device *drm)
>>       struct device_node *endpoint;
>>       u32 tft_r0b0g0[3];
>>       int ret;
>> -     static const u32 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,
>> -     };
>>
>>       endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
>>       if (!endpoint)
>> @@ -456,7 +440,8 @@ int pl111_display_init(struct drm_device *drm)
>>
>>       ret = drm_simple_display_pipe_init(drm, &priv->pipe,
>>                                          &pl111_display_funcs,
>> -                                        formats, ARRAY_SIZE(formats),
>> +                                        priv->variant->formats,
>> +                                        priv->variant->nformats,
>>                                          priv->connector);
>>       if (ret)
>>               return ret;
>> diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h
>> index 8804af0f8997..b316a8a0fbc0 100644
>> --- a/drivers/gpu/drm/pl111/pl111_drm.h
>> +++ b/drivers/gpu/drm/pl111/pl111_drm.h
>> @@ -31,6 +31,20 @@
>>
>>  struct drm_minor;
>>
>> +/**
>> + * struct pl111_variant_data - encodes IP differences
>> + * @name: the name of this variant
>> + * @is_pl110: this is the early 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;
>> +     const u32 *formats;
>> +     unsigned int nformats;
>> +};
>> +
>>  struct pl111_drm_dev_private {
>>       struct drm_device *drm;
>>
>> @@ -42,6 +56,8 @@ struct pl111_drm_dev_private {
>>       struct drm_fbdev_cma *fbdev;
>>
>>       void *regs;
>> +     u32 ienb;
>> +     u32 ctrl;
>>       /* The pixel clock (a reference to our clock divider off of CLCDCLK). */
>>       struct clk *clk;
>>       /* pl111's internal clock divider. */
>> @@ -50,6 +66,7 @@ struct pl111_drm_dev_private {
>>        * subsystem and pl111_display_enable().
>>        */
>>       spinlock_t tim2_lock;
>> +     struct pl111_variant_data *variant;
>>  };
>>
>>  int pl111_display_init(struct drm_device *dev);
>> diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
>> index e66cbf202e17..f6863c0fb809 100644
>> --- a/drivers/gpu/drm/pl111/pl111_drv.c
>> +++ b/drivers/gpu/drm/pl111/pl111_drv.c
>> @@ -206,6 +206,7 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
>>  {
>>       struct device *dev = &amba_dev->dev;
>>       struct pl111_drm_dev_private *priv;
>> +     struct pl111_variant_data *variant = id->data;
>>       struct drm_device *drm;
>>       int ret;
>>
>> @@ -219,6 +220,33 @@ 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
>> +      * swapped: interrupt enable and control. For this reason
>> +      * we use offsets that we can change per variant.
>> +      */
>> +     if (variant->is_pl110) {
>> +             /*
>> +              * The ARM Versatile boards are even more special:
>> +              * their PrimeCell ID say they are PL110 but the
>> +              * control and interrupt enable registers are anyway
>> +              * swapped to the PL111 order so they are not following
>> +              * the PL110 datasheet.
>> +              */
>> +             if (of_machine_is_compatible("arm,versatile-ab") ||
>> +                 of_machine_is_compatible("arm,versatile-pb")) {
>> +                     priv->ienb = CLCD_PL111_IENB;
>> +                     priv->ctrl = CLCD_PL111_CNTL;
>> +             } else {
>> +                     priv->ienb = CLCD_PL110_IENB;
>> +                     priv->ctrl = CLCD_PL110_CNTL;
>> +             }
>> +     } else {
>> +             priv->ienb = CLCD_PL111_IENB;
>> +             priv->ctrl = CLCD_PL111_CNTL;
>> +     }
>>
>>       priv->regs = devm_ioremap_resource(dev, &amba_dev->res);
>>       if (IS_ERR(priv->regs)) {
>> @@ -227,10 +255,10 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
>>       }
>>
>>       /* turn off interrupts before requesting the irq */
>> -     writel(0, priv->regs + CLCD_PL111_IENB);
>> +     writel(0, priv->regs + priv->ienb);
>>
>>       ret = devm_request_irq(dev, amba_dev->irq[0], pl111_irq, 0,
>> -                            "pl111", priv);
>> +                            variant->name, priv);
>>       if (ret != 0) {
>>               dev_err(dev, "%s failed irq %d\n", __func__, ret);
>>               return ret;
>> @@ -269,10 +297,62 @@ 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,
>> +};
>> +
>> +struct pl111_variant_data pl110_variant = {
>
> I think this (and `pl111_variant` below) can be `static const`, right?
>
Static - yes, const - I don't think so.
Struct amba_id::data lacks the constness, so the const qualifier will
get discarded.

In practise everyone considers/uses ::data as const, so that could be
tweaked with separate patch.

-Emil

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH 4/7] drm/pl111: Enable PL110 variant
@ 2017-09-01  8:23       ` Emil Velikov
  0 siblings, 0 replies; 32+ messages in thread
From: Emil Velikov @ 2017-09-01  8:23 UTC (permalink / raw)
  To: Eric Engestrom; +Cc: Daniel Vetter, LAKML, ML dri-devel

On 31 August 2017 at 11:19, Eric Engestrom <eric.engestrom@imgtec.com> wrote:
> On Wednesday, 2017-08-30 20:07:08 +0200, Linus Walleij wrote:
>> We detect and enable the use of the PL110 variant, an earlier
>> incarnation of PL111. The only real difference is that the
>> control and interrupt enable registers have swapped place.
>> The Versatile AB and Versatile PB have a variant inbetween
>> PL110 and PL111, it is PL110 but they have already swapped the
>> two registers so those two need a bit of special handling.
>>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>> ---
>>  drivers/gpu/drm/pl111/pl111_display.c | 27 +++--------
>>  drivers/gpu/drm/pl111/pl111_drm.h     | 17 +++++++
>>  drivers/gpu/drm/pl111/pl111_drv.c     | 84 ++++++++++++++++++++++++++++++++++-
>>  3 files changed, 105 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
>> index ef86ef60aed1..6447f36c243a 100644
>> --- a/drivers/gpu/drm/pl111/pl111_display.c
>> +++ b/drivers/gpu/drm/pl111/pl111_display.c
>> @@ -201,7 +201,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
>>               break;
>>       }
>>
>> -     writel(cntl, priv->regs + CLCD_PL111_CNTL);
>> +     writel(cntl, priv->regs + priv->ctrl);
>>
>>       drm_panel_enable(priv->panel);
>>
>> @@ -219,7 +219,7 @@ void pl111_display_disable(struct drm_simple_display_pipe *pipe)
>>       drm_panel_disable(priv->panel);
>>
>>       /* Disable and Power Down */
>> -     writel(0, priv->regs + CLCD_PL111_CNTL);
>> +     writel(0, priv->regs + priv->ctrl);
>>
>>       drm_panel_unprepare(priv->panel);
>>
>> @@ -259,7 +259,7 @@ int pl111_enable_vblank(struct drm_device *drm, unsigned int crtc)
>>  {
>>       struct pl111_drm_dev_private *priv = drm->dev_private;
>>
>> -     writel(CLCD_IRQ_NEXTBASE_UPDATE, priv->regs + CLCD_PL111_IENB);
>> +     writel(CLCD_IRQ_NEXTBASE_UPDATE, priv->regs + priv->ienb);
>>
>>       return 0;
>>  }
>> @@ -268,7 +268,7 @@ void pl111_disable_vblank(struct drm_device *drm, unsigned int crtc)
>>  {
>>       struct pl111_drm_dev_private *priv = drm->dev_private;
>>
>> -     writel(0, priv->regs + CLCD_PL111_IENB);
>> +     writel(0, priv->regs + priv->ienb);
>>  }
>>
>>  static int pl111_display_prepare_fb(struct drm_simple_display_pipe *pipe,
>> @@ -412,22 +412,6 @@ int pl111_display_init(struct drm_device *drm)
>>       struct device_node *endpoint;
>>       u32 tft_r0b0g0[3];
>>       int ret;
>> -     static const u32 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,
>> -     };
>>
>>       endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
>>       if (!endpoint)
>> @@ -456,7 +440,8 @@ int pl111_display_init(struct drm_device *drm)
>>
>>       ret = drm_simple_display_pipe_init(drm, &priv->pipe,
>>                                          &pl111_display_funcs,
>> -                                        formats, ARRAY_SIZE(formats),
>> +                                        priv->variant->formats,
>> +                                        priv->variant->nformats,
>>                                          priv->connector);
>>       if (ret)
>>               return ret;
>> diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h
>> index 8804af0f8997..b316a8a0fbc0 100644
>> --- a/drivers/gpu/drm/pl111/pl111_drm.h
>> +++ b/drivers/gpu/drm/pl111/pl111_drm.h
>> @@ -31,6 +31,20 @@
>>
>>  struct drm_minor;
>>
>> +/**
>> + * struct pl111_variant_data - encodes IP differences
>> + * @name: the name of this variant
>> + * @is_pl110: this is the early 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;
>> +     const u32 *formats;
>> +     unsigned int nformats;
>> +};
>> +
>>  struct pl111_drm_dev_private {
>>       struct drm_device *drm;
>>
>> @@ -42,6 +56,8 @@ struct pl111_drm_dev_private {
>>       struct drm_fbdev_cma *fbdev;
>>
>>       void *regs;
>> +     u32 ienb;
>> +     u32 ctrl;
>>       /* The pixel clock (a reference to our clock divider off of CLCDCLK). */
>>       struct clk *clk;
>>       /* pl111's internal clock divider. */
>> @@ -50,6 +66,7 @@ struct pl111_drm_dev_private {
>>        * subsystem and pl111_display_enable().
>>        */
>>       spinlock_t tim2_lock;
>> +     struct pl111_variant_data *variant;
>>  };
>>
>>  int pl111_display_init(struct drm_device *dev);
>> diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
>> index e66cbf202e17..f6863c0fb809 100644
>> --- a/drivers/gpu/drm/pl111/pl111_drv.c
>> +++ b/drivers/gpu/drm/pl111/pl111_drv.c
>> @@ -206,6 +206,7 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
>>  {
>>       struct device *dev = &amba_dev->dev;
>>       struct pl111_drm_dev_private *priv;
>> +     struct pl111_variant_data *variant = id->data;
>>       struct drm_device *drm;
>>       int ret;
>>
>> @@ -219,6 +220,33 @@ 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
>> +      * swapped: interrupt enable and control. For this reason
>> +      * we use offsets that we can change per variant.
>> +      */
>> +     if (variant->is_pl110) {
>> +             /*
>> +              * The ARM Versatile boards are even more special:
>> +              * their PrimeCell ID say they are PL110 but the
>> +              * control and interrupt enable registers are anyway
>> +              * swapped to the PL111 order so they are not following
>> +              * the PL110 datasheet.
>> +              */
>> +             if (of_machine_is_compatible("arm,versatile-ab") ||
>> +                 of_machine_is_compatible("arm,versatile-pb")) {
>> +                     priv->ienb = CLCD_PL111_IENB;
>> +                     priv->ctrl = CLCD_PL111_CNTL;
>> +             } else {
>> +                     priv->ienb = CLCD_PL110_IENB;
>> +                     priv->ctrl = CLCD_PL110_CNTL;
>> +             }
>> +     } else {
>> +             priv->ienb = CLCD_PL111_IENB;
>> +             priv->ctrl = CLCD_PL111_CNTL;
>> +     }
>>
>>       priv->regs = devm_ioremap_resource(dev, &amba_dev->res);
>>       if (IS_ERR(priv->regs)) {
>> @@ -227,10 +255,10 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
>>       }
>>
>>       /* turn off interrupts before requesting the irq */
>> -     writel(0, priv->regs + CLCD_PL111_IENB);
>> +     writel(0, priv->regs + priv->ienb);
>>
>>       ret = devm_request_irq(dev, amba_dev->irq[0], pl111_irq, 0,
>> -                            "pl111", priv);
>> +                            variant->name, priv);
>>       if (ret != 0) {
>>               dev_err(dev, "%s failed irq %d\n", __func__, ret);
>>               return ret;
>> @@ -269,10 +297,62 @@ 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,
>> +};
>> +
>> +struct pl111_variant_data pl110_variant = {
>
> I think this (and `pl111_variant` below) can be `static const`, right?
>
Static - yes, const - I don't think so.
Struct amba_id::data lacks the constness, so the const qualifier will
get discarded.

In practise everyone considers/uses ::data as const, so that could be
tweaked with separate patch.

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

^ permalink raw reply	[flat|nested] 32+ messages in thread
* [PATCH 4/7] drm/pl111: Enable PL110 variant
  2017-09-01  8:23       ` Emil Velikov
@ 2017-09-01  9:22         ` Linus Walleij
  -1 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2017-09-01  9:22 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Sep 1, 2017 at 10:23 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:

>>Eric E:
>>> +struct pl111_variant_data pl110_variant = {
>>
>> I think this (and `pl111_variant` below) can be `static const`, right?
>>
> Static - yes, const - I don't think so.
> Struct amba_id::data lacks the constness, so the const qualifier will
> get discarded.

I fixed it with a (void*) cast so we can have it right in the driver.

> In practise everyone considers/uses ::data as const, so that could be
> tweaked with separate patch.

I think it is not necessarily possible to make that const since the amba_id
is used in several dynamically generated board files. But I may
be wrong.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH 4/7] drm/pl111: Enable PL110 variant
@ 2017-09-01  9:22         ` Linus Walleij
  0 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2017-09-01  9:22 UTC (permalink / raw)
  To: Emil Velikov; +Cc: Daniel Vetter, LAKML, ML dri-devel

On Fri, Sep 1, 2017 at 10:23 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:

>>Eric E:
>>> +struct pl111_variant_data pl110_variant = {
>>
>> I think this (and `pl111_variant` below) can be `static const`, right?
>>
> Static - yes, const - I don't think so.
> Struct amba_id::data lacks the constness, so the const qualifier will
> get discarded.

I fixed it with a (void*) cast so we can have it right in the driver.

> In practise everyone considers/uses ::data as const, so that could be
> tweaked with separate patch.

I think it is not necessarily possible to make that const since the amba_id
is used in several dynamically generated board files. But I may
be wrong.

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] 32+ messages in thread
* [PATCH 7/7] drm/pl111: Add handling of Versatile platforms
  2017-08-31 17:58     ` Eric Anholt
@ 2017-09-01  9:27       ` Linus Walleij
  -1 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2017-09-01  9:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Aug 31, 2017 at 7:58 PM, Eric Anholt <eric@anholt.net> wrote:
> Linus Walleij <linus.walleij@linaro.org> writes:
>
>> The ARM reference designs in the Versatile family: Integrator,
>> Versatile and RealView can make use of the new DRM driver as well.
>> We just need to create a bit of platform-specific code for them
>> that we isolate to its own file.
>>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>> ---
>>  drivers/gpu/drm/pl111/Makefile          |   1 +
>>  drivers/gpu/drm/pl111/pl111_display.c   |   7 -
>>  drivers/gpu/drm/pl111/pl111_drv.c       |   5 +
>>  drivers/gpu/drm/pl111/pl111_versatile.c | 270 ++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/pl111/pl111_versatile.h |   9 ++
>>  5 files changed, 285 insertions(+), 7 deletions(-)
>>  create mode 100644 drivers/gpu/drm/pl111/pl111_versatile.c
>>  create mode 100644 drivers/gpu/drm/pl111/pl111_versatile.h
>>
>> diff --git a/drivers/gpu/drm/pl111/Makefile b/drivers/gpu/drm/pl111/Makefile
>> index c5f8f9684848..fce1453a93e1 100644
>> --- a/drivers/gpu/drm/pl111/Makefile
>> +++ b/drivers/gpu/drm/pl111/Makefile
>> @@ -1,4 +1,5 @@
>>  pl111_drm-y +=       pl111_display.o \
>> +             pl111_versatile.o \
>>               pl111_drv.o
>>
>>  pl111_drm-$(CONFIG_DEBUG_FS) += pl111_debugfs.o
>> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
>> index 37f409867934..f7b043f4fed6 100644
>> --- a/drivers/gpu/drm/pl111/pl111_display.c
>> +++ b/drivers/gpu/drm/pl111/pl111_display.c
>> @@ -454,13 +454,6 @@ int pl111_display_init(struct drm_device *drm)
>>       }
>>       of_node_put(endpoint);
>>
>> -     if (tft_r0b0g0[0] != 0 ||
>> -         tft_r0b0g0[1] != 8 ||
>> -         tft_r0b0g0[2] != 16) {
>> -             dev_err(dev, "arm,pl11x,tft-r0g0b0-pads != [0,8,16] not yet supported\n");
>> -             return -EINVAL;
>> -     }
>
> I had a note in the DOC section of pl111_drv.c about needing to filter
> available formats based on this property.  Could you update that with
> some explanation of the new state of things?  (Do we actually need to
> filter the formats?)

No we don't need to filter it really, only the Nomadik makes use of this
property and I haven't come to adding support for that yet.

> I haven't verified that we get the same pin routing setup as the fbdev
> driver did for these platforms, but even if they don't match yet this
> seems like an excellent start and we can make sure we sort them out as
> we add panel drivers.

This should be fine with panels I think, the problem is how the
CLCD/PL11x is integrated with the silicon, as sometimes the VHDL
coders just arbitrarily manage to switch things around. It should
be handled entirely in the PL11x driver.

> I've verified that my Cygnus board continues working with your series.

Nice! I have graphics working on the RealView machines.

Now I just need to figure out a few minor things like augmenting the
list of resolutions from the panel for the bus bandwidth and
some dumb VGA bridge business.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH 7/7] drm/pl111: Add handling of Versatile platforms
@ 2017-09-01  9:27       ` Linus Walleij
  0 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2017-09-01  9:27 UTC (permalink / raw)
  To: Eric Anholt; +Cc: linux-arm-kernel, Daniel Vetter, open list:DRM PANEL DRIVERS

On Thu, Aug 31, 2017 at 7:58 PM, Eric Anholt <eric@anholt.net> wrote:
> Linus Walleij <linus.walleij@linaro.org> writes:
>
>> The ARM reference designs in the Versatile family: Integrator,
>> Versatile and RealView can make use of the new DRM driver as well.
>> We just need to create a bit of platform-specific code for them
>> that we isolate to its own file.
>>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>> ---
>>  drivers/gpu/drm/pl111/Makefile          |   1 +
>>  drivers/gpu/drm/pl111/pl111_display.c   |   7 -
>>  drivers/gpu/drm/pl111/pl111_drv.c       |   5 +
>>  drivers/gpu/drm/pl111/pl111_versatile.c | 270 ++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/pl111/pl111_versatile.h |   9 ++
>>  5 files changed, 285 insertions(+), 7 deletions(-)
>>  create mode 100644 drivers/gpu/drm/pl111/pl111_versatile.c
>>  create mode 100644 drivers/gpu/drm/pl111/pl111_versatile.h
>>
>> diff --git a/drivers/gpu/drm/pl111/Makefile b/drivers/gpu/drm/pl111/Makefile
>> index c5f8f9684848..fce1453a93e1 100644
>> --- a/drivers/gpu/drm/pl111/Makefile
>> +++ b/drivers/gpu/drm/pl111/Makefile
>> @@ -1,4 +1,5 @@
>>  pl111_drm-y +=       pl111_display.o \
>> +             pl111_versatile.o \
>>               pl111_drv.o
>>
>>  pl111_drm-$(CONFIG_DEBUG_FS) += pl111_debugfs.o
>> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
>> index 37f409867934..f7b043f4fed6 100644
>> --- a/drivers/gpu/drm/pl111/pl111_display.c
>> +++ b/drivers/gpu/drm/pl111/pl111_display.c
>> @@ -454,13 +454,6 @@ int pl111_display_init(struct drm_device *drm)
>>       }
>>       of_node_put(endpoint);
>>
>> -     if (tft_r0b0g0[0] != 0 ||
>> -         tft_r0b0g0[1] != 8 ||
>> -         tft_r0b0g0[2] != 16) {
>> -             dev_err(dev, "arm,pl11x,tft-r0g0b0-pads != [0,8,16] not yet supported\n");
>> -             return -EINVAL;
>> -     }
>
> I had a note in the DOC section of pl111_drv.c about needing to filter
> available formats based on this property.  Could you update that with
> some explanation of the new state of things?  (Do we actually need to
> filter the formats?)

No we don't need to filter it really, only the Nomadik makes use of this
property and I haven't come to adding support for that yet.

> I haven't verified that we get the same pin routing setup as the fbdev
> driver did for these platforms, but even if they don't match yet this
> seems like an excellent start and we can make sure we sort them out as
> we add panel drivers.

This should be fine with panels I think, the problem is how the
CLCD/PL11x is integrated with the silicon, as sometimes the VHDL
coders just arbitrarily manage to switch things around. It should
be handled entirely in the PL11x driver.

> I've verified that my Cygnus board continues working with your series.

Nice! I have graphics working on the RealView machines.

Now I just need to figure out a few minor things like augmenting the
list of resolutions from the panel for the bus bandwidth and
some dumb VGA bridge business.

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] 32+ messages in thread
end of thread, other threads:[~2017-09-01  9:27 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-30 18:07 [PATCH 0/7] First set of PL111 enhancements Linus Walleij
2017-08-30 18:07 ` Linus Walleij
2017-08-30 18:07 ` [PATCH 1/7] drm/pl111: Cleanup local header file Linus Walleij
2017-08-30 18:07   ` Linus Walleij
2017-08-30 18:07 ` [PATCH 2/7] drm/pl111: Add all registers to debugfs Linus Walleij
2017-08-30 18:07   ` Linus Walleij
2017-08-31 17:40   ` Eric Anholt
2017-08-31 17:40     ` Eric Anholt
2017-08-30 18:07 ` [PATCH 3/7] drm/pl111: Replace custom connector with panel bridge Linus Walleij
2017-08-30 18:07   ` Linus Walleij
2017-08-31 17:38   ` Eric Anholt
2017-08-31 17:38     ` Eric Anholt
2017-08-30 18:07 ` [PATCH 4/7] drm/pl111: Enable PL110 variant Linus Walleij
2017-08-30 18:07   ` Linus Walleij
2017-08-31 10:19   ` Eric Engestrom
2017-08-31 10:19     ` Eric Engestrom
2017-09-01  8:23     ` Emil Velikov
2017-09-01  8:23       ` Emil Velikov
2017-09-01  9:22       ` Linus Walleij
2017-09-01  9:22         ` Linus Walleij
2017-08-31 17:46   ` Eric Anholt
2017-08-31 17:46     ` Eric Anholt
2017-08-30 18:07 ` [PATCH 5/7] drm/pl111: Insert delay before powering up PL11x Linus Walleij
2017-08-30 18:07   ` Linus Walleij
2017-08-30 18:07 ` [PATCH 6/7] drm/pl111: Add optional variant display en/disable callbacks Linus Walleij
2017-08-30 18:07   ` Linus Walleij
2017-08-30 18:07 ` [PATCH 7/7] drm/pl111: Add handling of Versatile platforms Linus Walleij
2017-08-30 18:07   ` Linus Walleij
2017-08-31 17:58   ` Eric Anholt
2017-08-31 17:58     ` Eric Anholt
2017-09-01  9:27     ` Linus Walleij
2017-09-01  9:27       ` Linus Walleij

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.