[PATCH 0/5 v2] PL111 Integrator and Versatile support

[PATCH 0/5 v2] PL111 Integrator and Versatile support

[Linus Walleij]

This adds the old ARM Reference Designs Integrator and Versatile
to be supported by the PL111 driver so we can migrate them
away from fbdev.

Some patches were sent before, some dropped, but I named them
all v2 for consistency and to avoid confusion.

Linus Walleij (5):
  drm/pl111: Properly detect the ARM PL110 variants
  drm/pl111: Handle the Versatile RGB/BGR565 mode
  drm/pl111: Support variants with broken clock divider
  drm/pl111: Support variants with broken VBLANK
  drm/pl111: Support multiple endpoints on the CLCD

 drivers/gpu/drm/pl111/pl111_display.c   |  29 ++++++--
 drivers/gpu/drm/pl111/pl111_drm.h       |   8 +++
 drivers/gpu/drm/pl111/pl111_drv.c       | 118 +++++++++++++++++++++-----------
 drivers/gpu/drm/pl111/pl111_versatile.c |  87 ++++++++++++++++++-----
 4 files changed, 178 insertions(+), 64 deletions(-)

-- 
2.14.3

[PATCH 1/5 v2] drm/pl111: Properly detect the ARM PL110 variants

[Linus Walleij]

With a bit of refactoring we can contain the variant data for
the strange PL110 versions that is feature-incomplete PL110 for
the ARM Integrator/CP and somewhere inbetween PL110 and PL111
for the ARM Versatile AB and Versatile PB.

We also accomodate for the custom duct-taped RGB565/BGR565 support
in the Versatile variant.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Push more logic into the pl111_versatile file and keep the
  driver core neutral.
- Pave the way better for the Integrator/CP variant as well.
---
 drivers/gpu/drm/pl111/pl111_drm.h       |  3 ++
 drivers/gpu/drm/pl111/pl111_drv.c       | 37 ++++----------
 drivers/gpu/drm/pl111/pl111_versatile.c | 85 +++++++++++++++++++++++++--------
 3 files changed, 79 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h
index 440f53ebee8c..c2f410f0b12e 100644
--- a/drivers/gpu/drm/pl111/pl111_drm.h
+++ b/drivers/gpu/drm/pl111/pl111_drm.h
@@ -36,12 +36,15 @@ struct drm_minor;
  * struct pl111_variant_data - encodes IP differences
  * @name: the name of this variant
  * @is_pl110: this is the early PL110 variant
+ * @external_bgr: this is the Versatile Pl110 variant with external
+ *	BGR/RGB routing
  * @formats: array of supported pixel formats on this variant
  * @nformats: the length of the array of supported pixel formats
  */
 struct pl111_variant_data {
 	const char *name;
 	bool is_pl110;
+	bool external_bgr;
 	const u32 *formats;
 	unsigned int nformats;
 };
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index 31a0c4268cc6..6967cd5428b2 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -205,7 +205,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;
+	const struct pl111_variant_data *variant = id->data;
 	struct drm_device *drm;
 	int ret;
 
@@ -221,27 +221,10 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
 	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.
-	 */
+	/* The two variants swap this register */
 	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;
-		}
+		priv->ienb = CLCD_PL110_IENB;
+		priv->ctrl = CLCD_PL110_CNTL;
 	} else {
 		priv->ienb = CLCD_PL111_IENB;
 		priv->ctrl = CLCD_PL111_CNTL;
@@ -253,6 +236,11 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
 		return PTR_ERR(priv->regs);
 	}
 
+	/* This may override some variant settings */
+	ret = pl111_versatile_init(dev, priv);
+	if (ret)
+		goto dev_unref;
+
 	/* turn off interrupts before requesting the irq */
 	writel(0, priv->regs + priv->ienb);
 
