public inbox for linux-arm-msm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 1/5] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon
@ 2020-06-09  3:38 Tanmay Shah
  2020-06-10  2:15 ` Stephen Boyd
  0 siblings, 1 reply; 17+ messages in thread
From: Tanmay Shah @ 2020-06-09  3:38 UTC (permalink / raw)
  To: devicetree
  Cc: sam, swboyd, seanpaul, freedreno, linux-arm-msm, chandanu,
	robdclark, abhinavk, nganji, dri-devel, linux-clk, Vara Reddy,
	Tanmay Shah

From: Chandan Uddaraju <chandanu@codeaurora.org>

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.

Signed-off-by: Chandan Uddaraju <chandanu@codeaurora.org>
Signed-off-by: Vara Reddy <varar@codeaurora.org>
Signed-off-by: Tanmay Shah <tanmay@codeaurora.org>
---
 .../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

diff --git a/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
new file mode 100644
index 0000000..5fdb915
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
@@ -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 <chandanu@codeaurora.org>
+  - Vara Reddy <varar@codeaurora.org>
+  - Tanmay Shah <tanmay@codeaurora.org>
+
+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 {
+                };
+            };
+        };
+    };
diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt
index 551ae26..30c8ab4 100644
--- a/Documentation/devicetree/bindings/display/msm/dpu.txt
+++ b/Documentation/devicetree/bindings/display/msm/dpu.txt
@@ -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>;
+					};
+				};
 			};
 		};
 	};
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 1/5] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon
  2020-06-09  3:38 [PATCH v6 1/5] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon Tanmay Shah
@ 2020-06-10  2:15 ` Stephen Boyd
  2020-06-11 20:07   ` tanmay
  0 siblings, 1 reply; 17+ messages in thread
From: Stephen Boyd @ 2020-06-10  2:15 UTC (permalink / raw)
  To: Tanmay Shah, devicetree
  Cc: sam, seanpaul, freedreno, linux-arm-msm, chandanu, robdclark,
	abhinavk, nganji, dri-devel, linux-clk, Vara Reddy, Tanmay Shah

Quoting Tanmay Shah (2020-06-08 20:38:18)
> diff --git a/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
> new file mode 100644
> index 0000000..5fdb915
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml

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.


> @@ -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 <chandanu@codeaurora.org>
> +  - Vara Reddy <varar@codeaurora.org>
> +  - Tanmay Shah <tanmay@codeaurora.org>
> +
> +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

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.

> +  "#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.

> +    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.

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.

> +
> +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 {
> +                };
> +            };
> +        };
> +    };

I believe there should be a '...' here.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 1/5] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon
  2020-06-10  2:15 ` Stephen Boyd
@ 2020-06-11 20:07   ` tanmay
  2020-06-16 10:55     ` Stephen Boyd
  0 siblings, 1 reply; 17+ messages in thread
From: tanmay @ 2020-06-11 20:07 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: devicetree, sam, seanpaul, freedreno, linux-arm-msm, chandanu,
	robdclark, abhinavk, nganji, dri-devel, linux-clk, Vara Reddy

On 2020-06-09 19:15, Stephen Boyd wrote:
> Quoting Tanmay Shah (2020-06-08 20:38:18)
>> diff --git 
>> a/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml 
>> b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
>> new file mode 100644
>> index 0000000..5fdb915
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
> 
> 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.
> 
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/
> 
>> @@ -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 <chandanu@codeaurora.org>
>> +  - Vara Reddy <varar@codeaurora.org>
>> +  - Tanmay Shah <tanmay@codeaurora.org>
>> +
>> +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
> 
> 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.
> 
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.

>> +  "#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.
> 
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

>> +    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.
> 
> 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.
>> +
>> +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 {
>> +                };
>> +            };
>> +        };
>> +    };
> 
> I believe there should be a '...' here.
I think you mean signature is missing? If not could you please explain?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v6 0/5] Add support for DisplayPort driver on
@ 2020-06-12  1:50 Tanmay Shah
  2020-06-12  1:50 ` [PATCH v6 1/5] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon Tanmay Shah
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Tanmay Shah @ 2020-06-12  1:50 UTC (permalink / raw)
  To: sam, robh+dt, swboyd
  Cc: devicetree, linux-kernel, linux-arm-msm, dri-devel, freedreno,
	seanpaul, robdclark, aravindh, abhinavk, Tanmay Shah

These patches add support for Display-Port driver on SnapDragon
hardware. It adds
DP driver and DP PLL driver files along with the needed device-tree
bindings.

The block diagram of DP driver is shown below:


                 +-------------+
                 |DRM FRAMEWORK|
                 +------+------+
                        |
                   +----v----+
                   | DP DRM  |
                   +----+----+
                        |
                   +----v----+
     +------------+|   DP    +----------++------+
     +        +---+| DISPLAY |+---+      |      |
     |        +    +-+-----+-+    |      |      |
     |        |      |     |      |      |      |
     |        |      |     |      |      |      |
     |        |      |     |      |      |      |
     v        v      v     v      v      v      v
 +------+ +------+ +---+ +----+ +----+ +---+ +-----+
 |  DP  | |  DP  | |DP | | DP | | DP | |DP | | DP  |
 |PARSER| | HPD  | |AUX| |LINK| |CTRL| |PHY| |POWER|
 +--+---+ +---+--+ +---+ +----+ +--+-+ +-+-+ +-----+
    |                              |     |
 +--v---+                         +v-----v+
 |DEVICE|                         |  DP   |
 | TREE |                         |CATALOG|
 +------+                         +---+---+
                                      |
                                  +---v----+
                                  |CTRL/PHY|
                                  |   HW   |
                                  +--------+


These patches have dependency on clock driver changes mentioned below:
https://patchwork.kernel.org/patch/11245895/
https://patchwork.kernel.org/cover/11069083/

Chandan Uddaraju (4):
  dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon
  drm: add constant N value in helper file
  drm/msm/dp: add displayPort driver support
  drm/msm/dp: add support for DP PLL driver

Jeykumar Sankaran (1):
  drm/msm/dpu: add display port support in DPU

 .../bindings/display/msm/dp-sc7180.yaml       |  142 ++
 .../devicetree/bindings/display/msm/dpu.txt   |    8 +
 drivers/gpu/drm/i915/display/intel_display.c  |    2 +-
 drivers/gpu/drm/msm/Kconfig                   |   21 +
 drivers/gpu/drm/msm/Makefile                  |   15 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   |   29 +-
 .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  |    8 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       |   65 +-
 drivers/gpu/drm/msm/dp/dp_aux.c               |  530 +++++
 drivers/gpu/drm/msm/dp/dp_aux.h               |   35 +
 drivers/gpu/drm/msm/dp/dp_catalog.c           | 1025 ++++++++++
 drivers/gpu/drm/msm/dp/dp_catalog.h           |   86 +
 drivers/gpu/drm/msm/dp/dp_ctrl.c              | 1709 +++++++++++++++++
 drivers/gpu/drm/msm/dp/dp_ctrl.h              |   35 +
 drivers/gpu/drm/msm/dp/dp_display.c           |  912 +++++++++
 drivers/gpu/drm/msm/dp/dp_display.h           |   31 +
 drivers/gpu/drm/msm/dp/dp_drm.c               |  170 ++
 drivers/gpu/drm/msm/dp/dp_drm.h               |   18 +
 drivers/gpu/drm/msm/dp/dp_hpd.c               |   69 +
 drivers/gpu/drm/msm/dp/dp_hpd.h               |   79 +
 drivers/gpu/drm/msm/dp/dp_link.c              | 1216 ++++++++++++
 drivers/gpu/drm/msm/dp/dp_link.h              |  132 ++
 drivers/gpu/drm/msm/dp/dp_panel.c             |  490 +++++
 drivers/gpu/drm/msm/dp/dp_panel.h             |   95 +
 drivers/gpu/drm/msm/dp/dp_parser.c            |  390 ++++
 drivers/gpu/drm/msm/dp/dp_parser.h            |  204 ++
 drivers/gpu/drm/msm/dp/dp_pll.c               |   93 +
 drivers/gpu/drm/msm/dp/dp_pll.h               |   59 +
 drivers/gpu/drm/msm/dp/dp_pll_10nm.c          |  903 +++++++++
 drivers/gpu/drm/msm/dp/dp_pll_private.h       |  103 +
 drivers/gpu/drm/msm/dp/dp_power.c             |  422 ++++
 drivers/gpu/drm/msm/dp/dp_power.h             |  115 ++
 drivers/gpu/drm/msm/dp/dp_reg.h               |  505 +++++
 drivers/gpu/drm/msm/msm_drv.c                 |    2 +
 drivers/gpu/drm/msm/msm_drv.h                 |   53 +-
 include/drm/drm_dp_helper.h                   |    1 +
 36 files changed, 9753 insertions(+), 19 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
 create mode 100644 drivers/gpu/drm/msm/dp/dp_aux.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_aux.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_catalog.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_catalog.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_ctrl.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_ctrl.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_display.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_display.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_drm.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_drm.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_hpd.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_hpd.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_link.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_link.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_panel.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_panel.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_parser.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_parser.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_pll.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_pll.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_pll_10nm.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_pll_private.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_power.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_power.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_reg.h


base-commit: 48f99181fc118d82dc8bf6c7221ad1c654cb8bc2
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v6 1/5] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon
  2020-06-12  1:50 [PATCH v6 0/5] Add support for DisplayPort driver on Tanmay Shah
@ 2020-06-12  1:50 ` Tanmay Shah
  2020-06-12 16:21   ` Rob Herring
  2020-06-16 11:15   ` Stephen Boyd
  2020-06-12  1:50 ` [PATCH v6 2/5] drm: add constant N value in helper file Tanmay Shah
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 17+ messages in thread
From: Tanmay Shah @ 2020-06-12  1:50 UTC (permalink / raw)
  To: sam, robh+dt, swboyd
  Cc: devicetree, linux-kernel, linux-arm-msm, dri-devel, freedreno,
	seanpaul, robdclark, aravindh, abhinavk, Chandan Uddaraju,
	Vara Reddy, Tanmay Shah

From: Chandan Uddaraju <chandanu@codeaurora.org>

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.

Signed-off-by: Chandan Uddaraju <chandanu@codeaurora.org>
Signed-off-by: Vara Reddy <varar@codeaurora.org>
Signed-off-by: Tanmay Shah <tanmay@codeaurora.org>
---
 .../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

diff --git a/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
new file mode 100644
index 000000000000..5fdb9153df00
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
@@ -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 <chandanu@codeaurora.org>
+  - Vara Reddy <varar@codeaurora.org>
+  - Tanmay Shah <tanmay@codeaurora.org>
+
+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 {
+                };
+            };
+        };
+    };
diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt
index 551ae26f60da..30c8ab479b02 100644
--- a/Documentation/devicetree/bindings/display/msm/dpu.txt
+++ b/Documentation/devicetree/bindings/display/msm/dpu.txt
@@ -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>;
+					};
+				};
 			};
 		};
 	};
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v6 2/5] drm: add constant N value in helper file
  2020-06-12  1:50 [PATCH v6 0/5] Add support for DisplayPort driver on Tanmay Shah
  2020-06-12  1:50 ` [PATCH v6 1/5] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon Tanmay Shah
@ 2020-06-12  1:50 ` Tanmay Shah
  2020-06-12  1:50 ` [PATCH v6 4/5] drm/msm/dp: add support for DP PLL driver Tanmay Shah
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 17+ messages in thread
From: Tanmay Shah @ 2020-06-12  1:50 UTC (permalink / raw)
  To: sam, robh+dt, swboyd
  Cc: devicetree, linux-kernel, linux-arm-msm, dri-devel, freedreno,
	seanpaul, robdclark, aravindh, abhinavk, Chandan Uddaraju,
	Vara Reddy, Tanmay Shah, Jani Nikula

From: Chandan Uddaraju <chandanu@codeaurora.org>

The constant N value (0x8000) is used by i915 DP
driver. Define this value in dp helper header file
to use in multiple Display Port drivers. Change
i915 driver accordingly.

Change in v6: Change commit message

Signed-off-by: Chandan Uddaraju <chandanu@codeaurora.org>
Signed-off-by: Vara Reddy <varar@codeaurora.org>
Signed-off-by: Tanmay Shah <tanmay@codeaurora.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Acked-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 2 +-
 include/drm/drm_dp_helper.h                  | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 9ea1a397d1b5..4b2cfff5b761 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8085,7 +8085,7 @@ static void compute_m_n(unsigned int m, unsigned int n,
 	 * which the devices expect also in synchronous clock mode.
 	 */
 	if (constant_n)
-		*ret_n = 0x8000;
+		*ret_n = DP_LINK_CONSTANT_N_VALUE;
 	else
 		*ret_n = min_t(unsigned int, roundup_pow_of_two(n), DATA_LINK_N_MAX);
 
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 2035ac44afde..589132ab9ab6 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1134,6 +1134,7 @@
 #define DP_MST_PHYSICAL_PORT_0 0
 #define DP_MST_LOGICAL_PORT_0 8
 
+#define DP_LINK_CONSTANT_N_VALUE 0x8000
 #define DP_LINK_STATUS_SIZE	   6
 bool drm_dp_channel_eq_ok(const u8 link_status[DP_LINK_STATUS_SIZE],
 			  int lane_count);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v6 4/5] drm/msm/dp: add support for DP PLL driver
  2020-06-12  1:50 [PATCH v6 0/5] Add support for DisplayPort driver on Tanmay Shah
  2020-06-12  1:50 ` [PATCH v6 1/5] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon Tanmay Shah
  2020-06-12  1:50 ` [PATCH v6 2/5] drm: add constant N value in helper file Tanmay Shah
