Hi, > Am 25.10.2026 um 20:16 schrieb Olivier Louvignes <[email protected]>: > > The X1600's display controller fetches frames through descriptors and > keeps its interface and timings in register blocks of their own (X1600 > programming manual, chapter 10), so it shares nothing with the > controllers ingenic,lcd.yaml describes. Its registers are those of the > X2000's display controller without the composer. > > Signed-off-by: Olivier Louvignes <[email protected]> > --- > .../bindings/display/ingenic,x1600-lcd.yaml | 70 +++++++++++++++++++ > MAINTAINERS | 5 ++ > 2 files changed, 75 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/ingenic,x1600-lcd.yaml > > diff --git a/Documentation/devicetree/bindings/display/ingenic,x1600-lcd.yaml > b/Documentation/devicetree/bindings/display/ingenic,x1600-lcd.yaml > new file mode 100644 > index 000000000000..171536c78f4b > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/ingenic,x1600-lcd.yaml > @@ -0,0 +1,70 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/ingenic,x1600-lcd.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Ingenic X1600 LCD controller > + > +maintainers: > + - Olivier Louvignes <[email protected]> > + > +description: > + The display controller of the Ingenic X1600 SoC. It fetches frames through > + descriptors in memory and drives a parallel RGB panel or a smart panel; > + its registers have nothing in common with the controllers described in > + ingenic,lcd.yaml. > + > +properties: > + $nodename: > + pattern: "^lcd-controller@[0-9a-f]+$" > + > + compatible: > + const: ingenic,x1600-lcd > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + clocks: > + items: > + - description: The pixel clock, whose gate also gates the controller > + > + clock-names: > + items: > + - const: lcd > + > + port: > + $ref: /schemas/graph.yaml#/properties/port > + description: The output, to the panel or bridge. > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + - clock-names > + - port > + > +additionalProperties: false > + > +examples: > + - | > + lcd-controller@13050000 { > + compatible = "ingenic,x1600-lcd"; > + reg = <0x13050000 0x10000>; > + > + interrupt-parent = <&intc>; > + interrupts = <31>; > + > + clocks = <&cgu 16>; > + clock-names = "lcd"; > + > + port { > + lcd_out: endpoint { > + remote-endpoint = <&panel_in>; > + }; > + }; > + }; > diff --git a/MAINTAINERS b/MAINTAINERS > index a08087fb6e67..98d46b1cc029 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -12927,6 +12927,11 @@ F: include/linux/mfd/ingenic-tcu.h > F: sound/soc/codecs/jz47* > F: sound/soc/jz4740/ > > +INGENIC X1600 DRM DRIVER > +M: Olivier Louvignes <[email protected]> > +S: Maintained > +F: Documentation/devicetree/bindings/display/ingenic,x1600-lcd.yaml > + > INJOINIC IP5xxx POWER BANK IC DRIVER > M: Samuel Holland <[email protected]> > S: Maintained > -- > 2.39.5
I've looked through the implementation and compared it with the hardware architecture and programming manuals for the Ingenic X-series (x1600, x2000, x2600). It is indeed a completely different display controller as found in the jz4 series so a new driver is reasonable. Here is some architectural feedback regarding the naming convention, compatibility, and why we need to future-proof this layout: 1) IP Naming and Architecture (LCD vs. DPU): The manual for the x1600 is a bit ambiguous, but the x2000 and x2600 documentation identifies this block as the "DPU (Display Processing Unit) Display Controller" (located at base address 0x13050000 on all three SoC). Calling the core driver or compatible string "x1600-lcd" is too restrictive. This IP block is a generic display engine designed to drive highly diverse protocols across the SoC generations: * x1000: MIPI-DBI (SLCD) * x1830: MIPI-DPI (TFT), MIPI-DBI (SLCD) * x1600: MIPI-DSI, MIPI-DPI (TFT) * x2000: MIPI-DSI, MIPI-DPI (TFT), MIPI-DBI (SLCD) * x2600: MIPI-DSI, MIPI-DPI (TFT), MIPI-DBI (SLCD), LVDS (NOTE: this does not match the presence of physical drivers for these protocols) Recommendation: We should decouple the core controller logic from the physical signaling interfaces. The driver framework needs to be prepared for these upcoming formats (alongside features like the x2600 rotation engine). Please update the compatible string to "ingenic,x1600-dpu" (with "ingenic,dpu" as the fallback) and rename the file to ingenic-dpu.c to reflect that this is a scalable DPU driver not limited to "LCD". 2) Vendor Kernel: A look at the vendor's v6.6 downstream kernel confirms this architecture. While their ingenicfb.c and ingenic_lcd_6800.c drivers are tailored for specific downstream use cases and non-DRM subsystems (old style fbdev), they explicitly reference the "DPU" and hook the x2600 rotation module into it. Adopting the generic ingenic-dpu naming upfront prevents us from breaking bindings or duplicating code when adding x2000/x2600 support later. 3) Testing and Validation Feedback: I've looked into integrating your patch series (including the pinctrl and panel drivers) on our LX16 platform. On this specific hardware, the LCD interface is only routed to internal test points. Therefore, I attempted to verify the pipeline by using a dummy/no-op hack for your hiby,lg35583 panel driver. The plan was to use a scope to watch framebuffer content on the test points. However, this approach triggered a kernel lockup during boot right after successfully running lg35583_get_modes(). Due to time constraints, I have not debuged the root cause of this lockup, so you can ignore it if the panel works for you. Nevertheless, looking at the code, the structural alignment of the components—including the core logic, the panel driver baseline, and the proposed pinctrl extension (lcd-18bit-msb added to x1600_lcd_groups[])—looks very promising. Once the naming is refactored to align with the generic DPU architecture, this looks like a very solid foundation to base future work on. BR and thanks, Nikolaus
