| Message ID | 20230315-topic-lagoon_gpu-v1-0-a74cbec4ecfc@linaro.org |
|---|---|
| Headers | show |
| Series | SM6350 GPU | expand |
On 16/03/2023 12:16, Konrad Dybcio wrote: > SM6350 GPUCC uses the same clock names as the rest of the gang, except > without a _src suffix. Account for that. Why not fixing the names instead (to use the same)? If the clocks are the same, why using different names for the inputs? To remind - these are not names of clocks in GCC, but names of clock inputs to the device. > > Fixes: 7b91b9d8cc6c ("dt-bindings: clock: add SM6350 QCOM Graphics clock bindings") > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > --- > .../devicetree/bindings/clock/qcom,gpucc.yaml | 29 +++++++++++++++++++--- > 1 file changed, 25 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml > index db53eb288995..d209060a619d 100644 > --- a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml > +++ b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml > @@ -43,10 +43,8 @@ properties: > - description: GPLL0 div branch source > > clock-names: > - items: > - - const: bi_tcxo > - - const: gcc_gpu_gpll0_clk_src > - - const: gcc_gpu_gpll0_div_clk_src > + minItems: 3 Drop minItems, not needed as it is implied by maxItems. > + maxItems: 3 > > '#clock-cells': > const: 1 > @@ -71,6 +69,29 @@ required: > > additionalProperties: false > > Best regards, Krzysztof
On Thu Mar 16, 2023 at 12:16 PM CET, Konrad Dybcio wrote: > From: Konrad Dybcio <konrad.dybcio@somainline.org> > > Add a node for the QFPROM NVMEM hw and define the GPU fuse. > > Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org> > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > --- > arch/arm64/boot/dts/qcom/sm6350.dtsi | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi b/arch/arm64/boot/dts/qcom/sm6350.dtsi > index 523c7edfa4b3..60b68d305e53 100644 > --- a/arch/arm64/boot/dts/qcom/sm6350.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi > @@ -637,6 +637,18 @@ ipcc: mailbox@408000 { > #mbox-cells = <2>; > }; > > + qfprom: qfprom@784000 { > + compatible = "qcom,sm6350-qfprom", "qcom,qfprom"; > + reg = <0 0x00784000 0 0x3000>; > + #address-cells = <1>; > + #size-cells = <1>; > + > + gpu_speed_bin: gpu_speed_bin@2015 { gpu-speed-bin@2015 ? With that fixed: Reviewed-by: Luca Weiss <luca.weiss@fairphone.com> > + reg = <0x2015 0x1>; > + bits = <0 8>; > + }; > + }; > + > rng: rng@793000 { > compatible = "qcom,prng-ee"; > reg = <0 0x00793000 0 0x1000>; > > -- > 2.39.2
On Thu Mar 16, 2023 at 12:17 PM CET, Konrad Dybcio wrote: > From: Konrad Dybcio <konrad.dybcio@somainline.org> > > The previous ZAP region definition was wrong. Fix it. > Note this is not a device-specific fixup, but a fixup to the generic > PIL load address. > > Fixes: 5f82b9cda61e ("arm64: dts: qcom: Add SM6350 device tree") > Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org> > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> Reviewed-by: Luca Weiss <luca.weiss@fairphone.com> > --- > arch/arm64/boot/dts/qcom/sm6350.dtsi | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi b/arch/arm64/boot/dts/qcom/sm6350.dtsi > index e967d06b0ad4..3fe4a5fa3021 100644 > --- a/arch/arm64/boot/dts/qcom/sm6350.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi > @@ -466,11 +466,6 @@ pil_ipa_gsi_mem: memory@8b710000 { > no-map; > }; > > - pil_gpu_mem: memory@8b715400 { > - reg = <0 0x8b715400 0 0x2000>; > - no-map; > - }; > - > pil_modem_mem: memory@8b800000 { > reg = <0 0x8b800000 0 0xf800000>; > no-map; > @@ -491,6 +486,11 @@ removed_region: memory@c0000000 { > no-map; > }; > > + pil_gpu_mem: memory@f0d00000 { > + reg = <0 0xf0d00000 0 0x1000>; > + no-map; > + }; > + > debug_region: memory@ffb00000 { > reg = <0 0xffb00000 0 0xc0000>; > no-map; > > -- > 2.39.2
On 17.03.2023 09:37, Krzysztof Kozlowski wrote: > On 16/03/2023 12:16, Konrad Dybcio wrote: >> SM6350 GPUCC uses the same clock names as the rest of the gang, except >> without a _src suffix. Account for that. > > Why not fixing the names instead (to use the same)? If the clocks are > the same, why using different names for the inputs? To remind - these > are not names of clocks in GCC, but names of clock inputs to the device. Considering SM6350 is the only used of SM6350_GPUCC and it's not yet in next and I don't think any other project using devicetree has Adreno up on any platform, let alone this one, I suppose the ABI could be broken and the driver could be made to expect the more common set of names? Or I could transition it to index-based lookup? Konrad > >> >> Fixes: 7b91b9d8cc6c ("dt-bindings: clock: add SM6350 QCOM Graphics clock bindings") >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >> --- >> .../devicetree/bindings/clock/qcom,gpucc.yaml | 29 +++++++++++++++++++--- >> 1 file changed, 25 insertions(+), 4 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml >> index db53eb288995..d209060a619d 100644 >> --- a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml >> +++ b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml >> @@ -43,10 +43,8 @@ properties: >> - description: GPLL0 div branch source >> >> clock-names: >> - items: >> - - const: bi_tcxo >> - - const: gcc_gpu_gpll0_clk_src >> - - const: gcc_gpu_gpll0_div_clk_src >> + minItems: 3 > > Drop minItems, not needed as it is implied by maxItems. > >> + maxItems: 3 >> >> '#clock-cells': >> const: 1 >> @@ -71,6 +69,29 @@ required: >> >> additionalProperties: false >> >> > > Best regards, > Krzysztof >
On 17.03.2023 13:11, Konrad Dybcio wrote: > > > On 17.03.2023 09:37, Krzysztof Kozlowski wrote: >> On 16/03/2023 12:16, Konrad Dybcio wrote: >>> SM6350 GPUCC uses the same clock names as the rest of the gang, except >>> without a _src suffix. Account for that. >> >> Why not fixing the names instead (to use the same)? If the clocks are >> the same, why using different names for the inputs? To remind - these >> are not names of clocks in GCC, but names of clock inputs to the device. > Considering SM6350 is the only used of SM6350_GPUCC and it's not yet > in next and I don't think any other project using devicetree has > Adreno up on any platform, let alone this one, I suppose the ABI could > be broken and the driver could be made to expect the more common set > of names? Or I could transition it to index-based lookup? Comments, please? Konrad > > Konrad >> >>> >>> Fixes: 7b91b9d8cc6c ("dt-bindings: clock: add SM6350 QCOM Graphics clock bindings") >>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >>> --- >>> .../devicetree/bindings/clock/qcom,gpucc.yaml | 29 +++++++++++++++++++--- >>> 1 file changed, 25 insertions(+), 4 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml >>> index db53eb288995..d209060a619d 100644 >>> --- a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml >>> +++ b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml >>> @@ -43,10 +43,8 @@ properties: >>> - description: GPLL0 div branch source >>> >>> clock-names: >>> - items: >>> - - const: bi_tcxo >>> - - const: gcc_gpu_gpll0_clk_src >>> - - const: gcc_gpu_gpll0_div_clk_src >>> + minItems: 3 >> >> Drop minItems, not needed as it is implied by maxItems. >> >>> + maxItems: 3 >>> >>> '#clock-cells': >>> const: 1 >>> @@ -71,6 +69,29 @@ required: >>> >>> additionalProperties: false >>> >>> >> >> Best regards, >> Krzysztof >>
Add all the required nodes for SM6350's A619 and fix up its GPUCC bindings. This has been ready for like 1.5y now, time to finally merge it as the display part will take some more time (due to the HW catalog rework). Depends on (bindings, admittedly I could have organized it better): https://lore.kernel.org/linux-arm-msm/20230314-topic-nvmem_compats-v1-0-508100c17603@linaro.org/#t Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> --- Konrad Dybcio (5): dt-bindings: clock: qcom,gpucc: Fix SM6350 clock names arm64: dts: qcom: sm6350: Add GPUCC node arm64: dts: qcom: sm6350: Add QFPROM node arm64: dts: qcom: sm6350: Add GPU nodes arm64: dts: qcom: sm6350: Fix ZAP region .../devicetree/bindings/clock/qcom,gpucc.yaml | 29 +++- arch/arm64/boot/dts/qcom/sm6350.dtsi | 177 ++++++++++++++++++++- 2 files changed, 197 insertions(+), 9 deletions(-) --- base-commit: 225b6b81afe63b3850b7cee0a3590f51144f2a75 change-id: 20230315-topic-lagoon_gpu-8c2abccbc6eb Best regards,