diff mbox series

[v7,22/27] dt-bindings: allwinner: add H616 DE33 mixer binding

Message ID 20250216183710.8443-23-ryan@testtoast.com (mailing list archive)
State New, archived
Headers show
Series drm: sun4i: add Display Engine 3.3 (DE33) support | expand

Commit Message

Ryan Walklin Feb. 16, 2025, 6:36 p.m. UTC
The Allwinner H616 and variants have a new display engine revision
(DE33).

The mixer configuration registers are significantly different to the DE3
and DE2 revisions, being split into separate top and display blocks,
therefore a fallback for the mixer compatible is not provided.

Add a display engine mixer binding for the DE33.

Signed-off-by: Ryan Walklin <ryan@testtoast.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Chen-Yu Tsai <wens@csie.org>

---
Changelog v2..v3:
- Separate content into three patches for three separate subsystems

Changelog v5..v6:
- increase reg maxItems to 3 and add conditional for h616-de33
---
 .../allwinner,sun8i-a83t-de2-mixer.yaml       | 21 ++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

Comments

Andre Przywara Feb. 24, 2025, 5:56 p.m. UTC | #1
On Mon, 17 Feb 2025 07:36:22 +1300
Ryan Walklin <ryan@testtoast.com> wrote:

Hi,

> The Allwinner H616 and variants have a new display engine revision
> (DE33).
> 
> The mixer configuration registers are significantly different to the DE3
> and DE2 revisions, being split into separate top and display blocks,
> therefore a fallback for the mixer compatible is not provided.
> 
> Add a display engine mixer binding for the DE33.
> 
> Signed-off-by: Ryan Walklin <ryan@testtoast.com>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> 
> ---
> Changelog v2..v3:
> - Separate content into three patches for three separate subsystems
> 
> Changelog v5..v6:
> - increase reg maxItems to 3 and add conditional for h616-de33
> ---
>  .../allwinner,sun8i-a83t-de2-mixer.yaml       | 21 ++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/allwinner,sun8i-a83t-de2-mixer.yaml b/Documentation/devicetree/bindings/display/allwinner,sun8i-a83t-de2-mixer.yaml
> index b75c1ec686ad2..274f5e6327333 100644
> --- a/Documentation/devicetree/bindings/display/allwinner,sun8i-a83t-de2-mixer.yaml
> +++ b/Documentation/devicetree/bindings/display/allwinner,sun8i-a83t-de2-mixer.yaml
> @@ -24,9 +24,11 @@ properties:
>        - allwinner,sun50i-a64-de2-mixer-0
>        - allwinner,sun50i-a64-de2-mixer-1
>        - allwinner,sun50i-h6-de3-mixer-0
> +      - allwinner,sun50i-h616-de33-mixer-0
>  
>    reg:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 3

What are those three regions? I wonder if we should have reg-names here,
to fix the order, and to document them on the way?

>  
>    clocks:
>      items:
> @@ -61,6 +63,23 @@ properties:
>      required:
>        - port@1
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - allwinner,sun50i-h616-de33-mixer-0
> +
> +    then:
> +      properties:
> +        reg:
> +          maxItems: 3

Should we override minItems here as well? I guess any driver would need
all three region to work?

Cheers,
Andre

> +
> +    else:
> +      properties:
> +        reg:
> +          maxItems: 1
> +
>  required:
>    - compatible
>    - reg
Ryan Walklin March 10, 2025, 9:30 a.m. UTC | #2
On Tue, 25 Feb 2025, at 6:56 AM, Andre Przywara wrote:

Apologies Andre, I came to review your comments on the TCON series and realised I had missed responding to this comment before sending v8. 

>> +      - allwinner,sun50i-h616-de33-mixer-0
>>  
>>    reg:
>> -    maxItems: 1
>> +    minItems: 1
>> +    maxItems: 3
>
> What are those three regions? I wonder if we should have reg-names here,
> to fix the order, and to document them on the way?

This would be the top, display and mixer groups for the DE333, and mixer for DE3 and earlier. Can certainly add in names for these. Is there any example elsewhere in the bindings to look at?

>> @@ -61,6 +63,23 @@ properties:
>>      required:
>>        - port@1
>>  
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          enum:
>> +            - allwinner,sun50i-h616-de33-mixer-0
>> +
>> +    then:
>> +      properties:
>> +        reg:
>> +          maxItems: 3
>
> Should we override minItems here as well? I guess any driver would need
> all three region to work?

This seems sensible, as you say it would always be 3 groups for the DE33.

Regards,

Ryan
Andre Przywara March 10, 2025, 10:48 a.m. UTC | #3
On Mon, 10 Mar 2025 22:30:36 +1300
"Ryan Walklin" <ryan@testtoast.com> wrote:

Hi Ryan,

> On Tue, 25 Feb 2025, at 6:56 AM, Andre Przywara wrote:
> 
> Apologies Andre, I came to review your comments on the TCON series and realised I had missed responding to this comment before sending v8. 

No worries about that!

> >> +      - allwinner,sun50i-h616-de33-mixer-0
> >>  
> >>    reg:
> >> -    maxItems: 1
> >> +    minItems: 1
> >> +    maxItems: 3  
> >
> > What are those three regions? I wonder if we should have reg-names here,
> > to fix the order, and to document them on the way?  
> 
> This would be the top, display and mixer groups for the DE333, and mixer for DE3 and earlier. Can certainly add in names for these. Is there any example elsewhere in the bindings to look at?

It's basically the same idea as for clock-names, as used in this very file
here (allwinner,sun8i-a83t-de2-mixer.yaml). You can find an explicit
example for reg-names in allwinner,sun4i-a10-mbus.yaml, for instance.
In the code you would use devm_platform_ioremap_resource_byname() then.

Cheers,
Andre

> >> @@ -61,6 +63,23 @@ properties:
> >>      required:
> >>        - port@1
> >>  
> >> +allOf:
> >> +  - if:
> >> +      properties:
> >> +        compatible:
> >> +          enum:
> >> +            - allwinner,sun50i-h616-de33-mixer-0
> >> +
> >> +    then:
> >> +      properties:
> >> +        reg:
> >> +          maxItems: 3  
> >
> > Should we override minItems here as well? I guess any driver would need
> > all three region to work?  
> 
> This seems sensible, as you say it would always be 3 groups for the DE33.
> 
> Regards,
> 
> Ryan
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/allwinner,sun8i-a83t-de2-mixer.yaml b/Documentation/devicetree/bindings/display/allwinner,sun8i-a83t-de2-mixer.yaml
index b75c1ec686ad2..274f5e6327333 100644
--- a/Documentation/devicetree/bindings/display/allwinner,sun8i-a83t-de2-mixer.yaml
+++ b/Documentation/devicetree/bindings/display/allwinner,sun8i-a83t-de2-mixer.yaml
@@ -24,9 +24,11 @@  properties:
       - allwinner,sun50i-a64-de2-mixer-0
       - allwinner,sun50i-a64-de2-mixer-1
       - allwinner,sun50i-h6-de3-mixer-0
+      - allwinner,sun50i-h616-de33-mixer-0
 
   reg:
-    maxItems: 1
+    minItems: 1
+    maxItems: 3
 
   clocks:
     items:
@@ -61,6 +63,23 @@  properties:
     required:
       - port@1
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          enum:
+            - allwinner,sun50i-h616-de33-mixer-0
+
+    then:
+      properties:
+        reg:
+          maxItems: 3
+
+    else:
+      properties:
+        reg:
+          maxItems: 1
+
 required:
   - compatible
   - reg