[RFC 00/16] Armada DRM support for OLPC XO-1.75 laptop

[RFC 00/16] Armada DRM support for OLPC XO-1.75 laptop

[Lubomir Rintel]

Hi,

here are patches that make the Armada DRM work on an OLPC XO-1.75.
They build on patches previously submitted by Russel King (included here for
completeness as well).

They certainly need some more love, but I'm not able to improve them until
I get to understand some things first. I'm posting a couple of questions
below in hope someone is kind enough to help me deal with my confusion.

The display pipeline on the laptop looks like this:

  Armada LCDC
  -----------
      |
      v

  Himax HX8837 "DCON"
  -------------------
  RGB input from LCDC
  Controlled via I2C
  Backlight control
  Can slow down the panel refresh rate to save power
  Optional dithering for color mode, "swizzling"
  Has DRAM attached, can freeze a single frame in it
  Doc: http://wiki.laptop.org/images/0/09/DCON_datasheet_HX8837-A.pdf
      |
      v

  Innolux LS075AT011 Panel
  ------------------------
  7.5", 1200x900
  No datasheet.
  http://wiki.laptop.org/go/Display
  Can opreate in two modes:

   R G B
   G B R ...   or   "e-book" daylight readable
   B R G            reflexive B&W
     .
     :

Here's what's not clear to me:

1.) Is the Himax chip an encoder here?

2.) What's the use of an encoder anyways? If a panel was connected directly
    do the RGB output from the LCDC, would we have to fake one? Is that the
    point of things like i.MX driver's drivers/gpu/drm/imx/parallel-display.c?

3.) My patch set currently contains the driver for the Himax that is
    modelled after tda998x. That one implements an encoder. Similar drivers
    seem to add a bridge too, but it's not entirely clear to me what a bridge
    is good for?

4.) How shall I expose the fancy functionality of the Himax to the userspace?
    Notably, the freeze frame. The OLPC laptops with the stock firmware like
    to suspend the SoC very aggressively to save power (in 20 seconds of
    inactivity or so). If the display is open (it can also be turned around
    for a tablet or e-book mode), it makes sense to freeze the picture and
    keep the panel on, if the laptop is closed, we want to turn it off.
    Should the behavior be exposed via sysfs (as it is with the current OLPC
    kernels), or a DRM property? Would it require libdrm support?

Thanks,
Lubo


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

[RFC 01/16] dt-bindings: display: armada: Rename the binding doc file

[Lubomir Rintel]

It's going to document more than just marvell,dove-lcd: more components
of the display subsystems with more compatible strings.

It seems to make sense to organize this the way it is done in
Documentation/devicetree/bindings/display/imx/fsl-imx-drm.txt

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 .../armada/{marvell,dove-lcd.txt => marvell-armada-drm.txt}       | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename Documentation/devicetree/bindings/display/armada/{marvell,dove-lcd.txt => marvell-armada-drm.txt} (100%)

diff --git a/Documentation/devicetree/bindings/display/armada/marvell,dove-lcd.txt b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
similarity index 100%
rename from Documentation/devicetree/bindings/display/armada/marvell,dove-lcd.txt
rename to Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
-- 
2.19.1

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

[RFC 02/16] dt-bindings: display: armada: Improve the LCDC documentation

[Lubomir Rintel]

The port is a child, not a property. And it deserves an example.

Also, make the title a bit more visually distinguishable -- this will
look better when the documentation of other Adrmada DRM nodes will be
present.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 .../display/armada/marvell-armada-drm.txt        | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
index 46525ea3e646..2606a8efc956 100644
--- a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
+++ b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
@@ -1,10 +1,11 @@
-Device Tree bindings for Armada DRM CRTC driver
+Marvell Armada LCD controller
+=============================
 
 Required properties:
+
  - compatible: value should be "marvell,dove-lcd".
  - reg: base address and size of the LCD controller
  - interrupts: single interrupt number for the LCD controller
- - port: video output port with endpoints, as described by graph.txt
 
 Optional properties:
 
@@ -19,6 +20,11 @@ Note: all clocks are optional but at least one must be specified.
 Further clocks may be added in the future according to requirements of
 different SoCs.
 
+Required child nodes:
+
+- port: video output port with endpoints, as described by
+  Documentation/devicetree/bindings/graph.txt
+
 Example:
 
 	lcd0: lcd-controller@820000 {
@@ -27,4 +33,10 @@ Example:
 		interrupts = <47>;
 		clocks = <&si5351 0>;
 		clock-names = "ext_ref_clk_1";
+
+		lcd0_port: port {
+			lcd0_rgb_out: endpoint {
+				remote-endpoint = <&encoder_rgb_in>;
+			};
+		};
 	};
-- 
2.19.1

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

[RFC 03/16] dt-bindings: display: armada: Add framebuffer reserved-mem binding

[Lubomir Rintel]

This is the binding for memory that is set aside for allocation of Marvell
Armada framebuffer objects.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 .../display/armada/marvell-armada-drm.txt     | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
index 2606a8efc956..0bbc5056225f 100644
--- a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
+++ b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
@@ -40,3 +40,26 @@ Example:
 			};
 		};
 	};
+
+Marvell Armada framebuffer reserved memory
+==========================================
+
+Memory set aside for allocation of Marvell Armada framebuffer objects.
+
+Required properties:
+
+ - compatible: value should be "marvell,dove-framebuffer".
+
+This binding is compatible with the binding, specified in
+Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt..
+
+Example:
+
+	reserved-memory {
+		display_reserved: framebuffer {
+			compatible = "marvell,dove-framebuffer";
+			size = <0x02000000>;
+			alignment = <0x02000000>;
+			no-map;
+		};
+	};
-- 
2.19.1

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

[RFC 04/16] dt-bindings: display: armada: Add display subsystem binding

[Lubomir Rintel]

The Marvell Armada DRM master device is a virtual device needed to list all
nodes that comprise the graphics subsystem.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 .../display/armada/marvell-armada-drm.txt     | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
index 0bbc5056225f..8a5d9907065e 100644
--- a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
+++ b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
@@ -1,3 +1,25 @@
+Marvell Armada DRM master device
+================================
+
+The Marvell Armada DRM master device is a virtual device needed to list all
+nodes that comprise the graphics subsystem.
+
+Required properties:
+
+ - compatible: value should be "marvell,dove-display-subsystem"
+ - ports: a list of phandles pointing to display interface ports of CRTC
+   devices
+ - memory-region: phandle to a node describing memory to be used for the
+   framebuffer
+
+Example:
+
+	display-subsystem {
+		compatible = "marvell,dove-display-subsystem";
+		memory-region = <&display_reserved>;
+		ports = <&lcd0_port>;
+	};
+
 Marvell Armada LCD controller
 =============================
 
-- 
2.19.1

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

[RFC 05/16] dt-bindings: display: armada: Add mmp2 compatible strings

[Lubomir Rintel]

The driver will work on a MMP2 as well.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>

---
Note that perhaps "marvell,armada-display-subsystem" and
"marvell,armada-framebuffer" would be a good idea in addition of dove/mmp2
specific strings since (at least for now) the driver code is the same.
---
 .../bindings/display/armada/marvell-armada-drm.txt        | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
index 8a5d9907065e..555995ce5a9c 100644
--- a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
+++ b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
@@ -6,7 +6,8 @@ nodes that comprise the graphics subsystem.
 
 Required properties:
 
- - compatible: value should be "marvell,dove-display-subsystem"
+ - compatible: value should be "marvell,dove-display-subsystem" or
+   "marvell,mmp2-display-subsystem"
  - ports: a list of phandles pointing to display interface ports of CRTC
    devices
  - memory-region: phandle to a node describing memory to be used for the
@@ -25,7 +26,7 @@ Marvell Armada LCD controller
 
 Required properties:
 
- - compatible: value should be "marvell,dove-lcd".
+ - compatible: value should be "marvell,dove-lcd" or "marvell,mmp2-lcd"
  - reg: base address and size of the LCD controller
  - interrupts: single interrupt number for the LCD controller
 
@@ -70,7 +71,8 @@ Memory set aside for allocation of Marvell Armada framebuffer objects.
 
 Required properties:
 
- - compatible: value should be "marvell,dove-framebuffer".
+ - compatible: value should be "marvell,dove-framebuffer" or
+   "marvell,mmp2-framebuffer"
 
 This binding is compatible with the binding, specified in
 Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt..
-- 
2.19.1

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

[RFC 06/16] dt-bindings: display: armada: Document bus-width property

[Lubomir Rintel]

This makes it possible to choose a different pixel format for the
endpoint. Modelled after what other LCD controllers use, including
marvell,pxa2xx-lcdc and atmel,hlcdc-display-controller and perhaps more.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 .../bindings/display/armada/marvell-armada-drm.txt          | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
index 555995ce5a9c..5122e737277b 100644
--- a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
+++ b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
@@ -47,6 +47,11 @@ Required child nodes:
 
 - port: video output port with endpoints, as described by
   Documentation/devicetree/bindings/graph.txt
+  The endpoints can optionally specify the following property:
+
+  - bus-width: recognized values are <12>, <16>, <18> and <24>, that
+    select "rgb444", "rgb565", "rgb666" or "rgb888" pixel format
+    respectively. Defaults to <24> if unspecified.
 
 Example:
 
@@ -59,6 +64,7 @@ Example:
 
 		lcd0_port: port {
 			lcd0_rgb_out: endpoint {
+				bus-width = <24>;
 				remote-endpoint = <&encoder_rgb_in>;
 			};
 		};
-- 
2.19.1

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

[RFC 07/16] dt-bindings: display: himax, hx8837: Add Himax HX8837 bindings

[Lubomir Rintel]

Himax HX8837 is a secondary display controller used to drive the panel
on OLPC platforms.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>

---
Changes since v3:
- Moved to bindings/display/
- Added the ports
- Removed Pavel's Ack, because the changes are substantial

Changes since v2:
- s/betweend/between/

Changes since v1:
- s/load-gpio/load-gpios/
- Use interrupt bindings instead of gpio for the IRQ
---
 .../bindings/display/bridge/himax,hx8837.txt  | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/himax,hx8837.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/himax,hx8837.txt b/Documentation/devicetree/bindings/display/bridge/himax,hx8837.txt
new file mode 100644
index 000000000000..c52274f05692
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/himax,hx8837.txt
@@ -0,0 +1,44 @@
+HX8837 Display Controller
+
+Required properties:
+- compatible: Should be "himax,hx8837".
+- reg: I2C address, must be 0x0d
+- stat-gpios: gpio specifier of DCON_STAT0 and DCON_STAT1 pins (active high)
+- load-gpios: gpio specifier of DCON_LOAD pin (active high)
+- interrupt: interrupt specifier of DCON_IRQ pin (edge falling)
+
+Required nodes:
+- ports: contains port nodes with endpoints, as described in
+  Documentation/devicetree/bindings/graph.txt
+  Port 0's endpoint is connected to the LCD controller's RGB data output
+  endpoint.
+  Port 1's endpoint is connected to the panel's input endpoint.
+
+Example:
+	dcon@d {
+		compatible = "himax,hx8837";
+		reg = <0x0d>;
+		stat-gpios = <&gpio 100 GPIO_ACTIVE_HIGH
+			      &gpio 101 GPIO_ACTIVE_HIGH>;
+		load-gpios = <&gpio 142 GPIO_ACTIVE_HIGH>;
+		interrupts = <&gpio 124 IRQ_TYPE_EDGE_FALLING>;
+
+		ports {
+			#address-cells = <0x01>;
+			#size-cells = <0x00>;
+
+			port@0 {
+				reg = <0x00>;
+				dcon_rgb_in: endpoint {
+					remote-endpoint = <&lcd0_rgb_out>;
+				};
+			};
+
+			port@1 {
+				reg = <0x01>;
+				dcon_gettl_out: endpoint {
+					remote-endpoint = <&panel_dettl_in>;
+				};
+			};
+		};
+	};
-- 
2.19.1

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

[RFC 08/16] dt-bindings: drm/panel: simple: Add Innolux LS075AT011 bindings

[Lubomir Rintel]

Innolux LS075AT011 panels are used on on OLPC laptops.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 .../bindings/display/panel/innolux,ls075at011.txt          | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/innolux,ls075at011.txt

diff --git a/Documentation/devicetree/bindings/display/panel/innolux,ls075at011.txt b/Documentation/devicetree/bindings/display/panel/innolux,ls075at011.txt
new file mode 100644
index 000000000000..ad5a0c30b05a
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/innolux,ls075at011.txt
@@ -0,0 +1,7 @@
+Innolux LS075AT011 7.5" 1200x900 panel
+
+Required properties:
+- compatible: should be "innolux,ls075at011", "simple-panel"
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
-- 
2.19.1

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

[RFC 09/16] drm/panel: simple: Add support for Innolux LS075AT011

[Lubomir Rintel]

This adds support for Innolux LS075AT011 7.5" 1200x900 panel.

The panel is not a regular LCD with RGB pixels. Its pixels are just of
one color, organized like this:

 g b r
 b r g . . .
 r g b
   .
   .
   .

It's controlled by the Himax HX8837, which takes the RGB data, optionally
dithers the colors by swizzling and drives the panel.  I couldn't determine
what the bus_format is. There's six data pins marked FD00, FD01, FD10,
FD11, FD20 and FD21.

There's no public data sheet. The clock is taken from the OLPC's
OpenFirmware, I don't know why precisely that number. It seems to work.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 drivers/gpu/drm/panel/panel-simple.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index a04ffb3b2174..d084132c3010 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -1326,6 +1326,29 @@ static const struct panel_desc innolux_g121x1_l03 = {
 	},
 };
 