@ 2020-06-12  1:50 ` Tanmay Shah
  2020-06-12  1:50 ` [PATCH v6 5/5] drm/msm/dpu: add display port support in DPU Tanmay Shah
  2020-06-12 23:26 ` [PATCH v6 0/5] Add support for DisplayPort driver on Stephen Boyd
  4 siblings, 0 replies; 17+ messages in thread
From: Tanmay Shah @ 2020-06-12  1:50 UTC (permalink / raw)
  To: sam, robh+dt, swboyd
  Cc: devicetree, linux-kernel, linux-arm-msm, dri-devel, freedreno,
	seanpaul, robdclark, aravindh, abhinavk, Chandan Uddaraju,
	Vara Reddy, Tanmay Shah

From: Chandan Uddaraju <chandanu@codeaurora.org>

Add the needed DP PLL specific files to support
display port interface on msm targets.

The DP driver calls the DP PLL driver registration.
The DP driver sets the link and pixel clock sources.

Changes in v2:
-- Update copyright markings on all relevant files.
-- Use DRM_DEBUG_DP for debug msgs.

Changes in v4:
-- Update the DP link clock provider names

Changes in V5:
-- Addressed comments from Stephen Boyd, Rob clark.

Changes in V6:
-- Remove PLL as separate driver and include PLL as DP module
-- Remove redundant clock parsing from PLL module and make DP as
   clock provider
-- Map USB3 DPCOM and PHY IO using hardcoded register address and
   move mapping form parser to PLL module
-- Access DP PHY modules from same base address using offsets instead of
   deriving base address of individual module from device tree.
-- Remove dp_pll_10nm_util.c and include its functionality in
   dp_pll_10nm.c
-- Introduce new data structures private to PLL module

Signed-off-by: Chandan Uddaraju <chandanu@codeaurora.org>
Signed-off-by: Vara Reddy <varar@codeaurora.org>
Signed-off-by: Tanmay Shah <tanmay@codeaurora.org>
---
 drivers/gpu/drm/msm/Kconfig             |  13 +
 drivers/gpu/drm/msm/Makefile            |   3 +
 drivers/gpu/drm/msm/dp/dp_catalog.c     |  64 +-
 drivers/gpu/drm/msm/dp/dp_display.c     |  17 +-
 drivers/gpu/drm/msm/dp/dp_display.h     |   3 +
 drivers/gpu/drm/msm/dp/dp_parser.c      |  51 +-
 drivers/gpu/drm/msm/dp/dp_parser.h      |   9 +-
 drivers/gpu/drm/msm/dp/dp_pll.c         |  93 +++
 drivers/gpu/drm/msm/dp/dp_pll.h         |  59 ++
 drivers/gpu/drm/msm/dp/dp_pll_10nm.c    | 903 ++++++++++++++++++++++++
 drivers/gpu/drm/msm/dp/dp_pll_private.h | 103 +++
 drivers/gpu/drm/msm/dp/dp_power.c       |  10 +
 drivers/gpu/drm/msm/dp/dp_power.h       |  71 +-
 drivers/gpu/drm/msm/dp/dp_reg.h         |  16 +
 14 files changed, 1349 insertions(+), 66 deletions(-)
 create mode 100644 drivers/gpu/drm/msm/dp/dp_pll.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_pll.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_pll_10nm.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_pll_private.h

diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index ea3c4d094d09..43544c18b4ee 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -65,6 +65,19 @@ config DRM_MSM_DP
 	  display support is enabled through this config option. It can
 	  be primary or secondary display on device.
 
+config DRM_MSM_DP_PLL
+	bool "Enable DP PLL driver in MSM DRM"
+	depends on DRM_MSM_DP && COMMON_CLK
+	help
+	  Choose this option to enable DP PLL driver which provides DP
+	  source clocks under common clock framework.
+
+config DRM_MSM_DP_10NM_PLL
+	bool "Enable DP 10nm PLL driver in MSM DRM (used by SDM845)"
+	depends on DRM_MSM_DP_PLL
+	help
+	  Choose this option if DP PLL on SDM845 is used on the platform.
+
 config DRM_MSM_DSI
 	bool "Enable DSI support in MSM DRM driver"
 	depends on DRM_MSM
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index af868e791210..af259c6a13da 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -140,4 +140,7 @@ msm-$(CONFIG_DRM_MSM_DSI_14NM_PHY) += dsi/pll/dsi_pll_14nm.o
 msm-$(CONFIG_DRM_MSM_DSI_10NM_PHY) += dsi/pll/dsi_pll_10nm.o
 endif
 
+msm-$(CONFIG_DRM_MSM_DP_PLL)+= dp/dp_pll.o
+msm-$(CONFIG_DRM_MSM_DP_10NM_PLL)+= dp/dp_pll_10nm.o
+
 obj-$(CONFIG_DRM_MSM)	+= msm.o
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c b/drivers/gpu/drm/msm/dp/dp_catalog.c
index d02f4eba5d1d..2b982f02c4cd 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -5,6 +5,7 @@
 
 #define pr_fmt(fmt)	"[drm-dp] %s: " fmt, __func__
 
+#include <linux/rational.h>
 #include <linux/delay.h>
 #include <linux/iopoll.h>
 #include <drm/drm_dp_helper.h>
@@ -134,59 +135,61 @@ static inline void dp_write_ahb(struct dp_catalog_private *catalog,
 	writel(data, catalog->io->dp_controller.base + offset);
 }
 
-static inline u32 dp_read_cc(struct dp_catalog_private *catalog, u32 offset)
-{
-	return readl_relaxed(catalog->io->dp_cc_io.base + offset);
-}
-
 static inline void dp_write_phy(struct dp_catalog_private *catalog,
 			       u32 offset, u32 data)
 {
+	offset += DP_PHY_REG_OFFSET;
 	/*
 	 * To make sure phy reg writes happens before any other operation,
 	 * this function uses writel() instread of writel_relaxed()
 	 */
-	writel(data, catalog->io->phy_io.base + offset);
+	writel(data, catalog->io->phy_reg.base + offset);
 }
 
 static inline u32 dp_read_phy(struct dp_catalog_private *catalog,
 			       u32 offset)
 {
+	offset += DP_PHY_REG_OFFSET;
 	/*
 	 * To make sure phy reg writes happens before any other operation,
 	 * this function uses writel() instread of writel_relaxed()
 	 */
-	return readl_relaxed(catalog->io->phy_io.base + offset);
+	return readl_relaxed(catalog->io->phy_reg.base + offset);
 }
 
 static inline void dp_write_pll(struct dp_catalog_private *catalog,
 			       u32 offset, u32 data)
 {
-	writel_relaxed(data, catalog->io->dp_pll_io.base + offset);
+	offset += DP_PHY_PLL_OFFSET;
+	writel_relaxed(data, catalog->io->phy_reg.base + offset);
 }
 
 static inline void dp_write_ln_tx0(struct dp_catalog_private *catalog,
 			       u32 offset, u32 data)
 {
-	writel_relaxed(data, catalog->io->ln_tx0_io.base + offset);
+	offset += DP_PHY_LN_TX0_OFFSET;
+	writel_relaxed(data, catalog->io->phy_reg.base + offset);
 }
 
 static inline void dp_write_ln_tx1(struct dp_catalog_private *catalog,
 			       u32 offset, u32 data)
 {
-	writel_relaxed(data, catalog->io->ln_tx1_io.base + offset);
+	offset += DP_PHY_LN_TX1_OFFSET;
+	writel_relaxed(data, catalog->io->phy_reg.base + offset);
 }
 
 static inline u32 dp_read_ln_tx0(struct dp_catalog_private *catalog,
 			       u32 offset)
 {
-	return readl_relaxed(catalog->io->ln_tx0_io.base + offset);
+	offset += DP_PHY_LN_TX0_OFFSET;
+	return readl_relaxed(catalog->io->phy_reg.base + offset);
 }
 
 static inline u32 dp_read_ln_tx1(struct dp_catalog_private *catalog,
 			       u32 offset)
 {
-	return readl_relaxed(catalog->io->ln_tx1_io.base + offset);
+	offset += DP_PHY_LN_TX1_OFFSET;
+	return readl_relaxed(catalog->io->phy_reg.base + offset);
 }
 
 static inline void dp_write_usb_cm(struct dp_catalog_private *catalog,
@@ -396,13 +399,16 @@ void dp_catalog_dump_regs(struct dp_catalog *dp_catalog)
 	dump_regs(catalog->io->usb3_dp_com.base, catalog->io->usb3_dp_com.len);
 
 	pr_info("LN TX0 regs\n");
-	dump_regs(catalog->io->ln_tx0_io.base, catalog->io->ln_tx0_io.len);
+	dump_regs(catalog->io->phy_reg.base + DP_PHY_LN_TX0_OFFSET,
+						DP_PHY_LN_TX0_SIZE);
 
 	pr_info("LN TX1 regs\n");
-	dump_regs(catalog->io->ln_tx1_io.base, catalog->io->ln_tx1_io.len);
+	dump_regs(catalog->io->phy_reg.base + DP_PHY_LN_TX1_OFFSET,
+						DP_PHY_LN_TX1_SIZE);
 
 	pr_info("DP PHY regs\n");
-	dump_regs(catalog->io->phy_io.base, catalog->io->phy_io.len);
+	dump_regs(catalog->io->phy_reg.base + DP_PHY_REG_OFFSET,
+						DP_PHY_REG_SIZE);
 }
 
 void dp_catalog_aux_setup(struct dp_catalog *dp_catalog,
@@ -568,17 +574,37 @@ void dp_catalog_ctrl_config_msa(struct dp_catalog *dp_catalog,
 					bool fixed_nvid)
 {
 	u32 pixel_m, pixel_n;
-	u32 mvid, nvid;
+	u32 mvid, nvid, div, pixel_div = 0, dispcc_input_rate;
 	u32 const nvid_fixed = DP_LINK_CONSTANT_N_VALUE;
 	u32 const link_rate_hbr2 = 540000;
 	u32 const link_rate_hbr3 = 810000;
+	unsigned long den, num;
 
 	struct dp_catalog_private *catalog = container_of(dp_catalog,
 				struct dp_catalog_private, dp_catalog);
 
-	pixel_m = dp_read_cc(catalog, MMSS_DP_PIXEL_M);
-	pixel_n = dp_read_cc(catalog, MMSS_DP_PIXEL_N);
-	DRM_DEBUG_DP("pixel_m=0x%x, pixel_n=0x%x\n", pixel_m, pixel_n);
+	div = dp_read_phy(catalog, REG_DP_PHY_VCO_DIV);
+	div &= 0x03;
+
+	if (div == 0)
+		pixel_div = 6;
+	else if (div == 1)
+		pixel_div = 2;
+	else if (div == 2)
+		pixel_div = 4;
+	else
+		DRM_ERROR("Invalid pixel mux divider\n");
+
+	dispcc_input_rate = (rate * 10) / pixel_div;
+
+	rational_best_approximation(dispcc_input_rate, stream_rate_khz,
+			(unsigned long)(1 << 16) - 1,
+			(unsigned long)(1 << 16) - 1, &den, &num);
+
+	den = ~(den - num);
+	den = den & 0xFFFF;
+	pixel_m = num;
+	pixel_n = den;
 
 	mvid = (pixel_m & 0xFFFF) * 5;
 	nvid = (0xFFFF & (~pixel_n)) + (pixel_m & 0xFFFF);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 58a3d86bc3f2..033d0e8246b0 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -22,6 +22,7 @@
 #include "dp_ctrl.h"
 #include "dp_display.h"
 #include "dp_drm.h"
+#include "dp_pll.h"
 
 static struct msm_dp *g_dp_display;
 #define HPD_STRING_SIZE 30
@@ -42,6 +43,7 @@ struct dp_display_private {
 
 	struct dp_usbpd   *usbpd;
 	struct dp_parser  *parser;
+	struct msm_dp_pll *pll;
 	struct dp_power   *power;
 	struct dp_catalog *catalog;
 	struct dp_aux     *aux;
@@ -223,7 +225,6 @@ static int dp_display_process_hpd_high(struct dp_display_private *dp)
 	edid = dp->panel->edid;
 
 	dp->audio_supported = drm_detect_monitor_audio(edid);
-
 	dp_panel_handle_sink_request(dp->panel);
 
 	dp->dp_display.max_pclk_khz = DP_MAX_PIXEL_CLK_KHZ;
@@ -411,6 +412,9 @@ static int dp_init_sub_modules(struct dp_display_private *dp)
 	struct dp_panel_in panel_in = {
 		.dev = dev,
 	};
+	struct dp_pll_in pll_in = {
+		.pdev = dp->pdev,
+	};
 
 	/* Callback APIs used for cable status change event */
 	cb->configure  = dp_display_usbpd_configure_cb;
@@ -441,6 +445,17 @@ static int dp_init_sub_modules(struct dp_display_private *dp)
 		goto error;
 	}
 
+	pll_in.parser = dp->parser;
+	dp->pll = dp_pll_get(&pll_in);
+	if (IS_ERR_OR_NULL(dp->pll)) {
+		rc = -EINVAL;
+		DRM_ERROR("failed to initialize pll, rc = %d\n", rc);
+		dp->pll = NULL;
+		goto error;
+	}
+
+	dp->parser->pll = dp->pll;
+
 	dp->power = dp_power_get(dp->parser);
 	if (IS_ERR(dp->power)) {
 		rc = PTR_ERR(dp->power);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h
index dad8610685a6..4c53ed55d1cc 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -25,4 +25,7 @@ int dp_display_request_irq(struct msm_dp *dp_display);
 bool dp_display_check_video_test(struct msm_dp *dp_display);
 int dp_display_get_test_bpp(struct msm_dp *dp_display);
 
+void __init msm_dp_pll_driver_register(void);
+void __exit msm_dp_pll_driver_unregister(void);
+
 #endif /* _DP_DISPLAY_H_ */
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
index e1f93f5e69ca..24e80bdfecef 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -6,6 +6,7 @@
 #include <linux/of_gpio.h>
 
 #include "dp_parser.h"
+#include "dp_reg.h"
 
 const u8 aux_cfg0_settings[] = {0x20,  0x00};
 const u8 aux_cfg1_settings[] = {0x24, 0x13, 0x23, 0x1d};
@@ -89,11 +90,7 @@ static void dp_parser_unmap_io_resources(struct dp_parser *parser)
 	struct dp_io *io = &parser->io;
 
 	msm_dss_iounmap(&io->dp_controller);
-	msm_dss_iounmap(&io->phy_io);
-	msm_dss_iounmap(&io->ln_tx0_io);
-	msm_dss_iounmap(&io->ln_tx0_io);
-	msm_dss_iounmap(&io->dp_pll_io);
-	msm_dss_iounmap(&io->dp_cc_io);
+	msm_dss_iounmap(&io->phy_reg);
 	msm_dss_iounmap(&io->usb3_dp_com);
 }
 
@@ -117,30 +114,6 @@ static int dp_parser_ctrl_res(struct dp_parser *parser)
 		goto err;
 	}
 
-	rc = msm_dss_ioremap_byname(pdev, &io->phy_io, "dp_phy");
-	if (rc) {
-		DRM_ERROR("unable to remap dp PHY resources, rc=%d\n", rc);
-		goto err;
-	}
-
-	rc = msm_dss_ioremap_byname(pdev, &io->ln_tx0_io, "dp_ln_tx0");
-	if (rc) {
-		DRM_ERROR("unable to remap dp TX0 resources, rc=%d\n", rc);
-		goto err;
-	}
-
-	rc = msm_dss_ioremap_byname(pdev, &io->ln_tx1_io, "dp_ln_tx1");
-	if (rc) {
-		DRM_ERROR("unable to remap dp TX1 resources, rc=%d\n", rc);
-		goto err;
-	}
-
-	rc = msm_dss_ioremap_byname(pdev, &io->dp_pll_io, "dp_pll");
-	if (rc) {
-		DRM_ERROR("unable to remap DP PLL resources, rc=%d\n", rc);
-		goto err;
-	}
-
 	return 0;
 err:
 	dp_parser_unmap_io_resources(parser);
@@ -151,25 +124,25 @@ static const char *dp_get_phy_aux_config_property(u32 cfg_type)
 {
 	switch (cfg_type) {
 	case PHY_AUX_CFG0:
-		return "qcom,aux-cfg0-settings";
+		return "aux-cfg0-settings";
 	case PHY_AUX_CFG1:
-		return "qcom,aux-cfg1-settings";
+		return "aux-cfg1-settings";
 	case PHY_AUX_CFG2:
-		return "qcom,aux-cfg2-settings";
+		return "aux-cfg2-settings";
 	case PHY_AUX_CFG3:
-		return "qcom,aux-cfg3-settings";
+		return "aux-cfg3-settings";
 	case PHY_AUX_CFG4:
-		return "qcom,aux-cfg4-settings";
+		return "aux-cfg4-settings";
 	case PHY_AUX_CFG5:
-		return "qcom,aux-cfg5-settings";
+		return "aux-cfg5-settings";
 	case PHY_AUX_CFG6:
-		return "qcom,aux-cfg6-settings";
+		return "aux-cfg6-settings";
 	case PHY_AUX_CFG7:
-		return "qcom,aux-cfg7-settings";
+		return "aux-cfg7-settings";
 	case PHY_AUX_CFG8:
-		return "qcom,aux-cfg8-settings";
+		return "aux-cfg8-settings";
 	case PHY_AUX_CFG9:
-		return "qcom,aux-cfg9-settings";
+		return "aux-cfg9-settings";
 	default:
 		return "unknown";
 	}
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h
index b99677f32469..dc24acf8bb86 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.h
+++ b/drivers/gpu/drm/msm/dp/dp_parser.h
@@ -12,6 +12,7 @@
 
 #include "dpu_io_util.h"
 #include "msm_drv.h"
+#include "dp_pll.h"
 
 #define DP_LABEL "MDSS DP DISPLAY"
 #define AUX_CFG_LEN	10
@@ -64,17 +65,12 @@ struct dp_display_data {
  * @phy_io: phy's mapped memory address
  * @ln_tx0_io: USB-DP lane TX0's mapped memory address
  * @ln_tx1_io: USB-DP lane TX1's mapped memory address
- * @dp_cc_io: DP cc's mapped memory address
  * @dp_pll_io: DP PLL mapped memory address
  * @usb3_dp_com: USB3 DP PHY combo mapped memory address
  */
 struct dp_io {
 	struct dss_io_data dp_controller;
-	struct dss_io_data phy_io;
-	struct dss_io_data ln_tx0_io;
-	struct dss_io_data ln_tx1_io;
-	struct dss_io_data dp_cc_io;
-	struct dss_io_data dp_pll_io;
+	struct dss_io_data phy_reg;
 	struct dss_io_data usb3_dp_com;
 };
 
@@ -183,6 +179,7 @@ struct dp_parser {
 	struct dp_pinctrl pinctrl;
 	struct dp_io io;
 	struct dp_display_data disp_data;
+	struct msm_dp_pll *pll;
 	const struct dp_regulator_cfg *regulator_cfg;
 	u8 l_map[4];
 	struct dp_aux_cfg aux_cfg[AUX_CFG_LEN];
diff --git a/drivers/gpu/drm/msm/dp/dp_pll.c b/drivers/gpu/drm/msm/dp/dp_pll.c
new file mode 100644
index 000000000000..7cd43edaa2cb
--- /dev/null
+++ b/drivers/gpu/drm/msm/dp/dp_pll.c
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2016-2020, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/device.h>
+
+#include "dp_parser.h"
+#include "dp_pll.h"
+#include "dp_pll_private.h"
+
+static int dp_pll_get_phy_io(struct dp_parser *parser)
+{
+	struct dp_io *io = &parser->io;
+
+	io->usb3_dp_com.base = ioremap(REG_USB3_DP_COM_REGION_BASE,
+					REG_USB3_DP_COM_REGION_SIZE);
+	if (!io->usb3_dp_com.base) {
+		DRM_ERROR("unable to map USB3 DP COM IO\n");
+		return -EIO;
+	}
+
+	/* ToDo(user): DP PLL and DP PHY will not be part of
+	 * DP driver eventually so for now Hardcode Base and offsets
+	 * of PHY registers so we can remove them from dts and bindings
+	 */
+	io->phy_reg.base = ioremap(REG_DP_PHY_REGION_BASE,
+					REG_DP_PHY_REGION_SIZE);
+	if (!io->phy_reg.base) {
+		DRM_ERROR("DP PHY io region mapping failed\n");
+		return -EIO;
+	}
+	io->phy_reg.len = REG_DP_PHY_REGION_SIZE;
+
+	return 0;
+}
+
+static int msm_dp_pll_init(struct msm_dp_pll *pll,
+			enum msm_dp_pll_type type, int id)
+{
+	struct device *dev = &pll->pdev->dev;
+	int ret = 0;
+
+	switch (type) {
+	case MSM_DP_PLL_10NM:
+		ret = msm_dp_pll_10nm_init(pll, id);
+		break;
+	default:
+		DRM_DEV_ERROR(dev, "%s: Wrong PLL type %d\n", __func__, type);
+		return -ENXIO;
+	}
+
+	if (ret) {
+		DRM_DEV_ERROR(dev, "%s: failed to init DP PLL\n", __func__);
+		return ret;
+	}
+
+	pll->type = type;
+
+	DRM_DEBUG_DP("DP:%d PLL registered", id);
+
+	return ret;
+}
+
+struct msm_dp_pll *dp_pll_get(struct dp_pll_in *pll_in)
+{
+	struct msm_dp_pll *dp_pll;
+	struct dp_parser *parser = pll_in->parser;
+	struct dp_io_pll *pll_io;
+	int ret;
+
+	dp_pll = devm_kzalloc(&pll_in->pdev->dev, sizeof(*dp_pll), GFP_KERNEL);
+	if (!dp_pll)
+		return ERR_PTR(-ENOMEM);
+
+	pll_io = &dp_pll->pll_io;
+	dp_pll->pdev = pll_in->pdev;
+
+	dp_pll_get_phy_io(parser);
+
+	pll_io->pll_base = parser->io.phy_reg.base + DP_PHY_PLL_OFFSET;
+	pll_io->phy_base = parser->io.phy_reg.base + DP_PHY_REG_OFFSET;
+	pll_io->ln_tx0_base = parser->io.phy_reg.base + DP_PHY_LN_TX0_OFFSET;
+	pll_io->ln_tx1_base = parser->io.phy_reg.base + DP_PHY_LN_TX1_OFFSET;
+
+	ret = msm_dp_pll_init(dp_pll, MSM_DP_PLL_10NM, 0);
+	if (ret) {
+		kfree(dp_pll);
+		return ERR_PTR(ret);
+	}
+
+	return dp_pll;
+}
diff --git a/drivers/gpu/drm/msm/dp/dp_pll.h b/drivers/gpu/drm/msm/dp/dp_pll.h
new file mode 100644
index 000000000000..ff38fb09aea5
--- /dev/null
+++ b/drivers/gpu/drm/msm/dp/dp_pll.h
@@ -0,0 +1,59 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2016-2020, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef __DP_PLL_H
+#define __DP_PLL_H
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/platform_device.h>
+
+#include "dpu_io_util.h"
+#include "msm_drv.h"
+#include "dp_parser.h"
+
+#define PLL_REG_W(base, offset, data)	\
+				writel((data), (base) + (offset))
+#define PLL_REG_R(base, offset)	readl((base) + (offset))
+
+enum msm_dp_pll_type {
+	MSM_DP_PLL_10NM,
+	MSM_DP_PLL_MAX
+};
+
+struct dp_pll_in {
+	struct platform_device *pdev;
+	struct dp_parser *parser;
+};
+
+struct dp_io_pll {
+	void __iomem *pll_base;
+	void __iomem *phy_base;
+	void __iomem *ln_tx0_base;
+	void __iomem *ln_tx1_base;
+};
+
+struct msm_dp_pll {
+	enum msm_dp_pll_type type;
+	bool		pll_on;
+
+	struct dp_io_pll pll_io;
+
+	/* clock-provider: */
+	struct clk_hw_onecell_data *hw_data;
+
+	struct platform_device *pdev;
+	void *priv;
+
+	/* Pll specific resources like GPIO, power supply, clocks, etc*/
+	struct dss_module_power mp;
+	int (*get_provider)(struct msm_dp_pll *pll,
+			struct clk **link_clk_provider,
+			struct clk **pixel_clk_provider);
+};
+
+struct msm_dp_pll *dp_pll_get(struct dp_pll_in *pll_in);
+
+#endif /* __DP_PLL_H */
diff --git a/drivers/gpu/drm/msm/dp/dp_pll_10nm.c b/drivers/gpu/drm/msm/dp/dp_pll_10nm.c
new file mode 100644
index 000000000000..998d659bb34c
--- /dev/null
+++ b/drivers/gpu/drm/msm/dp/dp_pll_10nm.c
@@ -0,0 +1,903 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2016-2020, The Linux Foundation. All rights reserved.
+ */
+
+/*
+ * Display Port PLL driver block diagram for branch clocks
+ *
+ *              +------------------------------+
+ *              |         DP_VCO_CLK           |
+ *              |                              |
+ *              |    +-------------------+     |
+ *              |    |   (DP PLL/VCO)    |     |
+ *              |    +---------+---------+     |
+ *              |              v               |
+ *              |   +----------+-----------+   |
+ *              |   | hsclk_divsel_clk_src |   |
+ *              |   +----------+-----------+   |
+ *              +------------------------------+
+ *                              |
+ *          +---------<---------v------------>----------+
+ *          |                                           |
+ * +--------v---------+                                 |
+ * |    dp_phy_pll    |                                 |
+ * |     link_clk     |                                 |
+ * +--------+---------+                                 |
+ *          |                                           |
+ *          |                                           |
+ *          v                                           v
+ * Input to DISPCC block                                |
+ * for link clk, crypto clk                             |
+ * and interface clock                                  |
+ *                                                      |
+ *                                                      |
+ *      +--------<------------+-----------------+---<---+
+ *      |                     |                 |
+ * +----v---------+  +--------v-----+  +--------v------+
+ * | vco_divided  |  | vco_divided  |  | vco_divided   |
+ * |    _clk_src  |  |    _clk_src  |  |    _clk_src   |
+ * |              |  |              |  |               |
+ * |divsel_six    |  |  divsel_two  |  |  divsel_four  |
+ * +-------+------+  +-----+--------+  +--------+------+
+ *         |                 |                  |
+ *         v---->----------v-------------<------v
+ *                         |
+ *              +----------+---------+
+ *              |   dp_phy_pll_vco   |
+ *              |       div_clk      |
+ *              +---------+----------+
+ *                        |
+ *                        v
+ *              Input to DISPCC block
+ *              for DP pixel clock
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/regmap.h>
+#include <linux/iopoll.h>
+
+#include "dp_hpd.h"
+#include "dp_pll.h"
+#include "dp_pll_private.h"
+
+#define NUM_PROVIDED_CLKS		2
+
+#define DP_LINK_CLK_SRC			0
+#define DP_PIXEL_CLK_SRC		1
+
+static struct dp_pll_db *dp_pdb;
+
+static const struct clk_ops dp_10nm_vco_clk_ops = {
+	.recalc_rate = dp_vco_recalc_rate_10nm,
+	.set_rate = dp_vco_set_rate_10nm,
+	.round_rate = dp_vco_round_rate_10nm,
+	.prepare = dp_vco_prepare_10nm,
+	.unprepare = dp_vco_unprepare_10nm,
+};
+
+struct dp_pll_10nm_pclksel {
+	struct clk_hw hw;
+
+	/* divider params */
+	u8 shift;
+	u8 width;
+	u8 flags; /* same flags as used by clk_divider struct */
+
+	struct dp_pll_db *pll;
+};
+
+#define to_pll_10nm_pclksel(_hw) \
+	container_of(_hw, struct dp_pll_10nm_pclksel, hw)
+
+static const struct clk_parent_data disp_cc_parent_data_0[] = {
+	{ .fw_name = "bi_tcxo" },
+	{ .fw_name = "dp_phy_pll_link_clk", .name = "dp_phy_pll_link_clk" },
+	{ .fw_name = "dp_phy_pll_vco_div_clk",
+				.name = "dp_phy_pll_vco_div_clk"},
+	{ .fw_name = "core_bi_pll_test_se", .name = "core_bi_pll_test_se" },
+};
+
+static struct dp_pll_vco_clk dp_vco_clk = {
+	.min_rate = DP_VCO_HSCLK_RATE_1620MHZDIV1000,
+	.max_rate = DP_VCO_HSCLK_RATE_8100MHZDIV1000,
+};
+
+static int dp_pll_mux_set_parent_10nm(struct clk_hw *hw, u8 val)
+{
+	struct dp_pll_10nm_pclksel *pclksel = to_pll_10nm_pclksel(hw);
+	struct dp_pll_db *dp_res = pclksel->pll;
+	struct dp_io_pll *pll_io = &dp_res->base->pll_io;
+	u32 auxclk_div;
+
+	auxclk_div = PLL_REG_R(pll_io->phy_base, REG_DP_PHY_VCO_DIV);
+	auxclk_div &= ~0x03;
+
+	if (val == 0)
+		auxclk_div |= 1;
+	else if (val == 1)
+		auxclk_div |= 2;
+	else if (val == 2)
+		auxclk_div |= 0;
+
+	PLL_REG_W(pll_io->phy_base,
+			REG_DP_PHY_VCO_DIV, auxclk_div);
+	DRM_DEBUG_DP("%s: mux=%d auxclk_div=%x\n", __func__, val, auxclk_div);
+
+	return 0;
+}
+
+static u8 dp_pll_mux_get_parent_10nm(struct clk_hw *hw)
+{
+	u32 auxclk_div = 0;
+	struct dp_pll_10nm_pclksel *pclksel = to_pll_10nm_pclksel(hw);
+	struct dp_pll_db *dp_res = pclksel->pll;
+	struct dp_io_pll *pll_io = &dp_res->base->pll_io;
+	u8 val = 0;
+
+	auxclk_div = PLL_REG_R(pll_io->phy_base, REG_DP_PHY_VCO_DIV);
+	auxclk_div &= 0x03;
+
+	if (auxclk_div == 1) /* Default divider */
+		val = 0;
+	else if (auxclk_div == 2)
+		val = 1;
+	else if (auxclk_div == 0)
+		val = 2;
+
+	DRM_DEBUG_DP("%s: auxclk_div=%d, val=%d\n", __func__, auxclk_div, val);
+
+	return val;
+}
+
+static int dp_pll_clk_mux_determine_rate(struct clk_hw *hw,
+				     struct clk_rate_request *req)
+{
+	unsigned long rate = 0;
+	int ret = 0;
+
+	rate = clk_get_rate(hw->clk);
+
+	if (rate <= 0) {
+		DRM_ERROR("Rate is not set properly\n");
+		return -EINVAL;
+	}
+
+	req->rate = rate;
+
+	DRM_DEBUG_DP("%s: rate=%ld\n", __func__, req->rate);
+	/* Set the new parent of mux if there is a new valid parent */
+	if (hw->clk && req->best_parent_hw->clk) {
+		ret = clk_set_parent(hw->clk, req->best_parent_hw->clk);
+		if (ret) {
+			DRM_ERROR("%s: clk_set_parent failed: ret=%d\n",
+					__func__, ret);
+			return ret;
+		}
+	}
+	return 0;
+}
+
+static unsigned long dp_pll_mux_recalc_rate(struct clk_hw *hw,
+					unsigned long parent_rate)
+{
+	struct clk_hw *div_clk_hw = NULL, *vco_clk_hw = NULL;
+	struct dp_pll_vco_clk *vco;
+
+	div_clk_hw = clk_hw_get_parent(hw);
+	if (!div_clk_hw)
+		return 0;
+
+	vco_clk_hw = clk_hw_get_parent(div_clk_hw);
+	if (!vco_clk_hw)
+		return 0;
+
+	vco = to_dp_vco_hw(vco_clk_hw);
+	if (!vco)
+		return 0;
+
+	if (vco->rate == DP_VCO_HSCLK_RATE_8100MHZDIV1000)
+		return (vco->rate / 6);
+	else if (vco->rate == DP_VCO_HSCLK_RATE_5400MHZDIV1000)
+		return (vco->rate / 4);
+	else
+		return (vco->rate / 2);
+}
+
+static int dp_pll_10nm_get_provider(struct msm_dp_pll *pll,
+				     struct clk **link_clk_provider,
+				     struct clk **pixel_clk_provider)
+{
+	struct clk_hw_onecell_data *hw_data = pll->hw_data;
+
+	if (link_clk_provider)
+		*link_clk_provider = hw_data->hws[DP_LINK_CLK_SRC]->clk;
+	if (pixel_clk_provider)
+		*pixel_clk_provider = hw_data->hws[DP_PIXEL_CLK_SRC]->clk;
+
+	return 0;
+}
+
+static const struct clk_ops dp_10nm_pclksel_clk_ops = {
+	.get_parent = dp_pll_mux_get_parent_10nm,
+	.set_parent = dp_pll_mux_set_parent_10nm,
+	.recalc_rate = dp_pll_mux_recalc_rate,
+	.determine_rate = dp_pll_clk_mux_determine_rate,
+};
+
+static struct clk_hw *dp_pll_10nm_pixel_clk_sel(struct dp_pll_db *pll_10nm)
+{
+	struct device *dev = &pll_10nm->pdev->dev;
+	struct dp_pll_10nm_pclksel *pll_pclksel;
+	struct clk_init_data pclksel_init = {
+		.parent_data = disp_cc_parent_data_0,
+		.num_parents = 3,
+		.name = "dp_phy_pll_vco_div_clk",
+		.ops = &dp_10nm_pclksel_clk_ops,
+	};
+	int ret;
+
+	pll_pclksel = devm_kzalloc(dev, sizeof(*pll_pclksel), GFP_KERNEL);
+	if (!pll_pclksel)
+		return ERR_PTR(-ENOMEM);
+
+	pll_pclksel->pll = pll_10nm;
+	pll_pclksel->shift = 0;
+	pll_pclksel->width = 4;
+	pll_pclksel->flags = CLK_DIVIDER_ONE_BASED;
+	pll_pclksel->hw.init = &pclksel_init;
+
+	ret = clk_hw_register(dev, &pll_pclksel->hw);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return &pll_pclksel->hw;
+}
+
+static int dp_pll_10nm_register(struct dp_pll_db *pll_10nm)
+{
+	struct clk_hw_onecell_data *hw_data;
+	int ret;
+	struct clk_hw *hw;
+
+	struct msm_dp_pll *pll = pll_10nm->base;
+	struct device *dev = &pll_10nm->pdev->dev;
+	struct clk_hw **hws = pll_10nm->hws;
+	int num = 0;
+	struct clk_init_data vco_init = {
+		.parent_data = &(const struct clk_parent_data){
+				.fw_name = "bi_tcxo",
+		},
+		.num_parents = 1,
+		.name = "dp_vco_clk",
+		.ops = &dp_10nm_vco_clk_ops,
+	};
+
+	DRM_DEBUG_DP("DP->id = %d", pll_10nm->id);
+
+	hw_data = devm_kzalloc(dev, sizeof(*hw_data) +
+			       NUM_PROVIDED_CLKS * sizeof(struct clk_hw *),
+			       GFP_KERNEL);
+	if (!hw_data)
+		return -ENOMEM;
+
+	dp_vco_clk.hw.init = &vco_init;
+	ret = clk_hw_register(dev, &dp_vco_clk.hw);
+	if (ret)
+		return ret;
+	hws[num++] = &dp_vco_clk.hw;
+
+	hw = clk_hw_register_fixed_factor(dev, "dp_phy_pll_link_clk",
+				"dp_vco_clk", CLK_SET_RATE_PARENT, 1, 10);
+
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	hws[num++] = hw;
+	hw_data->hws[DP_LINK_CLK_SRC] = hw;
+
+	hw = clk_hw_register_fixed_factor(dev, "dp_vco_divsel_two_clk_src",
+					"dp_vco_clk",  0, 1, 2);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	hws[num++] = hw;
+
+	hw = clk_hw_register_fixed_factor(dev, "dp_vco_divsel_four_clk_src",
+					 "dp_vco_clk", 0, 1, 4);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	hws[num++] = hw;
+
+	hw = clk_hw_register_fixed_factor(dev, "dp_vco_divsel_six_clk_src",
+					 "dp_vco_clk", 0, 1, 6);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	hws[num++] = hw;
+
+	hw = dp_pll_10nm_pixel_clk_sel(pll_10nm);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+
+	hws[num++] = hw;
+	hw_data->hws[DP_PIXEL_CLK_SRC] = hw;
+
+	pll_10nm->num_hws = num;
+
+	hw_data->num = NUM_PROVIDED_CLKS;
+	pll->hw_data = hw_data;
+
+	ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,
+				     pll->hw_data);
+	if (ret) {
+		DRM_DEV_ERROR(dev, "failed to register clk provider: %d\n",
+				ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+int msm_dp_pll_10nm_init(struct msm_dp_pll *pll, int id)
+{
+	struct dp_pll_db *dp_10nm_pll;
+	struct platform_device *pdev = pll->pdev;
+	int ret;
+
+	dp_10nm_pll = devm_kzalloc(&pdev->dev,
+					sizeof(*dp_10nm_pll), GFP_KERNEL);
+	if (!dp_10nm_pll)
+		return -ENOMEM;
+
+	DRM_DEBUG_DP("DP PLL%d", id);
+
+	dp_10nm_pll->base = pll;
+	dp_10nm_pll->pdev = pll->pdev;
+	dp_10nm_pll->id = id;
+	dp_pdb = dp_10nm_pll;
+	pll->priv = (void *)dp_10nm_pll;
+	dp_vco_clk.priv = pll;
+
+	ret = of_property_read_u32(pdev->dev.of_node, "cell-index",
+				&dp_10nm_pll->index);
+	if (ret) {
+		DRM_ERROR("Unable to get the cell-index ret=%d\n", ret);
+		dp_10nm_pll->index = 0;
+	}
+
+	ret = dp_pll_10nm_register(dp_10nm_pll);
+	if (ret) {
+		DRM_DEV_ERROR(&pdev->dev, "failed to register PLL: %d\n", ret);
+		return ret;
+	}
+
+	pll->get_provider = dp_pll_10nm_get_provider;
+
+	return ret;
+}
+
+static int dp_vco_pll_init_db_10nm(struct msm_dp_pll *pll,
+		unsigned long rate)
+{
+	u32 spare_value = 0;
+	struct dp_io_pll *pll_io;
+	struct dp_pll_db *dp_res = to_dp_pll_db(pll);
+
+	pll_io = &pll->pll_io;
+	spare_value = PLL_REG_R(pll_io->phy_base, REG_DP_PHY_SPARE0);
+	dp_res->lane_cnt = spare_value & 0x0F;
+	dp_res->orientation = (spare_value & 0xF0) >> 4;
+
+	DRM_DEBUG_DP("%s: spare_value=0x%x, ln_cnt=0x%x, orientation=0x%x\n",
+			__func__, spare_value, dp_res->lane_cnt,
+			dp_res->orientation);
+
+	switch (rate) {
+	case DP_VCO_HSCLK_RATE_1620MHZDIV1000:
+		DRM_DEBUG_DP("%s: VCO rate: %ld\n", __func__,
+				DP_VCO_RATE_9720MHZDIV1000);
+		dp_res->hsclk_sel = 0x0c;
+		dp_res->dec_start_mode0 = 0x69;
+		dp_res->div_frac_start1_mode0 = 0x00;
+		dp_res->div_frac_start2_mode0 = 0x80;
+		dp_res->div_frac_start3_mode0 = 0x07;
+		dp_res->integloop_gain0_mode0 = 0x3f;
+		dp_res->integloop_gain1_mode0 = 0x00;
+		dp_res->vco_tune_map = 0x00;
+		dp_res->lock_cmp1_mode0 = 0x6f;
+		dp_res->lock_cmp2_mode0 = 0x08;
+		dp_res->lock_cmp3_mode0 = 0x00;
+		dp_res->phy_vco_div = 0x1;
+		dp_res->lock_cmp_en = 0x00;
+		break;
+	case DP_VCO_HSCLK_RATE_2700MHZDIV1000:
+		DRM_DEBUG_DP("%s: VCO rate: %ld\n", __func__,
+				DP_VCO_RATE_10800MHZDIV1000);
+		dp_res->hsclk_sel = 0x04;
+		dp_res->dec_start_mode0 = 0x69;
+		dp_res->div_frac_start1_mode0 = 0x00;
+		dp_res->div_frac_start2_mode0 = 0x80;
+		dp_res->div_frac_start3_mode0 = 0x07;
+		dp_res->integloop_gain0_mode0 = 0x3f;
+		dp_res->integloop_gain1_mode0 = 0x00;
+		dp_res->vco_tune_map = 0x00;
+		dp_res->lock_cmp1_mode0 = 0x0f;
+		dp_res->lock_cmp2_mode0 = 0x0e;
+		dp_res->lock_cmp3_mode0 = 0x00;
+		dp_res->phy_vco_div = 0x1;
+		dp_res->lock_cmp_en = 0x00;
+		break;
+	case DP_VCO_HSCLK_RATE_5400MHZDIV1000:
+		DRM_DEBUG_DP("%s: VCO rate: %ld\n", __func__,
+				DP_VCO_RATE_10800MHZDIV1000);
+		dp_res->hsclk_sel = 0x00;
+		dp_res->dec_start_mode0 = 0x8c;
+		dp_res->div_frac_start1_mode0 = 0x00;
+		dp_res->div_frac_start2_mode0 = 0x00;
+		dp_res->div_frac_start3_mode0 = 0x0a;
+		dp_res->integloop_gain0_mode0 = 0x3f;
+		dp_res->integloop_gain1_mode0 = 0x00;
+		dp_res->vco_tune_map = 0x00;
+		dp_res->lock_cmp1_mode0 = 0x1f;
+		dp_res->lock_cmp2_mode0 = 0x1c;
+		dp_res->lock_cmp3_mode0 = 0x00;
+		dp_res->phy_vco_div = 0x2;
+		dp_res->lock_cmp_en = 0x00;
+		break;
+	case DP_VCO_HSCLK_RATE_8100MHZDIV1000:
+		DRM_DEBUG_DP("%s: VCO rate: %ld\n", __func__,
+				DP_VCO_RATE_8100MHZDIV1000);
+		dp_res->hsclk_sel = 0x03;
+		dp_res->dec_start_mode0 = 0x69;
+		dp_res->div_frac_start1_mode0 = 0x00;
+		dp_res->div_frac_start2_mode0 = 0x80;
+		dp_res->div_frac_start3_mode0 = 0x07;
+		dp_res->integloop_gain0_mode0 = 0x3f;
+		dp_res->integloop_gain1_mode0 = 0x00;
+		dp_res->vco_tune_map = 0x00;
+		dp_res->lock_cmp1_mode0 = 0x2f;
+		dp_res->lock_cmp2_mode0 = 0x2a;
+		dp_res->lock_cmp3_mode0 = 0x00;
+		dp_res->phy_vco_div = 0x0;
+		dp_res->lock_cmp_en = 0x08;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int dp_config_vco_rate_10nm(struct dp_pll_vco_clk *vco,
+		unsigned long rate)
+{
+	u32 res = 0;
+	struct msm_dp_pll *pll = vco->priv;
+	struct dp_io_pll *pll_io = &pll->pll_io;
+	struct dp_pll_db *dp_res = to_dp_pll_db(pll);
+
+	res = dp_vco_pll_init_db_10nm(pll, rate);
+	if (res) {
+		DRM_ERROR("VCO Init DB failed\n");
+		return res;
+	}
+
+	if (dp_res->lane_cnt != 4) {
+		if (dp_res->orientation == ORIENTATION_CC2)
+			PLL_REG_W(pll_io->phy_base, REG_DP_PHY_PD_CTL, 0x6d);
+		else
+			PLL_REG_W(pll_io->phy_base, REG_DP_PHY_PD_CTL, 0x75);
+	} else {
+		PLL_REG_W(pll_io->phy_base, REG_DP_PHY_PD_CTL, 0x7d);
+	}
+
+	PLL_REG_W(pll_io->pll_base, QSERDES_COM_SVS_MODE_CLK_SEL, 0x01);
+	PLL_REG_W(pll_io->pll_base, QSERDES_COM_SYSCLK_EN_SEL, 0x37);
+	PLL_REG_W(pll_io->pll_base, QSERDES_COM_SYS_CLK_CTRL, 0x02);
+	PLL_REG_W(pll_io->pll_base, QSERDES_COM_CLK_ENABLE1, 0x0e);
+	PLL_REG_W(pll_io->pll_base, QSERDES_COM_SYSCLK_BUF_ENABLE, 0x06);
+	PLL_REG_W(pll_io->pll_base, QSERDES_COM_CLK_SEL, 0x30);
+	PLL_REG_W(pll_io->pll_base, QSERDES_COM_CMN_CONFIG, 0x02);
+
+	/* Different for each clock rates */
+	PLL_REG_W(pll_io->pll_base,
+		QSERDES_COM_HSCLK_SEL, dp_res->hsclk_sel);
+	PLL_REG_W(pll_io->pll_base,
+		QSERDES_COM_DEC_START_MODE0, dp_res->dec_start_mode0);
+	PLL_REG_W(pll_io->pll_base,
+		QSERDES_COM_DIV_FRAC_START1_MODE0,
+		dp_res->div_frac_start1_mode0);
+	PLL_REG_W(pll_io->pll_base,
+		QSERDES_COM_DIV_FRAC_START2_MODE0,
+		dp_res->div_frac_start2_mode0);
+	PLL_REG_W(pll_io->pll_base,
+		QSERDES_COM_DIV_FRAC_START3_MODE0,
+		dp_res->div_frac_start3_mode0);
+	PLL_REG_W(pll_io->pll_base,
+		QSERDES_COM_INTEGLOOP_GAIN0_MODE0,
+		dp_res->integloop_gain0_mode0);
+	PLL_REG_W(pll_io->pll_base,
+		QSERDES_COM_INTEGLOOP_GAIN1_MODE0,
+		dp_res->integloop_gain1_mode0);
+	PLL_REG_W(pll_io->pll_base,
+		QSERDES_COM_VCO_TUNE_MAP, dp_res->vco_tune_map);
+	PLL_REG_W(pll_io->pll_base,
+		QSERDES_COM_LOCK_CMP1_MODE0, dp_res->lock_cmp1_mode0);
+	PLL_REG_W(pll_io->pll_base,
+		QSERDES_COM_LOCK_CMP2_MODE0, dp_res->lock_cmp2_mode0);
+	PLL_REG_W(pll_io->pll_base,
+		QSERDES_COM_LOCK_CMP3_MODE0, dp_res->lock_cmp3_mode0);
+
+	PLL_REG_W(pll_io->pll_base, QSERDES_COM_BG_TIMER, 0x0a);
+	PLL_REG_W(pll_io->pll_base, QSERDES_COM_CORECLK_DIV_MODE0, 0x0a);
+	PLL_REG_W(pll_io->pll_base, QSERDES_COM_VCO_TUNE_CTRL, 0x00);
+	PLL_REG_W(pll_io->pll_base, QSERDES_COM_BIAS_EN_CLKBUFLR_EN, 0x3f);
+	PLL_REG_W(pll_io->pll_base, QSERDES_COM_CORE_CLK_EN, 0x1f);
+	PLL_REG_W(pll_io->pll_base, QSERDES_COM_PLL_IVCO, 0x07);
+	PLL_REG_W(pll_io->pll_base,
+		QSERDES_COM_LOCK_CMP_EN, dp_res->lock_cmp_en);
+	PLL_REG_W(pll_io->pll_base, QSERDES_COM_PLL_CCTRL_MODE0, 0x36);
+	PLL_REG_W(pll_io->pll_base, QSERDES_COM_PLL_RCTRL_MODE0, 0x16);
+	PLL_REG_W(pll_io->pll_base, QSERDES_COM_CP_CTRL_MODE0, 0x06);
+
+	if (dp_res->orientation == ORIENTATION_CC2)
+		PLL_REG_W(pll_io->phy_base, REG_DP_PHY_MODE, 0x4c);
+	else
+		PLL_REG_W(pll_io->phy_base, REG_DP_PHY_MODE, 0x5c);
+
+	/* TX Lane configuration */
+	PLL_REG_W(pll_io->phy_base,
+			REG_DP_PHY_TX0_TX1_LANE_CTL, 0x05);
+	PLL_REG_W(pll_io->phy_base,
+			REG_DP_PHY_TX2_TX3_LANE_CTL, 0x05);
+
+	/* TX-0 register configuration */
+	PLL_REG_W(pll_io->ln_tx0_base,
+			REG_DP_PHY_TXn_TRANSCEIVER_BIAS_EN, 0x1a);
+	PLL_REG_W(pll_io->ln_tx0_base,
+			REG_DP_PHY_TXn_VMODE_CTRL1, 0x40);
+	PLL_REG_W(pll_io->ln_tx0_base,
+			REG_DP_PHY_TXn_PRE_STALL_LDO_BOOST_EN, 0x30);
+	PLL_REG_W(pll_io->ln_tx0_base,
+			REG_DP_PHY_TXn_INTERFACE_SELECT, 0x3d);
+	PLL_REG_W(pll_io->ln_tx0_base,
+			REG_DP_PHY_TXn_CLKBUF_ENABLE, 0x0f);
+	PLL_REG_W(pll_io->ln_tx0_base,
+			REG_DP_PHY_TXn_RESET_TSYNC_EN, 0x03);
+	PLL_REG_W(pll_io->ln_tx0_base,
+			REG_DP_PHY_TXn_TRAN_DRVR_EMP_EN, 0x03);
+	PLL_REG_W(pll_io->ln_tx0_base,
+			REG_DP_PHY_TXn_PARRATE_REC_DETECT_IDLE_EN, 0x00);
+	PLL_REG_W(pll_io->ln_tx0_base,
+			REG_DP_PHY_TXn_TX_INTERFACE_MODE, 0x00);
+	PLL_REG_W(pll_io->ln_tx0_base, REG_DP_PHY_TXn_TX_BAND, 0x4);
+
+	/* TX-1 register configuration */
+	PLL_REG_W(pll_io->ln_tx1_base,
+			REG_DP_PHY_TXn_TRANSCEIVER_BIAS_EN, 0x1a);
+	PLL_REG_W(pll_io->ln_tx1_base,
+			REG_DP_PHY_TXn_VMODE_CTRL1, 0x40);
+	PLL_REG_W(pll_io->ln_tx1_base,
+			REG_DP_PHY_TXn_PRE_STALL_LDO_BOOST_EN, 0x30);
+	PLL_REG_W(pll_io->ln_tx1_base,
+			REG_DP_PHY_TXn_INTERFACE_SELECT, 0x3d);
+	PLL_REG_W(pll_io->ln_tx1_base,
+			REG_DP_PHY_TXn_CLKBUF_ENABLE, 0x0f);
+	PLL_REG_W(pll_io->ln_tx1_base,
+			REG_DP_PHY_TXn_RESET_TSYNC_EN, 0x03);
+	PLL_REG_W(pll_io->ln_tx1_base,
+			REG_DP_PHY_TXn_TRAN_DRVR_EMP_EN, 0x03);
+	PLL_REG_W(pll_io->ln_tx1_base,
+			REG_DP_PHY_TXn_PARRATE_REC_DETECT_IDLE_EN, 0x00);
+	PLL_REG_W(pll_io->ln_tx1_base,
+			REG_DP_PHY_TXn_TX_INTERFACE_MODE, 0x00);
+	PLL_REG_W(pll_io->ln_tx1_base,
+			REG_DP_PHY_TXn_TX_BAND, 0x4);
+
+	/* dependent on the vco frequency */
+	PLL_REG_W(pll_io->phy_base,
+			REG_DP_PHY_VCO_DIV, dp_res->phy_vco_div);
+
+	return res;
+}
+
+static bool dp_10nm_pll_lock_status(struct dp_pll_db *dp_res)
+{
+	u32 status;
+	bool pll_locked;
+	struct dp_io_pll *pll_io = &dp_res->base->pll_io;
+
+	/* poll for PLL lock status */
+	if (readl_poll_timeout_atomic((pll_io->pll_base +
+			QSERDES_COM_C_READY_STATUS),
+			status,
+			((status & BIT(0)) > 0),
+			DP_PHY_PLL_POLL_SLEEP_US,
+			DP_PHY_PLL_POLL_TIMEOUT_US)) {
+		DRM_ERROR("%s: C_READY status is not high. Status=%x\n",
+				__func__, status);
+		pll_locked = false;
+	} else {
+		pll_locked = true;
+	}
+
+	return pll_locked;
+}
+
+static bool dp_10nm_phy_rdy_status(struct dp_pll_db *dp_res)
+{
+	u32 status;
+	bool phy_ready = true;
+	struct dp_io_pll *pll_io = &dp_res->base->pll_io;
+
+	/* poll for PHY ready status */
+	if (readl_poll_timeout_atomic((pll_io->phy_base +
+			REG_DP_PHY_STATUS),
+			status,
+			((status & (BIT(1))) > 0),
+			DP_PHY_PLL_POLL_SLEEP_US,
+			DP_PHY_PLL_POLL_TIMEOUT_US)) {
+		DRM_ERROR("%s: Phy_ready is not high. Status=%x\n",
+				__func__, status);
+		phy_ready = false;
+	}
+
+	return phy_ready;
+}
+
+static int dp_pll_enable_10nm(struct clk_hw *hw)
+{
+	int rc = 0;
+	u32 bias_en, drvr_en;
+	struct dp_io_pll *pll_io;
+	struct dp_pll_vco_clk *vco = to_dp_vco_hw(hw);
+	struct msm_dp_pll *pll = to_msm_dp_pll(vco);
+	struct dp_pll_db *dp_res = to_dp_pll_db(pll);
+
+	pll_io = &pll->pll_io;
+
+	PLL_REG_W(pll_io->phy_base, REG_DP_PHY_AUX_CFG2, 0x04);
+	PLL_REG_W(pll_io->phy_base, REG_DP_PHY_CFG, 0x01);
+	PLL_REG_W(pll_io->phy_base, REG_DP_PHY_CFG, 0x05);
+	PLL_REG_W(pll_io->phy_base, REG_DP_PHY_CFG, 0x01);
+	PLL_REG_W(pll_io->phy_base, REG_DP_PHY_CFG, 0x09);
+
+	PLL_REG_W(pll_io->pll_base, QSERDES_COM_RESETSM_CNTRL, 0x20);
+
+	if (!dp_10nm_pll_lock_status(dp_res)) {
+		rc = -EINVAL;
+		goto lock_err;
+	}
+
+	PLL_REG_W(pll_io->phy_base, REG_DP_PHY_CFG, 0x19);
+	/* poll for PHY ready status */
+	if (!dp_10nm_phy_rdy_status(dp_res)) {
+		rc = -EINVAL;
+		goto lock_err;
+	}
+
+	DRM_DEBUG_DP("%s: PLL is locked\n", __func__);
+
+	if (dp_res->lane_cnt == 1) {
+		bias_en = 0x3e;
+		drvr_en = 0x13;
+	} else {
+		bias_en = 0x3f;
+		drvr_en = 0x10;
+	}
+
+	if (dp_res->lane_cnt != 4) {
+		if (dp_res->orientation == ORIENTATION_CC1) {
+			PLL_REG_W(pll_io->ln_tx1_base,
+				REG_DP_PHY_TXn_HIGHZ_DRVR_EN, drvr_en);
+			PLL_REG_W(pll_io->ln_tx1_base,
+				REG_DP_PHY_TXn_TRANSCEIVER_BIAS_EN, bias_en);
+		} else {
+			PLL_REG_W(pll_io->ln_tx0_base,
+				REG_DP_PHY_TXn_HIGHZ_DRVR_EN, drvr_en);
+			PLL_REG_W(pll_io->ln_tx0_base,
+				REG_DP_PHY_TXn_TRANSCEIVER_BIAS_EN, bias_en);
+		}
+	} else {
+		PLL_REG_W(pll_io->ln_tx0_base,
+				REG_DP_PHY_TXn_HIGHZ_DRVR_EN, drvr_en);
+		PLL_REG_W(pll_io->ln_tx0_base,
+				REG_DP_PHY_TXn_TRANSCEIVER_BIAS_EN, bias_en);
+		PLL_REG_W(pll_io->ln_tx1_base,
+				REG_DP_PHY_TXn_HIGHZ_DRVR_EN, drvr_en);
+		PLL_REG_W(pll_io->ln_tx1_base,
+				REG_DP_PHY_TXn_TRANSCEIVER_BIAS_EN, bias_en);
+	}
+
+	PLL_REG_W(pll_io->ln_tx0_base, REG_DP_PHY_TXn_TX_POL_INV, 0x0a);
+	PLL_REG_W(pll_io->ln_tx1_base, REG_DP_PHY_TXn_TX_POL_INV, 0x0a);
+	PLL_REG_W(pll_io->phy_base, REG_DP_PHY_CFG, 0x18);
+	udelay(2000);
+
+	PLL_REG_W(pll_io->phy_base, REG_DP_PHY_CFG, 0x19);
+
+	/* poll for PHY ready status */
+	if (!dp_10nm_phy_rdy_status(dp_res)) {
+		rc = -EINVAL;
+		goto lock_err;
+	}
+
+	PLL_REG_W(pll_io->ln_tx0_base, REG_DP_PHY_TXn_TX_DRV_LVL, 0x38);
+	PLL_REG_W(pll_io->ln_tx1_base, REG_DP_PHY_TXn_TX_DRV_LVL, 0x38);
+	PLL_REG_W(pll_io->ln_tx0_base, REG_DP_PHY_TXn_TX_EMP_POST1_LVL, 0x20);
+	PLL_REG_W(pll_io->ln_tx1_base, REG_DP_PHY_TXn_TX_EMP_POST1_LVL, 0x20);
+	PLL_REG_W(pll_io->ln_tx0_base,
+			REG_DP_PHY_TXn_RES_CODE_LANE_OFFSET_TX, 0x06);
+	PLL_REG_W(pll_io->ln_tx1_base,
+			REG_DP_PHY_TXn_RES_CODE_LANE_OFFSET_TX, 0x06);
+	PLL_REG_W(pll_io->ln_tx0_base,
+			REG_DP_PHY_TXn_RES_CODE_LANE_OFFSET_RX, 0x07);
+	PLL_REG_W(pll_io->ln_tx1_base,
+			REG_DP_PHY_TXn_RES_CODE_LANE_OFFSET_RX, 0x07);
+
+lock_err:
+	return rc;
+}
+
+static int dp_pll_disable_10nm(struct clk_hw *hw)
+{
+	int rc = 0;
+	struct dp_pll_vco_clk *vco = to_dp_vco_hw(hw);
+	struct msm_dp_pll *pll = to_msm_dp_pll(vco);
+
+	/* Assert DP PHY power down */
+	PLL_REG_W(pll->pll_io.phy_base, REG_DP_PHY_PD_CTL, 0x2);
+
+	return rc;
+}
+
+
+int dp_vco_prepare_10nm(struct clk_hw *hw)
+{
+	int rc = 0;
+	struct dp_pll_vco_clk *vco = to_dp_vco_hw(hw);
+	struct msm_dp_pll *pll = (struct msm_dp_pll *)vco->priv;
+	struct dp_pll_db *dp_res = to_dp_pll_db(pll);
+
+	DRM_DEBUG_DP("%s: rate = %ld\n", __func__, vco->rate);
+	if ((dp_res->vco_cached_rate != 0)
+		&& (dp_res->vco_cached_rate == vco->rate)) {
+		rc = dp_vco_set_rate_10nm(hw,
+			dp_res->vco_cached_rate, dp_res->vco_cached_rate);
+		if (rc) {
+			DRM_ERROR("index=%d vco_set_rate failed. rc=%d\n",
+				rc, dp_res->index);
+			goto error;
+		}
+	}
+
+	rc = dp_pll_enable_10nm(hw);
+	if (rc) {
+		DRM_ERROR("ndx=%d failed to enable dp pll\n",
+					dp_res->index);
+		goto error;
+	}
+
+	pll->pll_on = true;
+error:
+	return rc;
+}
+
+void dp_vco_unprepare_10nm(struct clk_hw *hw)
+{
+	struct dp_pll_vco_clk *vco = to_dp_vco_hw(hw);
+	struct msm_dp_pll *pll = to_msm_dp_pll(vco);
+	struct dp_pll_db *dp_res = to_dp_pll_db(pll);
+
+	if (!dp_res) {
+		DRM_ERROR("Invalid input parameter\n");
+		return;
+	}
+
+	if (!pll->pll_on) {
+		DRM_ERROR("pll resource can't be enabled\n");
+		return;
+	}
+	dp_res->vco_cached_rate = vco->rate;
+	dp_pll_disable_10nm(hw);
+
+	pll->pll_on = false;
+}
+
+int dp_vco_set_rate_10nm(struct clk_hw *hw, unsigned long rate,
+					unsigned long parent_rate)
+{
+	struct dp_pll_vco_clk *vco = to_dp_vco_hw(hw);
+	int rc;
+
+	DRM_DEBUG_DP("DP lane CLK rate=%ld\n", rate);
+
+	rc = dp_config_vco_rate_10nm(vco, rate);
+	if (rc)
+		DRM_ERROR("%s: Failed to set clk rate\n", __func__);
+
+	vco->rate = rate;
+
+	return 0;
+}
+
+unsigned long dp_vco_recalc_rate_10nm(struct clk_hw *hw,
+					unsigned long parent_rate)
+{
+	u32 div, hsclk_div, link_clk_div = 0;
+	u64 vco_rate;
+	struct dp_io_pll *pll_io;
+	struct dp_pll_vco_clk *vco = to_dp_vco_hw(hw);
+	struct msm_dp_pll *pll = to_msm_dp_pll(vco);
+	struct dp_pll_db *dp_res = to_dp_pll_db(pll);
+
+	pll_io = &pll->pll_io;
+
+	div = PLL_REG_R(pll_io->pll_base, QSERDES_COM_HSCLK_SEL);
+	div &= 0x0f;
+
+	if (div == 12)
+		hsclk_div = 6; /* Default */
+	else if (div == 4)
+		hsclk_div = 4;
+	else if (div == 0)
+		hsclk_div = 2;
+	else if (div == 3)
+		hsclk_div = 1;
+	else {
+		DRM_DEBUG_DP("unknown divider. forcing to default\n");
+		hsclk_div = 5;
+	}
+
+	div = PLL_REG_R(pll_io->phy_base, REG_DP_PHY_AUX_CFG2);
+	div >>= 2;
+
+	if ((div & 0x3) == 0)
+		link_clk_div = 5;
+	else if ((div & 0x3) == 1)
+		link_clk_div = 10;
+	else if ((div & 0x3) == 2)
+		link_clk_div = 20;
+	else
+		DRM_ERROR("%s: unsupported div. Phy_mode: %d\n", __func__, div);
+
+	if (link_clk_div == 20) {
+		vco_rate = DP_VCO_HSCLK_RATE_2700MHZDIV1000;
+	} else {
+		if (hsclk_div == 6)
+			vco_rate = DP_VCO_HSCLK_RATE_1620MHZDIV1000;
+		else if (hsclk_div == 4)
+			vco_rate = DP_VCO_HSCLK_RATE_2700MHZDIV1000;
+		else if (hsclk_div == 2)
+			vco_rate = DP_VCO_HSCLK_RATE_5400MHZDIV1000;
+		else
+			vco_rate = DP_VCO_HSCLK_RATE_8100MHZDIV1000;
+	}
+
+	DRM_DEBUG_DP("returning vco rate = %lu\n", (unsigned long)vco_rate);
+
+	dp_res->vco_cached_rate = vco->rate = vco_rate;
+	return (unsigned long)vco_rate;
+}
+
+long dp_vco_round_rate_10nm(struct clk_hw *hw, unsigned long rate,
+			unsigned long *parent_rate)
+{
+	unsigned long rrate = rate;
+	struct dp_pll_vco_clk *vco = to_dp_vco_hw(hw);
+
+	if (rate <= vco->min_rate)
+		rrate = vco->min_rate;
+	else if (rate <= DP_VCO_HSCLK_RATE_2700MHZDIV1000)
+		rrate = DP_VCO_HSCLK_RATE_2700MHZDIV1000;
+	else if (rate <= DP_VCO_HSCLK_RATE_5400MHZDIV1000)
+		rrate = DP_VCO_HSCLK_RATE_5400MHZDIV1000;
+	else
+		rrate = vco->max_rate;
+
+	DRM_DEBUG_DP("%s: rrate=%ld\n", __func__, rrate);
+
+	*parent_rate = rrate;
+	return rrate;
+}
diff --git a/drivers/gpu/drm/msm/dp/dp_pll_private.h b/drivers/gpu/drm/msm/dp/dp_pll_private.h
new file mode 100644
index 000000000000..54b5cd9d6b38
--- /dev/null
+++ b/drivers/gpu/drm/msm/dp/dp_pll_private.h
@@ -0,0 +1,103 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2016-2020, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef __DP_PLL_10NM_H
+#define __DP_PLL_10NM_H
+
+#include "dp_pll.h"
+#include "dp_reg.h"
+
+#define DP_VCO_HSCLK_RATE_1620MHZDIV1000	1620000UL
+#define DP_VCO_HSCLK_RATE_2700MHZDIV1000	2700000UL
+#define DP_VCO_HSCLK_RATE_5400MHZDIV1000	5400000UL
+#define DP_VCO_HSCLK_RATE_8100MHZDIV1000	8100000UL
+
+#define NUM_DP_CLOCKS_MAX			6
+
+#define DP_PHY_PLL_POLL_SLEEP_US		500
+#define DP_PHY_PLL_POLL_TIMEOUT_US		10000
+
+#define DP_VCO_RATE_8100MHZDIV1000		8100000UL
+#define DP_VCO_RATE_9720MHZDIV1000		9720000UL
+#define DP_VCO_RATE_10800MHZDIV1000		10800000UL
+
+struct dp_pll_vco_clk {
+	struct clk_hw hw;
+	unsigned long	rate;		/* current vco rate */
+	u64		min_rate;	/* min vco rate */
+	u64		max_rate;	/* max vco rate */
+	void		*priv;
+};
+
+struct dp_pll_db {
+	struct msm_dp_pll *base;
+
+	int id;
+	struct platform_device *pdev;
+
+	/* private clocks: */
+	struct clk_hw *hws[NUM_DP_CLOCKS_MAX];
+	u32 num_hws;
+
+	/* lane and orientation settings */
+	u8 lane_cnt;
+	u8 orientation;
+
+	/* COM PHY settings */
+	u32 hsclk_sel;
+	u32 dec_start_mode0;
+	u32 div_frac_start1_mode0;
+	u32 div_frac_start2_mode0;
+	u32 div_frac_start3_mode0;
+	u32 integloop_gain0_mode0;
+	u32 integloop_gain1_mode0;
+	u32 vco_tune_map;
+	u32 lock_cmp1_mode0;
+	u32 lock_cmp2_mode0;
+	u32 lock_cmp3_mode0;
+	u32 lock_cmp_en;
+
+	/* PHY vco divider */
+	u32 phy_vco_div;
+	/*
+	 * Certain pll's needs to update the same vco rate after resume in
+	 * suspend/resume scenario. Cached the vco rate for such plls.
+	 */
+	unsigned long	vco_cached_rate;
+	u32		cached_cfg0;
+	u32		cached_cfg1;
+	u32		cached_outdiv;
+
+	uint32_t index;
+};
+
+static inline struct dp_pll_vco_clk *to_dp_vco_hw(struct clk_hw *hw)
+{
+	return container_of(hw, struct dp_pll_vco_clk, hw);
+}
+
+#define to_msm_dp_pll(vco) ((struct msm_dp_pll *)vco->priv)
+
+#define to_dp_pll_db(x)	((struct dp_pll_db *)x->priv)
+
+int dp_vco_set_rate_10nm(struct clk_hw *hw, unsigned long rate,
+				unsigned long parent_rate);
+unsigned long dp_vco_recalc_rate_10nm(struct clk_hw *hw,
+				unsigned long parent_rate);
+long dp_vco_round_rate_10nm(struct clk_hw *hw, unsigned long rate,
+				unsigned long *parent_rate);
+int dp_vco_prepare_10nm(struct clk_hw *hw);
+void dp_vco_unprepare_10nm(struct clk_hw *hw);
+
+#ifdef CONFIG_DRM_MSM_DP_10NM_PLL
+int msm_dp_pll_10nm_init(struct msm_dp_pll *dp_pll, int id);
+#else
+int msm_dp_pll_10nm_init(struct msm_dp_pll *dp_pll, int id)
+{
+	return ERR_PTR(-ENODEV);
+}
+#endif
+
+#endif /* __DP_PLL_10NM_H */
diff --git a/drivers/gpu/drm/msm/dp/dp_power.c b/drivers/gpu/drm/msm/dp/dp_power.c
index 19a1777a2374..427a4edcc1e9 100644
--- a/drivers/gpu/drm/msm/dp/dp_power.c
+++ b/drivers/gpu/drm/msm/dp/dp_power.c
@@ -101,6 +101,16 @@ static int dp_power_clk_init(struct dp_power_private *power)
 	core = &power->parser->mp[DP_CORE_PM];
 	ctrl = &power->parser->mp[DP_CTRL_PM];
 
+	if (power->parser->pll && power->parser->pll->get_provider) {
+		rc = power->parser->pll->get_provider(power->parser->pll,
+			&power->link_provider, &power->pixel_provider);
+		if (rc) {
+			DRM_ERROR("%s:provider failed,don't set parent\n",
+								__func__);
+			return 0;
+		}
+	}
+
 	rc = msm_dss_get_clk(dev, core->clk_config, core->num_clk);
 	if (rc) {
 		DRM_ERROR("failed to get %s clk. err=%d\n",
diff --git a/drivers/gpu/drm/msm/dp/dp_power.h b/drivers/gpu/drm/msm/dp/dp_power.h
index 9d24f36c68e5..3dc4a525f435 100644
--- a/drivers/gpu/drm/msm/dp/dp_power.h
+++ b/drivers/gpu/drm/msm/dp/dp_power.h
@@ -14,7 +14,6 @@
  * @init: initializes the regulators/core clocks/GPIOs/pinctrl
  * @deinit: turns off the regulators/core clocks/GPIOs/pinctrl
  * @clk_enable: enable/disable the DP clocks
- * @set_link_clk_parent: set the parent of DP link clock
  * @set_pixel_clk_parent: set the parent of DP pixel clock
  */
 struct dp_power {
@@ -22,13 +21,83 @@ struct dp_power {
 	bool link_clks_on;
 };
 
+/**
+ * dp_power_init() - enable power supplies for display controller
+ *
+ * @power: instance of power module
+ * @flip: bool for flipping gpio direction
+ * return: 0 if success or error if failure.
+ *
+ * This API will turn on the regulators and configures gpio's
+ * aux/hpd.
+ */
 int dp_power_init(struct dp_power *power, bool flip);
+
+/**
+ * dp_power_deinit() - turn off regulators and gpios.
+ *
+ * @power: instance of power module
+ * return: 0 for success
+ *
+ * This API will deinitialize the DisplayPort's power module
+ * ,turns off power and regulators.
+ */
 int dp_power_deinit(struct dp_power *power);
+
+/**
+ * dp_power_clk_enable() - enable display controller clocks
+ *
+ * @power: instance of power module
+ * @pm_type: type of pm, core/ctrl/phy
+ * @enable: enables or disables
+ * return: pointer to allocated power module data
+ *
+ * This API will call setrate and enable for DP clocks
+ */
+
 int dp_power_clk_enable(struct dp_power *power, enum dp_pm_type pm_type,
 				bool enable);
+/**
+ * dp_power_set_link_clk_parent() - configures parent of link clocks
+ *
+ * @power: instance of power module
+ * return: 0 for success, error for failures
+ *
+ * This API will set the link clock parent source
+ */
 int dp_power_set_link_clk_parent(struct dp_power *power);
+
+
+/**
+ * dp_power_set_pixel_clk_parent() - configures parent of pixel clocks
+ *
+ * @power: instance of power module
+ * return: 0 for success, error for failures
+ *
+ * This API will set the pixel clock parent source
+ */
 int dp_power_set_pixel_clk_parent(struct dp_power *power);
+
+/**
+ * dp_power_clinet_init() - initialize clock and regulator modules
+ *
+ * @power: instance of power module
+ * return: 0 for success, error for failure.
+ *
+ * This API will configure the DisplayPort's clocks and regulator
+ * modueles.
+ */
 int dp_power_client_init(struct dp_power *power);
+
+/**
+ * dp_power_clinet_deinit() - de-initialize clock and regulator modules
+ *
+ * @power: instance of power module
+ * return: 0 for success, error for failure.
+ *
+ * This API will de-initialize the DisplayPort's clocks and regulator
+ * modueles.
+ */
 void dp_power_client_deinit(struct dp_power *power);
 
 /**
diff --git a/drivers/gpu/drm/msm/dp/dp_reg.h b/drivers/gpu/drm/msm/dp/dp_reg.h
index 28c16822bb73..ad6f1760f893 100644
--- a/drivers/gpu/drm/msm/dp/dp_reg.h
+++ b/drivers/gpu/drm/msm/dp/dp_reg.h
@@ -6,6 +6,22 @@
 #ifndef _DP_REG_H_
 #define _DP_REG_H_
 
+/* DP PHY Register Regions */
+#define REG_DP_PHY_REGION_BASE			(0x088ea000)
+#define REG_DP_PHY_REGION_SIZE			(0x00000C00)
+
+#define REG_USB3_DP_COM_REGION_BASE		(0x088e8000)
+#define REG_USB3_DP_COM_REGION_SIZE		(0x00000020)
+
+#define DP_PHY_PLL_OFFSET			(0x00000000)
+#define DP_PHY_PLL_SIZE				(0x00000200)
+#define DP_PHY_REG_OFFSET			(0x00000A00)
+#define DP_PHY_REG_SIZE				(0x00000200)
+#define DP_PHY_LN_TX0_OFFSET			(0x00000200)
+#define DP_PHY_LN_TX0_SIZE			(0x00000200)
+#define DP_PHY_LN_TX1_OFFSET			(0x00000600)
+#define DP_PHY_LN_TX1_SIZE			(0x00000200)
+
 /* DP_TX Registers */
 #define REG_DP_HW_VERSION			(0x00000000)
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v6 5/5] drm/msm/dpu: add display port support in DPU
  2020-06-12  1:50 [PATCH v6 0/5] Add support for DisplayPort driver on Tanmay Shah
                   ` (2 preceding siblings ...)
  2020-06-12  1:50 ` [PATCH v6 4/5] drm/msm/dp: add support for DP PLL driver Tanmay Shah
@ 2020-06-12  1:50 ` Tanmay Shah
  2020-06-12 23:26 ` [PATCH v6 0/5] Add support for DisplayPort driver on Stephen Boyd
  4 siblings, 0 replies; 17+ messages in thread
From: Tanmay Shah @ 2020-06-12  1:50 UTC (permalink / raw)
  To: sam, robh+dt, swboyd
  Cc: devicetree, linux-kernel, linux-arm-msm, dri-devel, freedreno,
	seanpaul, robdclark, aravindh, abhinavk, Jeykumar Sankaran,
	Chandan Uddaraju, Vara Reddy, Tanmay Shah

From: Jeykumar Sankaran <jsanka@codeaurora.org>

Add display port support in DPU by creating hooks
for DP encoder enumeration and encoder mode
initialization.

This change is based on the Snapdragon Display port
driver changes[1].

changes in v2:
	- rebase on [2] (Sean Paul)
	- remove unwanted error checks and
	  switch cases (Jordan Crouse)

[1] https://lwn.net/Articles/768265/
[2] https://lkml.org/lkml/2018/11/17/87

changes in V3:
-- Moved this change as part of the DP driver changes.
-- Addressed compilation issues on the latest code base.

Changes in v6:
-- Fix checkpatch.pl warning

Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
Signed-off-by: Chandan Uddaraju <chandanu@codeaurora.org>
Signed-off-by: Vara Reddy <varar@codeaurora.org>
Signed-off-by: Tanmay Shah <tanmay@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  8 +--
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c     | 65 +++++++++++++++++----
 2 files changed, 58 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index d796710ff4aa..745d5ce7e821 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2017,7 +2017,7 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
 {
 	int ret = 0;
 	int i = 0;
-	enum dpu_intf_type intf_type;
+	enum dpu_intf_type intf_type = INTF_NONE;
 	struct dpu_enc_phys_init_params phys_params;
 
 	if (!dpu_enc) {
@@ -2039,9 +2039,9 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
 	case DRM_MODE_ENCODER_DSI:
 		intf_type = INTF_DSI;
 		break;
-	default:
-		DPU_ERROR_ENC(dpu_enc, "unsupported display interface type\n");
-		return -EINVAL;
+	case DRM_MODE_ENCODER_TMDS:
+		intf_type = INTF_DP;
+		break;
 	}
 
 	WARN_ON(disp_info->num_of_h_tiles < 1);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index b8615d4fe8a3..f6c219f875db 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -492,6 +492,33 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev,
 	return rc;
 }
 
+static int _dpu_kms_initialize_displayport(struct drm_device *dev,
+					    struct msm_drm_private *priv,
+					    struct dpu_kms *dpu_kms)
+{
+	struct drm_encoder *encoder = NULL;
+	int rc = 0;
+
+	if (!priv->dp)
+		return rc;
+
+	encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
+	if (IS_ERR(encoder)) {
+		DPU_ERROR("encoder init failed for dsi display\n");
+		return PTR_ERR(encoder);
+	}
+
+	rc = msm_dp_modeset_init(priv->dp, dev, encoder);
+	if (rc) {
+		DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
+		drm_encoder_cleanup(encoder);
+		return rc;
+	}
+
+	priv->encoders[priv->num_encoders++] = encoder;
+	return rc;
+}
+
 /**
  * _dpu_kms_setup_displays - create encoders, bridges and connectors
  *                           for underlying displays
@@ -504,12 +531,21 @@ static int _dpu_kms_setup_displays(struct drm_device *dev,
 				    struct msm_drm_private *priv,
 				    struct dpu_kms *dpu_kms)
 {
-	/**
-	 * Extend this function to initialize other
-	 * types of displays
-	 */
+	int rc = 0;
+
+	rc = _dpu_kms_initialize_dsi(dev, priv, dpu_kms);
+	if (rc) {
+		DPU_ERROR("initialize_dsi failed, rc = %d\n", rc);
+		return rc;
+	}
 
-	return _dpu_kms_initialize_dsi(dev, priv, dpu_kms);
+	rc = _dpu_kms_initialize_displayport(dev, priv, dpu_kms);
+	if (rc) {
+		DPU_ERROR("initialize_DP failed, rc = %d\n", rc);
+		return rc;
+	}
+
+	return rc;
 }
 
 static void _dpu_kms_drm_obj_destroy(struct dpu_kms *dpu_kms)
@@ -694,13 +730,20 @@ static void _dpu_kms_set_encoder_mode(struct msm_kms *kms,
 	info.capabilities = cmd_mode ? MSM_DISPLAY_CAP_CMD_MODE :
 			MSM_DISPLAY_CAP_VID_MODE;
 
-	/* TODO: No support for DSI swap */
-	for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
-		if (priv->dsi[i]) {
-			info.h_tile_instance[info.num_of_h_tiles] = i;
-			info.num_of_h_tiles++;
+	switch (info.intf_type) {
+	case DRM_MODE_ENCODER_DSI:
+		/* TODO: No support for DSI swap */
+		for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
+			if (priv->dsi[i]) {
+				info.h_tile_instance[info.num_of_h_tiles] = i;
+				info.num_of_h_tiles++;
+			}
 		}
-	}
+		break;
+	case DRM_MODE_ENCODER_TMDS:
+		info.num_of_h_tiles = 1;
+		break;
+	};
 
 	rc = dpu_encoder_setup(encoder->dev, encoder, &info);
 	if (rc)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 1/5] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon
  2020-06-12  1:50 ` [PATCH v6 1/5] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon Tanmay Shah
