[PATCH] arm64: dts: qcom: sm8550-qrd: add display and panel

[PATCH] arm64: dts: qcom: sm8550-qrd: add display and panel

[Krzysztof Kozlowski]

Enable Display Subsystem with Visionox VTDR6130 Panel (same as on
MTP8550).

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

Context in the patch depends on:
1. https://lore.kernel.org/linux-arm-msm/20230516133011.108093-1-krzysztof.kozlowski@linaro.org/T/#t
2. https://lore.kernel.org/linux-arm-msm/20230512160452.206585-1-krzysztof.kozlowski@linaro.org/
---
 arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 76 +++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
index 30b36a149125..03bf6bc2db4d 100644
--- a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
+++ b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
@@ -420,6 +420,10 @@ vreg_l3g_1p2: ldo3 {
 	};
 };
 
+&dispcc {
+	status = "okay";
+};
+
 &gcc {
 	clocks = <&bi_tcxo_div2>, <&sleep_clk>,
 		 <&pcie0_phy>,
@@ -431,6 +435,50 @@ &gcc {
 		 <&usb_dp_qmpphy QMP_USB43DP_USB3_PIPE_CLK>;
 };
 
+&mdss {
+	status = "okay";
+};
+
+&mdss_dsi0 {
+	vdda-supply = <&vreg_l3e_1p2>;
+	status = "okay";
+
+	panel@0 {
+		compatible = "visionox,vtdr6130";
+		reg = <0>;
+
+		pinctrl-names = "default", "sleep";
+		pinctrl-0 = <&sde_dsi_active>, <&sde_te_active>;
+		pinctrl-1 = <&sde_dsi_suspend>, <&sde_te_suspend>;
+
+		vddio-supply = <&vreg_l12b_1p8>;
+		vci-supply = <&vreg_l13b_3p0>;
+		vdd-supply = <&vreg_l11b_1p2>;
+
+		reset-gpios = <&tlmm 133 GPIO_ACTIVE_LOW>;
+
+		port {
+			panel0_in: endpoint {
+				remote-endpoint = <&mdss_dsi0_out>;
+			};
+		};
+	};
+};
+
+&mdss_dsi0_out {
+	remote-endpoint = <&panel0_in>;
+	data-lanes = <0 1 2 3>;
+};
+
+&mdss_dsi0_phy {
+	vdds-supply = <&vreg_l1e_0p88>;
+	status = "okay";
+};
+
+&mdss_mdp {
+	status = "okay";
+};
+
 &pcie_1_phy_aux_clk {
 	status = "disabled";
 };
@@ -532,6 +580,34 @@ wcd_tx: codec@0,3 {
 &tlmm {
 	gpio-reserved-ranges = <32 8>;
 
+	sde_dsi_active: sde-dsi-active-state {
+		pins = "gpio133";
+		function = "gpio";
+		drive-strength = <8>;
+		bias-disable;
+	};
+
+	sde_dsi_suspend: sde-dsi-suspend-state {
+		pins = "gpio133";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-pull-down;
+	};
+
+	sde_te_active: sde-te-active-state {
+		pins = "gpio86";
+		function = "mdp_vsync";
+		drive-strength = <2>;
+		bias-pull-down;
+	};
+
+	sde_te_suspend: sde-te-suspend-state {
+		pins = "gpio86";
+		function = "mdp_vsync";
+		drive-strength = <2>;
+		bias-pull-down;
+	};
+
 	wcd_default: wcd-reset-n-active-state {
 		pins = "gpio108";
 		function = "gpio";
-- 
2.34.1

Re: [PATCH] arm64: dts: qcom: sm8550-qrd: add display and panel

[Konrad Dybcio]

On 16.05.2023 17:17, Krzysztof Kozlowski wrote:
> Enable Display Subsystem with Visionox VTDR6130 Panel (same as on
> MTP8550).
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---
> 
> Context in the patch depends on:
> 1. https://lore.kernel.org/linux-arm-msm/20230516133011.108093-1-krzysztof.kozlowski@linaro.org/T/#t
> 2. https://lore.kernel.org/linux-arm-msm/20230512160452.206585-1-krzysztof.kozlowski@linaro.org/
> ---
>  arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 76 +++++++++++++++++++++++++
>  1 file changed, 76 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
> index 30b36a149125..03bf6bc2db4d 100644
> --- a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
> @@ -420,6 +420,10 @@ vreg_l3g_1p2: ldo3 {
>  	};
>  };
>  
> +&dispcc {
> +	status = "okay";
> +};
Missed this in the bigpatchdrop review.. It makes no sense to keep
dispcc disabled by default (other than for lazily "solving" UEFI
framebuffer being shut down)

> +
>  &gcc {
>  	clocks = <&bi_tcxo_div2>, <&sleep_clk>,
>  		 <&pcie0_phy>,
> @@ -431,6 +435,50 @@ &gcc {
>  		 <&usb_dp_qmpphy QMP_USB43DP_USB3_PIPE_CLK>;
>  };
>  
> +&mdss {
> +	status = "okay";
> +};
> +
> +&mdss_dsi0 {
> +	vdda-supply = <&vreg_l3e_1p2>;
> +	status = "okay";
> +
> +	panel@0 {
> +		compatible = "visionox,vtdr6130";
> +		reg = <0>;
> +
> +		pinctrl-names = "default", "sleep";
> +		pinctrl-0 = <&sde_dsi_active>, <&sde_te_active>;
> +		pinctrl-1 = <&sde_dsi_suspend>, <&sde_te_suspend>;
property-n
property-names

> +
> +		vddio-supply = <&vreg_l12b_1p8>;
> +		vci-supply = <&vreg_l13b_3p0>;
> +		vdd-supply = <&vreg_l11b_1p2>;
> +
> +		reset-gpios = <&tlmm 133 GPIO_ACTIVE_LOW>;
> +
> +		port {
> +			panel0_in: endpoint {
> +				remote-endpoint = <&mdss_dsi0_out>;
> +			};
> +		};
> +	};
> +};
> +
> +&mdss_dsi0_out {
> +	remote-endpoint = <&panel0_in>;
> +	data-lanes = <0 1 2 3>;
> +};
> +
> +&mdss_dsi0_phy {
> +	vdds-supply = <&vreg_l1e_0p88>;
> +	status = "okay";
> +};
> +
> +&mdss_mdp {
> +	status = "okay";
> +};
This should also be enabled by default, MDSS is useless when MDP is
disabled.

lgtm otherwise

Konrad

> +
>  &pcie_1_phy_aux_clk {
>  	status = "disabled";
>  };
> @@ -532,6 +580,34 @@ wcd_tx: codec@0,3 {
>  &tlmm {
>  	gpio-reserved-ranges = <32 8>;
>  
> +	sde_dsi_active: sde-dsi-active-state {
> +		pins = "gpio133";
> +		function = "gpio";
> +		drive-strength = <8>;
> +		bias-disable;
> +	};
> +
> +	sde_dsi_suspend: sde-dsi-suspend-state {
> +		pins = "gpio133";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-pull-down;
> +	};
> +
> +	sde_te_active: sde-te-active-state {
> +		pins = "gpio86";
> +		function = "mdp_vsync";
> +		drive-strength = <2>;
> +		bias-pull-down;
> +	};
> +
> +	sde_te_suspend: sde-te-suspend-state {
> +		pins = "gpio86";
> +		function = "mdp_vsync";
> +		drive-strength = <2>;
> +		bias-pull-down;
> +	};
> +
>  	wcd_default: wcd-reset-n-active-state {
>  		pins = "gpio108";
>  		function = "gpio";

Re: [PATCH] arm64: dts: qcom: sm8550-qrd: add display and panel

[Krzysztof Kozlowski]

On 16/05/2023 17:20, Konrad Dybcio wrote:
> 
> 
> On 16.05.2023 17:17, Krzysztof Kozlowski wrote:
>> Enable Display Subsystem with Visionox VTDR6130 Panel (same as on
>> MTP8550).
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>
>> ---
>>
>> Context in the patch depends on:
>> 1. https://lore.kernel.org/linux-arm-msm/20230516133011.108093-1-krzysztof.kozlowski@linaro.org/T/#t
>> 2. https://lore.kernel.org/linux-arm-msm/20230512160452.206585-1-krzysztof.kozlowski@linaro.org/
>> ---
>>  arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 76 +++++++++++++++++++++++++
>>  1 file changed, 76 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
>> index 30b36a149125..03bf6bc2db4d 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
>> +++ b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
>> @@ -420,6 +420,10 @@ vreg_l3g_1p2: ldo3 {
>>  	};
>>  };
>>  
>> +&dispcc {
>> +	status = "okay";
>> +};
> Missed this in the bigpatchdrop review.. It makes no sense to keep
> dispcc disabled by default (other than for lazily "solving" UEFI
> framebuffer being shut down)

Sure.

> 
>> +
>>  &gcc {
>>  	clocks = <&bi_tcxo_div2>, <&sleep_clk>,
>>  		 <&pcie0_phy>,
>> @@ -431,6 +435,50 @@ &gcc {
>>  		 <&usb_dp_qmpphy QMP_USB43DP_USB3_PIPE_CLK>;
>>  };
>>  
>> +&mdss {
>> +	status = "okay";
>> +};
>> +
>> +&mdss_dsi0 {
>> +	vdda-supply = <&vreg_l3e_1p2>;
>> +	status = "okay";
>> +
>> +	panel@0 {
>> +		compatible = "visionox,vtdr6130";
>> +		reg = <0>;
>> +
>> +		pinctrl-names = "default", "sleep";
>> +		pinctrl-0 = <&sde_dsi_active>, <&sde_te_active>;
>> +		pinctrl-1 = <&sde_dsi_suspend>, <&sde_te_suspend>;
> property-n
> property-names

Sure, copy-pasta from MTP8550.

>> +
>> +&mdss_mdp {
>> +	status = "okay";
>> +};
> This should also be enabled by default, MDSS is useless when MDP is
> disabled.

But don't we want to disable both when display is not used (not connected)?

Best regards,
Krzysztof

Re: [PATCH] arm64: dts: qcom: sm8550-qrd: add display and panel

[Konrad Dybcio]

On 16.05.2023 17:23, Krzysztof Kozlowski wrote:
> On 16/05/2023 17:20, Konrad Dybcio wrote:
>>
>>
>> On 16.05.2023 17:17, Krzysztof Kozlowski wrote:
>>> Enable Display Subsystem with Visionox VTDR6130 Panel (same as on
>>> MTP8550).
>>>
>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>
>>> ---
>>>
>>> Context in the patch depends on:
>>> 1. https://lore.kernel.org/linux-arm-msm/20230516133011.108093-1-krzysztof.kozlowski@linaro.org/T/#t
>>> 2. https://lore.kernel.org/linux-arm-msm/20230512160452.206585-1-krzysztof.kozlowski@linaro.org/
>>> ---
>>>  arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 76 +++++++++++++++++++++++++
>>>  1 file changed, 76 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
>>> index 30b36a149125..03bf6bc2db4d 100644
>>> --- a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
>>> +++ b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
>>> @@ -420,6 +420,10 @@ vreg_l3g_1p2: ldo3 {
>>>  	};
>>>  };
>>>  
>>> +&dispcc {
>>> +	status = "okay";
>>> +};
>> Missed this in the bigpatchdrop review.. It makes no sense to keep
>> dispcc disabled by default (other than for lazily "solving" UEFI
>> framebuffer being shut down)
> 
> Sure.
> 
>>
>>> +
>>>  &gcc {
>>>  	clocks = <&bi_tcxo_div2>, <&sleep_clk>,
>>>  		 <&pcie0_phy>,
>>> @@ -431,6 +435,50 @@ &gcc {
>>>  		 <&usb_dp_qmpphy QMP_USB43DP_USB3_PIPE_CLK>;
>>>  };
>>>  
>>> +&mdss {
>>> +	status = "okay";
>>> +};
>>> +
>>> +&mdss_dsi0 {
>>> +	vdda-supply = <&vreg_l3e_1p2>;
>>> +	status = "okay";
>>> +
>>> +	panel@0 {
>>> +		compatible = "visionox,vtdr6130";
>>> +		reg = <0>;
>>> +
>>> +		pinctrl-names = "default", "sleep";
>>> +		pinctrl-0 = <&sde_dsi_active>, <&sde_te_active>;
>>> +		pinctrl-1 = <&sde_dsi_suspend>, <&sde_te_suspend>;
>> property-n
>> property-names
> 
> Sure, copy-pasta from MTP8550.
> 
>>> +
>>> +&mdss_mdp {
>>> +	status = "okay";
>>> +};
>> This should also be enabled by default, MDSS is useless when MDP is
>> disabled.
> 
> But don't we want to disable both when display is not used (not connected)?
The MDSS bus device only has a 0x1000 slice of the 0x90000-long "full MDSS",
the rest is probed with MDP/DPU. It also calls of_something_populate that
make DSI, DSIPHY and DP/HDMI probe. But all of them ultimately need a graph
handle to MDP.

If we have a display (of any kind), MDP has to be enabled (or the display
engine will not have a way to be programmed).

If we don't, enabling MDSS makes no sense as all of the hardware will be
shut down right after probing.

So I'd say either both or none.

Konrad
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH] arm64: dts: qcom: sm8550-qrd: add display and panel

[Krzysztof Kozlowski]

On 16/05/2023 17:26, Konrad Dybcio wrote:
>>>> +&mdss_mdp {
>>>> +	status = "okay";
>>>> +};
>>> This should also be enabled by default, MDSS is useless when MDP is
>>> disabled.
>>
>> But don't we want to disable both when display is not used (not connected)?
> The MDSS bus device only has a 0x1000 slice of the 0x90000-long "full MDSS",
> the rest is probed with MDP/DPU. It also calls of_something_populate that
> make DSI, DSIPHY and DP/HDMI probe. But all of them ultimately need a graph
> handle to MDP.
> 
> If we have a display (of any kind), MDP has to be enabled (or the display
> engine will not have a way to be programmed).
> 
> If we don't, enabling MDSS makes no sense as all of the hardware will be
> shut down right after probing.
> 
> So I'd say either both or none.

Yes, so the current state - both disabled - is matching it.

Best regards,
Krzysztof

Re: [PATCH] arm64: dts: qcom: sm8550-qrd: add display and panel

[Konrad Dybcio]

On 16.05.2023 17:35, Krzysztof Kozlowski wrote:
> On 16/05/2023 17:26, Konrad Dybcio wrote:
>>>>> +&mdss_mdp {
>>>>> +	status = "okay";
>>>>> +};
>>>> This should also be enabled by default, MDSS is useless when MDP is
>>>> disabled.
>>>
>>> But don't we want to disable both when display is not used (not connected)?
>> The MDSS bus device only has a 0x1000 slice of the 0x90000-long "full MDSS",
>> the rest is probed with MDP/DPU. It also calls of_something_populate that
>> make DSI, DSIPHY and DP/HDMI probe. But all of them ultimately need a graph
>> handle to MDP.
>>
>> If we have a display (of any kind), MDP has to be enabled (or the display
>> engine will not have a way to be programmed).
>>
>> If we don't, enabling MDSS makes no sense as all of the hardware will be
>> shut down right after probing.
>>
>> So I'd say either both or none.
> 
> Yes, so the current state - both disabled - is matching it.
Right, but what i was trying to say is that if we leave MDP without
any status properties, it will follow MDSS.

Konrad
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH] arm64: dts: qcom: sm8550-qrd: add display and panel

[Krzysztof Kozlowski]

On 16/05/2023 17:36, Konrad Dybcio wrote:
>>>> But don't we want to disable both when display is not used (not connected)?
>>> The MDSS bus device only has a 0x1000 slice of the 0x90000-long "full MDSS",
>>> the rest is probed with MDP/DPU. It also calls of_something_populate that
>>> make DSI, DSIPHY and DP/HDMI probe. But all of them ultimately need a graph
>>> handle to MDP.
>>>
>>> If we have a display (of any kind), MDP has to be enabled (or the display
>>> engine will not have a way to be programmed).
>>>
>>> If we don't, enabling MDSS makes no sense as all of the hardware will be
>>> shut down right after probing.
>>>
>>> So I'd say either both or none.
>>
>> Yes, so the current state - both disabled - is matching it.
> Right, but what i was trying to say is that if we leave MDP without
> any status properties, it will follow MDSS.

Ah, I missed that it is MDSS child, so indeed there is no point to have
it explicitly disabled. I'll fix it.

Best regards,
Krzysztof