| Message ID | 20241125-pvr-reset-v1-1-b437b8052948@linumiz.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/imagination: add reset handling | expand |
On 11/25/24 11:37 PM, Conor Dooley wrote: > On Mon, Nov 25, 2024 at 10:07:03PM +0530, Parthiban Nallathambi wrote: >> GE8300 in Allwinner A133 have reset control from the ccu. >> Add the resets property as optional one to control it. > > There's no specific compatible here for an a133, but the binding > requires one. Where is your dts patch? A133 GPU is still work in progress in both Kernel and Mesa3D. Also power domain support needs an additional driver. But reset control is independent of those changes. Should reset control needs to be clubbed GPU dts changes? Thanks, Parthiban > >> >> Signed-off-by: Parthiban Nallathambi <parthiban@linumiz.com> >> --- >> Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >> index 256e252f8087..bb607d4b1e07 100644 >> --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >> @@ -37,6 +37,9 @@ properties: >> power-domains: >> maxItems: 1 >> >> + resets: >> + maxItems: 1 >> + >> required: >> - compatible >> - reg >> >> -- >> 2.39.2 >>
On 26/11/2024 04:46, Parthiban wrote: > On 11/25/24 11:37 PM, Conor Dooley wrote: >> On Mon, Nov 25, 2024 at 10:07:03PM +0530, Parthiban Nallathambi wrote: >>> GE8300 in Allwinner A133 have reset control from the ccu. >>> Add the resets property as optional one to control it. >> >> There's no specific compatible here for an a133, but the binding >> requires one. Where is your dts patch? > A133 GPU is still work in progress in both Kernel and Mesa3D. Also power > domain support needs an additional driver. > > But reset control is independent of those changes. Should reset control > needs to be clubbed GPU dts changes? How is it independent? Are you adding it for the new platforms? If yes, then it is part of new platforms. Don't add properties which are not used. Best regards, Krzysztof
On 25/11/2024 17:37, Parthiban Nallathambi wrote: > GE8300 in Allwinner A133 have reset control from the ccu. > Add the resets property as optional one to control it. > > Signed-off-by: Parthiban Nallathambi <parthiban@linumiz.com> > --- Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. For bindings, the preferred subjects are explained here: https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters Missing prefix for the device. You are not adding reset to all GPU devices. Best regards, Krzysztof
On 11/26/24 3:02 PM, Krzysztof Kozlowski wrote: > On 26/11/2024 04:46, Parthiban wrote: >> On 11/25/24 11:37 PM, Conor Dooley wrote: >>> On Mon, Nov 25, 2024 at 10:07:03PM +0530, Parthiban Nallathambi wrote: >>>> GE8300 in Allwinner A133 have reset control from the ccu. >>>> Add the resets property as optional one to control it. >>> >>> There's no specific compatible here for an a133, but the binding >>> requires one. Where is your dts patch? >> A133 GPU is still work in progress in both Kernel and Mesa3D. Also power >> domain support needs an additional driver. >> >> But reset control is independent of those changes. Should reset control >> needs to be clubbed GPU dts changes? > How is it independent? Are you adding it for the new platforms? If yes, > then it is part of new platforms. Don't add properties which are not used. Thanks for the review. Will address the points together when adding support for GE8300 GPU. Thanks, Parthiban > > Best regards, > Krzysztof
diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml index 256e252f8087..bb607d4b1e07 100644 --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml @@ -37,6 +37,9 @@ properties: power-domains: maxItems: 1 + resets: + maxItems: 1 + required: - compatible - reg
GE8300 in Allwinner A133 have reset control from the ccu. Add the resets property as optional one to control it. Signed-off-by: Parthiban Nallathambi <parthiban@linumiz.com> --- Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml | 3 +++ 1 file changed, 3 insertions(+)