@ 2020-06-12 16:21   ` Rob Herring
  2020-06-16 11:15   ` Stephen Boyd
  1 sibling, 0 replies; 17+ messages in thread
From: Rob Herring @ 2020-06-12 16:21 UTC (permalink / raw)
  To: Tanmay Shah
  Cc: Sam Ravnborg, Stephen Boyd, devicetree,
	linux-kernel@vger.kernel.org, linux-arm-msm, dri-devel, freedreno,
	Sean Paul, Rob Clark, aravindh, Abhinav Kumar, Chandan Uddaraju,
	Vara Reddy

On Thu, Jun 11, 2020 at 7:51 PM Tanmay Shah <tanmay@codeaurora.org> wrote:
>
> From: Chandan Uddaraju <chandanu@codeaurora.org>
>
> 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.
>
> Signed-off-by: Chandan Uddaraju <chandanu@codeaurora.org>
> Signed-off-by: Vara Reddy <varar@codeaurora.org>
> Signed-off-by: Tanmay Shah <tanmay@codeaurora.org>
> ---
>  .../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

Is it too much to ask for Qualcomm to coordinate your work? I'm not
going to do that for you. This conflicts with "[v4] dt-bindings: msm:
disp: add yaml schemas for DPU and DSI bindings".

> diff --git a/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
> new file mode 100644
> index 000000000000..5fdb9153df00
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
> @@ -0,0 +1,142 @@
> +# SPDX-License-Identifier: (GPL-2.0-only  OR BSD-2-Clause)