+static const struct drm_display_mode innolux_ls075at011_mode = {
+	.clock = 56930,
+	.hdisplay = 1200,
+	.hsync_start = 1200 + 26,
+	.hsync_end = 1200 + 26 + 6,
+	.htotal = 1200 + 26 + 6 + 24,
+	.vdisplay = 900,
+	.vsync_start = 900 + 4,
+	.vsync_end = 900 + 4 + 3,
+	.vtotal = 900 + 4 + 3 + 5,
+	.vrefresh = 50,
+	.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+};
+
+static const struct panel_desc innolux_ls075at011 = {
+	.modes = &innolux_ls075at011_mode,
+	.num_modes = 1,
+	.size = {
+		.width = 152,
+		.height = 115,
+	},
+};
+
 static const struct drm_display_mode innolux_n116bge_mode = {
 	.clock = 76420,
 	.hdisplay = 1366,
@@ -2449,6 +2472,9 @@ static const struct of_device_id platform_of_match[] = {
 	}, {
 		.compatible = "innolux,g121x1-l03",
 		.data = &innolux_g121x1_l03,
+	}, {
+		.compatible = "innolux,ls075at011",
+		.data = &innolux_ls075at011,
 	}, {
 		.compatible = "innolux,n116bge",
 		.data = &innolux_n116bge,
-- 
2.19.1

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

[RFC 10/16] drm/armada: fix compare_of() for LCD controllers

[Lubomir Rintel]

From: Russell King <rmk+kernel@armlinux.org.uk>

The DT node passed for LCD controllers is the "port" node within the
parent device.  Detect this and compare the parent node.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

---
 drivers/gpu/drm/armada/armada_drv.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
index fa31589b4fc0..9f2eb02df295 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -181,7 +181,10 @@ static void armada_drm_unbind(struct device *dev)
 
 static int compare_of(struct device *dev, void *data)
 {
-	return dev->of_node == data;
+	struct device_node *np = data;
+	if (of_node_cmp(np->name, "port") == 0)
+		np = np->parent;
+	return dev->of_node == np;
 }
 
 static int compare_dev_name(struct device *dev, void *data)
-- 
2.19.1

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

[RFC 11/16] drm/armada: add OF reserved memory support

[Lubomir Rintel]

From: Russell King <rmk+kernel@armlinux.org.uk>

Existing Armada DRM makes use of reserved memory for allocating
contiguous screen buffers, which currently prevents its use with
DT systems.  Add support for this for DT systems.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

---
 drivers/gpu/drm/armada/Makefile      |  3 ++
 drivers/gpu/drm/armada/armada_drv.c  | 24 ++++++++++++--
 drivers/gpu/drm/armada/armada_rmem.c | 49 ++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/armada/armada_rmem.c

diff --git a/drivers/gpu/drm/armada/Makefile b/drivers/gpu/drm/armada/Makefile
index 9bc3c3213724..d34843e121c7 100644
--- a/drivers/gpu/drm/armada/Makefile
+++ b/drivers/gpu/drm/armada/Makefile
@@ -5,3 +5,6 @@ armada-y	+= armada_510.o
 armada-$(CONFIG_DEBUG_FS) += armada_debugfs.o
 
 obj-$(CONFIG_DRM_ARMADA) := armada.o
+
+armada-rmem-$(CONFIG_DRM_ARMADA) += armada_rmem.o
+obj-y += $(armada-rmem-y) $(armada-rmem-m)
diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
index 9f2eb02df295..c1d3cbefd4d8 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -10,6 +10,7 @@
 #include <linux/module.h>
 #include <linux/of_graph.h>
 #include <drm/drm_atomic_helper.h>
+#include <linux/of_reserved_mem.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_of.h>
@@ -75,6 +76,9 @@ static int armada_drm_bind(struct device *dev)
 			return -EINVAL;
 	}
 
+	if (!mem && dev->of_node)
+		mem = dev->platform_data;
+
 	if (!mem)
 		return -ENXIO;
 
@@ -226,9 +230,17 @@ static int armada_drm_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	int ret;
 
-	ret = drm_of_component_probe(dev, compare_dev_name, &armada_master_ops);
-	if (ret != -EINVAL)
+	if (dev->of_node) {
+		ret = of_reserved_mem_device_init(dev);
+		if (ret && ret != -ENODEV)
+			return ret;
+
+		ret = drm_of_component_probe(dev, compare_of,
+					     &armada_master_ops);
+		if (ret)
+			of_reserved_mem_device_release(dev);
 		return ret;
+	}
 
 	if (dev->platform_data) {
 		char **devices = dev->platform_data;
@@ -263,6 +275,7 @@ static int armada_drm_probe(struct platform_device *pdev)
 static int armada_drm_remove(struct platform_device *pdev)
 {
 	component_master_del(&pdev->dev, &armada_master_ops);
+	of_reserved_mem_device_release(&pdev->dev);
 	return 0;
 }
 
@@ -276,11 +289,18 @@ static const struct platform_device_id armada_drm_platform_ids[] = {
 };
 MODULE_DEVICE_TABLE(platform, armada_drm_platform_ids);
 
+static const struct of_device_id armada_drm_dt_ids[] = {
+	{ .compatible = "marvell,dove-display-subsystem", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, armada_drm_dt_ids);
+
 static struct platform_driver armada_drm_platform_driver = {
 	.probe	= armada_drm_probe,
 	.remove	= armada_drm_remove,
 	.driver	= {
 		.name	= "armada-drm",
+		.of_match_table = armada_drm_dt_ids,
 	},
 	.id_table = armada_drm_platform_ids,
 };
diff --git a/drivers/gpu/drm/armada/armada_rmem.c b/drivers/gpu/drm/armada/armada_rmem.c
new file mode 100644
index 000000000000..36bb20e426b6
--- /dev/null
+++ b/drivers/gpu/drm/armada/armada_rmem.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2017 Russell King
+#include <linux/errno.h>
+#include <linux/of.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/slab.h>
+
+static int armada_rmem_dev_init(struct reserved_mem *rmem, struct device *dev)
+{
+	struct resource *r;
+
+	if (dev->platform_data)
+		return -EBUSY;
+
+	r = kzalloc(sizeof(*r), GFP_KERNEL);
+	if (!r)
+		return -ENOMEM;
+
+	r->start = rmem->base;
+	r->end = rmem->base + rmem->size - 1;
+	r->flags = IORESOURCE_MEM;
+
+	rmem->priv = r;
+	dev->platform_data = r;
+
+	return 0;
+}
+
+static void armada_rmem_dev_release(struct reserved_mem *rmem,
+	struct device *dev)
+{
+	kfree(rmem->priv);
+	rmem->priv = NULL;
+	dev->platform_data = NULL;
+}
+
+static const struct reserved_mem_ops armada_rmem_ops = {
+	.device_init = armada_rmem_dev_init,
+	.device_release = armada_rmem_dev_release,
+};
+
+static int __init armada_rmem_init(struct reserved_mem *rmem)
+{
+	rmem->ops = &armada_rmem_ops;
+	return 0;
+}
+
+RESERVEDMEM_OF_DECLARE(armada_rmem, "marvell,dove-framebuffer",
+			armada_rmem_init);
-- 
2.19.1

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

[RFC 12/16] drm/armada: add bus-width property to the output endpoint

[Lubomir Rintel]

This makes it possible to choose a different pixel format for the
endpoint. Modelled after what other LCD controllers use, including
marvell,pxa2xx-lcdc and atmel,hlcdc-display-controller and perhaps more.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 drivers/gpu/drm/armada/armada_crtc.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
index da9360688b55..5400fb925bcd 100644
--- a/drivers/gpu/drm/armada/armada_crtc.c
+++ b/drivers/gpu/drm/armada/armada_crtc.c
@@ -724,6 +724,8 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev,
 	struct armada_private *priv = drm->dev_private;
 	struct armada_crtc *dcrtc;
 	struct drm_plane *primary;
+	struct device_node *endpoint;
+	u32 bus_width = 24;
 	void __iomem *base;
 	int ret;
 
@@ -744,8 +746,30 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev,
 	dcrtc->base = base;
 	dcrtc->num = drm->mode_config.num_crtc;
 	dcrtc->clk = ERR_PTR(-EINVAL);
-	dcrtc->cfg_dumb_ctrl = DUMB24_RGB888_0;
 	dcrtc->spu_iopad_ctrl = CFG_VSCALE_LN_EN | CFG_IOPAD_DUMB24;
+
+	endpoint = of_get_next_child(port, NULL);
+	of_property_read_u32(endpoint, "bus-width", &bus_width);
+	of_node_put(endpoint);
+
+	switch (bus_width) {
+	case 12:
+		dcrtc->cfg_dumb_ctrl = DUMB12_RGB444_0;
+		break;
+	case 16:
+		dcrtc->cfg_dumb_ctrl = DUMB16_RGB565_0;
+		break;
+	case 18:
+		dcrtc->cfg_dumb_ctrl = DUMB18_RGB666_0;
+		break;
+	case 24:
+		dcrtc->cfg_dumb_ctrl = DUMB24_RGB888_0;
+		break;
+	default:
+		DRM_ERROR("unsupported bus width: %d\n", bus_width);
+		return -EINVAL;
+	}
+
 	spin_lock_init(&dcrtc->irq_lock);
 	dcrtc->irq_ena = CLEAN_SPU_IRQ_ISR;
 
-- 
2.19.1

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

[RFC 13/16] drm/armada: replace the simple-framebuffer

[Lubomir Rintel]

If there's a simple-framebuffer carried over from boot firmware, it's going
to stop working once we setup the LCDC for use via DRM. Kick it off from
the hardware.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 drivers/gpu/drm/armada/armada_drv.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
index c1d3cbefd4d8..deef34153c56 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -104,6 +104,17 @@ static int armada_drm_bind(struct device *dev)
 		return ret;
 	}
 
+	/* Remove early framebuffers */
+	ret = drm_fb_helper_remove_conflicting_framebuffers(NULL,
+							    "armada-drm-fb",
+							    false);
+	if (ret) {
+		dev_err(dev, "[" DRM_NAME ":%s] can't kick out simple-fb: %d\n",
+			__func__, ret);
+		kfree(priv);
+		return ret;
+	}
+
 	priv->drm.dev_private = priv;
 
 	dev_set_drvdata(dev, &priv->drm);
-- 
2.19.1

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

[RFC 14/16] drm/armada: optionally enable the AXI clock

[Lubomir Rintel]

It needs to be enabled (at least on MMP2) in order for the register
writes to LCDC to work.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 drivers/gpu/drm/armada/armada_crtc.c | 7 +++++++
 drivers/gpu/drm/armada/armada_crtc.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
index 5400fb925bcd..973c377975a1 100644
--- a/drivers/gpu/drm/armada/armada_crtc.c
+++ b/drivers/gpu/drm/armada/armada_crtc.c
@@ -679,6 +679,7 @@ static void armada_drm_crtc_destroy(struct drm_crtc *crtc)
 
 	of_node_put(dcrtc->crtc.port);
 
+	clk_disable_unprepare(dcrtc->axiclk);
 	kfree(dcrtc);
 }
 
@@ -748,6 +749,11 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev,
 	dcrtc->clk = ERR_PTR(-EINVAL);
 	dcrtc->spu_iopad_ctrl = CFG_VSCALE_LN_EN | CFG_IOPAD_DUMB24;
 
+	dcrtc->axiclk = devm_clk_get(dev, "axiclk");
+	if (IS_ERR(dcrtc->axiclk))
+		dcrtc->axiclk = NULL;
+	WARN_ON(clk_prepare_enable(dcrtc->axiclk));
+
 	endpoint = of_get_next_child(port, NULL);
 	of_property_read_u32(endpoint, "bus-width", &bus_width);
 	of_node_put(endpoint);
@@ -829,6 +835,7 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev,
 err_crtc_init:
 	primary->funcs->destroy(primary);
 err_crtc:
+	clk_disable_unprepare(dcrtc->axiclk);
 	kfree(dcrtc);
 
 	return ret;
diff --git a/drivers/gpu/drm/armada/armada_crtc.h b/drivers/gpu/drm/armada/armada_crtc.h
index 7ebd337b60af..b07faea7257d 100644
--- a/drivers/gpu/drm/armada/armada_crtc.h
+++ b/drivers/gpu/drm/armada/armada_crtc.h
@@ -39,6 +39,7 @@ struct armada_crtc {
 	const struct armada_variant *variant;
 	unsigned		num;
 	void __iomem		*base;
+	struct clk		*axiclk;
 	struct clk		*clk;
 	struct clk		*extclk[2];
 	struct {
-- 
2.19.1

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

[RFC 15/16] drm/armada: add mmp2 support

[Lubomir Rintel]

This pretty much boils down to setting the LCD_CFG_SCLK_DIV register, and
is tailored to the OLPC XO-1.75. I have no idea what are the meanings of
most bits there, so I'm just making sure it ends up being 0x40001102.

This means that the selection of the source clock is hardwired. Apparently
the bit 30 selects the AXI bus clock as base clock for the pixel clock.
It is not known to me what other options are there.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>

---
Notes; perhaps James or someone else with a datasheet could help me with
this.

I'm somewhat confused about the clock selection. The firmware contains
this line:

  h# 00000700 value pmua-disp-clk-sel  \ PLL1 / 7 -> 113.86 MHz

However, the Linux clock driver seems to consider the value of 7 to
configure the divisor to 8, so the resulting clock would end up being
100 MHz. Also, the clk-of-mmp2 driver suggests PLL1 outputs 800 MHz,
dividing that by 7 would end up being 114.29 MHz, not 113.86 MHz.

If the same logic was used here as Armada 510 driver uses, we'll end up
with the divisor of 1 in LCD_CFG_SCLK_DIV and disp0_div divisor of 16.
Would that be good also? (Do perhaps any of the bits in LCD_CFG_SCLK_DIV
allow for a fractional divisor, allowing us to get the clock we need
more precisely?)

The pxa168fb driver as used on the stock OLPC software just avoids
touching the register, preserving the value set from the firmware.
---
 drivers/gpu/drm/armada/Makefile      |  1 +
 drivers/gpu/drm/armada/armada_610.c  | 50 ++++++++++++++++++++++++++++
 drivers/gpu/drm/armada/armada_crtc.c |  4 +++
 drivers/gpu/drm/armada/armada_drm.h  |  1 +
 drivers/gpu/drm/armada/armada_drv.c  |  1 +
 drivers/gpu/drm/armada/armada_rmem.c |  4 ++-
 6 files changed, 60 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/armada/armada_610.c

diff --git a/drivers/gpu/drm/armada/Makefile b/drivers/gpu/drm/armada/Makefile
index d34843e121c7..ac299fe35173 100644
--- a/drivers/gpu/drm/armada/Makefile
+++ b/drivers/gpu/drm/armada/Makefile
@@ -2,6 +2,7 @@
 armada-y	:= armada_crtc.o armada_drv.o armada_fb.o armada_fbdev.o \
 		   armada_gem.o armada_overlay.o armada_plane.o armada_trace.o
 armada-y	+= armada_510.o
+armada-y	+= armada_610.o
 armada-$(CONFIG_DEBUG_FS) += armada_debugfs.o
 
 obj-$(CONFIG_DRM_ARMADA) := armada.o
diff --git a/drivers/gpu/drm/armada/armada_610.c b/drivers/gpu/drm/armada/armada_610.c
new file mode 100644
index 000000000000..e86b9fed9d2f
--- /dev/null
+++ b/drivers/gpu/drm/armada/armada_610.c
@@ -0,0 +1,50 @@
+/*
+ * Copyright (C) 2012 Russell King
+ * Copyright (C) 2018 Lubomir Rintel
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Armada MMP2 variant support
+ */
+#include <linux/clk.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_crtc.h>
+#include "armada_crtc.h"
+#include "armada_drm.h"
+
+/*
+ * This gets called with sclk = NULL to test whether the mode is
+ * supportable, and again with sclk != NULL to set the clocks up for
+ * that.  The former can return an error, but the latter is expected
+ * not to.
+ */
+static int armada610_crtc_compute_clock(struct armada_crtc *dcrtc,
+	const struct drm_display_mode *mode, uint32_t *sclk)
+{
+	struct clk *clk = dcrtc->axiclk;
+	uint32_t rate, ref, div;
+
+	if (!clk)
+		return -EINVAL;
+
+	rate = mode->clock * 1000;
+	ref = clk_get_rate(clk);
+	div = DIV_ROUND_UP(ref, rate);
+
+	if (div < 2)
+		return -EINVAL;
+
+	if (sclk) {
+		*sclk = 0x00001100; /* No idea */
+		*sclk |= (0x1 << 30); /* SCLK_SOURCE_SELECT = AXI bus clk */
+		*sclk |= div;
+	}
+
+	return 0;
+}
+
+const struct armada_variant armada610_ops = {
+	.compute_clock = armada610_crtc_compute_clock,
+};
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
index 973c377975a1..9d450669faea 100644
--- a/drivers/gpu/drm/armada/armada_crtc.c
+++ b/drivers/gpu/drm/armada/armada_crtc.c
@@ -915,6 +915,10 @@ static const struct of_device_id armada_lcd_of_match[] = {
 		.compatible	= "marvell,dove-lcd",
 		.data		= &armada510_ops,
 	},
+	{
+		.compatible	= "marvell,mmp2-lcd",
+		.data		= &armada610_ops,
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, armada_lcd_of_match);
diff --git a/drivers/gpu/drm/armada/armada_drm.h b/drivers/gpu/drm/armada/armada_drm.h
index f09083ff15d3..7cbcf33d0304 100644
--- a/drivers/gpu/drm/armada/armada_drm.h
+++ b/drivers/gpu/drm/armada/armada_drm.h
@@ -52,6 +52,7 @@ struct armada_variant {
 
 /* Variant ops */
 extern const struct armada_variant armada510_ops;
+extern const struct armada_variant armada610_ops;
 
 struct armada_private {
 	struct drm_device	drm;
diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
index deef34153c56..fed590e0916f 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -302,6 +302,7 @@ MODULE_DEVICE_TABLE(platform, armada_drm_platform_ids);
 
 static const struct of_device_id armada_drm_dt_ids[] = {
 	{ .compatible = "marvell,dove-display-subsystem", },
+	{ .compatible = "marvell,mmp2-display-subsystem", },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, armada_drm_dt_ids);
diff --git a/drivers/gpu/drm/armada/armada_rmem.c b/drivers/gpu/drm/armada/armada_rmem.c
index 36bb20e426b6..45f8b2eff822 100644
--- a/drivers/gpu/drm/armada/armada_rmem.c
+++ b/drivers/gpu/drm/armada/armada_rmem.c
@@ -45,5 +45,7 @@ static int __init armada_rmem_init(struct reserved_mem *rmem)
 	return 0;
 }
 
-RESERVEDMEM_OF_DECLARE(armada_rmem, "marvell,dove-framebuffer",
+RESERVEDMEM_OF_DECLARE(armada_dove_rmem, "marvell,dove-framebuffer",
+			armada_rmem_init);
+RESERVEDMEM_OF_DECLARE(armada_mmp2_rmem, "marvell,mmp2-framebuffer",
 			armada_rmem_init);
-- 
2.19.1

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

[RFC 16/16] drm/i2c: hx8837: add a Himax HX8837 display controller driver

[Lubomir Rintel]

Himax HX8837 is used to drive the LCD panel on OLPC platforms. It controls
backlight and is able to capture and freeze a frame when the LCD controller
(and the rest of the plaform) is powered off.

This driver is based on the same code as drivers/staging/olpc_dcon.
I modernized it to use managed GPIO, device-tree bindings, sysfs attribtue
groups, essentially fixing the staging driver's TODO. I've also converted
it to a DRM encoded driver.

Why I am not removing the staging driver now is because I've hobbled off
some functionality too (with an intent to bring add it back eventually):

* I've removed parts that talk directly to the OLPC EC to turn off/on the
  power. A separate patch will make the EC expose a regulator interface
  that should be used instead.

* Some work is likely needed to make XO 1 and XO 1.5 work. Both platforms
  are DT-based and could use the same bindings, but I haven't checked
  whether the cs5535 and vx855 GPIO drivers are good enough.
  Also, both machines only have a fbdev driver while this plugs into the
  DRM infrastructure. There's an out-of-tree "openchrome" driver that
  could work on the 1.5 though.

I've also renamed the driver to use the actual chip name instead of the
original name that I found too generic. This way the staging driver can be
used on XO 1 and XO 1.5 for the time being, while my XO 1.75 can utilize
this one.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>

---
FIXME: there are checkpatch complains, the sysfs files either need to go
or get det documented, etc.

Changes since v3:
- Turned into a DRM encoder

Changes since v2:
- s/controlls/controls/
- Rob Herring's Reviewed-by tag

Changes since v1:
- Use interrupt bindings instead of gpio for the IRQ
- Update the statement on XO 1/1.5 support in the commit message
---
 arch/arm/configs/olpc_xo175_defconfig |   1 -
 drivers/gpu/drm/i2c/Kconfig           |  13 +
 drivers/gpu/drm/i2c/Makefile          |   2 +
 drivers/gpu/drm/i2c/hx8837.c          | 780 ++++++++++++++++++++++++++
 4 files changed, 795 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/i2c/hx8837.c

diff --git a/arch/arm/configs/olpc_xo175_defconfig b/arch/arm/configs/olpc_xo175_defconfig
index 8c869bd686b2..6ebe342501a5 100644
--- a/arch/arm/configs/olpc_xo175_defconfig
+++ b/arch/arm/configs/olpc_xo175_defconfig
@@ -142,7 +142,6 @@ CONFIG_FB_MODE_HELPERS=y
 CONFIG_FB_SIMPLE=y
 CONFIG_BACKLIGHT_LCD_SUPPORT=y
 CONFIG_LCD_CLASS_DEVICE=y
-CONFIG_BACKLIGHT_CLASS_DEVICE=y
 CONFIG_BACKLIGHT_MAX8925=y
 CONFIG_FRAMEBUFFER_CONSOLE=y
 CONFIG_LOGO=y
diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig
index 65d3acb61c03..96b0f7924cdb 100644
--- a/drivers/gpu/drm/i2c/Kconfig
+++ b/drivers/gpu/drm/i2c/Kconfig
@@ -32,4 +32,17 @@ config DRM_I2C_NXP_TDA9950
 	select CEC_NOTIFIER
 	select CEC_CORE
 
+config DRM_I2C_HX8837
+	tristate "HiMax HX8837 OLPC Display Controller"
+	select BACKLIGHT_LCD_SUPPORT
+	select BACKLIGHT_CLASS_DEVICE
+	help
+	  Support for HiMax HX8837 Display Controller on an OLPC XO laptop.
+
+	  The HX8837 encodes R5G6B6 signal from the CRTC for panel's DETTL
+	  interface, optionally enabing color swizzling. It is also capable
+	  of freezing a single frame with LCD controller turned off for low
+	  power operation and controlling the backlight.
+
+	  If your laptop doesn't have green ears, say "N"
 endmenu
diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile
index a962f6f08568..142e7dd006c2 100644
--- a/drivers/gpu/drm/i2c/Makefile
+++ b/drivers/gpu/drm/i2c/Makefile
@@ -8,3 +8,5 @@ obj-$(CONFIG_DRM_I2C_SIL164) += sil164.o
 tda998x-y := tda998x_drv.o
 obj-$(CONFIG_DRM_I2C_NXP_TDA998X) += tda998x.o
 obj-$(CONFIG_DRM_I2C_NXP_TDA9950) += tda9950.o
+
+obj-$(CONFIG_DRM_I2C_HX8837) += hx8837.o
diff --git a/drivers/gpu/drm/i2c/hx8837.c b/drivers/gpu/drm/i2c/hx8837.c
new file mode 100644
index 000000000000..755ec70a979c
--- /dev/null
+++ b/drivers/gpu/drm/i2c/hx8837.c
@@ -0,0 +1,780 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Mainly by David Woodhouse, somewhat modified by Jordan Crouse.
+ * Modernized to use managed GPIO, device-tree, etc. by Lubomir Rintel.
+ *
+ * Copyright (C) 2006-2007  Red Hat, Inc.
+ * Copyright (C) 2006-2007  Advanced Micro Devices, Inc.
+ * Copyright (C) 2009       VIA Technology, Inc.
+ * Copyright (C) 2010-2011  Andres Salomon <dilinger@queued.net>
+ * Copyright (C) 2018       Lubomir Rintel <lkundrak@v3.sk>
+ */
+
+#include <linux/module.h>
+#include <linux/backlight.h>
+#include <linux/console.h>
+#include <linux/ctype.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/notifier.h>
+#include <linux/reboot.h>
+#include <linux/component.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_encoder.h>
+#include <drm/drm_connector.h>
+#include <drm/drm_of.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+
+/* DCON registers */
+
+#define DCON_REG_ID		 0
+#define DCON_REG_MODE		 1
+
+#define MODE_PASSTHRU		BIT(0)
+#define MODE_SLEEP		BIT(1)
+#define MODE_SLEEP_AUTO		BIT(2)
+#define MODE_BL_ENABLE		BIT(3)
+#define MODE_BLANK		BIT(4)
+#define MODE_CSWIZZLE		BIT(5)
+#define MODE_COL_AA		BIT(6)
+#define MODE_MONO_LUMA		BIT(7)
+#define MODE_SCAN_INT		BIT(8)
+#define MODE_CLOCKDIV		BIT(9)
+#define MODE_DEBUG		BIT(14)
+#define MODE_SELFTEST		BIT(15)
+
+#define DCON_REG_HRES		0x2
+#define DCON_REG_HTOTAL		0x3
+#define DCON_REG_HSYNC_WIDTH	0x4
+#define DCON_REG_VRES		0x5
+#define DCON_REG_VTOTAL		0x6
+#define DCON_REG_VSYNC_WIDTH	0x7
+#define DCON_REG_TIMEOUT	0x8
+#define DCON_REG_SCAN_INT	0x9
+#define DCON_REG_BRIGHT		0xa
+#define DCON_REG_MEM_OPT_A	0x41
+#define DCON_REG_MEM_OPT_B	0x42
+
+/* Load Delay Locked Loop (DLL) settings for clock delay */
+#define MEM_DLL_CLOCK_DELAY	BIT(0)
+/* Memory controller power down function */
+#define MEM_POWER_DOWN		BIT(8)
+/* Memory controller software reset */
+#define MEM_SOFT_RESET		BIT(0)
+
+/* Status values */
+
+#define DCONSTAT_SCANINT	0
+#define DCONSTAT_SCANINT_DCON	1
+#define DCONSTAT_DISPLAYLOAD	2
+#define DCONSTAT_MISSED		3
+
+/* Source values */
+
+#define DCON_SOURCE_DCON        0
+#define DCON_SOURCE_CPU         1
+
+struct hx8837_priv {
+	struct i2c_client *client;
+	struct backlight_device *bl_dev;
+
+	wait_queue_head_t waitq;
+	struct work_struct switch_source;
+	struct notifier_block panic_nb;
+
+	/* Scanline to interrupt on during resume */
+	ushort resumeline;
+
+	/* Shadow register for the DCON_REG_MODE register */
+	u8 disp_mode;
+
+	/* The current backlight value - this saves us some smbus traffic */
+	u8 bl_val;
+
+	/* Current source, initialized at probe time */
+	int curr_src;
+
+	/* Desired source */
+	int pending_src;
+
+	/* Variables used during switches */
+	bool switched;
+	ktime_t irq_time;
+	ktime_t load_time;
+
+	/* Current output type; true == mono, false == color */
+	bool mono;
+	/* This get set while controlling fb blank state from the driver */
+	bool ignore_fb_events;
+
+	struct gpio_desc *stat0_gpio;
+	struct gpio_desc *stat1_gpio;
+	struct gpio_desc *load_gpio;
+
+        struct drm_panel *panel;
+
+	struct drm_encoder encoder;
+	struct drm_connector connector;
+};
+
+static irqreturn_t hx8837_interrupt(int irq, void *id);
+
+/* I2C structures */
+
+static unsigned short normal_i2c[] = { 0x0d, I2C_CLIENT_END };
+
+static s32 hx8837_write(struct hx8837_priv *priv, u8 reg, u16 val)
+{
+	return i2c_smbus_write_word_data(priv->client, reg, val);
+}
+
+static s32 hx8837_read(struct hx8837_priv *priv, u8 reg)
+{
+	return i2c_smbus_read_word_data(priv->client, reg);
+}
+
+static void hx8837_set_backlight(struct hx8837_priv *priv, u8 level)
+{
+	hx8837_write(priv, DCON_REG_BRIGHT, level);
+
+	/* Purposely turn off the backlight when we go to level 0 */
+	if (level == 0) {
+		priv->disp_mode &= ~MODE_BL_ENABLE;
+		hx8837_write(priv, DCON_REG_MODE, priv->disp_mode);
+	} else if (!(priv->disp_mode & MODE_BL_ENABLE)) {
+		priv->disp_mode |= MODE_BL_ENABLE;
+		hx8837_write(priv, DCON_REG_MODE, priv->disp_mode);
+	}
+}
+
+/* Set the output type to either color or mono */
+static int hx8837_set_mono_mode(struct hx8837_priv *priv, bool enable_mono)
+{
+	if (priv->mono == enable_mono)
+		return 0;
+
+	priv->mono = enable_mono;
+
+	if (enable_mono) {
+		priv->disp_mode &= ~(MODE_CSWIZZLE | MODE_COL_AA);
+		priv->disp_mode |= MODE_MONO_LUMA;
+	} else {
+		priv->disp_mode &= ~(MODE_MONO_LUMA);
+		priv->disp_mode |= MODE_CSWIZZLE | MODE_COL_AA;
+	}
+
+	hx8837_write(priv, DCON_REG_MODE, priv->disp_mode);
+	return 0;
+}
+
+/* the DCON seems to get confused if we change DCONLOAD too
+ * frequently -- i.e., approximately faster than frame time.
+ * normally we don't change it this fast, so in general we won't
+ * delay here.
+ */
+static void hx8837_load_holdoff(struct hx8837_priv *priv)
+{
+	ktime_t delta_t, now;
+
+	while (1) {
+		now = ktime_get();
+		delta_t = ktime_sub(now, priv->load_time);
+		if (ktime_to_ns(delta_t) > NSEC_PER_MSEC * 20)
+			break;
+		mdelay(4);
+	}
+}
+
+/* Set the source of the display (CPU or DCON) */
+static void hx8837_source_switch(struct work_struct *work)
+{
+	struct hx8837_priv *priv = container_of(work, struct hx8837_priv,
+							switch_source);
+	struct device *dev = &priv->client->dev;
+	int source = priv->pending_src;
+
+	if (priv->curr_src == source)
+		return;
+
+	hx8837_load_holdoff(priv);
+
+	priv->switched = false;
+
+	switch (source) {
+	case DCON_SOURCE_CPU:
+		dev_info(dev, "%s to CPU\n", __func__);
+		/* Enable the scanline interrupt bit */
+		if (hx8837_write(priv, DCON_REG_MODE,
+			       priv->disp_mode | MODE_SCAN_INT))
+			dev_err(dev, "couldn't enable scanline interrupt!\n");
+		else
+			/* Wait up to one second for the scanline interrupt */
+			wait_event_timeout(priv->waitq, priv->switched, HZ);
+
+		if (!priv->switched)
+			dev_err(dev, "Timeout entering CPU mode; expect a screen glitch.\n");
+
+		/* Turn off the scanline interrupt */
+		if (hx8837_write(priv, DCON_REG_MODE, priv->disp_mode))
+			dev_err(dev, "couldn't disable scanline interrupt!\n");
+
+		/* And turn off the DCON */
+		gpiod_set_value(priv->load_gpio, 1);
+		priv->load_time = ktime_get();
+
+		dev_info(dev, "The CPU has control\n");
+		break;
+	case DCON_SOURCE_DCON:
+		dev_info(dev, "%s to DCON\n", __func__);
+
+		/* Clear DCONLOAD - this implies that the DCON is in control */
+		gpiod_set_value(priv->load_gpio, 0);
+		priv->load_time = ktime_get();
+
+		wait_event_timeout(priv->waitq, priv->switched, HZ / 2);
+
+		if (!priv->switched) {
+			dev_err(dev, "Timeout entering DCON mode; expect a screen glitch.\n");
+		} else {
+			ktime_t delta_t;
+
+			/* sometimes the DCON doesn't follow its own rules,
+			 * and doesn't wait for two vsync pulses before
+			 * ack'ing the frame load with an IRQ.  the result
+			 * is that the display shows the *previously*
+			 * loaded frame.  we can detect this by looking at
+			 * the time between asserting DCONLOAD and the IRQ --
+			 * if it's less than 20msec, then the DCON couldn't
+			 * have seen two VSYNC pulses.  in that case we
+			 * deassert and reassert, and hope for the best.
+			 * see http://dev.laptop.org/ticket/9664
+			 */
+			delta_t = ktime_sub(priv->irq_time, priv->load_time);
+			if (priv->switched && ktime_to_ns(delta_t)
+			    < NSEC_PER_MSEC * 20) {
+				dev_err(dev, "missed loading, retrying\n");
+				gpiod_set_value(priv->load_gpio, 1);
+				mdelay(41);
+				gpiod_set_value(priv->load_gpio, 0);
+				priv->load_time = ktime_get();
+				mdelay(41);
+			}
+		}
+
+		dev_info(dev, "The DCON has control\n");
+		break;
+	default:
+		WARN_ON(1);
+	}
+
+	priv->curr_src = source;
+}
+
+static void hx8837_set_source(struct hx8837_priv *priv, int arg)
+{
+	if (priv->pending_src == arg)
+		return;
+
+	priv->pending_src = arg;
+
+	if (priv->curr_src != arg)
+		schedule_work(&priv->switch_source);
+}
+
+static void hx8837_set_source_sync(struct hx8837_priv *priv, int arg)
+{
+	hx8837_set_source(priv, arg);
+	flush_scheduled_work();
+}
+
+static int hx8837_bl_update(struct backlight_device *dev)
+{
+	struct hx8837_priv *priv = bl_get_data(dev);
+	u8 level = dev->props.brightness & 0x0F;
+
+	priv->bl_val = level;
+
+	if (dev->props.power != FB_BLANK_UNBLANK)
+		level = 0;
+
+	if (dev->props.state & BL_CORE_FBBLANK)
+		level = 0;
+
+	hx8837_set_backlight(priv, level);
+
+	return 0;
+}
+
+static int hx8837_bl_get(struct backlight_device *dev)
+{
+	struct hx8837_priv *priv = bl_get_data(dev);
+
+	return priv->bl_val;
+}
+
+static const struct backlight_ops hx8837_bl_ops = {
+	.update_status = hx8837_bl_update,
+	.get_brightness = hx8837_bl_get,
+};
+
+static struct backlight_properties hx8837_bl_props = {
+	.max_brightness = 15,
+	.type = BACKLIGHT_RAW,
+	.power = FB_BLANK_UNBLANK,
+};
+
+static int hx8837_panic_notify(struct notifier_block *nb, unsigned long e,
+								void *p)
+{
+	struct hx8837_priv *priv = container_of(nb, struct hx8837_priv,
+								panic_nb);
+
+	gpiod_set_value(priv->load_gpio, 1);
+	return NOTIFY_DONE;
+}
+
+static int hx8837_detect(struct i2c_client *client, struct i2c_board_info *info)
+{
+	strlcpy(info->type, "hx8837", I2C_NAME_SIZE);
+
+	return 0;
+}
+
+static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
+								char *buf)
+{
+	struct hx8837_priv *priv = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%4.4X\n", priv->disp_mode);
+}
+
+static ssize_t freeze_show(struct device *dev, struct device_attribute *attr,
+								char *buf)
+{
+	struct hx8837_priv *priv = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", priv->curr_src == DCON_SOURCE_DCON ? 1 : 0);
+}
+
+static ssize_t monochrome_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct hx8837_priv *priv = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", priv->mono);
+}
+
+static ssize_t monochrome_store(struct device *dev,
+			struct device_attribute *attr,
+			const char *buf, size_t count)
+{
+	unsigned long enable_mono;
+	int rc;
+
+	rc = kstrtoul(buf, 10, &enable_mono);
+	if (rc)
+		return rc;
+
+	hx8837_set_mono_mode(dev_get_drvdata(dev), enable_mono ? true : false);
+
+	return count;
+}
+
+static ssize_t freeze_store(struct device *dev, struct device_attribute *attr,
+						const char *buf, size_t count)
+{
+	struct hx8837_priv *priv = dev_get_drvdata(dev);
+	unsigned long output;
+	int ret;
+
+	ret = kstrtoul(buf, 10, &output);
+	if (ret)
+		return ret;
+
+	switch (output) {
+	case 0:
+		hx8837_set_source(priv, DCON_SOURCE_CPU);
+		break;
+	case 1:
+		hx8837_set_source_sync(priv, DCON_SOURCE_DCON);
+		break;
+	case 2:  /* normally unused */
+		hx8837_set_source(priv, DCON_SOURCE_DCON);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return count;
+}
+
+static DEVICE_ATTR_RO(mode);
+static DEVICE_ATTR_RW(freeze);
+static DEVICE_ATTR_RW(monochrome);
+
+static struct attribute *hx8837_attrs[] = {
+	&dev_attr_mode.attr,
+	&dev_attr_freeze.attr,
+	&dev_attr_monochrome.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(hx8837);
+
+static void hx8837_encoder_destroy(struct drm_encoder *encoder)
+{
+	drm_encoder_cleanup(encoder);
+}
+
+static const struct drm_encoder_funcs hx8837_encoder_funcs = {
+	.destroy = hx8837_encoder_destroy,
+};
+
+static void hx8837_encoder_helper_enable(struct drm_encoder *encoder)
+{
+	struct hx8837_priv *priv = container_of(encoder, struct hx8837_priv, encoder);
+
+	priv->disp_mode |= MODE_BL_ENABLE;
+	hx8837_write(priv, DCON_REG_MODE, priv->disp_mode);
+}
+
+static void hx8837_encoder_helper_disable(struct drm_encoder *encoder)
+{
+	struct hx8837_priv *priv = container_of(encoder, struct hx8837_priv, encoder);
+
+	priv->disp_mode &= ~MODE_BL_ENABLE;
+	hx8837_write(priv, DCON_REG_MODE, priv->disp_mode);
+}
+
+static const struct drm_encoder_helper_funcs hx8837_encoder_helper_funcs = {
+	.enable = hx8837_encoder_helper_enable,
+	.disable = hx8837_encoder_helper_disable,
+};
+
+static const struct drm_connector_funcs hx8837_connector_funcs = {
+	.dpms = drm_helper_connector_dpms,
+	.reset = drm_atomic_helper_connector_reset,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = drm_connector_cleanup,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static int hx8837_connector_get_modes(struct drm_connector *connector)
+{
+	struct hx8837_priv *priv = container_of(connector, struct hx8837_priv, connector);
+
+	return priv->panel->funcs->get_modes(priv->panel);
+}
+
+static struct drm_encoder *
+hx8837_connector_best_encoder(struct drm_connector *connector)
+{
+	struct hx8837_priv *priv = container_of(connector, struct hx8837_priv, connector);
+
+	return &priv->encoder;
+}
+
+static const struct drm_connector_helper_funcs hx8837_connector_helper_funcs = {
+	.get_modes = hx8837_connector_get_modes,
+	.best_encoder = hx8837_connector_best_encoder,
+};
+
+static int hx8837_bind(struct device *dev, struct device *master, void *data)
+{
+	struct hx8837_priv *priv = dev_get_drvdata(dev);
+	struct drm_device *drm = data;
+	int ret;
+
+	priv->encoder.possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
+
+	drm_encoder_helper_add(&priv->encoder, &hx8837_encoder_helper_funcs);
+	ret = drm_encoder_init(drm, &priv->encoder, &hx8837_encoder_funcs, DRM_MODE_ENCODER_LVDS, NULL);
+	if (ret) {
+		dev_err(dev, "failed to init encoder\n");
+		return ret;
+	}
+
+	priv->connector.dpms = DRM_MODE_DPMS_OFF;
+	priv->connector.polled = 0;
+	drm_connector_helper_add(&priv->connector, &hx8837_connector_helper_funcs);
+	ret = drm_connector_init(drm, &priv->connector, &hx8837_connector_funcs, DRM_MODE_CONNECTOR_LVDS);
+	if (ret) {
+		dev_err(dev, "failed to init connector\n");
+		return ret;
+	}
+
+	ret = drm_panel_attach(priv->panel, &priv->connector);
+	if (ret) {
+		dev_err(dev, "failed to attach panel\n");
+		return ret;
+	}
+
+	ret = drm_connector_attach_encoder(&priv->connector, &priv->encoder);
+	if (ret) {
+		dev_err(dev, "failed to attach connector\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static void hx8837_unbind(struct device *dev, struct device *master,
+			   void *data)
+{
+	struct hx8837_priv *priv = dev_get_drvdata(dev);
+
+	drm_encoder_cleanup(&priv->encoder);
+}
+
+static const struct component_ops hx8837_ops = {
+	.bind = hx8837_bind,
+	.unbind = hx8837_unbind,
+};
+
+static int hx8837_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct hx8837_priv *priv;
+	struct drm_panel *panel;
+	u16 ver;
+	int rc;
+
+	rc = drm_of_find_panel_or_bridge(client->dev.of_node, 1, 0, &panel, NULL);
+	if (rc) {
+
+		if (rc != -EPROBE_DEFER)
+			dev_err(&client->dev, "no panel connected\n");
+		return rc;
+	}
+
+	priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	dev_set_drvdata(&client->dev, priv);
+
+	priv->client = client;
+	init_waitqueue_head(&priv->waitq);
+	INIT_WORK(&priv->switch_source, hx8837_source_switch);
+	priv->panic_nb.notifier_call = hx8837_panic_notify;
+	priv->panel = panel;
+
+	i2c_set_clientdata(client, priv);
+
+	priv->stat0_gpio = devm_gpiod_get_index(&client->dev, "stat", 0,
+								GPIOD_IN);
+	if (IS_ERR(priv->stat0_gpio)) {
+		dev_err(&client->dev, "failed to request STAT0 GPIO\n");
+		return PTR_ERR(priv->stat0_gpio);
+	};
+
+	priv->stat1_gpio = devm_gpiod_get_index(&client->dev, "stat", 1,
+								GPIOD_IN);
+	if (IS_ERR(priv->stat1_gpio)) {
+		dev_err(&client->dev, "failed to request STAT1 GPIO\n");
+		return PTR_ERR(priv->stat1_gpio);
+	};
+
+	priv->load_gpio = devm_gpiod_get(&client->dev, "load", GPIOD_IN);
+	if (IS_ERR(priv->load_gpio)) {
+		dev_err(&client->dev, "failed to request LOAD GPIO\n");
+		return PTR_ERR(priv->load_gpio);
+	};
+
+	ver = hx8837_read(priv, DCON_REG_ID);
+	if ((ver >> 8) != 0xDC) {
+		dev_err(&client->dev, "DCON ID not 0xDCxx: 0x%04x.\n", ver);
+		return -ENXIO;
+	}
+
+	dev_info(&client->dev, "Discovered DCON version %x\n", ver & 0xFF);
+
+	if (ver < 0xdc02) {
+		dev_err(&client->dev, "DCON v1 is unsupported, giving up.\n");
+		return -ENODEV;
+	}
+
+	/*
+	 * Determine the current state by reading the GPIO bit; earlier
+	 * stages of the boot process have established the state.
+	 */
+	priv->curr_src = gpiod_get_value(priv->load_gpio)
+			? DCON_SOURCE_CPU : DCON_SOURCE_DCON;
+	priv->pending_src = priv->curr_src;
+
+	/* Set the directions for the GPIO pins */
+	gpiod_direction_input(priv->stat0_gpio);
+	gpiod_direction_input(priv->stat1_gpio);
+	gpiod_direction_output(priv->load_gpio,
+		priv->curr_src == DCON_SOURCE_CPU);
+
+	/* Register the interrupt handler */
+	rc = devm_request_irq(&client->dev, client->irq, &hx8837_interrupt, 0,
+								"DCON", priv);
+	if (rc) {
+		dev_err(&client->dev, "IRQ request failed: %d\n", rc);
+		return rc;
+	}
+
+	/* SDRAM setup/hold time */
+	hx8837_write(priv, 0x3a, 0xc040);
+	hx8837_write(priv, DCON_REG_MEM_OPT_A, 0x0000);  /* clear option bits */
+	hx8837_write(priv, DCON_REG_MEM_OPT_A,
+		   MEM_DLL_CLOCK_DELAY | MEM_POWER_DOWN);
+	hx8837_write(priv, DCON_REG_MEM_OPT_B, MEM_SOFT_RESET);
+
+	/* Colour swizzle, AA, no passthrough, backlight */
+	priv->disp_mode = MODE_PASSTHRU | MODE_BL_ENABLE | MODE_CSWIZZLE | MODE_COL_AA;
+	hx8837_write(priv, DCON_REG_MODE, priv->disp_mode);
+
+	/* Set the scanline to interrupt on during resume */
+	/* FIXME: Calculate this from the mode. */
+	hx8837_write(priv, DCON_REG_SCAN_INT, 898);
+
+	priv->bl_val = hx8837_read(priv, DCON_REG_BRIGHT) & 0x0F;
+
+	/* Add the backlight device for the DCON */
+	hx8837_bl_props.brightness = priv->bl_val;
+	priv->bl_dev = devm_backlight_device_register(&client->dev, "hx8837-bl",
+			&client->dev, priv, &hx8837_bl_ops, &hx8837_bl_props);
+	if (IS_ERR(priv->bl_dev)) {
+		dev_err(&client->dev, "cannot register backlight dev (%ld)\n",
+			PTR_ERR(priv->bl_dev));
+		priv->bl_dev = NULL;
+	}
+
+	rc = devm_device_add_groups(&client->dev, hx8837_groups);
+	if (rc) {
+		dev_err(&client->dev, "failed to register sysfs groups\n");
+		return rc;
+	}
+
+        rc = component_add(&client->dev, &hx8837_ops);
+        if (rc) {
+		dev_err(&client->dev, "failed to register sysfs groups\n");
+		return rc;
+	}
+
+	atomic_notifier_chain_register(&panic_notifier_list, &priv->panic_nb);
+
+	return 0;
+}
+
+static int hx8837_remove(struct i2c_client *client)
+{
+	struct hx8837_priv *priv = i2c_get_clientdata(client);
+
+
+	component_del(&client->dev, &hx8837_ops);
+
+
+
+	atomic_notifier_chain_unregister(&panic_notifier_list,
+					&priv->panic_nb);
+
+	cancel_work_sync(&priv->switch_source);
+
+	return 0;
+}
+
+static irqreturn_t hx8837_interrupt(int irq, void *id)
+{
+	struct hx8837_priv *priv = id;
+	struct device *dev = &priv->client->dev;
+	u8 status;
+
+	status = gpiod_get_value(priv->stat0_gpio);
+	status |= gpiod_get_value(priv->stat1_gpio) << 1;
+
+
+	switch (status & 3) {
+	case 3:
+		dev_dbg(dev, "DCONLOAD_MISSED interrupt\n");
+		break;
+
+	case 2:	/* switch to DCON mode */
+	case 1: /* switch to CPU mode */
+		priv->switched = true;
+		priv->irq_time = ktime_get();
+		wake_up(&priv->waitq);
+		break;
+
+	case 0:
+		/* workaround resume case:  the DCON (on 1.5) doesn't
+		 * ever assert status 0x01 when switching to CPU mode
+		 * during resume.  this is because DCONLOAD is de-asserted
+		 * _immediately_ upon exiting S3, so the actual release
+		 * of the DCON happened long before this point.
+		 * see http://dev.laptop.org/ticket/9869
+		 */
+		if (priv->curr_src != priv->pending_src && !priv->switched) {
+			priv->switched = true;
+			priv->irq_time = ktime_get();
+			wake_up(&priv->waitq);
+			dev_dbg(dev, "switching w/ status 0/0\n");
+		} else {
+			dev_dbg(dev, "scanline interrupt w/CPU\n");
+		}
+	}
+
+	return IRQ_HANDLED;
+}
+
+#ifdef CONFIG_PM
+static int hx8837_suspend(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct hx8837_priv *priv = i2c_get_clientdata(client);
+
+	hx8837_set_source_sync(priv, DCON_SOURCE_DCON);
+
+	return 0;
+}
+
+static int hx8837_resume(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct hx8837_priv *priv = i2c_get_clientdata(client);
+
+	hx8837_set_source(priv, DCON_SOURCE_CPU);
+
+	return 0;
+}
+#endif /* CONFIG_PM */
+
+static SIMPLE_DEV_PM_OPS(hx8837_pm_ops, hx8837_suspend, hx8837_resume);
+
+static const struct of_device_id hx8837_dt_ids[] = {
+	{ .compatible = "himax,hx8837", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, hx8837_dt_ids);
+
+static const struct i2c_device_id hx8837_i2c_ids[] = {
+	{ .name = "hx8837", },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, hx8837_i2c_ids);
+
+static struct i2c_driver hx8837_driver = {
+	.driver = {
+		.name = "hx8837",
+		.pm = &hx8837_pm_ops,
+		.of_match_table = hx8837_dt_ids,
+	},
+	.class = I2C_CLASS_DDC | I2C_CLASS_HWMON,
+	.id_table = hx8837_i2c_ids,
+	.probe = hx8837_probe,
+	.remove = hx8837_remove,
+	.detect = hx8837_detect,
+	.address_list = normal_i2c,
+};
+
+module_i2c_driver(hx8837_driver);
+
+MODULE_DESCRIPTION("HX8837 Display Controller Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.19.1

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

Re: [RFC 15/16] drm/armada: add mmp2 support

[James Cameron]

On Tue, Dec 18, 2018 at 04:37:41PM +0100, Lubomir Rintel wrote:
> This pretty much boils down to setting the LCD_CFG_SCLK_DIV register, and
> is tailored to the OLPC XO-1.75. I have no idea what are the meanings of
> most bits there, so I'm just making sure it ends up being 0x40001102.
> 
> This means that the selection of the source clock is hardwired. Apparently
> the bit 30 selects the AXI bus clock as base clock for the pixel clock.
> It is not known to me what other options are there.

I don't think so.  I'd say Display Clock 1.  See my full answer below
and Open Firmware code comment in lcd.fth;

	h# 40001102 value clkdiv  \ Display Clock 1 / 2 -> 56.93 MHz

> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> 
> ---
> Notes; perhaps James or someone else with a datasheet could help me with
> this.
> 
> I'm somewhat confused about the clock selection. The firmware contains
> this line:
> 
>   h# 00000700 value pmua-disp-clk-sel  \ PLL1 / 7 -> 113.86 MHz

In Open Firmware this is a mask of bits set in each write to offset
0x4c in the power management unit when turning LCD clocks on or off.

Name in the datasheet is PMUA_DISPLAY1_CLK_RES_CTRL.

Bits 8 through 11 are CLK_DIV_SEL, clock divider select, where 0x1 is
divide by 1, up to 0x15 (must be a typo) is divide by 15.

Bits 6 and 7 are CLK_SEL, clock select, and 0x0 is PLL1 without divider.

Given pmua-disp-clk-sel is 0x700, that's bits 8 through 10 set, so it
is configured for divide by seven.

> However, the Linux clock driver seems to consider the value of 7 to
> configure the divisor to 8, so the resulting clock would end up being
> 100 MHz. Also, the clk-of-mmp2 driver suggests PLL1 outputs 800 MHz,
> dividing that by 7 would end up being 114.29 MHz, not 113.86 MHz.

Posit PLL1 at 797.02 MHz, other comments agree, such as

dev/olpc/mmp2camera/ccic.fth: \ 797/2 (PLL1/16) / 2 -> 24.9 MHz
cforth:src/app/arm-mmp2/lcd.fth: \ PLL1 / 7 -> 113.86 MHz
src/app/arm-xo-1.75/lcdcfg.fth \ PLL1 / 7 -> 113.86 MHz
src/app/arm-mmp3-thunderstone/initdram.fth:  \  PLL1:797
src/app/arm-xo-cl4/clockset.fth:   \ PLL1:797

I've engineering notes showing recommendation to configure PLL1 for
797 MHz.

There's also the fuse configuration, which you can dump with "ok
.fuses-all".  My usual test unit is a revision C or later, and reports
910 MHz.  My notes also say that at 910 MHz the CPU is not synchronous
to the I/O buses, but that the 800 MHz or 797.02 MHz operating point
is.  I'm not sure how relevant that is, sorry.

> If the same logic was used here as Armada 510 driver uses, we'll end up
> with the divisor of 1 in LCD_CFG_SCLK_DIV and disp0_div divisor of 16.
> Would that be good also? (Do perhaps any of the bits in LCD_CFG_SCLK_DIV
> allow for a fractional divisor, allowing us to get the clock we need
> more precisely?)

Yes.  For the purposes of interoperability;

LCD_CFG_SLKC_DIV is offset 0x1a8, which is LCD_SCLK_DIV in the
datasheet.

Bits 0-7 are panel path pixel clock integer divider, and is from 2 to
255.  Open Firmware configures as 2.

Bits 8-11 are panel path mipi bit clock divider, and is from 2 to 15,
with 1 meaning bypass, and 0 meaning disabled.  Open Firmware
configures as 1.

Bits 16-27 are panel path pixel clock fraction divider.  Open Firmware
configures as 0.

Bit 28 is panel path pixel click disable (1 means disabled).

Bit 30:31 are panel path clock source select, 0 is AXI, 1 and 2 are
LCD Display clocks, and 3 is HDMI PLL clock.  Open Firmware configures
display clock 1.

> The pxa168fb driver as used on the stock OLPC software just avoids
> touching the register, preserving the value set from the firmware.
> ---
> [...]

-- 
James Cameron
http://quozl.netrek.org/
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 00/16] Armada DRM support for OLPC XO-1.75 laptop

[Daniel Vetter]

On Wed, Dec 19, 2018 at 9:40 AM Lubomir Rintel <lkundrak@v3.sk> wrote:
>
> Hi,
>
> here are patches that make the Armada DRM work on an OLPC XO-1.75.
> They build on patches previously submitted by Russel King (included here for
> completeness as well).
>
> They certainly need some more love, but I'm not able to improve them until
> I get to understand some things first. I'm posting a couple of questions
> below in hope someone is kind enough to help me deal with my confusion.
>
> The display pipeline on the laptop looks like this:
>
>   Armada LCDC
>   -----------
>       |
>       v
>
>   Himax HX8837 "DCON"
>   -------------------
>   RGB input from LCDC
>   Controlled via I2C
>   Backlight control
>   Can slow down the panel refresh rate to save power
>   Optional dithering for color mode, "swizzling"
>   Has DRAM attached, can freeze a single frame in it
>   Doc: http://wiki.laptop.org/images/0/09/DCON_datasheet_HX8837-A.pdf
>       |
>       v
>
>   Innolux LS075AT011 Panel
>   ------------------------
>   7.5", 1200x900
>   No datasheet.
>   http://wiki.laptop.org/go/Display
>   Can opreate in two modes:
>
>    R G B
>    G B R ...   or   "e-book" daylight readable
>    B R G            reflexive B&W
>      .
>      :
>
> Here's what's not clear to me:
>
> 1.) Is the Himax chip an encoder here?

Since it's an external IP probably better to model it as a bridge.

> 2.) What's the use of an encoder anyways? If a panel was connected directly
>     do the RGB output from the LCDC, would we have to fake one? Is that the
>     point of things like i.MX driver's drivers/gpu/drm/imx/parallel-display.c?

encoder is the thing between crtc and connector. It's exposed to
userspace (which is a uapi design accident, but can't change that),
hence why it's required and some drivers have dummy ones.

The real usage is purely internally for the atomic helpers. Most
callbacks in the encoder should be optional, so a dummy encoder should
be a lot thinner than your imx example. The imx example should be
using the panel-bridge I think, which would take care of most of the
boilerplate (but panel-bridge didn't exist back then I guess).

> 3.) My patch set currently contains the driver for the Himax that is
>     modelled after tda998x. That one implements an encoder. Similar drivers
>     seem to add a bridge too, but it's not entirely clear to me what a bridge
>     is good for?

tda998x predates bridges, which is why there's still some
encoder_helper usage in there. It's become a proper bridge driver now
I think (but some drivers still use the old interface). All new
transcoder chip drivers should be bridges. Did you look at the
kerneldoc for bridges? If those aren't clear we need to fix them.

> 4.) How shall I expose the fancy functionality of the Himax to the userspace?
>     Notably, the freeze frame. The OLPC laptops with the stock firmware like
>     to suspend the SoC very aggressively to save power (in 20 seconds of
>     inactivity or so). If the display is open (it can also be turned around
>     for a tablet or e-book mode), it makes sense to freeze the picture and
>     keep the panel on, if the laptop is closed, we want to turn it off.
>     Should the behavior be exposed via sysfs (as it is with the current OLPC
>     kernels), or a DRM property? Would it require libdrm support?

I've accidentally seen how that's implemented for fbdev in the older
olpc drivers, and that's definitely _not_ how we want to support this.
Essentially what you have here is a fancy self-refresh panel. That's
already fully supported by DRM uapi, so I'd say first implement that.
Once we have that we can figure out a special property that tells the
driver to not shut down the screen on suspend, but just go into
self-refresh. Implementing the self-refresh stuff in DRM will be the
big pile of work (since that's something which goes beyond the generic
drm_bridge interface).


>   Himax HX8837 "DCON"
>   -------------------
>   RGB input from LCDC
>   Controlled via I2C

These two shouldn't be a problem.

>   Backlight control

We already have backlight interfaces

>   Can slow down the panel refresh rate to save power

Just expose this as modes with different vrefresh (and do that
modeswitch without doing a full modeset). Bonus if you do it
automatically, which can be done with the self-refresh infrastructure.

>   Optional dithering for color mode, "swizzling"

Not exactly clear, but I'm assuming the bus_format stuff on bridges
should help with this.

>   Has DRAM attached, can freeze a single frame in it

Classic self-refresh support. Noralf Tronnes has done great work here
past year to improve the infrastructure and let drivers implement this
with less typing. Also look at the recent work by vmwgfx folks.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 00/16] Armada DRM support for OLPC XO-1.75 laptop

[Lubomir Rintel]

On Wed, 2018-12-19 at 10:13 +0100, Daniel Vetter wrote:
> On Wed, Dec 19, 2018 at 9:40 AM Lubomir Rintel <lkundrak@v3.sk> wrote:
> > Hi,
> > 
> > here are patches that make the Armada DRM work on an OLPC XO-1.75.
> > They build on patches previously submitted by Russel King (included here for
> > completeness as well).
> > 
> > They certainly need some more love, but I'm not able to improve them until
> > I get to understand some things first. I'm posting a couple of questions
> > below in hope someone is kind enough to help me deal with my confusion.
> > 
> > The display pipeline on the laptop looks like this:
> > 
> >   Armada LCDC
> >   -----------
> >       |
> >       v
> > 
> >   Himax HX8837 "DCON"
> >   -------------------
> >   RGB input from LCDC
> >   Controlled via I2C
> >   Backlight control
> >   Can slow down the panel refresh rate to save power
> >   Optional dithering for color mode, "swizzling"
> >   Has DRAM attached, can freeze a single frame in it
> >   Doc: http://wiki.laptop.org/images/0/09/DCON_datasheet_HX8837-A.pdf
> >       |
> >       v
> > 
> >   Innolux LS075AT011 Panel
> >   ------------------------
> >   7.5", 1200x900
> >   No datasheet.
> >   http://wiki.laptop.org/go/Display
> >   Can opreate in two modes:
> > 
> >    R G B
> >    G B R ...   or   "e-book" daylight readable
> >    B R G            reflexive B&W
> >      .
> >      :
> > 
> > Here's what's not clear to me:
> > 
> > 1.) Is the Himax chip an encoder here?
> 
> Since it's an external IP probably better to model it as a bridge.
> 
> > 2.) What's the use of an encoder anyways? If a panel was connected directly
> >     do the RGB output from the LCDC, would we have to fake one? Is that the
> >     point of things like i.MX driver's drivers/gpu/drm/imx/parallel-display.c?
> 
> encoder is the thing between crtc and connector. It's exposed to
> userspace (which is a uapi design accident, but can't change that),
> hence why it's required and some drivers have dummy ones.
> 
> The real usage is purely internally for the atomic helpers.

I'm wondering how should the drm_encoder be instantiated? The imx uses
a dummy (compatible="fsl,imx-parallel-display") node. That almost
certainly counts as devicetree abuse and is a wrong thing to do.
Should the armada driver always create one? In that case the
transcoders that connect to it (just tda998x, I suppose) should cease
doing so and hook on a bridge instead.

Or should the driver for the Himax chip create an encoder and bridge
intance? Perhaps not, if my understanding is correct that drm_encoder
shall not be used for things outside the lcdc/crtc chip.

> Most
> callbacks in the encoder should be optional, so a dummy encoder should
> be a lot thinner than your imx example. The imx example should be
> using the panel-bridge I think, which would take care of most of the
> boilerplate (but panel-bridge didn't exist back then I guess).

> tda998x predates bridges, which is why there's still some
> encoder_helper usage in there. It's become a proper bridge driver now
> I think (but some drivers still use the old interface). All new
> transcoder chip drivers should be bridges.

Which drivers are good examples?

> Did you look at the
> kerneldoc for bridges? If those aren't clear we need to fix them.

Yes, if you mean this:
https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#bridges

If they weren't clear it might be entirely my fault. I'm totally new to
most of the stuff there and the sheer size of the structures and the
API is a tough challenge for my working memory.

The two important things that were not clear to me are essentially
what's discussed above. That is:

1.) Should I use this at all?
From "These are handy when a regular drm_encoder entity isn’t enough to
represent the entire encoder chain." I wrongly concluded that I
probably don't heed to.

2.) Which driver creates the drm_encoder object and which one connects
it to the drm_bridge.

> > 4.) How shall I expose the fancy functionality of the Himax to the userspace?
> >     Notably, the freeze frame. The OLPC laptops with the stock firmware like
> >     to suspend the SoC very aggressively to save power (in 20 seconds of
> >     inactivity or so). If the display is open (it can also be turned around
> >     for a tablet or e-book mode), it makes sense to freeze the picture and
> >     keep the panel on, if the laptop is closed, we want to turn it off.
> >     Should the behavior be exposed via sysfs (as it is with the current OLPC
> >     kernels), or a DRM property? Would it require libdrm support?
> 
> I've accidentally seen how that's implemented for fbdev in the older
> olpc drivers, and that's definitely _not_ how we want to support this.
> Essentially what you have here is a fancy self-refresh panel. That's
> already fully supported by DRM uapi, so I'd say first implement that.
> Once we have that we can figure out a special property that tells the
> driver to not shut down the screen on suspend, but just go into
> self-refresh. Implementing the self-refresh stuff in DRM will be the
> big pile of work (since that's something which goes beyond the generic
> drm_bridge interface).

Thank you, this looks pretty helpful.

Currently the MMP2 platform barely boots with the mainline kernel, let
alone suspends and resumes. I guess it would be all right if the self-
refresh-on-suspend trick gets worked out later, when the OLPC patch
queue drains a bit.

How do the DRM properties get set? Do they need support in some
userspace library, perhaps libdrm?

> >   Himax HX8837 "DCON"
> >   -------------------
> >   RGB input from LCDC
> >   Controlled via I2C
> 
> These two shouldn't be a problem.
> 
> >   Backlight control
> 
> We already have backlight interfaces

Yes; this is already supported in the RFC version of the driver chained
to this message.

> >   Can slow down the panel refresh rate to save power
> 
> Just expose this as modes with different vrefresh (and do that
> modeswitch without doing a full modeset). Bonus if you do it
> automatically, which can be done with the self-refresh infrastructure.
> 
> >   Optional dithering for color mode, "swizzling"
> 
> Not exactly clear, but I'm assuming the bus_format stuff on bridges
> should help with this.
> 
> >   Has DRAM attached, can freeze a single frame in it
> 
> Classic self-refresh support. Noralf Tronnes has done great work here
> past year to improve the infrastructure and let drivers implement this
> with less typing. Also look at the recent work by vmwgfx folks.

Thanks for above the pointers.

> Cheers, Daniel

I appreciate your response very much.

I'll try to digest it over the holidays and hopefully figure out how to
follow up with an updated version sometime early next year.

If I'm unsuccessful perhaps I'll manage to take some poor graphics
hacker hostage at FOSDEM.

Take care,
Lubo

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

Re: [RFC 00/16] Armada DRM support for OLPC XO-1.75 laptop

[Daniel Vetter]

On Wed, Dec 19, 2018 at 6:14 PM Lubomir Rintel <lkundrak@v3.sk> wrote:
>
> On Wed, 2018-12-19 at 10:13 +0100, Daniel Vetter wrote:
> > On Wed, Dec 19, 2018 at 9:40 AM Lubomir Rintel <lkundrak@v3.sk> wrote:
> > > Hi,
> > >
> > > here are patches that make the Armada DRM work on an OLPC XO-1.75.
> > > They build on patches previously submitted by Russel King (included here for
> > > completeness as well).
> > >
> > > They certainly need some more love, but I'm not able to improve them until
> > > I get to understand some things first. I'm posting a couple of questions
> > > below in hope someone is kind enough to help me deal with my confusion.
> > >
> > > The display pipeline on the laptop looks like this:
> > >
> > >   Armada LCDC
> > >   -----------
> > >       |
> > >       v
> > >
> > >   Himax HX8837 "DCON"
> > >   -------------------
> > >   RGB input from LCDC
> > >   Controlled via I2C
> > >   Backlight control
> > >   Can slow down the panel refresh rate to save power
> > >   Optional dithering for color mode, "swizzling"
> > >   Has DRAM attached, can freeze a single frame in it
> > >   Doc: http://wiki.laptop.org/images/0/09/DCON_datasheet_HX8837-A.pdf
> > >       |
> > >       v
> > >
> > >   Innolux LS075AT011 Panel
> > >   ------------------------
> > >   7.5", 1200x900
> > >   No datasheet.
> > >   http://wiki.laptop.org/go/Display
> > >   Can opreate in two modes:
> > >
> > >    R G B
> > >    G B R ...   or   "e-book" daylight readable
> > >    B R G            reflexive B&W
> > >      .
> > >      :
> > >
> > > Here's what's not clear to me:
> > >
> > > 1.) Is the Himax chip an encoder here?
> >
> > Since it's an external IP probably better to model it as a bridge.
> >
> > > 2.) What's the use of an encoder anyways? If a panel was connected directly
> > >     do the RGB output from the LCDC, would we have to fake one? Is that the
> > >     point of things like i.MX driver's drivers/gpu/drm/imx/parallel-display.c?
> >
> > encoder is the thing between crtc and connector. It's exposed to
> > userspace (which is a uapi design accident, but can't change that),
> > hence why it's required and some drivers have dummy ones.
> >
> > The real usage is purely internally for the atomic helpers.
>
> I'm wondering how should the drm_encoder be instantiated? The imx uses
> a dummy (compatible="fsl,imx-parallel-display") node. That almost
> certainly counts as devicetree abuse and is a wrong thing to do.
> Should the armada driver always create one? In that case the
> transcoders that connect to it (just tda998x, I suppose) should cease
> doing so and hook on a bridge instead.

Not a dt expert, but on all your other questions's I'd say yes. Note
that if there's any place where a 100% dummy drm_encoder blows up we
should change that. Anything beyond drm_encoder_init shouldn't be
necessary.

> Or should the driver for the Himax chip create an encoder and bridge
> intance? Perhaps not, if my understanding is correct that drm_encoder
> shall not be used for things outside the lcdc/crtc chip.

drm_bridge is the interface for external transcoder IP, not
drm_encoder. As mentioned, only reason tda998x creates an encoder in
some cases is for historical reasons.

>
> > Most
> > callbacks in the encoder should be optional, so a dummy encoder should
> > be a lot thinner than your imx example. The imx example should be
> > using the panel-bridge I think, which would take care of most of the
> > boilerplate (but panel-bridge didn't exist back then I guess).
>
> > tda998x predates bridges, which is why there's still some
> > encoder_helper usage in there. It's become a proper bridge driver now
> > I think (but some drivers still use the old interface). All new
> > transcoder chip drivers should be bridges.
>
> Which drivers are good examples?

Anything under drm/bridge should be fine.

> > Did you look at the
> > kerneldoc for bridges? If those aren't clear we need to fix them.
>
> Yes, if you mean this:
> https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#bridges
>
> If they weren't clear it might be entirely my fault. I'm totally new to
> most of the stuff there and the sheer size of the structures and the
> API is a tough challenge for my working memory.
>
> The two important things that were not clear to me are essentially
> what's discussed above. That is:
>
> 1.) Should I use this at all?
> From "These are handy when a regular drm_encoder entity isn’t enough to
> represent the entire encoder chain." I wrongly concluded that I
> probably don't heed to.

Hm yeah I guess that should mention a postive example like
- external transcoder block
- if you want to share transcoder code between different drivers.

Care to write a patch to improve the wording?

> 2.) Which driver creates the drm_encoder object and which one connects
> it to the drm_bridge.

