[PATCH 3/6] dt-bindings: gpu: powervr-rogue: Add MediaTek MT8173 GPU

Chen-Yu Tsai posted 6 patches 3 months, 3 weeks ago
[PATCH 3/6] dt-bindings: gpu: powervr-rogue: Add MediaTek MT8173 GPU
Posted by Chen-Yu Tsai 3 months, 3 weeks ago
The MediaTek MT8173 comes with a PowerVR Rogue GX6250, which is one
of the Series6XT GPUs, another sub-family of the Rogue family.

This was part of the very first few versions of the PowerVR submission,
but was later dropped. The compatible string has been updated to follow
the new naming scheme adopted for the AXE series.

In a previous iteration of the PowerVR binding submission [1], the
number of clocks required for the 6XT family was mentioned to be
always 3. This is also reflected here.

[1] https://lore.kernel.org/dri-devel/6eeccb26e09aad67fb30ffcd523c793a43c79c2a.camel@imgtec.com/

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 .../bindings/gpu/img,powervr-rogue.yaml       | 24 +++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
index 256e252f8087..48aa205b66b4 100644
--- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
+++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
@@ -12,10 +12,17 @@ maintainers:
 
 properties:
   compatible:
-    items:
-      - enum:
-          - ti,am62-gpu
-      - const: img,img-axe # IMG AXE GPU model/revision is fully discoverable
+    oneOf:
+      - items:
+          - enum:
+              - mediatek,mt8173-gpu
+          # PowerVR 6XT GPU model/revision is fully discoverable
+          - const: img,powervr-6xt
+      - items:
+          - enum:
+              - ti,am62-gpu
+          # IMG AXE GPU model/revision is fully discoverable
+          - const: img,img-axe
 
   reg:
     maxItems: 1
@@ -56,6 +63,15 @@ allOf:
       properties:
         clocks:
           maxItems: 1
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: img,powervr-6xt
+    then:
+      properties:
+        clocks:
+          minItems: 3
 
 examples:
   - |
-- 
2.45.1.288.g0e0cd299f1-goog
Re: [PATCH 3/6] dt-bindings: gpu: powervr-rogue: Add MediaTek MT8173 GPU
Posted by Frank Binns 3 months, 3 weeks ago
Hi ChenYu,

On Thu, 2024-05-30 at 16:35 +0800, Chen-Yu Tsai wrote:
> The MediaTek MT8173 comes with a PowerVR Rogue GX6250, which is one
> of the Series6XT GPUs, another sub-family of the Rogue family.

I've added Adam Ford who sent out some DT related patches [1] for the Renesas
variant of GX6250 and the GX6650 (another Series6XT GPU).

> 
> This was part of the very first few versions of the PowerVR submission,
> but was later dropped. The compatible string has been updated to follow
> the new naming scheme adopted for the AXE series.
> 
> In a previous iteration of the PowerVR binding submission [1], the
> number of clocks required for the 6XT family was mentioned to be
> always 3. This is also reflected here.
> 
> [1] https://lore.kernel.org/dri-devel/6eeccb26e09aad67fb30ffcd523c793a43c79c2a.camel@imgtec.com/
> 
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
>  .../bindings/gpu/img,powervr-rogue.yaml       | 24 +++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> index 256e252f8087..48aa205b66b4 100644
> --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> @@ -12,10 +12,17 @@ maintainers:
>  
>  properties:
>    compatible:
> -    items:
> -      - enum:
> -          - ti,am62-gpu
> -      - const: img,img-axe # IMG AXE GPU model/revision is fully discoverable
> +    oneOf:
> +      - items:
> +          - enum:
> +              - mediatek,mt8173-gpu
> +          # PowerVR 6XT GPU model/revision is fully discoverable
> +          - const: img,powervr-6xt
> +      - items:
> +          - enum:
> +              - ti,am62-gpu
> +          # IMG AXE GPU model/revision is fully discoverable
> +          - const: img,img-axe

The Series6XT GPU models have differing numbers of power domains (either 2, 4 or
5). Whereas, the AXE GPUs have a single power domain, so I assume there should
be a related change here.

