Re: [PATCH v1 3/3] arm64: dts: qcom: x1e80100: Add gpu support

Akhil P Oommen
2024-06-23T21:01:38Z

On Sun, Jun 23, 2024 at 03:40:06PM -0500, Bjorn Andersson wrote:
> On Sun, Jun 23, 2024 at 08:46:30PM GMT, Akhil P Oommen wrote:
> > On Sun, Jun 23, 2024 at 02:53:17PM +0200, Krzysztof Kozlowski wrote:
> > > On 23/06/2024 14:28, Akhil P Oommen wrote:
> > > > On Sun, Jun 23, 2024 at 01:17:16PM +0200, Krzysztof Kozlowski wrote:
> > > >> On 23/06/2024 13:06, Akhil P Oommen wrote:
> > > >>> Add the necessary dt nodes for gpu support in X1E80100.
> > > >>>
> > > >>> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> > > >>> ---
> > > >>> + gmu: gmu@3d6a000 {
> > > >>> + compatible = "qcom,adreno-gmu-x185.1", "qcom,adreno-gmu";
> > > >>> + reg = <0x0 0x03d50000 0x0 0x10000>,
> > > >>> + <0x0 0x03d6a000 0x0 0x35000>,
> > > >>> + <0x0 0x0b280000 0x0 0x10000>;
> > > >>> + reg-names = "rscc", "gmu", "gmu_pdc";
> > > >>
> > > >> Really, please start testing your patches. Your internal instructions
> > > >> tells you to do that, so please follow it carefully. Don't use the
> > > >> community as the tool, because you do not want to run checks and
> > > >> investigate results.
> > > >
> > > > This was obviously tested before (and retested now) and everything works. I am
> > > > confused about what you meant. Could you please elaborate a bit? The device
> > > > and the compilation/test setup is new for me, so I am wondering if I
> > > > made any silly mistake!
> > >
> > > Eh, your DTS is not correct, but this could not be pointed out by tests,
> > > because the binding does not work. :(
> >
> > I reordered both "reg" and "reg-names" arrays based on the address.
>
> The @3d6a000 should match the first reg entry.
>
> > Not sure if
> > that is what we are talking about here. Gpu driver uses platform_get_resource_byname()
> > to query mmio resources.
> >
> > I will retest dt-bindings and dts checks after picking the patches you
> > just posted and report back. Is the schema supposed to enforce strict
> > order?
>
> In your second hunk in patch 1, you are defining the order of reg,
> reg-names, clocks, and clock-names. This creates an ABI between DTB and
> implementation where ordering is significant - regardless of Linux using
> platform_get_resource_byname().

I will revert this to the original order. Thanks for the clarification, Bjorn/Krzysztof.

-Akhil.
>
> Regards,
> Bjorn
>
> >
> > -Akhil.
> > >
> > > I'll fix up the binding and then please test on top of my patch (see
> > > your internal guideline about necessary tests before sending any binding
> > > or DTS patch).
> > >
> > > Best regards,
> > > Krzysztof
> > >