Main driver needs to create the drm_encoder, shared transcoder driver
only creates the bridge.

Again, care to clarify this in a patch? Might be also good to add
links to the various functions for registering/finding a bridge and
how to link it up to the encoder, and who exactly should do that.

> > > 4.) How shall I expose the fancy functionality of the Himax to the userspace?
> > >     Notably, the freeze frame. The OLPC laptops with the stock firmware like
> > >     to suspend the SoC very aggressively to save power (in 20 seconds of
> > >     inactivity or so). If the display is open (it can also be turned around
> > >     for a tablet or e-book mode), it makes sense to freeze the picture and
> > >     keep the panel on, if the laptop is closed, we want to turn it off.
> > >     Should the behavior be exposed via sysfs (as it is with the current OLPC
> > >     kernels), or a DRM property? Would it require libdrm support?
> >
> > I've accidentally seen how that's implemented for fbdev in the older
> > olpc drivers, and that's definitely _not_ how we want to support this.
> > Essentially what you have here is a fancy self-refresh panel. That's
> > already fully supported by DRM uapi, so I'd say first implement that.
> > Once we have that we can figure out a special property that tells the
> > driver to not shut down the screen on suspend, but just go into
> > self-refresh. Implementing the self-refresh stuff in DRM will be the
> > big pile of work (since that's something which goes beyond the generic
> > drm_bridge interface).
>
> Thank you, this looks pretty helpful.
>
> Currently the MMP2 platform barely boots with the mainline kernel, let
> alone suspends and resumes. I guess it would be all right if the self-
> refresh-on-suspend trick gets worked out later, when the OLPC patch
> queue drains a bit.
>
> How do the DRM properties get set? Do they need support in some
> userspace library, perhaps libdrm?

