lkml.org 
[lkml]   [2020]   [Jun]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] arm64: dts: qcom: sc7180: Add Display Port dt node
Thanks Stephen for reviews.
I will post new change addressing your all comments.

On 2020-06-18 18:12, Stephen Boyd wrote:
> Quoting Tanmay Shah (2020-06-18 16:21:13)
>> Enable DP driver for sc7180.
>
> Add DP device node on sc7180? This isn't a driver.
>
Done.
>>
>> This change depends-on following series:
>> https://patchwork.freedesktop.org/series/78583/
>> and https://patchwork.freedesktop.org/patch/351990/
>>
>> Changes in v2:
>>
>> - Add assigned-clocks and assigned-clock-parents
>> - Remove cell-index and pixel_rcg
>> - Change compatible to qcom,sc7180-dp
>>
>> Signed-off-by: Tanmay Shah <tanmay@codeaurora.org>
>> ---
>> arch/arm64/boot/dts/qcom/sc7180.dtsi | 57
>> ++++++++++++++++++++++++++--
>> 1 file changed, 53 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> index 916401f7e87c..26fe623e3b0f 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> @@ -2216,10 +2216,19 @@ ports {
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> + port@1 {
>> + reg = <1>;
>> + dpu_intf1_out:
>> endpoint {
>> +
>> remote-endpoint =
>> +
>> <&dsi0_in>;
>> + };
>> + };
>> +
>> port@0 {
>> reg = <0>;
>> - dpu_intf1_out:
>> endpoint {
>> -
>> remote-endpoint = <&dsi0_in>;
>> + dpu_intf0_out:
>> endpoint {
>> +
>> remote-endpoint =
>> +
>> <&dp_in>;
>> };
>> };
>> };
>
> I thought this wasn't supposed to change? At least according to the
> binding it shouldn't be needed.
>
Thanks. Yes it should be port@2 as per new dpu-sc7180.yaml bindings in
below patch.
https://patchwork.freedesktop.org/patch/371087/?series=78583&rev=1

>> @@ -2293,6 +2302,46 @@ dsi_phy: dsi-phy@ae94400 {
>> };
>> };
>>
>> + msm_dp: displayport-controller@ae90000{
>> + status = "ok";
>
> Please use status = "disabled";
>
Ok got it. Board's DT will enable it.

>> + compatible = "qcom,sc7180-dp";
>> +
>> + reg = <0 0xae90000 0 0x1400>;
>> + reg-names = "dp_controller";
>> +
>> + interrupt-parent = <&mdss>;
>
> Any reason why this isn't under the mdss node like the other display
> device nodes?
>
Done. Moved msm_dp node under mdss.
>> + 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>;
>> + clock-names = "core_aux", "ctrl_link",
>> + "ctrl_link_iface",
>> "stream_pixel";
>> + #clock-cells = <1>;
>> + assigned-clocks = <&dispcc
>> DISP_CC_MDSS_DP_PIXEL_CLK_SRC>;
>> + assigned-clock-parents = <&msm_dp 1>;
>> +
>> + data-lanes = <0 1>;
>
> This can and should be left to the board files. At the SoC level my
> understanding is that there are four lanes possible, so no need to
> artificially limit it here.
>
Removed from here. Driver changes I will be posting accordingly.
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + port@0 {
>> + reg = <0>;
>> + dp_in: endpoint {
>> + remote-endpoint =
>> +
>> <&dpu_intf0_out>;
>
> I'd prefer these were on one line, regardless of the 80 character line
> limit/suggestion.
>
ok done.
>> + };
>> + };
>> +
>> + port@1 {
>> + reg = <1>;
>> + dp_out: endpoint { };
>> + };
>> + };
>> + };
>> +
>> dispcc: clock-controller@af00000 {
>> compatible = "qcom,sc7180-dispcc";
>> reg = <0 0x0af00000 0 0x200000>;

\
 
 \ /
  Last update: 2020-06-19 23:57    [W:0.054 / U:0.156 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site