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

Doug Anderson
2018-11-09T00:56:51Z

Hi,

On Fri, Nov 2, 2018 at 3:26 PM Jeykumar Sankaran <jsanka@codeaurora.org> wrote:
>
> DPU is short for the Display Processing Unit. It is the display
> controller on Qualcomm SDM845 chips.
>
> This change adds MDSS and DSI nodes to enable display on the
> target device.
>
> Changes in v2:
> - Beefed up commit message
> - Use SoC specific compatibles for mdss and dpu (Rob H)
> - Use assigned-clocks to set initial clock frequency(Rob H)
> Changes in v3:
> - added IOMMU node
> - Fix device naming (remove _phys)
> - Use correct IRQ_TYPE in interrupt specifiers
> Changes in v4:
> - move mdss node to preserve the unit address sort order
> - remove _clk suffix from dsi clocks
> (both the comments are from Doug Anderson)
>
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 191 +++++++++++++++++++++++++++++++++++
> 1 file changed, 191 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index b72bdb0..5728b4c 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -1248,6 +1248,197 @@
> };
> };
>
> + mdss: mdss@ae00000 {
> + compatible = "qcom,sdm845-mdss";
> + reg = <0xae00000 0x1000>;
> + reg-names = "mdss";
> +
> + power-domains = <&dispcc 0>;

Could be done in a follow-up patch, but technically the above should be:

power-domains = <&dispcc MDSS_GDSC>;

> + dsi0: dsi@ae94000 {
> + compatible = "qcom,mdss-dsi-ctrl";
> + reg = <0xae94000 0x400>;
> + reg-names = "dsi_ctrl";
> +
> + interrupt-parent = <&mdss>;
> + interrupts = <4 IRQ_TYPE_LEVEL_HIGH>;

Just out of curiousity, where does the 4 comes from? I guess this matches:

#define MDSS_HW_INTR_STATUS_INTR_DSI0 0x00000010
#define MDSS_HW_INTR_STATUS_INTR_DSI1 0x00000020

...where 0x10 == (1 << 4)

I don't see any bindings for this define so I guess hardcoding it is
fine (and the 5 for DSI1).

> + dsi0_phy: dsi-phy@ae94400 {
> + compatible = "qcom,dsi-phy-10nm";
> + reg = <0xae94400 0x200>,
> + <0xae94a00 0x1e0>,
> + <0xae94600 0x280>;
> + reg-names = "dsi_phy",
> + "dsi_pll",
> + "dsi_phy_lane";
> +
> + #clock-cells = <1>;
> + #phy-cells = <0>;
> +
> + clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>;
> + clock-names = "iface";
> + };

+Matthias will probably want to base a future patch on this one since
he's trying to hook the clocks up more properly. ...but what you have
above matches the current bindings so I think we're good.

Overall: I am not massively familiar with display / bridge / panel
bindings but as far as I can tell this patch is good and ready to
land. Future work (like fixing the nit about using the power domain
#define) can always be done in a follow-up patch. Thus:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

I've tested this patch and it's helped me get a working display. Thus:

Tested-by: Douglas Anderson <dianders@chromium.org>

-Doug
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org