Extra space.

> +%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 <chandanu@codeaurora.org>
> +  - Vara Reddy <varar@codeaurora.org>
> +  - Tanmay Shah <tanmay@codeaurora.org>
> +
> +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

That's what the h/w reference manual calls this? It should be SoC specific.

> +
> +  cell-index:
> +    description: Specifies the controller instance.

Pretty sure I already said no to this.

> +
> +  reg:
> +    items:
> +      - description: DP controller registers

Just: 'maxItems: 1'

> +
> +  interrupts:
> +    description: The interrupt signal from the DP block.

How many?

The description is useless. That's every 'interrupts'.

> +
> +  clocks:
> +    description: List of clock specifiers for clocks needed by the device.

That's every 'clocks' property. Drop.

> +    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.

Drop.

> +    items:
> +      - const: core_aux
> +      - const: ctrl_link
> +      - const: ctrl_link_iface
> +      - const: stream_pixel
> +      - const: pixel_rcg

blank line

> +  "#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>:

Err, what?!

> +    type: object

This is a DT node?

> +    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.

Look at other schemas and see how they are done.

> +         Documentation/devicetree/bindings/graph.txt and
> +         Documentation/devicetree/bindings/media/video-interfaces.txt.
> +
> +patternProperties:
> +  "^aux-cfg([0-9])-settings$":
> +    type: object

