On Sun, 16 Feb 2025 22:27:09 +1300
Ryan Walklin <ryan@xxxxxxxxxxxxx> wrote:
Hi,
> From: Jernej Skrabec <jernej.skrabec@xxxxxxxxx>
>
> The Allwinner H616 and variants (H618, H700 and T507) have a new display
> engine variant (DE33). Support has been added to the existing DE2/DE3
> sun4i driver in a previous patch series (x). The variant is selected via
> the appropriate mixer device tree compatible string.
>
> Add the respective device-tree nodes for the DE, bus, clock and mixer to
> the H616 DTSI, and the matching SRAM section for the DE.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@xxxxxxxxx>
> Signed-off-by: Ryan Walklin <ryan@xxxxxxxxxxxxx>
> ---
> .../arm64/boot/dts/allwinner/sun50i-h616.dtsi | 56 +++++++++++++++++++
> 1 file changed, 56 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
> index cdce3dcb8ec02..ab8b70ce7df89 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
> @@ -94,6 +94,12 @@ l2_cache: l2-cache {
> };
> };
>
> + de: display-engine {
> + compatible = "allwinner,sun50i-h6-display-engine";
That should either be sun50i-h616-display-engine, or it should use a
fallback. IIUC this "device" is just something more or less artificial
that ties things together? I don't see any differences between the
latest SoCs in the driver, but still we seem to use a separate compatible
for every SoC there, which I guess is intentional?
> + allwinner,pipelines = <&mixer0>;
> + status = "disabled";
> + };
> +
> reserved-memory {
> #address-cells = <2>;
> #size-cells = <2>;
> @@ -150,6 +156,51 @@ soc {
> #size-cells = <1>;
> ranges = <0x0 0x0 0x0 0x40000000>;
>
> + bus: bus@1000000 {
> + compatible = "allwinner,sun50i-h616-de33",
> + "allwinner,sun50i-a64-de2";
> + reg = <0x1000000 0x400000>;
> + allwinner,sram = <&de3_sram 1>;
Should this label be de33_sram?
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0x1000000 0x400000>;
> +
> + display_clocks: clock@8000 {
> + compatible = "allwinner,sun50i-h616-de33-clk";
> + reg = <0x8000 0x100>;
> + clocks = <&ccu CLK_DE>, <&ccu CLK_BUS_DE>;
> + clock-names = "mod", "bus";
> + resets = <&ccu RST_BUS_DE>;
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + };
> +
> + mixer0: mixer@100000 {
> + compatible = "allwinner,sun50i-h616-de33-mixer-0";
> + reg = <0x100000 0x100000>,
> + <0x8100 0x40>,
> + <0x280000 0x20000>;
As mentioned in the binding patch, I think having reg-names here would
help to make it clearer what those regions are for.
> + clocks = <&display_clocks CLK_BUS_MIXER0>,
> + <&display_clocks CLK_MIXER0>;
> + clock-names = "bus", "mod";
> + resets = <&display_clocks RST_MIXER0>;
> + iommus = <&iommu 0>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + mixer0_out: port@1 {
> + reg = <1>;
> +
> + mixer0_out_tcon_top_mixer0: endpoint {
> + remote-endpoint = <&tcon_top_mixer0_in_mixer0>;
> + };
> + };
> + };
> + };
> + };
> +
> crypto: crypto@1904000 {
> compatible = "allwinner,sun50i-h616-crypto";
> reg = <0x01904000 0x800>;
> @@ -173,6 +224,11 @@ sram_c: sram@28000 {
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0 0x00028000 0x30000>;
> +
> + de3_sram: sram-section@0 {
de33_sram?
> + compatible = "allwinner,sun50i-a64-sram-c";
I think we need a new compatible, with the A64 as a fallback. The H6 seems
to do this as well.
Cheers,
Andre
> + reg = <0x0000 0x1e000>;
> + };
> };
> };
>
