[Freedreno] [PATCH v4 0/3] sc7180: Add A618 GPU bindings

2020-02-04 Thread Sharat Masetty
I used this branch qcom/arm64-for-5.6-to-be-rebased as suggested by Matthias.
This patch needs the clock patches and the clock patches have not yet landed, so

please apply the following series and patches in order
a) All patches from 
https://patchwork.kernel.org/project/linux-clk/list/?series=203517&state=%2a&archive=both
b) Patches 1 and 2 from 
https://patchwork.kernel.org/project/linux-clk/list/?series=203527&archive=both&state=%2a
c) All patches from 
https://patchwork.kernel.org/project/linux-clk/list/?series=221739&archive=both&state=%2a
d) 
https://lore.kernel.org/linux-arm-msm/20200124144154.v2.10.I1a4b93fb005791e29a9dcf288fc8bd459a555a59%40changeid/raw
e) This patch "arm64: dts: qcom: sc7180: Add A618 gpu dt blob"

v3: Addressed review comments from previous submits. Also removed the
interconnect bindings from this patch and I will submit as a new patch with its
dependencies listed. Also I will be sending a new patch for updating the
bindings documentation.

v4: Add GX_GDSC power domain binding for GMU

Sharat Masetty (1):
  arm64: dts: qcom: sc7180: Add A618 gpu dt blob

Taniya Das (2):
  dt-bindings: clk: qcom: Add support for GPU GX GDSCR
  clk: qcom: gpucc: Add support for GX GDSC for SC7180

 arch/arm64/boot/dts/qcom/sc7180.dtsi  | 102 ++
 drivers/clk/qcom/gpucc-sc7180.c   |  37 ++
 include/dt-bindings/clock/qcom,gpucc-sc7180.h |   3 +-
 3 files changed, 141 insertions(+), 1 deletion(-)

--
1.9.1
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [PATCH v4 1/3] dt-bindings: clk: qcom: Add support for GPU GX GDSCR

2020-02-04 Thread Sharat Masetty
From: Taniya Das 

In the cases where the GPU SW requires to use the GX GDSCR add
support for the same.

Signed-off-by: Taniya Das 
---
 include/dt-bindings/clock/qcom,gpucc-sc7180.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/dt-bindings/clock/qcom,gpucc-sc7180.h 
b/include/dt-bindings/clock/qcom,gpucc-sc7180.h
index 0e4643b..65e706d 100644
--- a/include/dt-bindings/clock/qcom,gpucc-sc7180.h
+++ b/include/dt-bindings/clock/qcom,gpucc-sc7180.h
@@ -15,7 +15,8 @@
 #define GPU_CC_CXO_CLK 6
 #define GPU_CC_GMU_CLK_SRC 7
 
-/* CAM_CC GDSCRs */
+/* GPU_CC GDSCRs */
 #define CX_GDSC0
+#define GX_GDSC1
 
 #endif
-- 
1.9.1
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [PATCH v4 2/3] clk: qcom: gpucc: Add support for GX GDSC for SC7180

2020-02-04 Thread Sharat Masetty
From: Taniya Das 

 Most of the time the CPU should not be touching the GX domain on the
 GPU
 except for a very special use case when the CPU needs to force the GX
 headswitch off. Add a dummy enable function for the GX gdsc to simulate
 success so that the pm_runtime reference counting is correct.

Signed-off-by: Taniya Das 
---
 drivers/clk/qcom/gpucc-sc7180.c | 37 +
 1 file changed, 37 insertions(+)

diff --git a/drivers/clk/qcom/gpucc-sc7180.c b/drivers/clk/qcom/gpucc-sc7180.c
index ec61194..3b29f19 100644
--- a/drivers/clk/qcom/gpucc-sc7180.c
+++ b/drivers/clk/qcom/gpucc-sc7180.c
@@ -172,8 +172,45 @@ enum {
.flags = VOTABLE,
 };
 
+/*
+ * On SC7180 the GPU GX domain is *almost* entirely controlled by the GMU
+ * running in the CX domain so the CPU doesn't need to know anything about the
+ * GX domain EXCEPT
+ *
+ * Hardware constraints dictate that the GX be powered down before the CX. If
+ * the GMU crashes it could leave the GX on. In order to successfully bring 
back
+ * the device the CPU needs to disable the GX headswitch. There being no sane
+ * way to reach in and touch that register from deep inside the GPU driver we
+ * need to set up the infrastructure to be able to ensure that the GPU can
+ * ensure that the GX is off during this super special case. We do this by
+ * defining a GX gdsc with a dummy enable function and a "default" disable
+ * function.
+ *
+ * This allows us to attach with genpd_dev_pm_attach_by_name() in the GPU
+ * driver. During power up, nothing will happen from the CPU (and the GMU will
+ * power up normally but during power down this will ensure that the GX domain
+ * is *really* off - this gives us a semi standard way of doing what we need.
+ */
+static int gx_gdsc_enable(struct generic_pm_domain *domain)
+{
+   /* Do nothing but give genpd the impression that we were successful */
+   return 0;
+}
+
+static struct gdsc gx_gdsc = {
+   .gdscr = 0x100c,
+   .clamp_io_ctrl = 0x1508,
+   .pd = {
+   .name = "gpu_gx_gdsc",
+   .power_on = gx_gdsc_enable,
+   },
+   .pwrsts = PWRSTS_OFF_ON,
+   .flags = CLAMP_IO,
+};
+
 static struct gdsc *gpu_cc_sc7180_gdscs[] = {
[CX_GDSC] = &cx_gdsc,
+   [GX_GDSC] = &gx_gdsc,
 };
 
 static struct clk_regmap *gpu_cc_sc7180_clocks[] = {
-- 
1.9.1
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [PATCH v4 3/3] arm64: dts: qcom: sc7180: Add A618 gpu dt blob

2020-02-04 Thread Sharat Masetty
This patch adds the required dt nodes and properties
to enabled A618 GPU.

Signed-off-by: Sharat Masetty 
---
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 102 +++
 1 file changed, 102 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi 
b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index f3fcc5c..63fff15 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -1043,6 +1043,108 @@
};
};
 
+   gpu: gpu@500 {
+   compatible = "qcom,adreno-618.0", "qcom,adreno";
+   #stream-id-cells = <16>;
+   reg = <0 0x0500 0 0x4>, <0 0x0509e000 0 0x1000>,
+   <0 0x05061000 0 0x800>;
+   reg-names = "kgsl_3d0_reg_memory", "cx_mem", "cx_dbgc";
+   interrupts = ;
+   iommus = <&adreno_smmu 0>;
+   operating-points-v2 = <&gpu_opp_table>;
+   qcom,gmu = <&gmu>;
+
+   gpu_opp_table: opp-table {
+   compatible = "operating-points-v2";
+
+   opp-8 {
+   opp-hz = /bits/ 64 <8>;
+   opp-level = 
;
+   };
+
+   opp-65000 {
+   opp-hz = /bits/ 64 <65000>;
+   opp-level = 
;
+   };
+
+   opp-56500 {
+   opp-hz = /bits/ 64 <56500>;
+   opp-level = ;
+   };
+
+   opp-43000 {
+   opp-hz = /bits/ 64 <43000>;
+   opp-level = 
;
+   };
+
+   opp-35500 {
+   opp-hz = /bits/ 64 <35500>;
+   opp-level = ;
+   };
+
+   opp-26700 {
+   opp-hz = /bits/ 64 <26700>;
+   opp-level = 
;
+   };
+
+   opp-18000 {
+   opp-hz = /bits/ 64 <18000>;
+   opp-level = 
;
+   };
+   };
+   };
+
+   adreno_smmu: iommu@504 {
+   compatible = "qcom,sc7180-smmu-v2", "qcom,smmu-v2";
+   reg = <0 0x0504 0 0x1>;
+   #iommu-cells = <1>;
+   #global-interrupts = <2>;
+   interrupts = ,
+   ,
+   ,
+   ,
+   ,
+   ,
+   ,
+   ,
+   ,
+   ;
+   clocks = <&gcc GCC_GPU_MEMNOC_GFX_CLK>,
+   <&gcc GCC_GPU_CFG_AHB_CLK>,
+   <&gcc GCC_DDRSS_GPU_AXI_CLK>;
+
+   clock-names = "bus", "iface", "mem_iface_clk";
+   power-domains = <&gpucc CX_GDSC>;
+   };
+
+   gmu: gmu@506a000 {
+   compatible="qcom,adreno-gmu-618.0", "qcom,adreno-gmu";
+   reg = <0 0x0506a000 0 0x31000>, <0 0x0b29 0 
0x1>,
+   <0 0x0b49 0 0x1>;
+   reg-names = "gmu", "gmu_pdc", "gmu_pdc_seq";
+   interrupts = ,
+  ;
+   interrupt-names = "hfi", "gmu";
+   clocks = <&gpucc GPU_CC_CX_GMU_CLK>,
+  <&gpucc GPU_CC_CXO_CLK>,
+  <&gcc GCC_DDRSS_GPU_AXI_CLK>,
+  <&gcc GCC_GPU_MEMNOC_GFX_CLK>;
+   clock-names = "gmu", "cxo", "axi", "memnoc";
+   power-domains = <&gpucc CX_GDSC>, <&gpucc GX_GDSC>;
+   power-domain-names = "cx", "gx";
+   iommus = <&adreno_smmu 5>;
+   operating-points-v2 = <&gmu_opp_table>;
+
+   gmu_opp_table: opp-table {
+   compatible = "operating-points-v2";
+
+   opp-2 {
+   opp-hz = /bits/ 64 <200

Re: [Freedreno] [PATCH v3] arm64: dts: qcom: sc7180: Add A618 gpu dt blob

2020-02-04 Thread smasetty

On 2020-02-01 03:13, Doug Anderson wrote:

Hi,

On Fri, Jan 31, 2020 at 4:04 AM Sharat Masetty 
 wrote:


+   adreno_smmu: iommu@504 {
+   compatible = "qcom,sc7180-smmu-v2", 
"qcom,smmu-v2";

+   reg = <0 0x0504 0 0x1>;
+   #iommu-cells = <1>;
+   #global-interrupts = <2>;
+   interrupts = IRQ_TYPE_LEVEL_HIGH>,
+   IRQ_TYPE_LEVEL_HIGH>,
+   IRQ_TYPE_EDGE_RISING>,
+   IRQ_TYPE_EDGE_RISING>,
+   IRQ_TYPE_EDGE_RISING>,
+   IRQ_TYPE_EDGE_RISING>,
+   IRQ_TYPE_EDGE_RISING>,
+   IRQ_TYPE_EDGE_RISING>,
+   IRQ_TYPE_EDGE_RISING>,
+   IRQ_TYPE_EDGE_RISING>;

+   clocks = <&gcc GCC_GPU_MEMNOC_GFX_CLK>,
+   <&gcc GCC_GPU_CFG_AHB_CLK>,
+   <&gcc GCC_DDRSS_GPU_AXI_CLK>;
+
+   clock-names = "bus", "iface", "mem_iface_clk";


Repeated comment from v2 feedback:

Please send a patch to:

Documentation/devicetree/bindings/iommu/arm,smmu.yaml

...adding 'qcom,sc7180-smmu-v2'.  If you do this it will point out
that you've added a new clock: "mem_iface_clk".  Is this truly a new
clock in sc7180 compared to previous IOMMUs?  ...or is it not really
needed?
I can confirm that this clock is needed for SC7180. I will send out a 
new patch

to update the documentation this week.




+   gmu: gmu@506a000 {
+   compatible="qcom,adreno-gmu-618.0", 
"qcom,adreno-gmu";
+   reg = <0 0x0506a000 0 0x31000>, <0 0x0b29 
0 0x1>,

+   <0 0x0b49 0 0x1>;
+   reg-names = "gmu", "gmu_pdc", "gmu_pdc_seq";
+   interrupts = IRQ_TYPE_LEVEL_HIGH>,

+  ;
+   interrupt-names = "hfi", "gmu";
+   clocks = <&gpucc GPU_CC_CX_GMU_CLK>,
+  <&gpucc GPU_CC_CXO_CLK>,
+  <&gcc GCC_DDRSS_GPU_AXI_CLK>,
+  <&gcc GCC_GPU_MEMNOC_GFX_CLK>;
+   clock-names = "gmu", "cxo", "axi", "memnoc";
+   power-domains = <&gpucc CX_GDSC>;
+   power-domain-names = "cx";


As per continued comments on v2, please see if this works for you:

  power-domains = <&gpucc CX_GDSC>, <0>;
  power-domain-names = "cx", "gx";

...and work to get something more real for "gx" ASAP.  It did seem to
boot for me and (unless someone disagrees) it seems better than
totally leaving it out / violating the bindings?


-Doug

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [v5] arm64: dts: sc7180: add display dt nodes

2020-02-04 Thread Doug Anderson
Hi,

On Tue, Feb 4, 2020 at 6:15 AM Harigovindan P  wrote:
>
> Add display, DSI hardware DT nodes for sc7180.
>
> Co-developed-by: Kalyan Thota 
> Signed-off-by: Kalyan Thota 
> Signed-off-by: Harigovindan P 
> ---
>
> Changes in v1:
> - Added display DT nodes for sc7180
> Changes in v2:
> - Renamed node names
> - Corrected code alignments
> - Removed extra new line
> - Added DISP AHB clock for register access
>   under display_subsystem node for global settings
> Changes in v3:
> - Modified node names
> - Modified hard coded values
> - Removed mdss reg entry
> Changes in v4:
> - Reverting mdp node name
> - Setting status to disabled in main SOC dtsi file
> - Replacing _ to - for node names
> - Adding clock dependency patch link
> - Splitting idp dt file to a separate patch
> Changes in v5:
> - Renaming "gcc_bus" to "bus" as per bindings (Doug Anderson)
> - Making status as disabled for mdss and mdss_mdp by default (Doug 
> Anderson)
> - Removing "disp_cc" register space (Doug Anderson)
> - Renaming "dsi_controller" to "dsi" as per bindings (Doug Anderson)
> - Providing "ref" clk for dsi_phy (Doug Anderson)
> - Sorting mdss node before dispcc (Doug Anderson)
>
> This patch has dependency on the below series
> https://lkml.org/lkml/2019/12/27/73

You should have probably pointed to [1] which is a much newer version.


>  arch/arm64/boot/dts/qcom/sc7180.dtsi | 136 
> ++-
>  1 file changed, 134 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi 
> b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index bd2584d..3ac1b87 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -1173,13 +1173,145 @@
> #power-domain-cells = <1>;
> };
>
> +   mdss: mdss@ae0 {
> +   compatible = "qcom,sc7180-mdss";
> +   reg = <0 0x0ae0 0 0x1000>;
> +   reg-names = "mdss";
> +
> +   power-domains = <&dispcc MDSS_GDSC>;
> +
> +   clocks = <&gcc GCC_DISP_AHB_CLK>,
> +<&gcc GCC_DISP_HF_AXI_CLK>,
> +<&dispcc DISP_CC_MDSS_AHB_CLK>,
> +<&dispcc DISP_CC_MDSS_MDP_CLK>;
> +   clock-names = "iface", "bus", "ahb", "core";
> +
> +   assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>;
> +   assigned-clock-rates = <3>;
> +
> +   interrupts = ;
> +   interrupt-controller;
> +   #interrupt-cells = <1>;
> +
> +   iommus = <&apps_smmu 0x800 0x2>;
> +
> +   #address-cells = <2>;
> +   #size-cells = <2>;
> +   ranges;
> +
> +   status = "disabled";
> +
> +   mdp: mdp@ae01000 {
> +   compatible = "qcom,sc7180-dpu";
> +   reg = <0 0x0ae01000 0 0x8f000>,
> + <0 0x0aeb 0 0x2008>;
> +   reg-names = "mdp", "vbif";
> +
> +   clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
> +<&dispcc DISP_CC_MDSS_ROT_CLK>,
> +<&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
> +<&dispcc DISP_CC_MDSS_MDP_CLK>,
> +<&dispcc DISP_CC_MDSS_VSYNC_CLK>;
> +   clock-names = "iface", "rot", "lut", "core",
> + "vsync";
> +   assigned-clocks = <&dispcc 
> DISP_CC_MDSS_MDP_CLK>,
> + <&dispcc 
> DISP_CC_MDSS_VSYNC_CLK>;
> +   assigned-clock-rates = <3>,
> +  <1920>;
> +
> +   interrupt-parent = <&mdss>;
> +   interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> +
> +   status = "disabled";
> +
> +   ports {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   port@0 {
> +   reg = <0>;
> +   dpu_intf1_out: endpoint {
> +   remote-endpoint = 
> <&dsi0_in>;
> +   };
> +   };
> +

Re: [Freedreno] [v1] dt-bindings: msm:disp: update dsi and dpu bindings

2020-02-04 Thread Doug Anderson
Hi,

On Tue, Feb 4, 2020 at 6:15 AM Harigovindan P  wrote:
>
> Updating bindings of dsi and dpu by adding and removing certain
> properties.
>
> Signed-off-by: Harigovindan P 
> ---
>
> Changes in v1:
> - Adding "ahb" clock as a required property.
> - Adding "bus", "rot", "lut" as optional properties for sc7180 device.
> - Removing properties from dsi bindings that are unused.
> - Removing power-domain property since DSI is the child node of MDSS
>   and it will inherit supply from its parent.
>
>  Documentation/devicetree/bindings/display/msm/dpu.txt | 7 +++
>  Documentation/devicetree/bindings/display/msm/dsi.txt | 5 -
>  2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt 
> b/Documentation/devicetree/bindings/display/msm/dpu.txt
> index 551ae26..dd58472a 100644
> --- a/Documentation/devicetree/bindings/display/msm/dpu.txt
> +++ b/Documentation/devicetree/bindings/display/msm/dpu.txt
> @@ -19,6 +19,7 @@ Required properties:
>The following clocks are required:
>* "iface"
>* "bus"
> +  * "ahb"

This is only required for sc7180?  ...or old SoCs should have had it
all along too?


>* "core"
>  - interrupts: interrupt signal from MDSS.
>  - interrupt-controller: identifies the node as an interrupt controller.
> @@ -50,6 +51,8 @@ Required properties:
>  - clock-names: device clock names, must be in same order as clocks property.
>The following clocks are required.
>* "bus"
> +  For the device "qcom,sc7180-dpu":
> +  * "bus" - is an optional property due to architecture change.

This is a really odd way to write it for two reasons:
* You're breaking up the flow of the list.
* This shouldn't be listed as "optional" in sc7180 but unless there is
some reason to ever provide it on sc7180.  It should simply be
disallowed.

Maybe instead just:

   The following clocks are required.
-  * "bus"
+  * "bus" (anything other than qcom,sc7180-dpu)

We really need to get this into yaml ASAP but that'd probably be OK to
tide us over.

NOTE: when converting to yaml, ideally we'll have a separate file per
SoC to avoid crazy spaghetti, see commit 2a8aa18c1131 ("dt-bindings:
clk: qcom: Fix self-validation, split, and clean cruft") in clk-next
for an example of starting the transition to one yaml per SoC (at
least for anything majorly different).


>* "iface"
>* "core"
>* "vsync"
> @@ -70,6 +73,10 @@ Optional properties:
>  - assigned-clocks: list of clock specifiers for clocks needing rate 
> assignment
>  - assigned-clock-rates: list of clock frequencies sorted in the same order as
>the assigned-clocks property.
> +- For the device "qcom,sc7180-dpu":
> +  clock-names: optional device clocks, needed for accessing LUT blocks.
> +  * "rot"
> +  * "lut"
>
>  Example:
>
> diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt 
> b/Documentation/devicetree/bindings/display/msm/dsi.txt
> index af95586..61d659a 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi.txt
> +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt
> @@ -8,13 +8,10 @@ Required properties:
>  - reg-names: The names of register regions. The following regions are 
> required:
>* "dsi_ctrl"
>  - interrupts: The interrupt signal from the DSI block.
> -- power-domains: Should be <&mmcc MDSS_GDSC>.

Is this supposed to be removed from all SoCs using this bindings, or
just yours?

I'll also note that you left it in the "Example:" below.


>  - clocks: Phandles to device clocks.
>  - clock-names: the following clocks are required:
> -  * "mdp_core"
>* "iface"
>* "bus"
> -  * "core_mmss"

As Jeffrey pointed out, you shouldn't be removing these from old SoCs.
In "drivers/gpu/drm/msm/dsi/dsi_cfg.c" you can clearly see them used.
Maybe it's time for you to do the yaml conversion and handle this
correctly per-SoC.

-Doug
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH] drm/msm/a6xx: Remove unneeded GBIF unhalt

2020-02-04 Thread Rob Clark
+jstultz

On Tue, Feb 4, 2020 at 9:42 AM Jordan Crouse  wrote:
>
> Commit e812744c5f95 ("drm: msm: a6xx: Add support for A618") added a
> universal GBIF un-halt into a6xx_start(). This can cause problems for
> a630 targets which do not use GBIF and might have access protection
> enabled on the region now occupied by the GBIF registers.
>
> But it turns out that we didn't need to unhalt the GBIF in this path
> since the stop function already takes care of that after executing a flush
> but before turning off the headswitch. We should be confident that the
> GBIF is open for business when we restart the hardware.
>
> Signed-off-by: Jordan Crouse 
> ---
>
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 12 
>  1 file changed, 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index daf0780..e51c723 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -378,18 +378,6 @@ static int a6xx_hw_init(struct msm_gpu *gpu)
> struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
> int ret;
>
> -   /*
> -* During a previous slumber, GBIF halt is asserted to ensure
> -* no further transaction can go through GPU before GPU
> -* headswitch is turned off.
> -*
> -* This halt is deasserted once headswitch goes off but
> -* incase headswitch doesn't goes off clear GBIF halt
> -* here to ensure GPU wake-up doesn't fail because of
> -* halted GPU transactions.
> -*/
> -   gpu_write(gpu, REG_A6XX_GBIF_HALT, 0x0);
> -
> /* Make sure the GMU keeps the GPU on while we set it up */
> a6xx_gmu_set_oob(&a6xx_gpu->gmu, GMU_OOB_GPU_SET);
>
> --
> 2.7.4
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [PATCH] drm/msm/a6xx: Remove unneeded GBIF unhalt

2020-02-04 Thread Jordan Crouse
Commit e812744c5f95 ("drm: msm: a6xx: Add support for A618") added a
universal GBIF un-halt into a6xx_start(). This can cause problems for
a630 targets which do not use GBIF and might have access protection
enabled on the region now occupied by the GBIF registers.

But it turns out that we didn't need to unhalt the GBIF in this path
since the stop function already takes care of that after executing a flush
but before turning off the headswitch. We should be confident that the
GBIF is open for business when we restart the hardware.

Signed-off-by: Jordan Crouse 
---

 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 12 
 1 file changed, 12 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index daf0780..e51c723 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -378,18 +378,6 @@ static int a6xx_hw_init(struct msm_gpu *gpu)
struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
int ret;
 
-   /*
-* During a previous slumber, GBIF halt is asserted to ensure
-* no further transaction can go through GPU before GPU
-* headswitch is turned off.
-*
-* This halt is deasserted once headswitch goes off but
-* incase headswitch doesn't goes off clear GBIF halt
-* here to ensure GPU wake-up doesn't fail because of
-* halted GPU transactions.
-*/
-   gpu_write(gpu, REG_A6XX_GBIF_HALT, 0x0);
-
/* Make sure the GMU keeps the GPU on while we set it up */
a6xx_gmu_set_oob(&a6xx_gpu->gmu, GMU_OOB_GPU_SET);
 
-- 
2.7.4
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH v2 2/3] drm: msm: a6xx: Add support for A618

2020-02-04 Thread Jordan Crouse
On Mon, Feb 03, 2020 at 04:40:40PM -0800, Rob Clark wrote:
> On Mon, Feb 3, 2020 at 4:21 PM John Stultz  wrote:
> >
> > On Wed, Jan 22, 2020 at 11:19 PM Sharat Masetty  
> > wrote:
> > >
> > > This patch adds support for enabling Graphics Bus Interface(GBIF)
> > > used in multiple A6xx series chipets. Also makes changes to the
> > > PDC/RSC sequencing specifically required for A618. This is needed
> > > for proper interfacing with RPMH.
> > >
> > > Signed-off-by: Sharat Masetty 
> > > ---
> > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
> > > b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > > index dc8ec2c..2ac9a51 100644
> > > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > > @@ -378,6 +378,18 @@ static int a6xx_hw_init(struct msm_gpu *gpu)
> > > struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
> > > int ret;
> > >
> > > +   /*
> > > +* During a previous slumber, GBIF halt is asserted to ensure
> > > +* no further transaction can go through GPU before GPU
> > > +* headswitch is turned off.
> > > +*
> > > +* This halt is deasserted once headswitch goes off but
> > > +* incase headswitch doesn't goes off clear GBIF halt
> > > +* here to ensure GPU wake-up doesn't fail because of
> > > +* halted GPU transactions.
> > > +*/
> > > +   gpu_write(gpu, REG_A6XX_GBIF_HALT, 0x0);
> > > +
> > > /* Make sure the GMU keeps the GPU on while we set it up */
> > > a6xx_gmu_set_oob(&a6xx_gpu->gmu, GMU_OOB_GPU_SET);
> > >
> >
> > So I already brought this up on #freedreno but figured I'd follow up
> > on the list.
> >
> > With linus/master, I'm seeing hard crashes (into usb crash mode) with
> > the db845c, which I isolated down to this patch, and then to the chunk
> > above.
> 
> (repeating my speculation from #freedreno for benefit of those not on IRC)
> 
> I'm suspecting, that like the registers to take the GPU out of secure
> mode, this register is being blocked on LA devices (like db845c),
> which is why we didn't see this on cheza.
> 
> Maybe we can make this write conditional on whether we have a zap shader?

Sorry, I was WFH yesterday and didn't have IRC on.

The 845 doesn't have GBIF (it still uses VBIF) and on a AC enabled target large
chunks of unused register space would be blocked by default so Rob's hypothesis
is correct. Since the 845 is the only a6xx target that still has a VBIF a
!adreno_is_a630() check would do here, but I'm not 100% convinced we need this
code at all. We explicitly clear the GBIF halt in the stop function before the
headswitch is turned off so I think this is mostly unneeded paranoia.

I need to get a tree with the 618 code in it and I'll try to get a fix out
shortly.

Jordan

> > Dropping the gpu_write line above gets things booting again for me.
> >
> > Let me know if there are any follow on patches I can help validate.
> >
> > thanks
> > -john
> > ___
> > Freedreno mailing list
> > Freedreno@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/freedreno
> ___
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [v1] dt-bindings: msm:disp: update dsi and dpu bindings

2020-02-04 Thread Jeffrey Hugo
On Tue, Feb 4, 2020 at 7:15 AM Harigovindan P  wrote:
>
> Updating bindings of dsi and dpu by adding and removing certain
> properties.
>
> Signed-off-by: Harigovindan P 
> ---
>
> Changes in v1:
> - Adding "ahb" clock as a required property.
> - Adding "bus", "rot", "lut" as optional properties for sc7180 device.
> - Removing properties from dsi bindings that are unused.
> - Removing power-domain property since DSI is the child node of MDSS
>   and it will inherit supply from its parent.
>
>  Documentation/devicetree/bindings/display/msm/dpu.txt | 7 +++
>  Documentation/devicetree/bindings/display/msm/dsi.txt | 5 -
>  2 files changed, 7 insertions(+), 5 deletions(-)
> diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt 
> b/Documentation/devicetree/bindings/display/msm/dsi.txt
> index af95586..61d659a 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi.txt
> +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt
> @@ -8,13 +8,10 @@ Required properties:
>  - reg-names: The names of register regions. The following regions are 
> required:
>* "dsi_ctrl"
>  - interrupts: The interrupt signal from the DSI block.
> -- power-domains: Should be <&mmcc MDSS_GDSC>.
>  - clocks: Phandles to device clocks.
>  - clock-names: the following clocks are required:
> -  * "mdp_core"
>* "iface"
>* "bus"
> -  * "core_mmss"

Why do you think these are unused?  I see them used in the driver, and
as far as I can tell these get routed to the hardware, therefore they
should be described in DT.

>* "byte"
>* "pixel"
>* "core"
> @@ -156,7 +153,6 @@ Example:
> "core",
> "core_mmss",
> "iface",
> -   "mdp_core",
> "pixel";
> clocks =
> <&mmcc MDSS_AXI_CLK>,
> @@ -164,7 +160,6 @@ Example:
> <&mmcc MDSS_ESC0_CLK>,
> <&mmcc MMSS_MISC_AHB_CLK>,
> <&mmcc MDSS_AHB_CLK>,
> -   <&mmcc MDSS_MDP_CLK>,
> <&mmcc MDSS_PCLK0_CLK>;
>
> assigned-clocks =
> --
> 2.7.4
>
> ___
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [v1] dt-bindings: msm:disp: update dsi and dpu bindings

2020-02-04 Thread Harigovindan P
Updating bindings of dsi and dpu by adding and removing certain
properties.

Signed-off-by: Harigovindan P 
---

Changes in v1:
- Adding "ahb" clock as a required property.
- Adding "bus", "rot", "lut" as optional properties for sc7180 device.
- Removing properties from dsi bindings that are unused.
- Removing power-domain property since DSI is the child node of MDSS
  and it will inherit supply from its parent.

 Documentation/devicetree/bindings/display/msm/dpu.txt | 7 +++
 Documentation/devicetree/bindings/display/msm/dsi.txt | 5 -
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt 
b/Documentation/devicetree/bindings/display/msm/dpu.txt
index 551ae26..dd58472a 100644
--- a/Documentation/devicetree/bindings/display/msm/dpu.txt
+++ b/Documentation/devicetree/bindings/display/msm/dpu.txt
@@ -19,6 +19,7 @@ Required properties:
   The following clocks are required:
   * "iface"
   * "bus"
+  * "ahb"
   * "core"
 - interrupts: interrupt signal from MDSS.
 - interrupt-controller: identifies the node as an interrupt controller.
@@ -50,6 +51,8 @@ Required properties:
 - clock-names: device clock names, must be in same order as clocks property.
   The following clocks are required.
   * "bus"
+  For the device "qcom,sc7180-dpu":
+  * "bus" - is an optional property due to architecture change.
   * "iface"
   * "core"
   * "vsync"
@@ -70,6 +73,10 @@ Optional properties:
 - assigned-clocks: list of clock specifiers for clocks needing rate assignment
 - assigned-clock-rates: list of clock frequencies sorted in the same order as
   the assigned-clocks property.
+- For the device "qcom,sc7180-dpu":
+  clock-names: optional device clocks, needed for accessing LUT blocks.
+  * "rot"
+  * "lut"
 
 Example:
 
diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt 
b/Documentation/devicetree/bindings/display/msm/dsi.txt
index af95586..61d659a 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi.txt
+++ b/Documentation/devicetree/bindings/display/msm/dsi.txt
@@ -8,13 +8,10 @@ Required properties:
 - reg-names: The names of register regions. The following regions are required:
   * "dsi_ctrl"
 - interrupts: The interrupt signal from the DSI block.
-- power-domains: Should be <&mmcc MDSS_GDSC>.
 - clocks: Phandles to device clocks.
 - clock-names: the following clocks are required:
-  * "mdp_core"
   * "iface"
   * "bus"
-  * "core_mmss"
   * "byte"
   * "pixel"
   * "core"
@@ -156,7 +153,6 @@ Example:
"core",
"core_mmss",
"iface",
-   "mdp_core",
"pixel";
clocks =
<&mmcc MDSS_AXI_CLK>,
@@ -164,7 +160,6 @@ Example:
<&mmcc MDSS_ESC0_CLK>,
<&mmcc MMSS_MISC_AHB_CLK>,
<&mmcc MDSS_AHB_CLK>,
-   <&mmcc MDSS_MDP_CLK>,
<&mmcc MDSS_PCLK0_CLK>;
 
assigned-clocks =
-- 
2.7.4

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [v5] arm64: dts: sc7180: add display dt nodes

2020-02-04 Thread Harigovindan P
Add display, DSI hardware DT nodes for sc7180.

Co-developed-by: Kalyan Thota 
Signed-off-by: Kalyan Thota 
Signed-off-by: Harigovindan P 
---

Changes in v1:
- Added display DT nodes for sc7180
Changes in v2:
- Renamed node names
- Corrected code alignments
- Removed extra new line
- Added DISP AHB clock for register access
  under display_subsystem node for global settings
Changes in v3:
- Modified node names
- Modified hard coded values
- Removed mdss reg entry
Changes in v4:
- Reverting mdp node name
- Setting status to disabled in main SOC dtsi file
- Replacing _ to - for node names
- Adding clock dependency patch link
- Splitting idp dt file to a separate patch
Changes in v5:
- Renaming "gcc_bus" to "bus" as per bindings (Doug Anderson)
- Making status as disabled for mdss and mdss_mdp by default (Doug 
Anderson)
- Removing "disp_cc" register space (Doug Anderson)
- Renaming "dsi_controller" to "dsi" as per bindings (Doug Anderson)
- Providing "ref" clk for dsi_phy (Doug Anderson)
- Sorting mdss node before dispcc (Doug Anderson)

This patch has dependency on the below series
https://lkml.org/lkml/2019/12/27/73

 arch/arm64/boot/dts/qcom/sc7180.dtsi | 136 ++-
 1 file changed, 134 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi 
b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index bd2584d..3ac1b87 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -1173,13 +1173,145 @@
#power-domain-cells = <1>;
};
 
+   mdss: mdss@ae0 {
+   compatible = "qcom,sc7180-mdss";
+   reg = <0 0x0ae0 0 0x1000>;
+   reg-names = "mdss";
+
+   power-domains = <&dispcc MDSS_GDSC>;
+
+   clocks = <&gcc GCC_DISP_AHB_CLK>,
+<&gcc GCC_DISP_HF_AXI_CLK>,
+<&dispcc DISP_CC_MDSS_AHB_CLK>,
+<&dispcc DISP_CC_MDSS_MDP_CLK>;
+   clock-names = "iface", "bus", "ahb", "core";
+
+   assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>;
+   assigned-clock-rates = <3>;
+
+   interrupts = ;
+   interrupt-controller;
+   #interrupt-cells = <1>;
+
+   iommus = <&apps_smmu 0x800 0x2>;
+
+   #address-cells = <2>;
+   #size-cells = <2>;
+   ranges;
+
+   status = "disabled";
+
+   mdp: mdp@ae01000 {
+   compatible = "qcom,sc7180-dpu";
+   reg = <0 0x0ae01000 0 0x8f000>,
+ <0 0x0aeb 0 0x2008>;
+   reg-names = "mdp", "vbif";
+
+   clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
+<&dispcc DISP_CC_MDSS_ROT_CLK>,
+<&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
+<&dispcc DISP_CC_MDSS_MDP_CLK>,
+<&dispcc DISP_CC_MDSS_VSYNC_CLK>;
+   clock-names = "iface", "rot", "lut", "core",
+ "vsync";
+   assigned-clocks = <&dispcc 
DISP_CC_MDSS_MDP_CLK>,
+ <&dispcc 
DISP_CC_MDSS_VSYNC_CLK>;
+   assigned-clock-rates = <3>,
+  <1920>;
+
+   interrupt-parent = <&mdss>;
+   interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+
+   status = "disabled";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+   dpu_intf1_out: endpoint {
+   remote-endpoint = 
<&dsi0_in>;
+   };
+   };
+   };
+   };
+
+   dsi0: dsi@ae94000 {
+   compatible = "qcom,mdss-dsi-ctrl";
+   reg = <0 0x0ae94000 0 0x400>;
+   reg-names = "dsi_ctrl";
+
+   interrupt-parent = <&mdss>;
+  

Re: [Freedreno] [v4] arm64: dts: sc7180: add display dt nodes

2020-02-04 Thread harigovi

On 2020-02-01 01:02, Doug Anderson wrote:

Hi,

On Tue, Jan 28, 2020 at 5:25 AM Harigovindan P 
 wrote:


Add display, DSI hardware DT nodes for sc7180.

Signed-off-by: Harigovindan P 
---

Changes in v1:
-Added display DT nodes for sc7180
Changes in v2:
-Renamed node names
-Corrected code alignments
-Removed extra new line
-Added DISP AHB clock for register access
under display_subsystem node for global settings
Changes in v3:
-Modified node names
-Modified hard coded values
-Removed mdss reg entry
Changes in v4:
-Reverting mdp node name
-Setting status to disabled in main SOC dtsi file
-Replacing _ to - for node names
-Adding clock dependency patch link
-Splitting idp dt file to a separate patch

This patch has dependency on the below series
https://lkml.org/lkml/2019/12/27/73
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 128 
+++

 1 file changed, 128 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi 
b/arch/arm64/boot/dts/qcom/sc7180.dtsi

index 3bc3f64..c3883af 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -1184,6 +1184,134 @@
#power-domain-cells = <1>;
};

+   mdss: mdss@ae0 {
+   compatible = "qcom,sc7180-mdss";
+   reg = <0 0x0ae0 0 0x1000>;
+   reg-names = "mdss";
+
+   power-domains = <&dispcc MDSS_GDSC>;
+
+   clocks = <&gcc GCC_DISP_AHB_CLK>,
+<&gcc GCC_DISP_HF_AXI_CLK>,
+<&dispcc DISP_CC_MDSS_AHB_CLK>,
+<&dispcc DISP_CC_MDSS_MDP_CLK>;
+   clock-names = "iface", "gcc_bus", "ahb", 
"core";


The clock "ahb" is not in your bindings.  If it is truly needed,
please update your bindings.

The clock "gcc_bus" is just called "bus" in the bindings.  Assuming
this is the same clock, please use the name from the bindings.


+   assigned-clocks = <&dispcc 
DISP_CC_MDSS_MDP_CLK>;

+   assigned-clock-rates = <3>;
+
+   interrupts = ;
+   interrupt-controller;
+   #interrupt-cells = <1>;
+
+   iommus = <&apps_smmu 0x800 0x2>;
+
+   #address-cells = <2>;
+   #size-cells = <2>;
+   ranges;


Do we need a:
  status = "disabled";

I noticed that in sdm845 the top-level mdss node _does_ have that.  If
someone was building a board with your chip and they had no display at
all, would they want this node disabled?  If so then it should be
disabled by default and boards should opt-in.



+   mdss_mdp: mdp@ae01000 {
+   compatible = "qcom,sc7180-dpu";
+   reg = <0 0x0ae01000 0 0x8f000>,
+ <0 0x0aeb 0 0x2008>,
+ <0 0x0af03000 0 0x16>;
+   reg-names = "mdp", "vbif", "disp_cc";


Did I already ask why you need the "disp_cc" register space?  If I
didn't, can I ask now?  This is not in the bindings and I can't think
of why you'd want it.  Does the code use it?  It doesn't seem to...


+   clocks = <&dispcc 
DISP_CC_MDSS_AHB_CLK>,
+<&dispcc 
DISP_CC_MDSS_ROT_CLK>,
+<&dispcc 
DISP_CC_MDSS_MDP_LUT_CLK>,
+<&dispcc 
DISP_CC_MDSS_MDP_CLK>,
+<&dispcc 
DISP_CC_MDSS_VSYNC_CLK>;
+   clock-names = "iface", "rot", "lut", 
"core",

+ "vsync";


Your bindings doc says that "bus" is required, yet you don't pass it.
Should you?


"bus" is optional due to architecture change. Will update bindings in a 
new patch.




Your bindings doc says nothing about "rot" and "lut".  Presumably
those should be added if they are actually needed?


+   assigned-clocks = <&dispcc 
DISP_CC_MDSS_MDP_CLK>,
+ <&dispcc 
DISP_CC_MDSS_VSYNC_CLK>;

+   assigned-clock-rates = <3>,
+  <1920>;
+
+   interrupt-parent = <&mdss>;
+   interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;


Do we need a:
  status = "disabled";

I noticed that in sdm845 the mdss_mdp node _does_ have that.  NOTE:
you'd only want to add it if it ever made sense to turn on the
top-level mdss node but not this one.  If this should always be
enabled at the exact same time as the top-level mdss node then