The GX6250 has two power domains (lets call them A and B). There's a constraint
that if domain B is powered then domain A must also be powered.

In patch 6 [2] it's setting the power domain to MT8173_POWER_DOMAIN_MFG, which I
believe corresponds to power domain B. I assume this works because the MTK power
controller driver is encoding the constraint above, meaning that when we disable
or enable MT8173_POWER_DOMAIN_MFG it's also disabling/enabling MT8173_POWER_DOMA
IN_MFG_2D (domain A).

Thanks
Frank

[1] https://lists.freedesktop.org/archives/dri-devel/2024-February/443548.html
[2] https://lists.freedesktop.org/archives/dri-devel/2024-May/455833.html

>  
>    reg:
>      maxItems: 1
> @@ -56,6 +63,15 @@ allOf:
>        properties:
>          clocks:
>            maxItems: 1
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: img,powervr-6xt
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 3
>  
>  examples:
>    - |
Re: [PATCH 3/6] dt-bindings: gpu: powervr-rogue: Add MediaTek MT8173 GPU
Posted by Chen-Yu Tsai 3 months, 2 weeks ago
On Fri, May 31, 2024 at 9:37 PM Frank Binns <Frank.Binns@imgtec.com> wrote:
>
> Hi ChenYu,
>
> On Thu, 2024-05-30 at 16:35 +0800, Chen-Yu Tsai wrote:
> > The MediaTek MT8173 comes with a PowerVR Rogue GX6250, which is one
> > of the Series6XT GPUs, another sub-family of the Rogue family.
>
> I've added Adam Ford who sent out some DT related patches [1] for the Renesas
> variant of GX6250 and the GX6650 (another Series6XT GPU).
>
> >
> > This was part of the very first few versions of the PowerVR submission,
> > but was later dropped. The compatible string has been updated to follow
> > the new naming scheme adopted for the AXE series.
> >
> > In a previous iteration of the PowerVR binding submission [1], the
> > number of clocks required for the 6XT family was mentioned to be
> > always 3. This is also reflected here.
> >
> > [1] https://lore.kernel.org/dri-devel/6eeccb26e09aad67fb30ffcd523c793a43c79c2a.camel@imgtec.com/
> >
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > ---
> >  .../bindings/gpu/img,powervr-rogue.yaml       | 24 +++++++++++++++----
> >  1 file changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > index 256e252f8087..48aa205b66b4 100644
> > --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > @@ -12,10 +12,17 @@ maintainers:
> >
> >  properties:
> >    compatible:
> > -    items:
> > -      - enum:
> > -          - ti,am62-gpu
> > -      - const: img,img-axe # IMG AXE GPU model/revision is fully discoverable
> > +    oneOf:
> > +      - items:
> > +          - enum:
> > +              - mediatek,mt8173-gpu
> > +          # PowerVR 6XT GPU model/revision is fully discoverable
> > +          - const: img,powervr-6xt
> > +      - items:
> > +          - enum:
> > +              - ti,am62-gpu
> > +          # IMG AXE GPU model/revision is fully discoverable
> > +          - const: img,img-axe
>
> The Series6XT GPU models have differing numbers of power domains (either 2, 4 or
> 5). Whereas, the AXE GPUs have a single power domain, so I assume there should
> be a related change here.
>
> The GX6250 has two power domains (lets call them A and B). There's a constraint
> that if domain B is powered then domain A must also be powered.
>
> In patch 6 [2] it's setting the power domain to MT8173_POWER_DOMAIN_MFG, which I
> believe corresponds to power domain B. I assume this works because the MTK power
> controller driver is encoding the constraint above, meaning that when we disable
> or enable MT8173_POWER_DOMAIN_MFG it's also disabling/enabling MT8173_POWER_DOMA
> IN_MFG_2D (domain A).

It could also be that the power domains are split in the glue layer and there
is some sequencing handled there. I'll reach out to MediaTek to see if they
can dig up some design specifics.

