| Message ID | 20241105-sets-bxs-4-64-patch-v1-v1-4-4ed30e865892@imgtec.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Imagination BXS-4-64 MC1 GPU support | expand |
On Tue, Nov 05, 2024 at 03:58:10PM +0000, Matt Coster wrote: > This attribute will be required for the BXS-4-64 MC1 and will be enabled in > the DTS for the TI k3-j721s2 in a subsequent patch; add it now so > dtbs_check doesn't complain later. Sounds like the property should be made required for that integration. > > Signed-off-by: Matt Coster <matt.coster@imgtec.com> > --- > Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml > index 55f422be1bc5b7564e3e81f24c4b93857f3e12fe..8a1861ca960c48853b2ceef414f588cc343b49b2 100644 > --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml > +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml > @@ -60,6 +60,8 @@ properties: > - const: a > - const: b > > + dma-coherent: true > + > required: > - compatible > - reg > > -- > 2.47.0 >
On 05/11/2024 18:06, Conor Dooley wrote: > On Tue, Nov 05, 2024 at 03:58:10PM +0000, Matt Coster wrote: >> This attribute will be required for the BXS-4-64 MC1 and will be enabled in >> the DTS for the TI k3-j721s2 in a subsequent patch; add it now so >> dtbs_check doesn't complain later. > > Sounds like the property should be made required for that integration. This is something I went back and forth on. Where is the line drawn between things that should be enforced in bindings and things that only ever need to be specified once, so should just be left to the dt itself to be the source of truth? Having said that, I realise TI could spin a new SoC with a new dt but use the same compatible string for the GPU; the "single" source of truth then wouldn't be so single anymore. I guess by making this property required for this compatible string, we're saying any use of it must behave in exactly the same way, right? Cheers, Matt >> Signed-off-by: Matt Coster <matt.coster@imgtec.com> >> --- >> Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >> index 55f422be1bc5b7564e3e81f24c4b93857f3e12fe..8a1861ca960c48853b2ceef414f588cc343b49b2 100644 >> --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >> @@ -60,6 +60,8 @@ properties: >> - const: a >> - const: b >> >> + dma-coherent: true >> + >> required: >> - compatible >> - reg >> >> -- >> 2.47.0 >>
On Wed, Nov 06, 2024 at 10:18:06AM +0000, Matt Coster wrote: > On 05/11/2024 18:06, Conor Dooley wrote: > > On Tue, Nov 05, 2024 at 03:58:10PM +0000, Matt Coster wrote: > >> This attribute will be required for the BXS-4-64 MC1 and will be enabled in > >> the DTS for the TI k3-j721s2 in a subsequent patch; add it now so > >> dtbs_check doesn't complain later. > > > > Sounds like the property should be made required for that integration. > > This is something I went back and forth on. Where is the line drawn > between things that should be enforced in bindings and things that only > ever need to be specified once, so should just be left to the dt itself > to be the source of truth? > > Having said that, I realise TI could spin a new SoC with a new dt but > use the same compatible string for the GPU; No they can't. New SoC, new compatible. > the "single" source of truth > then wouldn't be so single anymore. I guess by making this property > required for this compatible string, we're saying any use of it must > behave in exactly the same way, right?
On 11/6/24 12:30 PM, Conor Dooley wrote: > On Wed, Nov 06, 2024 at 10:18:06AM +0000, Matt Coster wrote: >> On 05/11/2024 18:06, Conor Dooley wrote: >>> On Tue, Nov 05, 2024 at 03:58:10PM +0000, Matt Coster wrote: >>>> This attribute will be required for the BXS-4-64 MC1 and will be enabled in >>>> the DTS for the TI k3-j721s2 in a subsequent patch; add it now so >>>> dtbs_check doesn't complain later. >>> >>> Sounds like the property should be made required for that integration. >> >> This is something I went back and forth on. Where is the line drawn >> between things that should be enforced in bindings and things that only >> ever need to be specified once, so should just be left to the dt itself >> to be the source of truth? >> >> Having said that, I realise TI could spin a new SoC with a new dt but >> use the same compatible string for the GPU; > > No they can't. New SoC, new compatible. > We don't need to make a new SoC for that, our bus infra allows us to selectively include or exclude peripherals from coherency. The GPU being `dma-coherent` is selectable at boot time. Andrew >> the "single" source of truth >> then wouldn't be so single anymore. I guess by making this property >> required for this compatible string, we're saying any use of it must >> behave in exactly the same way, right?
diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml index 55f422be1bc5b7564e3e81f24c4b93857f3e12fe..8a1861ca960c48853b2ceef414f588cc343b49b2 100644 --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml @@ -60,6 +60,8 @@ properties: - const: a - const: b + dma-coherent: true + required: - compatible - reg
This attribute will be required for the BXS-4-64 MC1 and will be enabled in the DTS for the TI k3-j721s2 in a subsequent patch; add it now so dtbs_check doesn't complain later. Signed-off-by: Matt Coster <matt.coster@imgtec.com> --- Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml | 2 ++ 1 file changed, 2 insertions(+)