You'll need to write the compositor patches in some open source
userspace to demonstrate the full stack, see:

https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements

I guess you're still very far away from that point. libdrm itself
shouldn't need any changes.

> > >   Himax HX8837 "DCON"
> > >   -------------------
> > >   RGB input from LCDC
> > >   Controlled via I2C
> >
> > These two shouldn't be a problem.
> >
> > >   Backlight control
> >
> > We already have backlight interfaces
>
> Yes; this is already supported in the RFC version of the driver chained
> to this message.
>
> > >   Can slow down the panel refresh rate to save power
> >
> > Just expose this as modes with different vrefresh (and do that
> > modeswitch without doing a full modeset). Bonus if you do it
> > automatically, which can be done with the self-refresh infrastructure.
> >
> > >   Optional dithering for color mode, "swizzling"
> >
> > Not exactly clear, but I'm assuming the bus_format stuff on bridges
> > should help with this.
> >
> > >   Has DRAM attached, can freeze a single frame in it
> >
> > Classic self-refresh support. Noralf Tronnes has done great work here
> > past year to improve the infrastructure and let drivers implement this
> > with less typing. Also look at the recent work by vmwgfx folks.
>
> Thanks for above the pointers.
>
> > Cheers, Daniel
>
> I appreciate your response very much.