I assume you would like to see the separate power domains properly modeled
in the device tree?


Thanks
ChenYu

> Thanks
> Frank
>
> [1] https://lists.freedesktop.org/archives/dri-devel/2024-February/443548.html
> [2] https://lists.freedesktop.org/archives/dri-devel/2024-May/455833.html
>
> >
> >    reg:
> >      maxItems: 1
> > @@ -56,6 +63,15 @@ allOf:
> >        properties:
> >          clocks:
> >            maxItems: 1
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: img,powervr-6xt
> > +    then:
> > +      properties:
> > +        clocks:
> > +          minItems: 3
> >
> >  examples:
> >    - |
Re: [PATCH 3/6] dt-bindings: gpu: powervr-rogue: Add MediaTek MT8173 GPU
Posted by Chen-Yu Tsai 3 months, 1 week ago
On Tue, Jun 4, 2024 at 12:18 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> On Fri, May 31, 2024 at 9:37 PM Frank Binns <Frank.Binns@imgtec.com> wrote:
> >
> > Hi ChenYu,
> >
> > On Thu, 2024-05-30 at 16:35 +0800, Chen-Yu Tsai wrote:
> > > The MediaTek MT8173 comes with a PowerVR Rogue GX6250, which is one
> > > of the Series6XT GPUs, another sub-family of the Rogue family.
> >
> > I've added Adam Ford who sent out some DT related patches [1] for the Renesas
> > variant of GX6250 and the GX6650 (another Series6XT GPU).
> >
> > >
> > > This was part of the very first few versions of the PowerVR submission,
> > > but was later dropped. The compatible string has been updated to follow
> > > the new naming scheme adopted for the AXE series.
> > >
> > > In a previous iteration of the PowerVR binding submission [1], the
> > > number of clocks required for the 6XT family was mentioned to be
> > > always 3. This is also reflected here.
> > >
> > > [1] https://lore.kernel.org/dri-devel/6eeccb26e09aad67fb30ffcd523c793a43c79c2a.camel@imgtec.com/
> > >
> > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > > ---
> > >  .../bindings/gpu/img,powervr-rogue.yaml       | 24 +++++++++++++++----
> > >  1 file changed, 20 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > > index 256e252f8087..48aa205b66b4 100644
> > > --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > > +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > > @@ -12,10 +12,17 @@ maintainers:
> > >
> > >  properties:
> > >    compatible:
> > > -    items:
> > > -      - enum:
> > > -          - ti,am62-gpu
> > > -      - const: img,img-axe # IMG AXE GPU model/revision is fully discoverable
> > > +    oneOf:
> > > +      - items:
> > > +          - enum:
> > > +              - mediatek,mt8173-gpu
> > > +          # PowerVR 6XT GPU model/revision is fully discoverable
> > > +          - const: img,powervr-6xt
> > > +      - items:
> > > +          - enum:
> > > +              - ti,am62-gpu
> > > +          # IMG AXE GPU model/revision is fully discoverable
> > > +          - const: img,img-axe
> >
> > The Series6XT GPU models have differing numbers of power domains (either 2, 4 or
> > 5). Whereas, the AXE GPUs have a single power domain, so I assume there should
> > be a related change here.
> >
> > The GX6250 has two power domains (lets call them A and B). There's a constraint
> > that if domain B is powered then domain A must also be powered.
> >
> > In patch 6 [2] it's setting the power domain to MT8173_POWER_DOMAIN_MFG, which I
> > believe corresponds to power domain B. I assume this works because the MTK power
> > controller driver is encoding the constraint above, meaning that when we disable
> > or enable MT8173_POWER_DOMAIN_MFG it's also disabling/enabling MT8173_POWER_DOMA
> > IN_MFG_2D (domain A).
>
> It could also be that the power domains are split in the glue layer and there
> is some sequencing handled there. I'll reach out to MediaTek to see if they
> can dig up some design specifics.

Unfortunately they said they no longer have that information.

> I assume you would like to see the separate power domains properly modeled
> in the device tree?

