Update the img,powervr-rogue.yaml to include the T-HEAD TH1520 SoC's
specific GPU compatible string.
The thead,th1520-gpu compatible, along with its full chain
img,img-bxm-4-64, and img,img-rogue, is added to the
list of recognized GPU types.
The power-domains property requirement for img,img-bxm-4-64 is also
ensured by adding it to the relevant allOf condition.
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
index 4450e2e73b3ccf74d29f0e31e2e6687d7cbe5d65..9b241a0c1f5941dc58a1e23970f6d3773d427c22 100644
--- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
+++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
@@ -21,6 +21,11 @@ properties:
# work with newer dts.
- const: img,img-axe
- const: img,img-rogue
+ - items:
+ - enum:
+ - thead,th1520-gpu
+ - const: img,img-bxm-4-64
+ - const: img,img-rogue
- items:
- enum:
- ti,j721s2-gpu
@@ -93,7 +98,9 @@ allOf:
properties:
compatible:
contains:
- const: img,img-axe-1-16m
+ enum:
+ - img,img-axe-1-16m
+ - img,img-bxm-4-64
then:
properties:
power-domains:
--
2.34.1On 23/06/2025 12:42, Michal Wilczynski wrote: > Update the img,powervr-rogue.yaml to include the T-HEAD TH1520 SoC's > specific GPU compatible string. > > The thead,th1520-gpu compatible, along with its full chain > img,img-bxm-4-64, and img,img-rogue, is added to the > list of recognized GPU types. > > The power-domains property requirement for img,img-bxm-4-64 is also > ensured by adding it to the relevant allOf condition. > > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> > Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> > --- > Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml > index 4450e2e73b3ccf74d29f0e31e2e6687d7cbe5d65..9b241a0c1f5941dc58a1e23970f6d3773d427c22 100644 > --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml > +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml > @@ -21,6 +21,11 @@ properties: > # work with newer dts. > - const: img,img-axe > - const: img,img-rogue > + - items: > + - enum: > + - thead,th1520-gpu > + - const: img,img-bxm-4-64 > + - const: img,img-rogue > - items: > - enum: > - ti,j721s2-gpu > @@ -93,7 +98,9 @@ allOf: > properties: > compatible: > contains: > - const: img,img-axe-1-16m > + enum: > + - img,img-axe-1-16m > + - img,img-bxm-4-64 This isn't right – BXM-4-64 has two power domains like BXS-4-64. I don't really know what the right way to handle that in devicetree is given the TH1520 appears to expose only a top-level domain for the entire GPU, but there are definitely two separate domains underneath that as far as the GPU is concerned (see the attached snippet from integration guide). Since power nodes are ref-counted anyway, do we just use the same node for both domains and let the driver up/down-count it twice? Cheers, Matt > then: > properties: > power-domains: > -- Matt Coster E: matt.coster@imgtec.com
On 6/24/25 15:53, Matt Coster wrote: > On 23/06/2025 12:42, Michal Wilczynski wrote: >> Update the img,powervr-rogue.yaml to include the T-HEAD TH1520 SoC's >> specific GPU compatible string. >> >> The thead,th1520-gpu compatible, along with its full chain >> img,img-bxm-4-64, and img,img-rogue, is added to the >> list of recognized GPU types. >> >> The power-domains property requirement for img,img-bxm-4-64 is also >> ensured by adding it to the relevant allOf condition. >> >> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> >> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> >> --- >> Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >> index 4450e2e73b3ccf74d29f0e31e2e6687d7cbe5d65..9b241a0c1f5941dc58a1e23970f6d3773d427c22 100644 >> --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >> @@ -21,6 +21,11 @@ properties: >> # work with newer dts. >> - const: img,img-axe >> - const: img,img-rogue >> + - items: >> + - enum: >> + - thead,th1520-gpu >> + - const: img,img-bxm-4-64 >> + - const: img,img-rogue >> - items: >> - enum: >> - ti,j721s2-gpu >> @@ -93,7 +98,9 @@ allOf: >> properties: >> compatible: >> contains: >> - const: img,img-axe-1-16m >> + enum: >> + - img,img-axe-1-16m >> + - img,img-bxm-4-64 > > This isn't right – BXM-4-64 has two power domains like BXS-4-64. I don't > really know what the right way to handle that in devicetree is given the > TH1520 appears to expose only a top-level domain for the entire GPU, but > there are definitely two separate domains underneath that as far as the > GPU is concerned (see the attached snippet from integration guide). > > Since power nodes are ref-counted anyway, do we just use the same node > for both domains and let the driver up/down-count it twice? Hi Matt, Thanks for the very helpful insight. That's a great point, it seems the SoC's design presents a tricky case for the bindings. I see what you mean about potentially using the same power domain node twice. My only hesitation is that it might be a bit unclear for someone reading the devicetree later. Perhaps another option could be to relax the constraint for this compatible? Krzysztof, we'd be grateful for your thoughts on how to best model this situation. > > Cheers, > Matt > >> then: >> properties: >> power-domains: >> > > Best regards, -- Michal Wilczynski <m.wilczynski@samsung.com>
On 25/06/2025 14:45, Michal Wilczynski wrote: > > > On 6/24/25 15:53, Matt Coster wrote: >> On 23/06/2025 12:42, Michal Wilczynski wrote: >>> Update the img,powervr-rogue.yaml to include the T-HEAD TH1520 SoC's >>> specific GPU compatible string. >>> >>> The thead,th1520-gpu compatible, along with its full chain >>> img,img-bxm-4-64, and img,img-rogue, is added to the >>> list of recognized GPU types. >>> >>> The power-domains property requirement for img,img-bxm-4-64 is also >>> ensured by adding it to the relevant allOf condition. >>> >>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> >>> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> >>> --- >>> Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml | 9 ++++++++- >>> 1 file changed, 8 insertions(+), 1 deletion(-) >>> >>> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >>> index 4450e2e73b3ccf74d29f0e31e2e6687d7cbe5d65..9b241a0c1f5941dc58a1e23970f6d3773d427c22 100644 >>> --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >>> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >>> @@ -21,6 +21,11 @@ properties: >>> # work with newer dts. >>> - const: img,img-axe >>> - const: img,img-rogue >>> + - items: >>> + - enum: >>> + - thead,th1520-gpu >>> + - const: img,img-bxm-4-64 >>> + - const: img,img-rogue >>> - items: >>> - enum: >>> - ti,j721s2-gpu >>> @@ -93,7 +98,9 @@ allOf: >>> properties: >>> compatible: >>> contains: >>> - const: img,img-axe-1-16m >>> + enum: >>> + - img,img-axe-1-16m >>> + - img,img-bxm-4-64 >> >> This isn't right – BXM-4-64 has two power domains like BXS-4-64. I don't >> really know what the right way to handle that in devicetree is given the >> TH1520 appears to expose only a top-level domain for the entire GPU, but >> there are definitely two separate domains underneath that as far as the >> GPU is concerned (see the attached snippet from integration guide). >> >> Since power nodes are ref-counted anyway, do we just use the same node >> for both domains and let the driver up/down-count it twice? > > Hi Matt, > > Thanks for the very helpful insight. That's a great point, it seems the > SoC's design presents a tricky case for the bindings. > > I see what you mean about potentially using the same power domain node > twice. My only hesitation is that it might be a bit unclear for someone > reading the devicetree later. Perhaps another option could be to relax > the constraint for this compatible? > > Krzysztof, we'd be grateful for your thoughts on how to best model this > situation. It's your hardware, you should tell us, not me. I don't know how many power domains you have there, but for sure it is not one AND two domains the same time. It is either one or two, because power domains are not the same as regulator supplies. Best regards, Krzysztof
On 6/25/25 15:55, Krzysztof Kozlowski wrote: > On 25/06/2025 14:45, Michal Wilczynski wrote: >> >> >> On 6/24/25 15:53, Matt Coster wrote: >>> On 23/06/2025 12:42, Michal Wilczynski wrote: >>>> Update the img,powervr-rogue.yaml to include the T-HEAD TH1520 SoC's >>>> specific GPU compatible string. >>>> >>>> The thead,th1520-gpu compatible, along with its full chain >>>> img,img-bxm-4-64, and img,img-rogue, is added to the >>>> list of recognized GPU types. >>>> >>>> The power-domains property requirement for img,img-bxm-4-64 is also >>>> ensured by adding it to the relevant allOf condition. >>>> >>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>>> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> >>>> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> >>>> --- >>>> Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml | 9 ++++++++- >>>> 1 file changed, 8 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >>>> index 4450e2e73b3ccf74d29f0e31e2e6687d7cbe5d65..9b241a0c1f5941dc58a1e23970f6d3773d427c22 100644 >>>> --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >>>> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >>>> @@ -21,6 +21,11 @@ properties: >>>> # work with newer dts. >>>> - const: img,img-axe >>>> - const: img,img-rogue >>>> + - items: >>>> + - enum: >>>> + - thead,th1520-gpu >>>> + - const: img,img-bxm-4-64 >>>> + - const: img,img-rogue >>>> - items: >>>> - enum: >>>> - ti,j721s2-gpu >>>> @@ -93,7 +98,9 @@ allOf: >>>> properties: >>>> compatible: >>>> contains: >>>> - const: img,img-axe-1-16m >>>> + enum: >>>> + - img,img-axe-1-16m >>>> + - img,img-bxm-4-64 >>> >>> This isn't right – BXM-4-64 has two power domains like BXS-4-64. I don't >>> really know what the right way to handle that in devicetree is given the >>> TH1520 appears to expose only a top-level domain for the entire GPU, but >>> there are definitely two separate domains underneath that as far as the >>> GPU is concerned (see the attached snippet from integration guide). >>> >>> Since power nodes are ref-counted anyway, do we just use the same node >>> for both domains and let the driver up/down-count it twice? >> >> Hi Matt, >> >> Thanks for the very helpful insight. That's a great point, it seems the >> SoC's design presents a tricky case for the bindings. >> >> I see what you mean about potentially using the same power domain node >> twice. My only hesitation is that it might be a bit unclear for someone >> reading the devicetree later. Perhaps another option could be to relax >> the constraint for this compatible? >> >> Krzysztof, we'd be grateful for your thoughts on how to best model this >> situation. > > > It's your hardware, you should tell us, not me. I don't know how many > power domains you have there, but for sure it is not one AND two domains > the same time. It is either one or two, because power domains are not > the same as regulator supplies. Hi Krzysztof, Matt, The img,bxm-4-64 GPU IP itself is designed with two separate power domains. The TH1520 SoC, which integrates this GPU, wires both of these to a single OS controllable power gate (controlled via mailbox and E902 co-processor). This means a devicetree for the TH1520 can only ever provide one power domain for the GPU. However, a generic binding for img,bxm-4-64 should account for a future SoC that might implement both power domains. That's why I proposed to relax the constraints on the img,bmx-4-64 GPU. This makes the binding accurately represent the GPU's full capabilities while remaining compatible with SoCs like the TH1520 that have a limited implementation. Does this seem like the correct and robust approach to you? > > Best regards, > Krzysztof > Best regards, -- Michal Wilczynski <m.wilczynski@samsung.com>
On 25/06/2025 16:18, Michal Wilczynski wrote: > > > On 6/25/25 15:55, Krzysztof Kozlowski wrote: >> On 25/06/2025 14:45, Michal Wilczynski wrote: >>> >>> >>> On 6/24/25 15:53, Matt Coster wrote: >>>> On 23/06/2025 12:42, Michal Wilczynski wrote: >>>>> Update the img,powervr-rogue.yaml to include the T-HEAD TH1520 SoC's >>>>> specific GPU compatible string. >>>>> >>>>> The thead,th1520-gpu compatible, along with its full chain >>>>> img,img-bxm-4-64, and img,img-rogue, is added to the >>>>> list of recognized GPU types. >>>>> >>>>> The power-domains property requirement for img,img-bxm-4-64 is also >>>>> ensured by adding it to the relevant allOf condition. >>>>> >>>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>>>> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> >>>>> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> >>>>> --- >>>>> Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml | 9 ++++++++- >>>>> 1 file changed, 8 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >>>>> index 4450e2e73b3ccf74d29f0e31e2e6687d7cbe5d65..9b241a0c1f5941dc58a1e23970f6d3773d427c22 100644 >>>>> --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >>>>> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >>>>> @@ -21,6 +21,11 @@ properties: >>>>> # work with newer dts. >>>>> - const: img,img-axe >>>>> - const: img,img-rogue >>>>> + - items: >>>>> + - enum: >>>>> + - thead,th1520-gpu >>>>> + - const: img,img-bxm-4-64 >>>>> + - const: img,img-rogue >>>>> - items: >>>>> - enum: >>>>> - ti,j721s2-gpu >>>>> @@ -93,7 +98,9 @@ allOf: >>>>> properties: >>>>> compatible: >>>>> contains: >>>>> - const: img,img-axe-1-16m >>>>> + enum: >>>>> + - img,img-axe-1-16m >>>>> + - img,img-bxm-4-64 >>>> >>>> This isn't right – BXM-4-64 has two power domains like BXS-4-64. I don't >>>> really know what the right way to handle that in devicetree is given the >>>> TH1520 appears to expose only a top-level domain for the entire GPU, but >>>> there are definitely two separate domains underneath that as far as the >>>> GPU is concerned (see the attached snippet from integration guide). >>>> >>>> Since power nodes are ref-counted anyway, do we just use the same node >>>> for both domains and let the driver up/down-count it twice? >>> >>> Hi Matt, >>> >>> Thanks for the very helpful insight. That's a great point, it seems the >>> SoC's design presents a tricky case for the bindings. >>> >>> I see what you mean about potentially using the same power domain node >>> twice. My only hesitation is that it might be a bit unclear for someone >>> reading the devicetree later. Perhaps another option could be to relax >>> the constraint for this compatible? >>> >>> Krzysztof, we'd be grateful for your thoughts on how to best model this >>> situation. >> >> >> It's your hardware, you should tell us, not me. I don't know how many >> power domains you have there, but for sure it is not one AND two domains >> the same time. It is either one or two, because power domains are not >> the same as regulator supplies. > > Hi Krzysztof, Matt, > > The img,bxm-4-64 GPU IP itself is designed with two separate power > domains. The TH1520 SoC, which integrates this GPU, wires both of these > to a single OS controllable power gate (controlled via mailbox and E902 > co-processor). This helps... and also sounds a lot like regulator supplies, not power domains. :/ > > This means a devicetree for the TH1520 can only ever provide one power > domain for the GPU. However, a generic binding for img,bxm-4-64 should If this was a supply, you would have two supplies. Anyway internal wirings of GPU do not matter in such case and more important what the SoC has wired. And it has one power domain. > account for a future SoC that might implement both power domains. > > That's why I proposed to relax the constraints on the img,bmx-4-64 GPU. This should be constrained per each device, so 1 for you and 2 for everyone else. Best regards, Krzysztof
On 25/06/2025 15:41, Krzysztof Kozlowski wrote: > On 25/06/2025 16:18, Michal Wilczynski wrote: >> >> >> On 6/25/25 15:55, Krzysztof Kozlowski wrote: >>> On 25/06/2025 14:45, Michal Wilczynski wrote: >>>> >>>> >>>> On 6/24/25 15:53, Matt Coster wrote: >>>>> On 23/06/2025 12:42, Michal Wilczynski wrote: >>>>>> Update the img,powervr-rogue.yaml to include the T-HEAD TH1520 SoC's >>>>>> specific GPU compatible string. >>>>>> >>>>>> The thead,th1520-gpu compatible, along with its full chain >>>>>> img,img-bxm-4-64, and img,img-rogue, is added to the >>>>>> list of recognized GPU types. >>>>>> >>>>>> The power-domains property requirement for img,img-bxm-4-64 is also >>>>>> ensured by adding it to the relevant allOf condition. >>>>>> >>>>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>>>>> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> >>>>>> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>>>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> >>>>>> --- >>>>>> Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml | 9 ++++++++- >>>>>> 1 file changed, 8 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >>>>>> index 4450e2e73b3ccf74d29f0e31e2e6687d7cbe5d65..9b241a0c1f5941dc58a1e23970f6d3773d427c22 100644 >>>>>> --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >>>>>> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >>>>>> @@ -21,6 +21,11 @@ properties: >>>>>> # work with newer dts. >>>>>> - const: img,img-axe >>>>>> - const: img,img-rogue >>>>>> + - items: >>>>>> + - enum: >>>>>> + - thead,th1520-gpu >>>>>> + - const: img,img-bxm-4-64 >>>>>> + - const: img,img-rogue >>>>>> - items: >>>>>> - enum: >>>>>> - ti,j721s2-gpu >>>>>> @@ -93,7 +98,9 @@ allOf: >>>>>> properties: >>>>>> compatible: >>>>>> contains: >>>>>> - const: img,img-axe-1-16m >>>>>> + enum: >>>>>> + - img,img-axe-1-16m >>>>>> + - img,img-bxm-4-64 >>>>> >>>>> This isn't right – BXM-4-64 has two power domains like BXS-4-64. I don't >>>>> really know what the right way to handle that in devicetree is given the >>>>> TH1520 appears to expose only a top-level domain for the entire GPU, but >>>>> there are definitely two separate domains underneath that as far as the >>>>> GPU is concerned (see the attached snippet from integration guide). >>>>> >>>>> Since power nodes are ref-counted anyway, do we just use the same node >>>>> for both domains and let the driver up/down-count it twice? >>>> >>>> Hi Matt, >>>> >>>> Thanks for the very helpful insight. That's a great point, it seems the >>>> SoC's design presents a tricky case for the bindings. >>>> >>>> I see what you mean about potentially using the same power domain node >>>> twice. My only hesitation is that it might be a bit unclear for someone >>>> reading the devicetree later. Perhaps another option could be to relax >>>> the constraint for this compatible? >>>> >>>> Krzysztof, we'd be grateful for your thoughts on how to best model this >>>> situation. >>> >>> >>> It's your hardware, you should tell us, not me. I don't know how many >>> power domains you have there, but for sure it is not one AND two domains >>> the same time. It is either one or two, because power domains are not >>> the same as regulator supplies. >> >> Hi Krzysztof, Matt, >> >> The img,bxm-4-64 GPU IP itself is designed with two separate power >> domains. The TH1520 SoC, which integrates this GPU, wires both of these >> to a single OS controllable power gate (controlled via mailbox and E902 >> co-processor). > > This helps... and also sounds a lot like regulator supplies, not power > domains. :/ Apologies for taking so long to get back to you with this, I wanted to make sure I had the whole picture from our side before commenting again. From the GPU side, a "typical" integration of BXM-4-64 would use two power domains. Typically, these domains exist in silicon, regardless of whether they are exposed to the host OS, because the SoC's power controller must have control over them. As part of normal operation, the GPU firmware (always in domain "a" on Rogue) will request the power-up/down of the other domains, including during the initial boot sequence. This all happens transparently to the OS. The GPU block itself has no power gating at that level, it relies entirely on the SoC integration. However, it turns out (unknown to me until very recently) that this functionality is optional. The integrator can opt to forego the power-saving functionality afforded by firmware-controlled power gating and just throw everything into a single domain, which appears to be what's happened here. My only remaining issue here, then, is the naming. Since this integration doesn't use discrete domains, saying it has one domain called "a" isn't correct*. We should either: - Drop the name altogether for this integration (and others like it that don't use the low-power functionality, if there are any), or - Come up with a new domain name to signal this explicitly (perhaps simply "gpu")? Something that's unlikely to clash with the "real" names that are going to start appearing in the Volcanic bindings (where we finally ditched "a", "b", etc.). Cheers, Matt *Yes, I know that's what we said for the AXE-1-16M, but that tiny GPU is the exception to the rule; AFAIK it's the only one we've ever produced that truly has only one power domain. > >> >> This means a devicetree for the TH1520 can only ever provide one power >> domain for the GPU. However, a generic binding for img,bxm-4-64 should > > If this was a supply, you would have two supplies. Anyway internal > wirings of GPU do not matter in such case and more important what the > SoC has wired. And it has one power domain. > > >> account for a future SoC that might implement both power domains. >> >> That's why I proposed to relax the constraints on the img,bmx-4-64 GPU. > > This should be constrained per each device, so 1 for you and 2 for > everyone else. > > Best regards, > Krzysztof -- Matt Coster E: matt.coster@imgtec.com
On 7/23/25 11:45, Matt Coster wrote: > On 25/06/2025 15:41, Krzysztof Kozlowski wrote: >> On 25/06/2025 16:18, Michal Wilczynski wrote: >>> >>> >>> On 6/25/25 15:55, Krzysztof Kozlowski wrote: >>>> On 25/06/2025 14:45, Michal Wilczynski wrote: >>>>> >>>>> >>>>> On 6/24/25 15:53, Matt Coster wrote: >>>>>> On 23/06/2025 12:42, Michal Wilczynski wrote: >>>>>>> Update the img,powervr-rogue.yaml to include the T-HEAD TH1520 SoC's >>>>>>> specific GPU compatible string. >>>>>>> >>>>>>> The thead,th1520-gpu compatible, along with its full chain >>>>>>> img,img-bxm-4-64, and img,img-rogue, is added to the >>>>>>> list of recognized GPU types. >>>>>>> >>>>>>> The power-domains property requirement for img,img-bxm-4-64 is also >>>>>>> ensured by adding it to the relevant allOf condition. >>>>>>> >>>>>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>>>>>> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> >>>>>>> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>>>>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> >>>>>>> --- >>>>>>> Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml | 9 ++++++++- >>>>>>> 1 file changed, 8 insertions(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >>>>>>> index 4450e2e73b3ccf74d29f0e31e2e6687d7cbe5d65..9b241a0c1f5941dc58a1e23970f6d3773d427c22 100644 >>>>>>> --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >>>>>>> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >>>>>>> @@ -21,6 +21,11 @@ properties: >>>>>>> # work with newer dts. >>>>>>> - const: img,img-axe >>>>>>> - const: img,img-rogue >>>>>>> + - items: >>>>>>> + - enum: >>>>>>> + - thead,th1520-gpu >>>>>>> + - const: img,img-bxm-4-64 >>>>>>> + - const: img,img-rogue >>>>>>> - items: >>>>>>> - enum: >>>>>>> - ti,j721s2-gpu >>>>>>> @@ -93,7 +98,9 @@ allOf: >>>>>>> properties: >>>>>>> compatible: >>>>>>> contains: >>>>>>> - const: img,img-axe-1-16m >>>>>>> + enum: >>>>>>> + - img,img-axe-1-16m >>>>>>> + - img,img-bxm-4-64 >>>>>> >>>>>> This isn't right – BXM-4-64 has two power domains like BXS-4-64. I don't >>>>>> really know what the right way to handle that in devicetree is given the >>>>>> TH1520 appears to expose only a top-level domain for the entire GPU, but >>>>>> there are definitely two separate domains underneath that as far as the >>>>>> GPU is concerned (see the attached snippet from integration guide). >>>>>> >>>>>> Since power nodes are ref-counted anyway, do we just use the same node >>>>>> for both domains and let the driver up/down-count it twice? >>>>> >>>>> Hi Matt, >>>>> >>>>> Thanks for the very helpful insight. That's a great point, it seems the >>>>> SoC's design presents a tricky case for the bindings. >>>>> >>>>> I see what you mean about potentially using the same power domain node >>>>> twice. My only hesitation is that it might be a bit unclear for someone >>>>> reading the devicetree later. Perhaps another option could be to relax >>>>> the constraint for this compatible? >>>>> >>>>> Krzysztof, we'd be grateful for your thoughts on how to best model this >>>>> situation. >>>> >>>> >>>> It's your hardware, you should tell us, not me. I don't know how many >>>> power domains you have there, but for sure it is not one AND two domains >>>> the same time. It is either one or two, because power domains are not >>>> the same as regulator supplies. >>> >>> Hi Krzysztof, Matt, >>> >>> The img,bxm-4-64 GPU IP itself is designed with two separate power >>> domains. The TH1520 SoC, which integrates this GPU, wires both of these >>> to a single OS controllable power gate (controlled via mailbox and E902 >>> co-processor). >> >> This helps... and also sounds a lot like regulator supplies, not power >> domains. :/ > > Apologies for taking so long to get back to you with this, I wanted to > make sure I had the whole picture from our side before commenting again. > > From the GPU side, a "typical" integration of BXM-4-64 would use two > power domains. > > Typically, these domains exist in silicon, regardless of whether they > are exposed to the host OS, because the SoC's power controller must have > control over them. As part of normal operation, the GPU firmware (always > in domain "a" on Rogue) will request the power-up/down of the other > domains, including during the initial boot sequence. This all happens > transparently to the OS. The GPU block itself has no power gating at > that level, it relies entirely on the SoC integration. > > However, it turns out (unknown to me until very recently) that this > functionality is optional. The integrator can opt to forego the > power-saving functionality afforded by firmware-controlled power gating > and just throw everything into a single domain, which appears to be > what's happened here. > > My only remaining issue here, then, is the naming. Since this > integration doesn't use discrete domains, saying it has one domain > called "a" isn't correct*. We should either: > > - Drop the name altogether for this integration (and others like it > that don't use the low-power functionality, if there are any), or Hi Matt, Thanks for the detailed explanation, that clears things up perfectly. I agree with your assessment. Dropping the power-domain-names property for this integration seems like the cleanest solution. As you pointed out, since the OS sees only a single, undifferentiated power domain, giving it a name like "gpu" would be redundant. This approach correctly models the hardware without setting a potentially confusing precedent. To follow through on this, I assume we'll need to adjust pvr_power_domains_init() to handle nodes that don't have the power-domain-names property. Does that sound right to you? > - Come up with a new domain name to signal this explicitly (perhaps > simply "gpu")? Something that's unlikely to clash with the "real" > names that are going to start appearing in the Volcanic bindings > (where we finally ditched "a", "b", etc.). > > Cheers, > Matt > > *Yes, I know that's what we said for the AXE-1-16M, but that tiny GPU is > the exception to the rule; AFAIK it's the only one we've ever produced > that truly has only one power domain. > >> >>> >>> This means a devicetree for the TH1520 can only ever provide one power >>> domain for the GPU. However, a generic binding for img,bxm-4-64 should >> >> If this was a supply, you would have two supplies. Anyway internal >> wirings of GPU do not matter in such case and more important what the >> SoC has wired. And it has one power domain. >> >> >>> account for a future SoC that might implement both power domains. >>> >>> That's why I proposed to relax the constraints on the img,bmx-4-64 GPU. >> >> This should be constrained per each device, so 1 for you and 2 for >> everyone else. >> >> Best regards, >> Krzysztof > > Best regards, -- Michal Wilczynski <m.wilczynski@samsung.com>
On 23/07/2025 17:26, Michal Wilczynski wrote: > On 7/23/25 11:45, Matt Coster wrote: >> On 25/06/2025 15:41, Krzysztof Kozlowski wrote: >>> On 25/06/2025 16:18, Michal Wilczynski wrote: >>>> >>>> >>>> On 6/25/25 15:55, Krzysztof Kozlowski wrote: >>>>> On 25/06/2025 14:45, Michal Wilczynski wrote: >>>>>> >>>>>> >>>>>> On 6/24/25 15:53, Matt Coster wrote: >>>>>>> On 23/06/2025 12:42, Michal Wilczynski wrote: >>>>>>>> Update the img,powervr-rogue.yaml to include the T-HEAD TH1520 SoC's >>>>>>>> specific GPU compatible string. >>>>>>>> >>>>>>>> The thead,th1520-gpu compatible, along with its full chain >>>>>>>> img,img-bxm-4-64, and img,img-rogue, is added to the >>>>>>>> list of recognized GPU types. >>>>>>>> >>>>>>>> The power-domains property requirement for img,img-bxm-4-64 is also >>>>>>>> ensured by adding it to the relevant allOf condition. >>>>>>>> >>>>>>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>>>>>>> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> >>>>>>>> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>>>>>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> >>>>>>>> --- >>>>>>>> Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml | 9 ++++++++- >>>>>>>> 1 file changed, 8 insertions(+), 1 deletion(-) >>>>>>>> >>>>>>>> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >>>>>>>> index 4450e2e73b3ccf74d29f0e31e2e6687d7cbe5d65..9b241a0c1f5941dc58a1e23970f6d3773d427c22 100644 >>>>>>>> --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >>>>>>>> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >>>>>>>> @@ -21,6 +21,11 @@ properties: >>>>>>>> # work with newer dts. >>>>>>>> - const: img,img-axe >>>>>>>> - const: img,img-rogue >>>>>>>> + - items: >>>>>>>> + - enum: >>>>>>>> + - thead,th1520-gpu >>>>>>>> + - const: img,img-bxm-4-64 >>>>>>>> + - const: img,img-rogue >>>>>>>> - items: >>>>>>>> - enum: >>>>>>>> - ti,j721s2-gpu >>>>>>>> @@ -93,7 +98,9 @@ allOf: >>>>>>>> properties: >>>>>>>> compatible: >>>>>>>> contains: >>>>>>>> - const: img,img-axe-1-16m >>>>>>>> + enum: >>>>>>>> + - img,img-axe-1-16m >>>>>>>> + - img,img-bxm-4-64 >>>>>>> >>>>>>> This isn't right – BXM-4-64 has two power domains like BXS-4-64. I don't >>>>>>> really know what the right way to handle that in devicetree is given the >>>>>>> TH1520 appears to expose only a top-level domain for the entire GPU, but >>>>>>> there are definitely two separate domains underneath that as far as the >>>>>>> GPU is concerned (see the attached snippet from integration guide). >>>>>>> >>>>>>> Since power nodes are ref-counted anyway, do we just use the same node >>>>>>> for both domains and let the driver up/down-count it twice? >>>>>> >>>>>> Hi Matt, >>>>>> >>>>>> Thanks for the very helpful insight. That's a great point, it seems the >>>>>> SoC's design presents a tricky case for the bindings. >>>>>> >>>>>> I see what you mean about potentially using the same power domain node >>>>>> twice. My only hesitation is that it might be a bit unclear for someone >>>>>> reading the devicetree later. Perhaps another option could be to relax >>>>>> the constraint for this compatible? >>>>>> >>>>>> Krzysztof, we'd be grateful for your thoughts on how to best model this >>>>>> situation. >>>>> >>>>> >>>>> It's your hardware, you should tell us, not me. I don't know how many >>>>> power domains you have there, but for sure it is not one AND two domains >>>>> the same time. It is either one or two, because power domains are not >>>>> the same as regulator supplies. >>>> >>>> Hi Krzysztof, Matt, >>>> >>>> The img,bxm-4-64 GPU IP itself is designed with two separate power >>>> domains. The TH1520 SoC, which integrates this GPU, wires both of these >>>> to a single OS controllable power gate (controlled via mailbox and E902 >>>> co-processor). >>> >>> This helps... and also sounds a lot like regulator supplies, not power >>> domains. :/ >> >> Apologies for taking so long to get back to you with this, I wanted to >> make sure I had the whole picture from our side before commenting again. >> >> From the GPU side, a "typical" integration of BXM-4-64 would use two >> power domains. >> >> Typically, these domains exist in silicon, regardless of whether they >> are exposed to the host OS, because the SoC's power controller must have >> control over them. As part of normal operation, the GPU firmware (always >> in domain "a" on Rogue) will request the power-up/down of the other >> domains, including during the initial boot sequence. This all happens >> transparently to the OS. The GPU block itself has no power gating at >> that level, it relies entirely on the SoC integration. >> >> However, it turns out (unknown to me until very recently) that this >> functionality is optional. The integrator can opt to forego the >> power-saving functionality afforded by firmware-controlled power gating >> and just throw everything into a single domain, which appears to be >> what's happened here. >> >> My only remaining issue here, then, is the naming. Since this >> integration doesn't use discrete domains, saying it has one domain >> called "a" isn't correct*. We should either: >> >> - Drop the name altogether for this integration (and others like it >> that don't use the low-power functionality, if there are any), or > > Hi Matt, > > Thanks for the detailed explanation, that clears things up perfectly. I'm glad I could get to the bottom of this one, it was bothering me too! > > I agree with your assessment. Dropping the power-domain-names property > for this integration seems like the cleanest solution. As you pointed > out, since the OS sees only a single, undifferentiated power domain, > giving it a name like "gpu" would be redundant. This approach correctly > models the hardware without setting a potentially confusing precedent. That seems reasonable. I was very much on the fence for this one, so I'll happily go along with dropping the name altogether. Just to make sure I understand correctly, the change here would be to move "required: - power-domain-names" from "img,img-rogue" to every conditional block that constrains the number of domains? Can we add negative constraints in conditionals? Then we could add "power-domain-names: false" to the "thead,th1520-gpu" conditional block alongside "power-domains: maxItems: 1". > > To follow through on this, I assume we'll need to adjust > pvr_power_domains_init() to handle nodes that don't have the > power-domain-names property. Does that sound right to you? You should get away without making any code changes here, since we already shortcut on "domain_count <= 1" to just allow the pm_runtime to deal with the single (or missing) domain directly. If we ever start controlling the individual domains ourselves from the kernel (rather than just ensuring they all come on and off in the correct sequence), we can add checks/handling for the no-name case then. Cheers, Matt > >> - Come up with a new domain name to signal this explicitly (perhaps >> simply "gpu")? Something that's unlikely to clash with the "real" >> names that are going to start appearing in the Volcanic bindings >> (where we finally ditched "a", "b", etc.). >> >> Cheers, >> Matt >> >> *Yes, I know that's what we said for the AXE-1-16M, but that tiny GPU is >> the exception to the rule; AFAIK it's the only one we've ever produced >> that truly has only one power domain. >> >>> >>>> >>>> This means a devicetree for the TH1520 can only ever provide one power >>>> domain for the GPU. However, a generic binding for img,bxm-4-64 should >>> >>> If this was a supply, you would have two supplies. Anyway internal >>> wirings of GPU do not matter in such case and more important what the >>> SoC has wired. And it has one power domain. >>> >>> >>>> account for a future SoC that might implement both power domains. >>>> >>>> That's why I proposed to relax the constraints on the img,bmx-4-64 GPU. >>> >>> This should be constrained per each device, so 1 for you and 2 for >>> everyone else. >>> >>> Best regards, >>> Krzysztof >> >> > > Best regards, -- Matt Coster E: matt.coster@imgtec.com
On 7/23/25 18:50, Matt Coster wrote:
> On 23/07/2025 17:26, Michal Wilczynski wrote:
>> On 7/23/25 11:45, Matt Coster wrote:
>>> On 25/06/2025 15:41, Krzysztof Kozlowski wrote:
>>>> On 25/06/2025 16:18, Michal Wilczynski wrote:
>>>>>
>>>>>
>>>>> On 6/25/25 15:55, Krzysztof Kozlowski wrote:
>>>>>> On 25/06/2025 14:45, Michal Wilczynski wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 6/24/25 15:53, Matt Coster wrote:
>>>>>>>> On 23/06/2025 12:42, Michal Wilczynski wrote:
>>>>>>>>> Update the img,powervr-rogue.yaml to include the T-HEAD TH1520 SoC's
>>>>>>>>> specific GPU compatible string.
>>>>>>>>>
>>>>>>>>> The thead,th1520-gpu compatible, along with its full chain
>>>>>>>>> img,img-bxm-4-64, and img,img-rogue, is added to the
>>>>>>>>> list of recognized GPU types.
>>>>>>>>>
>>>>>>>>> The power-domains property requirement for img,img-bxm-4-64 is also
>>>>>>>>> ensured by adding it to the relevant allOf condition.
>>>>>>>>>
>>>>>>>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>>>>>> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
>>>>>>>>> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>>>>>>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
>>>>>>>>> ---
>>>>>>>>> Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml | 9 ++++++++-
>>>>>>>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>>>>>>>>
>>>>>>>>> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
>>>>>>>>> index 4450e2e73b3ccf74d29f0e31e2e6687d7cbe5d65..9b241a0c1f5941dc58a1e23970f6d3773d427c22 100644
>>>>>>>>> --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
>>>>>>>>> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
>>>>>>>>> @@ -21,6 +21,11 @@ properties:
>>>>>>>>> # work with newer dts.
>>>>>>>>> - const: img,img-axe
>>>>>>>>> - const: img,img-rogue
>>>>>>>>> + - items:
>>>>>>>>> + - enum:
>>>>>>>>> + - thead,th1520-gpu
>>>>>>>>> + - const: img,img-bxm-4-64
>>>>>>>>> + - const: img,img-rogue
>>>>>>>>> - items:
>>>>>>>>> - enum:
>>>>>>>>> - ti,j721s2-gpu
>>>>>>>>> @@ -93,7 +98,9 @@ allOf:
>>>>>>>>> properties:
>>>>>>>>> compatible:
>>>>>>>>> contains:
>>>>>>>>> - const: img,img-axe-1-16m
>>>>>>>>> + enum:
>>>>>>>>> + - img,img-axe-1-16m
>>>>>>>>> + - img,img-bxm-4-64
>>>>>>>>
>>>>>>>> This isn't right – BXM-4-64 has two power domains like BXS-4-64. I don't
>>>>>>>> really know what the right way to handle that in devicetree is given the
>>>>>>>> TH1520 appears to expose only a top-level domain for the entire GPU, but
>>>>>>>> there are definitely two separate domains underneath that as far as the
>>>>>>>> GPU is concerned (see the attached snippet from integration guide).
>>>>>>>>
>>>>>>>> Since power nodes are ref-counted anyway, do we just use the same node
>>>>>>>> for both domains and let the driver up/down-count it twice?
>>>>>>>
>>>>>>> Hi Matt,
>>>>>>>
>>>>>>> Thanks for the very helpful insight. That's a great point, it seems the
>>>>>>> SoC's design presents a tricky case for the bindings.
>>>>>>>
>>>>>>> I see what you mean about potentially using the same power domain node
>>>>>>> twice. My only hesitation is that it might be a bit unclear for someone
>>>>>>> reading the devicetree later. Perhaps another option could be to relax
>>>>>>> the constraint for this compatible?
>>>>>>>
>>>>>>> Krzysztof, we'd be grateful for your thoughts on how to best model this
>>>>>>> situation.
>>>>>>
>>>>>>
>>>>>> It's your hardware, you should tell us, not me. I don't know how many
>>>>>> power domains you have there, but for sure it is not one AND two domains
>>>>>> the same time. It is either one or two, because power domains are not
>>>>>> the same as regulator supplies.
>>>>>
>>>>> Hi Krzysztof, Matt,
>>>>>
>>>>> The img,bxm-4-64 GPU IP itself is designed with two separate power
>>>>> domains. The TH1520 SoC, which integrates this GPU, wires both of these
>>>>> to a single OS controllable power gate (controlled via mailbox and E902
>>>>> co-processor).
>>>>
>>>> This helps... and also sounds a lot like regulator supplies, not power
>>>> domains. :/
>>>
>>> Apologies for taking so long to get back to you with this, I wanted to
>>> make sure I had the whole picture from our side before commenting again.
>>>
>>> From the GPU side, a "typical" integration of BXM-4-64 would use two
>>> power domains.
>>>
>>> Typically, these domains exist in silicon, regardless of whether they
>>> are exposed to the host OS, because the SoC's power controller must have
>>> control over them. As part of normal operation, the GPU firmware (always
>>> in domain "a" on Rogue) will request the power-up/down of the other
>>> domains, including during the initial boot sequence. This all happens
>>> transparently to the OS. The GPU block itself has no power gating at
>>> that level, it relies entirely on the SoC integration.
>>>
>>> However, it turns out (unknown to me until very recently) that this
>>> functionality is optional. The integrator can opt to forego the
>>> power-saving functionality afforded by firmware-controlled power gating
>>> and just throw everything into a single domain, which appears to be
>>> what's happened here.
>>>
>>> My only remaining issue here, then, is the naming. Since this
>>> integration doesn't use discrete domains, saying it has one domain
>>> called "a" isn't correct*. We should either:
>>>
>>> - Drop the name altogether for this integration (and others like it
>>> that don't use the low-power functionality, if there are any), or
>>
>> Hi Matt,
>>
>> Thanks for the detailed explanation, that clears things up perfectly.
>
> I'm glad I could get to the bottom of this one, it was bothering me too!
>
>>
>> I agree with your assessment. Dropping the power-domain-names property
>> for this integration seems like the cleanest solution. As you pointed
>> out, since the OS sees only a single, undifferentiated power domain,
>> giving it a name like "gpu" would be redundant. This approach correctly
>> models the hardware without setting a potentially confusing precedent.
>
> That seems reasonable. I was very much on the fence for this one, so
> I'll happily go along with dropping the name altogether.
>
> Just to make sure I understand correctly, the change here would be to
> move "required: - power-domain-names" from "img,img-rogue" to every
> conditional block that constrains the number of domains?
>
> Can we add negative constraints in conditionals? Then we could add
> "power-domain-names: false" to the "thead,th1520-gpu" conditional block
> alongside "power-domains: maxItems: 1".
Yeah the diff with v7 would look like so:
diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
index 263c40c8438e..338746f39cbb 100644
--- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
+++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
@@ -89,6 +89,11 @@ allOf:
compatible:
contains:
const: img,img-rogue
+ not:
+ properties:
+ compatible:
+ contains:
+ const: thead,th1520-gpu
then:
required:
- power-domains
@@ -100,11 +105,12 @@ allOf:
contains:
const: thead,th1520-gpu
then:
+ required:
+ - power-domains
properties:
power-domains:
maxItems: 1
- power-domain-names:
- maxItems: 1
+ power-domain-names: false
- if:
properties:
This change ensures power-domain-names is required for all img,img-rogue
devices, except for thead,th1520-gpu. For the thead specifically,
power-domains remains required, but power-domain-names is explicitly
forbidden.
>
>>
>> To follow through on this, I assume we'll need to adjust
>> pvr_power_domains_init() to handle nodes that don't have the
>> power-domain-names property. Does that sound right to you?
>
> You should get away without making any code changes here, since we
> already shortcut on "domain_count <= 1" to just allow the pm_runtime to
> deal with the single (or missing) domain directly.
Great !
>
> If we ever start controlling the individual domains ourselves from the
> kernel (rather than just ensuring they all come on and off in the
> correct sequence), we can add checks/handling for the no-name case then.
>
> Cheers,
> Matt
>
>>
>>> - Come up with a new domain name to signal this explicitly (perhaps
>>> simply "gpu")? Something that's unlikely to clash with the "real"
>>> names that are going to start appearing in the Volcanic bindings
>>> (where we finally ditched "a", "b", etc.).
>>>
>>> Cheers,
>>> Matt
>>>
>>> *Yes, I know that's what we said for the AXE-1-16M, but that tiny GPU is
>>> the exception to the rule; AFAIK it's the only one we've ever produced
>>> that truly has only one power domain.
>>>
>>>>
>>>>>
>>>>> This means a devicetree for the TH1520 can only ever provide one power
>>>>> domain for the GPU. However, a generic binding for img,bxm-4-64 should
>>>>
>>>> If this was a supply, you would have two supplies. Anyway internal
>>>> wirings of GPU do not matter in such case and more important what the
>>>> SoC has wired. And it has one power domain.
>>>>
>>>>
>>>>> account for a future SoC that might implement both power domains.
>>>>>
>>>>> That's why I proposed to relax the constraints on the img,bmx-4-64 GPU.
>>>>
>>>> This should be constrained per each device, so 1 for you and 2 for
>>>> everyone else.
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>
>>>
>>
>> Best regards,
>
>
Best regards,
--
Michal Wilczynski <m.wilczynski@samsung.com>
On 23/07/2025 20:25, Michal Wilczynski wrote: > > > On 7/23/25 18:50, Matt Coster wrote: >> On 23/07/2025 17:26, Michal Wilczynski wrote: >>> On 7/23/25 11:45, Matt Coster wrote: >>>> On 25/06/2025 15:41, Krzysztof Kozlowski wrote: >>>>> On 25/06/2025 16:18, Michal Wilczynski wrote: >>>>>> >>>>>> >>>>>> On 6/25/25 15:55, Krzysztof Kozlowski wrote: >>>>>>> On 25/06/2025 14:45, Michal Wilczynski wrote: >>>>>>>> >>>>>>>> >>>>>>>> On 6/24/25 15:53, Matt Coster wrote: >>>>>>>>> On 23/06/2025 12:42, Michal Wilczynski wrote: >>>>>>>>>> Update the img,powervr-rogue.yaml to include the T-HEAD TH1520 SoC's >>>>>>>>>> specific GPU compatible string. >>>>>>>>>> >>>>>>>>>> The thead,th1520-gpu compatible, along with its full chain >>>>>>>>>> img,img-bxm-4-64, and img,img-rogue, is added to the >>>>>>>>>> list of recognized GPU types. >>>>>>>>>> >>>>>>>>>> The power-domains property requirement for img,img-bxm-4-64 is also >>>>>>>>>> ensured by adding it to the relevant allOf condition. >>>>>>>>>> >>>>>>>>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>>>>>>>>> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> >>>>>>>>>> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>>>>>>>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> >>>>>>>>>> --- >>>>>>>>>> Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml | 9 ++++++++- >>>>>>>>>> 1 file changed, 8 insertions(+), 1 deletion(-) >>>>>>>>>> >>>>>>>>>> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >>>>>>>>>> index 4450e2e73b3ccf74d29f0e31e2e6687d7cbe5d65..9b241a0c1f5941dc58a1e23970f6d3773d427c22 100644 >>>>>>>>>> --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >>>>>>>>>> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >>>>>>>>>> @@ -21,6 +21,11 @@ properties: >>>>>>>>>> # work with newer dts. >>>>>>>>>> - const: img,img-axe >>>>>>>>>> - const: img,img-rogue >>>>>>>>>> + - items: >>>>>>>>>> + - enum: >>>>>>>>>> + - thead,th1520-gpu >>>>>>>>>> + - const: img,img-bxm-4-64 >>>>>>>>>> + - const: img,img-rogue >>>>>>>>>> - items: >>>>>>>>>> - enum: >>>>>>>>>> - ti,j721s2-gpu >>>>>>>>>> @@ -93,7 +98,9 @@ allOf: >>>>>>>>>> properties: >>>>>>>>>> compatible: >>>>>>>>>> contains: >>>>>>>>>> - const: img,img-axe-1-16m >>>>>>>>>> + enum: >>>>>>>>>> + - img,img-axe-1-16m >>>>>>>>>> + - img,img-bxm-4-64 >>>>>>>>> >>>>>>>>> This isn't right – BXM-4-64 has two power domains like BXS-4-64. I don't >>>>>>>>> really know what the right way to handle that in devicetree is given the >>>>>>>>> TH1520 appears to expose only a top-level domain for the entire GPU, but >>>>>>>>> there are definitely two separate domains underneath that as far as the >>>>>>>>> GPU is concerned (see the attached snippet from integration guide). >>>>>>>>> >>>>>>>>> Since power nodes are ref-counted anyway, do we just use the same node >>>>>>>>> for both domains and let the driver up/down-count it twice? >>>>>>>> >>>>>>>> Hi Matt, >>>>>>>> >>>>>>>> Thanks for the very helpful insight. That's a great point, it seems the >>>>>>>> SoC's design presents a tricky case for the bindings. >>>>>>>> >>>>>>>> I see what you mean about potentially using the same power domain node >>>>>>>> twice. My only hesitation is that it might be a bit unclear for someone >>>>>>>> reading the devicetree later. Perhaps another option could be to relax >>>>>>>> the constraint for this compatible? >>>>>>>> >>>>>>>> Krzysztof, we'd be grateful for your thoughts on how to best model this >>>>>>>> situation. >>>>>>> >>>>>>> >>>>>>> It's your hardware, you should tell us, not me. I don't know how many >>>>>>> power domains you have there, but for sure it is not one AND two domains >>>>>>> the same time. It is either one or two, because power domains are not >>>>>>> the same as regulator supplies. >>>>>> >>>>>> Hi Krzysztof, Matt, >>>>>> >>>>>> The img,bxm-4-64 GPU IP itself is designed with two separate power >>>>>> domains. The TH1520 SoC, which integrates this GPU, wires both of these >>>>>> to a single OS controllable power gate (controlled via mailbox and E902 >>>>>> co-processor). >>>>> >>>>> This helps... and also sounds a lot like regulator supplies, not power >>>>> domains. :/ >>>> >>>> Apologies for taking so long to get back to you with this, I wanted to >>>> make sure I had the whole picture from our side before commenting again. >>>> >>>> From the GPU side, a "typical" integration of BXM-4-64 would use two >>>> power domains. >>>> >>>> Typically, these domains exist in silicon, regardless of whether they >>>> are exposed to the host OS, because the SoC's power controller must have >>>> control over them. As part of normal operation, the GPU firmware (always >>>> in domain "a" on Rogue) will request the power-up/down of the other >>>> domains, including during the initial boot sequence. This all happens >>>> transparently to the OS. The GPU block itself has no power gating at >>>> that level, it relies entirely on the SoC integration. >>>> >>>> However, it turns out (unknown to me until very recently) that this >>>> functionality is optional. The integrator can opt to forego the >>>> power-saving functionality afforded by firmware-controlled power gating >>>> and just throw everything into a single domain, which appears to be >>>> what's happened here. >>>> >>>> My only remaining issue here, then, is the naming. Since this >>>> integration doesn't use discrete domains, saying it has one domain >>>> called "a" isn't correct*. We should either: >>>> >>>> - Drop the name altogether for this integration (and others like it >>>> that don't use the low-power functionality, if there are any), or >>> >>> Hi Matt, >>> >>> Thanks for the detailed explanation, that clears things up perfectly. >> >> I'm glad I could get to the bottom of this one, it was bothering me too! >> >>> >>> I agree with your assessment. Dropping the power-domain-names property >>> for this integration seems like the cleanest solution. As you pointed >>> out, since the OS sees only a single, undifferentiated power domain, >>> giving it a name like "gpu" would be redundant. This approach correctly >>> models the hardware without setting a potentially confusing precedent. >> >> That seems reasonable. I was very much on the fence for this one, so >> I'll happily go along with dropping the name altogether. >> >> Just to make sure I understand correctly, the change here would be to >> move "required: - power-domain-names" from "img,img-rogue" to every >> conditional block that constrains the number of domains? >> >> Can we add negative constraints in conditionals? Then we could add >> "power-domain-names: false" to the "thead,th1520-gpu" conditional block >> alongside "power-domains: maxItems: 1". > > Yeah the diff with v7 would look like so: > diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml > index 263c40c8438e..338746f39cbb 100644 > --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml > +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml > @@ -89,6 +89,11 @@ allOf: > compatible: > contains: > const: img,img-rogue > + not: > + properties: > + compatible: > + contains: > + const: thead,th1520-gpu No, don't do that. Anyway, if you are going to rewrite patch, you MUST drop all tags, document it and explicitly ask for re-review. Best regards, Krzysztof
© 2016 - 2025 Red Hat, Inc.