Re: [PATCH v5 1/3] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file

[Jeykumar Sankaran <jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]

On 2018-12-03 16:57, Doug Anderson wrote:
> Hi,
> 
> On Mon, Dec 3, 2018 at 2:27 PM Jeykumar Sankaran 
> <jsanka@codeaurora.org>
> wrote:
>> +                       dsi0: dsi@ae94000 {
>> +                               compatible = "qcom,mdss-dsi-ctrl";
>> +                               reg = <0xae94000 0x400>;
>> +                               reg-names = "dsi_ctrl";
>> +
>> +                               interrupt-parent = <&mdss>;
>> +                               interrupts = <4 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> +                               clocks = <&dispcc 
>> DISP_CC_MDSS_BYTE0_CLK>,
>> +                                        <&dispcc
>> DISP_CC_MDSS_BYTE0_INTF_CLK>,
>> +                                        <&dispcc 
>> DISP_CC_MDSS_PCLK0_CLK>,
>> +                                        <&dispcc 
>> DISP_CC_MDSS_ESC0_CLK>,
>> +                                        <&dispcc 
>> DISP_CC_MDSS_AHB_CLK>,
>> +                                        <&dispcc 
>> DISP_CC_MDSS_AXI_CLK>;
>> +                               clock-names = "byte",
>> +                                             "byte_intf",
>> +                                             "pixel",
>> +                                             "core",
>> +                                             "iface",
>> +                                             "bus";
>> +
>> +                               phys = <&dsi0_phy>;
>> +                               phy-names = "dsi0";
> 
> I'm pretty sure that this should just be "dsi" and the one below in
> dsi1 should also be called "dsi".  +Jordan should confirm.
> 
Makes sense. I can fix that!
> 
>> +                       dsi1: dsi@ae96000 {
>> +                               compatible = "qcom,mdss-dsi-ctrl";
>> +                               reg = <0xae96000 0x400>;
>> +                               reg-names = "dsi_ctrl";
>> +
>> +                               interrupt-parent = <&mdss>;
>> +                               interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> +                               clocks = <&dispcc 
>> DISP_CC_MDSS_BYTE1_CLK>,
>> +                                        <&dispcc
>> DISP_CC_MDSS_BYTE1_INTF_CLK>,
>> +                                        <&dispcc 
>> DISP_CC_MDSS_PCLK1_CLK>,
>> +                                        <&dispcc 
>> DISP_CC_MDSS_ESC1_CLK>,
>> +                                        <&dispcc 
>> DISP_CC_MDSS_AHB_CLK>,
>> +                                        <&dispcc 
>> DISP_CC_MDSS_AXI_CLK>;
>> +                               clock-names = "byte",
>> +                                             "byte_intf",
>> +                                             "pixel",
>> +                                             "core",
>> +                                             "iface",
>> +                                             "bus";
>> +
>> +                               phys = <&dsi1_phy>;
>> +                               phy-names = "dsi1";
>> +
>> +                               status = "disabled";
> 
> This "disabled" is causing me problems.  I don't actually need "dsi1"
> but if I don't enable "dsi1" then my display doesn't come up.  :(  I
> ran out of time to debug but I wonder if this is this the standard
> thing where DRM needs to wait for all the components to probe until it
> can finish?  If nobody on this list just knows I'll dig tomorrow and
> confirm that my memory isn't faulty and see what we've done about this
> in the past.
> 
https://patchwork.kernel.org/patch/10467895/

Can you try out with this change (reviewed but not merged yet). It 
validates
the nodes before adding to the DSI list.
> 
> 
> One last note: it's pretty weird that you sent out only 1/3 and not
> 2/3 and 3/3.  If you're not ready to send out MTP stuff yet then you
> should send out v6 as just a singleton patch.
Yes. I was trying to separate this one out as an independent change. 
Sandeep
is working on the comments on removing the pinctrl nodes and updated
mtp nodes. He should be posting 2/3 and 3/3 in the next couple of days.

Thanks,
Jeykumar S.

> 
> 
> -Doug

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno