dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon
Subsystems (6)
PATCH v6 1/5 dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon
From: Chandan Uddaraju <[email protected]>
Add bindings for Snapdragon DisplayPort controller driver.
Changes in V2:
Provide details about sel-gpio
Changes in V4:
Provide details about max dp lanes
Change the commit text
Changes in V5:
moved dp.txt to yaml file
Changes in v6:
- Squash all AUX LUT properties into one pattern Property
- Make aux-cfg[0-9]-settings properties optional
- Remove PLL/PHY bindings from DP controller dts
- Add DP clocks description
- Remove _clk suffix from clock names
- Rename pixel clock to stream_pixel
- Remove redundant bindings (GPIO, PHY, HDCP clock, etc..)
- Fix indentation
- Add Display Port as interface of DPU in DPU bindings and add port mapping accordingly.
--- .../devicetree/bindings/display/msm/dp-sc7180.yaml | 142 +++++++++++++++++++++ .../devicetree/bindings/display/msm/dpu.txt | 8 ++ 2 files changed, 150 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +142 -0
@@ -0,0 +1,142 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/msm/dp-sc7180.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm Display Port Controller. + +maintainers: + - Chandan Uddaraju <[email protected]> + - Vara Reddy <[email protected]> + - Tanmay Shah <[email protected]> + +description: | + Device tree bindings for MSM Display Port which supports DP host controllers + that are compatible with VESA Display Port interface specification. + +properties: + compatible: + items: + - const: qcom,dp-display + + cell-index: + description: Specifies the controller instance. + + reg: + items: + - description: DP controller registers + + interrupts: + description: The interrupt signal from the DP block. + + clocks: + description: List of clock specifiers for clocks needed by the device. + items: + - description: Display Port AUX clock + - description: Display Port Link clock + - description: Link interface clock between DP and PHY + - description: Display Port Pixel clock + - description: Root clock generator for pixel clock + + clock-names: + description: | + Device clock names in the same order as mentioned in clocks property. + The required clocks are mentioned below. + items: + - const: core_aux + - const: ctrl_link + - const: ctrl_link_iface + - const: stream_pixel + - const: pixel_rcg + "#clock-cells": + const: 1 + + vdda-1p2-supply: + description: phandle to vdda 1.2V regulator node. + + vdda-0p9-supply: + description: phandle to vdda 0.9V regulator node. + + data-lanes = <0 1>: + type: object + description: Maximum number of lanes that can be used for Display port. + + ports: + description: | + Contains display port controller endpoint subnode. + remote-endpoint: | + For port@0, set to phandle of the connected panel/bridge's + input endpoint. For port@1, set to the DPU interface output. + Documentation/devicetree/bindings/graph.txt and + Documentation/devicetree/bindings/media/video-interfaces.txt. + +patternProperties: + "^aux-cfg([0-9])-settings$": + type: object + description: | + Specifies the DP AUX configuration [0-9] settings. + The first entry in this array corresponds to the register offset + within DP AUX, while the remaining entries indicate the + programmable values. + +required: + - compatible + - cell-index + - reg + - interrupts + - clocks + - clock-names + - vdda-1p2-supply + - vdda-0p9-supply + - data-lanes + - ports + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/clock/qcom,dispcc-sdm845.h> + #include <dt-bindings/clock/qcom,gcc-sdm845.h> + msm_dp: displayport-controller@ae90000{ + compatible = "qcom,dp-display"; + cell-index = <0>; + reg = <0 0xae90000 0 0x1400>; + reg-names = "dp_controller"; + + interrupt-parent = <&display_subsystem>; + interrupts = <12 0>; + + clocks = <&dispcc DISP_CC_MDSS_DP_AUX_CLK>, + <&dispcc DISP_CC_MDSS_DP_LINK_CLK>, + <&dispcc DISP_CC_MDSS_DP_LINK_INTF_CLK>, + <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK>, + <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK_SRC>; + clock-names = "core_aux", + "ctrl_link", + "ctrl_link_iface", "stream_pixel", + "pixel_rcg"; + #clock-cells = <1>; + + vdda-1p2-supply = <&vreg_l3c_1p2>; + vdda-0p9-supply = <&vreg_l4a_0p8>; + + data-lanes = <0 1>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dp_in: endpoint { + remote-endpoint = <&dpu_intf0_out>; + }; + }; + + port@1 { + reg = <1>; + dp_out: endpoint { + }; + }; + }; + };
Documentation/devicetree/bindings/display/msm/dpu.txt +8 -0
@@ -65,6 +65,7 @@ Required properties: Port 0 -> DPU_INTF1 (DSI1) Port 1 -> DPU_INTF2 (DSI2) + Port 2 -> DPU_INTF0 (DP) Optional properties: - assigned-clocks: list of clock specifiers for clocks needing rate assignment
@@ -136,6 +137,13 @@ Example: remote-endpoint = <&dsi1_in>; }; }; + + port@2 { + reg = <2>; + dpu_intf0_out: endpoint { + remote-endpoint = <&dp_in>; + }; + }; }; }; };
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Quoting Tanmay Shah (2020-06-08 20:38:18)
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
Quoted 5 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +0
-0
Typically the file name matches the compatible string. But the compatible string is just qcom,dp-display. Maybe the compatible string should be qcom,sc7180-dp? Notice that the SoC number comes first as is preferred.
Quoted 42 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +41
-0@@ -0,0 +1,142 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/msm/dp-sc7180.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm Display Port Controller. + +maintainers: + - Chandan Uddaraju <[email protected]> + - Vara Reddy <[email protected]> + - Tanmay Shah <[email protected]> + +description: | + Device tree bindings for MSM Display Port which supports DP host controllers + that are compatible with VESA Display Port interface specification. + +properties: + compatible: + items: + - const: qcom,dp-display + + cell-index: + description: Specifies the controller instance. + + reg: + items: + - description: DP controller registers + + interrupts: + description: The interrupt signal from the DP block. + + clocks: + description: List of clock specifiers for clocks needed by the device. + items: + - description: Display Port AUX clock + - description: Display Port Link clock + - description: Link interface clock between DP and PHY + - description: Display Port Pixel clock + - description: Root clock generator for pixel clock +
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml line 51
Quoted 10 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +10
-0+ clock-names: + description: | + Device clock names in the same order as mentioned in clocks property. + The required clocks are mentioned below. + items: + - const: core_aux + - const: ctrl_link + - const: ctrl_link_iface + - const: stream_pixel + - const: pixel_rcg
Why not just 'pixel'? And why is the root clk generator important? It looks like this binding should be using the assigned clock parents property instead so that it doesn't have to call clk_set_parent() explicitly.
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml line 61
Quoted 10 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +10
-0+ "#clock-cells": + const: 1 + + vdda-1p2-supply: + description: phandle to vdda 1.2V regulator node. + + vdda-0p9-supply: + description: phandle to vdda 0.9V regulator node. + + data-lanes = <0 1>:
Is this correct? We can have = <value> in the property name? Also feels generic and possibly should come from the phy binding instead of from the controller binding.
Quoted 10 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +10
-0+ type: object + description: Maximum number of lanes that can be used for Display port. + + ports: + description: | + Contains display port controller endpoint subnode. + remote-endpoint: | + For port@0, set to phandle of the connected panel/bridge's + input endpoint. For port@1, set to the DPU interface output. + Documentation/devicetree/bindings/graph.txt and
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml line 81
Quoted 10 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +10
-0+ Documentation/devicetree/bindings/media/video-interfaces.txt. + +patternProperties: + "^aux-cfg([0-9])-settings$": + type: object + description: | + Specifies the DP AUX configuration [0-9] settings. + The first entry in this array corresponds to the register offset + within DP AUX, while the remaining entries indicate the + programmable values.
I'd prefer this was removed from the binding and hardcoded in the driver until we can understand what the values are. If they're not understandable then they most likely don't change and should be done in the driver.
Quoted 51 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +51
-0+ +required: + - compatible + - cell-index + - reg + - interrupts + - clocks + - clock-names + - vdda-1p2-supply + - vdda-0p9-supply + - data-lanes + - ports + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/clock/qcom,dispcc-sdm845.h> + #include <dt-bindings/clock/qcom,gcc-sdm845.h> + msm_dp: displayport-controller@ae90000{ + compatible = "qcom,dp-display"; + cell-index = <0>; + reg = <0 0xae90000 0 0x1400>; + reg-names = "dp_controller"; + + interrupt-parent = <&display_subsystem>; + interrupts = <12 0>; + + clocks = <&dispcc DISP_CC_MDSS_DP_AUX_CLK>, + <&dispcc DISP_CC_MDSS_DP_LINK_CLK>, + <&dispcc DISP_CC_MDSS_DP_LINK_INTF_CLK>, + <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK>, + <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK_SRC>; + clock-names = "core_aux", + "ctrl_link", + "ctrl_link_iface", "stream_pixel", + "pixel_rcg"; + #clock-cells = <1>; + + vdda-1p2-supply = <&vreg_l3c_1p2>; + vdda-0p9-supply = <&vreg_l4a_0p8>; + + data-lanes = <0 1>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dp_in: endpoint { + remote-endpoint = <&dpu_intf0_out>;
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml line 142
Quoted 10 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +10
-0+ }; + }; + + port@1 { + reg = <1>; + dp_out: endpoint { + }; + }; + }; + };
I believe there should be a '...' here.
Quoted 12 lines · Stephen Boyd
Quoting Tanmay Shah (2020-06-08 20:38:18)
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
Quoted 5 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +0
-0Typically the file name matches the compatible string. But the compatible string is just qcom,dp-display. Maybe the compatible string should be qcom,sc7180-dp? Notice that the SoC number comes first as is preferred.
These bindings will be similar for upcoming SOC as well.
So just for understanding, when we add new SOC do we create new file with same bidings
with SOC number in new file name?
Instead we can keep this file's name as qcom,dp-display.yaml (same as compatible const) and we can include SOC number in compatible enum ? some examples:
https://patchwork.kernel.org/patch/11448357/
https://patchwork.kernel.org/patch/11164619/
Quoted 53 lines
Quoted 53 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +15
-0@@ -0,0 +1,142 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/msm/dp-sc7180.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm Display Port Controller. + +maintainers: + - Chandan Uddaraju <[email protected]> + - Vara Reddy <[email protected]> + - Tanmay Shah <[email protected]> + +description: | + Device tree bindings for MSM Display Port which supports DP hostcontrollers
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +19
-0+ that are compatible with VESA Display Port interface specification. + +properties: + compatible: + items: + - const: qcom,dp-display + + cell-index: + description: Specifies the controller instance. + + reg: + items: + - description: DP controller registers + + interrupts: + description: The interrupt signal from the DP block. + + clocks: + description: List of clock specifiers for clocks needed by thedevice.
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +10
-0+ items: + - description: Display Port AUX clock + - description: Display Port Link clock + - description: Link interface clock between DP and PHY + - description: Display Port Pixel clock + - description: Root clock generator for pixel clock + + clock-names: + description: | + Device clock names in the same order as mentioned in clocksproperty.
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +4
-0+ The required clocks are mentioned below. + items: + - const: core_aux + - const: ctrl_link
Quoted 9 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml line 51
Quoted 3 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +3
-0+ - const: ctrl_link_iface + - const: stream_pixel + - const: pixel_rcgWhy not just 'pixel'? And why is the root clk generator important? It looks like this binding should be using the assigned clock parents property instead so that it doesn't have to call
clk_set_parent()explicitly.
Are we talking about renaming stream_pixel to pixel only? We divide clocks in categories: core, control and stream clock. Similar terminology will be used in subsequent driver patches as well.
We can remove pixel_rcg use assigned clock parents property and remove clk_set_parent from driver.
Quoted 6 lines
Quoted 6 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +6
-0+ "#clock-cells": + const: 1 + + vdda-1p2-supply: + description: phandle to vdda 1.2V regulator node. +
Quoted 9 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml line 61
Quoted 4 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +4
-0+ vdda-0p9-supply: + description: phandle to vdda 0.9V regulator node. + + data-lanes = <0 1>:Is this correct? We can have = <value> in the property name? Also feels generic and possibly should come from the phy binding instead of from the controller binding.
We are using this property in DP controller programming sequence such as link training.
So I think we can keep this here.
You are right about <value>. <0 1> part should be in example only. It was passing through dt_binding_check though. Here it should be like:
data-lanes:
minItems:1
maxItems:4
Quoted 20 lines
Quoted 20 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +2
-0+ type: object + description: Maximum number of lanes that can be used for Displayport.
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +9
-0+ + ports: + description: | + Contains display port controller endpoint subnode. + remote-endpoint: | + For port@0, set to phandle of the connected panel/bridge's + input endpoint. For port@1, set to the DPU interface output. + Documentation/devicetree/bindings/graph.txt and +
Documentation/devicetree/bindings/media/video-interfaces.txt.Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +7
-0+ +patternProperties: + "^aux-cfg([0-9])-settings$": + type: object + description: | + Specifies the DP AUX configuration [0-9] settings. + The first entry in this array corresponds to the register
Quoted 9 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml line 81
Quoted 3 lines
offset
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +2
-0+ within DP AUX, while the remaining entries indicate the + programmable values.I'd prefer this was removed from the binding and hardcoded in the driver until we can understand what the values are. If they're not understandable then they most likely don't change and should be done in the driver.
Typically customers tune these values by working with vendor. So for different boards it can be different. Even though it is hard for customers to do this themselves, these are still board specific and belong to dts. As requested earlier, we have added default values already and made these properties optional but, we would like to keep it in bindings so we can have option to tune them as required.
Quoted 55 lines
Quoted 55 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +55
-0+ +required: + - compatible + - cell-index + - reg + - interrupts + - clocks + - clock-names + - vdda-1p2-supply + - vdda-0p9-supply + - data-lanes + - ports + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/clock/qcom,dispcc-sdm845.h> + #include <dt-bindings/clock/qcom,gcc-sdm845.h> + msm_dp: displayport-controller@ae90000{ + compatible = "qcom,dp-display"; + cell-index = <0>; + reg = <0 0xae90000 0 0x1400>; + reg-names = "dp_controller"; + + interrupt-parent = <&display_subsystem>; + interrupts = <12 0>; + + clocks = <&dispcc DISP_CC_MDSS_DP_AUX_CLK>, + <&dispcc DISP_CC_MDSS_DP_LINK_CLK>, + <&dispcc DISP_CC_MDSS_DP_LINK_INTF_CLK>, + <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK>, + <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK_SRC>; + clock-names = "core_aux", + "ctrl_link", + "ctrl_link_iface", "stream_pixel", + "pixel_rcg"; + #clock-cells = <1>; + + vdda-1p2-supply = <&vreg_l3c_1p2>; + vdda-0p9-supply = <&vreg_l4a_0p8>; + + data-lanes = <0 1>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dp_in: endpoint { + remote-endpoint = <&dpu_intf0_out>; + }; + }; + + port@1 {
Quoted 8 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml line 142
Quoted 6 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +6
-0+ reg = <1>; + dp_out: endpoint { + }; + }; + }; + };I believe there should be a '...' here.
I think you mean signature is missing? If not could you please explain?
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoting [email protected] (2020-06-11 13:07:09)
Quoted 15 lines · Stephen Boyd
Quoted 14 lines
Quoting Tanmay Shah (2020-06-08 20:38:18)
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
Quoted 7 lines
(quoted diff) +0
-0
a/Documentation/devicetree/bindings/display/msm/dp-sc7180.yamlb/Documentation/devicetree/bindings/display/msm/dp-sc7180.yamlnew file mode 100644
index 0000000..5fdb915Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +0
-0Typically the file name matches the compatible string. But the compatible string is just qcom,dp-display. Maybe the compatible string should be qcom,sc7180-dp? Notice that the SoC number comes first as is preferred.
Quoted 9 lines
These bindings will be similar for upcoming SOC as well. So just for understanding, when we add new SOC do we create new file with same bidings
with SOC number in new file name?
Instead we can keep this file's name as qcom,dp-display.yaml (same as compatible const) and we can include SOC number in compatible enum ? some examples:
https://patchwork.kernel.org/patch/11448357/
https://patchwork.kernel.org/patch/11164619/
Yes that works too. It's really up to robh here.
Quoted 62 lines
Quoted 53 lines
Quoted 53 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +15
-0@@ -0,0 +1,142 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/msm/dp-sc7180.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm Display Port Controller. + +maintainers: + - Chandan Uddaraju <[email protected]> + - Vara Reddy <[email protected]> + - Tanmay Shah <[email protected]> + +description: | + Device tree bindings for MSM Display Port which supports DP hostcontrollers
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +19
-0+ that are compatible with VESA Display Port interface specification. + +properties: + compatible: + items: + - const: qcom,dp-display + + cell-index: + description: Specifies the controller instance. + + reg: + items: + - description: DP controller registers + + interrupts: + description: The interrupt signal from the DP block. + + clocks: + description: List of clock specifiers for clocks needed by thedevice.
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +10
-0+ items: + - description: Display Port AUX clock + - description: Display Port Link clock + - description: Link interface clock between DP and PHY + - description: Display Port Pixel clock + - description: Root clock generator for pixel clock + + clock-names: + description: | + Device clock names in the same order as mentioned in clocksproperty.
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +4
-0+ The required clocks are mentioned below. + items: + - const: core_aux + - const: ctrl_linkQuoted 9 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml line 51
Quoted 3 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +3
-0+ - const: ctrl_link_iface + - const: stream_pixel + - const: pixel_rcgWhy not just 'pixel'? And why is the root clk generator important? It looks like this binding should be using the assigned clock parents property instead so that it doesn't have to call
clk_set_parent()explicitly.
Quoted 7 lines
Are we talking about renaming
stream_pixelto pixel only? We divide clocks in categories: core, control and stream clock. Similar terminology will be used in subsequent driver patches as well.We can remove
pixel_rcguse assigned clock parents property and removeclk_set_parent
from driver.
Cool. Using assigned clock parents is good.
Quoted 16 lines
Quoted 16 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml line 61
Quoted 11 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +10
-0+ "#clock-cells": + const: 1 + + vdda-1p2-supply: + description: phandle to vdda 1.2V regulator node. + + vdda-0p9-supply: + description: phandle to vdda 0.9V regulator node. + + data-lanes = <0 1>:Is this correct? We can have = <value> in the property name? Also feels generic and possibly should come from the phy binding instead of from the controller binding.
Quoted 9 lines
We are using this property in DP controller programming sequence such as link training.
So I think we can keep this here.
You are right about <value>. <0 1> part should be in example only. It was passing throughdt_binding_checkthough.
Here it should be like:
data-lanes:
minItems:1
maxItems:4
Ok.
Quoted 31 lines
Quoted 22 lines
Quoted 22 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +2
-0+ type: object + description: Maximum number of lanes that can be used for Displayport.
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +9
-0+ + ports: + description: | + Contains display port controller endpoint subnode. + remote-endpoint: | + For port@0, set to phandle of the connected panel/bridge's + input endpoint. For port@1, set to the DPU interface output. + Documentation/devicetree/bindings/graph.txt and +
Documentation/devicetree/bindings/media/video-interfaces.txt.Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +7
-0+ +patternProperties: + "^aux-cfg([0-9])-settings$": + type: object + description: | + Specifies the DP AUX configuration [0-9] settings. + The first entry in this array corresponds to the registeroffset
Quoted 9 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml line 81
Quoted 2 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +2
-0+ within DP AUX, while the remaining entries indicate the + programmable values.I'd prefer this was removed from the binding and hardcoded in the driver
until we can understand what the values are. If they're not understandable then they most likely don't change and should be done in the driver.
Quoted 6 lines
Typically customers tune these values by working with vendor. So for different boards it can be different. Even though it is hard for customers to do this themselves, these are still board specific and belong to dts. As requested earlier, we have added default values already and made these properties optional but, we would like to keep it in bindings so we can have option to tune them as required.
If they're in the binding then they should make sense instead of just being random values. So please move the defaults to the driver and have human understandable DT properties to tune these settings. This has been done for the qcom USB phy already (see things like qcom,hstx-trim-value for example).
Quoted 63 lines
Quoted 55 lines
Quoted 55 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +55
-0+ +required: + - compatible + - cell-index + - reg + - interrupts + - clocks + - clock-names + - vdda-1p2-supply + - vdda-0p9-supply + - data-lanes + - ports + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/clock/qcom,dispcc-sdm845.h> + #include <dt-bindings/clock/qcom,gcc-sdm845.h> + msm_dp: displayport-controller@ae90000{ + compatible = "qcom,dp-display"; + cell-index = <0>; + reg = <0 0xae90000 0 0x1400>; + reg-names = "dp_controller"; + + interrupt-parent = <&display_subsystem>; + interrupts = <12 0>; + + clocks = <&dispcc DISP_CC_MDSS_DP_AUX_CLK>, + <&dispcc DISP_CC_MDSS_DP_LINK_CLK>, + <&dispcc DISP_CC_MDSS_DP_LINK_INTF_CLK>, + <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK>, + <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK_SRC>; + clock-names = "core_aux", + "ctrl_link", + "ctrl_link_iface", "stream_pixel", + "pixel_rcg"; + #clock-cells = <1>; + + vdda-1p2-supply = <&vreg_l3c_1p2>; + vdda-0p9-supply = <&vreg_l4a_0p8>; + + data-lanes = <0 1>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dp_in: endpoint { + remote-endpoint = <&dpu_intf0_out>; + }; + }; + + port@1 {Quoted 8 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml line 142
Quoted 6 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +6
-0+ reg = <1>; + dp_out: endpoint { + }; + }; + }; + };I believe there should be a '...' here.
Quoted 1 line
I think you mean signature is missing? If not could you please explain?
No I mean there should be a triple dot at the end.
Thanks Stephen for your answers.
Quoted 166 lines · Stephen Boyd
Quoting [email protected] (2020-06-11 13:07:09)
Quoted 15 lines · Stephen Boyd
Quoted 14 lines
Quoting Tanmay Shah (2020-06-08 20:38:18)
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
Quoted 7 lines
diff --git
a/Documentation/devicetree/bindings/display/msm/dp-sc7180.yamlb/Documentation/devicetree/bindings/display/msm/dp-sc7180.yamlnew file mode 100644
index 0000000..5fdb915Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +0
-0Typically the file name matches the compatible string. But the compatible string is just qcom,dp-display. Maybe the compatible string should be qcom,sc7180-dp? Notice that the SoC number comes first as is preferred.
Quoted 9 lines
These bindings will be similar for upcoming SOC as well. So just for understanding, when we add new SOC do we create new file with same bidings
with SOC number in new file name?
Instead we can keep this file's name as qcom,dp-display.yaml (same as compatible const) and we can include SOC number in compatible enum ? some examples:
https://patchwork.kernel.org/patch/11448357/
https://patchwork.kernel.org/patch/11164619/Yes that works too. It's really up to robh here.
Quoted 62 lines
Quoted 53 lines
Quoted 53 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +15
-0@@ -0,0 +1,142 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/msm/dp-sc7180.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm Display Port Controller. + +maintainers: + - Chandan Uddaraju <[email protected]> + - Vara Reddy <[email protected]> + - Tanmay Shah <[email protected]> + +description: | + Device tree bindings for MSM Display Port which supports DP hostcontrollers
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +19
-0+ that are compatible with VESA Display Port interface specification. + +properties: + compatible: + items: + - const: qcom,dp-display + + cell-index: + description: Specifies the controller instance. + + reg: + items: + - description: DP controller registers + + interrupts: + description: The interrupt signal from the DP block. + + clocks: + description: List of clock specifiers for clocks needed by thedevice.
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +10
-0+ items: + - description: Display Port AUX clock + - description: Display Port Link clock + - description: Link interface clock between DP and PHY + - description: Display Port Pixel clock + - description: Root clock generator for pixel clock + + clock-names: + description: | + Device clock names in the same order as mentioned in clocksproperty.
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +4
-0+ The required clocks are mentioned below. + items: + - const: core_aux + - const: ctrl_linkQuoted 9 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml line 51
Quoted 3 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +3
-0+ - const: ctrl_link_iface + - const: stream_pixel + - const: pixel_rcgWhy not just 'pixel'? And why is the root clk generator important? It looks like this binding should be using the assigned clock parents property instead so that it doesn't have to call
clk_set_parent()explicitly.Quoted 7 lines
Are we talking about renaming
stream_pixelto pixel only? We divide clocks in categories: core, control and stream clock. Similar terminology will be used in subsequent driver patches as well.We can remove
pixel_rcguse assigned clock parents property and removeclk_set_parent
from driver.Cool. Using assigned clock parents is good.
Quoted 16 lines
Quoted 16 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml line 61
Quoted 11 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +10
-0+ "#clock-cells": + const: 1 + + vdda-1p2-supply: + description: phandle to vdda 1.2V regulator node. + + vdda-0p9-supply: + description: phandle to vdda 0.9V regulator node. + + data-lanes = <0 1>:Is this correct? We can have = <value> in the property name? Also feels generic and possibly should come from the phy binding instead of from the controller binding.
Quoted 9 lines
We are using this property in DP controller programming sequence such as link training.
So I think we can keep this here.
You are right about <value>. <0 1> part should be in example only. It was passing throughdt_binding_checkthough.
Here it should be like:
data-lanes:
minItems:1
maxItems:4Ok.
Quoted 31 lines
Quoted 22 lines
Quoted 22 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +2
-0+ type: object + description: Maximum number of lanes that can be used for Displayport.
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +9
-0+ + ports: + description: | + Contains display port controller endpoint subnode. + remote-endpoint: | + For port@0, set to phandle of the connected panel/bridge's + input endpoint. For port@1, set to the DPU interface output. + Documentation/devicetree/bindings/graph.txt and +
Documentation/devicetree/bindings/media/video-interfaces.txt.Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +7
-0+ +patternProperties: + "^aux-cfg([0-9])-settings$": + type: object + description: | + Specifies the DP AUX configuration [0-9] settings. + The first entry in this array corresponds to the registeroffset
Quoted 9 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml line 81
Quoted 2 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +2
-0+ within DP AUX, while the remaining entries indicate the + programmable values.I'd prefer this was removed from the binding and hardcoded in the driver
until we can understand what the values are. If they're not understandable then they most likely don't change and should be done in the driver.Quoted 6 lines
Typically customers tune these values by working with vendor. So for different boards it can be different. Even though it is hard for customers to do this themselves, these are still board specific and belong to dts. As requested earlier, we have added default values already and made these properties optional but, we would like to keep it in bindings so we can have option to tune them as required.
Quoted 6 lines
If they're in the binding then they should make sense instead of just being random values. So please move the defaults to the driver and have human understandable DT properties to tune these settings. This has been done for the qcom USB phy already (see things like qcom,hstx-trim-value for example).
Ok. For now I will move these values to driver and later we will add dt properties as required.
Quoted 63 lines
Quoted 63 lines
Quoted 63 lines
Quoted 51 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +51
-0+ +required: + - compatible + - cell-index + - reg + - interrupts + - clocks + - clock-names + - vdda-1p2-supply + - vdda-0p9-supply + - data-lanes + - ports + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/clock/qcom,dispcc-sdm845.h> + #include <dt-bindings/clock/qcom,gcc-sdm845.h> + msm_dp: displayport-controller@ae90000{ + compatible = "qcom,dp-display"; + cell-index = <0>; + reg = <0 0xae90000 0 0x1400>; + reg-names = "dp_controller"; + + interrupt-parent = <&display_subsystem>; + interrupts = <12 0>; + + clocks = <&dispcc DISP_CC_MDSS_DP_AUX_CLK>, + <&dispcc DISP_CC_MDSS_DP_LINK_CLK>, + <&dispcc DISP_CC_MDSS_DP_LINK_INTF_CLK>, + <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK>, + <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK_SRC>; + clock-names = "core_aux", + "ctrl_link", + "ctrl_link_iface", "stream_pixel", + "pixel_rcg"; + #clock-cells = <1>; + + vdda-1p2-supply = <&vreg_l3c_1p2>; + vdda-0p9-supply = <&vreg_l4a_0p8>; + + data-lanes = <0 1>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dp_in: endpoint { + remote-endpoint = <&dpu_intf0_out>;Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml line 142
Quoted 10 lines
Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml new file +10
-0+ }; + }; + + port@1 { + reg = <1>; + dp_out: endpoint { + }; + }; + }; + };I believe there should be a '...' here.
Quoted 3 lines
Quoted 1 line
I think you mean signature is missing? If not could you please explain?
No I mean there should be a triple dot at the end.
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel