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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2bc16f9a-e150-b3d0-6d72-a2ff0a4d05ae@linaro.org>
Date:   Tue, 16 May 2023 17:26:46 +0200
From:   Konrad Dybcio <konrad.dybcio@...aro.org>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     Neil Armstrong <neil.armstrong@...aro.org>,
        "Signed-off-by : Abel Vesa" <abel.vesa@...aro.org>
Subject: Re: [PATCH] arm64: dts: qcom: sm8550-qrd: add display and panel



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@...aro.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
>