Hi everyone,
This series adds support for the first display pipeline found on the
A31 and A31s SoCs, with output through the RGB LCD interface.
This has been tested on the Sinlinx SinA31s development board, with
its included 7" LCD panel, and the Merrii Hummingbird A31 development
board, with its dumb VGA DAC bridge.
Changes since v1:
- Made quirk structures static
- Dropped unused/unsupported quirks
- Dropped dotclock max frequency quirk patch.
The check for maximum PLL clock will be moved to the clk driver.
- Changed is_sun5i quirk to has_unknown_mux
- Dropped SinA31s LCD patch
- Added patch to support enable pin GPIO for dumb VGA DACs
- Added patch to enable VGA output via dumb VGA DAC on Hummingbird
A31 board
Regards
ChenYu
Chen-Yu Tsai (8):
drm/bridge: rgb-to-vga: Support an enable GPIO
drm/sun4i: sun6i-drc: Support DRC on A31 and A31s
drm/sun4i: tcon: Move SoC specific quirks to a DT matched data
structure
drm/sun4i: Add compatible string for A31/A31s TCON (timing controller)
drm/sun4i: Add compatible strings for A31/A31s display pipelines
ARM: dts: sun6i: Add device nodes for first display pipeline
ARM: dts: sun6i: Add A31 LCD0 RGB888 pins
ARM: dts: sun6i: hummingbird-a31: Enable display output through VGA
bridge
.../bindings/display/bridge/dumb-vga-dac.txt | 2 +
.../bindings/display/sunxi/sun4i-drm.txt | 10 +-
arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 56 +++++++
arch/arm/boot/dts/sun6i-a31.dtsi | 165 +++++++++++++++++++++
arch/arm/boot/dts/sun6i-a31s.dtsi | 8 +
drivers/gpu/drm/bridge/dumb-vga-dac.c | 28 ++++
drivers/gpu/drm/sun4i/sun4i_backend.c | 1 +
drivers/gpu/drm/sun4i/sun4i_drv.c | 5 +
drivers/gpu/drm/sun4i/sun4i_tcon.c | 43 ++++--
drivers/gpu/drm/sun4i/sun4i_tcon.h | 11 +-
drivers/gpu/drm/sun4i/sun6i_drc.c | 2 +
11 files changed, 311 insertions(+), 20 deletions(-)
--
2.9.3
Some rgb-to-vga bridges have an enable GPIO, either directly tied to
an enable pin on the bridge IC, or indirectly controlling a power
switch.
Add support for it.
Signed-off-by: Chen-Yu Tsai <[email protected]>
---
.../bindings/display/bridge/dumb-vga-dac.txt | 2 ++
drivers/gpu/drm/bridge/dumb-vga-dac.c | 28 ++++++++++++++++++++++
2 files changed, 30 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
index 003bc246a270..d3484822bf77 100644
--- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
+++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
@@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
- Video port 0 for RGB input
- Video port 1 for VGA output
+Optional properties:
+- enable-gpios: GPIO pin to enable or disable the bridge
Example
-------
diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index afec232185a7..b487e5e9b56d 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -10,6 +10,7 @@
* the License, or (at your option) any later version.
*/
+#include <linux/gpio/consumer.h>
#include <linux/module.h>
#include <linux/of_graph.h>
@@ -23,6 +24,7 @@ struct dumb_vga {
struct drm_connector connector;
struct i2c_adapter *ddc;
+ struct gpio_desc *enable_gpio;
};
static inline struct dumb_vga *
@@ -124,8 +126,26 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
return 0;
}
+static void dumb_vga_enable(struct drm_bridge *bridge)
+{
+ struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
+
+ if (vga->enable_gpio)
+ gpiod_set_value_cansleep(vga->enable_gpio, 1);
+}
+
+static void dumb_vga_disable(struct drm_bridge *bridge)
+{
+ struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
+
+ if (vga->enable_gpio)
+ gpiod_set_value_cansleep(vga->enable_gpio, 0);
+}
+
static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
.attach = dumb_vga_attach,
+ .enable = dumb_vga_enable,
+ .disable = dumb_vga_disable,
};
static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
@@ -169,6 +189,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
return -ENOMEM;
platform_set_drvdata(pdev, vga);
+ vga->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(vga->enable_gpio)) {
+ ret = PTR_ERR(vga->enable_gpio);
+ dev_err(&pdev->dev, "failed to request GPIO: %d\n", ret);
+ return ret;
+ }
+
vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
if (IS_ERR(vga->ddc)) {
if (PTR_ERR(vga->ddc) == -ENODEV) {
--
2.9.3
The Hummingbird A31 board has a RGB-to-VGA bridge which converts RGB
output from the LCD interface to VGA signals.
Enable this part of the display pipeline.
Signed-off-by: Chen-Yu Tsai <[email protected]>
---
arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 56 +++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
index 9a74637f677f..05a49b2147f1 100644
--- a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
+++ b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
@@ -63,6 +63,49 @@
stdout-path = "serial0:115200n8";
};
+ bridge {
+ compatible = "dumb-vga-dac";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ vga_bridge_in: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&tcon0_out_vga>;
+ };
+ };
+
+ port@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+
+ vga_bridge_out: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&vga_con_in>;
+ };
+ };
+ };
+ };
+
+ vga {
+ compatible = "vga-connector";
+
+ port {
+ vga_con_in: endpoint {
+ remote-endpoint = <&vga_bridge_out>;
+ };
+ };
+ };
+
wifi_pwrseq: wifi_pwrseq {
compatible = "mmc-pwrseq-simple";
reset-gpios = <&pio 6 10 GPIO_ACTIVE_LOW>; /* PG10 */
@@ -245,6 +288,19 @@
status = "okay";
};
+&tcon0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&lcd0_rgb888_pins>;
+ status = "okay";
+};
+
+&tcon0_out {
+ tcon0_out_vga: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&vga_bridge_in>;
+ };
+};
+
&uart0 {
pinctrl-names = "default";
pinctrl-0 = <&uart0_pins_a>;
--
2.9.3
The A31 and A31s also have the DRC as part of the display pipeline.
As we know virtually nothing about them, just add compatible strings
for both SoCs to the stub driver.
Signed-off-by: Chen-Yu Tsai <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 2 ++
drivers/gpu/drm/sun4i/sun6i_drc.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
index b95696d748c7..5368961cd727 100644
--- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
+++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
@@ -64,6 +64,8 @@ adaptive backlight control.
Required properties:
- compatible: value must be one of:
+ * allwinner,sun6i-a31-drc
+ * allwinner,sun6i-a31s-drc
* allwinner,sun8i-a33-drc
- reg: base address and size of the memory-mapped region.
- interrupts: interrupt associated to this IP
diff --git a/drivers/gpu/drm/sun4i/sun6i_drc.c b/drivers/gpu/drm/sun4i/sun6i_drc.c
index bf6d846d8132..6ef707c5a719 100644
--- a/drivers/gpu/drm/sun4i/sun6i_drc.c
+++ b/drivers/gpu/drm/sun4i/sun6i_drc.c
@@ -98,6 +98,8 @@ static int sun6i_drc_remove(struct platform_device *pdev)
}
static const struct of_device_id sun6i_drc_of_table[] = {
+ { .compatible = "allwinner,sun6i-a31-drc" },
+ { .compatible = "allwinner,sun6i-a31s-drc" },
{ .compatible = "allwinner,sun8i-a33-drc" },
{ }
};
--
2.9.3
The LCD0 controller on the A31 can do RGB output up to 8 bits per
channel. Add the pins for RGB888 output.
Signed-off-by: Chen-Yu Tsai <[email protected]>
---
arch/arm/boot/dts/sun6i-a31.dtsi | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index 4d2c7786b92a..2e8bf93dcfb2 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -540,6 +540,19 @@
allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
};
+ lcd0_rgb888_pins: lcd0_rgb888 {
+ allwinner,pins = "PD0", "PD1", "PD2", "PD3",
+ "PD4", "PD5", "PD6", "PD7",
+ "PD8", "PD9", "PD10", "PD11",
+ "PD12", "PD13", "PD14", "PD15",
+ "PD16", "PD17", "PD18", "PD19",
+ "PD20", "PD21", "PD22", "PD23",
+ "PD24", "PD25", "PD26", "PD27";
+ allwinner,function = "lcd0";
+ allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+ allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+ };
+
mmc0_pins_a: mmc0@0 {
allwinner,pins = "PF0", "PF1", "PF2",
"PF3", "PF4", "PF5";
--
2.9.3
The A31 has 2 parallel display pipelines, which can be intermixed.
However the driver currently only supports one of them.
Signed-off-by: Chen-Yu Tsai <[email protected]>
---
arch/arm/boot/dts/sun6i-a31.dtsi | 152 ++++++++++++++++++++++++++++++++++++++
arch/arm/boot/dts/sun6i-a31s.dtsi | 8 ++
2 files changed, 160 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index c1b891e75f18..4d2c7786b92a 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -231,6 +231,11 @@
};
};
+ de: display-engine {
+ compatible = "allwinner,sun6i-a31-display-engine";
+ allwinner,pipelines = <&fe0>;
+ };
+
soc@01c00000 {
compatible = "simple-bus";
#address-cells = <1>;
@@ -246,6 +251,44 @@
#dma-cells = <1>;
};
+ tcon0: lcd-controller@01c0c000 {
+ compatible = "allwinner,sun6i-a31-tcon";
+ reg = <0x01c0c000 0x1000>;
+ interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
+ resets = <&ccu RST_AHB1_LCD0>;
+ reset-names = "lcd";
+ clocks = <&ccu CLK_AHB1_LCD0>,
+ <&ccu CLK_LCD0_CH0>,
+ <&ccu CLK_LCD0_CH1>;
+ clock-names = "ahb",
+ "tcon-ch0",
+ "tcon-ch1";
+ clock-output-names = "tcon0-pixel-clock";
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ tcon0_in: port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ tcon0_in_drc0: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&drc0_out_tcon0>;
+ };
+ };
+
+ tcon0_out: port@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+ };
+ };
+ };
+
mmc0: mmc@01c0f000 {
compatible = "allwinner,sun7i-a20-mmc";
reg = <0x01c0f000 0x1000>;
@@ -799,6 +842,115 @@
interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
};
+ fe0: display-frontend@01e00000 {
+ compatible = "allwinner,sun6i-a31-display-frontend";
+ reg = <0x01e00000 0x20000>;
+ interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_AHB1_FE0>, <&ccu CLK_FE0>,
+ <&ccu CLK_DRAM_FE0>;
+ clock-names = "ahb", "mod",
+ "ram";
+ resets = <&ccu RST_AHB1_FE0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ fe0_out: port@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+
+ fe0_out_be0: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&be0_in_fe0>;
+ };
+ };
+ };
+ };
+
+ be0: display-backend@01e60000 {
+ compatible = "allwinner,sun6i-a31-display-backend";
+ reg = <0x01e60000 0x10000>;
+ interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_AHB1_BE0>, <&ccu CLK_BE0>,
+ <&ccu CLK_DRAM_BE0>;
+ clock-names = "ahb", "mod",
+ "ram";
+ resets = <&ccu RST_AHB1_BE0>;
+
+ assigned-clocks = <&ccu CLK_BE0>;
+ assigned-clock-rates = <300000000>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ be0_in: port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ be0_in_fe0: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&fe0_out_be0>;
+ };
+ };
+
+ be0_out: port@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+
+ be0_out_drc0: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&drc0_in_be0>;
+ };
+ };
+ };
+ };
+
+ drc0: drc@01e70000 {
+ compatible = "allwinner,sun6i-a31-drc";
+ reg = <0x01e70000 0x10000>;
+ interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_AHB1_DRC0>, <&ccu CLK_IEP_DRC0>,
+ <&ccu CLK_DRAM_DRC0>;
+ clock-names = "ahb", "mod",
+ "ram";
+ resets = <&ccu RST_AHB1_DRC0>;
+
+ assigned-clocks = <&ccu CLK_IEP_DRC0>;
+ assigned-clock-rates = <300000000>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ drc0_in: port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ drc0_in_be0: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&be0_out_drc0>;
+ };
+ };
+
+ drc0_out: port@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+
+ drc0_out_tcon0: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&tcon0_in_drc0>;
+ };
+ };
+ };
+ };
+
rtc: rtc@01f00000 {
compatible = "allwinner,sun6i-a31-rtc";
reg = <0x01f00000 0x54>;
diff --git a/arch/arm/boot/dts/sun6i-a31s.dtsi b/arch/arm/boot/dts/sun6i-a31s.dtsi
index c17a32771b98..97e2c51d0aea 100644
--- a/arch/arm/boot/dts/sun6i-a31s.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31s.dtsi
@@ -48,6 +48,14 @@
#include "sun6i-a31.dtsi"
+&de {
+ compatible = "allwinner,sun6i-a31s-display-engine";
+};
+
&pio {
compatible = "allwinner,sun6i-a31s-pinctrl";
};
+
+&tcon0 {
+ compatible = "allwinner,sun6i-a31s-tcon";
+};
--
2.9.3
The A31's display pipeline has 2 frontends, 2 backends, and 2 TCONs. It
also has new display enhancement blocks, such as the DRC (Dynamic Range
Controller), the DEU (Display Enhancement Unit), and the CMU (Color
Management Unit). It supports HDMI, MIPI DSI, and 2 LCD/LVDS channels.
The A31s display pipeline is almost the same, just without MIPI DSI.
Only the TCON seems to be different, due to the missing mux for MIPI
DSI.
Add compatible strings for both of them.
Signed-off-by: Chen-Yu Tsai <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 4 ++++
drivers/gpu/drm/sun4i/sun4i_backend.c | 1 +
drivers/gpu/drm/sun4i/sun4i_drv.c | 3 +++
3 files changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
index 15fdca8909f2..b82c00449468 100644
--- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
+++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
@@ -91,6 +91,7 @@ system.
Required properties:
- compatible: value must be one of:
* allwinner,sun5i-a13-display-backend
+ * allwinner,sun6i-a31-display-backend
* allwinner,sun8i-a33-display-backend
- reg: base address and size of the memory-mapped region.
- clocks: phandles to the clocks feeding the frontend and backend
@@ -121,6 +122,7 @@ deinterlacing and color space conversion.
Required properties:
- compatible: value must be one of:
* allwinner,sun5i-a13-display-frontend
+ * allwinner,sun6i-a31-display-frontend
* allwinner,sun8i-a33-display-frontend
- reg: base address and size of the memory-mapped region.
- interrupts: interrupt associated to this IP
@@ -146,6 +148,8 @@ extra node.
Required properties:
- compatible: value must be one of:
* allwinner,sun5i-a13-display-engine
+ * allwinner,sun6i-a31-display-engine
+ * allwinner,sun6i-a31s-display-engine
* allwinner,sun8i-a33-display-engine
- allwinner,pipelines: list of phandle to the display engine
diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index 32c0584e3c35..6e6c59a661b6 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -408,6 +408,7 @@ static int sun4i_backend_remove(struct platform_device *pdev)
static const struct of_device_id sun4i_backend_of_table[] = {
{ .compatible = "allwinner,sun5i-a13-display-backend" },
+ { .compatible = "allwinner,sun6i-a31-display-backend" },
{ .compatible = "allwinner,sun8i-a33-display-backend" },
{ }
};
diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
index a15c231fbd59..fa6568e1822a 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -201,6 +201,7 @@ static const struct component_master_ops sun4i_drv_master_ops = {
static bool sun4i_drv_node_is_frontend(struct device_node *node)
{
return of_device_is_compatible(node, "allwinner,sun5i-a13-display-frontend") ||
+ of_device_is_compatible(node, "allwinner,sun6i-a31-display-frontend") ||
of_device_is_compatible(node, "allwinner,sun8i-a33-display-frontend");
}
@@ -324,6 +325,8 @@ static int sun4i_drv_remove(struct platform_device *pdev)
static const struct of_device_id sun4i_drv_of_table[] = {
{ .compatible = "allwinner,sun5i-a13-display-engine" },
+ { .compatible = "allwinner,sun6i-a31-display-engine" },
+ { .compatible = "allwinner,sun6i-a31s-display-engine" },
{ .compatible = "allwinner,sun8i-a33-display-engine" },
{ }
};
--
2.9.3
The A31 TCON has mux controls for how TCON outputs are routed to the
HDMI and MIPI DSI blocks.
Since the A31s does not have MIPI DSI, it only has a mux for the HDMI
controller input.
This patch only adds support for the compatible strings. Actual support
for the mux controls should be added with HDMI and MIPI DSI support.
Signed-off-by: Chen-Yu Tsai <[email protected]>
---
Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 4 +++-
drivers/gpu/drm/sun4i/sun4i_drv.c | 2 ++
drivers/gpu/drm/sun4i/sun4i_tcon.c | 10 ++++++++++
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
index 5368961cd727..15fdca8909f2 100644
--- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
+++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
@@ -28,6 +28,8 @@ The TCON acts as a timing controller for RGB, LVDS and TV interfaces.
Required properties:
- compatible: value must be either:
* allwinner,sun5i-a13-tcon
+ * allwinner,sun6i-a31-tcon
+ * allwinner,sun6i-a31s-tcon
* allwinner,sun8i-a33-tcon
- reg: base address and size of memory-mapped region
- interrupts: interrupt associated to this IP
@@ -50,7 +52,7 @@ Required properties:
second the block connected to the TCON channel 1 (usually the TV
encoder)
-On the A13, there is one more clock required:
+On SoCs other than the A33, there is one more clock required:
- 'tcon-ch1': The clock driving the TCON channel 1
DRC
diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
index 0da9862ad8ed..a15c231fbd59 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -207,6 +207,8 @@ static bool sun4i_drv_node_is_frontend(struct device_node *node)
static bool sun4i_drv_node_is_tcon(struct device_node *node)
{
return of_device_is_compatible(node, "allwinner,sun5i-a13-tcon") ||
+ of_device_is_compatible(node, "allwinner,sun6i-a31-tcon") ||
+ of_device_is_compatible(node, "allwinner,sun6i-a31s-tcon") ||
of_device_is_compatible(node, "allwinner,sun8i-a33-tcon");
}
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 7658f0337e0b..c6afb2448655 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -587,12 +587,22 @@ static const struct sun4i_tcon_quirks sun5i_a13_quirks = {
.has_channel_1 = true,
};
+static const struct sun4i_tcon_quirks sun6i_a31_quirks = {
+ .has_channel_1 = true,
+};
+
+static const struct sun4i_tcon_quirks sun6i_a31s_quirks = {
+ .has_channel_1 = true,
+};
+
static const struct sun4i_tcon_quirks sun8i_a33_quirks = {
/* nothing is supported */
};
static const struct of_device_id sun4i_tcon_of_table[] = {
{ .compatible = "allwinner,sun5i-a13-tcon", .data = &sun5i_a13_quirks },
+ { .compatible = "allwinner,sun6i-a31-tcon", .data = &sun6i_a31_quirks },
+ { .compatible = "allwinner,sun6i-a31s-tcon", .data = &sun6i_a31s_quirks },
{ .compatible = "allwinner,sun8i-a33-tcon", .data = &sun8i_a33_quirks },
{ }
};
--
2.9.3
We already have some differences between the 2 supported SoCs.
More will be added as we support other SoCs. To avoid bloating
the probe function with even more conditionals, move the quirks
to a separate data structure that's tied to the compatible string.
Signed-off-by: Chen-Yu Tsai <[email protected]>
---
drivers/gpu/drm/sun4i/sun4i_tcon.c | 33 ++++++++++++++++++---------------
drivers/gpu/drm/sun4i/sun4i_tcon.h | 11 +++++++----
2 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index cadacb517f95..7658f0337e0b 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -20,6 +20,7 @@
#include <linux/component.h>
#include <linux/ioport.h>
#include <linux/of_address.h>
+#include <linux/of_device.h>
#include <linux/of_graph.h>
#include <linux/of_irq.h>
#include <linux/regmap.h>
@@ -62,7 +63,7 @@ void sun4i_tcon_channel_disable(struct sun4i_tcon *tcon, int channel)
return;
}
- WARN_ON(!tcon->has_channel_1);
+ WARN_ON(!tcon->quirks->has_channel_1);
regmap_update_bits(tcon->regs, SUN4I_TCON1_CTL_REG,
SUN4I_TCON1_CTL_TCON_ENABLE, 0);
clk_disable_unprepare(tcon->sclk1);
@@ -80,7 +81,7 @@ void sun4i_tcon_channel_enable(struct sun4i_tcon *tcon, int channel)
return;
}
- WARN_ON(!tcon->has_channel_1);
+ WARN_ON(!tcon->quirks->has_channel_1);
regmap_update_bits(tcon->regs, SUN4I_TCON1_CTL_REG,
SUN4I_TCON1_CTL_TCON_ENABLE,
SUN4I_TCON1_CTL_TCON_ENABLE);
@@ -202,7 +203,7 @@ void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon,
u8 clk_delay;
u32 val;
- WARN_ON(!tcon->has_channel_1);
+ WARN_ON(!tcon->quirks->has_channel_1);
/* Adjust clock delay */
clk_delay = sun4i_tcon_get_clk_delay(mode, 1);
@@ -266,7 +267,7 @@ void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon,
/*
* FIXME: Undocumented bits
*/
- if (tcon->has_mux)
+ if (tcon->quirks->has_unknown_mux)
regmap_write(tcon->regs, SUN4I_TCON_MUX_CTRL_REG, 1);
}
EXPORT_SYMBOL(sun4i_tcon1_mode_set);
@@ -327,7 +328,7 @@ static int sun4i_tcon_init_clocks(struct device *dev,
return PTR_ERR(tcon->sclk0);
}
- if (tcon->has_channel_1) {
+ if (tcon->quirks->has_channel_1) {
tcon->sclk1 = devm_clk_get(dev, "tcon-ch1");
if (IS_ERR(tcon->sclk1)) {
dev_err(dev, "Couldn't get the TCON channel 1 clock\n");
@@ -487,14 +488,7 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master,
drv->tcon = tcon;
tcon->drm = drm;
tcon->dev = dev;
-
- if (of_device_is_compatible(dev->of_node, "allwinner,sun5i-a13-tcon")) {
- tcon->has_mux = true;
- tcon->has_channel_1 = true;
- } else {
- tcon->has_mux = false;
- tcon->has_channel_1 = false;
- }
+ tcon->quirks = of_device_get_match_data(dev);
tcon->lcd_rst = devm_reset_control_get(dev, "lcd");
if (IS_ERR(tcon->lcd_rst)) {
@@ -588,9 +582,18 @@ static int sun4i_tcon_remove(struct platform_device *pdev)
return 0;
}
+static const struct sun4i_tcon_quirks sun5i_a13_quirks = {
+ .has_unknown_mux = true,
+ .has_channel_1 = true,
+};
+
+static const struct sun4i_tcon_quirks sun8i_a33_quirks = {
+ /* nothing is supported */
+};
+
static const struct of_device_id sun4i_tcon_of_table[] = {
- { .compatible = "allwinner,sun5i-a13-tcon" },
- { .compatible = "allwinner,sun8i-a33-tcon" },
+ { .compatible = "allwinner,sun5i-a13-tcon", .data = &sun5i_a13_quirks },
+ { .compatible = "allwinner,sun8i-a33-tcon", .data = &sun8i_a33_quirks },
{ }
};
MODULE_DEVICE_TABLE(of, sun4i_tcon_of_table);
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index 12bd48925f4d..166064bafe2e 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -142,6 +142,11 @@
#define SUN4I_TCON_MAX_CHANNELS 2
+struct sun4i_tcon_quirks {
+ bool has_unknown_mux; /* sun5i has undocumented mux */
+ bool has_channel_1; /* a33 does not have channel 1 */
+};
+
struct sun4i_tcon {
struct device *dev;
struct drm_device *drm;
@@ -160,12 +165,10 @@ struct sun4i_tcon {
/* Reset control */
struct reset_control *lcd_rst;
- /* Platform adjustments */
- bool has_mux;
-
struct drm_panel *panel;
- bool has_channel_1;
+ /* Platform adjustments */
+ const struct sun4i_tcon_quirks *quirks;
};
struct drm_bridge *sun4i_tcon_find_bridge(struct device_node *node);
--
2.9.3
On Thu, Oct 20, 2016 at 11:43:37AM +0800, Chen-Yu Tsai wrote:
> Some rgb-to-vga bridges have an enable GPIO, either directly tied to
> an enable pin on the bridge IC, or indirectly controlling a power
> switch.
>
> Add support for it.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
Acked-by: Maxime Ripard <[email protected]>
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On Thu, Oct 20, 2016 at 11:43:38AM +0800, Chen-Yu Tsai wrote:
> The A31 and A31s also have the DRC as part of the display pipeline.
> As we know virtually nothing about them, just add compatible strings
> for both SoCs to the stub driver.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
> Acked-by: Rob Herring <[email protected]>
Applied, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On Thu, Oct 20, 2016 at 11:43:39AM +0800, Chen-Yu Tsai wrote:
> We already have some differences between the 2 supported SoCs.
> More will be added as we support other SoCs. To avoid bloating
> the probe function with even more conditionals, move the quirks
> to a separate data structure that's tied to the compatible string.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
Applied, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On Thu, Oct 20, 2016 at 11:43:40AM +0800, Chen-Yu Tsai wrote:
> The A31 TCON has mux controls for how TCON outputs are routed to the
> HDMI and MIPI DSI blocks.
>
> Since the A31s does not have MIPI DSI, it only has a mux for the HDMI
> controller input.
>
> This patch only adds support for the compatible strings. Actual support
> for the mux controls should be added with HDMI and MIPI DSI support.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
Applied, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On Thu, Oct 20, 2016 at 11:43:41AM +0800, Chen-Yu Tsai wrote:
> The A31's display pipeline has 2 frontends, 2 backends, and 2 TCONs. It
> also has new display enhancement blocks, such as the DRC (Dynamic Range
> Controller), the DEU (Display Enhancement Unit), and the CMU (Color
> Management Unit). It supports HDMI, MIPI DSI, and 2 LCD/LVDS channels.
>
> The A31s display pipeline is almost the same, just without MIPI DSI.
> Only the TCON seems to be different, due to the missing mux for MIPI
> DSI.
>
> Add compatible strings for both of them.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
> Acked-by: Rob Herring <[email protected]>
Applied, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On Thu, Oct 20, 2016 at 11:43:43AM +0800, Chen-Yu Tsai wrote:
> The LCD0 controller on the A31 can do RGB output up to 8 bits per
> channel. Add the pins for RGB888 output.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
Applied, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On Thu, Oct 20, 2016 at 11:43:42AM +0800, Chen-Yu Tsai wrote:
> The A31 has 2 parallel display pipelines, which can be intermixed.
> However the driver currently only supports one of them.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
Applied, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
1;4402;0c
On Thu, Oct 20, 2016 at 11:43:44AM +0800, Chen-Yu Tsai wrote:
> The Hummingbird A31 board has a RGB-to-VGA bridge which converts RGB
> output from the LCD interface to VGA signals.
>
> Enable this part of the display pipeline.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
This one looks nice, but I'm going to wait for Archit answers before
merging it.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Hi,
On 10/20/2016 09:13 AM, Chen-Yu Tsai wrote:
> Some rgb-to-vga bridges have an enable GPIO, either directly tied to
> an enable pin on the bridge IC, or indirectly controlling a power
> switch.
>
> Add support for it.
Does the bridge on your platform have an active/passive DAC, or is it a
smarter encoder chip that is capable of doing more? If so, it might be
good to have a separate DT compatible string to it, like what's done
in the patch titled:
drm: bridge: vga-dac: Add adi,adv7123 compatible string
so that we can switch to a different driver later if needed.
Thanks,
Archit
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
> ---
> .../bindings/display/bridge/dumb-vga-dac.txt | 2 ++
> drivers/gpu/drm/bridge/dumb-vga-dac.c | 28 ++++++++++++++++++++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> index 003bc246a270..d3484822bf77 100644
> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> - Video port 0 for RGB input
> - Video port 1 for VGA output
>
> +Optional properties:
> +- enable-gpios: GPIO pin to enable or disable the bridge
>
> Example
> -------
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> index afec232185a7..b487e5e9b56d 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -10,6 +10,7 @@
> * the License, or (at your option) any later version.
> */
>
> +#include <linux/gpio/consumer.h>
> #include <linux/module.h>
> #include <linux/of_graph.h>
>
> @@ -23,6 +24,7 @@ struct dumb_vga {
> struct drm_connector connector;
>
> struct i2c_adapter *ddc;
> + struct gpio_desc *enable_gpio;
> };
>
> static inline struct dumb_vga *
> @@ -124,8 +126,26 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
> return 0;
> }
>
> +static void dumb_vga_enable(struct drm_bridge *bridge)
> +{
> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> +
> + if (vga->enable_gpio)
> + gpiod_set_value_cansleep(vga->enable_gpio, 1);
> +}
> +
> +static void dumb_vga_disable(struct drm_bridge *bridge)
> +{
> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> +
> + if (vga->enable_gpio)
> + gpiod_set_value_cansleep(vga->enable_gpio, 0);
> +}
> +
> static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
> .attach = dumb_vga_attach,
> + .enable = dumb_vga_enable,
> + .disable = dumb_vga_disable,
> };
>
> static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
> @@ -169,6 +189,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
> return -ENOMEM;
> platform_set_drvdata(pdev, vga);
>
> + vga->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(vga->enable_gpio)) {
> + ret = PTR_ERR(vga->enable_gpio);
> + dev_err(&pdev->dev, "failed to request GPIO: %d\n", ret);
> + return ret;
> + }
> +
> vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
> if (IS_ERR(vga->ddc)) {
> if (PTR_ERR(vga->ddc) == -ENODEV) {
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 10/20/2016 09:13 AM, Chen-Yu Tsai wrote:
> The Hummingbird A31 board has a RGB-to-VGA bridge which converts RGB
> output from the LCD interface to VGA signals.
>
> Enable this part of the display pipeline.
I couldn't find the enable-gpios binding for the bridge that you
introduced in the previous patch. Is that intentional?
Thanks,
Archit
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
> ---
> arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 56 +++++++++++++++++++++++++++++
> 1 file changed, 56 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
> index 9a74637f677f..05a49b2147f1 100644
> --- a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
> +++ b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
> @@ -63,6 +63,49 @@
> stdout-path = "serial0:115200n8";
> };
>
> + bridge {
> + compatible = "dumb-vga-dac";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0>;
> +
> + vga_bridge_in: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&tcon0_out_vga>;
> + };
> + };
> +
> + port@1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> +
> + vga_bridge_out: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&vga_con_in>;
> + };
> + };
> + };
> + };
> +
> + vga {
> + compatible = "vga-connector";
> +
> + port {
> + vga_con_in: endpoint {
> + remote-endpoint = <&vga_bridge_out>;
> + };
> + };
> + };
> +
> wifi_pwrseq: wifi_pwrseq {
> compatible = "mmc-pwrseq-simple";
> reset-gpios = <&pio 6 10 GPIO_ACTIVE_LOW>; /* PG10 */
> @@ -245,6 +288,19 @@
> status = "okay";
> };
>
> +&tcon0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&lcd0_rgb888_pins>;
> + status = "okay";
> +};
> +
> +&tcon0_out {
> + tcon0_out_vga: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&vga_bridge_in>;
> + };
> +};
> +
> &uart0 {
> pinctrl-names = "default";
> pinctrl-0 = <&uart0_pins_a>;
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On Tue, Oct 25, 2016 at 4:13 PM, Archit Taneja <[email protected]> wrote:
>
>
> On 10/20/2016 09:13 AM, Chen-Yu Tsai wrote:
>>
>> The Hummingbird A31 board has a RGB-to-VGA bridge which converts RGB
>> output from the LCD interface to VGA signals.
>>
>> Enable this part of the display pipeline.
>
>
> I couldn't find the enable-gpios binding for the bridge that you
> introduced in the previous patch. Is that intentional?
Error on my part. Thanks for spotting that.
ChenYu
>
> Thanks,
> Archit
>
>
>>
>> Signed-off-by: Chen-Yu Tsai <[email protected]>
>> ---
>> arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 56
>> +++++++++++++++++++++++++++++
>> 1 file changed, 56 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
>> b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
>> index 9a74637f677f..05a49b2147f1 100644
>> --- a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
>> +++ b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
>> @@ -63,6 +63,49 @@
>> stdout-path = "serial0:115200n8";
>> };
>>
>> + bridge {
>> + compatible = "dumb-vga-dac";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <0>;
>> +
>> + vga_bridge_in: endpoint@0 {
>> + reg = <0>;
>> + remote-endpoint =
>> <&tcon0_out_vga>;
>> + };
>> + };
>> +
>> + port@1 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <1>;
>> +
>> + vga_bridge_out: endpoint@0 {
>> + reg = <0>;
>> + remote-endpoint = <&vga_con_in>;
>> + };
>> + };
>> + };
>> + };
>> +
>> + vga {
>> + compatible = "vga-connector";
>> +
>> + port {
>> + vga_con_in: endpoint {
>> + remote-endpoint = <&vga_bridge_out>;
>> + };
>> + };
>> + };
>> +
>> wifi_pwrseq: wifi_pwrseq {
>> compatible = "mmc-pwrseq-simple";
>> reset-gpios = <&pio 6 10 GPIO_ACTIVE_LOW>; /* PG10 */
>> @@ -245,6 +288,19 @@
>> status = "okay";
>> };
>>
>> +&tcon0 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&lcd0_rgb888_pins>;
>> + status = "okay";
>> +};
>> +
>> +&tcon0_out {
>> + tcon0_out_vga: endpoint@0 {
>> + reg = <0>;
>> + remote-endpoint = <&vga_bridge_in>;
>> + };
>> +};
>> +
>> &uart0 {
>> pinctrl-names = "default";
>> pinctrl-0 = <&uart0_pins_a>;
>>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
On Tue, Oct 25, 2016 at 4:09 PM, Archit Taneja <[email protected]> wrote:
> Hi,
>
> On 10/20/2016 09:13 AM, Chen-Yu Tsai wrote:
>>
>> Some rgb-to-vga bridges have an enable GPIO, either directly tied to
>> an enable pin on the bridge IC, or indirectly controlling a power
>> switch.
>>
>> Add support for it.
>
>
> Does the bridge on your platform have an active/passive DAC, or is it a
> smarter encoder chip that is capable of doing more? If so, it might be
> good to have a separate DT compatible string to it, like what's done
> in the patch titled:
>
> drm: bridge: vga-dac: Add adi,adv7123 compatible string
>
> so that we can switch to a different driver later if needed.
The chip is GM7123. It is not configurable. It just takes the LCD RGB/SYNC
signals and converts them to analog. The only things you can change are
putting it into sleep mode and tweaking the output drive strength by
changing the external reference resistor. The latter would be a hardware
design decision. I would say this qualifies as "dumb".
I revisited the board schematics, and the enable GPIO actually toggles
an external LDO regulator. So this might be better modeled as a regulator
supply?
Thanks
ChenYu
>
> Thanks,
> Archit
>
>
>>
>> Signed-off-by: Chen-Yu Tsai <[email protected]>
>> ---
>> .../bindings/display/bridge/dumb-vga-dac.txt | 2 ++
>> drivers/gpu/drm/bridge/dumb-vga-dac.c | 28
>> ++++++++++++++++++++++
>> 2 files changed, 30 insertions(+)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>> b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>> index 003bc246a270..d3484822bf77 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>> @@ -16,6 +16,8 @@ graph bindings specified in
>> Documentation/devicetree/bindings/graph.txt.
>> - Video port 0 for RGB input
>> - Video port 1 for VGA output
>>
>> +Optional properties:
>> +- enable-gpios: GPIO pin to enable or disable the bridge
>>
>> Example
>> -------
>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> index afec232185a7..b487e5e9b56d 100644
>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> @@ -10,6 +10,7 @@
>> * the License, or (at your option) any later version.
>> */
>>
>> +#include <linux/gpio/consumer.h>
>> #include <linux/module.h>
>> #include <linux/of_graph.h>
>>
>> @@ -23,6 +24,7 @@ struct dumb_vga {
>> struct drm_connector connector;
>>
>> struct i2c_adapter *ddc;
>> + struct gpio_desc *enable_gpio;
>> };
>>
>> static inline struct dumb_vga *
>> @@ -124,8 +126,26 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
>> return 0;
>> }
>>
>> +static void dumb_vga_enable(struct drm_bridge *bridge)
>> +{
>> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>> +
>> + if (vga->enable_gpio)
>> + gpiod_set_value_cansleep(vga->enable_gpio, 1);
>> +}
>> +
>> +static void dumb_vga_disable(struct drm_bridge *bridge)
>> +{
>> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>> +
>> + if (vga->enable_gpio)
>> + gpiod_set_value_cansleep(vga->enable_gpio, 0);
>> +}
>> +
>> static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
>> .attach = dumb_vga_attach,
>> + .enable = dumb_vga_enable,
>> + .disable = dumb_vga_disable,
>> };
>>
>> static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
>> @@ -169,6 +189,14 @@ static int dumb_vga_probe(struct platform_device
>> *pdev)
>> return -ENOMEM;
>> platform_set_drvdata(pdev, vga);
>>
>> + vga->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable",
>> + GPIOD_OUT_LOW);
>> + if (IS_ERR(vga->enable_gpio)) {
>> + ret = PTR_ERR(vga->enable_gpio);
>> + dev_err(&pdev->dev, "failed to request GPIO: %d\n", ret);
>> + return ret;
>> + }
>> +
>> vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
>> if (IS_ERR(vga->ddc)) {
>> if (PTR_ERR(vga->ddc) == -ENODEV) {
>>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
On Thu, Oct 20, 2016 at 11:43:37AM +0800, Chen-Yu Tsai wrote:
> Some rgb-to-vga bridges have an enable GPIO, either directly tied to
> an enable pin on the bridge IC, or indirectly controlling a power
> switch.
>
> Add support for it.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
> ---
> .../bindings/display/bridge/dumb-vga-dac.txt | 2 ++
> drivers/gpu/drm/bridge/dumb-vga-dac.c | 28 ++++++++++++++++++++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> index 003bc246a270..d3484822bf77 100644
> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> - Video port 0 for RGB input
> - Video port 1 for VGA output
>
> +Optional properties:
> +- enable-gpios: GPIO pin to enable or disable the bridge
This should also define the active state.
> +static void dumb_vga_enable(struct drm_bridge *bridge)
> +{
> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> +
> + if (vga->enable_gpio)
> + gpiod_set_value_cansleep(vga->enable_gpio, 1);
So the driver should allow either active high or low.
> +}
> +
> +static void dumb_vga_disable(struct drm_bridge *bridge)
> +{
> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> +
> + if (vga->enable_gpio)
> + gpiod_set_value_cansleep(vga->enable_gpio, 0);
> +}
> +
Hi Rob,
On Wed, Oct 26, 2016 at 05:13:46PM -0500, Rob Herring wrote:
> On Thu, Oct 20, 2016 at 11:43:37AM +0800, Chen-Yu Tsai wrote:
> > Some rgb-to-vga bridges have an enable GPIO, either directly tied to
> > an enable pin on the bridge IC, or indirectly controlling a power
> > switch.
> >
> > Add support for it.
> >
> > Signed-off-by: Chen-Yu Tsai <[email protected]>
> > ---
> > .../bindings/display/bridge/dumb-vga-dac.txt | 2 ++
> > drivers/gpu/drm/bridge/dumb-vga-dac.c | 28 ++++++++++++++++++++++
> > 2 files changed, 30 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> > index 003bc246a270..d3484822bf77 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> > +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> > @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> > - Video port 0 for RGB input
> > - Video port 1 for VGA output
> >
> > +Optional properties:
> > +- enable-gpios: GPIO pin to enable or disable the bridge
>
> This should also define the active state.
>
> > +static void dumb_vga_enable(struct drm_bridge *bridge)
> > +{
> > + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> > +
> > + if (vga->enable_gpio)
> > + gpiod_set_value_cansleep(vga->enable_gpio, 1);
>
> So the driver should allow either active high or low.
You mean like having a enable-active-high property? Isn't that
redundant with the GPIO flags?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On 10/25/2016 02:29 PM, Chen-Yu Tsai wrote:
> On Tue, Oct 25, 2016 at 4:09 PM, Archit Taneja <[email protected]> wrote:
>> Hi,
>>
>> On 10/20/2016 09:13 AM, Chen-Yu Tsai wrote:
>>>
>>> Some rgb-to-vga bridges have an enable GPIO, either directly tied to
>>> an enable pin on the bridge IC, or indirectly controlling a power
>>> switch.
>>>
>>> Add support for it.
>>
>>
>> Does the bridge on your platform have an active/passive DAC, or is it a
>> smarter encoder chip that is capable of doing more? If so, it might be
>> good to have a separate DT compatible string to it, like what's done
>> in the patch titled:
>>
>> drm: bridge: vga-dac: Add adi,adv7123 compatible string
>>
>> so that we can switch to a different driver later if needed.
>
> The chip is GM7123. It is not configurable. It just takes the LCD RGB/SYNC
> signals and converts them to analog. The only things you can change are
> putting it into sleep mode and tweaking the output drive strength by
Is sleep mode the thing that's controlled by this gpio?
> changing the external reference resistor. The latter would be a hardware
> design decision. I would say this qualifies as "dumb".
Yeah, I agree. I'd want feedback from Laurent too, since he had comments
on the usage of the original dumb-vga-dac driver.
>
> I revisited the board schematics, and the enable GPIO actually toggles
> an external LDO regulator. So this might be better modeled as a regulator
> supply?
If you model it as a regulator, how would you toggle the GPIO on your
platform?
Looking at the chip pin out, there is a 3.3V VDD supply needed for the
chip, so it would be good to have an optional 'power' regulator supply
anyway.
Archit
>
>
> Thanks
> ChenYu
>
>>
>> Thanks,
>> Archit
>>
>>
>>>
>>> Signed-off-by: Chen-Yu Tsai <[email protected]>
>>> ---
>>> .../bindings/display/bridge/dumb-vga-dac.txt | 2 ++
>>> drivers/gpu/drm/bridge/dumb-vga-dac.c | 28
>>> ++++++++++++++++++++++
>>> 2 files changed, 30 insertions(+)
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> index 003bc246a270..d3484822bf77 100644
>>> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> @@ -16,6 +16,8 @@ graph bindings specified in
>>> Documentation/devicetree/bindings/graph.txt.
>>> - Video port 0 for RGB input
>>> - Video port 1 for VGA output
>>>
>>> +Optional properties:
>>> +- enable-gpios: GPIO pin to enable or disable the bridge
>>>
>>> Example
>>> -------
>>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> index afec232185a7..b487e5e9b56d 100644
>>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> @@ -10,6 +10,7 @@
>>> * the License, or (at your option) any later version.
>>> */
>>>
>>> +#include <linux/gpio/consumer.h>
>>> #include <linux/module.h>
>>> #include <linux/of_graph.h>
>>>
>>> @@ -23,6 +24,7 @@ struct dumb_vga {
>>> struct drm_connector connector;
>>>
>>> struct i2c_adapter *ddc;
>>> + struct gpio_desc *enable_gpio;
>>> };
>>>
>>> static inline struct dumb_vga *
>>> @@ -124,8 +126,26 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
>>> return 0;
>>> }
>>>
>>> +static void dumb_vga_enable(struct drm_bridge *bridge)
>>> +{
>>> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>> +
>>> + if (vga->enable_gpio)
>>> + gpiod_set_value_cansleep(vga->enable_gpio, 1);
>>> +}
>>> +
>>> +static void dumb_vga_disable(struct drm_bridge *bridge)
>>> +{
>>> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>> +
>>> + if (vga->enable_gpio)
>>> + gpiod_set_value_cansleep(vga->enable_gpio, 0);
>>> +}
>>> +
>>> static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
>>> .attach = dumb_vga_attach,
>>> + .enable = dumb_vga_enable,
>>> + .disable = dumb_vga_disable,
>>> };
>>>
>>> static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
>>> @@ -169,6 +189,14 @@ static int dumb_vga_probe(struct platform_device
>>> *pdev)
>>> return -ENOMEM;
>>> platform_set_drvdata(pdev, vga);
>>>
>>> + vga->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable",
>>> + GPIOD_OUT_LOW);
>>> + if (IS_ERR(vga->enable_gpio)) {
>>> + ret = PTR_ERR(vga->enable_gpio);
>>> + dev_err(&pdev->dev, "failed to request GPIO: %d\n", ret);
>>> + return ret;
>>> + }
>>> +
>>> vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
>>> if (IS_ERR(vga->ddc)) {
>>> if (PTR_ERR(vga->ddc) == -ENODEV) {
>>>
>>
>> --
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> a Linux Foundation Collaborative Project
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On Thu, Oct 27, 2016 at 2:40 PM, Archit Taneja <[email protected]> wrote:
>
>
> On 10/25/2016 02:29 PM, Chen-Yu Tsai wrote:
>>
>> On Tue, Oct 25, 2016 at 4:09 PM, Archit Taneja <[email protected]>
>> wrote:
>>>
>>> Hi,
>>>
>>> On 10/20/2016 09:13 AM, Chen-Yu Tsai wrote:
>>>>
>>>>
>>>> Some rgb-to-vga bridges have an enable GPIO, either directly tied to
>>>> an enable pin on the bridge IC, or indirectly controlling a power
>>>> switch.
>>>>
>>>> Add support for it.
>>>
>>>
>>>
>>> Does the bridge on your platform have an active/passive DAC, or is it a
>>> smarter encoder chip that is capable of doing more? If so, it might be
>>> good to have a separate DT compatible string to it, like what's done
>>> in the patch titled:
>>>
>>> drm: bridge: vga-dac: Add adi,adv7123 compatible string
>>>
>>> so that we can switch to a different driver later if needed.
>>
>>
>> The chip is GM7123. It is not configurable. It just takes the LCD RGB/SYNC
>> signals and converts them to analog. The only things you can change are
>> putting it into sleep mode and tweaking the output drive strength by
>
>
> Is sleep mode the thing that's controlled by this gpio?
Not on this particular board. The gpio controls the external LDO that
supplies 3.3V to VDD.
>
>> changing the external reference resistor. The latter would be a hardware
>> design decision. I would say this qualifies as "dumb".
>
>
> Yeah, I agree. I'd want feedback from Laurent too, since he had comments
> on the usage of the original dumb-vga-dac driver.
>
>>
>> I revisited the board schematics, and the enable GPIO actually toggles
>> an external LDO regulator. So this might be better modeled as a regulator
>> supply?
>
>
> If you model it as a regulator, how would you toggle the GPIO on your
> platform?
>
> Looking at the chip pin out, there is a 3.3V VDD supply needed for the
> chip, so it would be good to have an optional 'power' regulator supply
> anyway.
Yes, that it what I plan to do. I'll drop the enable-gpios property,
and add a power-supply property for the regulator.
Regards
ChenYu
>
> Archit
>
>
>>
>>
>> Thanks
>> ChenYu
>>
>>>
>>> Thanks,
>>> Archit
>>>
>>>
>>>>
>>>> Signed-off-by: Chen-Yu Tsai <[email protected]>
>>>> ---
>>>> .../bindings/display/bridge/dumb-vga-dac.txt | 2 ++
>>>> drivers/gpu/drm/bridge/dumb-vga-dac.c | 28
>>>> ++++++++++++++++++++++
>>>> 2 files changed, 30 insertions(+)
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>>> b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>>> index 003bc246a270..d3484822bf77 100644
>>>> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>>> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>>> @@ -16,6 +16,8 @@ graph bindings specified in
>>>> Documentation/devicetree/bindings/graph.txt.
>>>> - Video port 0 for RGB input
>>>> - Video port 1 for VGA output
>>>>
>>>> +Optional properties:
>>>> +- enable-gpios: GPIO pin to enable or disable the bridge
>>>>
>>>> Example
>>>> -------
>>>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>>> b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>>> index afec232185a7..b487e5e9b56d 100644
>>>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>>> @@ -10,6 +10,7 @@
>>>> * the License, or (at your option) any later version.
>>>> */
>>>>
>>>> +#include <linux/gpio/consumer.h>
>>>> #include <linux/module.h>
>>>> #include <linux/of_graph.h>
>>>>
>>>> @@ -23,6 +24,7 @@ struct dumb_vga {
>>>> struct drm_connector connector;
>>>>
>>>> struct i2c_adapter *ddc;
>>>> + struct gpio_desc *enable_gpio;
>>>> };
>>>>
>>>> static inline struct dumb_vga *
>>>> @@ -124,8 +126,26 @@ static int dumb_vga_attach(struct drm_bridge
>>>> *bridge)
>>>> return 0;
>>>> }
>>>>
>>>> +static void dumb_vga_enable(struct drm_bridge *bridge)
>>>> +{
>>>> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>>> +
>>>> + if (vga->enable_gpio)
>>>> + gpiod_set_value_cansleep(vga->enable_gpio, 1);
>>>> +}
>>>> +
>>>> +static void dumb_vga_disable(struct drm_bridge *bridge)
>>>> +{
>>>> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>>> +
>>>> + if (vga->enable_gpio)
>>>> + gpiod_set_value_cansleep(vga->enable_gpio, 0);
>>>> +}
>>>> +
>>>> static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
>>>> .attach = dumb_vga_attach,
>>>> + .enable = dumb_vga_enable,
>>>> + .disable = dumb_vga_disable,
>>>> };
>>>>
>>>> static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
>>>> @@ -169,6 +189,14 @@ static int dumb_vga_probe(struct platform_device
>>>> *pdev)
>>>> return -ENOMEM;
>>>> platform_set_drvdata(pdev, vga);
>>>>
>>>> + vga->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable",
>>>> + GPIOD_OUT_LOW);
>>>> + if (IS_ERR(vga->enable_gpio)) {
>>>> + ret = PTR_ERR(vga->enable_gpio);
>>>> + dev_err(&pdev->dev, "failed to request GPIO: %d\n",
>>>> ret);
>>>> + return ret;
>>>> + }
>>>> +
>>>> vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
>>>> if (IS_ERR(vga->ddc)) {
>>>> if (PTR_ERR(vga->ddc) == -ENODEV) {
>>>>
>>>
>>> --
>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>>> a Linux Foundation Collaborative Project
>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
On Thu, Oct 27, 2016 at 12:17:24AM +0200, Maxime Ripard wrote:
> Hi Rob,
>
> On Wed, Oct 26, 2016 at 05:13:46PM -0500, Rob Herring wrote:
> > On Thu, Oct 20, 2016 at 11:43:37AM +0800, Chen-Yu Tsai wrote:
> > > Some rgb-to-vga bridges have an enable GPIO, either directly tied to
> > > an enable pin on the bridge IC, or indirectly controlling a power
> > > switch.
> > >
> > > Add support for it.
> > >
> > > Signed-off-by: Chen-Yu Tsai <[email protected]>
> > > ---
> > > .../bindings/display/bridge/dumb-vga-dac.txt | 2 ++
> > > drivers/gpu/drm/bridge/dumb-vga-dac.c | 28 ++++++++++++++++++++++
> > > 2 files changed, 30 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> > > index 003bc246a270..d3484822bf77 100644
> > > --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> > > +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> > > @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> > > - Video port 0 for RGB input
> > > - Video port 1 for VGA output
> > >
> > > +Optional properties:
> > > +- enable-gpios: GPIO pin to enable or disable the bridge
> >
> > This should also define the active state.
> >
> > > +static void dumb_vga_enable(struct drm_bridge *bridge)
> > > +{
> > > + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> > > +
> > > + if (vga->enable_gpio)
> > > + gpiod_set_value_cansleep(vga->enable_gpio, 1);
> >
> > So the driver should allow either active high or low.
>
> You mean like having a enable-active-high property? Isn't that
> redundant with the GPIO flags?
Correct - the gpiod APIs remove the need for drivers to know the
polarity of the signal, handling it inside the GPIO subsystem
instead, controlled either from the gpiod lookup tables in legacy
board files, or the DT specification for the GPIO.
So, in drivers, gpiod_set_value*(, 1) means "set gpio to active
level" and gpiod_set_value*(, 0) means "set gpio to inactive level".
Far nicer than all the bugs we've had with the legacy GPIO interfaces
with random different drivers implementing random different ways to
invert the signal, with all the pain that brings with it when a
platform comes along with a different inversion state.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.