This is a DT node?

> +    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

Add:
additionalProperties: false

> +
> +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 {
> +                };
> +            };
> +        };
> +    };
> diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt
> index 551ae26f60da..30c8ab479b02 100644
> --- a/Documentation/devicetree/bindings/display/msm/dpu.txt
> +++ b/Documentation/devicetree/bindings/display/msm/dpu.txt
> @@ -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>;
> +                                       };
> +                               };
>                         };
>                 };
>         };
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 0/5] Add support for DisplayPort driver on
  2020-06-12  1:50 [PATCH v6 0/5] Add support for DisplayPort driver on Tanmay Shah
                   ` (3 preceding siblings ...)
  2020-06-12  1:50 ` [PATCH v6 5/5] drm/msm/dpu: add display port support in DPU Tanmay Shah
@ 2020-06-12 23:26 ` Stephen Boyd
  2020-06-15 22:51   ` tanmay
  4 siblings, 1 reply; 17+ messages in thread
From: Stephen Boyd @ 2020-06-12 23:26 UTC (permalink / raw)
  To: Tanmay Shah, robh+dt, sam
  Cc: devicetree, linux-kernel, linux-arm-msm, dri-devel, freedreno,
	seanpaul, robdclark, aravindh, abhinavk, Tanmay Shah

