Re: [PATCH v2 06/10] dt-bindings: gpu: mali-bifrost: Add a compatible for MediaTek MT8186

[AngeloGioacchino Del Regno]

Il 23/02/23 03:43, Chen-Yu Tsai ha scritto:
> On Wed, Feb 22, 2023 at 5:13 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@xxxxxxxxxxxxx> wrote:
> > Il 22/02/23 09:37, Chen-Yu Tsai ha scritto:
> > > On Tue, Feb 21, 2023 at 11:37 PM AngeloGioacchino Del Regno
> > > <angelogioacchino.delregno@xxxxxxxxxxxxx> wrote:
> > > > Get GPU support on MT8186 by adding its compatible.
> > > >
> > > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>
> > > > ---
> > > >    Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml | 5 +++++
> > > >    1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
> > > > index be18b161959b..43a841d4e94d 100644
> > > > --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
> > > > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
> > > > @@ -15,6 +15,11 @@ properties:
> > > >
> > > >      compatible:
> > > >        oneOf:
> > > > +      - items:
> > > > +          - enum:
> > > > +              - mediatek,mt8186-mali
> > > > +          - const: mediatek,mt8183b-mali
> > > > +          - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable
> > >
> > > The MT8186 has Mali-G52 MC2 2EE, while the MT8183 has Mali-G72 MP3.
> >
> > Keeping in mind the obvious - which is that G52 and G72 are both Bifrost....
> >
> > > So we actually need a new entry with two power domains.
> >
> > ...This is my node for MT8186:
> >
> >                  gpu: gpu@13040000 {
> >                          compatible = "mediatek,mt8186-mali",
> >                                       "mediatek,mt8183b-mali",
> >                                       "arm,mali-bifrost";
> >                          reg = <0 0x13040000 0 0x4000>;
> >
> >                          clocks = <&mfgsys CLK_MFG_BG3D>;
> >                          interrupts = <GIC_SPI 276 IRQ_TYPE_LEVEL_HIGH 0>,
> >                                       <GIC_SPI 275 IRQ_TYPE_LEVEL_HIGH 0>,
> >                                       <GIC_SPI 274 IRQ_TYPE_LEVEL_HIGH 0>;
> >                          interrupt-names = "job", "mmu", "gpu";
> >                          power-domains = <&spm MT8186_POWER_DOMAIN_MFG1>,
> >                                          <&spm MT8186_POWER_DOMAIN_MFG2>,
> >                                          <&spm MT8186_POWER_DOMAIN_MFG3>;
> >                          power-domain-names = "core0", "core1", "core2";
> >
> >                          /* Please ignore speedbin, that's for another time :-) */
> >                          nvmem-cells = <&gpu_volt_bin>;
> >                          nvmem-cell-names = "speed-bin";
> >                          #cooling-cells = <2>;
> >                  };
> >
> > There are three MFG power domains... MFG2 and MFG3 are parents of MFG1, on that
> > I agree, but we can avoid adding a new entry just for MT8186 and use the MT8183-b
> > one while still being technically correct.
> >
> > Besides, Mali G52 and Mali G72 are both Bifrost... so I don't think that this
> > commit is incorrect. For the sake of simplicity, I would push on getting this
> > one picked.
>
> I'm aware. In case it wasn't obvious, Mali-G52 MC2 2EE has 2 cores, while
> Mali-G72 MP3 has 3 cores. I think that is reason enough to do a new entry.
> Otherwise you are describing power domains for 3 cores for a GPU that only
> has two.
>
> > Unless there are any real-strong opinions against...
>
> Yes.

Ok, I will do that for v3!

Cheers,
Angelo

> ChenYu