diff mbox

[v4,3/8] drm/tilcdc: Add blue-and-red-crossed devicetree property

Message ID 097cda0d23741ab21aa13bd3f152ca2f32bf9ab3.1472719892.git.jsarha@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jyri Sarha Sept. 1, 2016, 9:09 a.m. UTC
Add "blue-and-red-wiring"-device tree property and update devicetree
binding document.

The red and blue components are reversed between 24 and 16 bit modes
on am335x LCDC output pins. To get 24 RGB format the red and blue
wires has to be crossed and this in turn causes 16 colors output to be
in BGR format. With straight wiring the 16 color is RGB and 24 bit is
BGR.

The new property describes whether the red and blue wires are crossed
or not. If the property is not present or its value is not recognized
the legacy mode is assumed. The legacy configuration supports RGB565,
RGB888 and XRGB8888 formats. However, depending on wiring, the red and
blue colors are swapped in either 16 or 24-bit color modes.

For more details see section 3.1.1 in AM335x Silicon Errata:
http://www.ti.com/general/docs/lit/getliterature.tsp?baseLiteratureNumber=sprz360

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 .../devicetree/bindings/display/tilcdc/tilcdc.txt  | 22 ++++++++++++
 drivers/gpu/drm/tilcdc/tilcdc_drv.c                | 41 ++++++++++++++++++++++
 drivers/gpu/drm/tilcdc/tilcdc_drv.h                |  4 +++
 drivers/gpu/drm/tilcdc/tilcdc_plane.c              |  9 ++---
 4 files changed, 70 insertions(+), 6 deletions(-)

Comments

Rob Herring (Arm) Sept. 12, 2016, 12:58 p.m. UTC | #1
On Thu, Sep 01, 2016 at 12:09:07PM +0300, Jyri Sarha wrote:
> Add "blue-and-red-wiring"-device tree property and update devicetree
> binding document.
> 
> The red and blue components are reversed between 24 and 16 bit modes
> on am335x LCDC output pins. To get 24 RGB format the red and blue
> wires has to be crossed and this in turn causes 16 colors output to be
> in BGR format. With straight wiring the 16 color is RGB and 24 bit is
> BGR.
> 
> The new property describes whether the red and blue wires are crossed
> or not. If the property is not present or its value is not recognized
> the legacy mode is assumed. The legacy configuration supports RGB565,
> RGB888 and XRGB8888 formats. However, depending on wiring, the red and
> blue colors are swapped in either 16 or 24-bit color modes.
> 
> For more details see section 3.1.1 in AM335x Silicon Errata:
> http://www.ti.com/general/docs/lit/getliterature.tsp?baseLiteratureNumber=sprz360
> 
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
>  .../devicetree/bindings/display/tilcdc/tilcdc.txt  | 22 ++++++++++++
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c                | 41 ++++++++++++++++++++++
>  drivers/gpu/drm/tilcdc/tilcdc_drv.h                |  4 +++
>  drivers/gpu/drm/tilcdc/tilcdc_plane.c              |  9 ++---
>  4 files changed, 70 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/tilcdc/tilcdc.txt b/Documentation/devicetree/bindings/display/tilcdc/tilcdc.txt
> index 6efa4c5..a5007aa 100644
> --- a/Documentation/devicetree/bindings/display/tilcdc/tilcdc.txt
> +++ b/Documentation/devicetree/bindings/display/tilcdc/tilcdc.txt
> @@ -17,6 +17,18 @@ Optional properties:
>     the lcd controller.
>   - max-pixelclock: The maximum pixel clock that can be supported
>     by the lcd controller in KHz.
> + - blue-and-red-wiring: Recognized values "default", "straight" or

Need to drop default from here. With that,

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