So how should we go about this? Adam, do you have this information for
your platform?

Thanks
ChenYu

>
> Thanks
> ChenYu
>
> > Thanks
> > Frank
> >
> > [1] https://lists.freedesktop.org/archives/dri-devel/2024-February/443548.html
> > [2] https://lists.freedesktop.org/archives/dri-devel/2024-May/455833.html
> >
> > >
> > >    reg:
> > >      maxItems: 1
> > > @@ -56,6 +63,15 @@ allOf:
> > >        properties:
> > >          clocks:
> > >            maxItems: 1
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            const: img,powervr-6xt
> > > +    then:
> > > +      properties:
> > > +        clocks:
> > > +          minItems: 3
> > >
> > >  examples:
> > >    - |
Re: [PATCH 3/6] dt-bindings: gpu: powervr-rogue: Add MediaTek MT8173 GPU
Posted by Adam Ford 3 months, 1 week ago
On Thu, Jun 13, 2024 at 4:10 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> On Tue, Jun 4, 2024 at 12:18 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
> >
> > On Fri, May 31, 2024 at 9:37 PM Frank Binns <Frank.Binns@imgtec.com> wrote:
> > >
> > > Hi ChenYu,
> > >
> > > On Thu, 2024-05-30 at 16:35 +0800, Chen-Yu Tsai wrote:
> > > > The MediaTek MT8173 comes with a PowerVR Rogue GX6250, which is one
> > > > of the Series6XT GPUs, another sub-family of the Rogue family.
> > >
> > > I've added Adam Ford who sent out some DT related patches [1] for the Renesas
> > > variant of GX6250 and the GX6650 (another Series6XT GPU).
> > >
> > > >
> > > > This was part of the very first few versions of the PowerVR submission,
> > > > but was later dropped. The compatible string has been updated to follow
> > > > the new naming scheme adopted for the AXE series.
> > > >
> > > > In a previous iteration of the PowerVR binding submission [1], the
> > > > number of clocks required for the 6XT family was mentioned to be
> > > > always 3. This is also reflected here.
> > > >
> > > > [1] https://lore.kernel.org/dri-devel/6eeccb26e09aad67fb30ffcd523c793a43c79c2a.camel@imgtec.com/
> > > >
> > > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > > > ---
> > > >  .../bindings/gpu/img,powervr-rogue.yaml       | 24 +++++++++++++++----
> > > >  1 file changed, 20 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > > > index 256e252f8087..48aa205b66b4 100644
> > > > --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > > > +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > > > @@ -12,10 +12,17 @@ maintainers:
> > > >
> > > >  properties:
> > > >    compatible:
> > > > -    items:
> > > > -      - enum:
> > > > -          - ti,am62-gpu
> > > > -      - const: img,img-axe # IMG AXE GPU model/revision is fully discoverable
> > > > +    oneOf:
> > > > +      - items:
> > > > +          - enum:
> > > > +              - mediatek,mt8173-gpu
> > > > +          # PowerVR 6XT GPU model/revision is fully discoverable
> > > > +          - const: img,powervr-6xt
> > > > +      - items:
> > > > +          - enum:
> > > > +              - ti,am62-gpu
> > > > +          # IMG AXE GPU model/revision is fully discoverable
> > > > +          - const: img,img-axe
> > >
> > > The Series6XT GPU models have differing numbers of power domains (either 2, 4 or
> > > 5). Whereas, the AXE GPUs have a single power domain, so I assume there should
> > > be a related change here.
> > >
> > > The GX6250 has two power domains (lets call them A and B). There's a constraint
> > > that if domain B is powered then domain A must also be powered.
> > >
> > > In patch 6 [2] it's setting the power domain to MT8173_POWER_DOMAIN_MFG, which I
> > > believe corresponds to power domain B. I assume this works because the MTK power
> > > controller driver is encoding the constraint above, meaning that when we disable
> > > or enable MT8173_POWER_DOMAIN_MFG it's also disabling/enabling MT8173_POWER_DOMA
> > > IN_MFG_2D (domain A).
> >
> > It could also be that the power domains are split in the glue layer and there
> > is some sequencing handled there. I'll reach out to MediaTek to see if they
> > can dig up some design specifics.
>
> Unfortunately they said they no longer have that information.
>
> > I assume you would like to see the separate power domains properly modeled
> > in the device tree?
>
> So how should we go about this? Adam, do you have this information for
> your platform?