@@ -263,10 +251,6 @@ 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;
@@ -299,8 +283,7 @@ static int pl111_amba_remove(struct amba_device *amba_dev)
 }
 
 /*
- * This variant exist in early versions like the ARM Integrator
- * and this version lacks the 565 and 444 pixel formats.
+ * This early variant lacks the 565 and 444 pixel formats.
  */
 static const u32 pl110_pixel_formats[] = {
 	DRM_FORMAT_ABGR8888,
diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c
index 97d4af6925a3..893d527fb42f 100644
--- a/drivers/gpu/drm/pl111/pl111_versatile.c
+++ b/drivers/gpu/drm/pl111/pl111_versatile.c
@@ -1,3 +1,4 @@
+#include <linux/amba/clcd-regs.h>
 #include <linux/device.h>
 #include <linux/of.h>
 #include <linux/regmap.h>
@@ -64,10 +65,8 @@ static const struct of_device_id versatile_clcd_of_match[] = {
 #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))
+/* Bits 11,12,13 controls the LCD or VGA bridge type */
 #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))
@@ -82,16 +81,7 @@ static const struct of_device_id versatile_clcd_of_match[] = {
 /* 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)
+#define INTEGRATOR_CLCD_MASK		GENMASK(19,8)
 
 static void pl111_integrator_enable(struct drm_device *drm, u32 format)
 {
@@ -106,11 +96,8 @@ static void pl111_integrator_enable(struct drm_device *drm, u32 format)
 	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;
+		/* 24bit formats */
+		val |= INTEGRATOR_CLCD_LCDMUX_VGA24;
 		break;
 	case DRM_FORMAT_XBGR1555:
 	case DRM_FORMAT_XRGB1555:
@@ -217,6 +204,55 @@ static void pl111_realview_clcd_enable(struct drm_device *drm, u32 format)
 			   SYS_CLCD_NLCDIOON | SYS_CLCD_PWR3V5SWITCH);
 }
 
+/* PL110 pixel formats for Integrator, vanilla PL110 */
+static const u32 pl110_integrator_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,
+};
+
+/* Extended PL110 pixel formats for Integrator and Versatile */
+static const u32 pl110_versatile_pixel_formats[] = {
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_ARGB8888,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_BGR565, /* Uses external PLD */
+	DRM_FORMAT_RGB565, /* Uses external PLD */
+	DRM_FORMAT_ABGR1555,
+	DRM_FORMAT_XBGR1555,
+	DRM_FORMAT_ARGB1555,
+	DRM_FORMAT_XRGB1555,
+};
+
+/*
+ * The Integrator variant is a PL110 with a bunch of broken, or not
+ * yet implemented features
+ */
+static const struct pl111_variant_data pl110_integrator = {
+	.name = "PL110 Integrator",
+	.is_pl110 = true,
+	.formats = pl110_integrator_pixel_formats,
+	.nformats = ARRAY_SIZE(pl110_integrator_pixel_formats),
+};
+
+/*
+ * This is the in-between PL110 variant found in the ARM Versatile,
+ * supporting RGB565/BGR565
+ */
+static const struct pl111_variant_data pl110_versatile = {
+	.name = "PL110 Versatile",
+	.is_pl110 = true,
+	.external_bgr = true,
+	.formats = pl110_versatile_pixel_formats,
+	.nformats = ARRAY_SIZE(pl110_versatile_pixel_formats),
+};
+
 int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
 {
 	const struct of_device_id *clcd_id;
@@ -241,14 +277,25 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
 	switch (versatile_clcd_type) {
 	case INTEGRATOR_CLCD_CM:
 		versatile_syscon_map = map;
+		/* This can do RGB565 with external PLD */
+		priv->variant = &pl110_integrator;
 		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;
+		/* This can do RGB565 with external PLD */
+		priv->variant = &pl110_versatile;
 		priv->variant_display_enable = pl111_versatile_enable;
 		priv->variant_display_disable = pl111_versatile_disable;
-		dev_info(dev, "set up callbacks for Versatile PL110+\n");
+		/*
+		 * The Versatile has a variant halfway between PL110
+		 * and PL111 where these two registers have already been
+		 * swapped.
+		 */
+		priv->ienb = CLCD_PL111_IENB;
+		priv->ctrl = CLCD_PL111_CNTL;
+		dev_info(dev, "set up callbacks for Versatile PL110\n");
 		break;
 	case REALVIEW_CLCD_EB:
 	case REALVIEW_CLCD_PB1176:
-- 
2.14.3

[PATCH 1/5 v2] drm/pl111: Properly detect the ARM PL110 variants

[Eric Anholt]

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

> With a bit of refactoring we can contain the variant data for
> the strange PL110 versions that is feature-incomplete PL110 for
> the ARM Integrator/CP and somewhere inbetween PL110 and PL111
> for the ARM Versatile AB and Versatile PB.
>
> We also accomodate for the custom duct-taped RGB565/BGR565 support
> in the Versatile variant.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Push more logic into the pl111_versatile file and keep the
>   driver core neutral.
> - Pave the way better for the Integrator/CP variant as well.
> ---
>  drivers/gpu/drm/pl111/pl111_drm.h       |  3 ++
>  drivers/gpu/drm/pl111/pl111_drv.c       | 37 ++++----------
>  drivers/gpu/drm/pl111/pl111_versatile.c | 85 +++++++++++++++++++++++++--------
>  3 files changed, 79 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h
> index 440f53ebee8c..c2f410f0b12e 100644
> --- a/drivers/gpu/drm/pl111/pl111_drm.h
> +++ b/drivers/gpu/drm/pl111/pl111_drm.h
> @@ -36,12 +36,15 @@ struct drm_minor;
>   * struct pl111_variant_data - encodes IP differences
>   * @name: the name of this variant
>   * @is_pl110: this is the early PL110 variant
> + * @external_bgr: this is the Versatile Pl110 variant with external
> + *	BGR/RGB routing
>   * @formats: array of supported pixel formats on this variant
>   * @nformats: the length of the array of supported pixel formats
>   */
>  struct pl111_variant_data {
>  	const char *name;
>  	bool is_pl110;
> +	bool external_bgr;
>  	const u32 *formats;
>  	unsigned int nformats;
>  };
> diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
> index 31a0c4268cc6..6967cd5428b2 100644
> --- a/drivers/gpu/drm/pl111/pl111_drv.c
> +++ b/drivers/gpu/drm/pl111/pl111_drv.c
> @@ -205,7 +205,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;
> +	const struct pl111_variant_data *variant = id->data;
>  	struct drm_device *drm;
>  	int ret;
>  
> @@ -221,27 +221,10 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
>  	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.
> -	 */
> +	/* The two variants swap this register */
>  	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;
> -		}
> +		priv->ienb = CLCD_PL110_IENB;
> +		priv->ctrl = CLCD_PL110_CNTL;
>  	} else {
>  		priv->ienb = CLCD_PL111_IENB;
>  		priv->ctrl = CLCD_PL111_CNTL;
> @@ -253,6 +236,11 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
>  		return PTR_ERR(priv->regs);
>  	}
>  
> +	/* This may override some variant settings */
> +	ret = pl111_versatile_init(dev, priv);
> +	if (ret)
> +		goto dev_unref;
> +
>  	/* turn off interrupts before requesting the irq */
>  	writel(0, priv->regs + priv->ienb);
>  
> @@ -263,10 +251,6 @@ 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;
> @@ -299,8 +283,7 @@ static int pl111_amba_remove(struct amba_device *amba_dev)
>  }
>  
>  /*
> - * This variant exist in early versions like the ARM Integrator
> - * and this version lacks the 565 and 444 pixel formats.
> + * This early variant lacks the 565 and 444 pixel formats.
>   */
>  static const u32 pl110_pixel_formats[] = {
>  	DRM_FORMAT_ABGR8888,
> diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c
> index 97d4af6925a3..893d527fb42f 100644
> --- a/drivers/gpu/drm/pl111/pl111_versatile.c
> +++ b/drivers/gpu/drm/pl111/pl111_versatile.c
> @@ -1,3 +1,4 @@
> +#include <linux/amba/clcd-regs.h>
>  #include <linux/device.h>
>  #include <linux/of.h>
>  #include <linux/regmap.h>
> @@ -64,10 +65,8 @@ static const struct of_device_id versatile_clcd_of_match[] = {
>  #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))
> +/* Bits 11,12,13 controls the LCD or VGA bridge type */
>  #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))
> @@ -82,16 +81,7 @@ static const struct of_device_id versatile_clcd_of_match[] = {
>  /* 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)
> +#define INTEGRATOR_CLCD_MASK		GENMASK(19,8)
>  
>  static void pl111_integrator_enable(struct drm_device *drm, u32 format)
>  {
> @@ -106,11 +96,8 @@ static void pl111_integrator_enable(struct drm_device *drm, u32 format)
>  	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;
> +		/* 24bit formats */
> +		val |= INTEGRATOR_CLCD_LCDMUX_VGA24;
>  		break;
>  	case DRM_FORMAT_XBGR1555:
>  	case DRM_FORMAT_XRGB1555:
> @@ -217,6 +204,55 @@ static void pl111_realview_clcd_enable(struct drm_device *drm, u32 format)
>  			   SYS_CLCD_NLCDIOON | SYS_CLCD_PWR3V5SWITCH);
>  }
>  
> +/* PL110 pixel formats for Integrator, vanilla PL110 */
> +static const u32 pl110_integrator_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,
> +};
> +
> +/* Extended PL110 pixel formats for Integrator and Versatile */
> +static const u32 pl110_versatile_pixel_formats[] = {
> +	DRM_FORMAT_ABGR8888,
> +	DRM_FORMAT_XBGR8888,
> +	DRM_FORMAT_ARGB8888,
> +	DRM_FORMAT_XRGB8888,
> +	DRM_FORMAT_BGR565, /* Uses external PLD */
> +	DRM_FORMAT_RGB565, /* Uses external PLD */
> +	DRM_FORMAT_ABGR1555,
> +	DRM_FORMAT_XBGR1555,
> +	DRM_FORMAT_ARGB1555,
> +	DRM_FORMAT_XRGB1555,
> +};
> +
> +/*
> + * The Integrator variant is a PL110 with a bunch of broken, or not
> + * yet implemented features
> + */
> +static const struct pl111_variant_data pl110_integrator = {
> +	.name = "PL110 Integrator",
> +	.is_pl110 = true,
> +	.formats = pl110_integrator_pixel_formats,
> +	.nformats = ARRAY_SIZE(pl110_integrator_pixel_formats),
> +};
> +
> +/*
> + * This is the in-between PL110 variant found in the ARM Versatile,
> + * supporting RGB565/BGR565
> + */
> +static const struct pl111_variant_data pl110_versatile = {
> +	.name = "PL110 Versatile",
> +	.is_pl110 = true,
> +	.external_bgr = true,
> +	.formats = pl110_versatile_pixel_formats,
> +	.nformats = ARRAY_SIZE(pl110_versatile_pixel_formats),
> +};
> +
>  int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
>  {
>  	const struct of_device_id *clcd_id;
> @@ -241,14 +277,25 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
>  	switch (versatile_clcd_type) {
>  	case INTEGRATOR_CLCD_CM:
>  		versatile_syscon_map = map;
> +		/* This can do RGB565 with external PLD */

I don't think you meant to have this comment here, since RGB565 isn't in
integrator's format lits.  Other than that, the patch gets my r-b.

> +		priv->variant = &pl110_integrator;
>  		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;
> +		/* This can do RGB565 with external PLD */
> +		priv->variant = &pl110_versatile;
>  		priv->variant_display_enable = pl111_versatile_enable;
>  		priv->variant_display_disable = pl111_versatile_disable;
> -		dev_info(dev, "set up callbacks for Versatile PL110+\n");
> +		/*
> +		 * The Versatile has a variant halfway between PL110
> +		 * and PL111 where these two registers have already been
> +		 * swapped.
> +		 */
> +		priv->ienb = CLCD_PL111_IENB;
> +		priv->ctrl = CLCD_PL111_CNTL;
> +		dev_info(dev, "set up callbacks for Versatile PL110\n");
>  		break;
>  	case REALVIEW_CLCD_EB:
>  	case REALVIEW_CLCD_PB1176:
> -- 
> 2.14.3
-------------- 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/20180202/9704ba2e/attachment.sig>

[PATCH 2/5 v2] drm/pl111: Handle the Versatile RGB/BGR565 mode

[Linus Walleij]

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

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Rebase on earlier changes.
---
 drivers/gpu/drm/pl111/pl111_display.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 7fe4040aea46..55ada00ec974 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -199,10 +199,17 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 		cntl |= CNTL_LCDBPP24 | CNTL_BGR;
 		break;
 	case DRM_FORMAT_BGR565:
-		cntl |= CNTL_LCDBPP16_565;
+		if (priv->variant->is_pl110)
+			cntl |= CNTL_LCDBPP16;
+		else
+			cntl |= CNTL_LCDBPP16_565;
 		break;
 	case DRM_FORMAT_RGB565:
-		cntl |= CNTL_LCDBPP16_565 | CNTL_BGR;
+		if (priv->variant->is_pl110)
+			cntl |= CNTL_LCDBPP16;
+		else
+			cntl |= CNTL_LCDBPP16_565;
+		cntl |= CNTL_BGR;
 		break;
 	case DRM_FORMAT_ABGR1555:
 	case DRM_FORMAT_XBGR1555:
@@ -226,6 +233,10 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 		break;
 	}
 
+	/* The PL110 in Integrator/Versatile does the BGR routing externally */
+	if (priv->variant->external_bgr)
+		cntl &= ~CNTL_BGR;
+
 	/* Power sequence: first enable and chill */
 	writel(cntl, priv->regs + priv->ctrl);
 
-- 
2.14.3

[PATCH 3/5 v2] drm/pl111: Support variants with broken clock divider

[Linus Walleij]

The early Integrator CLCD synthesized in the Integrator CP and
IM-PD1 FPGAs are broken: their clock dividers do not work
properly. Support disabling the clock divider and drive the
clock directly from the parent under these circumstances.

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

diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 55ada00ec974..4d4e38b4c9d5 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -138,6 +138,9 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 	tim2 = readl(priv->regs + CLCD_TIM2);
 	tim2 &= (TIM2_BCD | TIM2_PCD_LO_MASK | TIM2_PCD_HI_MASK);
 
+	if (priv->variant->broken_clockdivider)
+		tim2 |= TIM2_BCD;
+
 	if (mode->flags & DRM_MODE_FLAG_NHSYNC)
 		tim2 |= TIM2_IHS;
 
@@ -455,6 +458,11 @@ pl111_init_clock_divider(struct drm_device *drm)
 		dev_err(drm->dev, "CLCD: unable to get clcdclk.\n");
 		return PTR_ERR(parent);
 	}