Happy to help out, especially if it results in improved documentation.
Often the experts don't see what's missing because too close, and
people newly learning are best suited to spot&fill the gaps.

> I'll try to digest it over the holidays and hopefully figure out how to
> follow up with an updated version sometime early next year.
>
> If I'm unsuccessful perhaps I'll manage to take some poor graphics
> hacker hostage at FOSDEM.

I won't be at fosdem, but should be plenty of victims for you there :-)

Cheers, Daniel

>
> Take care,
> Lubo
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 00/16] Armada DRM support for OLPC XO-1.75 laptop

[Daniel Vetter]

On Wed, Dec 19, 2018 at 7:35 PM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Wed, Dec 19, 2018 at 6:14 PM Lubomir Rintel <lkundrak@v3.sk> wrote:
> >
> > On Wed, 2018-12-19 at 10:13 +0100, Daniel Vetter wrote:
> > > On Wed, Dec 19, 2018 at 9:40 AM Lubomir Rintel <lkundrak@v3.sk> wrote:
> > > > Hi,
> > > >
> > > > here are patches that make the Armada DRM work on an OLPC XO-1.75.
> > > > They build on patches previously submitted by Russel King (included here for
> > > > completeness as well).
> > > >
> > > > They certainly need some more love, but I'm not able to improve them until
> > > > I get to understand some things first. I'm posting a couple of questions
> > > > below in hope someone is kind enough to help me deal with my confusion.
> > > >
> > > > The display pipeline on the laptop looks like this:
> > > >
> > > >   Armada LCDC
> > > >   -----------
> > > >       |
> > > >       v
> > > >
> > > >   Himax HX8837 "DCON"
> > > >   -------------------
> > > >   RGB input from LCDC
> > > >   Controlled via I2C
> > > >   Backlight control
> > > >   Can slow down the panel refresh rate to save power
> > > >   Optional dithering for color mode, "swizzling"
> > > >   Has DRAM attached, can freeze a single frame in it
> > > >   Doc: http://wiki.laptop.org/images/0/09/DCON_datasheet_HX8837-A.pdf
> > > >       |
> > > >       v
> > > >
> > > >   Innolux LS075AT011 Panel
> > > >   ------------------------
> > > >   7.5", 1200x900
> > > >   No datasheet.
> > > >   http://wiki.laptop.org/go/Display
> > > >   Can opreate in two modes:
> > > >
> > > >    R G B
> > > >    G B R ...   or   "e-book" daylight readable
> > > >    B R G            reflexive B&W
> > > >      .
> > > >      :
> > > >
> > > > Here's what's not clear to me:
> > > >
> > > > 1.) Is the Himax chip an encoder here?
> > >
> > > Since it's an external IP probably better to model it as a bridge.
> > >
> > > > 2.) What's the use of an encoder anyways? If a panel was connected directly
> > > >     do the RGB output from the LCDC, would we have to fake one? Is that the
> > > >     point of things like i.MX driver's drivers/gpu/drm/imx/parallel-display.c?
> > >
> > > encoder is the thing between crtc and connector. It's exposed to
> > > userspace (which is a uapi design accident, but can't change that),
> > > hence why it's required and some drivers have dummy ones.
> > >
> > > The real usage is purely internally for the atomic helpers.
> >
> > I'm wondering how should the drm_encoder be instantiated? The imx uses
> > a dummy (compatible="fsl,imx-parallel-display") node. That almost
> > certainly counts as devicetree abuse and is a wrong thing to do.
> > Should the armada driver always create one? In that case the
> > transcoders that connect to it (just tda998x, I suppose) should cease
> > doing so and hook on a bridge instead.
>
> Not a dt expert, but on all your other questions's I'd say yes. Note
> that if there's any place where a 100% dummy drm_encoder blows up we
> should change that. Anything beyond drm_encoder_init shouldn't be
> necessary.
>
> > Or should the driver for the Himax chip create an encoder and bridge
> > intance? Perhaps not, if my understanding is correct that drm_encoder
> > shall not be used for things outside the lcdc/crtc chip.
>
> drm_bridge is the interface for external transcoder IP, not
> drm_encoder. As mentioned, only reason tda998x creates an encoder in
> some cases is for historical reasons.

kms overview might also help with navigating the big picture:

https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html

(I just noticed a misplace link target, patch on its way).
-Daniel

> >
> > > Most
> > > callbacks in the encoder should be optional, so a dummy encoder should
> > > be a lot thinner than your imx example. The imx example should be
> > > using the panel-bridge I think, which would take care of most of the
> > > boilerplate (but panel-bridge didn't exist back then I guess).
> >
> > > tda998x predates bridges, which is why there's still some
> > > encoder_helper usage in there. It's become a proper bridge driver now
> > > I think (but some drivers still use the old interface). All new
> > > transcoder chip drivers should be bridges.
> >
> > Which drivers are good examples?
>
> Anything under drm/bridge should be fine.
>
> > > Did you look at the
> > > kerneldoc for bridges? If those aren't clear we need to fix them.
> >
> > Yes, if you mean this:
> > https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#bridges
> >
> > If they weren't clear it might be entirely my fault. I'm totally new to
> > most of the stuff there and the sheer size of the structures and the
> > API is a tough challenge for my working memory.
> >
> > The two important things that were not clear to me are essentially
> > what's discussed above. That is:
> >
> > 1.) Should I use this at all?
> > From "These are handy when a regular drm_encoder entity isn’t enough to
> > represent the entire encoder chain." I wrongly concluded that I
> > probably don't heed to.
>
> Hm yeah I guess that should mention a postive example like
> - external transcoder block
> - if you want to share transcoder code between different drivers.
>
> Care to write a patch to improve the wording?
>
> > 2.) Which driver creates the drm_encoder object and which one connects
> > it to the drm_bridge.
>
> Main driver needs to create the drm_encoder, shared transcoder driver
> only creates the bridge.
>
> Again, care to clarify this in a patch? Might be also good to add
> links to the various functions for registering/finding a bridge and
> how to link it up to the encoder, and who exactly should do that.
>
> > > > 4.) How shall I expose the fancy functionality of the Himax to the userspace?
> > > >     Notably, the freeze frame. The OLPC laptops with the stock firmware like
> > > >     to suspend the SoC very aggressively to save power (in 20 seconds of
> > > >     inactivity or so). If the display is open (it can also be turned around
> > > >     for a tablet or e-book mode), it makes sense to freeze the picture and
> > > >     keep the panel on, if the laptop is closed, we want to turn it off.
> > > >     Should the behavior be exposed via sysfs (as it is with the current OLPC
> > > >     kernels), or a DRM property? Would it require libdrm support?
> > >
> > > I've accidentally seen how that's implemented for fbdev in the older
> > > olpc drivers, and that's definitely _not_ how we want to support this.
> > > Essentially what you have here is a fancy self-refresh panel. That's
> > > already fully supported by DRM uapi, so I'd say first implement that.
> > > Once we have that we can figure out a special property that tells the
> > > driver to not shut down the screen on suspend, but just go into
> > > self-refresh. Implementing the self-refresh stuff in DRM will be the
> > > big pile of work (since that's something which goes beyond the generic
> > > drm_bridge interface).
> >
> > Thank you, this looks pretty helpful.
> >
> > Currently the MMP2 platform barely boots with the mainline kernel, let
> > alone suspends and resumes. I guess it would be all right if the self-
> > refresh-on-suspend trick gets worked out later, when the OLPC patch
> > queue drains a bit.
> >
> > How do the DRM properties get set? Do they need support in some
> > userspace library, perhaps libdrm?
>
> You'll need to write the compositor patches in some open source
> userspace to demonstrate the full stack, see:
>
> https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements
>
> I guess you're still very far away from that point. libdrm itself
> shouldn't need any changes.
>
> > > >   Himax HX8837 "DCON"
> > > >   -------------------
> > > >   RGB input from LCDC
> > > >   Controlled via I2C
> > >
> > > These two shouldn't be a problem.
> > >
> > > >   Backlight control
> > >
> > > We already have backlight interfaces
> >
> > Yes; this is already supported in the RFC version of the driver chained
> > to this message.
> >
> > > >   Can slow down the panel refresh rate to save power
> > >
> > > Just expose this as modes with different vrefresh (and do that
> > > modeswitch without doing a full modeset). Bonus if you do it
> > > automatically, which can be done with the self-refresh infrastructure.
> > >
> > > >   Optional dithering for color mode, "swizzling"
> > >
> > > Not exactly clear, but I'm assuming the bus_format stuff on bridges
> > > should help with this.
> > >
> > > >   Has DRAM attached, can freeze a single frame in it
> > >
> > > Classic self-refresh support. Noralf Tronnes has done great work here
> > > past year to improve the infrastructure and let drivers implement this
> > > with less typing. Also look at the recent work by vmwgfx folks.
> >
> > Thanks for above the pointers.
> >
> > > Cheers, Daniel
> >
> > I appreciate your response very much.
>
> Happy to help out, especially if it results in improved documentation.
> Often the experts don't see what's missing because too close, and
> people newly learning are best suited to spot&fill the gaps.
>
> > I'll try to digest it over the holidays and hopefully figure out how to
> > follow up with an updated version sometime early next year.
> >
> > If I'm unsuccessful perhaps I'll manage to take some poor graphics
> > hacker hostage at FOSDEM.
>
> I won't be at fosdem, but should be plenty of victims for you there :-)
>
> Cheers, Daniel
>
> >
> > Take care,
> > Lubo
> >
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 00/16] Armada DRM support for OLPC XO-1.75 laptop