In the Renesas platform, I only had to add one clock [1] and one
power-domain [2] to get the GPU to come up.  In Renesas' downstream
driver, they only use one clock, but Geert, the Renesas tree
maintainer, sent me the three clocks to assign to the 6xt graphics if
it's determined that 3 clocks are required.  In terms of the power
domain, there appear to be 2, but one is dependent on another, so
turning on the 'B' power domain turns on the 'A' power domain
automatically.  I should note that I can't get the graphics to
function, since the 6xt isn't supported yet, so there could be some
elements missing that I am unaware of.

adam

[1] - https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/clk/renesas?h=next-20240613&id=f7b0dfffd3e0897ca73916a0c3d3fb61c61df51e
[2] - https://patchwork.kernel.org/project/linux-renesas-soc/patch/20240227034539.193573-3-aford173@gmail.com/



>
> Thanks
> ChenYu
>
> >
> > Thanks
> > ChenYu
> >
> > > Thanks
> > > Frank
> > >
> > > [1] https://lists.freedesktop.org/archives/dri-devel/2024-February/443548.html
> > > [2] https://lists.freedesktop.org/archives/dri-devel/2024-May/455833.html
> > >
> > > >
> > > >    reg:
> > > >      maxItems: 1
> > > > @@ -56,6 +63,15 @@ allOf:
> > > >        properties:
> > > >          clocks:
> > > >            maxItems: 1
> > > > +  - if:
> > > > +      properties:
> > > > +        compatible:
> > > > +          contains:
> > > > +            const: img,powervr-6xt
> > > > +    then:
> > > > +      properties:
> > > > +        clocks:
> > > > +          minItems: 3
> > > >
> > > >  examples:
> > > >    - |
Re: [PATCH 3/6] dt-bindings: gpu: powervr-rogue: Add MediaTek MT8173 GPU
Posted by Adam Ford 3 months, 3 weeks ago
On Fri, May 31, 2024 at 8:37 AM Frank Binns <Frank.Binns@imgtec.com> wrote:
>
> Hi ChenYu,
>
> On Thu, 2024-05-30 at 16:35 +0800, Chen-Yu Tsai wrote:
> > The MediaTek MT8173 comes with a PowerVR Rogue GX6250, which is one
> > of the Series6XT GPUs, another sub-family of the Rogue family.
>
> I've added Adam Ford who sent out some DT related patches [1] for the Renesas
> variant of GX6250 and the GX6650 (another Series6XT GPU).
>

Thanks for including me.

> >
> > This was part of the very first few versions of the PowerVR submission,
> > but was later dropped. The compatible string has been updated to follow
> > the new naming scheme adopted for the AXE series.
> >
> > In a previous iteration of the PowerVR binding submission [1], the
> > number of clocks required for the 6XT family was mentioned to be
> > always 3. This is also reflected here.
> >
> > [1] https://lore.kernel.org/dri-devel/6eeccb26e09aad67fb30ffcd523c793a43c79c2a.camel@imgtec.com/
> >
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > ---
> >  .../bindings/gpu/img,powervr-rogue.yaml       | 24 +++++++++++++++----
> >  1 file changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > index 256e252f8087..48aa205b66b4 100644
> > --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > @@ -12,10 +12,17 @@ maintainers:
> >
> >  properties:
> >    compatible:
> > -    items:
> > -      - enum:
> > -          - ti,am62-gpu
> > -      - const: img,img-axe # IMG AXE GPU model/revision is fully discoverable
> > +    oneOf:
> > +      - items:
> > +          - enum:
> > +              - mediatek,mt8173-gpu
> > +          # PowerVR 6XT GPU model/revision is fully discoverable
> > +          - const: img,powervr-6xt
> > +      - items:
> > +          - enum:
> > +              - ti,am62-gpu
> > +          # IMG AXE GPU model/revision is fully discoverable
> > +          - const: img,img-axe
>
> The Series6XT GPU models have differing numbers of power domains (either 2, 4 or
> 5). Whereas, the AXE GPUs have a single power domain, so I assume there should
> be a related change here.
>
> The GX6250 has two power domains (lets call them A and B). There's a constraint
> that if domain B is powered then domain A must also be powered.
>
> In patch 6 [2] it's setting the power domain to MT8173_POWER_DOMAIN_MFG, which I
> believe corresponds to power domain B. I assume this works because the MTK power
> controller driver is encoding the constraint above, meaning that when we disable
> or enable MT8173_POWER_DOMAIN_MFG it's also disabling/enabling MT8173_POWER_DOMA
> IN_MFG_2D (domain A).
>