+	/* If the clock divider is broken, use the parent directly */
+	if (priv->variant->broken_clockdivider) {
+		priv->clk = parent;
+		return 0;
+	}
 	parent_name = __clk_get_name(parent);
 
 	spin_lock_init(&priv->tim2_lock);
diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h
index c2f410f0b12e..f75c5d4645b2 100644
--- a/drivers/gpu/drm/pl111/pl111_drm.h
+++ b/drivers/gpu/drm/pl111/pl111_drm.h
@@ -38,6 +38,8 @@ struct drm_minor;
  * @is_pl110: this is the early PL110 variant
  * @external_bgr: this is the Versatile Pl110 variant with external
  *	BGR/RGB routing
+ * @broken_clockdivider: the clock divider is broken and we need to
+ *	use the supplied clock directly
  * @formats: array of supported pixel formats on this variant
  * @nformats: the length of the array of supported pixel formats
  */
@@ -45,6 +47,7 @@ struct pl111_variant_data {
 	const char *name;
 	bool is_pl110;
 	bool external_bgr;
+	bool broken_clockdivider;
 	const u32 *formats;
 	unsigned int nformats;
 };
diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c
index 893d527fb42f..0f228543a97c 100644
--- a/drivers/gpu/drm/pl111/pl111_versatile.c
+++ b/drivers/gpu/drm/pl111/pl111_versatile.c
@@ -237,6 +237,7 @@ static const u32 pl110_versatile_pixel_formats[] = {
 static const struct pl111_variant_data pl110_integrator = {
 	.name = "PL110 Integrator",
 	.is_pl110 = true,
+	.broken_clockdivider = true,
 	.formats = pl110_integrator_pixel_formats,
 	.nformats = ARRAY_SIZE(pl110_integrator_pixel_formats),
 };
-- 
2.14.3

[PATCH 4/5 v2] drm/pl111: Support variants with broken VBLANK

[Linus Walleij]

The early Integrator CLCD synthesized in the Integrator CP and
IM-PD1 FPGAs are broken: their vertical and next base interrupts
are not functional. Support these variants by simply disabling
the use of the vblank interrupt on these variants.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/pl111/pl111_display.c   |  6 ++++--
 drivers/gpu/drm/pl111/pl111_drm.h       |  2 ++
 drivers/gpu/drm/pl111/pl111_drv.c       | 19 +++++++++++--------
 drivers/gpu/drm/pl111/pl111_versatile.c |  1 +
 4 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 4d4e38b4c9d5..d75923896609 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -256,7 +256,8 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 	cntl |= CNTL_LCDPWR;
 	writel(cntl, priv->regs + priv->ctrl);
 
-	drm_crtc_vblank_on(crtc);
+	if (!priv->variant->broken_vblank)
+		drm_crtc_vblank_on(crtc);
 }
 
 void pl111_display_disable(struct drm_simple_display_pipe *pipe)