[Lubomir Rintel]

Hi,

On Wed, 2018-12-19 at 10:13 +0100, Daniel Vetter wrote:
> On Wed, Dec 19, 2018 at 9:40 AM Lubomir Rintel <lkundrak@v3.sk> wrote:
> > Hi,
> > 
> > here are patches that make the Armada DRM work on an OLPC XO-1.75.
> > They build on patches previously submitted by Russel King (included here for
> > completeness as well).
> > 
> > They certainly need some more love, but I'm not able to improve them until
> > I get to understand some things first. I'm posting a couple of questions
> > below in hope someone is kind enough to help me deal with my confusion.
> > 
> > The display pipeline on the laptop looks like this:
> > 
> >   Armada LCDC
> >   -----------
> >       |
> >       v
> > 
> >   Himax HX8837 "DCON"
> >   -------------------
> >   RGB input from LCDC
> >   Controlled via I2C
> >   Backlight control
> >   Can slow down the panel refresh rate to save power
> >   Optional dithering for color mode, "swizzling"
> >   Has DRAM attached, can freeze a single frame in it
> >   Doc: http://wiki.laptop.org/images/0/09/DCON_datasheet_HX8837-A.pdf
> >       |
> >       v
> > 
> >   Innolux LS075AT011 Panel
> >   ------------------------
> >   7.5", 1200x900
> >   No datasheet.
> >   http://wiki.laptop.org/go/Display
> >   Can opreate in two modes:
> > 
> >    R G B
> >    G B R ...   or   "e-book" daylight readable
> >    B R G            reflexive B&W
> >      .
> >      :
> > 
> > Here's what's not clear to me:
> > 
> > 1.) Is the Himax chip an encoder here?
> 
> Since it's an external IP probably better to model it as a bridge.
> 
> > 2.) What's the use of an encoder anyways? If a panel was connected directly
> >     do the RGB output from the LCDC, would we have to fake one? Is that the
> >     point of things like i.MX driver's drivers/gpu/drm/imx/parallel-display.c?
> 
> encoder is the thing between crtc and connector. It's exposed to
> userspace (which is a uapi design accident, but can't change that),
> hence why it's required and some drivers have dummy ones.
> 
> The real usage is purely internally for the atomic helpers. Most
> callbacks in the encoder should be optional, so a dummy encoder should
> be a lot thinner than your imx example. The imx example should be
> using the panel-bridge I think, which would take care of most of the
> boilerplate (but panel-bridge didn't exist back then I guess).
> 
> > 3.) My patch set currently contains the driver for the Himax that is
> >     modelled after tda998x. That one implements an encoder. Similar drivers
> >     seem to add a bridge too, but it's not entirely clear to me what a bridge
> >     is good for?
> 
> tda998x predates bridges, which is why there's still some
> encoder_helper usage in there. It's become a proper bridge driver now
> I think (but some drivers still use the old interface). All new
> transcoder chip drivers should be bridges. Did you look at the
> kerneldoc for bridges? If those aren't clear we need to fix them.
> 
> > 4.) How shall I expose the fancy functionality of the Himax to the userspace?
> >     Notably, the freeze frame. The OLPC laptops with the stock firmware like
> >     to suspend the SoC very aggressively to save power (in 20 seconds of
> >     inactivity or so). If the display is open (it can also be turned around
> >     for a tablet or e-book mode), it makes sense to freeze the picture and
> >     keep the panel on, if the laptop is closed, we want to turn it off.
> >     Should the behavior be exposed via sysfs (as it is with the current OLPC
> >     kernels), or a DRM property? Would it require libdrm support?
> 
> I've accidentally seen how that's implemented for fbdev in the older
> olpc drivers, and that's definitely _not_ how we want to support this.
> Essentially what you have here is a fancy self-refresh panel. That's
> already fully supported by DRM uapi, so I'd say first implement that.
> Once we have that we can figure out a special property that tells the
> driver to not shut down the screen on suspend, but just go into
> self-refresh. Implementing the self-refresh stuff in DRM will be the
> big pile of work (since that's something which goes beyond the generic
> drm_bridge interface).
> 
> 
> >   Himax HX8837 "DCON"
> >   -------------------
> >   RGB input from LCDC
> >   Controlled via I2C
> 
> These two shouldn't be a problem.
> 
> >   Backlight control
> 
> We already have backlight interfaces
> 
> >   Can slow down the panel refresh rate to save power
> 
> Just expose this as modes with different vrefresh (and do that
> modeswitch without doing a full modeset). Bonus if you do it
> automatically, which can be done with the self-refresh infrastructure.
>
> >   Optional dithering for color mode, "swizzling"
> 
> Not exactly clear, but I'm assuming the bus_format stuff on bridges
> should help with this.
> 
> >   Has DRAM attached, can freeze a single frame in it
> 
> Classic self-refresh support. Noralf Tronnes has done great work here
> past year to improve the infrastructure and let drivers implement this
> with less typing. Also look at the recent work by vmwgfx folks.

