Currently, none of the upstream Qualcomm DT files have the display device nodes populated, even when the DT binding documents are upstream.
I am not aware of all the issues with the bindings which has prevented it from getting merged, but 2 properties "connectors" (maintains a list of all the external interfaces (DSI, HDMI etc)) and "gpus" (list of GPU devices) seem like the ones that can't be merged.
This patch set aligns with the graph bindings to describe the connection between the display controller block (MDP) and the external encoder interfaces.
This is similar to the graph bindings used by some of the drivers (imx, rockchip), but not exactly the same. These drivers expect a top level "display-subsytem" DT node which lists down the display interface ports to be used. Our implementation just parses the interface ports in the MDP node as is, and add the components that are needed. I've Cc-ed Heiko and Philipp in case they had any comments on this.
The 'gpu' property is removed in a hack-ish way. The driver contains a list of all the compatible strings for gpus, and searches the entire OF firmware for a matching node. Once we know what's the right way to link the gpu and display nodes together (if needed at all) we can add the required binding.
The rest of the changes are minor cleanups and fixes to prepare the driver and binding documents before we really start adding the display nodes.
Archit Taneja (9): drm/msm: Get mdss components via parsing ports drm/msm: Drop the gpu binding drm/msm/mdp: mdp4: Update LCDC/LVDS port parsing dt-bindings: msm/mdp: Remove connector and gpu bindings dt-bindings: msm/dsi: Some binding doc cleanups drm/msm/dsi: Modify port parsing drm/msm/dsi: Use generic PHY bindings dt-bindings: msm/dsi: Modify port and PHY bindings dt-bindings: msm/dsi: Add assigned clocks bindings
.../devicetree/bindings/display/msm/dsi.txt | 79 +++++++++++++++------ .../devicetree/bindings/display/msm/mdp.txt | 75 ++++++++++++++++++-- drivers/gpu/drm/msm/dsi/dsi.c | 2 +- drivers/gpu/drm/msm/dsi/dsi_host.c | 10 +-- drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 8 ++- drivers/gpu/drm/msm/msm_drv.c | 80 +++++++++++++++++++--- 6 files changed, 213 insertions(+), 41 deletions(-)
The driver currently identifies all the mdss components it needs by parsing a phandle list from the 'connectors' DT property.
Instead of this, describe a list of ports that the MDP hardware provides to the external world. These ports are linked to external encoder interfaces such as DSI, HDMI in MDSS. These are also the subcomponent devices that we need add. This description of ports complies with the generic graph bindings.
In MDP4, the LVDS port's output connects directly to the LVDS panel. In this case, we don't try to add it as a component.
Signed-off-by: Archit Taneja architt@codeaurora.org --- drivers/gpu/drm/msm/msm_drv.c | 54 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 955ddfd..30b8f3b 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -1087,6 +1087,58 @@ static int add_components(struct device *dev, struct component_match **matchptr, return 0; }
+/* + * Identify what components need to be added by parsing what the endpoints in + * our output ports are we. In the case of LVDS, there is no external component + * that we need to add since it's a part of MDP itself. + */ +static int add_mdss_components(struct device *dev, + struct component_match **matchptr) +{ + struct device_node *np = dev->of_node; + struct device_node *ep_node; + + for_each_endpoint_of_node(np, ep_node) { + struct device_node *intf; + struct of_endpoint ep; + int ret; + + ret = of_graph_parse_endpoint(ep_node, &ep); + if (ret) { + dev_err(dev, "unable to parse port endpoint\n"); + of_node_put(ep_node); + return ret; + } + + /* + * The LCDC/LVDS port on MDP4 is a speacial case where the + * remote-endpoint isn't a component that we need to add + */ + if (of_device_is_compatible(np, "qcom,mdp4") && ep.port == 0) { + of_node_put(ep_node); + continue; + } + + /* + * It's okay if some of the ports don't have a remote endpoint + * specified. It just means that the port isn't connected to + * any external interface. + */ + intf = of_graph_get_remote_port_parent(ep_node); + if (!intf) { + of_node_put(ep_node); + continue; + } + + component_match_add(dev, matchptr, compare_of, intf); + + of_node_put(intf); + of_node_put(ep_node); + } + + return 0; +} + static int msm_drm_bind(struct device *dev) { return msm_drm_init(dev, &msm_driver); @@ -1110,7 +1162,7 @@ static int msm_pdev_probe(struct platform_device *pdev) { struct component_match *match = NULL;
- add_components(&pdev->dev, &match, "connectors"); + add_mdss_components(&pdev->dev, &match); add_components(&pdev->dev, &match, "gpus");
pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
The driver currently identifies the GPU components it needs by parsing a phandle list from the 'gpus' DT property.
This isn't the right binding to go with. So, for now, just search all device nodes and find the gpu node we need by parsing a list of compatible strings.
Once we know how to link the kms and gpu drivers, we'll drop this method and use the correct binding.
Signed-off-by: Archit Taneja architt@codeaurora.org --- drivers/gpu/drm/msm/msm_drv.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 30b8f3b..f717a69 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -1068,20 +1068,32 @@ static int compare_of(struct device *dev, void *data) return dev->of_node == data; }
-static int add_components(struct device *dev, struct component_match **matchptr, - const char *name) +static const char * const msm_compatible_gpus[] = { + "qcom,adreno-3xx", + "qcom,kgsl-3d0", +}; + +/* + * We don't know what's the best binding to link the gpu with the drm device. + * Fow now, we just hunt for all the possible gpus that we support, and add them + * as components. + */ +static int add_gpu_components(struct device *dev, + struct component_match **matchptr) { - struct device_node *np = dev->of_node; unsigned i;
- for (i = 0; ; i++) { + for (i = 0; i < ARRAY_SIZE(msm_compatible_gpus); i++) { struct device_node *node;
- node = of_parse_phandle(np, name, i); + node = of_find_compatible_node(NULL, NULL, + msm_compatible_gpus[i]); if (!node) - break; + continue;
component_match_add(dev, matchptr, compare_of, node); + + of_node_put(node); }
return 0; @@ -1163,7 +1175,7 @@ static int msm_pdev_probe(struct platform_device *pdev) struct component_match *match = NULL;
add_mdss_components(&pdev->dev, &match); - add_components(&pdev->dev, &match, "gpus"); + add_gpu_components(&pdev->dev, &match);
pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32); return component_master_add_with_match(&pdev->dev, &msm_drm_ops, match);
On Tue, May 3, 2016 at 5:57 AM, Archit Taneja architt@codeaurora.org wrote:
The driver currently identifies the GPU components it needs by parsing a phandle list from the 'gpus' DT property.
This isn't the right binding to go with. So, for now, just search all device nodes and find the gpu node we need by parsing a list of compatible strings.
Once we know how to link the kms and gpu drivers, we'll drop this method and use the correct binding.
Signed-off-by: Archit Taneja architt@codeaurora.org
drivers/gpu/drm/msm/msm_drv.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 30b8f3b..f717a69 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -1068,20 +1068,32 @@ static int compare_of(struct device *dev, void *data) return dev->of_node == data; }
-static int add_components(struct device *dev, struct component_match **matchptr,
const char *name)+static const char * const msm_compatible_gpus[] = {
"qcom,adreno-3xx","qcom,kgsl-3d0",+};
+/*
- We don't know what's the best binding to link the gpu with the drm device.
- Fow now, we just hunt for all the possible gpus that we support, and add them
- as components.
- */
+static int add_gpu_components(struct device *dev,
struct component_match **matchptr){
struct device_node *np = dev->of_node; unsigned i;for (i = 0; ; i++) {
for (i = 0; i < ARRAY_SIZE(msm_compatible_gpus); i++) {
You can use of_find_matching_node() here instead of a loop.
Rob
On 05/03/2016 06:12 PM, Rob Herring wrote:
On Tue, May 3, 2016 at 5:57 AM, Archit Taneja architt@codeaurora.org wrote:
The driver currently identifies the GPU components it needs by parsing a phandle list from the 'gpus' DT property.
This isn't the right binding to go with. So, for now, just search all device nodes and find the gpu node we need by parsing a list of compatible strings.
Once we know how to link the kms and gpu drivers, we'll drop this method and use the correct binding.
Signed-off-by: Archit Taneja architt@codeaurora.org
drivers/gpu/drm/msm/msm_drv.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 30b8f3b..f717a69 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -1068,20 +1068,32 @@ static int compare_of(struct device *dev, void *data) return dev->of_node == data; }
-static int add_components(struct device *dev, struct component_match **matchptr,
const char *name)+static const char * const msm_compatible_gpus[] = {
"qcom,adreno-3xx","qcom,kgsl-3d0",+};
+/*
- We don't know what's the best binding to link the gpu with the drm device.
- Fow now, we just hunt for all the possible gpus that we support, and add them
- as components.
- */
+static int add_gpu_components(struct device *dev,
struct component_match **matchptr)
struct device_node *np = dev->of_node; unsigned i;for (i = 0; ; i++) {
for (i = 0; i < ARRAY_SIZE(msm_compatible_gpus); i++) {You can use of_find_matching_node() here instead of a loop.
I'll switch to that.
Thanks, Archit
The LCDC port is the first in the list of the output ports in MDP4. Mention this explicitly in the driver.
Signed-off-by: Archit Taneja architt@codeaurora.org --- drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c index f8e0cea..cd129f4 100644 --- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c +++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c @@ -263,9 +263,13 @@ static struct device_node *mdp4_detect_lcdc_panel(struct drm_device *dev) struct device_node *endpoint, *panel_node; struct device_node *np = dev->dev->of_node;
- endpoint = of_graph_get_next_endpoint(np, NULL); + /* + * LCDC/LVDS is the first port described in the list of ports in the + * MDP4 DT node. + */ + endpoint = of_graph_get_endpoint_by_regs(np, 0, -1); if (!endpoint) { - DBG("no endpoint in MDP4 to fetch LVDS panel\n"); + DBG("no LVDS panel remote endpoint\n"); return NULL; }
Am Dienstag, den 03.05.2016, 16:27 +0530 schrieb Archit Taneja:
The LCDC port is the first in the list of the output ports in MDP4. Mention this explicitly in the driver.
Signed-off-by: Archit Taneja architt@codeaurora.org
drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c index f8e0cea..cd129f4 100644 --- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c +++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c @@ -263,9 +263,13 @@ static struct device_node *mdp4_detect_lcdc_panel(struct drm_device *dev) struct device_node *endpoint, *panel_node; struct device_node *np = dev->dev->of_node;
- endpoint = of_graph_get_next_endpoint(np, NULL);
- /*
* LCDC/LVDS is the first port described in the list of ports in the* MDP4 DT node.*/- endpoint = of_graph_get_endpoint_by_regs(np, 0, -1); if (!endpoint) {
DBG("no endpoint in MDP4 to fetch LVDS panel\n");
DBG("no LVDS panel remote endpoint\n");
Oh nice, I wasn't aware of of_graph_get_endpoint_by_regs(). We should switch imx-drm and exynos to use it, too.
regards Philipp
The MDP DT node now contains a list of ports that describe how it connects to external encoder interfaces like DSI and HDMI. These follow the standard of_graph bindings, and allow us to get rid of the 'connectors' phandle that contained a list of all the external encoders connected to MDP.
The GPU phandle is removed too until we figure out what's the right way to specify it in DT.
Signed-off-by: Archit Taneja architt@codeaurora.org --- .../devicetree/bindings/display/msm/mdp.txt | 75 ++++++++++++++++++++-- 1 file changed, 71 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/mdp.txt b/Documentation/devicetree/bindings/display/msm/mdp.txt index a214f6c..5f6ae3c 100644 --- a/Documentation/devicetree/bindings/display/msm/mdp.txt +++ b/Documentation/devicetree/bindings/display/msm/mdp.txt @@ -6,7 +6,6 @@ Required properties: * "qcom,mdp5" - mdp5 - reg: Physical base address and length of the controller's registers. - interrupts: The interrupt signal from the display controller. -- connectors: array of phandles for output device(s) - clocks: device clocks See ../clocks/clock-bindings.txt for details. - clock-names: the following clocks are required. @@ -24,9 +23,33 @@ Required properties: * "core_clk" * "lut_clk" (some MDP5 versions may not need this) * "vsync_clk" +- ports: contains the list of output ports from MDP. These connect to interfaces + that are external to the MDP hardware, such as HDMI, DSI, EDP etc (LVDS is a + special case since it is a part of the MDP block itself). + + Each output port contains an endpoint that describes how it is connected to an + external interface. These are described by the standard properties documented + here: + Documentation/devicetree/bindings/graph.txt + Documentation/devicetree/bindings/media/video-interfaces.txt + + For MDP4, the output port mappings are: + Port 0 -> LCDC/LVDS + Port 1 -> DSI1 Cmd/Video + Port 2 -> DSI2 Cmd/Video + Port 3 -> DTV + + For MDP5, the availability of output ports vary across each SoC revision, but + they generally have the following mapping: + Port 0 -> MDP_INTF0 (eDP) + Port 1 -> MDP_INTF1 (DSI1) + Port 2 -> MDP_INTF2 (DSI2) + Port 3 -> MDP_INTF3 (HDMI) + + See drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c to see what all INTFs a particular + SoC revision has enabled.
Optional properties: -- gpus: phandle for gpu device - clock-names: the following clocks are optional: * "lut_clk"
@@ -35,11 +58,25 @@ Example: / { ...
- mdp: qcom,mdp@5100000 { + hdmi: hdmi@4a00000 { + ... + ports { + ... + port@0 { + reg = <0>; + hdmi_in: endpoint { + remote-endpoint = <&mdp_dtv_out>; + }; + }; + ... + }; + ... + }; + + mdp: mdp@5100000 { compatible = "qcom,mdp4"; reg = <0x05100000 0xf0000>; interrupts = <GIC_SPI 75 0>; - connectors = <&hdmi>; gpus = <&gpu>; clock-names = "core_clk", @@ -55,5 +92,35 @@ Example: <&mmcc TV_SRC>, <&mmcc HDMI_TV_CLK>, <&mmcc MDP_TV_CLK>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + mdp_lvds_out: endpoint { + }; + }; + + port@1 { + reg = <1>; + mdp_dsi1_out: endpoint { + }; + }; + + port@2 { + reg = <2>; + mdp_dsi2_out: endpoint { + }; + }; + + port@3 { + reg = <3>; + mdp_dtv_out: endpoint { + remote-endpoint = <&hdmi_in>; + }; + }; + }; }; };
On Tue, May 03, 2016 at 04:27:56PM +0530, Archit Taneja wrote:
The MDP DT node now contains a list of ports that describe how it connects to external encoder interfaces like DSI and HDMI. These follow the standard of_graph bindings, and allow us to get rid of the 'connectors' phandle that contained a list of all the external encoders connected to MDP.
The GPU phandle is removed too until we figure out what's the right way to specify it in DT.
[...]
- For MDP4, the output port mappings are:
- Port 0 -> LCDC/LVDS
- Port 1 -> DSI1 Cmd/Video
- Port 2 -> DSI2 Cmd/Video
- Port 3 -> DTV
- For MDP5, the availability of output ports vary across each SoC revision, but
- they generally have the following mapping:
- Port 0 -> MDP_INTF0 (eDP)
- Port 1 -> MDP_INTF1 (DSI1)
- Port 2 -> MDP_INTF2 (DSI2)
- Port 3 -> MDP_INTF3 (HDMI)
- See drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c to see what all INTFs a particular
- SoC revision has enabled.
The binding doc shouldn't depend on kernel code. You need to document it here.
Rob
On 5/4/2016 7:08 PM, Rob Herring wrote:
On Tue, May 03, 2016 at 04:27:56PM +0530, Archit Taneja wrote:
The MDP DT node now contains a list of ports that describe how it connects to external encoder interfaces like DSI and HDMI. These follow the standard of_graph bindings, and allow us to get rid of the 'connectors' phandle that contained a list of all the external encoders connected to MDP.
The GPU phandle is removed too until we figure out what's the right way to specify it in DT.
[...]
- For MDP4, the output port mappings are:
- Port 0 -> LCDC/LVDS
- Port 1 -> DSI1 Cmd/Video
- Port 2 -> DSI2 Cmd/Video
- Port 3 -> DTV
- For MDP5, the availability of output ports vary across each SoC revision, but
- they generally have the following mapping:
- Port 0 -> MDP_INTF0 (eDP)
- Port 1 -> MDP_INTF1 (DSI1)
- Port 2 -> MDP_INTF2 (DSI2)
- Port 3 -> MDP_INTF3 (HDMI)
- See drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c to see what all INTFs a particular
- SoC revision has enabled.
The binding doc shouldn't depend on kernel code. You need to document it here.
Okay. I'll list the available interfaces for each SoC revision.
Thanks, Archit
Some cleanups:
- Use simpler names for DT nodes in the example - Fix phandle for specifying data lane mapping in the example. - Use references instead of dumping Document links everywhere
Signed-off-by: Archit Taneja architt@codeaurora.org --- .../devicetree/bindings/display/msm/dsi.txt | 23 +++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt index f5948c4..bf41d7c 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi.txt +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt @@ -11,8 +11,7 @@ Required properties: be 0 or 1, since we have 2 DSI controllers at most for now. - interrupts: The interrupt signal from the DSI block. - power-domains: Should be <&mmcc MDSS_GDSC>. -- clocks: device clocks - See Documentation/devicetree/bindings/clocks/clock-bindings.txt for details. +- clocks: Phandles to device clocks as descirbed in [1] - clock-names: the following clocks are required: * "mdp_core_clk" * "iface_clk" @@ -31,8 +30,7 @@ Required properties:
Optional properties: - panel@0: Node of panel connected to this DSI controller. - See files in Documentation/devicetree/bindings/display/panel/ for each supported - panel. + See files in [2] for each supported panel. - qcom,dual-dsi-mode: Boolean value indicating if the DSI controller is driving a panel which needs 2 DSI links. - qcom,master-dsi: Boolean value indicating if the DSI controller is driving @@ -48,7 +46,7 @@ Optional properties:
DSI Endpoint properties: - remote-endpoint: set to phandle of the connected panel's endpoint. - See Documentation/devicetree/bindings/graph.txt for device graph info. + See [3] for device graph info. - qcom,data-lane-map: this describes how the logical DSI lanes are mapped to the physical lanes on the given platform. The value contained in index n describes what logical data lane is mapped to the physical data @@ -89,8 +87,7 @@ Required properties: - qcom,dsi-phy-index: The ID of DSI PHY hardware instance. This should be 0 or 1, since we have 2 DSI PHYs at most for now. - power-domains: Should be <&mmcc MDSS_GDSC>. -- clocks: device clocks - See Documentation/devicetree/bindings/clocks/clock-bindings.txt for details. +- clocks: Phandles to device clocks as descirbed in [1] - clock-names: the following clocks are required: * "iface_clk" - vddio-supply: phandle to vdd-io regulator device node @@ -99,8 +96,12 @@ Optional properties: - qcom,dsi-phy-regulator-ldo-mode: Boolean value indicating if the LDO mode PHY regulator is wanted.
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt +[2] Documentation/devicetree/bindings/display/panel/ +[3] Documentation/devicetree/bindings/graph.txt + Example: - mdss_dsi0: qcom,mdss_dsi@fd922800 { + dsi0: dsi@fd922800 { compatible = "qcom,mdss-dsi-ctrl"; qcom,dsi-host-index = <0>; interrupt-parent = <&mdss_mdp>; @@ -128,7 +129,7 @@ Example: vdd-supply = <&pma8084_l22>; vddio-supply = <&pma8084_l12>;
- qcom,dsi-phy = <&mdss_dsi_phy0>; + qcom,dsi-phy = <&dsi_phy0>;
qcom,dual-dsi-mode; qcom,master-dsi; @@ -156,12 +157,12 @@ Example: port { dsi0_out: endpoint { remote-endpoint = <&panel_in>; - lanes = <0 1 2 3>; + qcom,data-lane-map = <0 1 2 3>; }; }; };
- mdss_dsi_phy0: qcom,mdss_dsi_phy@fd922a00 { + dsi_phy0: dsi_phy@fd922a00 { compatible = "qcom,dsi-phy-28nm-hpm"; qcom,dsi-phy-index = <0>; reg-names =
Am Dienstag, den 03.05.2016, 16:27 +0530 schrieb Archit Taneja:
Some cleanups:
- Use simpler names for DT nodes in the example
- Fix phandle for specifying data lane mapping in the example.
- Use references instead of dumping Document links everywhere
Signed-off-by: Archit Taneja architt@codeaurora.org
.../devicetree/bindings/display/msm/dsi.txt | 23 +++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt index f5948c4..bf41d7c 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi.txt +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt @@ -11,8 +11,7 @@ Required properties: be 0 or 1, since we have 2 DSI controllers at most for now.
- interrupts: The interrupt signal from the DSI block.
- power-domains: Should be <&mmcc MDSS_GDSC>.
-- clocks: device clocks
- See Documentation/devicetree/bindings/clocks/clock-bindings.txt for details.
+- clocks: Phandles to device clocks as descirbed in [1]
s/descirbed/described/
- clock-names: the following clocks are required:
- "mdp_core_clk"
- "iface_clk"
@@ -31,8 +30,7 @@ Required properties:
Optional properties:
- panel@0: Node of panel connected to this DSI controller.
- See files in Documentation/devicetree/bindings/display/panel/ for each supported
- panel.
- See files in [2] for each supported panel.
- qcom,dual-dsi-mode: Boolean value indicating if the DSI controller is driving a panel which needs 2 DSI links.
- qcom,master-dsi: Boolean value indicating if the DSI controller is driving
@@ -48,7 +46,7 @@ Optional properties:
DSI Endpoint properties:
- remote-endpoint: set to phandle of the connected panel's endpoint.
- See Documentation/devicetree/bindings/graph.txt for device graph info.
- See [3] for device graph info.
to the physical lanes on the given platform. The value contained in index n describes what logical data lane is mapped to the physical data
- qcom,data-lane-map: this describes how the logical DSI lanes are mapped
@@ -89,8 +87,7 @@ Required properties:
- qcom,dsi-phy-index: The ID of DSI PHY hardware instance. This should be 0 or 1, since we have 2 DSI PHYs at most for now.
- power-domains: Should be <&mmcc MDSS_GDSC>.
-- clocks: device clocks
- See Documentation/devicetree/bindings/clocks/clock-bindings.txt for details.
+- clocks: Phandles to device clocks as descirbed in [1]
s/descirbed/described/
- clock-names: the following clocks are required:
- "iface_clk"
- vddio-supply: phandle to vdd-io regulator device node
@@ -99,8 +96,12 @@ Optional properties:
- qcom,dsi-phy-regulator-ldo-mode: Boolean value indicating if the LDO mode PHY regulator is wanted.
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt +[2] Documentation/devicetree/bindings/display/panel/ +[3] Documentation/devicetree/bindings/graph.txt
Example:
- mdss_dsi0: qcom,mdss_dsi@fd922800 {
- dsi0: dsi@fd922800 { compatible = "qcom,mdss-dsi-ctrl"; qcom,dsi-host-index = <0>; interrupt-parent = <&mdss_mdp>;
@@ -128,7 +129,7 @@ Example: vdd-supply = <&pma8084_l22>; vddio-supply = <&pma8084_l12>;
qcom,dsi-phy = <&mdss_dsi_phy0>;
qcom,dsi-phy = <&dsi_phy0>;qcom,dual-dsi-mode; qcom,master-dsi;
@@ -156,12 +157,12 @@ Example: port { dsi0_out: endpoint { remote-endpoint = <&panel_in>;
lanes = <0 1 2 3>;
qcom,data-lane-map = <0 1 2 3>;
See my comment about the CSI-2 data-lanes binding in the other mail, could the existing binding be reused?
regards Philipp
On 05/03/2016 07:35 PM, Philipp Zabel wrote:
Am Dienstag, den 03.05.2016, 16:27 +0530 schrieb Archit Taneja:
Some cleanups:
- Use simpler names for DT nodes in the example
- Fix phandle for specifying data lane mapping in the example.
- Use references instead of dumping Document links everywhere
Signed-off-by: Archit Taneja architt@codeaurora.org
.../devicetree/bindings/display/msm/dsi.txt | 23 +++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt index f5948c4..bf41d7c 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi.txt +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt @@ -11,8 +11,7 @@ Required properties: be 0 or 1, since we have 2 DSI controllers at most for now.
- interrupts: The interrupt signal from the DSI block.
- power-domains: Should be <&mmcc MDSS_GDSC>.
-- clocks: device clocks
- See Documentation/devicetree/bindings/clocks/clock-bindings.txt for details.
+- clocks: Phandles to device clocks as descirbed in [1]
s/descirbed/described/
- clock-names: the following clocks are required:
- "mdp_core_clk"
- "iface_clk"
@@ -31,8 +30,7 @@ Required properties:
Optional properties:
- panel@0: Node of panel connected to this DSI controller.
- See files in Documentation/devicetree/bindings/display/panel/ for each supported
- panel.
- See files in [2] for each supported panel.
driving a panel which needs 2 DSI links.
- qcom,dual-dsi-mode: Boolean value indicating if the DSI controller is
- qcom,master-dsi: Boolean value indicating if the DSI controller is driving
@@ -48,7 +46,7 @@ Optional properties:
DSI Endpoint properties: - remote-endpoint: set to phandle of the connected panel's endpoint.
- See Documentation/devicetree/bindings/graph.txt for device graph info.
- See [3] for device graph info.
to the physical lanes on the given platform. The value contained in index n describes what logical data lane is mapped to the physical data
- qcom,data-lane-map: this describes how the logical DSI lanes are mapped
@@ -89,8 +87,7 @@ Required properties:
- qcom,dsi-phy-index: The ID of DSI PHY hardware instance. This should be 0 or 1, since we have 2 DSI PHYs at most for now.
- power-domains: Should be <&mmcc MDSS_GDSC>.
-- clocks: device clocks
- See Documentation/devicetree/bindings/clocks/clock-bindings.txt for details.
+- clocks: Phandles to device clocks as descirbed in [1]
s/descirbed/described/
Thanks, will fix these.
- clock-names: the following clocks are required:
- "iface_clk"
- vddio-supply: phandle to vdd-io regulator device node
@@ -99,8 +96,12 @@ Optional properties:
- qcom,dsi-phy-regulator-ldo-mode: Boolean value indicating if the LDO mode PHY regulator is wanted.
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt +[2] Documentation/devicetree/bindings/display/panel/ +[3] Documentation/devicetree/bindings/graph.txt
- Example:
- mdss_dsi0: qcom,mdss_dsi@fd922800 {
- dsi0: dsi@fd922800 { compatible = "qcom,mdss-dsi-ctrl"; qcom,dsi-host-index = <0>; interrupt-parent = <&mdss_mdp>;
@@ -128,7 +129,7 @@ Example: vdd-supply = <&pma8084_l22>; vddio-supply = <&pma8084_l12>;
qcom,dsi-phy = <&mdss_dsi_phy0>;
qcom,dsi-phy = <&dsi_phy0>;qcom,dual-dsi-mode; qcom,master-dsi;
@@ -156,12 +157,12 @@ Example: port { dsi0_out: endpoint { remote-endpoint = <&panel_in>;
lanes = <0 1 2 3>;
qcom,data-lane-map = <0 1 2 3>;See my comment about the CSI-2 data-lanes binding in the other mail, could the existing binding be reused?
Yes, I'll incorporate the existing binding.
Thanks for the review.
Archit
The DSI interface is going to have two ports defined in its device node. The first port is always going to be the link between the MDP output and the input to DSI, the second port is going to be the link between the DSI output and the connected panel/bridge:
----- ----- ------- | MDP | ------> | DSI | ------> | Panel | ----- ----- ------- (Port 0) (Port 1)
Until now, there was only one Port representing the output. Update the DSI host driver such that it parses Port #1 for a connected device.
Signed-off-by: Archit Taneja architt@codeaurora.org --- drivers/gpu/drm/msm/dsi/dsi_host.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 51fbbf8..15c70ac 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -1616,12 +1616,12 @@ static int dsi_host_parse_dt(struct msm_dsi_host *msm_host) }
/* - * Get the first endpoint node. In our case, dsi has one output port - * to which the panel is connected. Don't return an error if a port - * isn't defined. It's possible that there is nothing connected to - * the dsi output. + * Get the endpoint of the output port of the DSI host. In our case, + * this is mapped to port number with reg = 1. Don't return an error if + * the remote endpoint isn't defined. It's possible that there is + * nothing connected to the dsi output. */ - endpoint = of_graph_get_next_endpoint(np, NULL); + endpoint = of_graph_get_endpoint_by_regs(np, 1, -1); if (!endpoint) { dev_dbg(dev, "%s: no endpoint\n", __func__); return 0;
The DSI host links to the DSI PHY device using a custom binding. Switch to the generic PHY bindings. The DSI PHY driver itself doesn't use the common PHY framework for now.
Signed-off-by: Archit Taneja architt@codeaurora.org --- drivers/gpu/drm/msm/dsi/dsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c index 6edcd6f..ec572f8 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.c +++ b/drivers/gpu/drm/msm/dsi/dsi.c @@ -29,7 +29,7 @@ static int dsi_get_phy(struct msm_dsi *msm_dsi) struct platform_device *phy_pdev; struct device_node *phy_node;
- phy_node = of_parse_phandle(pdev->dev.of_node, "qcom,dsi-phy", 0); + phy_node = of_parse_phandle(pdev->dev.of_node, "phys", 0); if (!phy_node) { dev_err(&pdev->dev, "cannot find phy device\n"); return -ENXIO;
The DSI node now has two ports that describe the connection between the MDP interface output and the DSI input, and the connection between the DSI output and the connected panel/bridge. Update the properties and the example.
Also, use generic PHY bindings instead of the custom one.
Signed-off-by: Archit Taneja architt@codeaurora.org --- .../devicetree/bindings/display/msm/dsi.txt | 53 +++++++++++++++------- 1 file changed, 37 insertions(+), 16 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt index bf41d7c..0223f06 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi.txt +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt @@ -25,12 +25,18 @@ Required properties: - vdd-supply: phandle to vdd regulator device node - vddio-supply: phandle to vdd-io regulator device node - vdda-supply: phandle to vdda regulator device node -- qcom,dsi-phy: phandle to DSI PHY device node +- phys: phandle to DSI PHY device node +- phy-names: the name of the corresponding PHY device - syscon-sfpb: A phandle to mmss_sfpb syscon node (only for DSIv2) +- ports: Contains 2 DSI controller ports as child nodes. Each port contains + an endpoint subnode as defined in [2] and [3]. port@0 describes the + connection between the MDP interface output and the DSI input. port@1 + describes the connection between the DSI output and the connected + panel/bridge.
Optional properties: - panel@0: Node of panel connected to this DSI controller. - See files in [2] for each supported panel. + See files in [4] for each supported panel. - qcom,dual-dsi-mode: Boolean value indicating if the DSI controller is driving a panel which needs 2 DSI links. - qcom,master-dsi: Boolean value indicating if the DSI controller is driving @@ -42,15 +48,15 @@ Optional properties: - pinctrl-names: the pin control state names; should contain "default" - pinctrl-0: the default pinctrl state (active) - pinctrl-n: the "sleep" pinctrl state -- port: DSI controller output port, containing one endpoint subnode.
DSI Endpoint properties: - - remote-endpoint: set to phandle of the connected panel's endpoint. - See [3] for device graph info. + - remote-endpoint: For port@0, set to phandle of the connected panel/bridge's + input endpoint. For port@1, set to the MDP interface output. - qcom,data-lane-map: this describes how the logical DSI lanes are mapped to the physical lanes on the given platform. The value contained in index n describes what logical data lane is mapped to the physical data - lane n (DATAn, where n lies between 0 and 3). + lane n (DATAn, where n lies between 0 and 3). Only for the endpoint in + port@1.
For example:
@@ -97,8 +103,9 @@ Optional properties: regulator is wanted.
[1] Documentation/devicetree/bindings/clock/clock-bindings.txt -[2] Documentation/devicetree/bindings/display/panel/ -[3] Documentation/devicetree/bindings/graph.txt +[2] Documentation/devicetree/bindings/graph.txt +[3] Documentation/devicetree/bindings/media/video-interfaces.txt +[4] Documentation/devicetree/bindings/display/panel/
Example: dsi0: dsi@fd922800 { @@ -129,7 +136,8 @@ Example: vdd-supply = <&pma8084_l22>; vddio-supply = <&pma8084_l12>;
- qcom,dsi-phy = <&dsi_phy0>; + phys = <&dsi_phy0>; + phy-names ="dsi_phy";
qcom,dual-dsi-mode; qcom,master-dsi; @@ -139,6 +147,26 @@ Example: pinctrl-0 = <&mdss_dsi_active>; pinctrl-1 = <&mdss_dsi_suspend>;
+ ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dsi0_in: endpoint { + remote-endpoint = <&mdp_intf1_out>; + }; + }; + + port@1 { + reg = <1>; + dsi0_out: endpoint { + remote-endpoint = <&panel_in>; + qcom,data-lane-map = <0 1 2 3>; + }; + }; + }; + panel: panel@0 { compatible = "sharp,lq101r1sx01"; reg = <0>; @@ -153,13 +181,6 @@ Example: }; }; }; - - port { - dsi0_out: endpoint { - remote-endpoint = <&panel_in>; - qcom,data-lane-map = <0 1 2 3>; - }; - }; };
dsi_phy0: dsi_phy@fd922a00 {
Am Dienstag, den 03.05.2016, 16:28 +0530 schrieb Archit Taneja:
The DSI node now has two ports that describe the connection between the MDP interface output and the DSI input, and the connection between the DSI output and the connected panel/bridge. Update the properties and the example.
Also, use generic PHY bindings instead of the custom one.
Signed-off-by: Archit Taneja architt@codeaurora.org
.../devicetree/bindings/display/msm/dsi.txt | 53 +++++++++++++++------- 1 file changed, 37 insertions(+), 16 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt index bf41d7c..0223f06 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi.txt +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt @@ -25,12 +25,18 @@ Required properties:
- vdd-supply: phandle to vdd regulator device node
- vddio-supply: phandle to vdd-io regulator device node
- vdda-supply: phandle to vdda regulator device node
-- qcom,dsi-phy: phandle to DSI PHY device node +- phys: phandle to DSI PHY device node +- phy-names: the name of the corresponding PHY device
Should "dsi_phy" be specified here?
Also is there any kind of system to the PHY naming? I've seen more bindings use hyphen instead of underscore in the name, for example. I have called the MediaTek MIPI_TX PHY "dphy" for no other reason than it's a MIPI D-PHY.
- syscon-sfpb: A phandle to mmss_sfpb syscon node (only for DSIv2)
+- ports: Contains 2 DSI controller ports as child nodes. Each port contains
- an endpoint subnode as defined in [2] and [3]. port@0 describes the
- connection between the MDP interface output and the DSI input. port@1
- describes the connection between the DSI output and the connected
- panel/bridge.
Optional properties:
- panel@0: Node of panel connected to this DSI controller.
- See files in [2] for each supported panel.
- See files in [4] for each supported panel.
- qcom,dual-dsi-mode: Boolean value indicating if the DSI controller is driving a panel which needs 2 DSI links.
- qcom,master-dsi: Boolean value indicating if the DSI controller is driving
@@ -42,15 +48,15 @@ Optional properties:
- pinctrl-names: the pin control state names; should contain "default"
- pinctrl-0: the default pinctrl state (active)
- pinctrl-n: the "sleep" pinctrl state
-- port: DSI controller output port, containing one endpoint subnode.
DSI Endpoint properties:
- remote-endpoint: set to phandle of the connected panel's endpoint.
- See [3] for device graph info.
- remote-endpoint: For port@0, set to phandle of the connected panel/bridge's
- input endpoint. For port@1, set to the MDP interface output.
to the physical lanes on the given platform. The value contained in index n describes what logical data lane is mapped to the physical data
- qcom,data-lane-map: this describes how the logical DSI lanes are mapped
- lane n (DATAn, where n lies between 0 and 3).
- lane n (DATAn, where n lies between 0 and 3). Only for the endpoint in
- port@1.
I approve of the of graph change, but I notice that the qcom,data-lane-map is very similar to the data-lanes property documented in Documentation/devicetree/bindings/media/video-interfaces.txt for MIPI CSI-2. Could that maybe be reused for DSI?
For example:@@ -97,8 +103,9 @@ Optional properties: regulator is wanted.
[1] Documentation/devicetree/bindings/clock/clock-bindings.txt -[2] Documentation/devicetree/bindings/display/panel/ -[3] Documentation/devicetree/bindings/graph.txt +[2] Documentation/devicetree/bindings/graph.txt +[3] Documentation/devicetree/bindings/media/video-interfaces.txt +[4] Documentation/devicetree/bindings/display/panel/
Example: dsi0: dsi@fd922800 { @@ -129,7 +136,8 @@ Example: vdd-supply = <&pma8084_l22>; vddio-supply = <&pma8084_l12>;
qcom,dsi-phy = <&dsi_phy0>;
phys = <&dsi_phy0>;phy-names ="dsi_phy";qcom,dual-dsi-mode; qcom,master-dsi;
@@ -139,6 +147,26 @@ Example: pinctrl-0 = <&mdss_dsi_active>; pinctrl-1 = <&mdss_dsi_suspend>;
ports {#address-cells = <1>;#size-cells = <0>;port@0 {reg = <0>;dsi0_in: endpoint {remote-endpoint = <&mdp_intf1_out>;};};port@1 {reg = <1>;dsi0_out: endpoint {remote-endpoint = <&panel_in>;qcom,data-lane-map = <0 1 2 3>;};};};- panel: panel@0 { compatible = "sharp,lq101r1sx01"; reg = <0>;
If the panel is connected via port@1, why is this still needed?
@@ -153,13 +181,6 @@ Example: }; }; };
port {dsi0_out: endpoint {remote-endpoint = <&panel_in>;qcom,data-lane-map = <0 1 2 3>;};};};
dsi_phy0: dsi_phy@fd922a00 {
regards Philipp
On 05/03/2016 07:32 PM, Philipp Zabel wrote:
Am Dienstag, den 03.05.2016, 16:28 +0530 schrieb Archit Taneja:
The DSI node now has two ports that describe the connection between the MDP interface output and the DSI input, and the connection between the DSI output and the connected panel/bridge. Update the properties and the example.
Also, use generic PHY bindings instead of the custom one.
Signed-off-by: Archit Taneja architt@codeaurora.org
.../devicetree/bindings/display/msm/dsi.txt | 53 +++++++++++++++------- 1 file changed, 37 insertions(+), 16 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt index bf41d7c..0223f06 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi.txt +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt @@ -25,12 +25,18 @@ Required properties:
- vdd-supply: phandle to vdd regulator device node
- vddio-supply: phandle to vdd-io regulator device node
- vdda-supply: phandle to vdda regulator device node
-- qcom,dsi-phy: phandle to DSI PHY device node +- phys: phandle to DSI PHY device node +- phy-names: the name of the corresponding PHY device
Should "dsi_phy" be specified here?
Also is there any kind of system to the PHY naming? I've seen more bindings use hyphen instead of underscore in the name, for example. I have called the MediaTek MIPI_TX PHY "dphy" for no other reason than it's a MIPI D-PHY.
I agree with the hyphen part, I'll switch to that.
The DSI PHY block isn't only the D-PHY interface, it also contains a PLL and some logic to support more complex configurations (for example, syncing with the PHY connected to another DSI output). That's the main reason why I left the name as "dsi_phy".
- syscon-sfpb: A phandle to mmss_sfpb syscon node (only for DSIv2)
+- ports: Contains 2 DSI controller ports as child nodes. Each port contains
an endpoint subnode as defined in [2] and [3]. port@0 describes the
connection between the MDP interface output and the DSI input. port@1
describes the connection between the DSI output and the connected
panel/bridge.
Optional properties:
- panel@0: Node of panel connected to this DSI controller.
- See files in [2] for each supported panel.
- See files in [4] for each supported panel.
driving a panel which needs 2 DSI links.
- qcom,dual-dsi-mode: Boolean value indicating if the DSI controller is
- qcom,master-dsi: Boolean value indicating if the DSI controller is driving
@@ -42,15 +48,15 @@ Optional properties:
- pinctrl-names: the pin control state names; should contain "default"
- pinctrl-0: the default pinctrl state (active)
- pinctrl-n: the "sleep" pinctrl state
-- port: DSI controller output port, containing one endpoint subnode.
DSI Endpoint properties:
- remote-endpoint: set to phandle of the connected panel's endpoint.
- See [3] for device graph info.
- remote-endpoint: For port@0, set to phandle of the connected panel/bridge's
- input endpoint. For port@1, set to the MDP interface output.
to the physical lanes on the given platform. The value contained in index n describes what logical data lane is mapped to the physical data
- qcom,data-lane-map: this describes how the logical DSI lanes are mapped
- lane n (DATAn, where n lies between 0 and 3).
- lane n (DATAn, where n lies between 0 and 3). Only for the endpoint in
- port@1.
I approve of the of graph change, but I notice that the qcom,data-lane-map is very similar to the data-lanes property documented in Documentation/devicetree/bindings/media/video-interfaces.txt for MIPI CSI-2. Could that maybe be reused for DSI?
You're right. I missed out on the existing binding when I posted this. One difference in this binding is that the indices here point to the physical lanes, and the value contained in the index is the logical lane (that's how it's described in the register documentation). For the "data-lanes" property, it's the other way round. It's still better to use the existing binding. I'll post a separate patch to update this.
For example:@@ -97,8 +103,9 @@ Optional properties: regulator is wanted.
[1] Documentation/devicetree/bindings/clock/clock-bindings.txt -[2] Documentation/devicetree/bindings/display/panel/ -[3] Documentation/devicetree/bindings/graph.txt +[2] Documentation/devicetree/bindings/graph.txt +[3] Documentation/devicetree/bindings/media/video-interfaces.txt +[4] Documentation/devicetree/bindings/display/panel/
Example: dsi0: dsi@fd922800 { @@ -129,7 +136,8 @@ Example: vdd-supply = <&pma8084_l22>; vddio-supply = <&pma8084_l12>;
qcom,dsi-phy = <&dsi_phy0>;
phys = <&dsi_phy0>;phy-names ="dsi_phy";qcom,dual-dsi-mode; qcom,master-dsi;
@@ -139,6 +147,26 @@ Example: pinctrl-0 = <&mdss_dsi_active>; pinctrl-1 = <&mdss_dsi_suspend>;
ports {#address-cells = <1>;#size-cells = <0>;port@0 {reg = <0>;dsi0_in: endpoint {remote-endpoint = <&mdp_intf1_out>;};};port@1 {reg = <1>;dsi0_out: endpoint {remote-endpoint = <&panel_in>;qcom,data-lane-map = <0 1 2 3>;};};};- panel: panel@0 { compatible = "sharp,lq101r1sx01"; reg = <0>;
If the panel is connected via port@1, why is this still needed?
Did you mean that the numbering(reg prop) isn't needed? If so, yeah, I missed out on that. I'll remove it. Thanks for pointing it out.
Archit
The PLL in the DSI PHY block generates 2 clock outputs (Byte and Pixel clocks) that are fed into the Multimedia Clock Controller (MMCC). The MMCC uses these as source clocks for some of its RCGs to generate clocks that finally feed to the DSI host controller.
Use the assigned clocks DT bindings to set up the MMCC RCGs that feed to the DSI host. Use the DSI PHY provided clocks to set up the parents of these assigned clocks.
Signed-off-by: Archit Taneja architt@codeaurora.org --- Documentation/devicetree/bindings/display/msm/dsi.txt | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt index 0223f06..686f475 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi.txt +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt @@ -22,6 +22,10 @@ Required properties: * "core_clk" For DSIv2, we need an additional clock: * "src_clk" +- assigned-clocks: Parents of "byte_clk" and "pixel_clk" for the given platform. + See [1] for more details. +- assigned-clock-parents: The Byte clock and Pixel clock PLL outputs provided + by a DSI PHY block. - vdd-supply: phandle to vdd regulator device node - vddio-supply: phandle to vdd-io regulator device node - vdda-supply: phandle to vdda regulator device node @@ -90,6 +94,8 @@ Required properties: * "dsi_pll" * "dsi_phy" * "dsi_phy_regulator" +- clock-cells: Must be 1. The DSI PHY block acts as a clock provider, creating + 2 clocks: A byte clock (index 0), and a pixel clock (index 1). - qcom,dsi-phy-index: The ID of DSI PHY hardware instance. This should be 0 or 1, since we have 2 DSI PHYs at most for now. - power-domains: Should be <&mmcc MDSS_GDSC>. @@ -132,6 +138,14 @@ Example: <&mmcc MDSS_AHB_CLK>, <&mmcc MDSS_MDP_CLK>, <&mmcc MDSS_PCLK0_CLK>; + + assigned-clocks = + <&mmcc BYTE0_CLK_SRC>, + <&mmcc PCLK0_CLK_SRC>; + assigned-clock-parents = + <&dsi0_phy 0>, + <&dsi0_phy 1>; + vdda-supply = <&pma8084_l2>; vdd-supply = <&pma8084_l22>; vddio-supply = <&pma8084_l12>; @@ -195,6 +209,7 @@ Example: <0xfd922d80 0x7b>; clock-names = "iface_clk"; clocks = <&mmcc MDSS_AHB_CLK>; + #clock-cells = <1>; vddio-supply = <&pma8084_l12>;
qcom,dsi-phy-regulator-ldo-mode;
On Tue, May 03, 2016 at 04:28:01PM +0530, Archit Taneja wrote:
The PLL in the DSI PHY block generates 2 clock outputs (Byte and Pixel clocks) that are fed into the Multimedia Clock Controller (MMCC). The MMCC uses these as source clocks for some of its RCGs to generate clocks that finally feed to the DSI host controller.
Use the assigned clocks DT bindings to set up the MMCC RCGs that feed to the DSI host. Use the DSI PHY provided clocks to set up the parents of these assigned clocks.
Signed-off-by: Archit Taneja architt@codeaurora.org
Documentation/devicetree/bindings/display/msm/dsi.txt | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt index 0223f06..686f475 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi.txt +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt @@ -22,6 +22,10 @@ Required properties:
- "core_clk"
For DSIv2, we need an additional clock: * "src_clk" +- assigned-clocks: Parents of "byte_clk" and "pixel_clk" for the given platform.
- See [1] for more details.
+- assigned-clock-parents: The Byte clock and Pixel clock PLL outputs provided
- by a DSI PHY block.
- vdd-supply: phandle to vdd regulator device node
- vddio-supply: phandle to vdd-io regulator device node
- vdda-supply: phandle to vdda regulator device node
@@ -90,6 +94,8 @@ Required properties:
- "dsi_pll"
- "dsi_phy"
- "dsi_phy_regulator"
+- clock-cells: Must be 1. The DSI PHY block acts as a clock provider, creating
- 2 clocks: A byte clock (index 0), and a pixel clock (index 1).
You can't really add new required properties unless they are for a new compatible string.
Rob
On 5/4/2016 7:14 PM, Rob Herring wrote:
On Tue, May 03, 2016 at 04:28:01PM +0530, Archit Taneja wrote:
The PLL in the DSI PHY block generates 2 clock outputs (Byte and Pixel clocks) that are fed into the Multimedia Clock Controller (MMCC). The MMCC uses these as source clocks for some of its RCGs to generate clocks that finally feed to the DSI host controller.
Use the assigned clocks DT bindings to set up the MMCC RCGs that feed to the DSI host. Use the DSI PHY provided clocks to set up the parents of these assigned clocks.
Signed-off-by: Archit Taneja architt@codeaurora.org
Documentation/devicetree/bindings/display/msm/dsi.txt | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt index 0223f06..686f475 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi.txt +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt @@ -22,6 +22,10 @@ Required properties: * "core_clk" For DSIv2, we need an additional clock: * "src_clk" +- assigned-clocks: Parents of "byte_clk" and "pixel_clk" for the given platform.
- See [1] for more details.
+- assigned-clock-parents: The Byte clock and Pixel clock PLL outputs provided
- by a DSI PHY block.
- vdd-supply: phandle to vdd regulator device node
- vddio-supply: phandle to vdd-io regulator device node
- vdda-supply: phandle to vdda regulator device node
@@ -90,6 +94,8 @@ Required properties: * "dsi_pll" * "dsi_phy" * "dsi_phy_regulator" +- clock-cells: Must be 1. The DSI PHY block acts as a clock provider, creating
- 2 clocks: A byte clock (index 0), and a pixel clock (index 1).
You can't really add new required properties unless they are for a new compatible string.
Does this hold even when currently there isn't any device tree file in the kernel that has this DT node in it?
I was trying to get all the properties in place before posting out patches that actually add the nodes into the platform files. Currently, they exist only DT files in downstream kernels.
Thanks, Archit
On Wed, May 04, 2016 at 11:34:39PM +0530, Archit Taneja wrote:
On 5/4/2016 7:14 PM, Rob Herring wrote:
On Tue, May 03, 2016 at 04:28:01PM +0530, Archit Taneja wrote:
The PLL in the DSI PHY block generates 2 clock outputs (Byte and Pixel clocks) that are fed into the Multimedia Clock Controller (MMCC). The MMCC uses these as source clocks for some of its RCGs to generate clocks that finally feed to the DSI host controller.
Use the assigned clocks DT bindings to set up the MMCC RCGs that feed to the DSI host. Use the DSI PHY provided clocks to set up the parents of these assigned clocks.
Signed-off-by: Archit Taneja architt@codeaurora.org
Documentation/devicetree/bindings/display/msm/dsi.txt | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt index 0223f06..686f475 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi.txt +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt @@ -22,6 +22,10 @@ Required properties:
- "core_clk"
For DSIv2, we need an additional clock: * "src_clk" +- assigned-clocks: Parents of "byte_clk" and "pixel_clk" for the given platform.
- See [1] for more details.
+- assigned-clock-parents: The Byte clock and Pixel clock PLL outputs provided
- by a DSI PHY block.
- vdd-supply: phandle to vdd regulator device node
- vddio-supply: phandle to vdd-io regulator device node
- vdda-supply: phandle to vdda regulator device node
@@ -90,6 +94,8 @@ Required properties:
- "dsi_pll"
- "dsi_phy"
- "dsi_phy_regulator"
+- clock-cells: Must be 1. The DSI PHY block acts as a clock provider, creating
- 2 clocks: A byte clock (index 0), and a pixel clock (index 1).
You can't really add new required properties unless they are for a new compatible string.
Does this hold even when currently there isn't any device tree file in the kernel that has this DT node in it?
Generally it should, but in this case that is fine.
Acked-by: Rob Herring robh@kernel.org
I was trying to get all the properties in place before posting out patches that actually add the nodes into the platform files. Currently, they exist only DT files in downstream kernels.
"If it is not upstream, it doesn't exist." :)
Rob
Currently, none of the upstream Qualcomm DT files have the display device nodes populated, even when the DT binding documents are upstream.
This revision drops the patches that add components via parsing ports. The component addition changes will be handled in a separate patchset later. These contain mostly misc DT fixes, in preparation for full DT support. This also incorporates some of the comments given for the previous revision.
changes in v2: - Drop patches that allowed component addition via MDP port parsing. - Use a more standard data lanes binding for DSI. - Use hyphen in the common PHY binding names for the examples. - Add some DT related fixes for MDP4/5 clocks.
Original revision: https://lists.freedesktop.org/archives/dri-devel/2016-May/106653.html
Archit Taneja (10): drm/msm/mdp5: Don't get source of MDP core clock drm/msm/mdp4: Clean up some MDP4 clocks dt-bindings: msm/mdp: Fix up clock related bindings drm/msm/dsi: Modify port parsing drm/msm/dsi: Use generic PHY bindings drm/msm/dsi: Use a standard DT binding for data lanes dt-bindings: msm/dsi: Use standard data lanes binding dt-bindings: msm/dsi: Modify port and PHY bindings dt-bindings: msm/dsi: Add assigned clocks bindings dt-bindings: msm/dsi: Some binding doc cleanups
.../devicetree/bindings/display/msm/dsi.txt | 117 ++++++++++++++------- .../devicetree/bindings/display/msm/mdp.txt | 10 +- drivers/gpu/drm/msm/dsi/dsi.c | 2 +- drivers/gpu/drm/msm/dsi/dsi_host.c | 27 +++-- drivers/gpu/drm/msm/mdp/mdp4/mdp4_dtv_encoder.c | 31 +++--- drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 2 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 7 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 1 - 8 files changed, 116 insertions(+), 81 deletions(-)
The driver expects DT to provide the parent to MDP core clock. The only operation done to the parent clock is to set a rate. This can be achieved by setting the rate on the core clock itsef. Don't try to get the parent clock anymore.
Signed-off-by: Archit Taneja architt@codeaurora.org --- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 7 ++----- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 1 - 2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c index 1d840ae..b46961e 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c @@ -629,9 +629,6 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev) ret = get_clk(pdev, &mdp5_kms->ahb_clk, "iface_clk", true); if (ret) goto fail; - ret = get_clk(pdev, &mdp5_kms->src_clk, "core_clk_src", true); - if (ret) - goto fail; ret = get_clk(pdev, &mdp5_kms->core_clk, "core_clk", true); if (ret) goto fail; @@ -646,7 +643,7 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev) * rate first, then figure out hw revision, and then set a * more optimal rate: */ - clk_set_rate(mdp5_kms->src_clk, 200000000); + clk_set_rate(mdp5_kms->core_clk, 200000000);
read_hw_revision(mdp5_kms, &major, &minor);
@@ -661,7 +658,7 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev) mdp5_kms->caps = config->hw->mdp.caps;
/* TODO: compute core clock rate at runtime */ - clk_set_rate(mdp5_kms->src_clk, config->hw->max_clk); + clk_set_rate(mdp5_kms->core_clk, config->hw->max_clk);
/* * Some chipsets have a Shared Memory Pool (SMP), while others diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h index 9a25898..9cf5aa4 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h @@ -49,7 +49,6 @@ struct mdp5_kms {
struct clk *axi_clk; struct clk *ahb_clk; - struct clk *src_clk; struct clk *core_clk; struct clk *lut_clk; struct clk *vsync_clk;
Fix some issues with MDP4 clocks:
- mdp4_dtv_encoder tries to get "src_clk", which is a RCG(TV_SRC) in MSM8960 and APQ8064. This isn't something the driver should access or configure. Instead of this, configure the "mdp_clk" (MDP_TV_CLK), a branch clock in MMCC that has the TV_SRC as its parent. Setting rate/enabling the "mdp_clk" will eventually configure "src_clk", which is what we want. - Rename "mdp_clk" to "tv_clk" because that's slightly less confusing. - Rename "mdp_axi_clk" to "bus_clk" because that's what we do elsewhere too.
Signed-off-by: Archit Taneja architt@codeaurora.org --- drivers/gpu/drm/msm/mdp/mdp4/mdp4_dtv_encoder.c | 31 ++++++++++--------------- drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 2 +- 2 files changed, 13 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_dtv_encoder.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_dtv_encoder.c index 35ad78a..24258e3 100644 --- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_dtv_encoder.c +++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_dtv_encoder.c @@ -23,7 +23,6 @@
struct mdp4_dtv_encoder { struct drm_encoder base; - struct clk *src_clk; struct clk *hdmi_clk; struct clk *mdp_clk; unsigned long int pixclock; @@ -179,7 +178,6 @@ static void mdp4_dtv_encoder_disable(struct drm_encoder *encoder) */ mdp_irq_wait(&mdp4_kms->base, MDP4_IRQ_EXTERNAL_VSYNC);
- clk_disable_unprepare(mdp4_dtv_encoder->src_clk); clk_disable_unprepare(mdp4_dtv_encoder->hdmi_clk); clk_disable_unprepare(mdp4_dtv_encoder->mdp_clk);
@@ -208,19 +206,21 @@ static void mdp4_dtv_encoder_enable(struct drm_encoder *encoder)
bs_set(mdp4_dtv_encoder, 1);
- DBG("setting src_clk=%lu", pc); + DBG("setting mdp_clk=%lu", pc);
- ret = clk_set_rate(mdp4_dtv_encoder->src_clk, pc); + ret = clk_set_rate(mdp4_dtv_encoder->mdp_clk, pc); if (ret) - dev_err(dev->dev, "failed to set src_clk to %lu: %d\n", pc, ret); - clk_prepare_enable(mdp4_dtv_encoder->src_clk); - ret = clk_prepare_enable(mdp4_dtv_encoder->hdmi_clk); - if (ret) - dev_err(dev->dev, "failed to enable hdmi_clk: %d\n", ret); + dev_err(dev->dev, "failed to set mdp_clk to %lu: %d\n", + pc, ret); + ret = clk_prepare_enable(mdp4_dtv_encoder->mdp_clk); if (ret) dev_err(dev->dev, "failed to enabled mdp_clk: %d\n", ret);
+ ret = clk_prepare_enable(mdp4_dtv_encoder->hdmi_clk); + if (ret) + dev_err(dev->dev, "failed to enable hdmi_clk: %d\n", ret); + mdp4_write(mdp4_kms, REG_MDP4_DTV_ENABLE, 1);
mdp4_dtv_encoder->enabled = true; @@ -235,7 +235,7 @@ static const struct drm_encoder_helper_funcs mdp4_dtv_encoder_helper_funcs = { long mdp4_dtv_round_pixclk(struct drm_encoder *encoder, unsigned long rate) { struct mdp4_dtv_encoder *mdp4_dtv_encoder = to_mdp4_dtv_encoder(encoder); - return clk_round_rate(mdp4_dtv_encoder->src_clk, rate); + return clk_round_rate(mdp4_dtv_encoder->mdp_clk, rate); }
/* initialize encoder */ @@ -257,13 +257,6 @@ struct drm_encoder *mdp4_dtv_encoder_init(struct drm_device *dev) DRM_MODE_ENCODER_TMDS, NULL); drm_encoder_helper_add(encoder, &mdp4_dtv_encoder_helper_funcs);
- mdp4_dtv_encoder->src_clk = devm_clk_get(dev->dev, "src_clk"); - if (IS_ERR(mdp4_dtv_encoder->src_clk)) { - dev_err(dev->dev, "failed to get src_clk\n"); - ret = PTR_ERR(mdp4_dtv_encoder->src_clk); - goto fail; - } - mdp4_dtv_encoder->hdmi_clk = devm_clk_get(dev->dev, "hdmi_clk"); if (IS_ERR(mdp4_dtv_encoder->hdmi_clk)) { dev_err(dev->dev, "failed to get hdmi_clk\n"); @@ -271,9 +264,9 @@ struct drm_encoder *mdp4_dtv_encoder_init(struct drm_device *dev) goto fail; }
- mdp4_dtv_encoder->mdp_clk = devm_clk_get(dev->dev, "mdp_clk"); + mdp4_dtv_encoder->mdp_clk = devm_clk_get(dev->dev, "tv_clk"); if (IS_ERR(mdp4_dtv_encoder->mdp_clk)) { - dev_err(dev->dev, "failed to get mdp_clk\n"); + dev_err(dev->dev, "failed to get tv_clk\n"); ret = PTR_ERR(mdp4_dtv_encoder->mdp_clk); goto fail; } diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c index f145d25..388663a 100644 --- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c +++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c @@ -492,7 +492,7 @@ struct msm_kms *mdp4_kms_init(struct drm_device *dev) goto fail; }
- mdp4_kms->axi_clk = devm_clk_get(&pdev->dev, "mdp_axi_clk"); + mdp4_kms->axi_clk = devm_clk_get(&pdev->dev, "bus_clk"); if (IS_ERR(mdp4_kms->axi_clk)) { dev_err(dev->dev, "failed to get axi_clk\n"); ret = PTR_ERR(mdp4_kms->axi_clk);
MDP5: - Don't ask for source clock
MDP4: - Give a better name for MDP_TV_CLK - Remove TV_SRC - Add MDP_AXI_CLK
Signed-off-by: Archit Taneja architt@codeaurora.org --- Documentation/devicetree/bindings/display/msm/mdp.txt | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/mdp.txt b/Documentation/devicetree/bindings/display/msm/mdp.txt index a214f6c..1d3d79d 100644 --- a/Documentation/devicetree/bindings/display/msm/mdp.txt +++ b/Documentation/devicetree/bindings/display/msm/mdp.txt @@ -13,14 +13,13 @@ Required properties: For MDP4: * "core_clk" * "iface_clk" + * "bus_clk" * "lut_clk" - * "src_clk" * "hdmi_clk" - * "mdp_clk" + * "tv_clk" For MDP5: * "bus_clk" * "iface_clk" - * "core_clk_src" * "core_clk" * "lut_clk" (some MDP5 versions may not need this) * "vsync_clk" @@ -45,14 +44,13 @@ Example: "core_clk", "iface_clk", "lut_clk", - "src_clk", "hdmi_clk", "mdp_clk"; clocks = - <&mmcc MDP_SRC>, + <&mmcc MDP_CLK>, <&mmcc MDP_AHB_CLK>, + <&mmcc MDP_AXI_CLK>, <&mmcc MDP_LUT_CLK>, - <&mmcc TV_SRC>, <&mmcc HDMI_TV_CLK>, <&mmcc MDP_TV_CLK>; };
On Fri, Jun 10, 2016 at 04:16:33PM +0530, Archit Taneja wrote:
MDP5:
- Don't ask for source clock
MDP4:
- Give a better name for MDP_TV_CLK
- Remove TV_SRC
- Add MDP_AXI_CLK
This could use the explanation in the commit msg why it is okay you are breaking compatibility.
With that,
Acked-by: Rob Herring robh@kernel.org
On 06/14/2016 12:33 AM, Rob Herring wrote:
On Fri, Jun 10, 2016 at 04:16:33PM +0530, Archit Taneja wrote:
MDP5:
- Don't ask for source clock
MDP4:
- Give a better name for MDP_TV_CLK
- Remove TV_SRC
- Add MDP_AXI_CLK
This could use the explanation in the commit msg why it is okay you are breaking compatibility.
Sure, I'll do that. Thanks for the reviews.
Archit
With that,
Acked-by: Rob Herring robh@kernel.org
The DSI interface is going to have two ports defined in its device node. The first port is always going to be the link between the MDP output and the input to DSI, the second port is going to be the link between the DSI output and the connected panel/bridge:
----- ----- ------- | MDP | ------> | DSI | ------> | Panel | ----- ----- ------- (Port 0) (Port 1)
Until now, there was only one Port representing the output. Update the DSI host driver such that it parses Port #1 for a connected device.
Signed-off-by: Archit Taneja architt@codeaurora.org --- drivers/gpu/drm/msm/dsi/dsi_host.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index a3e47ad8..4f1335e 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -1602,12 +1602,12 @@ static int dsi_host_parse_dt(struct msm_dsi_host *msm_host) }
/* - * Get the first endpoint node. In our case, dsi has one output port - * to which the panel is connected. Don't return an error if a port - * isn't defined. It's possible that there is nothing connected to - * the dsi output. + * Get the endpoint of the output port of the DSI host. In our case, + * this is mapped to port number with reg = 1. Don't return an error if + * the remote endpoint isn't defined. It's possible that there is + * nothing connected to the dsi output. */ - endpoint = of_graph_get_next_endpoint(np, NULL); + endpoint = of_graph_get_endpoint_by_regs(np, 1, -1); if (!endpoint) { dev_dbg(dev, "%s: no endpoint\n", __func__); return 0;
The DSI host links to the DSI PHY device using a custom binding. Switch to the generic PHY bindings. The DSI PHY driver itself doesn't use the common PHY framework for now.
Signed-off-by: Archit Taneja architt@codeaurora.org --- drivers/gpu/drm/msm/dsi/dsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c index 6edcd6f..ec572f8 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.c +++ b/drivers/gpu/drm/msm/dsi/dsi.c @@ -29,7 +29,7 @@ static int dsi_get_phy(struct msm_dsi *msm_dsi) struct platform_device *phy_pdev; struct device_node *phy_node;
- phy_node = of_parse_phandle(pdev->dev.of_node, "qcom,dsi-phy", 0); + phy_node = of_parse_phandle(pdev->dev.of_node, "phys", 0); if (!phy_node) { dev_err(&pdev->dev, "cannot find phy device\n"); return -ENXIO;
A more standard DT binding describing data lanes already exists here: Documentation/devicetree/bindings/media/video-interfaces.txt
Use this binding instead of "qcom,data-lane-map". One difference in the standard binding w.r.t to the existing binding is that it provides a logical to physical mapping instead of the other way round. Tweak the code to translate the data the way we want it.
The MSM DSI DT bindings aren't used anywhere at the moment, so it's okay to update this property.
Signed-off-by: Archit Taneja architt@codeaurora.org --- drivers/gpu/drm/msm/dsi/dsi_host.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 4f1335e..e6a8cd1 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -1543,7 +1543,7 @@ static int dsi_host_parse_lane_data(struct msm_dsi_host *msm_host, u32 lane_map[4]; int ret, i, len, num_lanes;
- prop = of_find_property(ep, "qcom,data-lane-map", &len); + prop = of_find_property(ep, "data-lanes", &len); if (!prop) { dev_dbg(dev, "failed to find data lane mapping\n"); return -EINVAL; @@ -1558,7 +1558,7 @@ static int dsi_host_parse_lane_data(struct msm_dsi_host *msm_host,
msm_host->num_data_lanes = num_lanes;
- ret = of_property_read_u32_array(ep, "qcom,data-lane-map", lane_map, + ret = of_property_read_u32_array(ep, "data-lanes", lane_map, num_lanes); if (ret) { dev_err(dev, "failed to read lane data\n"); @@ -1573,8 +1573,19 @@ static int dsi_host_parse_lane_data(struct msm_dsi_host *msm_host, const int *swap = supported_data_lane_swaps[i]; int j;
+ /* + * the data-lanes array we get from DT has a logical->physical + * mapping. The "data lane swap" register field represents + * supported configurations in a physical->logical mapping. + * Translate the DT mapping to what we understand and find a + * configuration that works. + */ for (j = 0; j < num_lanes; j++) { - if (swap[j] != lane_map[j]) + if (lane_map[j] < 0 || lane_map[j] > 3) + dev_err(dev, "bad physical lane entry %u\n", + lane_map[j]); + + if (swap[lane_map[j]] != j) break; }
The "qcom,data-lane-map" binding mentioned in the document is changed to the more generic "data-lanes" property specified in:
Documentation/devicetree/bindings/media/video-interfaces.txt
The previous binding expressed physical to logical data lane mappings, the standard "data-lanes" binding uses logical to physical data lane mappings. Update the docs to reflect this change. The example had the property incorrectly named as "lanes", update this too.
The MSM DSI DT bindings aren't used anywhere at the moment, so it's okay to update this property.
Signed-off-by: Archit Taneja architt@codeaurora.org --- .../devicetree/bindings/display/msm/dsi.txt | 37 +++++++++++----------- 1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt index f5948c4..9d2d2bb 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi.txt +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt @@ -49,29 +49,30 @@ Optional properties: DSI Endpoint properties: - remote-endpoint: set to phandle of the connected panel's endpoint. See Documentation/devicetree/bindings/graph.txt for device graph info. - - qcom,data-lane-map: this describes how the logical DSI lanes are mapped - to the physical lanes on the given platform. The value contained in - index n describes what logical data lane is mapped to the physical data - lane n (DATAn, where n lies between 0 and 3). + - data-lanes: this describes how the physical DSI data lanes are mapped + to the logical lanes on the given platform. The value contained in + index n describes what physical lane is mapped to the logical lane n + (DATAn, where n lies between 0 and 3). The clock lane position is fixed + and can't be changed. Hence, they aren't a part of the DT bindings. For + more info, see Documentation/devicetree/bindings/media/video-interfaces.txt
For example:
- qcom,data-lane-map = <3 0 1 2>; + data-lanes = <3 0 1 2>;
- The above mapping describes that the logical data lane DATA3 is mapped to - the physical data lane DATA0, logical DATA0 to physical DATA1, logic DATA1 - to phys DATA2 and logic DATA2 to phys DATA3. + The above mapping describes that the logical data lane DATA0 is mapped to + the physical data lane DATA3, logical DATA1 to physical DATA0, logic DATA2 + to phys DATA1 and logic DATA3 to phys DATA2.
There are only a limited number of physical to logical mappings possible: - - "0123": Logic 0->Phys 0; Logic 1->Phys 1; Logic 2->Phys 2; Logic 3->Phys 3; - "3012": Logic 3->Phys 0; Logic 0->Phys 1; Logic 1->Phys 2; Logic 2->Phys 3; - "2301": Logic 2->Phys 0; Logic 3->Phys 1; Logic 0->Phys 2; Logic 1->Phys 3; - "1230": Logic 1->Phys 0; Logic 2->Phys 1; Logic 3->Phys 2; Logic 0->Phys 3; - "0321": Logic 0->Phys 0; Logic 3->Phys 1; Logic 2->Phys 2; Logic 1->Phys 3; - "1032": Logic 1->Phys 0; Logic 0->Phys 1; Logic 3->Phys 2; Logic 2->Phys 3; - "2103": Logic 2->Phys 0; Logic 1->Phys 1; Logic 0->Phys 2; Logic 3->Phys 3; - "3210": Logic 3->Phys 0; Logic 2->Phys 1; Logic 1->Phys 2; Logic 0->Phys 3; + <0 1 2 3> + <1 2 3 0> + <2 3 0 1> + <3 0 1 2> + <0 3 2 1> + <1 0 3 2> + <2 1 0 3> + <3 2 1 0>
DSI PHY: Required properties: @@ -156,7 +157,7 @@ Example: port { dsi0_out: endpoint { remote-endpoint = <&panel_in>; - lanes = <0 1 2 3>; + data-lanes = <0 1 2 3>; }; }; };
On Fri, Jun 10, 2016 at 04:16:37PM +0530, Archit Taneja wrote:
The "qcom,data-lane-map" binding mentioned in the document is changed to the more generic "data-lanes" property specified in:
Documentation/devicetree/bindings/media/video-interfaces.txt
The previous binding expressed physical to logical data lane mappings, the standard "data-lanes" binding uses logical to physical data lane mappings. Update the docs to reflect this change. The example had the property incorrectly named as "lanes", update this too.
The MSM DSI DT bindings aren't used anywhere at the moment, so it's okay to update this property.
Signed-off-by: Archit Taneja architt@codeaurora.org
.../devicetree/bindings/display/msm/dsi.txt | 37 +++++++++++----------- 1 file changed, 19 insertions(+), 18 deletions(-)
Acked-by: Rob Herring robh@kernel.org
The DSI node now has two ports that describe the connection between the MDP interface output and the DSI input, and the connection between the DSI output and the connected panel/bridge. Update the properties and the example.
Also, use generic PHY bindings instead of the custom one.
Signed-off-by: Archit Taneja architt@codeaurora.org --- v2: - Use hyphen for dsi phy name
.../devicetree/bindings/display/msm/dsi.txt | 45 ++++++++++++++++------ 1 file changed, 34 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt index 9d2d2bb..6442408 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi.txt +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt @@ -26,8 +26,14 @@ Required properties: - vdd-supply: phandle to vdd regulator device node - vddio-supply: phandle to vdd-io regulator device node - vdda-supply: phandle to vdda regulator device node -- qcom,dsi-phy: phandle to DSI PHY device node +- phys: phandle to DSI PHY device node +- phy-names: the name of the corresponding PHY device - syscon-sfpb: A phandle to mmss_sfpb syscon node (only for DSIv2) +- ports: Contains 2 DSI controller ports as child nodes. Each port contains + an endpoint subnode as defined in these documents: + + Documentation/devicetree/bindings/graph.txt + Documentation/devicetree/bindings/media/video-interfaces.txt
Optional properties: - panel@0: Node of panel connected to this DSI controller. @@ -44,11 +50,14 @@ Optional properties: - pinctrl-names: the pin control state names; should contain "default" - pinctrl-0: the default pinctrl state (active) - pinctrl-n: the "sleep" pinctrl state -- port: DSI controller output port, containing one endpoint subnode. +- ports: contains DSI controller input and output ports as children, each + containing one endpoint subnode.
DSI Endpoint properties: - - remote-endpoint: set to phandle of the connected panel's endpoint. + - remote-endpoint: For port@0, set to phandle of the connected panel/bridge's + input endpoint. For port@1, set to the MDP interface output. See Documentation/devicetree/bindings/graph.txt for device graph info. + - data-lanes: this describes how the physical DSI data lanes are mapped to the logical lanes on the given platform. The value contained in index n describes what physical lane is mapped to the logical lane n @@ -129,7 +138,8 @@ Example: vdd-supply = <&pma8084_l22>; vddio-supply = <&pma8084_l12>;
- qcom,dsi-phy = <&mdss_dsi_phy0>; + phys = <&mdss_dsi_phy0>; + phy-names ="dsi-phy";
qcom,dual-dsi-mode; qcom,master-dsi; @@ -139,6 +149,26 @@ Example: pinctrl-0 = <&mdss_dsi_active>; pinctrl-1 = <&mdss_dsi_suspend>;
+ ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dsi0_in: endpoint { + remote-endpoint = <&mdp_intf1_out>; + }; + }; + + port@1 { + reg = <1>; + dsi0_out: endpoint { + remote-endpoint = <&panel_in>; + data-lanes = <0 1 2 3>; + }; + }; + }; + panel: panel@0 { compatible = "sharp,lq101r1sx01"; reg = <0>; @@ -153,13 +183,6 @@ Example: }; }; }; - - port { - dsi0_out: endpoint { - remote-endpoint = <&panel_in>; - data-lanes = <0 1 2 3>; - }; - }; };
mdss_dsi_phy0: qcom,mdss_dsi_phy@fd922a00 {
On Fri, Jun 10, 2016 at 04:16:38PM +0530, Archit Taneja wrote:
The DSI node now has two ports that describe the connection between the MDP interface output and the DSI input, and the connection between the DSI output and the connected panel/bridge. Update the properties and the example.
Also, use generic PHY bindings instead of the custom one.
Signed-off-by: Archit Taneja architt@codeaurora.org
v2:
- Use hyphen for dsi phy name
.../devicetree/bindings/display/msm/dsi.txt | 45 ++++++++++++++++------ 1 file changed, 34 insertions(+), 11 deletions(-)
Acked-by: Rob Herring robh@kernel.org
The PLL in the DSI PHY block generates 2 clock outputs (Byte and Pixel clocks) that are fed into the Multimedia Clock Controller (MMCC). The MMCC uses these as source clocks for some of its RCGs to generate clocks that finally feed to the DSI host controller.
Use the assigned clocks DT bindings to set up the MMCC RCGs that feed to the DSI host. Use the DSI PHY provided clocks to set up the parents of these assigned clocks.
Acked-by: Rob Herring robh@kernel.org Signed-off-by: Archit Taneja architt@codeaurora.org --- Documentation/devicetree/bindings/display/msm/dsi.txt | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt index 6442408..f458929 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi.txt +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt @@ -12,7 +12,6 @@ Required properties: - interrupts: The interrupt signal from the DSI block. - power-domains: Should be <&mmcc MDSS_GDSC>. - clocks: device clocks - See Documentation/devicetree/bindings/clocks/clock-bindings.txt for details. - clock-names: the following clocks are required: * "mdp_core_clk" * "iface_clk" @@ -23,6 +22,10 @@ Required properties: * "core_clk" For DSIv2, we need an additional clock: * "src_clk" +- assigned-clocks: Parents of "byte_clk" and "pixel_clk" for the given platform. +- assigned-clock-parents: The Byte clock and Pixel clock PLL outputs provided + by a DSI PHY block. + See Documentation/devicetree/bindings/clocks/clock-bindings.txt for details. - vdd-supply: phandle to vdd regulator device node - vddio-supply: phandle to vdd-io regulator device node - vdda-supply: phandle to vdda regulator device node @@ -96,6 +99,8 @@ Required properties: * "dsi_pll" * "dsi_phy" * "dsi_phy_regulator" +- clock-cells: Must be 1. The DSI PHY block acts as a clock provider, creating + 2 clocks: A byte clock (index 0), and a pixel clock (index 1). - qcom,dsi-phy-index: The ID of DSI PHY hardware instance. This should be 0 or 1, since we have 2 DSI PHYs at most for now. - power-domains: Should be <&mmcc MDSS_GDSC>. @@ -134,6 +139,14 @@ Example: <&mmcc MDSS_AHB_CLK>, <&mmcc MDSS_MDP_CLK>, <&mmcc MDSS_PCLK0_CLK>; + + assigned-clocks = + <&mmcc BYTE0_CLK_SRC>, + <&mmcc PCLK0_CLK_SRC>; + assigned-clock-parents = + <&mdss_dsi_phy0 0>, + <&mdss_dsi_phy0 1>; + vdda-supply = <&pma8084_l2>; vdd-supply = <&pma8084_l22>; vddio-supply = <&pma8084_l12>; @@ -197,6 +210,7 @@ Example: <0xfd922d80 0x7b>; clock-names = "iface_clk"; clocks = <&mmcc MDSS_AHB_CLK>; + #clock-cells = <1>; vddio-supply = <&pma8084_l12>;
qcom,dsi-phy-regulator-ldo-mode;
Some cleanups:
- Use simpler names for DT nodes in the example - Use references instead of dumping Document links everywhere
Signed-off-by: Archit Taneja architt@codeaurora.org --- .../devicetree/bindings/display/msm/dsi.txt | 45 +++++++++++----------- 1 file changed, 22 insertions(+), 23 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt index f458929..6b1cab1 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi.txt +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt @@ -11,7 +11,7 @@ Required properties: be 0 or 1, since we have 2 DSI controllers at most for now. - interrupts: The interrupt signal from the DSI block. - power-domains: Should be <&mmcc MDSS_GDSC>. -- clocks: device clocks +- clocks: Phandles to device clocks. - clock-names: the following clocks are required: * "mdp_core_clk" * "iface_clk" @@ -24,8 +24,7 @@ Required properties: * "src_clk" - assigned-clocks: Parents of "byte_clk" and "pixel_clk" for the given platform. - assigned-clock-parents: The Byte clock and Pixel clock PLL outputs provided - by a DSI PHY block. - See Documentation/devicetree/bindings/clocks/clock-bindings.txt for details. + by a DSI PHY block. See [1] for details on clock bindings. - vdd-supply: phandle to vdd regulator device node - vddio-supply: phandle to vdd-io regulator device node - vdda-supply: phandle to vdda regulator device node @@ -33,15 +32,11 @@ Required properties: - phy-names: the name of the corresponding PHY device - syscon-sfpb: A phandle to mmss_sfpb syscon node (only for DSIv2) - ports: Contains 2 DSI controller ports as child nodes. Each port contains - an endpoint subnode as defined in these documents: - - Documentation/devicetree/bindings/graph.txt - Documentation/devicetree/bindings/media/video-interfaces.txt + an endpoint subnode as defined in [2] and [3].
Optional properties: - panel@0: Node of panel connected to this DSI controller. - See files in Documentation/devicetree/bindings/display/panel/ for each supported - panel. + See files in [4] for each supported panel. - qcom,dual-dsi-mode: Boolean value indicating if the DSI controller is driving a panel which needs 2 DSI links. - qcom,master-dsi: Boolean value indicating if the DSI controller is driving @@ -58,15 +53,15 @@ Optional properties:
DSI Endpoint properties: - remote-endpoint: For port@0, set to phandle of the connected panel/bridge's - input endpoint. For port@1, set to the MDP interface output. - See Documentation/devicetree/bindings/graph.txt for device graph info. + input endpoint. For port@1, set to the MDP interface output. See [2] for + device graph info.
- data-lanes: this describes how the physical DSI data lanes are mapped to the logical lanes on the given platform. The value contained in index n describes what physical lane is mapped to the logical lane n (DATAn, where n lies between 0 and 3). The clock lane position is fixed - and can't be changed. Hence, they aren't a part of the DT bindings. For - more info, see Documentation/devicetree/bindings/media/video-interfaces.txt + and can't be changed. Hence, they aren't a part of the DT bindings. See + [3] for more info on the data-lanes property.
For example:
@@ -104,8 +99,7 @@ Required properties: - qcom,dsi-phy-index: The ID of DSI PHY hardware instance. This should be 0 or 1, since we have 2 DSI PHYs at most for now. - power-domains: Should be <&mmcc MDSS_GDSC>. -- clocks: device clocks - See Documentation/devicetree/bindings/clocks/clock-bindings.txt for details. +- clocks: Phandles to device clocks. See [1] for details on clock bindings. - clock-names: the following clocks are required: * "iface_clk" - vddio-supply: phandle to vdd-io regulator device node @@ -114,11 +108,16 @@ Optional properties: - qcom,dsi-phy-regulator-ldo-mode: Boolean value indicating if the LDO mode PHY regulator is wanted.
+[1] Documentation/devicetree/bindings/clocks/clock-bindings.txt +[2] Documentation/devicetree/bindings/graph.txt +[3] Documentation/devicetree/bindings/media/video-interfaces.txt +[4] Documentation/devicetree/bindings/display/panel/ + Example: - mdss_dsi0: qcom,mdss_dsi@fd922800 { + dsi0: dsi@fd922800 { compatible = "qcom,mdss-dsi-ctrl"; qcom,dsi-host-index = <0>; - interrupt-parent = <&mdss_mdp>; + interrupt-parent = <&mdp>; interrupts = <4 0>; reg-names = "dsi_ctrl"; reg = <0xfd922800 0x200>; @@ -144,14 +143,14 @@ Example: <&mmcc BYTE0_CLK_SRC>, <&mmcc PCLK0_CLK_SRC>; assigned-clock-parents = - <&mdss_dsi_phy0 0>, - <&mdss_dsi_phy0 1>; + <&dsi_phy0 0>, + <&dsi_phy0 1>;
vdda-supply = <&pma8084_l2>; vdd-supply = <&pma8084_l22>; vddio-supply = <&pma8084_l12>;
- phys = <&mdss_dsi_phy0>; + phys = <&dsi_phy0>; phy-names ="dsi-phy";
qcom,dual-dsi-mode; @@ -159,8 +158,8 @@ Example: qcom,sync-dual-dsi;
pinctrl-names = "default", "sleep"; - pinctrl-0 = <&mdss_dsi_active>; - pinctrl-1 = <&mdss_dsi_suspend>; + pinctrl-0 = <&dsi_active>; + pinctrl-1 = <&dsi_suspend>;
ports { #address-cells = <1>; @@ -198,7 +197,7 @@ Example: }; };
- mdss_dsi_phy0: qcom,mdss_dsi_phy@fd922a00 { + dsi_phy0: dsi-phy@fd922a00 { compatible = "qcom,dsi-phy-28nm-hpm"; qcom,dsi-phy-index = <0>; reg-names =
On Fri, Jun 10, 2016 at 04:16:40PM +0530, Archit Taneja wrote:
Some cleanups:
- Use simpler names for DT nodes in the example
- Use references instead of dumping Document links everywhere
Signed-off-by: Archit Taneja architt@codeaurora.org
.../devicetree/bindings/display/msm/dsi.txt | 45 +++++++++++----------- 1 file changed, 22 insertions(+), 23 deletions(-)
Acked-by: Rob Herring robh@kernel.org
dri-devel@lists.freedesktop.org
-
Archit Taneja -
Philipp Zabel -
Rob Herring