RE: [PATCH V2 1/2] arm: dts: exynos: Add GPU/Mali T604 node to exynos5250

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
> Sent: 25 July 2019 13:19
> To: Guillaume Gardet <Guillaume.Gardet@xxxxxxx>
> Cc: linux-samsung-soc@xxxxxxxxxxxxxxx; Kukjin Kim <kgene@xxxxxxxxxx>; linux-
> arm-kernel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH V2 1/2] arm: dts: exynos: Add GPU/Mali T604 node to
> exynos5250
>
> On Thu, 25 Jul 2019 at 10:34, Guillaume <guillaume.gardet@xxxxxxx> wrote:
> >
> > From: Guillaume GARDET <guillaume.gardet@xxxxxxx>
> >
> > Add nodes for GPU (Mali T604) to Exynos5250.
> > Tested with kmscube and glmark2-es2-drm on Chromebook snow.
> > Load tested on arndale board.
> >
> > Signed-off-by: Guillaume GARDET <guillaume.gardet@xxxxxxx>
> >
> > Cc: Kukjin Kim <kgene@xxxxxxxxxx>
> > Cc: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
> > Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> > ---
> > V2 changes:
> >   * move mali node as /soc sub-node
> >   * move gpu_opp_table as mali sub-node
> >   * minor style updates
> >   * test on arndale board
> >   * enable by default
>
> I was not specific enough. It can stay disabled in the DTSI because not all
> resources are provided. The board DTSes (all or ones which are
> tested) will enable it and provide missing resources - mali-supply.

I think all resources are provided, since no mali-supply property is needed here.
So, I can either enable it by default for all exynos5250 platforms there, or I can enable it only for Arndale board and Chromebook snow. What do you prefer?

> Just like:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f
> 0a6208b90bdd44f48f5718c8bb0eb1e763d14c0
>
> >   * add dt bindings, in a separate patch
>
> Put the bindings before DTS change.

Yes, makes sense.

>
> >
> >  arch/arm/boot/dts/exynos5250.dtsi | 47
> > +++++++++++++++++++++++++++++++
> >  1 file changed, 47 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/exynos5250.dtsi
> > b/arch/arm/boot/dts/exynos5250.dtsi
> > index d5e0392b409e..f7f34d469bde 100644
> > --- a/arch/arm/boot/dts/exynos5250.dtsi
> > +++ b/arch/arm/boot/dts/exynos5250.dtsi
> > @@ -328,6 +328,52 @@
> >                         iommus = <&sysmmu_rotator>;
> >                 };
> >
> > +               mali: gpu@11800000 {
> > +                       compatible = "samsung,exynos5250-mali", "arm,mali-t604";
> > +                       reg = <0x11800000 0x5000>;
> > +                       interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>,
> > +                                    <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>,
> > +                                    <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
> > +                       interrupt-names = "job", "mmu", "gpu";
> > +                       clocks = <&clock CLK_G3D>;
> > +                       clock-names = "g3d";
>
> According to bindings this is not proper clock name. I wonder why it was
> working... it seems the driver expects first clock to be always the core clock.

Yes, it works, because 1st clock is assumed to be the core clock.
Clock names are optional and could be omitted as in https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/rk3288.dtsi?h=v5.3-rc1#n1284
Not sure if it would be better to rename it to 'core', or  just skip it? Any opinion?

Thanks,
Guillaume

>
> Best regards,
> Krzysztof
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.




[Index of Archives]     [Linux SoC Development]     [Linux Rockchip Development]     [Linux for Synopsys ARC Processors]    
  • [Linux on Unisoc (RDA Micro) SoCs]     [Linux Actions SoC]     [Linux USB Development]     [Video for Linux]     [Linux Audio Users]     [Linux SCSI]     [Yosemite News]

  •   Powered by Linux