Quoting Tanmay Shah (2020-06-11 18:50:25)
> These patches add support for Display-Port driver on SnapDragon
> hardware. It adds
> DP driver and DP PLL driver files along with the needed device-tree
> bindings.
> 
> The block diagram of DP driver is shown below:
> 
> 
>                  +-------------+
>                  |DRM FRAMEWORK|
>                  +------+------+
>                         |
>                    +----v----+
>                    | DP DRM  |
>                    +----+----+
>                         |
>                    +----v----+
>      +------------+|   DP    +----------++------+
>      +        +---+| DISPLAY |+---+      |      |
>      |        +    +-+-----+-+    |      |      |
>      |        |      |     |      |      |      |
>      |        |      |     |      |      |      |
>      |        |      |     |      |      |      |
>      v        v      v     v      v      v      v
>  +------+ +------+ +---+ +----+ +----+ +---+ +-----+
>  |  DP  | |  DP  | |DP | | DP | | DP | |DP | | DP  |
>  |PARSER| | HPD  | |AUX| |LINK| |CTRL| |PHY| |POWER|
>  +--+---+ +---+--+ +---+ +----+ +--+-+ +-+-+ +-----+
>     |                              |     |
>  +--v---+                         +v-----v+
>  |DEVICE|                         |  DP   |
>  | TREE |                         |CATALOG|
>  +------+                         +---+---+
>                                       |
>                                   +---v----+
>                                   |CTRL/PHY|
>                                   |   HW   |
>                                   +--------+
> 