> +   "crossed". This property deals with the LCDC revision 2 (found on
> +   AM335x) color errata [1].
> +    - "straight" indicates normal wiring that supports RGB565,
> +      BGR888, and XBGR8888 color formats.
> +    - "crossed" indicates wiring that has blue and red wires
> +      crossed. This setup supports BGR565, RGB888 and XRGB8888
> +      formats.
> +    - If the property is not present or its value is not recognized
> +      the legacy mode is assumed. This configuration supports RGB565,
> +      RGB888 and XRGB8888 formats. However, depending on wiring, the red
> +      and blue colors are swapped in either 16 or 24-bit color modes.
>  
>  Optional nodes:
>  
> @@ -28,6 +40,14 @@ Optional nodes:
>     Documentation/devicetree/bindings/display/tilcdc/tfp410.txt for connecting
>     tfp410 DVI encoder or lcd panel to lcdc
>  
> +[1] There is an errata about AM335x color wiring. For 16-bit color mode
> +    the wires work as they should (LCD_DATA[0:4] is for Blue[3:7]),
> +    but for 24 bit color modes the wiring of blue and red components is
> +    crossed and LCD_DATA[0:4] is for Red[3:7] and LCD_DATA[11:15] is
> +    for Blue[3-7]. For more details see section 3.1.1 in AM335x
> +    Silicon Errata:
> +    http://www.ti.com/general/docs/lit/getliterature.tsp?baseLiteratureNumber=sprz360
> +
>  Example:
>  
>  	fb: fb@4830e000 {
> @@ -37,6 +57,8 @@ Example:
>  		interrupts = <36>;
>  		ti,hwmods = "lcdc";
>  
> +		blue-and-red-wiring = "crossed";
> +
>  		port {
>  			lcdc_0: endpoint@0 {
>  				remote-endpoint = <&hdmi_0>;
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> index e45c268..ed4dc5c 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> @@ -33,6 +33,20 @@
>  
>  static LIST_HEAD(module_list);
>  
> +static const u32 tilcdc_rev1_formats[] = { DRM_FORMAT_RGB565 };
> +
> +static const u32 tilcdc_straight_formats[] = { DRM_FORMAT_RGB565,
> +					       DRM_FORMAT_BGR888,
> +					       DRM_FORMAT_XBGR8888 };
> +
> +static const u32 tilcdc_crossed_formats[] = { DRM_FORMAT_BGR565,
> +					      DRM_FORMAT_RGB888,
> +					      DRM_FORMAT_XRGB8888 };
> +
> +static const u32 tilcdc_legacy_formats[] = { DRM_FORMAT_RGB565,
> +					     DRM_FORMAT_RGB888,
> +					     DRM_FORMAT_XRGB8888 };
> +
>  void tilcdc_module_init(struct tilcdc_module *mod, const char *name,
>  		const struct tilcdc_module_ops *funcs)
>  {
> @@ -318,6 +332,33 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
>  
>  	pm_runtime_put_sync(dev->dev);
>  
> +	if (priv->rev == 1) {
> +		DBG("Revision 1 LCDC supports only RGB565 format");
> +		priv->pixelformats = tilcdc_rev1_formats;
> +		priv->num_pixelformats = ARRAY_SIZE(tilcdc_rev1_formats);
> +	} else {
> +		const char *str = "\0";
> +
> +		of_property_read_string(node, "blue-and-red-wiring", &str);
> +		if (0 == strcmp(str, "crossed")) {
> +			DBG("Configured for crossed blue and red wires");
> +			priv->pixelformats = tilcdc_crossed_formats;
> +			priv->num_pixelformats =
> +				ARRAY_SIZE(tilcdc_crossed_formats);
> +		} else if (0 == strcmp(str, "straight")) {
> +			DBG("Configured for straight blue and red wires");
> +			priv->pixelformats = tilcdc_straight_formats;
> +			priv->num_pixelformats =
> +				ARRAY_SIZE(tilcdc_straight_formats);
> +		} else {
> +			DBG("Blue and red wiring '%s' unknown, use legacy mode",
> +			    str);
> +			priv->pixelformats = tilcdc_legacy_formats;
> +			priv->num_pixelformats =
> +				ARRAY_SIZE(tilcdc_legacy_formats);
> +		}
> +	}
> +
>  	ret = modeset_init(dev);
>  	if (ret < 0) {
>  		dev_err(dev->dev, "failed to initialize mode setting\n");
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> index 13001df..0e19c14 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> @@ -65,6 +65,10 @@ struct tilcdc_drm_private {
>  	 */
>  	uint32_t max_width;
>  
> +	/* Supported pixel formats */
> +	const uint32_t *pixelformats;
> +	uint32_t num_pixelformats;
> +
>  	/* The context for pm susped/resume cycle is stored here */
>  	struct drm_atomic_state *saved_state;
>  
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_plane.c b/drivers/gpu/drm/tilcdc/tilcdc_plane.c
> index 41911e3..74c65fa 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_plane.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_plane.c
> @@ -24,10 +24,6 @@
>  
>  #include "tilcdc_drv.h"
>  
> -static const u32 tilcdc_formats[] = { DRM_FORMAT_RGB565,
> -				      DRM_FORMAT_RGB888,
> -				      DRM_FORMAT_XRGB8888 };
> -
>  static struct drm_plane_funcs tilcdc_plane_funcs = {
>  	.update_plane	= drm_atomic_helper_update_plane,
>  	.disable_plane	= drm_atomic_helper_disable_plane,
> @@ -114,12 +110,13 @@ static const struct drm_plane_helper_funcs plane_helper_funcs = {
>  int tilcdc_plane_init(struct drm_device *dev,
>  		      struct drm_plane *plane)
>  {
> +	struct tilcdc_drm_private *priv = dev->dev_private;
>  	int ret;
>  
>  	ret = drm_plane_init(dev, plane, 1,
>  			     &tilcdc_plane_funcs,
> -			     tilcdc_formats,
> -			     ARRAY_SIZE(tilcdc_formats),
> +			     priv->pixelformats,
> +			     priv->num_pixelformats,
>  			     true);
>  	if (ret) {
>  		dev_err(dev->dev, "Failed to initialize plane: %d\n", ret);
> -- 
> 1.9.1
>
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/display/tilcdc/tilcdc.txt b/Documentation/devicetree/bindings/display/tilcdc/tilcdc.txt
index 6efa4c5..a5007aa 100644
--- a/Documentation/devicetree/bindings/display/tilcdc/tilcdc.txt
+++ b/Documentation/devicetree/bindings/display/tilcdc/tilcdc.txt
@@ -17,6 +17,18 @@  Optional properties:
    the lcd controller.
  - max-pixelclock: The maximum pixel clock that can be supported
    by the lcd controller in KHz.
+ - blue-and-red-wiring: Recognized values "default", "straight" or
+   "crossed". This property deals with the LCDC revision 2 (found on
+   AM335x) color errata [1].
+    - "straight" indicates normal wiring that supports RGB565,
+      BGR888, and XBGR8888 color formats.
+    - "crossed" indicates wiring that has blue and red wires
+      crossed. This setup supports BGR565, RGB888 and XRGB8888
+      formats.
+    - If the property is not present or its value is not recognized
+      the legacy mode is assumed. This configuration supports RGB565,
+      RGB888 and XRGB8888 formats. However, depending on wiring, the red
+      and blue colors are swapped in either 16 or 24-bit color modes.
 
 Optional nodes:
 
@@ -28,6 +40,14 @@  Optional nodes:
    Documentation/devicetree/bindings/display/tilcdc/tfp410.txt for connecting
    tfp410 DVI encoder or lcd panel to lcdc
 
+[1] There is an errata about AM335x color wiring. For 16-bit color mode
+    the wires work as they should (LCD_DATA[0:4] is for Blue[3:7]),
+    but for 24 bit color modes the wiring of blue and red components is
+    crossed and LCD_DATA[0:4] is for Red[3:7] and LCD_DATA[11:15] is
+    for Blue[3-7]. For more details see section 3.1.1 in AM335x
+    Silicon Errata:
+    http://www.ti.com/general/docs/lit/getliterature.tsp?baseLiteratureNumber=sprz360
+
 Example:
 
 	fb: fb@4830e000 {
@@ -37,6 +57,8 @@  Example:
 		interrupts = <36>;
 		ti,hwmods = "lcdc";
 
+		blue-and-red-wiring = "crossed";
+
 		port {
 			lcdc_0: endpoint@0 {
 				remote-endpoint = <&hdmi_0>;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index e45c268..ed4dc5c 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -33,6 +33,20 @@ 
 
 static LIST_HEAD(module_list);
 
+static const u32 tilcdc_rev1_formats[] = { DRM_FORMAT_RGB565 };
+
+static const u32 tilcdc_straight_formats[] = { DRM_FORMAT_RGB565,
+					       DRM_FORMAT_BGR888,
+					       DRM_FORMAT_XBGR8888 };
+
+static const u32 tilcdc_crossed_formats[] = { DRM_FORMAT_BGR565,
+					      DRM_FORMAT_RGB888,
+					      DRM_FORMAT_XRGB8888 };
+
+static const u32 tilcdc_legacy_formats[] = { DRM_FORMAT_RGB565,
+					     DRM_FORMAT_RGB888,
+					     DRM_FORMAT_XRGB8888 };
+
 void tilcdc_module_init(struct tilcdc_module *mod, const char *name,
 		const struct tilcdc_module_ops *funcs)
 {
@@ -318,6 +332,33 @@  static int tilcdc_load(struct drm_device *dev, unsigned long flags)
 
 	pm_runtime_put_sync(dev->dev);
 
+	if (priv->rev == 1) {
+		DBG("Revision 1 LCDC supports only RGB565 format");
+		priv->pixelformats = tilcdc_rev1_formats;
+		priv->num_pixelformats = ARRAY_SIZE(tilcdc_rev1_formats);
+	} else {
+		const char *str = "\0";
+
+		of_property_read_string(node, "blue-and-red-wiring", &str);
+		if (0 == strcmp(str, "crossed")) {
+			DBG("Configured for crossed blue and red wires");
+			priv->pixelformats = tilcdc_crossed_formats;
+			priv->num_pixelformats =
+				ARRAY_SIZE(tilcdc_crossed_formats);
+		} else if (0 == strcmp(str, "straight")) {
+			DBG("Configured for straight blue and red wires");
+			priv->pixelformats = tilcdc_straight_formats;
+			priv->num_pixelformats =
+				ARRAY_SIZE(tilcdc_straight_formats);
+		} else {
+			DBG("Blue and red wiring '%s' unknown, use legacy mode",
+			    str);
+			priv->pixelformats = tilcdc_legacy_formats;
+			priv->num_pixelformats =
+				ARRAY_SIZE(tilcdc_legacy_formats);
+		}
+	}
+
 	ret = modeset_init(dev);
 	if (ret < 0) {
 		dev_err(dev->dev, "failed to initialize mode setting\n");
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index 13001df..0e19c14 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -65,6 +65,10 @@  struct tilcdc_drm_private {
 	 */
 	uint32_t max_width;
 
+	/* Supported pixel formats */
+	const uint32_t *pixelformats;
+	uint32_t num_pixelformats;
+
 	/* The context for pm susped/resume cycle is stored here */
 	struct drm_atomic_state *saved_state;
 
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_plane.c b/drivers/gpu/drm/tilcdc/tilcdc_plane.c
index 41911e3..74c65fa 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_plane.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_plane.c
@@ -24,10 +24,6 @@ 
 
 #include "tilcdc_drv.h"
 
-static const u32 tilcdc_formats[] = { DRM_FORMAT_RGB565,
-				      DRM_FORMAT_RGB888,
-				      DRM_FORMAT_XRGB8888 };
-
 static struct drm_plane_funcs tilcdc_plane_funcs = {
 	.update_plane	= drm_atomic_helper_update_plane,
 	.disable_plane	= drm_atomic_helper_disable_plane,
@@ -114,12 +110,13 @@  static const struct drm_plane_helper_funcs plane_helper_funcs = {
 int tilcdc_plane_init(struct drm_device *dev,
 		      struct drm_plane *plane)
 {
+	struct tilcdc_drm_private *priv = dev->dev_private;
 	int ret;
 
 	ret = drm_plane_init(dev, plane, 1,
 			     &tilcdc_plane_funcs,
-			     tilcdc_formats,
-			     ARRAY_SIZE(tilcdc_formats),
+			     priv->pixelformats,
+			     priv->num_pixelformats,
 			     true);
 	if (ret) {
 		dev_err(dev->dev, "Failed to initialize plane: %d\n", ret);