@@ -266,7 +267,8 @@ void pl111_display_disable(struct drm_simple_display_pipe *pipe)
 	struct pl111_drm_dev_private *priv = drm->dev_private;
 	u32 cntl;
 
-	drm_crtc_vblank_off(crtc);
+	if (!priv->variant->broken_vblank)
+		drm_crtc_vblank_off(crtc);
 
 	/* Power Down */
 	cntl = readl(priv->regs + priv->ctrl);
diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h
index f75c5d4645b2..d74076c6b7ef 100644
--- a/drivers/gpu/drm/pl111/pl111_drm.h
+++ b/drivers/gpu/drm/pl111/pl111_drm.h
@@ -40,6 +40,7 @@ struct drm_minor;
  *	BGR/RGB routing
  * @broken_clockdivider: the clock divider is broken and we need to
  *	use the supplied clock directly
+ * @broken_vblank: the vblank IRQ is broken on this variant
  * @formats: array of supported pixel formats on this variant
  * @nformats: the length of the array of supported pixel formats
  */
@@ -48,6 +49,7 @@ struct pl111_variant_data {
 	bool is_pl110;
 	bool external_bgr;
 	bool broken_clockdivider;
+	bool broken_vblank;
 	const u32 *formats;
 	unsigned int nformats;
 };
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index 6967cd5428b2..e6fa897c740c 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -131,10 +131,12 @@ static int pl111_modeset_init(struct drm_device *dev)
 	if (ret)
 		return ret;
 
