diff mbox series

[DPU,1/3] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon 845

Message ID 1539191759-14836-2-git-send-email-chandanu@codeaurora.org (mailing list archive)
State New, archived
Headers show
Series Add support for DisplayPort driver on SnapDragon 845 | expand

Commit Message

Chandan Uddaraju Oct. 10, 2018, 5:15 p.m. UTC
Add bindings for Snapdragon 845 DisplayPort and
display-port PLL driver.

Signed-off-by: Chandan Uddaraju <chandanu@codeaurora.org>
---
 .../devicetree/bindings/display/msm/dp.txt         | 249 +++++++++++++++++++++
 .../devicetree/bindings/display/msm/dpu.txt        |  16 +-
 2 files changed, 261 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/msm/dp.txt

Comments

Sean Paul Oct. 10, 2018, 5:41 p.m. UTC | #1
On Wed, Oct 10, 2018 at 10:15:57AM -0700, Chandan Uddaraju wrote:
> Add bindings for Snapdragon 845 DisplayPort and
> display-port PLL driver.
> 

This won't get Rob Herring's review unless it's sent to the devicetree list.

> Signed-off-by: Chandan Uddaraju <chandanu@codeaurora.org>
> ---
>  .../devicetree/bindings/display/msm/dp.txt         | 249 +++++++++++++++++++++
>  .../devicetree/bindings/display/msm/dpu.txt        |  16 +-
>  2 files changed, 261 insertions(+), 4 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/display/msm/dp.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/dp.txt b/Documentation/devicetree/bindings/display/msm/dp.txt
> new file mode 100644
> index 0000000..0155266
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/msm/dp.txt
> @@ -0,0 +1,249 @@
> +Qualcomm Technologies, Inc.
> +DP is the master Display Port device which supports DP host controllers that are compatible with VESA Display Port interface specification.
> +DP Controller: Required properties:
> +- compatible:           Should be "qcom,dp-display".
> +- reg:                  Base address and length of DP hardware's memory mapped regions.
> +- cell-index:           Specifies the controller instance.
> +- reg-names:            A list of strings that name the list of regs.
> +			"dp_ahb" - DP controller memory region.
> +			"dp_aux" - DP AUX memory region.
> +			"dp_link" - DP link layer memory region.
> +			"dp_p0" - DP pixel clock domain memory region.
> +			"dp_phy" - DP PHY memory region.
> +			"dp_ln_tx0" - USB3 DP PHY combo TX-0 lane memory region.
> +			"dp_ln_tx1" - USB3 DP PHY combo TX-1 lane memory region.
> +			"dp_mmss_cc" - Display Clock Control memory region.
> +			"qfprom_physical" - QFPROM Phys memory region.
> +			"dp_pll" - USB3 DP combo PLL memory region.
> +			"usb3_dp_com" - USB3 DP PHY combo memory region.
> +			"hdcp_physical" - DP HDCP memory region.
> +- interrupt-parent	phandle to the interrupt parent device node.
> +- interrupts:		The interrupt signal from the DP block.
> +- clocks:               Clocks required for Display Port operation. See [1] for details on clock bindings.
> +- clock-names:          Names of the clocks corresponding to handles. Following clocks are required:
> +			"core_aux_clk", "core_usb_ref_clk_src","core_usb_ref_clk", "core_usb_cfg_ahb_clk",
> +			"core_usb_pipe_clk", "ctrl_link_clk", "ctrl_link_iface_clk", "ctrl_crypto_clk",
> +			"ctrl_pixel_clk", "pixel_clk_rcg", "pixel_parent".
> +- pll-node:		phandle to DP PLL node.
> +- vdda-1p2-supply:		phandle to vdda 1.2V regulator node.
> +- vdda-0p9-supply:		phandle to vdda 0.9V regulator node.
> +- qcom,aux-cfg0-settings:		Specifies the DP AUX configuration 0 settings. The first
> +					entry in this array corresponds to the register offset
> +					within DP AUX, while the remaining entries indicate the
> +					programmable values.
> +- qcom,aux-cfg1-settings:		Specifies the DP AUX configuration 1 settings. The first
> +					entry in this array corresponds to the register offset
> +					within DP AUX, while the remaining entries indicate the
> +					programmable values.
> +- qcom,aux-cfg2-settings:		Specifies the DP AUX configuration 2 settings. The first
> +					entry in this array corresponds to the register offset
> +					within DP AUX, while the remaining entries indicate the
> +					programmable values.
> +- qcom,aux-cfg3-settings:		Specifies the DP AUX configuration 3 settings. The first
> +					entry in this array corresponds to the register offset
> +					within DP AUX, while the remaining entries indicate the
> +					programmable values.
> +- qcom,aux-cfg4-settings:		Specifies the DP AUX configuration 4 settings. The first
> +					entry in this array corresponds to the register offset
> +					within DP AUX, while the remaining entries indicate the
> +					programmable values.
> +- qcom,aux-cfg5-settings:		Specifies the DP AUX configuration 5 settings. The first
> +					entry in this array corresponds to the register offset
> +					within DP AUX, while the remaining entries indicate the
> +					programmable values.
> +- qcom,aux-cfg6-settings:		Specifies the DP AUX configuration 6 settings. The first
> +					entry in this array corresponds to the register offset
> +					within DP AUX, while the remaining entries indicate the
> +					programmable values.
> +- qcom,aux-cfg7-settings:		Specifies the DP AUX configuration 7 settings. The first
> +					entry in this array corresponds to the register offset
> +					within DP AUX, while the remaining entries indicate the
> +					programmable values.
> +- qcom,aux-cfg8-settings:		Specifies the DP AUX configuration 8 settings. The first
> +					entry in this array corresponds to the register offset
> +					within DP AUX, while the remaining entries indicate the
> +					programmable values.
> +- qcom,aux-cfg9-settings:		Specifies the DP AUX configuration 9 settings. The first
> +					entry in this array corresponds to the register offset
> +					within DP AUX, while the remaining entries indicate the
> +					programmable values.