In the cover letter of this series, it was noted that the GPU
enumerates, but it doesn' fully function yet.  This is also the case
for both of the Renesas variants I have been testing, and I was nicely
asked to postpone my series until the driver was closer to being
ready.

Even if the driver isn't ready yet, it would be nice to move the
bindings forward.

adam

> Thanks
> Frank
>
> [1] https://lists.freedesktop.org/archives/dri-devel/2024-February/443548.html
> [2] https://lists.freedesktop.org/archives/dri-devel/2024-May/455833.html
>
> >
> >    reg:
> >      maxItems: 1
> > @@ -56,6 +63,15 @@ allOf:
> >        properties:
> >          clocks:
> >            maxItems: 1
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: img,powervr-6xt
> > +    then:
> > +      properties:
> > +        clocks:
> > +          minItems: 3
> >
> >  examples:
> >    - |
Re: [PATCH 3/6] dt-bindings: gpu: powervr-rogue: Add MediaTek MT8173 GPU
Posted by Chen-Yu Tsai 3 months, 2 weeks ago
On Fri, May 31, 2024 at 10:25 PM Adam Ford <aford173@gmail.com> wrote:
>
> On Fri, May 31, 2024 at 8:37 AM Frank Binns <Frank.Binns@imgtec.com> wrote:
> >
> > Hi ChenYu,
> >
> > On Thu, 2024-05-30 at 16:35 +0800, Chen-Yu Tsai wrote:
> > > The MediaTek MT8173 comes with a PowerVR Rogue GX6250, which is one
> > > of the Series6XT GPUs, another sub-family of the Rogue family.
> >
> > I've added Adam Ford who sent out some DT related patches [1] for the Renesas
> > variant of GX6250 and the GX6650 (another Series6XT GPU).
> >
>
> Thanks for including me.
>
> > >
> > > This was part of the very first few versions of the PowerVR submission,
> > > but was later dropped. The compatible string has been updated to follow
> > > the new naming scheme adopted for the AXE series.
> > >
> > > In a previous iteration of the PowerVR binding submission [1], the
> > > number of clocks required for the 6XT family was mentioned to be
> > > always 3. This is also reflected here.
> > >
> > > [1] https://lore.kernel.org/dri-devel/6eeccb26e09aad67fb30ffcd523c793a43c79c2a.camel@imgtec.com/
> > >
> > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > > ---
> > >  .../bindings/gpu/img,powervr-rogue.yaml       | 24 +++++++++++++++----
> > >  1 file changed, 20 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > > index 256e252f8087..48aa205b66b4 100644
> > > --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > > +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > > @@ -12,10 +12,17 @@ maintainers:
> > >
> > >  properties:
> > >    compatible:
> > > -    items:
> > > -      - enum:
> > > -          - ti,am62-gpu
> > > -      - const: img,img-axe # IMG AXE GPU model/revision is fully discoverable
> > > +    oneOf:
> > > +      - items:
> > > +          - enum:
> > > +              - mediatek,mt8173-gpu
> > > +          # PowerVR 6XT GPU model/revision is fully discoverable
> > > +          - const: img,powervr-6xt
> > > +      - items:
> > > +          - enum:
> > > +              - ti,am62-gpu
> > > +          # IMG AXE GPU model/revision is fully discoverable
> > > +          - const: img,img-axe
> >
> > The Series6XT GPU models have differing numbers of power domains (either 2, 4 or
> > 5). Whereas, the AXE GPUs have a single power domain, so I assume there should
> > be a related change here.
> >
> > The GX6250 has two power domains (lets call them A and B). There's a constraint
> > that if domain B is powered then domain A must also be powered.
> >
> > In patch 6 [2] it's setting the power domain to MT8173_POWER_DOMAIN_MFG, which I
> > believe corresponds to power domain B. I assume this works because the MTK power
> > controller driver is encoding the constraint above, meaning that when we disable
> > or enable MT8173_POWER_DOMAIN_MFG it's also disabling/enabling MT8173_POWER_DOMA
> > IN_MFG_2D (domain A).
> >
>
> In the cover letter of this series, it was noted that the GPU
> enumerates, but it doesn' fully function yet.  This is also the case
> for both of the Renesas variants I have been testing, and I was nicely
> asked to postpone my series until the driver was closer to being
> ready.