I've never seen a block diagram for a driver before...

> 
> These patches have dependency on clock driver changes mentioned below:
> https://patchwork.kernel.org/patch/11245895/
> https://patchwork.kernel.org/cover/11069083/

These are merged right? Don't need to include this if it's already
merged.

Can you include a changelog in the cover letter too so we know what has
changed between versions of the patchset?

> 
> Chandan Uddaraju (4):
>   dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon
>   drm: add constant N value in helper file
>   drm/msm/dp: add displayPort driver support
>   drm/msm/dp: add support for DP PLL driver
> 
> Jeykumar Sankaran (1):
>   drm/msm/dpu: add display port support in DPU
> 
[...]
> 
> 
> base-commit: 48f99181fc118d82dc8bf6c7221ad1c654cb8bc2

What is this commit? I don't see it in linux-next.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 0/5] Add support for DisplayPort driver on
  2020-06-12 23:26 ` [PATCH v6 0/5] Add support for DisplayPort driver on Stephen Boyd
@ 2020-06-15 22:51   ` tanmay
  2020-06-15 23:04     ` [Freedreno] " Jeffrey Hugo
  0 siblings, 1 reply; 17+ messages in thread
From: tanmay @ 2020-06-15 22:51 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: robh+dt, sam, devicetree, linux-kernel, linux-arm-msm, dri-devel,
	freedreno, seanpaul, robdclark, aravindh, abhinavk

On 2020-06-12 16:26, Stephen Boyd wrote:

Thanks for reviews Stephen.

> Quoting Tanmay Shah (2020-06-11 18:50:25)
>> These patches add support for Display-Port driver on SnapDragon
>> hardware. It adds
>> DP driver and DP PLL driver files along with the needed device-tree
>> bindings.
>> 
>> The block diagram of DP driver is shown below:
>> 
>> 
>>                  +-------------+
>>                  |DRM FRAMEWORK|
>>                  +------+------+
>>                         |
>>                    +----v----+
>>                    | DP DRM  |
>>                    +----+----+
>>                         |
>>                    +----v----+
>>      +------------+|   DP    +----------++------+
>>      +        +---+| DISPLAY |+---+      |      |
>>      |        +    +-+-----+-+    |      |      |
>>      |        |      |     |      |      |      |
>>      |        |      |     |      |      |      |
>>      |        |      |     |      |      |      |
>>      v        v      v     v      v      v      v
>>  +------+ +------+ +---+ +----+ +----+ +---+ +-----+
>>  |  DP  | |  DP  | |DP | | DP | | DP | |DP | | DP  |
>>  |PARSER| | HPD  | |AUX| |LINK| |CTRL| |PHY| |POWER|
>>  +--+---+ +---+--+ +---+ +----+ +--+-+ +-+-+ +-----+
>>     |                              |     |
>>  +--v---+                         +v-----v+
>>  |DEVICE|                         |  DP   |
>>  | TREE |                         |CATALOG|
>>  +------+                         +---+---+
>>                                       |
>>                                   +---v----+
>>                                   |CTRL/PHY|
>>                                   |   HW   |
>>                                   +--------+
>> 
> 
> I've never seen a block diagram for a driver before...
> 
It is here for v5. https://patchwork.freedesktop.org/series/74312/

>> 
>> These patches have dependency on clock driver changes mentioned below:
>> https://patchwork.kernel.org/patch/11245895/
>> https://patchwork.kernel.org/cover/11069083/
> 
> These are merged right? Don't need to include this if it's already
> merged.
> 
Ok Thanks.

> Can you include a changelog in the cover letter too so we know what has
> changed between versions of the patchset?
> 
Sure.
>> 
>> Chandan Uddaraju (4):
>>   dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon
>>   drm: add constant N value in helper file
>>   drm/msm/dp: add displayPort driver support
>>   drm/msm/dp: add support for DP PLL driver
>> 
>> Jeykumar Sankaran (1):
>>   drm/msm/dpu: add display port support in DPU
>> 
> [...]
>> 
>> 
>> base-commit: 48f99181fc118d82dc8bf6c7221ad1c654cb8bc2
> 
> What is this commit? I don't see it in linux-next.
Here: 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20200603&id=48f99181fc118d82dc8bf6c7221ad1c654cb8bc2

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Freedreno] [PATCH v6 0/5] Add support for DisplayPort driver on
  2020-06-15 22:51   ` tanmay