I'm wondering if you can share some more specific pointers?

Grepping for "self-refresh" yields references to the eDP self refresh,
but that seems not relevant as that one depends on hardware to decide
when to enter the self-refresh mode.

Perhaps I was not looking at the right place (I searched git and
patchwork), but I was not able to find work that would seem relevant. 

> Cheers, Daniel

Thank you,
Lubo

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

Re: [RFC 12/16] drm/armada: add bus-width property to the output endpoint

[Russell King - ARM Linux]

On Tue, Dec 18, 2018 at 04:37:38PM +0100, Lubomir Rintel wrote:
> This makes it possible to choose a different pixel format for the
> endpoint. Modelled after what other LCD controllers use, including
> marvell,pxa2xx-lcdc and atmel,hlcdc-display-controller and perhaps more.
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
>  drivers/gpu/drm/armada/armada_crtc.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
> index da9360688b55..5400fb925bcd 100644
> --- a/drivers/gpu/drm/armada/armada_crtc.c
> +++ b/drivers/gpu/drm/armada/armada_crtc.c
> @@ -724,6 +724,8 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev,
>  	struct armada_private *priv = drm->dev_private;
>  	struct armada_crtc *dcrtc;
>  	struct drm_plane *primary;
> +	struct device_node *endpoint;
> +	u32 bus_width = 24;
>  	void __iomem *base;
>  	int ret;
>  
> @@ -744,8 +746,30 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev,
>  	dcrtc->base = base;
>  	dcrtc->num = drm->mode_config.num_crtc;
>  	dcrtc->clk = ERR_PTR(-EINVAL);
> -	dcrtc->cfg_dumb_ctrl = DUMB24_RGB888_0;
>  	dcrtc->spu_iopad_ctrl = CFG_VSCALE_LN_EN | CFG_IOPAD_DUMB24;
> +
> +	endpoint = of_get_next_child(port, NULL);
> +	of_property_read_u32(endpoint, "bus-width", &bus_width);
> +	of_node_put(endpoint);
> +
> +	switch (bus_width) {
> +	case 12:
> +		dcrtc->cfg_dumb_ctrl = DUMB12_RGB444_0;
> +		break;
> +	case 16:
> +		dcrtc->cfg_dumb_ctrl = DUMB16_RGB565_0;
> +		break;
> +	case 18:
> +		dcrtc->cfg_dumb_ctrl = DUMB18_RGB666_0;
> +		break;
> +	case 24:
> +		dcrtc->cfg_dumb_ctrl = DUMB24_RGB888_0;
> +		break;
> +	default:
> +		DRM_ERROR("unsupported bus width: %d\n", bus_width);
> +		return -EINVAL;
> +	}
> +

This needs to default to 24bpp if the property is not present.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 12/16] drm/armada: add bus-width property to the output endpoint

[Lubomir Rintel]

