Message-ID: <2e60530d-e214-4044-8dfc-5293832ac4fd@imgtec.com>
Date: Fri, 25 Jul 2025 14:16:58 +0000
From: Matt Coster <Matt.Coster@...tec.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Michal Wilczynski
	<m.wilczynski@...sung.com>
CC: Guo Ren <guoren@...nel.org>, Fu Wei <wefu@...hat.com>,
        Rob Herring
	<robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley
	<conor+dt@...nel.org>,
        Bartosz Golaszewski <brgl@...ev.pl>,
        Philipp Zabel
	<p.zabel@...gutronix.de>,
        Frank Binns <Frank.Binns@...tec.com>,
        Maarten
 Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard
	<mripard@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        David Airlie
	<airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
        Paul Walmsley
	<paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou
	<aou@...s.berkeley.edu>, Alexandre Ghiti <alex@...ti.fr>,
        Ulf Hansson
	<ulf.hansson@...aro.org>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Drew
 Fustini <fustini@...nel.org>,
        Bartosz Golaszewski
	<bartosz.golaszewski@...aro.org>,
        "linux-riscv@...ts.infradead.org"
	<linux-riscv@...ts.infradead.org>,
        "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        "linux-pm@...r.kernel.org"
	<linux-pm@...r.kernel.org>,
        "dri-devel@...ts.freedesktop.org"
	<dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH v8 2/4] dt-bindings: gpu: img,powervr-rogue: Add TH1520
 GPU compatible

On 25/07/2025 12:08, Krzysztof Kozlowski wrote:
> On 25/07/2025 11:00, Matt Coster wrote:
>> On 25/07/2025 07:59, Krzysztof Kozlowski wrote:
>>> On Thu, Jul 24, 2025 at 04:18:59PM +0200, 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.
>>>>
>>>> While the BXM-4-64 GPU IP is designed with two distinct power domains,
>>>> the TH1520 SoC integrates it with only a single, unified power gate that
>>>> is controllable by the kernel.
>>>>
>>>> To model this reality correctly while keeping the binding accurate for
>>>> other devices, add conditional constraints to the `allOf` section:
>>>>  - An if block for thead,th1520-gpu enforces a maximum of one
>>>>    power domain and disallows the power-domain-names property.
>>>
>>> Why?
>>>
>>> This solves nothing, because you did not change the meaning of power
>>> domain entry.
>>
>> Hi Krzysztof,
>>
>> Just to clarify, is this an issue that can be resolved by documenting
>> the semantics of ">=1 power domains with names" vs "1 unnamed power
>> domain" in the binding file? Or are you suggesting an alternative method
>> of encoding this information in devicetree?
> 
> Currently, through power-domain names, the first entry in power domains
> is the 'a' domain. We usually prefer this to be explicit - listing items
> - but here, probably due to obviousness of names A and B, it did not happen.

I'm probably being slow (it is a Friday afternoon after all), but I'm
not sure I follow. For context, see my response to an earlier version of
this series[1], but I'll include relevant details here.

The "typical" integration of a Rogue GPU involves minimum two power
domains. Annoyingly, we literally call these A, B, etc. Domain A always
contains the firmware controller, which can control the gating of the
other domains in hardware without getting the host OS involved. The
purpose of declaring all these domains in devicetree is that they must
be powered on in order (it so happens in Rogue that A->B->... is always
valid so we didn't bother adding more complexity here to define the true
dependencies between the domains where more than 2 are present).

However, there are two exceptions to this:

 - The AXE-1-16M is a tiny GPU that only uses one power domain. It was
   also the first GPU we added support for, and as such we didn't
   consider the ramifications of not requiring power-domain-names in the
   bindings. That's the historical context for power-domain-names
   currently only being required for img,img-rogue compatibles, we
   introduced that top-level compatible as part of an overhaul to ensure
   we'd be able to describe all Rogue hardware more accurately. Where
   devicetrees containing AXE-1-16M have been updated to use the new
   compatible strings, they also now include power-domain-names: "a".

 - In a few integrations (possibly even just this one), the SoC
   designers chose not to take advantage of the firmware-controlled
   power gating and placed the entire GPU in a single power domain.

The latter point is the context in which we're talking about allowing a
single unnamed power domain; when we have exactly one and it covers the
entire GPU. Semantically, this is subtly different from the first point.
Is there any practical difference? Probably not, but devicetree is
supposed to describe hardware so we may as well get pedantic.

In my previous response[1], I also suggested the alternative of
specifying a name for that domain (IIRC "gpu" or "top" were my original
suggestions), but Michal (reasonably IMO) didn't see the point.

> 
> Disallowing power-domain names does magically change existing binding.

Does this refer to the explicit power-domain-names: false? If so, is
there a good/proper way to say "don't name this domain" or are we better
off falling back to the alternative of giving it a generic name that we
can enforce in bindings?

> 
> I think you should list the power-domains items explicitly for each
> variant (see any of my other standard examples how this is done, e.g.
> clock controllers).

We could re-jig the bindings to do that for sure, it wouldn't change the
semantics we currently have (prior to this patch). That would probably
be easier to scan than the current method of having "a", "b", etc. at
the top level and requiring the specific {min,max}Items by variant.

[1]: https://lore.kernel.org/r/f25c1e7f-bef2-47b1-8fa8-14c9c51087a8@imgtec.com

> 
> 
> Best regards,
> Krzysztof


-- 
Matt Coster
E: matt.coster@...tec.com


Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (237 bytes)

Powered by blists - more mailing lists