@ 2020-06-15 23:04     ` Jeffrey Hugo
  2020-06-15 23:36       ` tanmay
  0 siblings, 1 reply; 17+ messages in thread
From: Jeffrey Hugo @ 2020-06-15 23:04 UTC (permalink / raw)
  To: tanmay
  Cc: Stephen Boyd, DTML, aravindh, MSM, lkml,
	open list:DRM PANEL DRIVERS, Rob Clark, Rob Herring, Sean Paul,
	Abhinav Kumar, freedreno, Sam Ravnborg

On Mon, Jun 15, 2020 at 4:51 PM <tanmay@codeaurora.org> wrote:
>
> On 2020-06-12 16:26, Stephen Boyd wrote:
>
> Thanks for reviews Stephen.
>
> > Quoting Tanmay Shah (2020-06-11 18:50:25)
> >> These patches add support for Display-Port driver on SnapDragon
> >> hardware. It adds
> >> DP driver and DP PLL driver files along with the needed device-tree
> >> bindings.
> >>
> >> The block diagram of DP driver is shown below:
> >>
> >>
> >>                  +-------------+
> >>                  |DRM FRAMEWORK|
> >>                  +------+------+
> >>                         |
> >>                    +----v----+
> >>                    | DP DRM  |
> >>                    +----+----+
> >>                         |
> >>                    +----v----+
> >>      +------------+|   DP    +----------++------+
> >>      +        +---+| DISPLAY |+---+      |      |
> >>      |        +    +-+-----+-+    |      |      |
> >>      |        |      |     |      |      |      |
> >>      |        |      |     |      |      |      |
> >>      |        |      |     |      |      |      |
> >>      v        v      v     v      v      v      v
> >>  +------+ +------+ +---+ +----+ +----+ +---+ +-----+
> >>  |  DP  | |  DP  | |DP | | DP | | DP | |DP | | DP  |
> >>  |PARSER| | HPD  | |AUX| |LINK| |CTRL| |PHY| |POWER|
> >>  +--+---+ +---+--+ +---+ +----+ +--+-+ +-+-+ +-----+
> >>     |                              |     |
> >>  +--v---+                         +v-----v+
> >>  |DEVICE|                         |  DP   |
> >>  | TREE |                         |CATALOG|
> >>  +------+                         +---+---+
> >>                                       |
> >>                                   +---v----+
> >>                                   |CTRL/PHY|
> >>                                   |   HW   |
> >>                                   +--------+
> >>
> >
> > I've never seen a block diagram for a driver before...
> >
> It is here for v5. https://patchwork.freedesktop.org/series/74312/

I think Stephen is nitpicking your wording, and you seem to not be
understanding his comment.  I'm sorry if I am mistaken.

The "DP driver" would seem to refer to the linux software driver you
are proposing patches for, however this diagram looks like a hardware
diagram of the various hardware blocks that the Linux driver code (the
"DP driver") is expected to interact with.  I believe you should
re-word "The block diagram of DP driver is shown below:" to be more
specific of what you are describing with your figure.  IE your words
say this is a block diagram of the software, when it looks like it is
a block diagram of the hardware.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Freedreno] [PATCH v6 0/5] Add support for DisplayPort driver on
  2020-06-15 23:04     ` [Freedreno] " Jeffrey Hugo
@ 2020-06-15 23:36       ` tanmay
  0 siblings, 0 replies; 17+ messages in thread
From: tanmay @ 2020-06-15 23:36 UTC (permalink / raw)
  To: Jeffrey Hugo
  Cc: Stephen Boyd, DTML, aravindh, MSM, lkml,
	open list:DRM PANEL DRIVERS, Rob Clark, Rob Herring, Sean Paul,
	Abhinav Kumar, freedreno, Sam Ravnborg

On 2020-06-15 16:04, Jeffrey Hugo wrote:
> On Mon, Jun 15, 2020 at 4:51 PM <tanmay@codeaurora.org> wrote:
>> 
>> On 2020-06-12 16:26, Stephen Boyd wrote:
>> 
>> Thanks for reviews Stephen.
>> 
>> > Quoting Tanmay Shah (2020-06-11 18:50:25)
>> >> These patches add support for Display-Port driver on SnapDragon
>> >> hardware. It adds
>> >> DP driver and DP PLL driver files along with the needed device-tree
>> >> bindings.
>> >>
>> >> The block diagram of DP driver is shown below:
>> >>
>> >>
>> >>                  +-------------+
>> >>                  |DRM FRAMEWORK|
>> >>                  +------+------+
>> >>                         |
>> >>                    +----v----+
>> >>                    | DP DRM  |
>> >>                    +----+----+
>> >>                         |
>> >>                    +----v----+
>> >>      +------------+|   DP    +----------++------+
>> >>      +        +---+| DISPLAY |+---+      |      |
>> >>      |        +    +-+-----+-+    |      |      |
>> >>      |        |      |     |      |      |      |
>> >>      |        |      |     |      |      |      |
>> >>      |        |      |     |      |      |      |
>> >>      v        v      v     v      v      v      v
>> >>  +------+ +------+ +---+ +----+ +----+ +---+ +-----+
>> >>  |  DP  | |  DP  | |DP | | DP | | DP | |DP | | DP  |
>> >>  |PARSER| | HPD  | |AUX| |LINK| |CTRL| |PHY| |POWER|
>> >>  +--+---+ +---+--+ +---+ +----+ +--+-+ +-+-+ +-----+
>> >>     |                              |     |
>> >>  +--v---+                         +v-----v+
>> >>  |DEVICE|                         |  DP   |
>> >>  | TREE |                         |CATALOG|
>> >>  +------+                         +---+---+
>> >>                                       |
>> >>                                   +---v----+
>> >>                                   |CTRL/PHY|
>> >>                                   |   HW   |
>> >>                                   +--------+
>> >>
>> >
>> > I've never seen a block diagram for a driver before...
>> >
>> It is here for v5. https://patchwork.freedesktop.org/series/74312/
> 
> I think Stephen is nitpicking your wording, and you seem to not be
> understanding his comment.  I'm sorry if I am mistaken.
> 
> The "DP driver" would seem to refer to the linux software driver you
> are proposing patches for, however this diagram looks like a hardware
> diagram of the various hardware blocks that the Linux driver code (the
> "DP driver") is expected to interact with.  I believe you should
> re-word "The block diagram of DP driver is shown below:" to be more
> specific of what you are describing with your figure.  IE your words
> say this is a block diagram of the software, when it looks like it is
> a block diagram of the hardware.

Thanks for reviews.

I am not sure what Stephen meant, but this diagram was available before.

Just for clarification this is not hardware diagram at all.
This is modeling of DP driver for msm.
Each box name above except "DRM framework", is file name in driver i.e. 
software module.
Each line and arrow shows how modules interact with each other.

For example, "DP PARSER" Box is pointing towards "DEVICE TREE" Box, that 
means
dp_parser.c file contains functions which are parsing device tree 
properties and so on...

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 1/5] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon
  2020-06-11 20:07   ` tanmay
@ 2020-06-16 10:55     ` Stephen Boyd
  2020-06-16 22:30       ` tanmay
  0 siblings, 1 reply; 17+ messages in thread
From: Stephen Boyd @ 2020-06-16 10:55 UTC (permalink / raw)
  To: tanmay
  Cc: devicetree, sam, seanpaul, freedreno, linux-arm-msm, chandanu,
	robdclark, abhinavk, nganji, dri-devel, linux-clk, Vara Reddy

Quoting tanmay@codeaurora.org (2020-06-11 13:07:09)
> On 2020-06-09 19:15, Stephen Boyd wrote:
> > Quoting Tanmay Shah (2020-06-08 20:38:18)
> >> diff --git 
> >> a/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml 
> >> b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
> >> new file mode 100644
> >> index 0000000..5fdb915
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
> > 
> > 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.
> > 
> 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.

> > 
> >> @@ -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 <chandanu@codeaurora.org>
> >> +  - Vara Reddy <varar@codeaurora.org>
> >> +  - Tanmay Shah <tanmay@codeaurora.org>
> >> +
> >> +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
> > 
> > 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.
> > 
> 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.

Cool. Using assigned clock parents is good.

> 
> >> +  "#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.
> > 
> 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

Ok.

> 
> >> +    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.
> > 
> > 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.

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).

> >> +
> >> +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 {
> >> +                };
> >> +            };
> >> +        };
> >> +    };
> > 
> > I believe there should be a '...' here.
> 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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 1/5] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon
  2020-06-12  1:50 ` [PATCH v6 1/5] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon Tanmay Shah
  2020-06-12 16:21   ` Rob Herring
@ 2020-06-16 11:15   ` Stephen Boyd
  2020-06-17 15:38     ` Rob Herring
  1 sibling, 1 reply; 17+ messages in thread
From: Stephen Boyd @ 2020-06-16 11:15 UTC (permalink / raw)
  To: Tanmay Shah, robh+dt, sam
  Cc: devicetree, linux-kernel, linux-arm-msm, dri-devel, freedreno,
	seanpaul, robdclark, aravindh, abhinavk, Chandan Uddaraju,
	Vara Reddy, Tanmay Shah

Quoting Tanmay Shah (2020-06-11 18:50:26)
> diff --git a/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
> new file mode 100644
> index 000000000000..5fdb9153df00
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
> @@ -0,0 +1,142 @@
> +        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 {

Just curious what is eventually connected here? This is possibly a
question for Rob Herring, but I can't figure out how we're supposed to
connect this to the USB type-c connector that is receiving the DP
signal. Does the type-c connector binding support connecting to this end
of the graph? Or should this connect to the DP phy and then the phy
connects to the USB type-c connector node? Right now it is empty which
seems wrong.

> +                };
> +            };
> +        };
> +    };

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 1/5] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon
  2020-06-16 10:55     ` Stephen Boyd
@ 2020-06-16 22:30       ` tanmay
  0 siblings, 0 replies; 17+ messages in thread
From: tanmay @ 2020-06-16 22:30 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: devicetree, sam, seanpaul, freedreno, linux-arm-msm, chandanu,
	robdclark, abhinavk, nganji, dri-devel, linux-clk, Vara Reddy

Thanks Stephen for your answers.

On 2020-06-16 03:55, Stephen Boyd wrote:
> Quoting tanmay@codeaurora.org (2020-06-11 13:07:09)
>> On 2020-06-09 19:15, Stephen Boyd wrote:
>> > Quoting Tanmay Shah (2020-06-08 20:38:18)
>> >> diff --git
>> >> a/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
>> >> b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
>> >> new file mode 100644
>> >> index 0000000..5fdb915
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
>> >
>> > 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.
>> >
>> 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.
> 
>> >
>> >> @@ -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 <chandanu@codeaurora.org>
>> >> +  - Vara Reddy <varar@codeaurora.org>
>> >> +  - Tanmay Shah <tanmay@codeaurora.org>
>> >> +
>> >> +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
>> >
>> > 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.
>> >
>> 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.
> 
> Cool. Using assigned clock parents is good.
> 
>> 
>> >> +  "#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.
>> >
>> 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
> 
> Ok.
> 
>> 
>> >> +    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.
>> >
>> > 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.
> 
> 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.

>> >> +
>> >> +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 {
>> >> +                };
>> >> +            };
>> >> +        };
>> >> +    };
>> >
>> > I believe there should be a '...' here.
>> 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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 1/5] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon
  2020-06-16 11:15   ` Stephen Boyd
@ 2020-06-17 15:38     ` Rob Herring
  0 siblings, 0 replies; 17+ messages in thread
From: Rob Herring @ 2020-06-17 15:38 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Tanmay Shah, Sam Ravnborg, devicetree,
	linux-kernel@vger.kernel.org, linux-arm-msm, dri-devel, freedreno,
	Sean Paul, Rob Clark, aravindh, Abhinav Kumar, Chandan Uddaraju,
	Vara Reddy

On Tue, Jun 16, 2020 at 5:15 AM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Quoting Tanmay Shah (2020-06-11 18:50:26)
> > diff --git a/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
> > new file mode 100644
> > index 000000000000..5fdb9153df00
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
> > @@ -0,0 +1,142 @@
> > +        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 {
>
> Just curious what is eventually connected here? This is possibly a
> question for Rob Herring, but I can't figure out how we're supposed to
> connect this to the USB type-c connector that is receiving the DP
> signal. Does the type-c connector binding support connecting to this end
> of the graph? Or should this connect to the DP phy and then the phy
> connects to the USB type-c connector node? Right now it is empty which
> seems wrong.

It should connect to the Type-C connector perhaps thru some sort of
switching/muxing node, but that's not really flushed out though. See
'dt-bindings: chrome: Add cros-ec-typec mux props' discussion with
your CrOS colleagues.

Rob

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2020-06-17 15:38 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-12  1:50 [PATCH v6 0/5] Add support for DisplayPort driver on Tanmay Shah
2020-06-12  1:50 ` [PATCH v6 1/5] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon Tanmay Shah
2020-06-12 16:21   ` Rob Herring
2020-06-16 11:15   ` Stephen Boyd
2020-06-17 15:38     ` Rob Herring
2020-06-12  1:50 ` [PATCH v6 2/5] drm: add constant N value in helper file Tanmay Shah
2020-06-12  1:50 ` [PATCH v6 4/5] drm/msm/dp: add support for DP PLL driver Tanmay Shah
2020-06-12  1:50 ` [PATCH v6 5/5] drm/msm/dpu: add display port support in DPU Tanmay Shah
2020-06-12 23:26 ` [PATCH v6 0/5] Add support for DisplayPort driver on Stephen Boyd
2020-06-15 22:51   ` tanmay
2020-06-15 23:04     ` [Freedreno] " Jeffrey Hugo
2020-06-15 23:36       ` tanmay
  -- strict thread matches above, loose matches on Subject: below --
2020-06-09  3:38 [PATCH v6 1/5] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon Tanmay Shah
2020-06-10  2:15 ` Stephen Boyd
2020-06-11 20:07   ` tanmay
2020-06-16 10:55     ` Stephen Boyd
2020-06-16 22:30       ` tanmay

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox