[Freedreno] [PATCH v4 0/3] sc7180: Add A618 GPU bindings
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
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
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
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
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
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
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
+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
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
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
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
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
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
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