Yeah. Frank laid out the current state of GX6250 support and future plans
in his reply to the clk driver patch.

> Even if the driver isn't ready yet, it would be nice to move the
> bindings forward.

Agreed. It would be nice to have an agreed upon set of bindings. We
can then move our downstream stuff comply with it.


Thanks
ChenYu

> adam
>
> > Thanks
> > Frank
> >
> > [1] https://lists.freedesktop.org/archives/dri-devel/2024-February/443548.html
> > [2] https://lists.freedesktop.org/archives/dri-devel/2024-May/455833.html
> >
> > >
> > >    reg:
> > >      maxItems: 1
> > > @@ -56,6 +63,15 @@ allOf:
> > >        properties:
> > >          clocks:
> > >            maxItems: 1
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            const: img,powervr-6xt
> > > +    then:
> > > +      properties:
> > > +        clocks:
> > > +          minItems: 3
> > >
> > >  examples:
> > >    - |
Re: [PATCH 3/6] dt-bindings: gpu: powervr-rogue: Add MediaTek MT8173 GPU
Posted by Conor Dooley 3 months, 3 weeks ago
On Thu, May 30, 2024 at 04:35:02PM +0800, Chen-Yu Tsai wrote:
> The MediaTek MT8173 comes with a PowerVR Rogue GX6250, which is one
> of the Series6XT GPUs, another sub-family of the Rogue family.
> 
> This was part of the very first few versions of the PowerVR submission,
> but was later dropped. The compatible string has been updated to follow
> the new naming scheme adopted for the AXE series.
> 
> In a previous iteration of the PowerVR binding submission [1], the
> number of clocks required for the 6XT family was mentioned to be
> always 3. This is also reflected here.
> 
> [1] https://lore.kernel.org/dri-devel/6eeccb26e09aad67fb30ffcd523c793a43c79c2a.camel@imgtec.com/
> 
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.
Re: [PATCH 3/6] dt-bindings: gpu: powervr-rogue: Add MediaTek MT8173 GPU
Posted by AngeloGioacchino Del Regno 3 months, 3 weeks ago
Il 30/05/24 10:35, Chen-Yu Tsai ha scritto:
> The MediaTek MT8173 comes with a PowerVR Rogue GX6250, which is one
> of the Series6XT GPUs, another sub-family of the Rogue family.
> 
> This was part of the very first few versions of the PowerVR submission,
> but was later dropped. The compatible string has been updated to follow
> the new naming scheme adopted for the AXE series.
> 
> In a previous iteration of the PowerVR binding submission [1], the
> number of clocks required for the 6XT family was mentioned to be
> always 3. This is also reflected here.
> 
> [1] https://lore.kernel.org/dri-devel/6eeccb26e09aad67fb30ffcd523c793a43c79c2a.camel@imgtec.com/
> 
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>