-	ret = drm_vblank_init(dev, 1);
-	if (ret != 0) {
-		dev_err(dev->dev, "Failed to init vblank\n");
-		goto out_bridge;
+	if (!priv->variant->broken_vblank) {
+		ret = drm_vblank_init(dev, 1);
+		if (ret != 0) {
+			dev_err(dev->dev, "Failed to init vblank\n");
+			goto out_bridge;
+		}
 	}
 
 	drm_mode_config_reset(dev);
@@ -184,10 +186,6 @@ static struct drm_driver pl111_drm_driver = {
 	.dumb_create = drm_gem_cma_dumb_create,
 	.gem_free_object_unlocked = drm_gem_cma_free_object,
 	.gem_vm_ops = &drm_gem_cma_vm_ops,
-
-	.enable_vblank = pl111_enable_vblank,
-	.disable_vblank = pl111_disable_vblank,
-
 	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
 	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_import = drm_gem_prime_import,
@@ -213,6 +211,11 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
 	if (!priv)
 		return -ENOMEM;
 
+	if (!variant->broken_vblank) {
+		pl111_drm_driver.enable_vblank = pl111_enable_vblank;
+		pl111_drm_driver.disable_vblank = pl111_disable_vblank;
+	}
+
 	drm = drm_dev_alloc(&pl111_drm_driver, dev);
 	if (IS_ERR(drm))
 		return PTR_ERR(drm);
diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c
index 0f228543a97c..26427b838b2c 100644
--- a/drivers/gpu/drm/pl111/pl111_versatile.c
+++ b/drivers/gpu/drm/pl111/pl111_versatile.c
@@ -238,6 +238,7 @@ static const struct pl111_variant_data pl110_integrator = {
 	.name = "PL110 Integrator",
 	.is_pl110 = true,
 	.broken_clockdivider = true,
+	.broken_vblank = true,
 	.formats = pl110_integrator_pixel_formats,
 	.nformats = ARRAY_SIZE(pl110_integrator_pixel_formats),
 };
-- 
2.14.3

[PATCH 4/5 v2] drm/pl111: Support variants with broken VBLANK

[Eric Anholt]

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

> The early Integrator CLCD synthesized in the Integrator CP and
> IM-PD1 FPGAs are broken: their vertical and next base interrupts
> are not functional. Support these variants by simply disabling
> the use of the vblank interrupt on these variants.

I do wonder if we just have the vblank interrupt number wrong or
something for Integrator.  However, let's get to at least functional
parity with fbdev before we worry too much about the new features that
DRM can bring us.

The remainder of this series is also:

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/20180203/1d6c2089/attachment.sig>

[PATCH 4/5 v2] drm/pl111: Support variants with broken VBLANK

[Linus Walleij]

On Sun, Feb 4, 2018 at 12:44 AM, Eric Anholt <eric@anholt.net> wrote:
> Linus Walleij <linus.walleij@linaro.org> writes:
>
>> The early Integrator CLCD synthesized in the Integrator CP and
>> IM-PD1 FPGAs are broken: their vertical and next base interrupts
>> are not functional. Support these variants by simply disabling
>> the use of the vblank interrupt on these variants.
>
> I do wonder if we just have the vblank interrupt number wrong or
> something for Integrator.  However, let's get to at least functional
> parity with fbdev before we worry too much about the new features that
> DRM can bring us.
>
> The remainder of this series is also:
>
> Reviewed-by: Eric Anholt <eric@anholt.net>

Thanks man, sent it out again with your review tag (I do this
to pick it to the dim-script thing with tags and all). And as a last
checkpoint for everyone else.

Yours,
Linus Walleij

[PATCH 5/5 v2] drm/pl111: Support multiple endpoints on the CLCD

[Linus Walleij]

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

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

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

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

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

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

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