On Mon, Jan 27, 2020 at 02:29:53PM -0800, Doug Anderson wrote: > Hi, > > On Mon, Jan 27, 2020 at 1:30 AM Sharat Masetty <smasetty@xxxxxxxxxxxxxx> wrote: > > > > This patch adds the required dt nodes and properties > > to enabled A618 GPU. > > > > Signed-off-by: Sharat Masetty <smasetty@xxxxxxxxxxxxxx> > > --- > > arch/arm64/boot/dts/qcom/sc7180.dtsi | 103 +++++++++++++++++++++++++++++++++++ > > 1 file changed, 103 insertions(+) > > Note that +Matthias Kaehlcke commented on v1 your patch: > > https://lore.kernel.org/r/20191204220033.GH228856@xxxxxxxxxx/ > > ...so he should have been CCed on v2. I would also note that some of > the comments below are echos of what Matthias already said in the > previous version but just weren't addressed. > > > > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi > > index b859431..277d84d 100644 > > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi > > @@ -7,6 +7,7 @@ > > > > #include <dt-bindings/clock/qcom,gcc-sc7180.h> > > #include <dt-bindings/clock/qcom,rpmh.h> > > +#include <dt-bindings/clock/qcom,gpucc-sc7180.h> > > Header files should be sorted alphabetically. ...or, even better, > base your patch atop mine: > > https://lore.kernel.org/r/20200124144154.v2.10.I1a4b93fb005791e29a9dcf288fc8bd459a555a59@changeid/ > > ...which adds the gpucc header file so you don't have to. ...and when > you do so, email out a Reviewed-by and/or Tested-by for my patch. ;-) > > > > #include <dt-bindings/interrupt-controller/arm-gic.h> > > #include <dt-bindings/interconnect/qcom,sc7180.h> > > #include <dt-bindings/phy/phy-qcom-qusb2.h> > > @@ -1619,6 +1620,108 @@ > > #interconnect-cells = <1>; > > qcom,bcm-voters = <&apps_bcm_voter>; > > }; > > + > > + gpu: gpu@5000000 { > > + compatible = "qcom,adreno-618.0", "qcom,adreno"; > > Though it's not controversial, please send a patch to: > > Documentation/devicetree/bindings/display/msm/gmu.txt > > ...to add 'qcom,adreno-618.0', like: > > for example: > "qcom,adreno-gmu-618.0", "qcom,adreno-gmu" > "qcom,adreno-gmu-630.2", "qcom,adreno-gmu" > > Probably as part of this you will be asked to convert this file to > yaml. IMO we don't need to block landing this patch on the effort to > convert it to yaml, but you should still work on it. ...or maybe > Jordan wants to work on it? I'll toss it on my to-do list. There always seems to be something more urgent but I should just bite the bullet and get it done. Jordan -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