On Thu, 2019-01-03 at 13:15 +0000, Russell King - ARM Linux wrote:
> On Tue, Dec 18, 2018 at 04:37:38PM +0100, Lubomir Rintel wrote:
> > This makes it possible to choose a different pixel format for the
> > endpoint. Modelled after what other LCD controllers use, including
> > marvell,pxa2xx-lcdc and atmel,hlcdc-display-controller and perhaps
> > more.
> > 
> > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > ---
> >  drivers/gpu/drm/armada/armada_crtc.c | 26
> > +++++++++++++++++++++++++-
> >  1 file changed, 25 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/armada/armada_crtc.c
> > b/drivers/gpu/drm/armada/armada_crtc.c
> > index da9360688b55..5400fb925bcd 100644
> > --- a/drivers/gpu/drm/armada/armada_crtc.c
> > +++ b/drivers/gpu/drm/armada/armada_crtc.c
> > @@ -724,6 +724,8 @@ static int armada_drm_crtc_create(struct
> > drm_device *drm, struct device *dev,
> >  	struct armada_private *priv = drm->dev_private;
> >  	struct armada_crtc *dcrtc;
> >  	struct drm_plane *primary;
> > +	struct device_node *endpoint;
> > +	u32 bus_width = 24;
> >  	void __iomem *base;
> >  	int ret;
> >  
> > @@ -744,8 +746,30 @@ static int armada_drm_crtc_create(struct
> > drm_device *drm, struct device *dev,
> >  	dcrtc->base = base;
> >  	dcrtc->num = drm->mode_config.num_crtc;
> >  	dcrtc->clk = ERR_PTR(-EINVAL);
> > -	dcrtc->cfg_dumb_ctrl = DUMB24_RGB888_0;
> >  	dcrtc->spu_iopad_ctrl = CFG_VSCALE_LN_EN | CFG_IOPAD_DUMB24;
> > +
> > +	endpoint = of_get_next_child(port, NULL);
> > +	of_property_read_u32(endpoint, "bus-width", &bus_width);
> > +	of_node_put(endpoint);
> > +
> > +	switch (bus_width) {
> > +	case 12:
> > +		dcrtc->cfg_dumb_ctrl = DUMB12_RGB444_0;
> > +		break;
> > +	case 16:
> > +		dcrtc->cfg_dumb_ctrl = DUMB16_RGB565_0;
> > +		break;
> > +	case 18:
> > +		dcrtc->cfg_dumb_ctrl = DUMB18_RGB666_0;
> > +		break;
> > +	case 24:
> > +		dcrtc->cfg_dumb_ctrl = DUMB24_RGB888_0;
> > +		break;
> > +	default:
> > +		DRM_ERROR("unsupported bus width: %d\n", bus_width);
> > +		return -EINVAL;
> > +	}
> > +
> 
> This needs to default to 24bpp if the property is not present.

It actually does: bus_width is initialized to 24 and
of_property_read_u32() does not change it if the property is not
present.

Perhaps the style is poor and I should just set the bus_width=24 in the
case's default branch instead.

Lubo

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

Re: [RFC 12/16] drm/armada: add bus-width property to the output endpoint

[Russell King - ARM Linux]

On Thu, Jan 03, 2019 at 02:20:00PM +0100, Lubomir Rintel wrote:
> On Thu, 2019-01-03 at 13:15 +0000, Russell King - ARM Linux wrote:
> > On Tue, Dec 18, 2018 at 04:37:38PM +0100, Lubomir Rintel wrote:
> > > This makes it possible to choose a different pixel format for the
> > > endpoint. Modelled after what other LCD controllers use, including
> > > marvell,pxa2xx-lcdc and atmel,hlcdc-display-controller and perhaps
> > > more.
> > > 
> > > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > > ---
> > >  drivers/gpu/drm/armada/armada_crtc.c | 26
> > > +++++++++++++++++++++++++-
> > >  1 file changed, 25 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/armada/armada_crtc.c
> > > b/drivers/gpu/drm/armada/armada_crtc.c
> > > index da9360688b55..5400fb925bcd 100644
> > > --- a/drivers/gpu/drm/armada/armada_crtc.c
> > > +++ b/drivers/gpu/drm/armada/armada_crtc.c
> > > @@ -724,6 +724,8 @@ static int armada_drm_crtc_create(struct
> > > drm_device *drm, struct device *dev,
> > >  	struct armada_private *priv = drm->dev_private;
> > >  	struct armada_crtc *dcrtc;
> > >  	struct drm_plane *primary;
> > > +	struct device_node *endpoint;
> > > +	u32 bus_width = 24;
> > >  	void __iomem *base;
> > >  	int ret;
> > >  
> > > @@ -744,8 +746,30 @@ static int armada_drm_crtc_create(struct
> > > drm_device *drm, struct device *dev,
> > >  	dcrtc->base = base;
> > >  	dcrtc->num = drm->mode_config.num_crtc;
> > >  	dcrtc->clk = ERR_PTR(-EINVAL);
> > > -	dcrtc->cfg_dumb_ctrl = DUMB24_RGB888_0;
> > >  	dcrtc->spu_iopad_ctrl = CFG_VSCALE_LN_EN | CFG_IOPAD_DUMB24;
> > > +
> > > +	endpoint = of_get_next_child(port, NULL);
> > > +	of_property_read_u32(endpoint, "bus-width", &bus_width);
> > > +	of_node_put(endpoint);
> > > +
> > > +	switch (bus_width) {
> > > +	case 12:
> > > +		dcrtc->cfg_dumb_ctrl = DUMB12_RGB444_0;
> > > +		break;
> > > +	case 16:
> > > +		dcrtc->cfg_dumb_ctrl = DUMB16_RGB565_0;
> > > +		break;
> > > +	case 18:
> > > +		dcrtc->cfg_dumb_ctrl = DUMB18_RGB666_0;
> > > +		break;
> > > +	case 24:
> > > +		dcrtc->cfg_dumb_ctrl = DUMB24_RGB888_0;
> > > +		break;
> > > +	default:
> > > +		DRM_ERROR("unsupported bus width: %d\n", bus_width);
> > > +		return -EINVAL;
> > > +	}
> > > +
> > 
> > This needs to default to 24bpp if the property is not present.
> 
> It actually does: bus_width is initialized to 24 and
> of_property_read_u32() does not change it if the property is not
> present.
> 
> Perhaps the style is poor and I should just set the bus_width=24 in the
> case's default branch instead.

No, that's fine then.  Thanks for pointing that out.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 00/16] Armada DRM support for OLPC XO-1.75 laptop

[Daniel Vetter]

On Thu, Jan 03, 2019 at 11:03:36AM +0100, Lubomir Rintel wrote:
> Hi,
> 
> On Wed, 2018-12-19 at 10:13 +0100, Daniel Vetter wrote:
> > On Wed, Dec 19, 2018 at 9:40 AM Lubomir Rintel <lkundrak@v3.sk> wrote:
> > > Hi,
> > > 
> > > here are patches that make the Armada DRM work on an OLPC XO-1.75.
> > > They build on patches previously submitted by Russel King (included here for
> > > completeness as well).
> > > 
> > > They certainly need some more love, but I'm not able to improve them until
> > > I get to understand some things first. I'm posting a couple of questions
> > > below in hope someone is kind enough to help me deal with my confusion.
> > > 
> > > The display pipeline on the laptop looks like this:
> > > 
> > >   Armada LCDC
> > >   -----------
> > >       |
> > >       v
> > > 
> > >   Himax HX8837 "DCON"
> > >   -------------------
> > >   RGB input from LCDC
> > >   Controlled via I2C
> > >   Backlight control
> > >   Can slow down the panel refresh rate to save power
> > >   Optional dithering for color mode, "swizzling"
> > >   Has DRAM attached, can freeze a single frame in it
> > >   Doc: http://wiki.laptop.org/images/0/09/DCON_datasheet_HX8837-A.pdf
> > >       |
> > >       v
> > > 
> > >   Innolux LS075AT011 Panel
> > >   ------------------------
> > >   7.5", 1200x900
> > >   No datasheet.
> > >   http://wiki.laptop.org/go/Display
> > >   Can opreate in two modes:
> > > 
> > >    R G B
> > >    G B R ...   or   "e-book" daylight readable
> > >    B R G            reflexive B&W
> > >      .
> > >      :
> > > 
> > > Here's what's not clear to me:
> > > 
> > > 1.) Is the Himax chip an encoder here?
> > 
> > Since it's an external IP probably better to model it as a bridge.
> > 
> > > 2.) What's the use of an encoder anyways? If a panel was connected directly
> > >     do the RGB output from the LCDC, would we have to fake one? Is that the
> > >     point of things like i.MX driver's drivers/gpu/drm/imx/parallel-display.c?
> > 
> > encoder is the thing between crtc and connector. It's exposed to
> > userspace (which is a uapi design accident, but can't change that),
> > hence why it's required and some drivers have dummy ones.
> > 
> > The real usage is purely internally for the atomic helpers. Most
> > callbacks in the encoder should be optional, so a dummy encoder should
> > be a lot thinner than your imx example. The imx example should be
> > using the panel-bridge I think, which would take care of most of the
> > boilerplate (but panel-bridge didn't exist back then I guess).
> > 
> > > 3.) My patch set currently contains the driver for the Himax that is
> > >     modelled after tda998x. That one implements an encoder. Similar drivers
> > >     seem to add a bridge too, but it's not entirely clear to me what a bridge
> > >     is good for?
> > 
> > tda998x predates bridges, which is why there's still some
> > encoder_helper usage in there. It's become a proper bridge driver now
> > I think (but some drivers still use the old interface). All new
> > transcoder chip drivers should be bridges. Did you look at the
> > kerneldoc for bridges? If those aren't clear we need to fix them.
> > 
> > > 4.) How shall I expose the fancy functionality of the Himax to the userspace?
> > >     Notably, the freeze frame. The OLPC laptops with the stock firmware like
> > >     to suspend the SoC very aggressively to save power (in 20 seconds of
> > >     inactivity or so). If the display is open (it can also be turned around
> > >     for a tablet or e-book mode), it makes sense to freeze the picture and
> > >     keep the panel on, if the laptop is closed, we want to turn it off.
> > >     Should the behavior be exposed via sysfs (as it is with the current OLPC
> > >     kernels), or a DRM property? Would it require libdrm support?
> > 
> > I've accidentally seen how that's implemented for fbdev in the older
> > olpc drivers, and that's definitely _not_ how we want to support this.
> > Essentially what you have here is a fancy self-refresh panel. That's
> > already fully supported by DRM uapi, so I'd say first implement that.
> > Once we have that we can figure out a special property that tells the
> > driver to not shut down the screen on suspend, but just go into
> > self-refresh. Implementing the self-refresh stuff in DRM will be the
> > big pile of work (since that's something which goes beyond the generic
> > drm_bridge interface).
> > 
> > 
> > >   Himax HX8837 "DCON"
> > >   -------------------
> > >   RGB input from LCDC
> > >   Controlled via I2C
> > 
> > These two shouldn't be a problem.
> > 
> > >   Backlight control
> > 
> > We already have backlight interfaces
> > 
> > >   Can slow down the panel refresh rate to save power
> > 
> > Just expose this as modes with different vrefresh (and do that
> > modeswitch without doing a full modeset). Bonus if you do it
> > automatically, which can be done with the self-refresh infrastructure.
> >
> > >   Optional dithering for color mode, "swizzling"
> > 
> > Not exactly clear, but I'm assuming the bus_format stuff on bridges
> > should help with this.
> > 
> > >   Has DRAM attached, can freeze a single frame in it
> > 
> > Classic self-refresh support. Noralf Tronnes has done great work here
> > past year to improve the infrastructure and let drivers implement this
> > with less typing. Also look at the recent work by vmwgfx folks.
> 
> I'm wondering if you can share some more specific pointers?
> 
> Grepping for "self-refresh" yields references to the eDP self refresh,
> but that seems not relevant as that one depends on hardware to decide
> when to enter the self-refresh mode.
> 
> Perhaps I was not looking at the right place (I searched git and
> patchwork), but I was not able to find work that would seem relevant. 

self-refresh is a generic term not tied to eDP. It just means that the
panel can refresh itself, and doesn't need to be fed the pixel data all
the time (as long as nothing changes), allowing the display hw (and in
most cases also the entire SoC) to be shut down.

How exactly it's implemented is up to the panel/display hw. For eDP
there's no requirement that the hw detects changes, that's just how i915
implements it. Afaik other drivers implemented self-refresh entirely using
software logic. Self-refresh also exists for dsi, and the olpc hw you have
here (and the fbdev driver thing I've recently found) seems to fit that
paradigm too.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 00/16] Armada DRM support for OLPC XO-1.75 laptop

[Russell King - ARM Linux admin]

On Tue, Dec 18, 2018 at 04:37:26PM +0100, Lubomir Rintel wrote:
> here are patches that make the Armada DRM work on an OLPC XO-1.75.
> They build on patches previously submitted by Russel King (included here for
> completeness as well).

Hi,

Would it be possible for you to re-post patches 1 through 6 to include
the DT maintainers so that they can review those patches (as is
required for their acceptance) - once they have done that, I can pick
them up and resubmit some of the patches I have queued.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 14/16] drm/armada: optionally enable the AXI clock

[Russell King - ARM Linux admin]

On Tue, Dec 18, 2018 at 04:37:40PM +0100, Lubomir Rintel wrote:
> It needs to be enabled (at least on MMP2) in order for the register
> writes to LCDC to work.

Dove also has an AXI clock input to the LCDC, but it isn't clear
whether that is necessary for register access or not.  From a quick
search of the documentation, there doesn't appear to be an enable bit
for the AXI clock.  Unfortunately, the documentation of clocks on Dove
is not very good.

However, Dove LCDCs can have four clock inputs for the pixel clock -
AXI, PLL and two external clock inputs.  It isn't clear what this AXI
clock is, and whether it's the same clock used for register access.

Can you check whether the AXI clock used for the pixel clock is the
same as the AXI bus clock for MMP2 and document that please?

Thanks.

> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
>  drivers/gpu/drm/armada/armada_crtc.c | 7 +++++++
>  drivers/gpu/drm/armada/armada_crtc.h | 1 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
> index 5400fb925bcd..973c377975a1 100644
> --- a/drivers/gpu/drm/armada/armada_crtc.c
> +++ b/drivers/gpu/drm/armada/armada_crtc.c
> @@ -679,6 +679,7 @@ static void armada_drm_crtc_destroy(struct drm_crtc *crtc)
>  
>  	of_node_put(dcrtc->crtc.port);
>  
> +	clk_disable_unprepare(dcrtc->axiclk);
>  	kfree(dcrtc);
>  }
>  
> @@ -748,6 +749,11 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev,
>  	dcrtc->clk = ERR_PTR(-EINVAL);
>  	dcrtc->spu_iopad_ctrl = CFG_VSCALE_LN_EN | CFG_IOPAD_DUMB24;
>  
> +	dcrtc->axiclk = devm_clk_get(dev, "axiclk");
> +	if (IS_ERR(dcrtc->axiclk))
> +		dcrtc->axiclk = NULL;
> +	WARN_ON(clk_prepare_enable(dcrtc->axiclk));
> +
>  	endpoint = of_get_next_child(port, NULL);
>  	of_property_read_u32(endpoint, "bus-width", &bus_width);
>  	of_node_put(endpoint);
> @@ -829,6 +835,7 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev,
>  err_crtc_init:
>  	primary->funcs->destroy(primary);
>  err_crtc:
> +	clk_disable_unprepare(dcrtc->axiclk);
>  	kfree(dcrtc);
>  
>  	return ret;
> diff --git a/drivers/gpu/drm/armada/armada_crtc.h b/drivers/gpu/drm/armada/armada_crtc.h
> index 7ebd337b60af..b07faea7257d 100644
> --- a/drivers/gpu/drm/armada/armada_crtc.h
> +++ b/drivers/gpu/drm/armada/armada_crtc.h
> @@ -39,6 +39,7 @@ struct armada_crtc {
>  	const struct armada_variant *variant;
>  	unsigned		num;
>  	void __iomem		*base;
> +	struct clk		*axiclk;
>  	struct clk		*clk;
>  	struct clk		*extclk[2];
>  	struct {
> -- 
> 2.19.1
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 00/16] Armada DRM support for OLPC XO-1.75 laptop

[Lubomir Rintel]

On Thu, 2019-01-17 at 10:55 +0000, Russell King - ARM Linux admin
wrote:
> On Tue, Dec 18, 2018 at 04:37:26PM +0100, Lubomir Rintel wrote:
> > here are patches that make the Armada DRM work on an OLPC XO-1.75.
> > They build on patches previously submitted by Russel King (included here for
> > completeness as well).
> 
> Hi,
> 
> Would it be possible for you to re-post patches 1 through 6 to include
> the DT maintainers so that they can review those patches (as is
> required for their acceptance) - once they have done that, I can pick
> them up and resubmit some of the patches I have queued.

Yes. Before I do so, I'd like to know your opinion about the compatible
strings of the framebuffer and display-subsystem nodes.

In the "[RFC 05/16] dt-bindings: display: armada: Add mmp2 compatible
strings" message [1] I wrote:
> Note that perhaps "marvell,armada-display-subsystem" and
> "marvell,armada-framebuffer" would be a good idea in addition of
> dove/mmp2 specific strings since (at least for now) the driver code
> is the same.

[1] https://lists.freedesktop.org/archives/dri-devel/2018-December/201016.html

I essentially documented what you implemented, but I'd prefer if you
used the "marvell,armada-display-subsystem" and
"marvell,armada-framebuffer" compatible strings in the driver code
instead.

What do you think?

> 
> Thanks.

Thank you
Lubo

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

Re: [RFC 00/16] Armada DRM support for OLPC XO-1.75 laptop

[Russell King - ARM Linux admin]

On Thu, Jan 17, 2019 at 04:03:45PM +0100, Lubomir Rintel wrote:
> On Thu, 2019-01-17 at 10:55 +0000, Russell King - ARM Linux admin
> wrote:
> > On Tue, Dec 18, 2018 at 04:37:26PM +0100, Lubomir Rintel wrote:
> > > here are patches that make the Armada DRM work on an OLPC XO-1.75.
> > > They build on patches previously submitted by Russel King (included here for
> > > completeness as well).
> > 
> > Hi,
> > 
> > Would it be possible for you to re-post patches 1 through 6 to include
> > the DT maintainers so that they can review those patches (as is
> > required for their acceptance) - once they have done that, I can pick
> > them up and resubmit some of the patches I have queued.
> 
> Yes. Before I do so, I'd like to know your opinion about the compatible
> strings of the framebuffer and display-subsystem nodes.
> 
> In the "[RFC 05/16] dt-bindings: display: armada: Add mmp2 compatible
> strings" message [1] I wrote:
> > Note that perhaps "marvell,armada-display-subsystem" and
> > "marvell,armada-framebuffer" would be a good idea in addition of
> > dove/mmp2 specific strings since (at least for now) the driver code
> > is the same.
> 
> [1] https://lists.freedesktop.org/archives/dri-devel/2018-December/201016.html
> 
> I essentially documented what you implemented, but I'd prefer if you
> used the "marvell,armada-display-subsystem" and
> "marvell,armada-framebuffer" compatible strings in the driver code
> instead.

Things in DT tend to be named by the first user who comes along - so
we often get DT compatibles that refer to the first device.  However,
we do currently have the opportunity to change it, so please do as you
suggest.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 14/16] drm/armada: optionally enable the AXI clock

[Lubomir Rintel]

On Thu, 2019-01-17 at 11:09 +0000, Russell King - ARM Linux admin
wrote:
> On Tue, Dec 18, 2018 at 04:37:40PM +0100, Lubomir Rintel wrote:
> > It needs to be enabled (at least on MMP2) in order for the register
> > writes to LCDC to work.
> 
> Dove also has an AXI clock input to the LCDC, but it isn't clear
> whether that is necessary for register access or not.  From a quick
> search of the documentation, there doesn't appear to be an enable bit
> for the AXI clock.  Unfortunately, the documentation of clocks on Dove
> is not very good.
> 
> However, Dove LCDCs can have four clock inputs for the pixel clock -
> AXI, PLL and two external clock inputs.  It isn't clear what this AXI
> clock is, and whether it's the same clock used for register access.

The MMP2 LCDC also has four. I don't have the documentation but
according to James [1], who has one, there are:

0 - AXI clock
1 - Display 1, from APMU
2 - Display 2, from APMU
3 - HDMI PLL

[1] https://lists.freedesktop.org/archives/dri-devel/2018-December/201021.html

> Can you check whether the AXI clock used for the pixel clock is the
> same as the AXI bus clock for MMP2 and document that please?

Turns out -- it is not. The PMU generates a separate "peripheral" clock
that is required for the LCDC register access and two "AXI" clocks
(with a configurable divider).

However, the clk-of-mmp2 driver confuses this a bit: it exports the
first AXI clock together with peripheral clock as a single clock [2].

[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/mmp/clk-of-mmp2.c?h=v4.20#n232

There's no documentation, but openfirmware strongly suggests it is the
case [3] (d428284c is the APMU's Display 1 control register) :

   setreg d428284c 08    \ PMUA_DISPLAY1_CLK_RES_CTL - AXI Clk enabled
   setreg d428284c 09    \ plus AXI released from reset
   setreg d428284c 19    \ plus peripheral clock enabled
   setreg d428284c 1b    \ plus peripheral released from reset

[3] http://dev.laptop.org/git/users/quozl/openfirmware/tree/cpu/arm/mmp2/sp.bth#n322

I guess I'll need to fix clk-of-mmp2 to expose the two as separate
clocks...

Here are the configurations I've observed to work on the XO laptop,
confirming the above:

(0xd428284c = PMUA_DISPLAY1_CLK_RES_CTL, APMU Display 1 clocks
0xd4282910 = PMUA_DISPLAY1_CLK_RES_CTL, APMU Display 2 clocks
0xd420b1a8 = LCDC LCD_CFG_SCLK_DIV)

  Only the peripheral clock enabled in PMUA:
  # busybox devmem 0xd428284c 32 0x00000009
  # busybox devmem 0xd4282910 32 0x00000000
  Let LCDC use its AXI clock (0), divisor 5 (dunno why)
  # busybox devmem 0xd420b1a8 32 0x00001105
 
  Enable perhipheral clock and generate display 1 clock
  from AXI, divisor 7
  # busybox devmem 0xd428284c 32 0x0000071b
  # busybox devmem 0xd4282910 32 0x00000000
  Pick display 1 clock, divisor 2
  # busybox devmem 0xd420b1a8 32 0x40001102
  This is the configuration used by OLPC firmware
 
  Enable perhipheral clock and generate display 2 clock
  from AXI, divisor 7
  # busybox devmem 0xd428284c 32 0x00000009
  # busybox devmem 0xd4282910 32 0x00000712
  Pick display 2 clock, divisor 2
  # busybox devmem 0xd420b1a8 32 0x80001102

I don't actually have a clue which clock is actually used as an input
when the LCDC is configured to use "AXI" clock. I don't think I need to
care at this point -- I'm not going to use it and will go with
"Display  1".

Please let me know if the above makes sense to you and I'll go ahead
and prepare (hopefully well commented) patches.

Thank you
Lubo

> 
> Thanks.
> 
> > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > ---
> >  drivers/gpu/drm/armada/armada_crtc.c | 7 +++++++
> >  drivers/gpu/drm/armada/armada_crtc.h | 1 +
> >  2 files changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
> > index 5400fb925bcd..973c377975a1 100644
> > --- a/drivers/gpu/drm/armada/armada_crtc.c
> > +++ b/drivers/gpu/drm/armada/armada_crtc.c
> > @@ -679,6 +679,7 @@ static void armada_drm_crtc_destroy(struct drm_crtc *crtc)
> >  
> >  	of_node_put(dcrtc->crtc.port);
> >  
> > +	clk_disable_unprepare(dcrtc->axiclk);
> >  	kfree(dcrtc);
> >  }
> >  
> > @@ -748,6 +749,11 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev,
> >  	dcrtc->clk = ERR_PTR(-EINVAL);
> >  	dcrtc->spu_iopad_ctrl = CFG_VSCALE_LN_EN | CFG_IOPAD_DUMB24;
> >  
> > +	dcrtc->axiclk = devm_clk_get(dev, "axiclk");
> > +	if (IS_ERR(dcrtc->axiclk))
> > +		dcrtc->axiclk = NULL;
> > +	WARN_ON(clk_prepare_enable(dcrtc->axiclk));
> > +
> >  	endpoint = of_get_next_child(port, NULL);
> >  	of_property_read_u32(endpoint, "bus-width", &bus_width);
> >  	of_node_put(endpoint);
> > @@ -829,6 +835,7 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev,
> >  err_crtc_init:
> >  	primary->funcs->destroy(primary);
> >  err_crtc:
> > +	clk_disable_unprepare(dcrtc->axiclk);
> >  	kfree(dcrtc);
> >  
> >  	return ret;
> > diff --git a/drivers/gpu/drm/armada/armada_crtc.h b/drivers/gpu/drm/armada/armada_crtc.h
> > index 7ebd337b60af..b07faea7257d 100644
> > --- a/drivers/gpu/drm/armada/armada_crtc.h
> > +++ b/drivers/gpu/drm/armada/armada_crtc.h
> > @@ -39,6 +39,7 @@ struct armada_crtc {
> >  	const struct armada_variant *variant;
> >  	unsigned		num;
> >  	void __iomem		*base;
> > +	struct clk		*axiclk;
> >  	struct clk		*clk;
> >  	struct clk		*extclk[2];
> >  	struct {
> > -- 
> > 2.19.1
> > 

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