On 4/30/2020 11:51 PM, Doug Anderson wrote:
Hi,
On Thu, Apr 30, 2020 at 11:12 AM Jordan Crouse <jcrouse@xxxxxxxxxxxxxx> wrote:
On Thu, Apr 30, 2020 at 09:29:47AM +0530, Sharat Masetty wrote:
This patch adds a new compatible string for sc7180 and also an
additional clock listing needed to power the TBUs and the TCU.
Signed-off-by: Sharat Masetty <smasetty@xxxxxxxxxxxxxx>
---
v2: Addressed review comments from Doug
Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
index 6515dbe..ba5dba4 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
@@ -28,6 +28,7 @@ properties:
- enum:
- qcom,msm8996-smmu-v2
- qcom,msm8998-smmu-v2
+ - qcom,sc7180-smmu-v2
- qcom,sdm845-smmu-v2
- const: qcom,smmu-v2
@@ -113,16 +114,23 @@ properties:
present in such cases.
clock-names:
+ minItems: 2
+ maxItems: 3
items:
- const: bus
- const: iface
+ - const: mem_iface
Hi Sharat -
I think there was a bit of confusion due to renaming between downstream and
upstream. Currently for the sdm845 and friends we have:
clocks = <&gcc GCC_GPU_MEMNOC_GFX_CLK>,
<&gcc GCC_GPU_CFG_AHB_CLK>;
clock-names = "bus", "iface";
Confusingly these same clocks downstream are "mem_iface_clk" and "iface_clk"
respectively.
It looks like you are trying to add GCC_DDRSS_GPU_AXI_CLK as "mem_iface" which
was formerly "mem_clk" downstream. I'm not sure if the naming change is
intentional or you were trying to make upstream and downstream match and didn't
realize that they were renamed.
I'm not sure if we need DDRSS_GPU_AXI_CLK or not. Empirically it works without
it for sdm845 (I don't have a sc7180 to test) but we should probably loop back
with either the clock team or the hardware designers to be sure there isn't a
corner case that is missing. I agree with Doug that its always best if we don't
need to add a clock.
Thanks Jordan and Doug for the updates. My intention was to add the
third clock as listed downstream, but as you said the naming is a bit
misleading. From the clock GCC_DDRSS_GPU_AXI_CLK description, this is
needed for the GPU to DDR access and all transactions to the DDR from
the GPU go through the SMMU. It is listed in the SMMU dt node because
its needed by SMMU to perform pagetable walks.
I think we may be fine by not listing this clock in the SMMU node
because the same clock is listed in both the GMU and also the GPU.
I can confirm that on sc7180 the GPU seems to come up just fine
without the clock being specified in the iommu node. Definitely would
be good to know what's broken and if nothing is broken maybe we can
change this patch to just add the sc7180 compatible string and drop
the clock. I do note that the GMU already has a reference to the same
"GCC_DDRSS_GPU_AXI_CLK" clock.
-Doug
_______________________________________________
Freedreno mailing list
Freedreno@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/freedreno