What about the AUX configuration are these configuring? What are the permissible
values? If they're not describing the hardware, they probably don't belong
here.

> +- qcom,max-pclk-frequency-khz:	An integer specifying the maximum. pixel clock in KHz supported by Display Port.
> +- extcon:				Phandle for the external connector class interface.
> +- qcom,<type>-supply-entries:		A node that lists the elements of the supply used by the a particular "type" of DP module. The module "types"
> +					can be "core", "ctrl", and "phy". Within the same type,
> +					there can be more than one instance of this binding,
> +					in which case the entry would be appended with the
> +					supply entry index.
> +					e.g. qcom,ctrl-supply-entry@0
> +					-- qcom,supply-name: name of the supply (vdd/vdda/vddio)
> +					-- qcom,supply-min-voltage: minimum voltage level (uV)
> +					-- qcom,supply-max-voltage: maximum voltage level (uV)
> +					-- qcom,supply-enable-load: load drawn (uA) from enabled supply
> +					-- qcom,supply-disable-load: load drawn (uA) from disabled supply
> +					-- qcom,supply-pre-on-sleep: time to sleep (ms) before turning on
> +					-- qcom,supply-post-on-sleep: time to sleep (ms) after turning on
> +					-- qcom,supply-pre-off-sleep: time to sleep (ms) before turning off
> +					-- qcom,supply-post-off-sleep: time to sleep (ms) after turning off
> +- pinctrl-names:	List of names to assign mdss pin states defined in pinctrl device node
> +					Refer to pinctrl-bindings.txt
> +- pinctrl-<0..n>:	Lists phandles each pointing to the pin configuration node within a pin
> +					controller. These pin configurations are installed in the pinctrl
> +					device node. Refer to pinctrl-bindings.txt
> +DP Endpoint properties:
> +  - 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. See [2] for
> +    device graph info.
> +
> +Optional properties:
> +- qcom,aux-en-gpio:		Specifies the aux-channel enable gpio.
> +- qcom,aux-sel-gpio:		Specifies the aux-channel select gpio.

What does the select gpio do? Could you please describe it?

