Hi,

On 09/08/2016 05:02 AM, John Stultz wrote:
> Sort of tagging on to Archit's patchset here.
>
> Adds the core gpu, and dsi nodes for the apq8064 needed
> to get graphics working on the nexus7 and other devices.
>
> Feedback would be greatly appreciated!
>
> Cc: Archit Taneja <architt at codeaurora.org>
> Cc: vinay simha <vinaysimha at inforcecomputing.com>
> Cc: andy.gross at linaro.org
> Cc: robdclark at gmail.com
> Cc: linux-arm-msm at vger.kernel.org
> Cc: dri-devel at lists.freedesktop.org
> Signed-off-by: John Stultz <john.stultz at linaro.org>
> ---
>   arch/arm/boot/dts/qcom-apq8064-pins.dtsi |  10 ++
>   arch/arm/boot/dts/qcom-apq8064.dtsi      | 227 
> +++++++++++++++++++++++++++++++
>   2 files changed, 237 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-apq8064-pins.dtsi 
> b/arch/arm/boot/dts/qcom-apq8064-pins.dtsi
> index 6b801e7..7bb0677 100644
> --- a/arch/arm/boot/dts/qcom-apq8064-pins.dtsi
> +++ b/arch/arm/boot/dts/qcom-apq8064-pins.dtsi
> @@ -284,4 +284,14 @@
>                       bias-disable = <0>;
>               };
>       };
> +
> +     dsi_panel_pinctrl: dsi-panel-pinctrl {
> +             mux {
> +                     pins = "gpio54";
> +                     function = "gpio";
> +                     bias-pull-up;
> +                     drive-strength = <8>;
> +             };
> +     };
> +
>   };
> diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi 
> b/arch/arm/boot/dts/qcom-apq8064.dtsi
> index 35a5759..49333cc 100644
> --- a/arch/arm/boot/dts/qcom-apq8064.dtsi
> +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
> @@ -919,6 +919,228 @@
>                       reg = <0x1a400000 0x100>;
>               };
>
> +             gpu: adreno-3xx at 4300000 {
> +                     compatible = "qcom,adreno-3xx";
> +                     reg = <0x04300000 0x20000>;
> +                     reg-names = "kgsl_3d0_reg_memory";
> +                     interrupts = <GIC_SPI 80 0>;
> +                     interrupt-names = "kgsl_3d0_irq";
> +                     clock-names =
> +                         "core_clk",
> +                         "iface_clk",
> +                         "mem_clk",
> +                         "mem_iface_clk";
> +                     clocks =
> +                         <&mmcc GFX3D_CLK>,
> +                         <&mmcc GFX3D_AHB_CLK>,
> +                         <&mmcc GFX3D_AXI_CLK>,
> +                         <&mmcc MMSS_IMEM_AHB_CLK>;
> +                     qcom,chipid = <0x03020002>;
> +
> +                     iommus = <&gfx3d 0
> +                               &gfx3d 1
> +                               &gfx3d 2
> +                               &gfx3d 3
> +                               &gfx3d 4
> +                               &gfx3d 5
> +                               &gfx3d 6
> +                               &gfx3d 7
> +                               &gfx3d 8
> +                               &gfx3d 9
> +                               &gfx3d 10
> +                               &gfx3d 11
> +                               &gfx3d 12
> +                               &gfx3d 13
> +                               &gfx3d 14
> +                               &gfx3d 15
> +                               &gfx3d 16
> +                               &gfx3d 17
> +                               &gfx3d 18
> +                               &gfx3d 19
> +                               &gfx3d 20
> +                               &gfx3d 21
> +                               &gfx3d 22
> +                               &gfx3d 23
> +                               &gfx3d 24
> +                               &gfx3d 25
> +                               &gfx3d 26
> +                               &gfx3d 27
> +                               &gfx3d 28
> +                               &gfx3d 29
> +                               &gfx3d 30
> +                               &gfx3d 31
> +                               &gfx3d1 0
> +                               &gfx3d1 1
> +                               &gfx3d1 2
> +                               &gfx3d1 3
> +                               &gfx3d1 4
> +                               &gfx3d1 5
> +                               &gfx3d1 6
> +                               &gfx3d1 7
> +                               &gfx3d1 8
> +                               &gfx3d1 9
> +                               &gfx3d1 10
> +                               &gfx3d1 11
> +                               &gfx3d1 12
> +                               &gfx3d1 13
> +                               &gfx3d1 14
> +                               &gfx3d1 15
> +                               &gfx3d1 16
> +                               &gfx3d1 17
> +                               &gfx3d1 18
> +                               &gfx3d1 19
> +                               &gfx3d1 20
> +                               &gfx3d1 21
> +                               &gfx3d1 22
> +                               &gfx3d1 23
> +                               &gfx3d1 24
> +                               &gfx3d1 25
> +                               &gfx3d1 26
> +                               &gfx3d1 27
> +                               &gfx3d1 28
> +                               &gfx3d1 29
> +                               &gfx3d1 30
> +                               &gfx3d1 31>;
> +
> +                     qcom,gpu-pwrlevels {
> +                             compatible = "qcom,gpu-pwrlevels";
> +                             qcom,gpu-pwrlevel at 0 {
> +                                     qcom,gpu-freq = <450000000>;
> +                             };
> +                             qcom,gpu-pwrlevel at 1 {
> +                                     qcom,gpu-freq = <27000000>;
> +                             };
> +                     };
> +             };
> +
> +             mmss_sfpb: syscon at 5700000 {
> +                     compatible = "syscon";
> +                     reg = <0x5700000 0x70>;
> +             };
> +
> +             dsi0: qcom,mdss_dsi at 4700000 {

The 'qcom,' prefix above isn't preferred here. This should
rather be:
                dsi0: dsi at 4700000 {

> +                     compatible = "qcom,mdss-dsi-ctrl";
> +                     label = "MDSS DSI CTRL->0";
> +                     #address-cells = <1>;
> +                     #size-cells = <0>;
> +                     interrupts = <GIC_SPI 82 0>;
> +                     reg = <0x04700000 0x200>;
> +                     reg-names = "dsi_ctrl";
> +
> +                     clocks = <&mmcc DSI_M_AHB_CLK>,
> +                             <&mmcc DSI_S_AHB_CLK>,
> +                             <&mmcc AMP_AHB_CLK>,
> +                             <&mmcc DSI_CLK>,
> +                             <&mmcc DSI1_BYTE_CLK>,
> +                             <&mmcc DSI_PIXEL_CLK>,
> +                             <&mmcc DSI1_ESC_CLK>,
> +                             <&mmcc DSI1_BYTE_SRC>,
> +                             <&mmcc DSI1_ESC_SRC>,
> +                             <&mmcc DSI_SRC>,
> +                             <&mmcc DSI_PIXEL_SRC>;
> +                     clock-names = "iface_clk", "bus_clk", "core_mmss_clk",
> +                                     "src_clk", "byte_clk", "pixel_clk",
> +                                     "core_clk", "byte_clk_src", 
> "esc_clk_src",
> +                                     "dsi_clk_src", "pixel_clk_src";

The last 4 clocks shouldn't be specified since they aren't actually
inputs to the DSI block. Instead, we should set default parents for
these clocks using assigned clocks:

                assigned-clocks = <&mmcc DSI1_BYTE_SRC>,
                                  <&mmcc DSI1_ESC_SRC>,
                                  <&mmcc DSI_SRC>,
                                  <&mmcc DSI_PIXEL_SRC>;
                assigned-clock-parents = <&dsi0_phy 0>,
                                         <&dsi0_phy 0>,
                                         <&dsi0_phy 1>,
                                         <&dsi0_phy 1>;

> +
> +                     syscon-sfpb = <&mmss_sfpb>;
> +                     phys = <&dsi0_phy>;
> +                     ports {
> +                             #address-cells = <1>;
> +                             #size-cells = <0>;
> +
> +                             port at 0 {
> +                                     reg = <0>;
> +                                     dsi0_in: endpoint {
> +                                     };
> +                             };
> +
> +                             port at 1 {
> +                                     reg = <1>;
> +                                     dsi0_out: endpoint {
> +                                     };
> +                             };
> +                     };
> +             };
> +
> +
> +             dsi0_phy: qcom,mdss_dsi_phy at 4700200 {

Same as suggestion as for dsi0. Also, we tend to use hyphen
instead of underscore for phy names. This should be:

                dsi0_phy: dsi-phy at 4700200 {

The DSI bindings look good otherwise. Thanks for taking this up.

Archit

> +                     compatible = "qcom,dsi-phy-28nm-8960";
> +                     #clock-cells = <1>;
> +
> +                     reg = <0x04700200 0x100>,
> +                             <0x04700300 0x200>,
> +                             <0x04700500 0x5c>;
> +                     reg-names = "dsi_pll", "dsi_phy", "dsi_phy_regulator";
> +                     clock-names = "iface_clk";
> +                     clocks = <&mmcc DSI_M_AHB_CLK>;
> +             };
> +
> +
> +             mdp_port0: iommu at 7500000 {
> +                     compatible = "qcom,apq8064-iommu";
> +                     #iommu-cells = <1>;
> +                     clock-names =
> +                         "smmu_pclk",
> +                         "iommu_clk";
> +                     clocks =
> +                         <&mmcc SMMU_AHB_CLK>,
> +                         <&mmcc MDP_AXI_CLK>;
> +                     reg = <0x07500000 0x100000>;
> +                     interrupts =
> +                         <GIC_SPI 63 0>,
> +                         <GIC_SPI 64 0>;
> +                     qcom,ncb = <2>;
> +             };
> +
> +             mdp_port1: iommu at 7600000 {
> +                     compatible = "qcom,apq8064-iommu";
> +                     #iommu-cells = <1>;
> +                     clock-names =
> +                         "smmu_pclk",
> +                         "iommu_clk";
> +                     clocks =
> +                         <&mmcc SMMU_AHB_CLK>,
> +                         <&mmcc MDP_AXI_CLK>;
> +                     reg = <0x07600000 0x100000>;
> +                     interrupts =
> +                         <GIC_SPI 61 0>,
> +                         <GIC_SPI 62 0>;
> +                     qcom,ncb = <2>;
> +             };
> +
> +             gfx3d: iommu at 7c00000 {
> +                     compatible = "qcom,apq8064-iommu";
> +                     #iommu-cells = <1>;
> +                     clock-names =
> +                         "smmu_pclk",
> +                         "iommu_clk";
> +                     clocks =
> +                         <&mmcc SMMU_AHB_CLK>,
> +                         <&mmcc GFX3D_AXI_CLK>;
> +                     reg = <0x07c00000 0x100000>;
> +                     interrupts =
> +                         <GIC_SPI 69 0>,
> +                         <GIC_SPI 70 0>;
> +                     qcom,ncb = <3>;
> +             };
> +
> +             gfx3d1: iommu at 7d00000 {
> +                     compatible = "qcom,apq8064-iommu";
> +                     #iommu-cells = <1>;
> +                     clock-names =
> +                         "smmu_pclk",
> +                         "iommu_clk";
> +                     clocks =
> +                         <&mmcc SMMU_AHB_CLK>,
> +                         <&mmcc GFX3D_AXI_CLK>;
> +                     reg = <0x07d00000 0x100000>;
> +                     interrupts =
> +                         <GIC_SPI 210 0>,
> +                         <GIC_SPI 211 0>;
> +                     qcom,ncb = <3>;
> +             };
> +
>               pcie: pci at 1b500000 {
>                       compatible = "qcom,pcie-apq8064", "snps,dw-pcie";
>                       reg = <0x1b500000 0x1000
> @@ -1016,6 +1238,11 @@
>                                     "hdmi_clk",
>                                     "tv_clk";
>
> +                     iommus = <&mdp_port0 0
> +                               &mdp_port0 2
> +                               &mdp_port1 0
> +                               &mdp_port1 2>;
> +
>                       ports {
>                               #address-cells = <1>;
>                               #size-cells = <0>;
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Reply via email to