> +
> +
> +DP PLL: Required properties:
> +- compatible:           Should be "qcom,dp-pll-10nm".
> +- reg:                  Base address and length of DP hardware's memory mapped regions.
> +- cell-index:           Specifies the PLL instance.
> +- reg-names:            A list of strings that name the list of regs.
> +			"pll_base" - DP PLL memory region.
> +			"phy_base" - DP PHY memory region.
> +			"ln_tx0_base" - USB3 DP PHY combo TX-0 lane memory region.
> +			"ln_tx1_base" - USB3 DP PHY combo TX-1 lane memory region.
> +			"gdsc_base" - gdsc memory region.
> +- interrupt-parent	phandle to the interrupt parent device node.
> +- interrupts:		The interrupt signal from the DP block.
> +- clocks:               Clocks required for Display Port operation. See [1] for details on clock bindings.
> +- clock-names:          Names of the clocks corresponding to handles. Following clocks are required:
> +			"iface_clk", "ref_clk", cfg_ahb_clk", "pipe_clk".
> +- clock-rate:           Initial clock rate to be configured. For the shared clocks, the default value			     is set to zero so that minimum clock value is configured. Non-zero clock
> +			value can be used to configure DP pixel clock.
> +
> +
> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +[2] Documentation/devicetree/bindings/graph.txt
> +
> +Example:
> +	msm_dp: dp_display@ae90000{
> +		cell-index = <0>;
> +		compatible = "qcom,dp-display";
> +
> +		reg =   <0 0x90000 0x0dc>,
> +			<0 0x90200 0x0c0>,
> +			<0 0x90400 0x508>,
> +			<0 0x90a00 0x094>,
> +			<1 0xeaa00 0x200>,
> +			<1 0xea200 0x200>,
> +			<1 0xea600 0x200>,
> +			<2 0x02000 0x1a0>,
> +			<3 0x00000 0x621c>,
> +			<1 0xea000 0x180>,
> +			<1 0xe8000 0x20>,
> +			<4 0xe1000 0x034>;
> +		reg-names = "dp_ahb", "dp_aux", "dp_link",
> +			"dp_p0", "dp_phy", "dp_ln_tx0", "dp_ln_tx1",
> +			"dp_mmss_cc", "qfprom_physical", "dp_pll",
> +			"usb3_dp_com", "hdcp_physical";
> +
> +		interrupt-parent = <&mdss>;
> +		interrupts = <12 0>;
> +
> +		extcon = <&usb_1_ssphy>;
> +		clocks =  <&dispcc DISP_CC_MDSS_DP_AUX_CLK>,
> +			<&rpmhcc RPMH_CXO_CLK>,
> +			<&gcc GCC_USB3_PRIM_CLKREF_CLK>,
> +			<&gcc GCC_USB_PHY_CFG_AHB2PHY_CLK>,
> +			<&gcc GCC_USB3_PRIM_PHY_PIPE_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_CRYPTO_CLK>,
> +			<&dispcc DISP_CC_MDSS_DP_PIXEL_CLK_SRC>;
> +		clock-names = "core_aux_clk", "core_ref_clk_src",
> +			"core_usb_ref_clk", "core_usb_cfg_ahb_clk",
> +			"core_usb_pipe_clk", "ctrl_link_clk",
> +			"ctrl_link_iface_clk", "ctrl_pixel_clk",
> +			"crypto_clk", "pixel_clk_rcg";
> +
> +		pll-node = <&dp_pll>;
> +		qcom,aux-cfg0-settings = [20 00];
> +		qcom,aux-cfg1-settings = [24 13 23 1d];
> +		qcom,aux-cfg2-settings = [28 24];
> +		qcom,aux-cfg3-settings = [2c 00];
> +		qcom,aux-cfg4-settings = [30 0a];
> +		qcom,aux-cfg5-settings = [34 26];
> +		qcom,aux-cfg6-settings = [38 0a];
> +		qcom,aux-cfg7-settings = [3c 03];
> +		qcom,aux-cfg8-settings = [40 bb];
> +		qcom,aux-cfg9-settings = [44 03];
> +
> +		qcom,max-pclk-frequency-khz = <675000>;
> +
> +		qcom,ctrl-supply-entries {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			qcom,ctrl-supply-entry@0 {
> +				reg = <0>;
> +				qcom,supply-name = "vdda-1p2";
> +				qcom,supply-min-voltage = <1200000>;
> +				qcom,supply-max-voltage = <1200000>;
> +				qcom,supply-enable-load = <21800>;
> +				qcom,supply-disable-load = <4>;
> +			};
> +		};
> +
> +		qcom,phy-supply-entries {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			qcom,phy-supply-entry@0 {
> +				reg = <0>;
> +				qcom,supply-name = "vdda-0p9";
> +				qcom,supply-min-voltage = <880000>;
> +				qcom,supply-max-voltage = <880000>;
> +				qcom,supply-enable-load = <36000>;
> +				qcom,supply-disable-load = <32>;
> +			};
> +		};
> +
> +		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 {
> +				};
> +			};
> +		};
> +	};
> +
> +	dp_pll: dp-pll@c011000 {
> +		compatible = "qcom,dp-pll-10nm";
> +		label = "DP PLL";
> +		cell-index = <0>;
> +		#clock-cells = <1>;
> +
> +		reg = <1 0xea000 0x200>,
> +		      <1 0xeaa00 0x200>,
> +		      <1 0xea200 0x200>,
> +		      <1 0xea600 0x200>,
> +		      <2 0x03000 0x8>;
> +		reg-names = "pll_base", "phy_base", "ln_tx0_base",
> +			"ln_tx1_base", "gdsc_base";
> +
> +		clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
> +			 <&gcc GCC_USB3_PRIM_CLKREF_CLK>,
> +			 <&gcc GCC_USB_PHY_CFG_AHB2PHY_CLK>,
> +			 <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
> +		clock-names = "iface_clk", "ref_clk",
> +			"cfg_ahb_clk", "pipe_clk";
> +		clock-rate = <0>;
> +
> +	};
> diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt
> index ad2e883..ab2d1f6 100644
> --- a/Documentation/devicetree/bindings/display/msm/dpu.txt
> +++ b/Documentation/devicetree/bindings/display/msm/dpu.txt
> @@ -58,8 +58,9 @@ Required properties:
>  	Documentation/devicetree/bindings/graph.txt
>  	Documentation/devicetree/bindings/media/video-interfaces.txt
>  
> -	Port 0 -> DPU_INTF1 (DSI1)
> -	Port 1 -> DPU_INTF2 (DSI2)
> +	Port 0 -> DPU_INTF0 (DP)
> +	Port 1 -> DPU_INTF1 (DSI1)
> +	Port 2 -> DPU_INTF2 (DSI2)
>  
>  Optional properties:
>  - assigned-clocks: list of clock specifiers for clocks needing rate assignment
> @@ -115,13 +116,20 @@ Example:
>  
>  				port@0 {
>  					reg = <0>;
> -					dpu_intf1_out: endpoint {
> -						remote-endpoint = <&dsi0_in>;
> +					dpu_intf0_out: endpoint {
> +						remote-endpoint = <&dp_in>;
>  					};
>  				};
>  
>  				port@1 {
>  					reg = <1>;
> +					dpu_intf1_out: endpoint {
> +						remote-endpoint = <&dsi0_in>;
> +					};
> +				};
> +
> +				port@2 {
> +					reg = <2>;
>  					dpu_intf2_out: endpoint {
>  						remote-endpoint = <&dsi1_in>;
>  					};
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
Stephen Boyd Nov. 6, 2018, 11:45 p.m. UTC | #2
Quoting Chandan Uddaraju (2018-10-10 10:15:57)
> diff --git a/Documentation/devicetree/bindings/display/msm/dp.txt b/Documentation/devicetree/bindings/display/msm/dp.txt
> new file mode 100644
> index 0000000..0155266
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/msm/dp.txt
> @@ -0,0 +1,249 @@
> +Qualcomm Technologies, Inc.
> +DP is the master Display Port device which supports DP host controllers that are compatible with VESA Display Port interface specification.
> +DP Controller: Required properties:
> +- compatible:           Should be "qcom,dp-display".
> +- reg:                  Base address and length of DP hardware's memory mapped regions.
> +- cell-index:           Specifies the controller instance.
> +- reg-names:            A list of strings that name the list of regs.

This is a lot of register regions! There are probably multiple devices
in here.

> +                       "dp_ahb" - DP controller memory region.
> +                       "dp_aux" - DP AUX memory region.
> +                       "dp_link" - DP link layer memory region.
> +                       "dp_p0" - DP pixel clock domain memory region.
> +                       "dp_phy" - DP PHY memory region.

So a phy driver? Should be different binding and device than the DP
controller?

> +                       "dp_ln_tx0" - USB3 DP PHY combo TX-0 lane memory region.
> +                       "dp_ln_tx1" - USB3 DP PHY combo TX-1 lane memory region.

Presumably part of the phy device. Again, different binding?

> +                       "dp_mmss_cc" - Display Clock Control memory region.

No. This shouldn't be reaching into the display clock controller region.
That should be it's own node and device driver.

> +                       "qfprom_physical" - QFPROM Phys memory region.

qfprom is accessed through nvmem framework, please use it instead of
remapping it and reading random bits directly from your driver.

> +                       "dp_pll" - USB3 DP combo PLL memory region.
> +                       "usb3_dp_com" - USB3 DP PHY combo memory region.

I guess this would go with the DP phy node?

> +                       "hdcp_physical" - DP HDCP memory region.

Goes with the main DP device?

Please drop the full stop from all of these lines.

> +- interrupt-parent     phandle to the interrupt parent device node.

We don't need this sort of duplicate information. If it's part of
standard spec and not specific to this binding it can be left out.

> +- interrupts:          The interrupt signal from the DP block.
> +- clocks:               Clocks required for Display Port operation. See [1] for details on clock bindings.
> +- clock-names:          Names of the clocks corresponding to handles. Following clocks are required:
> +                       "core_aux_clk", "core_usb_ref_clk_src","core_usb_ref_clk", "core_usb_cfg_ahb_clk",
> +                       "core_usb_pipe_clk", "ctrl_link_clk", "ctrl_link_iface_clk", "ctrl_crypto_clk",
> +                       "ctrl_pixel_clk", "pixel_clk_rcg", "pixel_parent".

Please remove "_clk" from all your clock-names. It's redundant and makes
things harder to read.

> +- pll-node:            phandle to DP PLL node.

Huh?

> +- vdda-1p2-supply:             phandle to vdda 1.2V regulator node.
> +- vdda-0p9-supply:             phandle to vdda 0.9V regulator node.

Perhaps these have other names besides vdda? Maybe vdda-pll and vdda?
I'd also drop the 1p2 and 0p9 from them and just describe that in
freeform in the binding.

> +- qcom,aux-cfg0-settings:              Specifies the DP AUX configuration 0 settings. The first
> +                                       entry in this array corresponds to the register offset
> +                                       within DP AUX, while the remaining entries indicate the
> +                                       programmable values.
> +- qcom,aux-cfg1-settings:              Specifies the DP AUX configuration 1 settings. The first
> +                                       entry in this array corresponds to the register offset
> +                                       within DP AUX, while the remaining entries indicate the
> +                                       programmable values.
> +- qcom,aux-cfg2-settings:              Specifies the DP AUX configuration 2 settings. The first
> +                                       entry in this array corresponds to the register offset
> +                                       within DP AUX, while the remaining entries indicate the
> +                                       programmable values.
> +- qcom,aux-cfg3-settings:              Specifies the DP AUX configuration 3 settings. The first
> +                                       entry in this array corresponds to the register offset
> +                                       within DP AUX, while the remaining entries indicate the
> +                                       programmable values.
> +- qcom,aux-cfg4-settings:              Specifies the DP AUX configuration 4 settings. The first
> +                                       entry in this array corresponds to the register offset
> +                                       within DP AUX, while the remaining entries indicate the
> +                                       programmable values.
> +- qcom,aux-cfg5-settings:              Specifies the DP AUX configuration 5 settings. The first
> +                                       entry in this array corresponds to the register offset
> +                                       within DP AUX, while the remaining entries indicate the
> +                                       programmable values.
> +- qcom,aux-cfg6-settings:              Specifies the DP AUX configuration 6 settings. The first
> +                                       entry in this array corresponds to the register offset
> +                                       within DP AUX, while the remaining entries indicate the
> +                                       programmable values.
> +- qcom,aux-cfg7-settings:              Specifies the DP AUX configuration 7 settings. The first
> +                                       entry in this array corresponds to the register offset
> +                                       within DP AUX, while the remaining entries indicate the
> +                                       programmable values.
> +- qcom,aux-cfg8-settings:              Specifies the DP AUX configuration 8 settings. The first
> +                                       entry in this array corresponds to the register offset
> +                                       within DP AUX, while the remaining entries indicate the
> +                                       programmable values.
> +- qcom,aux-cfg9-settings:              Specifies the DP AUX configuration 9 settings. The first
> +                                       entry in this array corresponds to the register offset
> +                                       within DP AUX, while the remaining entries indicate the
> +                                       programmable values.

Look like magic bits. Can they just be thrown into the driver? Or
specified in some way that us mere mortals know what to put into these
bits.

> +- qcom,max-pclk-frequency-khz: An integer specifying the maximum. pixel clock in KHz supported by Display Port.

Why is this in the binding? It isn't known based on the device
compatible string?

> +- extcon:                              Phandle for the external connector class interface.

Is this for typec? Would be good to say what it's for.

> +- qcom,<type>-supply-entries:          A node that lists the elements of the supply used by the a particular "type" of DP module. The module "types"
> +                                       can be "core", "ctrl", and "phy". Within the same type,
> +                                       there can be more than one instance of this binding,
> +                                       in which case the entry would be appended with the
> +                                       supply entry index.
> +                                       e.g. qcom,ctrl-supply-entry@0
> +                                       -- qcom,supply-name: name of the supply (vdd/vdda/vddio)
> +                                       -- qcom,supply-min-voltage: minimum voltage level (uV)
> +                                       -- qcom,supply-max-voltage: maximum voltage level (uV)
> +                                       -- qcom,supply-enable-load: load drawn (uA) from enabled supply
> +                                       -- qcom,supply-disable-load: load drawn (uA) from disabled supply
> +                                       -- qcom,supply-pre-on-sleep: time to sleep (ms) before turning on
> +                                       -- qcom,supply-post-on-sleep: time to sleep (ms) after turning on
> +                                       -- qcom,supply-pre-off-sleep: time to sleep (ms) before turning off
> +                                       -- qcom,supply-post-off-sleep: time to sleep (ms) after turning off

All these properties should go away. min/max should be handled by board
constraints. Sleep times should be handled by regulator framework for
those particular regulators. Maybe the load would be important, but I'm
still sort of lost how that wouldn't be expressed through some SoC
compatible string for the device and then hardcoded into the driver.

> +- pinctrl-names:       List of names to assign mdss pin states defined in pinctrl device node
> +                                       Refer to pinctrl-bindings.txt
> +- pinctrl-<0..n>:      Lists phandles each pointing to the pin configuration node within a pin
> +                                       controller. These pin configurations are installed in the pinctrl
> +                                       device node. Refer to pinctrl-bindings.txt

This isn't needed in the binding. Or at least, I don't see where it's
expressed in the example, so probably just noise?

> +DP Endpoint properties:
> +  - 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. See [2] for
> +    device graph info.
> +
> +Optional properties:
> +- qcom,aux-en-gpio:            Specifies the aux-channel enable gpio.
> +- qcom,aux-sel-gpio:           Specifies the aux-channel select gpio.

These should follow standard 'gpios' properties, like aux-enable-gpios
and aux-select-gpios. I guess these would need pinctrl muxing too.

> +
> +
> +DP PLL: Required properties:
> +- compatible:           Should be "qcom,dp-pll-10nm".
> +- reg:                  Base address and length of DP hardware's memory mapped regions.
> +- cell-index:           Specifies the PLL instance.
> +- reg-names:            A list of strings that name the list of regs.
> +                       "pll_base" - DP PLL memory region.
> +                       "phy_base" - DP PHY memory region.
> +                       "ln_tx0_base" - USB3 DP PHY combo TX-0 lane memory region.
> +                       "ln_tx1_base" - USB3 DP PHY combo TX-1 lane memory region.
> +                       "gdsc_base" - gdsc memory region.

Why a gdsc_base? GDSCs should be handled by power domains?

> +- interrupt-parent     phandle to the interrupt parent device node.

Again, drop this one.

> +- interrupts:          The interrupt signal from the DP block.
> +- clocks:               Clocks required for Display Port operation. See [1] for details on clock bindings.
> +- clock-names:          Names of the clocks corresponding to handles. Following clocks are required:
> +                       "iface_clk", "ref_clk", cfg_ahb_clk", "pipe_clk".

Again, drop "_clk" please.

> +- clock-rate:           Initial clock rate to be configured. For the shared clocks, the default value                       is set to zero so that minimum clock value is configured. Non-zero clock
> +                       value can be used to configure DP pixel clock.

We have assigned-clock-rates for this. Can you use that?

> +
> +
> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +[2] Documentation/devicetree/bindings/graph.txt
> +
> +Example:
> +       msm_dp: dp_display@ae90000{
> +               cell-index = <0>;

Is this needed?

> +               compatible = "qcom,dp-display";
> +
> +               reg =   <0 0x90000 0x0dc>,
> +                       <0 0x90200 0x0c0>,
> +                       <0 0x90400 0x508>,
> +                       <0 0x90a00 0x094>,

Looks like one device with one reg property:

		<0x90000 0x1000>

> +                       <1 0xeaa00 0x200>,
> +                       <1 0xea200 0x200>,
> +                       <1 0xea600 0x200>,

Looks like another device with one reg property:

		<0xea000 0x1000>;

> +                       <2 0x02000 0x1a0>,
> +                       <3 0x00000 0x621c>,
> +                       <1 0xea000 0x180>,

This is part of the one above.

> +                       <1 0xe8000 0x20>,

Well maybe this is also part of the DP phy? Then it would start
somewhere a little earlier I supposed.

> +                       <4 0xe1000 0x034>;
> +               reg-names = "dp_ahb", "dp_aux", "dp_link",
> +                       "dp_p0", "dp_phy", "dp_ln_tx0", "dp_ln_tx1",
> +                       "dp_mmss_cc", "qfprom_physical", "dp_pll",
> +                       "usb3_dp_com", "hdcp_physical";
> +
> +               interrupt-parent = <&mdss>;
> +               interrupts = <12 0>;

Why is this interrupt-parent stuff being done?

> +
> +               extcon = <&usb_1_ssphy>;
> +               clocks =  <&dispcc DISP_CC_MDSS_DP_AUX_CLK>,
> +                       <&rpmhcc RPMH_CXO_CLK>,
> +                       <&gcc GCC_USB3_PRIM_CLKREF_CLK>,
> +                       <&gcc GCC_USB_PHY_CFG_AHB2PHY_CLK>,
> +                       <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>,

It's really odd that this device needs USB phy clocks. Is it really
necessary to do this?

> +                       <&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_CRYPTO_CLK>,
> +                       <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK_SRC>;
> +               clock-names = "core_aux_clk", "core_ref_clk_src",
> +                       "core_usb_ref_clk", "core_usb_cfg_ahb_clk",
> +                       "core_usb_pipe_clk", "ctrl_link_clk",
> +                       "ctrl_link_iface_clk", "ctrl_pixel_clk",
> +                       "crypto_clk", "pixel_clk_rcg";
> +
> +               pll-node = <&dp_pll>;
> +               qcom,aux-cfg0-settings = [20 00];
> +               qcom,aux-cfg1-settings = [24 13 23 1d];
> +               qcom,aux-cfg2-settings = [28 24];
> +               qcom,aux-cfg3-settings = [2c 00];
> +               qcom,aux-cfg4-settings = [30 0a];
> +               qcom,aux-cfg5-settings = [34 26];
> +               qcom,aux-cfg6-settings = [38 0a];
> +               qcom,aux-cfg7-settings = [3c 03];
> +               qcom,aux-cfg8-settings = [40 bb];
> +               qcom,aux-cfg9-settings = [44 03];
> +
> +               qcom,max-pclk-frequency-khz = <675000>;
> +
> +               qcom,ctrl-supply-entries {
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +
> +                       qcom,ctrl-supply-entry@0 {
> +                               reg = <0>;
> +                               qcom,supply-name = "vdda-1p2";
> +                               qcom,supply-min-voltage = <1200000>;
> +                               qcom,supply-max-voltage = <1200000>;
> +                               qcom,supply-enable-load = <21800>;
> +                               qcom,supply-disable-load = <4>;
> +                       };
> +               };
> +
> +               qcom,phy-supply-entries {
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +
> +                       qcom,phy-supply-entry@0 {
> +                               reg = <0>;
> +                               qcom,supply-name = "vdda-0p9";
> +                               qcom,supply-min-voltage = <880000>;
> +                               qcom,supply-max-voltage = <880000>;
> +                               qcom,supply-enable-load = <36000>;
> +                               qcom,supply-disable-load = <32>;
> +                       };
> +               };

NAK on the above two nodes.

> +
> +               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 {
> +                               };
> +                       };
> +               };
> +       };
> +
> +       dp_pll: dp-pll@c011000 {

This doesn't even match reg property.

> +               compatible = "qcom,dp-pll-10nm";
> +               label = "DP PLL";
> +               cell-index = <0>;
> +               #clock-cells = <1>;
> +
> +               reg = <1 0xea000 0x200>,
> +                     <1 0xeaa00 0x200>,
> +                     <1 0xea200 0x200>,
> +                     <1 0xea600 0x200>,
> +                     <2 0x03000 0x8>;

Why 1 and 2? What's going on in this example.

> +               reg-names = "pll_base", "phy_base", "ln_tx0_base",
> +                       "ln_tx1_base", "gdsc_base";

Don't think we really need "_base" on these reg-names either. Seems
redundant.

> +
> +               clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
> +                        <&gcc GCC_USB3_PRIM_CLKREF_CLK>,
> +                        <&gcc GCC_USB_PHY_CFG_AHB2PHY_CLK>,
> +                        <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
> +               clock-names = "iface_clk", "ref_clk",
> +                       "cfg_ahb_clk", "pipe_clk";
> +               clock-rate = <0>;
> +
> +       };
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/msm/dp.txt b/Documentation/devicetree/bindings/display/msm/dp.txt
new file mode 100644
index 0000000..0155266
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/msm/dp.txt
@@ -0,0 +1,249 @@ 
+Qualcomm Technologies, Inc.
+DP is the master Display Port device which supports DP host controllers that are compatible with VESA Display Port interface specification.
+DP Controller: Required properties:
+- compatible:           Should be "qcom,dp-display".
+- reg:                  Base address and length of DP hardware's memory mapped regions.
+- cell-index:           Specifies the controller instance.
+- reg-names:            A list of strings that name the list of regs.
+			"dp_ahb" - DP controller memory region.
+			"dp_aux" - DP AUX memory region.
+			"dp_link" - DP link layer memory region.
+			"dp_p0" - DP pixel clock domain memory region.
+			"dp_phy" - DP PHY memory region.
+			"dp_ln_tx0" - USB3 DP PHY combo TX-0 lane memory region.
+			"dp_ln_tx1" - USB3 DP PHY combo TX-1 lane memory region.
+			"dp_mmss_cc" - Display Clock Control memory region.
+			"qfprom_physical" - QFPROM Phys memory region.
+			"dp_pll" - USB3 DP combo PLL memory region.
+			"usb3_dp_com" - USB3 DP PHY combo memory region.
+			"hdcp_physical" - DP HDCP memory region.
+- interrupt-parent	phandle to the interrupt parent device node.
+- interrupts:		The interrupt signal from the DP block.
+- clocks:               Clocks required for Display Port operation. See [1] for details on clock bindings.
+- clock-names:          Names of the clocks corresponding to handles. Following clocks are required:
+			"core_aux_clk", "core_usb_ref_clk_src","core_usb_ref_clk", "core_usb_cfg_ahb_clk",
+			"core_usb_pipe_clk", "ctrl_link_clk", "ctrl_link_iface_clk", "ctrl_crypto_clk",
+			"ctrl_pixel_clk", "pixel_clk_rcg", "pixel_parent".
+- pll-node:		phandle to DP PLL node.
+- vdda-1p2-supply:		phandle to vdda 1.2V regulator node.
+- vdda-0p9-supply:		phandle to vdda 0.9V regulator node.
+- qcom,aux-cfg0-settings:		Specifies the DP AUX configuration 0 settings. The first
+					entry in this array corresponds to the register offset
+					within DP AUX, while the remaining entries indicate the
+					programmable values.
+- qcom,aux-cfg1-settings:		Specifies the DP AUX configuration 1 settings. The first
+					entry in this array corresponds to the register offset
+					within DP AUX, while the remaining entries indicate the
+					programmable values.
+- qcom,aux-cfg2-settings:		Specifies the DP AUX configuration 2 settings. The first
+					entry in this array corresponds to the register offset
+					within DP AUX, while the remaining entries indicate the
+					programmable values.
+- qcom,aux-cfg3-settings:		Specifies the DP AUX configuration 3 settings. The first
+					entry in this array corresponds to the register offset
+					within DP AUX, while the remaining entries indicate the
+					programmable values.
+- qcom,aux-cfg4-settings:		Specifies the DP AUX configuration 4 settings. The first
+					entry in this array corresponds to the register offset
+					within DP AUX, while the remaining entries indicate the
+					programmable values.
+- qcom,aux-cfg5-settings:		Specifies the DP AUX configuration 5 settings. The first
+					entry in this array corresponds to the register offset
+					within DP AUX, while the remaining entries indicate the
+					programmable values.
+- qcom,aux-cfg6-settings:		Specifies the DP AUX configuration 6 settings. The first
+					entry in this array corresponds to the register offset
+					within DP AUX, while the remaining entries indicate the
+					programmable values.
+- qcom,aux-cfg7-settings:		Specifies the DP AUX configuration 7 settings. The first
+					entry in this array corresponds to the register offset
+					within DP AUX, while the remaining entries indicate the
+					programmable values.
+- qcom,aux-cfg8-settings:		Specifies the DP AUX configuration 8 settings. The first
+					entry in this array corresponds to the register offset
+					within DP AUX, while the remaining entries indicate the
+					programmable values.
+- qcom,aux-cfg9-settings:		Specifies the DP AUX configuration 9 settings. The first
+					entry in this array corresponds to the register offset
+					within DP AUX, while the remaining entries indicate the
+					programmable values.
+- qcom,max-pclk-frequency-khz:	An integer specifying the maximum. pixel clock in KHz supported by Display Port.
+- extcon:				Phandle for the external connector class interface.
+- qcom,<type>-supply-entries:		A node that lists the elements of the supply used by the a particular "type" of DP module. The module "types"
+					can be "core", "ctrl", and "phy". Within the same type,
+					there can be more than one instance of this binding,
+					in which case the entry would be appended with the
+					supply entry index.
+					e.g. qcom,ctrl-supply-entry@0
+					-- qcom,supply-name: name of the supply (vdd/vdda/vddio)
+					-- qcom,supply-min-voltage: minimum voltage level (uV)
+					-- qcom,supply-max-voltage: maximum voltage level (uV)
+					-- qcom,supply-enable-load: load drawn (uA) from enabled supply
+					-- qcom,supply-disable-load: load drawn (uA) from disabled supply
+					-- qcom,supply-pre-on-sleep: time to sleep (ms) before turning on
+					-- qcom,supply-post-on-sleep: time to sleep (ms) after turning on
+					-- qcom,supply-pre-off-sleep: time to sleep (ms) before turning off
+					-- qcom,supply-post-off-sleep: time to sleep (ms) after turning off
+- pinctrl-names:	List of names to assign mdss pin states defined in pinctrl device node
+					Refer to pinctrl-bindings.txt
+- pinctrl-<0..n>:	Lists phandles each pointing to the pin configuration node within a pin
+					controller. These pin configurations are installed in the pinctrl
+					device node. Refer to pinctrl-bindings.txt
+DP Endpoint properties:
+  - 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. See [2] for
+    device graph info.
+
+Optional properties:
+- qcom,aux-en-gpio:		Specifies the aux-channel enable gpio.
+- qcom,aux-sel-gpio:		Specifies the aux-channel select gpio.
+
+
+DP PLL: Required properties:
+- compatible:           Should be "qcom,dp-pll-10nm".
+- reg:                  Base address and length of DP hardware's memory mapped regions.
+- cell-index:           Specifies the PLL instance.
+- reg-names:            A list of strings that name the list of regs.
+			"pll_base" - DP PLL memory region.
+			"phy_base" - DP PHY memory region.
+			"ln_tx0_base" - USB3 DP PHY combo TX-0 lane memory region.
+			"ln_tx1_base" - USB3 DP PHY combo TX-1 lane memory region.
+			"gdsc_base" - gdsc memory region.
+- interrupt-parent	phandle to the interrupt parent device node.
+- interrupts:		The interrupt signal from the DP block.
+- clocks:               Clocks required for Display Port operation. See [1] for details on clock bindings.
+- clock-names:          Names of the clocks corresponding to handles. Following clocks are required:
+			"iface_clk", "ref_clk", cfg_ahb_clk", "pipe_clk".
+- clock-rate:           Initial clock rate to be configured. For the shared clocks, the default value			     is set to zero so that minimum clock value is configured. Non-zero clock
+			value can be used to configure DP pixel clock.
+
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+[2] Documentation/devicetree/bindings/graph.txt
+
+Example:
+	msm_dp: dp_display@ae90000{
+		cell-index = <0>;
+		compatible = "qcom,dp-display";
+
+		reg =   <0 0x90000 0x0dc>,
+			<0 0x90200 0x0c0>,
+			<0 0x90400 0x508>,
+			<0 0x90a00 0x094>,
+			<1 0xeaa00 0x200>,
+			<1 0xea200 0x200>,
+			<1 0xea600 0x200>,
+			<2 0x02000 0x1a0>,
+			<3 0x00000 0x621c>,
+			<1 0xea000 0x180>,
+			<1 0xe8000 0x20>,
+			<4 0xe1000 0x034>;
+		reg-names = "dp_ahb", "dp_aux", "dp_link",
+			"dp_p0", "dp_phy", "dp_ln_tx0", "dp_ln_tx1",
+			"dp_mmss_cc", "qfprom_physical", "dp_pll",
+			"usb3_dp_com", "hdcp_physical";
+
+		interrupt-parent = <&mdss>;
+		interrupts = <12 0>;
+
+		extcon = <&usb_1_ssphy>;
+		clocks =  <&dispcc DISP_CC_MDSS_DP_AUX_CLK>,
+			<&rpmhcc RPMH_CXO_CLK>,
+			<&gcc GCC_USB3_PRIM_CLKREF_CLK>,
+			<&gcc GCC_USB_PHY_CFG_AHB2PHY_CLK>,
+			<&gcc GCC_USB3_PRIM_PHY_PIPE_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_CRYPTO_CLK>,
+			<&dispcc DISP_CC_MDSS_DP_PIXEL_CLK_SRC>;
+		clock-names = "core_aux_clk", "core_ref_clk_src",
+			"core_usb_ref_clk", "core_usb_cfg_ahb_clk",
+			"core_usb_pipe_clk", "ctrl_link_clk",
+			"ctrl_link_iface_clk", "ctrl_pixel_clk",
+			"crypto_clk", "pixel_clk_rcg";
+
+		pll-node = <&dp_pll>;
+		qcom,aux-cfg0-settings = [20 00];
+		qcom,aux-cfg1-settings = [24 13 23 1d];
+		qcom,aux-cfg2-settings = [28 24];
+		qcom,aux-cfg3-settings = [2c 00];
+		qcom,aux-cfg4-settings = [30 0a];
+		qcom,aux-cfg5-settings = [34 26];
+		qcom,aux-cfg6-settings = [38 0a];
+		qcom,aux-cfg7-settings = [3c 03];
+		qcom,aux-cfg8-settings = [40 bb];
+		qcom,aux-cfg9-settings = [44 03];
+
+		qcom,max-pclk-frequency-khz = <675000>;
+
+		qcom,ctrl-supply-entries {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			qcom,ctrl-supply-entry@0 {
+				reg = <0>;
+				qcom,supply-name = "vdda-1p2";
+				qcom,supply-min-voltage = <1200000>;
+				qcom,supply-max-voltage = <1200000>;
+				qcom,supply-enable-load = <21800>;
+				qcom,supply-disable-load = <4>;
+			};
+		};
+
+		qcom,phy-supply-entries {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			qcom,phy-supply-entry@0 {
+				reg = <0>;
+				qcom,supply-name = "vdda-0p9";
+				qcom,supply-min-voltage = <880000>;
+				qcom,supply-max-voltage = <880000>;
+				qcom,supply-enable-load = <36000>;
+				qcom,supply-disable-load = <32>;
+			};
+		};
+
+		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 {
+				};
+			};
+		};
+	};
+
+	dp_pll: dp-pll@c011000 {
+		compatible = "qcom,dp-pll-10nm";
+		label = "DP PLL";
+		cell-index = <0>;
+		#clock-cells = <1>;
+
+		reg = <1 0xea000 0x200>,
+		      <1 0xeaa00 0x200>,
+		      <1 0xea200 0x200>,
+		      <1 0xea600 0x200>,
+		      <2 0x03000 0x8>;
+		reg-names = "pll_base", "phy_base", "ln_tx0_base",
+			"ln_tx1_base", "gdsc_base";
+
+		clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
+			 <&gcc GCC_USB3_PRIM_CLKREF_CLK>,
+			 <&gcc GCC_USB_PHY_CFG_AHB2PHY_CLK>,
+			 <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
+		clock-names = "iface_clk", "ref_clk",
+			"cfg_ahb_clk", "pipe_clk";
+		clock-rate = <0>;
+
+	};
diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt
index ad2e883..ab2d1f6 100644
--- a/Documentation/devicetree/bindings/display/msm/dpu.txt
+++ b/Documentation/devicetree/bindings/display/msm/dpu.txt
@@ -58,8 +58,9 @@  Required properties:
 	Documentation/devicetree/bindings/graph.txt
 	Documentation/devicetree/bindings/media/video-interfaces.txt
 
-	Port 0 -> DPU_INTF1 (DSI1)
-	Port 1 -> DPU_INTF2 (DSI2)
+	Port 0 -> DPU_INTF0 (DP)
+	Port 1 -> DPU_INTF1 (DSI1)
+	Port 2 -> DPU_INTF2 (DSI2)
 
 Optional properties:
 - assigned-clocks: list of clock specifiers for clocks needing rate assignment
@@ -115,13 +116,20 @@  Example:
 
 				port@0 {
 					reg = <0>;
-					dpu_intf1_out: endpoint {
-						remote-endpoint = <&dsi0_in>;
+					dpu_intf0_out: endpoint {
+						remote-endpoint = <&dp_in>;
 					};
 				};
 
 				port@1 {
 					reg = <1>;
+					dpu_intf1_out: endpoint {
+						remote-endpoint = <&dsi0_in>;
+					};
+				};
+
+				port@2 {
+					reg = <2>;
 					dpu_intf2_out: endpoint {
 						remote-endpoint = <&dsi1_in>;
 					};